diff --git a/admin_tests/nodes/test_views.py b/admin_tests/nodes/test_views.py index b34d102b0e07..8e68e2e4a2ba 100644 --- a/admin_tests/nodes/test_views.py +++ b/admin_tests/nodes/test_views.py @@ -22,6 +22,8 @@ RemoveStuckRegistrationsView ) from admin_tests.utilities import setup_log_view, setup_view +from api.share.utils import shtrove_ingest_url +from api_tests.share._utils import assert_ingest_request from website import settings from nose import tools as nt from django.utils import timezone @@ -268,13 +270,9 @@ def test_reindex_node_share(self): with mock.patch('api.share.utils.settings.SHARE_ENABLED', True): with mock.patch('api.share.utils.settings.SHARE_API_TOKEN', 'mock-api-token'): with responses.RequestsMock(assert_all_requests_are_fired=True) as rsps: - rsps.add(responses.POST, 'https://share.osf.io/api/v2/normalizeddata/') + rsps.add(responses.POST, shtrove_ingest_url()) view.post(self.request) - data = json.loads(rsps.calls[-1].request.body.decode()) - - share_graph = data['data']['attributes']['data']['@graph'] - identifier_node = next(n for n in share_graph if n['@type'] == 'workidentifier') - assert identifier_node['creative_work']['@type'] == 'project' + assert_ingest_request(rsps.calls[-1].request, self.node._id, token='mock-api-token') nt.assert_equal(AdminLogEntry.objects.count(), count + 1) def test_reindex_registration_share(self): diff --git a/admin_tests/preprints/test_views.py b/admin_tests/preprints/test_views.py index 092258647258..08a58a08ae76 100644 --- a/admin_tests/preprints/test_views.py +++ b/admin_tests/preprints/test_views.py @@ -1,7 +1,5 @@ import pytest import mock -import json -import responses from django.test import RequestFactory from django.urls import reverse @@ -24,6 +22,7 @@ from osf.models.spam import SpamStatus from osf.utils.workflows import DefaultStates, RequestTypes +from api_tests.share._utils import expect_ingest_request from admin_tests.utilities import setup_view, setup_log_view from admin.preprints import views @@ -334,7 +333,7 @@ def test_change_preprint_provider_subjects_change_permissions(self, plain_view, @pytest.mark.enable_implicit_clean class TestPreprintReindex: - def test_reindex_preprint_share(self, preprint, req): + def test_reindex_preprint_share(self, preprint, req, mock_share_requests): preprint.provider.access_token = 'totally real access token I bought from a guy wearing a trenchcoat in the summer' preprint.provider.save() @@ -342,14 +341,9 @@ def test_reindex_preprint_share(self, preprint, req): view = views.PreprintReindexShare() view = setup_log_view(view, req, guid=preprint._id) - with mock.patch('api.share.utils.settings.SHARE_ENABLED', True): - with mock.patch('api.share.utils.settings.SHARE_API_TOKEN', 'mock-api-token'): - with responses.RequestsMock(assert_all_requests_are_fired=True) as rsps: - rsps.add(responses.POST, 'https://share.osf.io/api/v2/normalizeddata/') - view.post(req) - data = json.loads(rsps.calls[0].request.body.decode()) - assert data['data']['type'] == 'NormalizedData' - assert AdminLogEntry.objects.count() == count + 1 + with expect_ingest_request(mock_share_requests, preprint._id, token=preprint.provider.access_token): + view.post(req) + assert AdminLogEntry.objects.count() == count + 1 @mock.patch('website.search.search.update_preprint') def test_reindex_preprint_elastic(self, mock_update_search, preprint, req): diff --git a/admin_tests/registration_providers/test_views.py b/admin_tests/registration_providers/test_views.py index c043fb8879c1..4abc27bc82d3 100644 --- a/admin_tests/registration_providers/test_views.py +++ b/admin_tests/registration_providers/test_views.py @@ -144,9 +144,9 @@ def view(self, req, provider): view = views.ShareSourceRegistrationProvider() return setup_view(view, req, registration_provider_id=provider.id) - def test_share_source(self, mock_share, view, provider, req): - mock_share.reset() - mock_share.add( + def test_share_source(self, mock_share_responses, view, provider, req): + mock_share_responses.reset() + mock_share_responses.add( responses.POST, f'{settings.SHARE_URL}api/v2/sources/', json.dumps( @@ -172,9 +172,9 @@ def test_share_source(self, mock_share, view, provider, req): assert provider.access_token == 'test access token' @mock.patch.object(settings, 'SHARE_PROVIDER_PREPEND', 'testenv') - def test_share_source_prefix(self, mock_share, view, provider, req): - mock_share.reset() - mock_share.add( + def test_share_source_prefix(self, mock_share_responses, view, provider, req): + mock_share_responses.reset() + mock_share_responses.add( responses.POST, f'{settings.SHARE_URL}api/v2/sources/', json.dumps( diff --git a/api/share/utils.py b/api/share/utils.py index 534414c96035..8c608d36387a 100644 --- a/api/share/utils.py +++ b/api/share/utils.py @@ -49,21 +49,11 @@ def update_share(resource): if not _osfguid_value: logger.warning(f'update_share skipping resource that has no guids: {resource}') return - enqueue_task(task__update_share.s(_osfguid_value)) + _enqueue_update_share(_osfguid_value) -def do_update_share(osfguid: str): - logger.debug('%s.do_update_share("%s")', __name__, osfguid) - _guid_instance = apps.get_model('osf.Guid').load(osfguid) - if _guid_instance is None: - raise ValueError(f'unknown osfguid "{osfguid}"') - _resource = _guid_instance.referent - _response = ( - pls_delete_trove_indexcard(_resource) - if _should_delete_indexcard(_resource) - else pls_send_trove_indexcard(_resource) - ) - return _response +def _enqueue_update_share(osfguid_value: str): + enqueue_task(task__update_share.s(osfguid_value)) @celery_app.task(bind=True, max_retries=4, acks_late=True) @@ -74,7 +64,7 @@ def task__update_share(self, guid: str): :param guid: :return: """ - resp = do_update_share(guid) + resp = _do_update_share(guid) try: resp.raise_for_status() except Exception as e: @@ -123,6 +113,20 @@ def pls_delete_trove_indexcard(osf_item): ) +def _do_update_share(osfguid: str): + logger.warning('%s._do_update_share("%s")', __name__, osfguid) + _guid_instance = apps.get_model('osf.Guid').load(osfguid) + if _guid_instance is None: + raise ValueError(f'unknown osfguid "{osfguid}"') + _resource = _guid_instance.referent + _response = ( + pls_delete_trove_indexcard(_resource) + if _should_delete_indexcard(_resource) + else pls_send_trove_indexcard(_resource) + ) + return _response + + def _shtrove_record_identifier(osf_item): return osf_item.guids.values_list('_id', flat=True).first() diff --git a/api_tests/share/_utils.py b/api_tests/share/_utils.py index caa6bd066b0a..cb7bac3842a8 100644 --- a/api_tests/share/_utils.py +++ b/api_tests/share/_utils.py @@ -7,11 +7,11 @@ @contextlib.contextmanager -def expect_ingest_request(mock_share, osfguid, *, token=None, delete=False, count=1): - mock_share._calls.reset() +def expect_ingest_request(mock_share_responses, osfguid, *, token=None, delete=False, count=1): + mock_share_responses._calls.reset() yield - assert len(mock_share.calls) == count - for _call in mock_share.calls: + assert len(mock_share_responses.calls) == count + for _call in mock_share_responses.calls: assert_ingest_request(_call.request, osfguid, token=token, delete=delete) diff --git a/api_tests/share/test_share_node.py b/api_tests/share/test_share_node.py index 7671927e733d..e9751606b162 100644 --- a/api_tests/share/test_share_node.py +++ b/api_tests/share/test_share_node.py @@ -98,15 +98,15 @@ def registration_outcome(self, registration): ) return o - def test_update_node_share(self, mock_share, node, user): - with expect_ingest_request(mock_share, node._id): + def test_update_node_share(self, mock_share_responses, node, user): + with expect_ingest_request(mock_share_responses, node._id): on_node_updated(node._id, user._id, False, {'is_public'}) - def test_update_registration_share(self, mock_share, registration, user): - with expect_ingest_request(mock_share, registration._id): + def test_update_registration_share(self, mock_share_responses, registration, user): + with expect_ingest_request(mock_share_responses, registration._id): on_node_updated(registration._id, user._id, False, {'is_public'}) - def test_update_share_correctly_for_projects(self, mock_share, node, user): + def test_update_share_correctly_for_projects(self, mock_share_responses, node, user): cases = [{ 'is_deleted': False, 'attrs': {'is_public': True, 'is_deleted': False, 'spam_status': SpamStatus.HAM} @@ -121,14 +121,14 @@ def test_update_share_correctly_for_projects(self, mock_share, node, user): 'attrs': {'is_public': True, 'is_deleted': False, 'spam_status': SpamStatus.SPAM} }] - mock_share._calls.reset() # reset after factory calls + mock_share_responses._calls.reset() # reset after factory calls for i, case in enumerate(cases): for attr, value in case['attrs'].items(): setattr(node, attr, value) - with expect_ingest_request(mock_share, node._id, delete=case['is_deleted']): + with expect_ingest_request(mock_share_responses, node._id, delete=case['is_deleted']): node.save() - def test_update_share_correctly_for_registrations(self, mock_share, registration, user): + def test_update_share_correctly_for_registrations(self, mock_share_responses, registration, user): cases = [{ 'is_deleted': True, 'attrs': {'is_public': False, 'is_deleted': False} @@ -140,42 +140,42 @@ def test_update_share_correctly_for_registrations(self, mock_share, registration 'attrs': {'is_public': True, 'is_deleted': False} }] - mock_share._calls.reset() # reset after factory calls + mock_share_responses._calls.reset() # reset after factory calls for i, case in enumerate(cases): for attr, value in case['attrs'].items(): setattr(registration, attr, value) - with expect_ingest_request(mock_share, registration._id, delete=case['is_deleted']): + with expect_ingest_request(mock_share_responses, registration._id, delete=case['is_deleted']): registration.save() assert registration.is_registration - def test_update_share_correctly_for_projects_with_qa_tags(self, mock_share, node, user): - with expect_ingest_request(mock_share, node._id, delete=True): + def test_update_share_correctly_for_projects_with_qa_tags(self, mock_share_responses, node, user): + with expect_ingest_request(mock_share_responses, node._id, delete=True): node.add_tag(settings.DO_NOT_INDEX_LIST['tags'][0], auth=Auth(user)) - with expect_ingest_request(mock_share, node._id, delete=False): + with expect_ingest_request(mock_share_responses, node._id, delete=False): node.remove_tag(settings.DO_NOT_INDEX_LIST['tags'][0], auth=Auth(user), save=True) - def test_update_share_correctly_for_registrations_with_qa_tags(self, mock_share, registration, user): - with expect_ingest_request(mock_share, registration._id, delete=True): + def test_update_share_correctly_for_registrations_with_qa_tags(self, mock_share_responses, registration, user): + with expect_ingest_request(mock_share_responses, registration._id, delete=True): registration.add_tag(settings.DO_NOT_INDEX_LIST['tags'][0], auth=Auth(user)) - with expect_ingest_request(mock_share, registration._id): + with expect_ingest_request(mock_share_responses, registration._id): registration.remove_tag(settings.DO_NOT_INDEX_LIST['tags'][0], auth=Auth(user), save=True) - def test_update_share_correctly_for_projects_with_qa_titles(self, mock_share, node, user): + def test_update_share_correctly_for_projects_with_qa_titles(self, mock_share_responses, node, user): node.title = settings.DO_NOT_INDEX_LIST['titles'][0] + ' arbitary text for test title.' node.save() - with expect_ingest_request(mock_share, node._id, delete=True): + with expect_ingest_request(mock_share_responses, node._id, delete=True): on_node_updated(node._id, user._id, False, {'is_public'}) node.title = 'Not a qa title' - with expect_ingest_request(mock_share, node._id): + with expect_ingest_request(mock_share_responses, node._id): node.save() assert node.title not in settings.DO_NOT_INDEX_LIST['titles'] - def test_update_share_correctly_for_registrations_with_qa_titles(self, mock_share, registration, user): + def test_update_share_correctly_for_registrations_with_qa_titles(self, mock_share_responses, registration, user): registration.title = settings.DO_NOT_INDEX_LIST['titles'][0] + ' arbitary text for test title.' - with expect_ingest_request(mock_share, registration._id, delete=True): + with expect_ingest_request(mock_share_responses, registration._id, delete=True): registration.save() registration.title = 'Not a qa title' - with expect_ingest_request(mock_share, registration._id): + with expect_ingest_request(mock_share_responses, registration._id): registration.save() assert registration.title not in settings.DO_NOT_INDEX_LIST['titles'] @@ -184,21 +184,21 @@ def test_skips_no_settings(self, node, user): on_node_updated(node._id, user._id, False, {'is_public'}) assert len(responses.calls) == 0 - def test_call_async_update_on_500_retry(self, mock_share, node, user): + def test_call_async_update_on_500_retry(self, mock_share_responses, node, user): """This is meant to simulate a temporary outage, so the retry mechanism should kick in and complete it.""" - mock_share.replace(responses.POST, shtrove_ingest_url(), status=500) - mock_share.add(responses.POST, shtrove_ingest_url(), status=200) - with expect_ingest_request(mock_share, node._id, count=2): + mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=500) + mock_share_responses.add(responses.POST, shtrove_ingest_url(), status=200) + with expect_ingest_request(mock_share_responses, node._id, count=2): on_node_updated(node._id, user._id, False, {'is_public'}) - def test_call_async_update_on_500_failure(self, mock_share, node, user): + def test_call_async_update_on_500_failure(self, mock_share_responses, node, user): """This is meant to simulate a total outage, so the retry mechanism should try X number of times and quit.""" - mock_share.assert_all_requests_are_fired = False # allows it to retry indefinitely - mock_share.replace(responses.POST, shtrove_ingest_url(), status=500) - with expect_ingest_request(mock_share, node._id, count=5): # tries five times + mock_share_responses.assert_all_requests_are_fired = False # allows it to retry indefinitely + mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=500) + with expect_ingest_request(mock_share_responses, node._id, count=5): # tries five times on_node_updated(node._id, user._id, False, {'is_public'}) - def test_no_call_async_update_on_400_failure(self, mock_share, node, user): - mock_share.replace(responses.POST, shtrove_ingest_url(), status=400) - with expect_ingest_request(mock_share, node._id): + def test_no_call_async_update_on_400_failure(self, mock_share_responses, node, user): + mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=400) + with expect_ingest_request(mock_share_responses, node._id): on_node_updated(node._id, user._id, False, {'is_public'}) diff --git a/api_tests/share/test_share_preprint.py b/api_tests/share/test_share_preprint.py index fcb9e7708359..2eb8b5a2dd07 100644 --- a/api_tests/share/test_share_preprint.py +++ b/api_tests/share/test_share_preprint.py @@ -46,7 +46,7 @@ def provider(self): ) @pytest.fixture - def project(self, user, mock_share): + def project(self, user, mock_share_responses): return ProjectFactory(creator=user, is_public=True) @pytest.fixture @@ -68,93 +68,93 @@ def preprint(self, project, user, provider, subject): is_published=False ) - def test_save_unpublished_not_called(self, mock_share, preprint): + def test_save_unpublished_not_called(self, mock_share_responses, preprint): # expecting no ingest requests (delete or otherwise) - with _expect_preprint_ingest_request(mock_share, preprint, count=0): + with _expect_preprint_ingest_request(mock_share_responses, preprint, count=0): preprint.save() - def test_save_published_called(self, mock_share, preprint, user, auth): - with _expect_preprint_ingest_request(mock_share, preprint): + def test_save_published_called(self, mock_share_responses, preprint, user, auth): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.set_published(True, auth=auth, save=True) # This covers an edge case where a preprint is forced back to unpublished # that it sends the information back to share - def test_save_unpublished_called_forced(self, mock_share, auth, preprint): - with _expect_preprint_ingest_request(mock_share, preprint): + def test_save_unpublished_called_forced(self, mock_share_responses, auth, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.set_published(True, auth=auth, save=True) - with _expect_preprint_ingest_request(mock_share, preprint, delete=True): + with _expect_preprint_ingest_request(mock_share_responses, preprint, delete=True): preprint.is_published = False preprint.save(**{'force_update': True}) - def test_save_published_subject_change_called(self, mock_share, auth, preprint, subject, subject_two): + def test_save_published_subject_change_called(self, mock_share_responses, auth, preprint, subject, subject_two): preprint.set_published(True, auth=auth, save=True) - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.set_subjects([[subject_two._id]], auth=auth) - def test_save_unpublished_subject_change_not_called(self, mock_share, auth, preprint, subject_two): - with _expect_preprint_ingest_request(mock_share, preprint, delete=True): + def test_save_unpublished_subject_change_not_called(self, mock_share_responses, auth, preprint, subject_two): + with _expect_preprint_ingest_request(mock_share_responses, preprint, delete=True): preprint.set_subjects([[subject_two._id]], auth=auth) - def test_send_to_share_is_true(self, mock_share, auth, preprint): + def test_send_to_share_is_true(self, mock_share_responses, auth, preprint): preprint.set_published(True, auth=auth, save=True) - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): on_preprint_updated(preprint._id, saved_fields=['title']) - def test_preprint_contributor_changes_updates_preprints_share(self, mock_share, user, auth): + def test_preprint_contributor_changes_updates_preprints_share(self, mock_share_responses, user, auth): preprint = PreprintFactory(is_published=True, creator=user) preprint.set_published(True, auth=auth, save=True) user2 = AuthUserFactory() - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.add_contributor(contributor=user2, auth=auth, save=True) - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.move_contributor(contributor=user, index=0, auth=auth, save=True) data = [{'id': user._id, 'permissions': ADMIN, 'visible': True}, {'id': user2._id, 'permissions': WRITE, 'visible': False}] - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.manage_contributors(data, auth=auth, save=True) - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.update_contributor(user2, READ, True, auth=auth, save=True) - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.remove_contributor(contributor=user2, auth=auth) - def test_call_async_update_on_500_failure(self, mock_share, preprint, auth): - mock_share.replace(responses.POST, shtrove_ingest_url(), status=500) + def test_call_async_update_on_500_failure(self, mock_share_responses, preprint, auth): + mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=500) preprint.set_published(True, auth=auth, save=True) - with _expect_preprint_ingest_request(mock_share, preprint, count=5): + with _expect_preprint_ingest_request(mock_share_responses, preprint, count=5): preprint.update_search() - def test_no_call_async_update_on_400_failure(self, mock_share, preprint, auth): - mock_share.replace(responses.POST, shtrove_ingest_url(), status=400) + def test_no_call_async_update_on_400_failure(self, mock_share_responses, preprint, auth): + mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=400) preprint.set_published(True, auth=auth, save=True) - with _expect_preprint_ingest_request(mock_share, preprint, count=1): + with _expect_preprint_ingest_request(mock_share_responses, preprint, count=1): preprint.update_search() - def test_delete_from_share(self, mock_share): + def test_delete_from_share(self, mock_share_responses): preprint = PreprintFactory() - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.update_search() preprint.date_withdrawn = datetime.now() preprint.save() - with _expect_preprint_ingest_request(mock_share, preprint): + with _expect_preprint_ingest_request(mock_share_responses, preprint): preprint.update_search() preprint.spam_status = SpamStatus.SPAM preprint.save() - with _expect_preprint_ingest_request(mock_share, preprint, delete=True): + with _expect_preprint_ingest_request(mock_share_responses, preprint, delete=True): preprint.update_search() @contextlib.contextmanager -def _expect_preprint_ingest_request(mock_share, preprint, *, delete=False, count=1): +def _expect_preprint_ingest_request(mock_share_responses, preprint, *, delete=False, count=1): # same as expect_ingest_request, but with convenience for preprint specifics # and postcommit-task handling (so on_preprint_updated actually runs) with expect_ingest_request( - mock_share, + mock_share_responses, preprint._id, token=preprint.provider.access_token, delete=delete, diff --git a/conftest.py b/conftest.py index 54f195fe3c7b..cfb01932534a 100644 --- a/conftest.py +++ b/conftest.py @@ -151,14 +151,23 @@ def teardown_es(): @pytest.fixture -def mock_share(): - with mock.patch('api.share.utils.settings.SHARE_ENABLED', True): - with mock.patch('api.share.utils.settings.SHARE_API_TOKEN', 'mock-api-token'): - with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps: - _ingest_url = shtrove_ingest_url() - rsps.add(responses.POST, _ingest_url, status=200) - rsps.add(responses.DELETE, _ingest_url, status=200) - yield rsps +def mock_share_responses(): + with mock.patch.object(website_settings, 'SHARE_ENABLED', True): + with mock.patch.object(website_settings, 'SHARE_API_TOKEN', 'mock-api-token'): + # run "on_update" tasks synchronously: + with mock.patch.object(website_settings, 'USE_CELERY', False): + with responses.RequestsMock(assert_all_requests_are_fired=False) as _rsps: + _ingest_url = shtrove_ingest_url() + _rsps.add(responses.POST, _ingest_url, status=200) + _rsps.add(responses.DELETE, _ingest_url, status=200) + yield _rsps + + +@pytest.fixture +def mock_update_share(): + with mock.patch.object(website_settings, 'SHARE_ENABLED', True): + with mock.patch('api.share.utils._enqueue_update_share') as _mock_update_share: + yield _mock_update_share @pytest.fixture diff --git a/osf_tests/management_commands/test_recatalog_metadata.py b/osf_tests/management_commands/test_recatalog_metadata.py index 91c1a8d5006a..8c6ed14e94c5 100644 --- a/osf_tests/management_commands/test_recatalog_metadata.py +++ b/osf_tests/management_commands/test_recatalog_metadata.py @@ -55,9 +55,8 @@ def projects(self, registrations): for registration in registrations ]) - @mock.patch('api.share.utils.update_share') def test_recatalog_metadata(self, mock_update_share, preprint_provider, preprints, registration_provider, registrations, projects): - + mock_update_share.reset_mock() # test preprints call_command( 'recatalog_metadata', @@ -66,7 +65,7 @@ def test_recatalog_metadata(self, mock_update_share, preprint_provider, preprint preprint_provider._id, ) expected_update_share_calls = [ - mock.call(preprint) + mock.call(preprint._id) for preprint in preprints ] assert mock_update_share.mock_calls == expected_update_share_calls @@ -81,7 +80,7 @@ def test_recatalog_metadata(self, mock_update_share, preprint_provider, preprint registration_provider._id, ) expected_update_share_calls = [ - mock.call(registration) + mock.call(registration._id) for registration in registrations ] assert mock_update_share.mock_calls == expected_update_share_calls @@ -95,7 +94,7 @@ def test_recatalog_metadata(self, mock_update_share, preprint_provider, preprint '--all-providers', ) expected_update_share_calls = [ - mock.call(project) + mock.call(project._id) for project in projects # already ordered by id ] assert mock_update_share.mock_calls == expected_update_share_calls @@ -112,7 +111,7 @@ def test_recatalog_metadata(self, mock_update_share, preprint_provider, preprint '--chunk-count=1', ) expected_update_share_calls = [ - mock.call(registration) + mock.call(registration._id) for registration in registrations[1:4] ] assert mock_update_share.mock_calls == expected_update_share_calls @@ -121,7 +120,7 @@ def test_recatalog_metadata(self, mock_update_share, preprint_provider, preprint # slightly different chunking expected_update_share_calls = [ - mock.call(registration) + mock.call(registration._id) for registration in registrations[2:6] # already ordered by id ] call_command( diff --git a/osf_tests/test_tag.py b/osf_tests/test_tag.py index f9a0382dd11e..24c4bbbfbf4d 100644 --- a/osf_tests/test_tag.py +++ b/osf_tests/test_tag.py @@ -52,13 +52,11 @@ def project(self): def auth(self, project): return Auth(project.creator) - @mock.patch('osf.models.node.update_share') - @mock.patch('api.share.utils.settings.SHARE_ENABLED', True) def test_add_tag(self, mock_update_share, project, auth): project.add_tag('scientific', auth=auth) assert 'scientific' in list(project.tags.values_list('name', flat=True)) assert project.logs.latest().action == 'tag_added' - mock_update_share.assert_called_once_with(project) + mock_update_share.assert_called_once_with(project._id) @pytest.mark.skip('TODO: 128 is no longer max length, consider shortening') def test_add_tag_too_long(self, project, auth): @@ -69,16 +67,12 @@ def test_add_tag_way_too_long(self, project, auth): with pytest.raises(DataError): project.add_tag('asdf' * 257, auth=auth) - @mock.patch('osf.models.node.update_share') - @mock.patch('api.share.utils.settings.SHARE_ENABLED', True) def test_remove_tag(self, mock_update_share, project, auth): project.add_tag('scientific', auth=auth) - mock_update_share.assert_called_once_with(project) - + mock_update_share.assert_called_once_with(project._id) + mock_update_share.reset() project.remove_tag('scientific', auth=auth) - assert mock_update_share.call_count == 2 - assert mock_update_share.call_args_list[1][0][0] == project - + mock_update_share.assert_called_once_with(project._id) assert 'scientific' not in list(project.tags.values_list('name', flat=True)) assert project.logs.latest().action == 'tag_removed' diff --git a/tests/test_registrations/test_retractions.py b/tests/test_registrations/test_retractions.py index ffe946551508..d91aae7e0e68 100644 --- a/tests/test_registrations/test_retractions.py +++ b/tests/test_registrations/test_retractions.py @@ -427,8 +427,6 @@ def setUp(self): # Reload the registration; else tests won't catch failures to svae self.registration.reload() - @mock.patch('api.share.utils.settings.SHARE_ENABLED', True) - @mock.patch('api.share.utils.send_share_json') def test_approval_retracts_descendant_nodes(self, mock_update_share): # Initiate retraction for parent registration self.registration.retract_registration(self.user) @@ -478,8 +476,6 @@ def test_disapproval_cancels_retraction_on_descendant_nodes(self): assert_false(node.is_pending_retraction) assert_false(node.is_retracted) - @mock.patch('api.share.utils.settings.SHARE_ENABLED', True) - @mock.patch('api.share.utils.send_share_json') def test_approval_cancels_pending_embargoes_on_descendant_nodes(self, mock_update_share): # Initiate embargo for registration self.registration.embargo_registration( @@ -516,8 +512,6 @@ def test_approval_cancels_pending_embargoes_on_descendant_nodes(self, mock_updat assert mock_update_share.called - @mock.patch('api.share.utils.settings.SHARE_ENABLED', True) - @mock.patch('api.share.utils.send_share_json') def test_approval_cancels_active_embargoes_on_descendant_nodes(self, mock_update_share): # Initiate embargo for registration self.registration.embargo_registration( @@ -570,8 +564,6 @@ def setUp(self): # Reload the registration; else tests won't catch failures to svae self.registration.reload() - @mock.patch('api.share.utils.settings.SHARE_ENABLED', True) - @mock.patch('api.share.utils.send_share_json') def test_approval_calls_share_hook(self, mock_update_share): # Initiate retraction for parent registration self.registration.retract_registration(self.user) @@ -583,8 +575,6 @@ def test_approval_calls_share_hook(self, mock_update_share): assert_true(self.registration.is_retracted) assert mock_update_share.called - @mock.patch('api.share.utils.settings.SHARE_ENABLED', True) - @mock.patch('api.share.utils.send_share_json') def test_disapproval_does_not_call_share_hook(self, mock_update_share): # Initiate retraction for parent registration self.registration.retract_registration(self.user)