Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api_tests/preprints/views/test_preprint_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
73 changes: 73 additions & 0 deletions osf/management/commands/fix_versioned_guids.py
Original file line number Diff line number Diff line change
@@ -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}"
)
72 changes: 39 additions & 33 deletions osf/models/preprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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):
Expand All @@ -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
Expand All @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions tests/test_preprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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
Comment on lines +2564 to +2565
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another case that I forgot to ask during my first review. And this is probably a product question too.

  • Can you verify that rejected preprints will keep the version number? (Rejection only happens for pre-mod).
  • Does product really want the behavior that non-version guid leads to rejected preprints? Or should we change the version to point back to an earlier "latest version" upon rejection?


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
Expand Down
Loading