diff --git a/api/preprints/serializers.py b/api/preprints/serializers.py index f46c8d7d4df..fab7dcf2c03 100644 --- a/api/preprints/serializers.py +++ b/api/preprints/serializers.py @@ -38,7 +38,7 @@ from api.base.metrics import MetricsSerializerMixin from api.institutions.utils import update_institutions_if_user_associated from api.taxonomies.serializers import TaxonomizableSerializerMixin -from framework.exceptions import PermissionsError, PendingPreprintVersionExists +from framework.exceptions import PermissionsError, UnpublishedPendingPreprintVersionExists from website.project import signals as project_signals from osf.exceptions import NodeStateError, PreprintStateError from osf.models import ( @@ -591,7 +591,7 @@ def create(self, validated_data): preprint, update_data = Preprint.create_version(create_from_guid, auth) except PermissionsError: raise PermissionDenied(detail='You must have admin permissions to create new version.') - except PendingPreprintVersionExists: + except UnpublishedPendingPreprintVersionExists: raise Conflict(detail='Before creating a new version, you must publish the latest version.') # TODO add more checks return self.update(preprint, update_data) diff --git a/framework/exceptions/__init__.py b/framework/exceptions/__init__.py index 1706828be2d..328dbd1fe13 100644 --- a/framework/exceptions/__init__.py +++ b/framework/exceptions/__init__.py @@ -111,7 +111,7 @@ def __init__(self, code, message=None, redirect_url=None, data=None, template=No self.template = template super().__init__(code, message, redirect_url, data) -class PendingPreprintVersionExists(FrameworkError): - """Raised if an pending preprint version exists +class UnpublishedPendingPreprintVersionExists(FrameworkError): + """Raised if an unpublished pending preprint version exists """ pass diff --git a/osf/models/preprint.py b/osf/models/preprint.py index ad0397d7eeb..2c886987612 100644 --- a/osf/models/preprint.py +++ b/osf/models/preprint.py @@ -16,7 +16,7 @@ from django.db.models.signals import post_save from framework.auth import Auth -from framework.exceptions import PermissionsError, PendingPreprintVersionExists +from framework.exceptions import PermissionsError, UnpublishedPendingPreprintVersionExists from framework.auth import oauth_scopes from rest_framework.exceptions import NotFound @@ -312,50 +312,49 @@ def create(cls, provider, title, creator, description): return preprint - def has_pending_version(self): - guid = self.guids.first() - last_version = guid.versions.order_by('-version').first().referent - return last_version.machine_state == 'pending' + def has_unpublished_pending_version(self): + guid_obj = self.guids.first() + last_not_rejected_version = guid_obj.versions.filter(is_rejected=False).order_by('-version').first().referent + return last_not_rejected_version.machine_state == 'pending' and not last_not_rejected_version.is_published @classmethod def create_version(cls, create_from_guid, auth): - base_guid = Guid.load(create_from_guid) - source_preprint = cls.load(base_guid._id) + + guid_obj = Guid.load(create_from_guid) + # Guid object always points to the latest ever-published (i.e. either published or withdrawn) version + source_preprint = cls.load(guid_obj._id) if not source_preprint: raise NotFound if not source_preprint.has_permission(auth.user, ADMIN): raise PermissionsError - if source_preprint.has_pending_version(): - raise PendingPreprintVersionExists - - last_version = base_guid.versions.order_by('-version').first().version - data_for_update = {} - data_for_update['tags'] = source_preprint.tags.all().values_list('name', flat=True) + if source_preprint.has_unpublished_pending_version(): + raise UnpublishedPendingPreprintVersionExists + # Note: last version may not be the latest version + last_version_number = guid_obj.versions.order_by('-version').first().version + + # Prepare data to clone/update + data_for_update = { + 'subjects': [[el] for el in source_preprint.subjects.all().values_list('_id', flat=True)], + 'tags': source_preprint.tags.all().values_list('name', flat=True), + 'original_publication_date': source_preprint.original_publication_date, + 'custom_publication_citation': source_preprint.custom_publication_citation, + 'article_doi': source_preprint.article_doi, 'has_coi': source_preprint.has_coi, + 'conflict_of_interest_statement': source_preprint.conflict_of_interest_statement, + 'has_data_links': source_preprint.has_data_links, 'why_no_data': source_preprint.why_no_data, + 'data_links': source_preprint.data_links, + 'has_prereg_links': source_preprint.has_prereg_links, + 'why_no_prereg': source_preprint.why_no_prereg, 'prereg_links': source_preprint.prereg_links, + } if source_preprint.license: data_for_update['license_type'] = source_preprint.license.node_license data_for_update['license'] = { 'copyright_holders': source_preprint.license.copyright_holders, 'year': source_preprint.license.year } - - data_for_update['subjects'] = [[el] for el in source_preprint.subjects.all().values_list('_id', flat=True)] - - data_for_update['original_publication_date'] = source_preprint.original_publication_date - data_for_update['custom_publication_citation'] = source_preprint.custom_publication_citation - data_for_update['article_doi'] = source_preprint.article_doi - - data_for_update['has_coi'] = source_preprint.has_coi - data_for_update['conflict_of_interest_statement'] = source_preprint.conflict_of_interest_statement - data_for_update['has_data_links'] = source_preprint.has_data_links - data_for_update['why_no_data'] = source_preprint.why_no_data - data_for_update['data_links'] = source_preprint.data_links - data_for_update['has_prereg_links'] = source_preprint.has_prereg_links - data_for_update['why_no_prereg'] = source_preprint.why_no_prereg - data_for_update['prereg_links'] = source_preprint.prereg_links - if source_preprint.node: data_for_update['node'] = source_preprint.node + # Create a preprint obj for the new version preprint = cls( provider=source_preprint.provider, title=source_preprint.title, @@ -363,26 +362,29 @@ def create_version(cls, create_from_guid, auth): description=source_preprint.description, ) preprint.save() - # add contributors + + # Add contributors for contributor_info in source_preprint.contributor_set.exclude(user=source_preprint.creator).values('visible', 'user_id', '_order'): preprint.contributor_set.create(**{**contributor_info, 'preprint_id': preprint.id}) - # add affiliated institutions + # Add affiliated institutions for institution in source_preprint.affiliated_institutions.all(): preprint.add_affiliated_institution(institution, auth.user, ignore_user_affiliation=True) + # Update Guid obj to point to the new version if there is no moderation if not preprint.provider.reviews_workflow: - base_guid.referent = preprint - base_guid.object_id = preprint.pk - base_guid.content_type = ContentType.objects.get_for_model(preprint) - base_guid.save() + guid_obj.referent = preprint + guid_obj.object_id = preprint.pk + guid_obj.content_type = ContentType.objects.get_for_model(preprint) + guid_obj.save() + # Create an entry in the `GuidVersionsThrough` table guid_version = GuidVersionsThrough( - referent=base_guid.referent, - object_id=base_guid.object_id, - content_type=base_guid.content_type, - version=last_version + 1, - guid=base_guid + referent=guid_obj.referent, + object_id=guid_obj.object_id, + content_type=guid_obj.content_type, + version=last_version_number + 1, + guid=guid_obj ) guid_version.save() diff --git a/osf_tests/factories.py b/osf_tests/factories.py index c53260a508e..ffeb48075e9 100644 --- a/osf_tests/factories.py +++ b/osf_tests/factories.py @@ -4,11 +4,11 @@ import datetime from random import randint from unittest import mock + from factory import SubFactory from factory.fuzzy import FuzzyDateTime, FuzzyAttribute, FuzzyChoice from unittest.mock import patch, Mock -import factory import pytz import factory.django from factory.django import DjangoModelFactory @@ -794,41 +794,36 @@ def _create(cls, target_class, *args, **kwargs): instance.save() return instance + # TODO: maybe we should use model's `create_version` and use api's update_data @classmethod - def create_version(cls, *args, **kwargs): - - update_task_patcher = mock.patch('website.preprints.tasks.on_preprint_updated.si') - update_task_patcher.start() + def create_version(cls, create_from, creator=None, final_machine_state='accepted', is_published=True, set_doi=True): - source_preprint = kwargs.pop('preprint') - # TODO: assert not source_preprint.has_unpublished_pending_version() - guid_obj = source_preprint.guids.first() - latest_version = guid_obj.versions.filter(is_rejected=False).order_by('-version').first().referent + # Run a few checks + assert final_machine_state in ['pending', 'accepted'] + assert create_from and not create_from.has_unpublished_pending_version() + guid_obj = create_from.guids.first() + latest_version = guid_obj.referent assert latest_version.is_latest_version last_version_number = guid_obj.versions.order_by('-version').first().version + if not creator: + creator = latest_version.creator - finish = kwargs.pop('finish', True) - set_doi = kwargs.pop('set_doi', True) - is_published = kwargs.pop('is_published', True) - # TODO: machine_state = kwargs.pop('machine_state', 'initial') - license_details = None - if latest_version.license: - license_details = { - 'id': latest_version.license.node_license.license_id, - 'copyrightHolders': latest_version.license.copyright_holders, - 'year': latest_version.license.year - } - subjects = [[el] for el in latest_version.subjects.all().values_list('_id', flat=True)] + update_task_patcher = mock.patch('website.preprints.tasks.on_preprint_updated.si') + update_task_patcher.start() + # Create new version from the latest version target_class = cls._meta.model - instance = cls._build(target_class, *args, **kwargs) - instance.article_doi = latest_version.article_doi - instance.date_published = timezone.now() - user = kwargs.pop('creator', None) or instance.creator + instance = cls._build( + target_class, + provider=latest_version.provider, + title=latest_version.title, + description=latest_version.description, + creator=creator, + node=latest_version.node, + ) instance.machine_state = 'initial' instance.provider = latest_version.provider instance.save() - models.GuidVersionsThrough.objects.create( referent=instance, content_type=ContentType.objects.get_for_model(instance), @@ -837,8 +832,8 @@ def create_version(cls, *args, **kwargs): version=last_version_number + 1 ) - filename = kwargs.pop('filename', None) or f'preprint_file_{last_version_number + 1}_file.txt' - file_size = kwargs.pop('file_size', 1337) + # Prepare and add file + filename = f'preprint_file_{last_version_number + 1}.txt' preprint_file = OsfStorageFile.create( target_object_id=instance.id, target_content_type=ContentType.objects.get_for_model(instance), @@ -847,36 +842,59 @@ def create_version(cls, *args, **kwargs): materialized_path=f'/{filename}' ) preprint_file.save() - # TODO: why setting machine state here? it should happen in `if finish` - # TODO: instance.machine_state = machine_state + auth = Auth(instance.creator) + instance.set_primary_file(preprint_file, auth=auth, save=True) from addons.osfstorage import settings as osfstorage_settings - preprint_file.create_version(user, { + location = { 'object': '06d80e', 'service': 'cloud', osfstorage_settings.WATERBUTLER_RESOURCE: 'osf', - }, { - 'size': file_size, + } + metadata = { + 'size': 1357, 'contentType': 'img/png' - }).save() + } + preprint_file.create_version(creator, location, metadata=metadata).save() + + # Set relationships + contributor_list = latest_version.contributor_set.exclude( + user=latest_version.creator + ).values('visible', 'user_id', '_order') + for contributor in contributor_list: + instance.contributor_set.create(**{**contributor, 'preprint_id': instance.id}) + for institution in latest_version.affiliated_institutions.all(): + instance.add_affiliated_institution(institution, auth.user, ignore_user_affiliation=True) + + # Set other fields + # TODO: we should copy all fields + if latest_version.license: + license_details = { + 'id': latest_version.license.node_license.license_id, + 'copyrightHolders': latest_version.license.copyright_holders, + 'year': latest_version.license.year + } + instance.set_preprint_license(license_details, auth=auth) + subjects = [[el] for el in latest_version.subjects.all().values_list('_id', flat=True)] + instance.set_subjects(subjects, auth=auth) + instance.article_doi = latest_version.article_doi + instance.set_published(is_published, auth=auth) + # Note: machine_state needs to be adjusted after `set_published` is called. + instance.machine_state = final_machine_state + instance.save() update_task_patcher.stop() - if finish: - auth = Auth(user) - # TODO: make sure user is an admin contributor - instance.set_primary_file(preprint_file, auth=auth, save=True) - instance.set_subjects(subjects, auth=auth) - if license_details: - instance.set_preprint_license(license_details, auth=auth) - instance.set_published(is_published, auth=auth) - create_task_patcher = mock.patch('website.identifiers.utils.request_identifiers') - mock_create_identifier = create_task_patcher.start() - if is_published and set_doi: + if not is_published: + assert not set_doi and final_machine_state != 'accepted' + else: + if set_doi: + create_task_patcher = mock.patch('website.identifiers.utils.request_identifiers') + mock_create_identifier = create_task_patcher.start() mock_create_identifier.side_effect = sync_set_identifiers(instance) - guid_obj.referent = instance - guid_obj.object_id = instance.pk - guid_obj.save() - # TODO: instance.machine_state = machine_state - create_task_patcher.stop() + create_task_patcher.stop() + instance.is_published = True + guid_obj.referent = instance + guid_obj.object_id = instance.pk + guid_obj.save() instance.save() return instance diff --git a/tests/identifiers/test_crossref.py b/tests/identifiers/test_crossref.py index 0ad1501e9f8..7cb5518df46 100644 --- a/tests/identifiers/test_crossref.py +++ b/tests/identifiers/test_crossref.py @@ -42,7 +42,7 @@ def preprint(): @pytest.fixture() def preprint_version(preprint): - versioned_preprint = PreprintFactory.create_version(preprint=preprint) + versioned_preprint = PreprintFactory.create_version(preprint) return versioned_preprint @pytest.fixture()