Skip to content

Commit 1686ef8

Browse files
authored
Merge pull request #11130 from Vlad0n20/feature/ENG-7839
[ENG-7839] [ENG-7766] Add celery task to update Datacite metadata + Use new GV API endpoint to retrieve node's verified links + Update unit tests
2 parents b7373f4 + 2190a32 commit 1686ef8

36 files changed

+175
-8
lines changed

admin_tests/nodes/test_views.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,7 @@ def setUp(self):
505505
self.user = AuthUserFactory()
506506
self.node = ProjectFactory(creator=self.user)
507507

508+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
508509
def test_request_approval_is_approved(self):
509510
now = timezone.now()
510511
self.approval = RegistrationApprovalFactory(

admin_tests/registrations/test_registrations.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,14 @@ def test_embargoed_registration_from_changed_to_private_project_spam_ham(self, e
9292
embargoed_registration_from_changed_to_private_project.confirm_ham(save=True)
9393
assert not embargoed_registration_from_changed_to_private_project.is_public
9494

95+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
9596
def test_public_registration_from_public_project_spam_ham(self, superuser, public_registration_from_public_project):
9697
public_registration_from_public_project.confirm_spam(save=True)
9798
assert not public_registration_from_public_project.is_public
9899
public_registration_from_public_project.confirm_ham(save=True)
99100
assert public_registration_from_public_project.is_public
100101

102+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
101103
def test_public_registration_from_private_project_spam_ham(self, superuser, public_registration_from_private_project):
102104
public_registration_from_private_project.confirm_spam(save=True)
103105
assert not public_registration_from_private_project.is_public
@@ -110,18 +112,21 @@ def test_private_registration_from_private_project_spam_ham(self, superuser, pri
110112
private_registration_from_public_project.confirm_ham(save=True)
111113
assert not private_registration_from_public_project.is_public
112114

115+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
113116
def test_public_registration_from_changed_to_public_project_spam_ham(self, superuser, public_registration_from_changed_to_public_project):
114117
public_registration_from_changed_to_public_project.confirm_spam(save=True)
115118
assert not public_registration_from_changed_to_public_project.is_public
116119
public_registration_from_changed_to_public_project.confirm_ham(save=True)
117120
assert public_registration_from_changed_to_public_project.is_public
118121

122+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
119123
def test_public_registration_from_changed_to_private_project_spam_ham(self, superuser, public_registration_from_changed_to_private_project):
120124
public_registration_from_changed_to_private_project.confirm_spam(save=True)
121125
assert not public_registration_from_changed_to_private_project.is_public
122126
public_registration_from_changed_to_private_project.confirm_ham(save=True)
123127
assert public_registration_from_changed_to_private_project.is_public
124128

129+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
125130
def test_unapproved_registration_task(self, embargoed_registration_from_changed_to_public_project):
126131
embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved'
127132
embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3)
@@ -131,6 +136,7 @@ def test_unapproved_registration_task(self, embargoed_registration_from_changed_
131136
embargoed_registration_from_changed_to_public_project.registration_approval.refresh_from_db()
132137
assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'approved'
133138

139+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
134140
def test_unapproved_registration_task_after_spam(self, embargoed_registration_from_changed_to_public_project):
135141
embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved'
136142
embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3)
@@ -141,6 +147,7 @@ def test_unapproved_registration_task_after_spam(self, embargoed_registration_fr
141147
embargoed_registration_from_changed_to_public_project.registration_approval.refresh_from_db()
142148
assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'unapproved'
143149

150+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
144151
def test_unapproved_registration_task_after_spam_ham(self, embargoed_registration_from_changed_to_public_project):
145152
embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved'
146153
embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3)

api_tests/identifiers/managment_commands/test_sync_dois.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ def preprint_identifier(self, preprint):
5353
identifier.save(update_modified=False)
5454
return identifier
5555

56+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
5657
@pytest.mark.enable_enqueue_task
5758
def test_doi_synced_datacite(self, app, registration, registration_identifier, mock_datacite):
5859
assert registration_identifier.modified.date() < datetime.datetime.now().date()
@@ -66,6 +67,7 @@ def test_doi_synced_datacite(self, app, registration, registration_identifier, m
6667
registration_identifier.reload()
6768
assert registration_identifier.modified.date() == datetime.datetime.now().date()
6869

70+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
6971
@pytest.mark.enable_enqueue_task
7072
def test_doi_synced_crossref(self, app, preprint_identifier, mock_crossref):
7173
assert preprint_identifier.modified.date() < datetime.datetime.now().date()
@@ -77,6 +79,7 @@ def test_doi_synced_crossref(self, app, preprint_identifier, mock_crossref):
7779
preprint_identifier.reload()
7880
assert preprint_identifier.modified.date() == datetime.datetime.now().date()
7981

82+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
8083
@pytest.mark.enable_enqueue_task
8184
def test_doi_sync_private(self, app, registration_private, registration_identifier, mock_datacite):
8285

@@ -91,6 +94,7 @@ def test_doi_sync_private(self, app, registration_private, registration_identifi
9194
assert registration_identifier.modified.date() < datetime.datetime.now().date()
9295
assert registration_identifier.modified.date() < datetime.datetime.now().date()
9396

97+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
9498
@pytest.mark.enable_enqueue_task
9599
def test_doi_sync_public_only(self, app, registration_private, registration_identifier, mock_datacite):
96100
call_command('sync_doi_metadata', f'-m={datetime.datetime.now()}')

api_tests/identifiers/views/test_identifier_list.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ def ark_payload(self):
475475
def client(self, resource):
476476
return DataCiteClient(resource)
477477

478+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
478479
@responses.activate
479480
def test_create_identifier(self, app, resource, client, identifier_url, identifier_payload, user,
480481
write_contributor, read_contributor, ark_payload):

api_tests/registrations/views/test_registration_detail.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,8 @@ def license_cc0(self):
335335
@pytest.mark.django_db
336336
@pytest.mark.enable_implicit_clean
337337
class TestRegistrationUpdate(TestRegistrationUpdateTestCase):
338+
339+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
338340
def test_update_registration(
339341
self, app, user, read_only_contributor,
340342
read_write_contributor, public_registration,
@@ -415,6 +417,7 @@ def test_update_registration(
415417
expect_errors=True)
416418
assert res.status_code == 403
417419

420+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
418421
def test_fields(
419422
self, app, user, public_registration,
420423
private_registration, public_url, institution_one,
@@ -561,6 +564,7 @@ def test_fields(
561564
expect_errors=True)
562565
assert res.status_code == 400
563566

567+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
564568
def test_turning_private_registrations_public(
565569
self, app, user, make_payload):
566570
private_project = ProjectFactory(creator=user, is_public=False)
@@ -776,6 +780,7 @@ def test_initiate_withdrawal_success(self, mock_send_mail, app, user, public_reg
776780
assert public_registration.registered_from.logs.first().action == 'retraction_initiated'
777781
assert mock_send_mail.called
778782

783+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
779784
def test_initiate_withdrawal_with_embargo_ends_embargo(
780785
self, app, user, public_project, public_registration, public_url, public_payload):
781786
public_registration.embargo_registration(
@@ -934,6 +939,7 @@ def new_tag_payload_withdrawn(self, registration_withdrawn):
934939
}
935940
}
936941

942+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
937943
def test_registration_tags(
938944
self, app, registration_public, registration_private,
939945
url_registration_public, url_registration_private,

api_tests/registrations/views/test_registrations_childrens_list.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,15 @@ def test_registrations_children_list(self, user, app, registration_with_children
6868
assert component_one._id in ids
6969
assert component_two._id in ids
7070

71-
def test_return_registrations_list_no_auth_approved(self, user, app, registration_with_children_approved, registration_with_children_approved_url):
71+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
72+
def test_return_registrations_list_no_auth_approved(
73+
self,
74+
user,
75+
app,
76+
registration_with_children_approved,
77+
registration_with_children_approved_url
78+
):
79+
7280
component_one, component_two, component_three, component_four = registration_with_children_approved.nodes
7381

7482
res = app.get(registration_with_children_approved_url)

api_tests/registrations/views/test_withdrawn_registrations.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ def test_field_specific_related_counts_retrieved_if_visible_field_on_withdrawn_r
224224
assert res.status_code == 200
225225
assert res.json['data']['relationships']['contributors']['links']['related']['meta']['count'] == 1
226226

227+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
227228
def test_child_inherits_withdrawal_justification_and_date_withdrawn(
228229
self, app, user, withdrawn_registration_with_child, registration_with_child):
229230

api_tests/registries_moderation/test_submissions.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,7 @@ def test_registries_moderation_permission_log(self, app, registration_log_url, r
312312
resp = app.get(registration_log_url, auth=moderator.auth)
313313
assert resp.status_code == 200
314314

315+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
315316
def test_registries_moderation_post_accept(self, app, registration, moderator, registration_actions_url, actions_payload_base, reg_creator):
316317
registration.require_approval(user=registration.creator)
317318
registration.registration_approval.accept()
@@ -378,6 +379,7 @@ def test_registries_moderation_post_embargo_reject(self, app, embargo_registrati
378379
embargo_registration.refresh_from_db()
379380
assert embargo_registration.moderation_state == RegistrationModerationStates.REJECTED.db_name
380381

382+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
381383
def test_registries_moderation_post_withdraw_accept(self, app, retract_registration, moderator, retract_registration_actions_url, actions_payload_base, provider):
382384
retract_registration.sanction.accept()
383385
retract_registration.refresh_from_db()
@@ -408,6 +410,7 @@ def test_registries_moderation_post_withdraw_reject(self, app, retract_registrat
408410
retract_registration.refresh_from_db()
409411
assert retract_registration.moderation_state == RegistrationModerationStates.ACCEPTED.db_name
410412

413+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
411414
def test_registries_moderation_post_force_withdraw(self, app, registration, moderator, registration_actions_url, actions_payload_base, provider, reg_creator):
412415
registration.require_approval(user=registration.creator)
413416
registration.registration_approval.accept()
@@ -477,6 +480,7 @@ def test_registries_moderation_post_embargo_admin_cant_accept(self, app, embargo
477480
resp = app.post_json_api(embargo_registration_actions_url, actions_payload_base, auth=reg_creator.auth, expect_errors=True)
478481
assert resp.status_code == 403
479482

483+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
480484
def test_registries_moderation_post_admin_cant_force_withdraw(self, app, registration, moderator, registration_actions_url, actions_payload_base, provider, reg_creator):
481485
registration.require_approval(user=registration.creator)
482486

@@ -509,6 +513,7 @@ def test_registries_moderation_post_admin_cant_force_withdraw(self, app, registr
509513
RegistrationModerationTriggers.REJECT_SUBMISSION
510514
]
511515
)
516+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
512517
def test_post_submission_action_persists_comment(self, app, registration, moderator, registration_actions_url, actions_payload_base, moderator_trigger):
513518
assert registration.actions.count() == 0
514519
registration.require_approval(user=registration.creator)
@@ -692,6 +697,7 @@ def test_public_project_with_embargo_is_private_after_spam_and_ham_and_moderatio
692697
assert registration.moderation_state == RegistrationModerationStates.EMBARGO.db_name
693698
assert registration.is_public is False
694699

700+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
695701
def test_private_project_without_embargo_is_public_after_spam_and_ham_and_moderation_approval(self, app, registration, moderator, registration_actions_url, actions_payload_base, provider):
696702
user = AuthUserFactory()
697703
user.is_staff = True
@@ -728,6 +734,7 @@ def test_private_project_without_embargo_is_public_after_spam_and_ham_and_modera
728734
assert registration.moderation_state == RegistrationModerationStates.ACCEPTED.db_name
729735
assert registration.is_public is True
730736

737+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
731738
def test_public_project_without_embargo_is_public_after_spam_and_ham_and_moderation_approval(self, app, registration, moderator, registration_actions_url, actions_payload_base, provider):
732739
user = AuthUserFactory()
733740
user.is_staff = True

api_tests/search/serializers/test_serializers.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
@pytest.mark.django_db
1717
class TestSearchSerializer:
1818

19+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
1920
def test_search_serializer_mixed_model(self):
2021

2122
user = AuthUserFactory()

api_tests/search/views/test_views.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ def registration_private(self, project_private, schema):
566566
registration_private.update_search()
567567
return registration_private
568568

569+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
569570
def test_search_registrations(
570571
self, app, url_registration_search, user, user_one, user_two,
571572
registration, registration_public, registration_private):

conftest.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,3 +357,17 @@ def helpful_thing(self):
357357
```
358358
"""
359359
yield from rolledback_transaction('function_transaction')
360+
361+
@pytest.fixture
362+
def mock_gravy_valet_get_verified_links():
363+
"""This fixture is used to mock a GV request which is made during node's identifier update. More specifically, when
364+
the tree walker in datacite metadata building process asks GV for verified links. As a result, this request must be
365+
mocked in many tests. The following decoration can be applied to either a test class or individual test methods.
366+
367+
```
368+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
369+
```
370+
"""
371+
with mock.patch('osf.external.gravy_valet.translations.get_verified_links') as mock_get_verified_links:
372+
mock_get_verified_links.return_value = []
373+
yield mock_get_verified_links

osf/external/gravy_valet/request_helpers.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,9 @@ def iterate_gv_results(
247247
auth=auth
248248
)
249249

250-
if not response and raise_on_error:
251-
raise GVException
252-
253250
if not response:
251+
if raise_on_error:
252+
raise GVException
254253
return
255254

256255
response_json = response.json()

osf/metadata/serializers/datacite/datacite_tree_walker.py

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import rdflib
88

9+
from framework import sentry
910
from osf.exceptions import MetadataSerializationError
1011
from osf.metadata import gather
1112
from osf.metadata.rdfutils import (
@@ -113,7 +114,7 @@ def walk(self, doi_override=None):
113114
self._visit_rights(self.root)
114115
self._visit_descriptions(self.root, self.basket.focus.iri)
115116
self._visit_funding_references(self.root)
116-
self._visit_related(self.root)
117+
self._visit_related_and_verified_links(self.root)
117118

118119
def _visit_identifier(self, parent_el, *, doi_override=None):
119120
if doi_override is None:
@@ -373,13 +374,16 @@ def _visit_related_identifier_and_item(self, identifier_parent_el, item_parent_e
373374
self._visit_publication_year(related_item_el, related_iri)
374375
self._visit_publisher(related_item_el, related_iri)
375376

376-
def _visit_related(self, parent_el):
377+
def _visit_related_and_verified_links(self, parent_el):
377378
relation_pairs = set()
378379
for relation_iri, datacite_relation in RELATED_IDENTIFIER_TYPE_MAP.items():
379380
for related_iri in self.basket[relation_iri]:
380381
relation_pairs.add((datacite_relation, related_iri))
382+
381383
related_identifiers_el = self.visit(parent_el, 'relatedIdentifiers', is_list=True)
382384
related_items_el = self.visit(parent_el, 'relatedItems', is_list=True)
385+
386+
# First add regular related identifiers
383387
for datacite_relation, related_iri in sorted(relation_pairs):
384388
self._visit_related_identifier_and_item(
385389
related_identifiers_el,
@@ -388,6 +392,35 @@ def _visit_related(self, parent_el):
388392
datacite_relation,
389393
)
390394

395+
# Then add verified links to same relatedIdentifiers element
396+
osf_item = self.basket.focus.dbmodel
397+
from osf.models import AbstractNode
398+
399+
if isinstance(osf_item, AbstractNode):
400+
gv_verified_link_list = osf_item.get_verified_links()
401+
skipped_items = []
402+
for item in gv_verified_link_list:
403+
verified_link, resource_type = item.get('target_url', None), item.get('resource_type', None)
404+
if not verified_link or not resource_type:
405+
logger.error(f'Must have both verified_link and resource_type: [item={item}]')
406+
skipped_items.append(f'Missing data: [link={verified_link}, type={resource_type}]')
407+
continue
408+
if not smells_like_iri(verified_link):
409+
skipped_items.append(f'Invalid link: [link={verified_link}, type={resource_type}]')
410+
continue
411+
self.visit(
412+
related_identifiers_el,
413+
'relatedIdentifier',
414+
text=verified_link,
415+
attrib={
416+
'relatedIdentifierType': 'URL',
417+
'relationType': 'IsReferencedBy',
418+
'resourceTypeGeneral': resource_type.title()
419+
}
420+
)
421+
if skipped_items:
422+
sentry.log_message(f'Skipped items for node [{osf_item._id}]: {'; '.join(skipped_items)}. ')
423+
391424
def _visit_name_identifiers(self, parent_el, agent_iri):
392425
for identifier in sorted(self.basket[agent_iri:DCTERMS.identifier]):
393426
identifier_type, identifier_value = self._identifier_type_and_value(identifier)

osf_tests/embargoes/test_embargoes.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ def registration(self, user):
2929
embargo.save()
3030
return embargo.registrations.last()
3131

32+
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
3233
def test_request_early_termination_too_late(self, registration, user):
3334
"""
3435
This is for an edge case test for where embargos are frozen and never expire when the user requests they be

osf_tests/metadata/expected_metadata_files/preprint_supplement.turtle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
@prefix osf: <https://osf.io/vocab/2022/> .
22
@prefix skos: <http://www.w3.org/2004/02/skos/core#> .
3+
@prefix xsd: <http://www.w3.org/2001/XMLSchema#> .
34

45
<http://localhost:5000/w4ibb> osf:storageByteCount 1337 ;
56
osf:storageRegion <http://localhost:8000/v2/regions/us/> .

osf_tests/metadata/expected_metadata_files/project_basic.datacite.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@
6363
"relatedIdentifier": "11.pp/FK2osf.io/w4ibb_v1",
6464
"relatedIdentifierType": "DOI",
6565
"relationType": "IsSupplementTo"
66+
},
67+
{
68+
"relatedIdentifier": "https://foo.bar",
69+
"relatedIdentifierType": "URL",
70+
"relationType": "IsReferencedBy",
71+
"resourceTypeGeneral": "Other"
6672
}
6773
],
6874
"relatedItems": [

0 commit comments

Comments
 (0)