Skip to content

[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 #11130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from 10 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
1 change: 1 addition & 0 deletions admin_tests/nodes/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ def setUp(self):
self.user = AuthUserFactory()
self.node = ProjectFactory(creator=self.user)

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_request_approval_is_approved(self):
now = timezone.now()
self.approval = RegistrationApprovalFactory(
Expand Down
7 changes: 7 additions & 0 deletions admin_tests/registrations/test_registrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,14 @@ def test_embargoed_registration_from_changed_to_private_project_spam_ham(self, e
embargoed_registration_from_changed_to_private_project.confirm_ham(save=True)
assert not embargoed_registration_from_changed_to_private_project.is_public

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_public_registration_from_public_project_spam_ham(self, superuser, public_registration_from_public_project):
public_registration_from_public_project.confirm_spam(save=True)
assert not public_registration_from_public_project.is_public
public_registration_from_public_project.confirm_ham(save=True)
assert public_registration_from_public_project.is_public

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_public_registration_from_private_project_spam_ham(self, superuser, public_registration_from_private_project):
public_registration_from_private_project.confirm_spam(save=True)
assert not public_registration_from_private_project.is_public
Expand All @@ -110,18 +112,21 @@ def test_private_registration_from_private_project_spam_ham(self, superuser, pri
private_registration_from_public_project.confirm_ham(save=True)
assert not private_registration_from_public_project.is_public

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_public_registration_from_changed_to_public_project_spam_ham(self, superuser, public_registration_from_changed_to_public_project):
public_registration_from_changed_to_public_project.confirm_spam(save=True)
assert not public_registration_from_changed_to_public_project.is_public
public_registration_from_changed_to_public_project.confirm_ham(save=True)
assert public_registration_from_changed_to_public_project.is_public

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_public_registration_from_changed_to_private_project_spam_ham(self, superuser, public_registration_from_changed_to_private_project):
public_registration_from_changed_to_private_project.confirm_spam(save=True)
assert not public_registration_from_changed_to_private_project.is_public
public_registration_from_changed_to_private_project.confirm_ham(save=True)
assert public_registration_from_changed_to_private_project.is_public

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_unapproved_registration_task(self, embargoed_registration_from_changed_to_public_project):
embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved'
embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3)
Expand All @@ -131,6 +136,7 @@ def test_unapproved_registration_task(self, embargoed_registration_from_changed_
embargoed_registration_from_changed_to_public_project.registration_approval.refresh_from_db()
assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'approved'

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_unapproved_registration_task_after_spam(self, embargoed_registration_from_changed_to_public_project):
embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved'
embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3)
Expand All @@ -141,6 +147,7 @@ def test_unapproved_registration_task_after_spam(self, embargoed_registration_fr
embargoed_registration_from_changed_to_public_project.registration_approval.refresh_from_db()
assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'unapproved'

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_unapproved_registration_task_after_spam_ham(self, embargoed_registration_from_changed_to_public_project):
embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved'
embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3)
Expand Down
11 changes: 11 additions & 0 deletions api_tests/identifiers/managment_commands/test_sync_dois.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import datetime

from unittest import mock
import pytest
from django.core.management import call_command
from django.utils import timezone
Expand Down Expand Up @@ -53,6 +54,13 @@ def preprint_identifier(self, preprint):
identifier.save(update_modified=False)
return identifier

@pytest.fixture(autouse=True)
def mock_gravy_valet_get_links(self):
with mock.patch('osf.models.node.AbstractNode.get_verified_links') as mock_get_links:
mock_get_links.return_value = []
yield mock_get_links

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
@pytest.mark.enable_enqueue_task
def test_doi_synced_datacite(self, app, registration, registration_identifier, mock_datacite):
assert registration_identifier.modified.date() < datetime.datetime.now().date()
Expand All @@ -66,6 +74,7 @@ def test_doi_synced_datacite(self, app, registration, registration_identifier, m
registration_identifier.reload()
assert registration_identifier.modified.date() == datetime.datetime.now().date()

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
@pytest.mark.enable_enqueue_task
def test_doi_synced_crossref(self, app, preprint_identifier, mock_crossref):
assert preprint_identifier.modified.date() < datetime.datetime.now().date()
Expand All @@ -77,6 +86,7 @@ def test_doi_synced_crossref(self, app, preprint_identifier, mock_crossref):
preprint_identifier.reload()
assert preprint_identifier.modified.date() == datetime.datetime.now().date()

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
@pytest.mark.enable_enqueue_task
def test_doi_sync_private(self, app, registration_private, registration_identifier, mock_datacite):

Expand All @@ -91,6 +101,7 @@ def test_doi_sync_private(self, app, registration_private, registration_identifi
assert registration_identifier.modified.date() < datetime.datetime.now().date()
assert registration_identifier.modified.date() < datetime.datetime.now().date()

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
@pytest.mark.enable_enqueue_task
def test_doi_sync_public_only(self, app, registration_private, registration_identifier, mock_datacite):
call_command('sync_doi_metadata', f'-m={datetime.datetime.now()}')
Expand Down
9 changes: 9 additions & 0 deletions api_tests/identifiers/views/test_identifier_list.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

import pytest

from urllib.parse import urlparse
Expand Down Expand Up @@ -475,6 +477,13 @@ def ark_payload(self):
def client(self, resource):
return DataCiteClient(resource)

@pytest.fixture
def mock_gravy_valet_get_links(self):
with mock.patch('osf.models.node.AbstractNode.get_verified_links') as mock_get_links:
mock_get_links.return_value = []
yield mock_get_links

@pytest.mark.usefixtures('mock_gravy_valet_get_links')
@responses.activate
def test_create_identifier(self, app, resource, client, identifier_url, identifier_payload, user,
write_contributor, read_contributor, ark_payload):
Expand Down
6 changes: 6 additions & 0 deletions api_tests/registrations/views/test_registration_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,8 @@ def license_cc0(self):
@pytest.mark.django_db
@pytest.mark.enable_implicit_clean
class TestRegistrationUpdate(TestRegistrationUpdateTestCase):

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_update_registration(
self, app, user, read_only_contributor,
read_write_contributor, public_registration,
Expand Down Expand Up @@ -415,6 +417,7 @@ def test_update_registration(
expect_errors=True)
assert res.status_code == 403

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_fields(
self, app, user, public_registration,
private_registration, public_url, institution_one,
Expand Down Expand Up @@ -561,6 +564,7 @@ def test_fields(
expect_errors=True)
assert res.status_code == 400

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

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_initiate_withdrawal_with_embargo_ends_embargo(
self, app, user, public_project, public_registration, public_url, public_payload):
public_registration.embargo_registration(
Expand Down Expand Up @@ -934,6 +939,7 @@ def new_tag_payload_withdrawn(self, registration_withdrawn):
}
}

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_registration_tags(
self, app, registration_public, registration_private,
url_registration_public, url_registration_private,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,15 @@ def test_registrations_children_list(self, user, app, registration_with_children
assert component_one._id in ids
assert component_two._id in ids

def test_return_registrations_list_no_auth_approved(self, user, app, registration_with_children_approved, registration_with_children_approved_url):
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_return_registrations_list_no_auth_approved(
self,
user,
app,
registration_with_children_approved,
registration_with_children_approved_url
):

component_one, component_two, component_three, component_four = registration_with_children_approved.nodes

res = app.get(registration_with_children_approved_url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ def test_field_specific_related_counts_retrieved_if_visible_field_on_withdrawn_r
assert res.status_code == 200
assert res.json['data']['relationships']['contributors']['links']['related']['meta']['count'] == 1

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_child_inherits_withdrawal_justification_and_date_withdrawn(
self, app, user, withdrawn_registration_with_child, registration_with_child):

Expand Down
15 changes: 15 additions & 0 deletions api_tests/registries_moderation/test_submissions.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

import pytest
import datetime

Expand Down Expand Up @@ -164,6 +166,12 @@ def actions_payload_base(self):
}
return payload

@pytest.fixture(autouse=True)
def mock_gravy_valet_get_links(self):
with mock.patch('osf.models.node.AbstractNode.get_verified_links') as mock_get_links:
mock_get_links.return_value = []
yield mock_get_links

def test_get_provider_requests(self, app, provider_requests_url, registration_with_withdraw_request, access_request, moderator, moderator_wrong_provider):
resp = app.get(provider_requests_url, expect_errors=True)
assert resp.status_code == 401
Expand Down Expand Up @@ -312,6 +320,7 @@ def test_registries_moderation_permission_log(self, app, registration_log_url, r
resp = app.get(registration_log_url, auth=moderator.auth)
assert resp.status_code == 200

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_registries_moderation_post_accept(self, app, registration, moderator, registration_actions_url, actions_payload_base, reg_creator):
registration.require_approval(user=registration.creator)
registration.registration_approval.accept()
Expand Down Expand Up @@ -378,6 +387,7 @@ def test_registries_moderation_post_embargo_reject(self, app, embargo_registrati
embargo_registration.refresh_from_db()
assert embargo_registration.moderation_state == RegistrationModerationStates.REJECTED.db_name

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_registries_moderation_post_withdraw_accept(self, app, retract_registration, moderator, retract_registration_actions_url, actions_payload_base, provider):
retract_registration.sanction.accept()
retract_registration.refresh_from_db()
Expand Down Expand Up @@ -408,6 +418,7 @@ def test_registries_moderation_post_withdraw_reject(self, app, retract_registrat
retract_registration.refresh_from_db()
assert retract_registration.moderation_state == RegistrationModerationStates.ACCEPTED.db_name

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_registries_moderation_post_force_withdraw(self, app, registration, moderator, registration_actions_url, actions_payload_base, provider, reg_creator):
registration.require_approval(user=registration.creator)
registration.registration_approval.accept()
Expand Down Expand Up @@ -477,6 +488,7 @@ def test_registries_moderation_post_embargo_admin_cant_accept(self, app, embargo
resp = app.post_json_api(embargo_registration_actions_url, actions_payload_base, auth=reg_creator.auth, expect_errors=True)
assert resp.status_code == 403

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

Expand Down Expand Up @@ -509,6 +521,7 @@ def test_registries_moderation_post_admin_cant_force_withdraw(self, app, registr
RegistrationModerationTriggers.REJECT_SUBMISSION
]
)
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_post_submission_action_persists_comment(self, app, registration, moderator, registration_actions_url, actions_payload_base, moderator_trigger):
assert registration.actions.count() == 0
registration.require_approval(user=registration.creator)
Expand Down Expand Up @@ -692,6 +705,7 @@ def test_public_project_with_embargo_is_private_after_spam_and_ham_and_moderatio
assert registration.moderation_state == RegistrationModerationStates.EMBARGO.db_name
assert registration.is_public is False

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
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):
user = AuthUserFactory()
user.is_staff = True
Expand Down Expand Up @@ -728,6 +742,7 @@ def test_private_project_without_embargo_is_public_after_spam_and_ham_and_modera
assert registration.moderation_state == RegistrationModerationStates.ACCEPTED.db_name
assert registration.is_public is True

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
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):
user = AuthUserFactory()
user.is_staff = True
Expand Down
1 change: 1 addition & 0 deletions api_tests/search/serializers/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
@pytest.mark.django_db
class TestSearchSerializer:

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_search_serializer_mixed_model(self):

user = AuthUserFactory()
Expand Down
1 change: 1 addition & 0 deletions api_tests/search/views/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ def registration_private(self, project_private, schema):
registration_private.update_search()
return registration_private

@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
def test_search_registrations(
self, app, url_registration_search, user, user_one, user_two,
registration, registration_public, registration_private):
Expand Down
7 changes: 7 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,10 @@ def helpful_thing(self):
```
"""
yield from rolledback_transaction('function_transaction')

@pytest.fixture
def mock_gravy_valet_get_verified_links():
'''This fix is used to mock a GV request for TreeWalker node metadata'''
with mock.patch('osf.external.gravy_valet.translations.get_verified_links') as mock_get_verified_links:
mock_get_verified_links.return_value = []
yield mock_get_verified_links
30 changes: 28 additions & 2 deletions osf/metadata/serializers/datacite/datacite_tree_walker.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import rdflib

from framework import sentry
from osf.exceptions import MetadataSerializationError
from osf.metadata import gather
from osf.metadata.rdfutils import (
Expand Down Expand Up @@ -113,7 +114,7 @@ def walk(self, doi_override=None):
self._visit_rights(self.root)
self._visit_descriptions(self.root, self.basket.focus.iri)
self._visit_funding_references(self.root)
self._visit_related(self.root)
self._visit_related_and_verified_links(self.root)

def _visit_identifier(self, parent_el, *, doi_override=None):
if doi_override is None:
Expand Down Expand Up @@ -373,13 +374,16 @@ def _visit_related_identifier_and_item(self, identifier_parent_el, item_parent_e
self._visit_publication_year(related_item_el, related_iri)
self._visit_publisher(related_item_el, related_iri)

def _visit_related(self, parent_el):
def _visit_related_and_verified_links(self, parent_el):
relation_pairs = set()
for relation_iri, datacite_relation in RELATED_IDENTIFIER_TYPE_MAP.items():
for related_iri in self.basket[relation_iri]:
relation_pairs.add((datacite_relation, related_iri))

related_identifiers_el = self.visit(parent_el, 'relatedIdentifiers', is_list=True)
related_items_el = self.visit(parent_el, 'relatedItems', is_list=True)

# First add regular related identifiers
for datacite_relation, related_iri in sorted(relation_pairs):
self._visit_related_identifier_and_item(
related_identifiers_el,
Expand All @@ -388,6 +392,28 @@ def _visit_related(self, parent_el):
datacite_relation,
)

# Then add verified links to same relatedIdentifiers element
osf_item = self.basket.focus.dbmodel
from osf.models import AbstractNode

if isinstance(osf_item, AbstractNode):
gv_verified_link_list = osf_item.get_verified_links()
if gv_verified_link_list:
non_url_verified_links = []
for item in gv_verified_link_list:
verified_link, resource_type = item.get('target_url', None), item.get('resource_type', None)
if verified_link and resource_type:
if smells_like_iri(verified_link):
self.visit(related_identifiers_el, 'relatedIdentifier', text=verified_link, attrib={
'relatedIdentifierType': 'URL',
'relationType': 'IsReferencedBy',
'resourceTypeGeneral': resource_type.title()
})
else:
non_url_verified_links.append(verified_link)
if non_url_verified_links:
sentry.log_message(f'Skipped - {','.join(non_url_verified_links)} for node {osf_item._id}')

def _visit_name_identifiers(self, parent_el, agent_iri):
for identifier in sorted(self.basket[agent_iri:DCTERMS.identifier]):
identifier_type, identifier_value = self._identifier_type_and_value(identifier)
Expand Down
1 change: 1 addition & 0 deletions osf_tests/embargoes/test_embargoes.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def registration(self, user):
embargo.save()
return embargo.registrations.last()

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