[ENG-8832] Most recent numbered version of a preprint fails to show for unversioned URL (#11315)

* fix versioned guids resolving to the last published version instead of last version number

* fix migration job

* handle initial review_state in fix_versioned_guids job

* adjusted tests to pass with new guid resolve strategy

* handle the case for withdrawn/rejected preprints

* fix missing.all()

Author: opaduchak

api_tests/preprints/views/test_preprint_versions.py

@@ -807,7 +807,7 @@ def test_list_versions_pre_mod(self):
 
         # Pre moderation V4 (Pending)
         pre_mod_preprint_v4 = PreprintFactory.create_version(
-            create_from=pre_mod_preprint_v2,
+            create_from=pre_mod_preprint_v3,
             creator=self.creator,
             final_machine_state='initial',
             is_published=False,

osf/management/commands/fix_versioned_guids.py

@@ -0,0 +1,73 @@
+import logging
+
+from django.contrib.contenttypes.models import ContentType
+from django.core.management.base import BaseCommand
+from django.db import transaction
+from django.db.models import Prefetch
+
+from osf.models import GuidVersionsThrough, Guid, Preprint
+from osf.utils.workflows import ReviewStates
+
+logger = logging.getLogger(__name__)
+
+
+class Command(BaseCommand):
+    help = 'Fix'
+
+    def add_arguments(self, parser):
+        parser.add_argument(
+            '--dry-run',
+            action='store_true',
+            dest='dry_run',
+            help='Run the command without saving changes',
+        )
+
+    @transaction.atomic
+    def handle(self, *args, **options):
+        dry_run = options.get('dry_run', False)
+        fix_versioned_guids(dry_run=dry_run)
+        if dry_run:
+            transaction.set_rollback(True)
+
+
+def fix_versioned_guids(dry_run: bool):
+    content_type = ContentType.objects.get_for_model(Preprint)
+    versions_queryset = GuidVersionsThrough.objects.order_by('-version')
+    processed_count = 0
+    updated_count = 0
+    skipped_count = 0
+    errors_count = 0
+    for guid in (
+        Guid.objects.filter(content_type=content_type)
+        .prefetch_related(Prefetch('versions', queryset=versions_queryset))
+        .iterator(chunk_size=500)
+    ):
+        processed_count += 1
+        if not guid.versions:
+            skipped_count += 1
+            continue
+        for version in guid.versions.all():
+            last_version_object_id = version.object_id
+            if guid.object_id == last_version_object_id:
+                skipped_count += 1
+                break
+            if version.referent.machine_state not in (ReviewStates.INITIAL.value, ReviewStates.REJECTED.value, ReviewStates.WITHDRAWN.value):
+                continue
+            try:
+                guid.object_id = last_version_object_id
+                guid.referent = version.referent
+                if not dry_run:
+                    guid.save()
+                updated_count += 1
+            except Exception as e:
+                logger.error(f"Error occurred during patching {guid._id=}", exc_info=e)
+                errors_count += 1
+
+    if dry_run:
+        logger.error(
+            f"Processed: {processed_count}, Would update: {updated_count}, Skipped: {skipped_count}, Errors: {errors_count}"
+        )
+    else:
+        logger.error(
+            f"Processed: {processed_count}, Updated: {updated_count}, Skipped: {skipped_count}, Errors: {errors_count}"
+        )

osf/models/preprint.py

@@ -59,7 +59,6 @@
 )
 from django.contrib.postgres.fields import ArrayField
 from api.share.utils import update_share
-from api.providers.workflows import Workflows
 
 logger = logging.getLogger(__name__)
 
@@ -1679,27 +1678,13 @@ def run_submit(self, user):
             user: The user triggering this transition.
         """
         ret = super().run_submit(user=user)
-        provider = self.provider
-        reviews_workflow = provider.reviews_workflow
-        # Only post moderation is relevant for Preprint, and hybrid moderation is included for integrity purpose.
-        need_guid_update = any(
-            [
-                reviews_workflow == Workflows.POST_MODERATION.value,
-                reviews_workflow == Workflows.HYBRID_MODERATION.value and
-                any([
-                    provider.get_group('moderator') in user.groups.all(),
-                    provider.get_group('admin') in user.groups.all()
-                ])
-            ]
-        )
-        # Only update the base guid obj to refer to the new version 1) if the provider is post-moderation, or 2) if the
-        # provider is hybrid-moderation and if the user who submits the preprint is a moderator or admin.
-        if need_guid_update:
-            base_guid_obj = self.versioned_guids.first().guid
-            base_guid_obj.referent = self
-            base_guid_obj.object_id = self.pk
-            base_guid_obj.content_type = ContentType.objects.get_for_model(self)
-            base_guid_obj.save()
+
+        base_guid_obj = self.versioned_guids.first().guid
+        base_guid_obj.referent = self
+        base_guid_obj.object_id = self.pk
+        base_guid_obj.content_type = ContentType.objects.get_for_model(self)
+        base_guid_obj.save()
+
         return ret
 
     def run_accept(self, user, comment, **kwargs):
@@ -1711,14 +1696,6 @@ def run_accept(self, user, comment, **kwargs):
             comment: Text describing why.
         """
         ret = super().run_accept(user=user, comment=comment, **kwargs)
-        reviews_workflow = self.provider.reviews_workflow
-        if reviews_workflow == Workflows.PRE_MODERATION.value or reviews_workflow == Workflows.HYBRID_MODERATION.value:
-            base_guid_obj = self.versioned_guids.first().guid
-            base_guid_obj.referent = self
-            base_guid_obj.object_id = self.pk
-            base_guid_obj.content_type = ContentType.objects.get_for_model(self)
-            base_guid_obj.save()
-
         versioned_guid = self.versioned_guids.first()
         if versioned_guid.is_rejected:
             versioned_guid.is_rejected = False
@@ -1734,9 +1711,38 @@ def run_reject(self, user, comment):
             comment: Text describing why.
         """
         ret = super().run_reject(user=user, comment=comment)
-        versioned_guid = self.versioned_guids.first()
-        versioned_guid.is_rejected = True
-        versioned_guid.save()
+        current_version_guid = self.versioned_guids.first()
+        current_version_guid.is_rejected = True
+        current_version_guid.save()
+
+        self.rollback_main_guid()
+
+        return ret
+
+    def rollback_main_guid(self):
+        """Reset main guid to resolve to last versioned guid which is not withdrawn/rejected if there is one.
+        """
+        guid = None
+        for version in self.versioned_guids.all()[1:]:  # skip first guid as it refers to current version
+            guid = version.guid
+            if guid.referent.machine_state not in (ReviewStates.REJECTED, ReviewStates.WITHDRAWN):
+                break
+        if guid:
+            guid.referent = self
+            guid.object_id = self.pk
+            guid.content_type = ContentType.objects.get_for_model(self)
+            guid.save()
+
+    def run_withdraw(self, user, comment):
+        """Override `ReviewableMixin`/`MachineableMixin`.
+        Run the 'withdraw' state transition and create a corresponding Action.
+
+        Params:
+            user: The user triggering this transition.
+            comment: Text describing why.
+        """
+        ret = super().run_withdraw(user=user, comment=comment)
+        self.rollback_main_guid()
         return ret
 
 @receiver(post_save, sender=Preprint)

tests/test_preprints.py

@@ -2518,8 +2518,8 @@ def test_unpublished_version_pre_mod_submit_and_accept(self, preprint_pre_mod, c
         assert new_version.is_published is False
         assert new_version.machine_state == ReviewStates.PENDING.value
         guid_obj = new_version.get_guid()
-        assert guid_obj.object_id == preprint_pre_mod.pk
-        assert guid_obj.referent == preprint_pre_mod
+        assert guid_obj.object_id == new_version.pk
+        assert guid_obj.referent == new_version
         assert guid_obj.content_type == ContentType.objects.get_for_model(Preprint)
 
         new_version.run_accept(moderator, 'comment')
@@ -2552,17 +2552,17 @@ def test_new_version_pre_mod_submit_and_reject(self, preprint_pre_mod, creator,
         assert new_version.is_published is False
         assert new_version.machine_state == ReviewStates.PENDING.value
         guid_obj = new_version.get_guid()
-        assert guid_obj.object_id == preprint_pre_mod.pk
-        assert guid_obj.referent == preprint_pre_mod
+        assert guid_obj.object_id == new_version.pk
+        assert guid_obj.referent == new_version
         assert guid_obj.content_type == ContentType.objects.get_for_model(Preprint)
 
         new_version.run_reject(moderator, 'comment')
         assert new_version.is_published is False
         assert new_version.machine_state == ReviewStates.REJECTED.value
         assert new_version.versioned_guids.first().is_rejected is True
         guid_obj = new_version.get_guid()
-        assert guid_obj.object_id == preprint_pre_mod.pk
-        assert guid_obj.referent == preprint_pre_mod
+        assert guid_obj.object_id == new_version.pk
+        assert guid_obj.referent == new_version
 
     def test_unpublished_preprint_post_mod_submit_and_accept(self, unpublished_preprint_post_mod, creator, moderator):
         assert unpublished_preprint_post_mod.is_published is False