diff --git a/api/preprints/views.py b/api/preprints/views.py index efdb351f42a..9caba1c43ff 100644 --- a/api/preprints/views.py +++ b/api/preprints/views.py @@ -60,6 +60,7 @@ PreprintFilesPermissions, PreprintInstitutionPermissionList, ) +from api.providers.workflows import Workflows from api.nodes.permissions import ContributorOrPublic from api.base.permissions import WriteOrPublicForRelationshipInstitutions from api.requests.permissions import PreprintRequestPermission @@ -75,7 +76,10 @@ class PreprintOldVersionsImmutableMixin: """ def update(self, request, *args, **kwargs): preprint = self.get_preprint(check_object_permissions=False) - if preprint.is_latest_version or preprint.machine_state == 'initial': + is_pre_mod_pending = ( + preprint.provider.reviews_workflow == Workflows.PRE_MODERATION.value and preprint.machine_state == 'pending' + ) + if preprint.is_latest_version or preprint.machine_state == 'initial' or is_pre_mod_pending: return super().update(request, *args, **kwargs) message = f'User can not edit previous versions of a preprint: [_id={preprint._id}]' sentry.log_message(message) @@ -83,7 +87,10 @@ def update(self, request, *args, **kwargs): def create(self, request, *args, **kwargs): preprint = self.get_preprint(check_object_permissions=False) - if preprint.is_latest_version or preprint.machine_state == 'initial': + is_pre_mod_pending = ( + preprint.provider.reviews_workflow == Workflows.PRE_MODERATION.value and preprint.machine_state == 'pending' + ) + if preprint.is_latest_version or preprint.machine_state == 'initial' or is_pre_mod_pending: return super().create(request, *args, **kwargs) message = f'User can not edit previous versions of a preprint: [_id={preprint._id}]' sentry.log_message(message) @@ -91,7 +98,10 @@ def create(self, request, *args, **kwargs): def destroy(self, request, *args, **kwargs): preprint = self.get_preprint(check_object_permissions=False) - if preprint.is_latest_version or preprint.machine_state == 'initial': + is_pre_mod_pending = ( + preprint.provider.reviews_workflow == Workflows.PRE_MODERATION.value and preprint.machine_state == 'pending' + ) + if preprint.is_latest_version or preprint.machine_state == 'initial' or is_pre_mod_pending: return super().destroy(request, *args, **kwargs) message = f'User can not edit previous versions of a preprint: [_id={preprint._id}]' sentry.log_message(message) diff --git a/api/users/views.py b/api/users/views.py index 296826ca48b..f3bbfc172cf 100644 --- a/api/users/views.py +++ b/api/users/views.py @@ -407,7 +407,7 @@ def get_default_queryset(self): # Permissions on the list objects are handled by the query default_qs = Preprint.objects.filter(_contributors__guids___id=target_user._id).exclude(machine_state='initial') - return self.preprints_queryset(default_qs, auth_user, allow_contribs=False) + return self.preprints_queryset(default_qs, auth_user, allow_contribs=False, latest_only=True) def get_queryset(self): return self.get_queryset_from_request() diff --git a/osf/models/base.py b/osf/models/base.py index 9e4136565b0..594522ed63a 100644 --- a/osf/models/base.py +++ b/osf/models/base.py @@ -524,9 +524,8 @@ class Meta: versioned_guids = GenericRelation('GuidVersionsThrough', related_name='referent', related_query_name='referents') - @property + @cached_property def _id(self): - # TODO: maybe a cached property? try: versioned_guid = self.versioned_guids if not versioned_guid.exists(): @@ -551,9 +550,9 @@ def _id(self, value): _primary_key = _id - @property + @cached_property def version(self): - # TODO: maybe a cached property? + # Once assigned, version number never changes return self.versioned_guids.first().version @classmethod diff --git a/osf/models/preprint.py b/osf/models/preprint.py index fbeb0645470..d5066dc6088 100644 --- a/osf/models/preprint.py +++ b/osf/models/preprint.py @@ -4,7 +4,7 @@ import re from dirtyfields import DirtyFieldsMixin -from django.db import models +from django.db import models, IntegrityError from django.db.models import Q from django.utils import timezone from django.contrib.contenttypes.fields import GenericRelation @@ -296,7 +296,7 @@ def create(cls, provider, title, creator, description): creator=creator, description=description, ) - preprint.save() + preprint.save(guid_ready=False) # Step 2: Create the base guid obj base_guid_obj = Guid.objects.create() base_guid_obj.referent = preprint @@ -312,6 +312,7 @@ def create(cls, provider, title, creator, description): guid=base_guid_obj ) versioned_guid.save() + preprint.save(guid_ready=True, first_save=True) return preprint @@ -412,7 +413,7 @@ def create_version(cls, create_from_guid, auth): creator=auth.user, description=latest_version.description, ) - preprint.save() + preprint.save(guid_ready=False) # Create a new entry in the `GuidVersionsThrough` table to store version information, which must happen right # after the first `.save()` of the new preprint version object, which enables `preprint._id` to be computed. guid_version = GuidVersionsThrough( @@ -423,6 +424,7 @@ def create_version(cls, create_from_guid, auth): guid=guid_obj ) guid_version.save() + preprint.save(guid_ready=True, first_save=True) # Add contributors for contributor_info in latest_version.contributor_set.exclude(user=latest_version.creator).values('visible', 'user_id', '_order'): @@ -822,9 +824,33 @@ def get_doi_client(self): return None def save(self, *args, **kwargs): - # TODO: document how `.save()` is customized for preprint - # TODO: insert a bare-minimum condition for saving preprint before guid and/or versioned guid is created - first_save = not bool(self.pk) + """Customize preprint save process, which has three steps. + + 1. Initial: this save happens before guid and versioned guid are created for the preprint; this save + creates the pk; after this save, none of `guids`, `versioned_guids` or `._id` is available. + 2. First: this save happens and must happen right after versioned guid have been created; this is the + same "first save" as it was before preprint became versioned; the only change is that `pk` already exists + 3. This is the case for all subsequent saves after initial and first save. + + Note: When creating a preprint or new version , must use Preprint.create() or Preprint.create_version() + respectively, which handles the save process automatically. + """ + initial_save = not kwargs.pop('guid_ready', True) + if initial_save: + # Save when guid and versioned guid are not ready + return super().save(*args, **kwargs) + + # Preprint must have PK and _id set before continue + if not bool(self.pk): + err_msg = 'Preprint must have pk!' + sentry.log_message(err_msg) + raise IntegrityError(err_msg) + if not self._id: + err_msg = 'Preprint must have _id!' + sentry.log_message(err_msg) + raise IntegrityError(err_msg) + + first_save = kwargs.pop('first_save', False) saved_fields = self.get_dirty_fields() or [] if not first_save and ('ever_public' in saved_fields and saved_fields['ever_public']): diff --git a/osf/utils/machines.py b/osf/utils/machines.py index 8e4b8f07338..1b286f3cee3 100644 --- a/osf/utils/machines.py +++ b/osf/utils/machines.py @@ -123,7 +123,17 @@ def save_changes(self, ev): self.machineable.save() def resubmission_allowed(self, ev): - return self.machineable.provider.reviews_workflow == Workflows.PRE_MODERATION.value + """Allow resubmission 1) if the preprint uses the PRE_MODERATION workflow, or 2) if it uses the POST_MODERATION + workflow and is in a pending state. + """ + workflow = self.machineable.provider.reviews_workflow + result = any( + [ + workflow == Workflows.PRE_MODERATION.value, + workflow == Workflows.POST_MODERATION.value and self.machineable.machine_state == 'pending' + ] + ) + return result def perform_withdraw(self, ev): self.machineable.date_withdrawn = self.action.created if self.action is not None else timezone.now() diff --git a/osf_tests/factories.py b/osf_tests/factories.py index 7ec4f9e3eb8..9ae6a84db2c 100644 --- a/osf_tests/factories.py +++ b/osf_tests/factories.py @@ -727,29 +727,18 @@ def _build(cls, target_class, *args, **kwargs): @classmethod def _create(cls, target_class, *args, **kwargs): + update_task_patcher = mock.patch('website.preprints.tasks.on_preprint_updated.si') update_task_patcher.start() - finish = kwargs.pop('finish', True) - set_doi = kwargs.pop('set_doi', True) - is_published = kwargs.pop('is_published', True) + # Step 1: create prerpint, guid and versioned guid instance = cls._build(target_class, *args, **kwargs) - file_size = kwargs.pop('file_size', 1337) - - doi = kwargs.pop('doi', None) - license_details = kwargs.pop('license_details', None) - filename = kwargs.pop('filename', None) or 'preprint_file.txt' - subjects = kwargs.pop('subjects', None) or [[SubjectFactory()._id]] - instance.article_doi = doi - - user = kwargs.pop('creator', None) or instance.creator - instance.save() + instance.save(guid_ready=False) guid = models.Guid.objects.create( referent=instance, content_type=ContentType.objects.get_for_model(instance), object_id=instance.pk, ) - models.GuidVersionsThrough.objects.create( referent=instance, content_type=ContentType.objects.get_for_model(instance), @@ -757,6 +746,21 @@ def _create(cls, target_class, *args, **kwargs): version=1, guid=guid ) + instance.save(guid_ready=True, first_save=True) + + # Step 2: finish preprint creation work flow + finish = kwargs.pop('finish', True) + set_doi = kwargs.pop('set_doi', True) + user = kwargs.pop('creator', None) or instance.creator + is_published = kwargs.pop('is_published', True) + file_size = kwargs.pop('file_size', 1337) + doi = kwargs.pop('doi', None) + license_details = kwargs.pop('license_details', None) + filename = kwargs.pop('filename', None) or 'preprint_file.txt' + subjects = kwargs.pop('subjects', None) or [[SubjectFactory()._id]] + instance.article_doi = doi + machine_state = kwargs.pop('machine_state', 'initial') + preprint_file = OsfStorageFile.create( target_object_id=instance.id, target_content_type=ContentType.objects.get_for_model(instance), @@ -764,7 +768,7 @@ def _create(cls, target_class, *args, **kwargs): name=filename, materialized_path=f'/{filename}') - instance.machine_state = kwargs.pop('machine_state', 'initial') + instance.machine_state = machine_state preprint_file.save() from addons.osfstorage import settings as osfstorage_settings @@ -818,12 +822,9 @@ def create_version(cls, create_from, creator=None, final_machine_state='accepted provider=latest_version.provider, title=latest_version.title, description=latest_version.description, - creator=creator, - node=latest_version.node, + creator=creator ) - instance.machine_state = 'initial' - instance.provider = latest_version.provider - instance.save() + instance.save(guid_ready=False) models.GuidVersionsThrough.objects.create( referent=instance, content_type=ContentType.objects.get_for_model(instance), @@ -831,6 +832,9 @@ def create_version(cls, create_from, creator=None, final_machine_state='accepted guid=guid_obj, version=last_version_number + 1 ) + instance.machine_state = 'initial' + instance.node = latest_version.node + instance.save(guid_ready=True, first_save=True) # Prepare and add file filename = f'preprint_file_{last_version_number + 1}.txt' diff --git a/osf_tests/test_elastic_search.py b/osf_tests/test_elastic_search.py index b2815fc169f..56c42391095 100644 --- a/osf_tests/test_elastic_search.py +++ b/osf_tests/test_elastic_search.py @@ -524,7 +524,7 @@ def setUp(self): search.delete_index(elastic_search.INDEX) search.create_index(elastic_search.INDEX) self.user = factories.UserFactory(fullname='John Deacon') - self.preprint = Preprint( + self.preprint = Preprint.create( title='Red Special', description='We are the champions', creator=self.user, diff --git a/tests/test_preprints.py b/tests/test_preprints.py index 7c92ad9b8e4..522ff146718 100644 --- a/tests/test_preprints.py +++ b/tests/test_preprints.py @@ -282,9 +282,10 @@ class TestPreprintCreation: def test_creator_is_added_as_contributor(self, fake): user = UserFactory() - preprint = Preprint( + preprint = Preprint.create( title=fake.bs(), creator=user, + description=fake.bs(), provider=PreprintProviderFactory() ) preprint.save() @@ -297,9 +298,10 @@ def test_creator_is_added_as_contributor(self, fake): def test_default_region_set_to_user_settings_osfstorage_default(self, fake): user = UserFactory() - preprint = Preprint( + preprint = Preprint.create( title=fake.bs, creator=user, + description=fake.bs(), provider=PreprintProviderFactory() ) preprint.save() @@ -308,9 +310,10 @@ def test_default_region_set_to_user_settings_osfstorage_default(self, fake): def test_root_folder_created_automatically(self, fake): user = UserFactory() - preprint = Preprint( + preprint = Preprint.create( title=fake.bs, creator=user, + description=fake.bs(), provider=PreprintProviderFactory() ) preprint.save()