diff --git a/api/base/filters.py b/api/base/filters.py index 412b1426cb4..4d9bc37a442 100644 --- a/api/base/filters.py +++ b/api/base/filters.py @@ -601,12 +601,29 @@ class PreprintFilterMixin(ListFilterMixin): Subclasses must define `get_default_queryset()`. """ + + @staticmethod + def postprocess_versioned_guid_id_query_param(operation): + """Handle query parameters when filtering on `id` for preprint and versioned guid. With versioned guid, + preprint no longer has guid._id for every version, and thus must switch to filter by pk. + """ + object_ids = [] + for val in operation['value']: + referent, version = Guid.load_referent(val) + if referent is None: + continue + object_ids.append(referent.id) + # Override the operation to filter `id__in=object_ids` + operation['source_field_name'] = 'id__in' + operation['value'] = object_ids + operation['op'] = 'eq' + def postprocess_query_param(self, key, field_name, operation): if field_name == 'provider': operation['source_field_name'] = 'provider___id' if field_name == 'id': - operation['source_field_name'] = 'guids___id' + PreprintFilterMixin.postprocess_versioned_guid_id_query_param(operation) if field_name == 'subjects': self.postprocess_subject_query_param(operation) diff --git a/api/users/views.py b/api/users/views.py index 927b5dc2f9b..296826ca48b 100644 --- a/api/users/views.py +++ b/api/users/views.py @@ -792,9 +792,11 @@ def post(self, request, *args, **kwargs): record_id = (request.data.get('id', None) or '').lower().strip() if not record_id: raise ValidationError('Must specify record "id".') + claimed_user = self.get_user(check_permissions=True) # Ensures claimability if claimed_user.is_disabled: raise ValidationError('Cannot claim disabled account.') + try: record_referent = Guid.objects.get(_id=record_id).referent except Guid.DoesNotExist: @@ -803,7 +805,10 @@ def post(self, request, *args, **kwargs): try: unclaimed_record = claimed_user.unclaimed_records[record_referent._id] except KeyError: - if isinstance(record_referent, Preprint) and record_referent.node and record_referent.node._id in claimed_user.unclaimed_records: + if isinstance( + record_referent, + Preprint, + ) and record_referent.node and record_referent.node._id in claimed_user.unclaimed_records: record_referent = record_referent.node unclaimed_record = claimed_user.unclaimed_records[record_referent._id] else: @@ -825,9 +830,9 @@ def post(self, request, *args, **kwargs): self._send_claim_email(claimer, claimed_user, record_referent, registered=True) except HTTPError as e: raise ValidationError(e.data['message_long']) - else: raise ValidationError('Must either be logged in or specify claim email.') + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/api_tests/preprints/filters/test_filters.py b/api_tests/preprints/filters/test_filters.py index f52b7accea2..9a483ad55ec 100644 --- a/api_tests/preprints/filters/test_filters.py +++ b/api_tests/preprints/filters/test_filters.py @@ -141,8 +141,7 @@ def test_id_filter_null(self, app, user, id_url): actual = [preprint['id'] for preprint in res.json['data']] assert expected == actual - def test_id_filter_equals_returns_one( - self, app, user, preprint_two, id_url): + def test_id_filter_equals_returns_one(self, app, user, preprint_two, id_url): expected = [preprint_two._id] res = app.get(f'{id_url}{preprint_two._id}', auth=user.auth) actual = [preprint['id'] for preprint in res.json['data']] diff --git a/api_tests/preprints/views/test_preprint_list.py b/api_tests/preprints/views/test_preprint_list.py index 5fd8f224a01..181351eecd1 100644 --- a/api_tests/preprints/views/test_preprint_list.py +++ b/api_tests/preprints/views/test_preprint_list.py @@ -16,7 +16,7 @@ PreprintIsValidListMixin, ) from api_tests.reviews.mixins.filter_mixins import ReviewableFilterMixin -from osf.models import Preprint, Node +from osf.models import Guid, Node, Preprint from osf import features from osf.utils.workflows import DefaultStates from osf.utils import permissions @@ -123,13 +123,21 @@ def test_create_preprint_does_not_create_a_node(self, app, user_one, provider, u def test_create_preprint_with_supplementary_node( self, app, user_one, provider, url, preprint_payload, supplementary_project ): - preprint_payload['data']['relationships']['node'] = {'data': {'id': supplementary_project._id, 'type': 'nodes'}} - res = app.post_json_api(url, preprint_payload, auth=user_one.auth) + preprint_payload['data']['relationships']['node'] = { + 'data': {'id': supplementary_project._id, 'type': 'nodes'} + } + res = app.post_json_api(url, preprint_payload, auth=user_one.auth) assert res.status_code == 201 preprint = Preprint.load(res.json['data']['id']) + preprint_id, version = Guid.split_guid(res.json['data']['id']) + assert preprint.node == supplementary_project - assert Node.objects.filter(preprints__guids___id=res.json['data']['id']).exists() + filtered_nodes = Node.objects.filter( + preprints__versioned_guids__guid___id=preprint_id, + preprints__versioned_guids__version=version + ) + assert supplementary_project in filtered_nodes def test_create_preprint_with_incorrectly_specified_node( self, app, user_one, provider, url, preprint_payload, supplementary_project diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 8b62c5cd3c1..d47af6c3b68 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1383,7 +1383,11 @@ def add_contributor(self, contributor, permissions=None, visible=True, ) if save: self.save() - + # TODO: Remove this comment after `Preprint.save()` has been fixed. There is a bug for Preprint when the + # first `.save()` is called. It triggers a series of events which includes adding creator as one + # contributor. However, with versioned guid, there is a gap between when preprint os saved and when + # guid and versioned guid are saved. During this gap, `self._id` is unfortunately `None` and the + # signal can not be be sent. if self._id and contrib_to_add: project_signals.contributor_added.send(self, contributor=contributor,