From a19b742a12abe396c0eeeb700bf52656d6235b90 Mon Sep 17 00:00:00 2001 From: Ostap Zherebetskyi Date: Wed, 31 Dec 2025 17:55:57 +0200 Subject: [PATCH 1/3] update contributor notification logic in DraftRegistration --- osf/models/registrations.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/osf/models/registrations.py b/osf/models/registrations.py index ea37f783bde..7426121de98 100644 --- a/osf/models/registrations.py +++ b/osf/models/registrations.py @@ -1374,13 +1374,15 @@ def create_from_node( if not node: draft.affiliated_institutions.add(*draft.creator.get_affiliated_institutions()) - initiator_permissions = draft.contributor_set.get(user=user).permission + + current_contributors = draft.contributor_set.all() + for contributor in current_contributors: signals.contributor_added.send( draft, - contributor=user, - auth=None, - notification_type=notification_type, - permissions=initiator_permissions + contributor=contributor.user, + auth=Auth(user) if user != contributor.user else None, + notification_type=notification_type if contributor.user.is_confirmed else NotificationType.Type.USER_INVITE_DRAFT_REGISTRATION, + permissions=contributor.permission ) return draft From f3fb97d2c26ba7e7c85387bc96fb303c2b9d9496 Mon Sep 17 00:00:00 2001 From: Ostap Zherebetskyi Date: Wed, 31 Dec 2025 19:06:50 +0200 Subject: [PATCH 2/3] fix unit tests --- api_tests/base/test_serializers.py | 5 +- .../views/test_draft_registration_list.py | 28 +++--- .../test_node_draft_registration_list.py | 96 ++++++++++--------- osf_tests/test_draft_node.py | 13 +-- osf_tests/test_draft_registration.py | 30 +++--- osf_tests/test_node.py | 13 +-- osf_tests/test_private_link.py | 14 +-- osf_tests/test_registrations.py | 5 +- ...test_fix_registration_unclaimed_records.py | 10 +- tests/test_registrations/test_views.py | 3 +- 10 files changed, 115 insertions(+), 102 deletions(-) diff --git a/api_tests/base/test_serializers.py b/api_tests/base/test_serializers.py index b5371aee878..ad953494d36 100644 --- a/api_tests/base/test_serializers.py +++ b/api_tests/base/test_serializers.py @@ -11,7 +11,7 @@ from tests.base import ApiTestCase, DbTestCase from osf_tests import factories -from tests.utils import make_drf_request_with_version +from tests.utils import make_drf_request_with_version, capture_notifications from osf.models import RegistrationSchema @@ -142,7 +142,8 @@ def test_deprecation_warning_for_snake_case(self): } } } - res = self.app.post_json_api(url, payload, auth=user_auth.auth) + with capture_notifications(): + res = self.app.post_json_api(url, payload, auth=user_auth.auth) assert res.json['data']['type'] == 'draft-registrations' assert res.json['meta']['warning'] == 'As of API Version {0}, all types are now Kebab-case. {0} will accept snake_case, but this will be deprecated in future versions.'.format(KEBAB_CASE_VERSION) diff --git a/api_tests/draft_registrations/views/test_draft_registration_list.py b/api_tests/draft_registrations/views/test_draft_registration_list.py index 21b00ff3e4a..4aed087605a 100644 --- a/api_tests/draft_registrations/views/test_draft_registration_list.py +++ b/api_tests/draft_registrations/views/test_draft_registration_list.py @@ -274,7 +274,8 @@ def test_draft_registration_attributes_copied_from_node( res = app.post_json_api(url_draft_registrations, payload_alt, auth=read_contrib.auth, expect_errors=True) assert res.status_code == 403 - res = app.post_json_api(url_draft_registrations, payload_alt, auth=user.auth) + with capture_notifications(): + res = app.post_json_api(url_draft_registrations, payload_alt, auth=user.auth) assert res.status_code == 201 attributes = res.json['data']['attributes'] assert attributes['title'] == project_public.title @@ -336,9 +337,6 @@ def test_logged_in_non_contributor_cannot_create_draft( ) assert res.status_code == 403 - def test_create_project_based_draft_does_not_email_initiator(self, app, user, url_draft_registrations, payload): - app.post_json_api(f'{url_draft_registrations}?embed=branched_from&embed=initiator', payload, auth=user.auth) - def test_affiliated_institutions_are_copied_from_node_no_institutions(self, app, user, url_draft_registrations, payload): """ Draft registrations that are based on projects get those project's user institutional affiliation, @@ -349,11 +347,12 @@ def test_affiliated_institutions_are_copied_from_node_no_institutions(self, app, """ project = ProjectFactory(is_public=True, creator=user) payload['data']['relationships']['branched_from']['data']['id'] = project._id - res = app.post_json_api( - url_draft_registrations, - payload, - auth=user.auth, - ) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, + auth=user.auth, + ) assert res.status_code == 201 draft_registration = DraftRegistration.load(res.json['data']['id']) assert not draft_registration.affiliated_institutions.exists() @@ -371,11 +370,12 @@ def test_affiliated_institutions_are_copied_from_node(self, app, user, url_draft project = ProjectFactory(is_public=True, creator=user) project.affiliated_institutions.add(institution) payload['data']['relationships']['branched_from']['data']['id'] = project._id - res = app.post_json_api( - url_draft_registrations, - payload, - auth=user.auth, - ) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, + auth=user.auth, + ) assert res.status_code == 201 draft_registration = DraftRegistration.load(res.json['data']['id']) assert list(draft_registration.affiliated_institutions.all()) == list(project.affiliated_institutions.all()) diff --git a/api_tests/nodes/views/test_node_draft_registration_list.py b/api_tests/nodes/views/test_node_draft_registration_list.py index 08099337dfd..ba33c6434b5 100644 --- a/api_tests/nodes/views/test_node_draft_registration_list.py +++ b/api_tests/nodes/views/test_node_draft_registration_list.py @@ -16,6 +16,7 @@ from osf.utils import permissions from website.project.metadata.utils import create_jsonschema_from_metaschema from website import settings +from tests.utils import capture_notifications OPEN_ENDED_SCHEMA_VERSION = 3 SCHEMA_VERSION = 2 @@ -334,11 +335,12 @@ def test_type_is_draft_registrations(self, app, user, metaschema_open_ended, url def test_admin_can_create_draft( self, app, user, project_public, url_draft_registrations, payload, metaschema_open_ended ): - res = app.post_json_api( - f'{url_draft_registrations}&embed=branched_from&embed=initiator', - payload, - auth=user.auth - ) + with capture_notifications(): + res = app.post_json_api( + f'{url_draft_registrations}&embed=branched_from&embed=initiator', + payload, + auth=user.auth + ) assert res.status_code == 201 data = res.json['data'] assert metaschema_open_ended._id in data['relationships']['registration_schema']['links']['related']['href'] @@ -407,11 +409,11 @@ def test_schema_validation( assert res.status_code == 400 payload['data']['relationships']['registration_schema']['data']['id'] = schema._id - - res = app.post_json_api( - url_draft_registrations, - payload, - auth=user.auth) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, + auth=user.auth) assert res.status_code == 201 # Non-Default provider does not accept everything @@ -590,10 +592,11 @@ def test_supply_registration_responses_on_creation( 'datacompletion': 'No, data collection has not begun', 'comments': '' } - res = app.post_json_api( - url_draft_registrations, - payload, auth=user.auth, - expect_errors=True) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, auth=user.auth, + expect_errors=True) attributes = res.json['data']['attributes'] assert attributes['registration_responses'] == { @@ -641,10 +644,11 @@ def test_registration_metadata_question_values_must_be_dictionaries( payload['data']['attributes']['registration_metadata'] = {} payload['data']['attributes']['registration_metadata']['datacompletion'] = 'No, data collection has not begun' - res = app.post_json_api( - url_draft_registrations, - payload, auth=user.auth, - expect_errors=True) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, auth=user.auth, + expect_errors=True) errors = res.json['errors'][0] assert res.status_code == 400 assert errors['detail'] == 'For your registration your response to the \'Data collection status\' field' \ @@ -661,10 +665,11 @@ def test_registration_metadata_question_keys_must_be_value( payload['data']['attributes']['registration_metadata']['datacompletion'] = { 'incorrect_key': 'No, data collection has not begun'} - res = app.post_json_api( - url_draft_registrations, - payload, auth=user.auth, - expect_errors=True) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, auth=user.auth, + expect_errors=True) errors = res.json['errors'][0] assert res.status_code == 400 assert errors['detail'] == 'For your registration your response to the \'Data collection status\' ' \ @@ -682,10 +687,11 @@ def test_question_in_registration_metadata_must_be_in_schema( 'value': 'No, data collection has not begun' } - res = app.post_json_api( - url_draft_registrations, - payload, auth=user.auth, - expect_errors=True) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, auth=user.auth, + expect_errors=True) errors = res.json['errors'][0] assert res.status_code == 400 assert errors['detail'] == 'For your registration the \'datacompletion\' field is extraneous and not' \ @@ -701,11 +707,11 @@ def test_multiple_choice_question_value_must_match_value_in_schema( payload['data']['attributes']['registration_metadata'] = {} payload['data']['attributes']['registration_metadata']['datacompletion'] = { 'value': 'Nope, data collection has not begun'} - - res = app.post_json_api( - url_draft_registrations, - payload, auth=user.auth, - expect_errors=True) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, auth=user.auth, + expect_errors=True) errors = res.json['errors'][0] assert res.status_code == 400 assert errors['detail'] == 'For your registration your response to the \'Data collection status\'' \ @@ -733,10 +739,11 @@ def test_registration_responses_question_values_must_not_be_dictionaries( payload['data']['attributes']['registration_responses'] = {} payload['data']['attributes']['registration_responses']['datacompletion'] = {'value': 'No, data collection has not begun'} - res = app.post_json_api( - url_draft_registrations, - payload, auth=user.auth, - expect_errors=True) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, auth=user.auth, + expect_errors=True) errors = res.json['errors'][0] assert res.status_code == 400 assert errors['detail'] == 'For your registration, your response to the \'Data collection status\' field' \ @@ -752,10 +759,11 @@ def test_question_in_registration_responses_must_be_in_schema( payload['data']['attributes']['registration_responses'] = {} payload['data']['attributes']['registration_responses']['q11'] = 'No, data collection has not begun' - res = app.post_json_api( - url_draft_registrations, - payload, auth=user.auth, - expect_errors=True) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, auth=user.auth, + expect_errors=True) errors = res.json['errors'][0] assert res.status_code == 400 assert errors['detail'] == 'Additional properties are not allowed (\'q11\' was unexpected)' @@ -769,11 +777,11 @@ def test_registration_responses_multiple_choice_question_value_must_match_value_ payload['data']['relationships']['registration_schema']['data']['id'] = schema._id payload['data']['attributes']['registration_responses'] = {} payload['data']['attributes']['registration_responses']['datacompletion'] = 'Nope, data collection has not begun' - - res = app.post_json_api( - url_draft_registrations, - payload, auth=user.auth, - expect_errors=True) + with capture_notifications(): + res = app.post_json_api( + url_draft_registrations, + payload, auth=user.auth, + expect_errors=True) errors = res.json['errors'][0] assert res.status_code == 400 assert errors['detail'] == 'For your registration, your response to the \'Data collection status\'' \ diff --git a/osf_tests/test_draft_node.py b/osf_tests/test_draft_node.py index 29ddbcf4ab8..9f549fd2a12 100644 --- a/osf_tests/test_draft_node.py +++ b/osf_tests/test_draft_node.py @@ -82,12 +82,13 @@ def license(): def make_complex_draft_registration(title, institution, description, category, write_contrib, license, subject, user): def make_draft_registration(node=None): - draft_registration = DraftRegistration.create_from_node( - user=user, - schema=get_default_metaschema(), - data={}, - node=node if node else None - ) + with capture_notifications(): + draft_registration = DraftRegistration.create_from_node( + user=user, + schema=get_default_metaschema(), + data={}, + node=node if node else None + ) user.add_or_update_affiliated_institution(institution) draft_registration.set_title(title, Auth(user)) draft_registration.set_description(description, Auth(user)) diff --git a/osf_tests/test_draft_registration.py b/osf_tests/test_draft_registration.py index 0672d81fdc8..7d6938814eb 100644 --- a/osf_tests/test_draft_registration.py +++ b/osf_tests/test_draft_registration.py @@ -221,12 +221,12 @@ def test_create_from_node_existing(self, user): node.set_subjects([[subject._id]], auth=Auth(node.creator)) node.affiliated_institutions.add(institution) node.save() - - draft = DraftRegistration.create_from_node( - node=node, - user=user, - schema=factories.get_default_metaschema(), - ) + with capture_notifications(): + draft = DraftRegistration.create_from_node( + node=node, + user=user, + schema=factories.get_default_metaschema(), + ) # Assert existing metadata-like node attributes are copied to the draft assert draft.title == title @@ -292,15 +292,15 @@ def test_can_view_property(self, user): write_contrib = factories.UserFactory() read_contrib = factories.UserFactory() non_contrib = factories.UserFactory() - - draft = DraftRegistration.create_from_node( - user=user, - node=project, - schema=factories.get_default_metaschema() - ) - project.add_contributor(non_contrib, ADMIN, save=True) - draft.add_contributor(write_contrib, WRITE, save=True) - draft.add_contributor(read_contrib, READ, save=True) + with capture_notifications(): + draft = DraftRegistration.create_from_node( + user=user, + node=project, + schema=factories.get_default_metaschema() + ) + project.add_contributor(non_contrib, ADMIN, save=True) + draft.add_contributor(write_contrib, WRITE, save=True) + draft.add_contributor(read_contrib, READ, save=True) assert draft.get_permissions(user) == [READ, WRITE, ADMIN] assert draft.get_permissions(write_contrib) == [READ, WRITE] diff --git a/osf_tests/test_node.py b/osf_tests/test_node.py index bf08a8ed208..6b52539c491 100644 --- a/osf_tests/test_node.py +++ b/osf_tests/test_node.py @@ -2565,12 +2565,13 @@ def test_create_from_node(self): user = proj.creator schema = RegistrationSchema.objects.first() data = {'some': 'data'} - draft = DraftRegistration.create_from_node( - node=proj, - user=user, - schema=schema, - data=data, - ) + with capture_notifications(): + draft = DraftRegistration.create_from_node( + node=proj, + user=user, + schema=schema, + data=data, + ) assert user == draft.initiator assert schema == draft.registration_schema assert data == draft.registration_metadata diff --git a/osf_tests/test_private_link.py b/osf_tests/test_private_link.py index 71230382b47..29a35f721d0 100644 --- a/osf_tests/test_private_link.py +++ b/osf_tests/test_private_link.py @@ -4,6 +4,7 @@ from .factories import PrivateLinkFactory, NodeFactory from osf.models import RegistrationSchema, DraftRegistration, NodeLog +from tests.utils import capture_notifications @pytest.mark.django_db def test_factory(): @@ -50,12 +51,13 @@ def test_create_from_node(self): user = proj.creator schema = RegistrationSchema.objects.first() data = {'some': 'data'} - draft = DraftRegistration.create_from_node( - node=proj, - user=user, - schema=schema, - data=data, - ) + with capture_notifications(): + draft = DraftRegistration.create_from_node( + node=proj, + user=user, + schema=schema, + data=data, + ) assert user == draft.initiator assert schema == draft.registration_schema assert data == draft.registration_metadata diff --git a/osf_tests/test_registrations.py b/osf_tests/test_registrations.py index 268377e9c2b..d64af521982 100644 --- a/osf_tests/test_registrations.py +++ b/osf_tests/test_registrations.py @@ -447,8 +447,9 @@ def contributor_unregistered_no_email(self, user, auth, project_two, component): @pytest.fixture() def registration(self, project_two, component, contributor_unregistered, contributor_unregistered_no_email): - with mock_archive(project_two, autoapprove=True) as registration: - return registration + with capture_notifications(): + with mock_archive(project_two, autoapprove=True) as registration: + return registration @pytest.mark.usefixtures('mock_gravy_valet_get_verified_links') def test_unregistered_contributors_unclaimed_records_get_copied(self, user, project, component, registration, contributor_unregistered, contributor_unregistered_no_email): diff --git a/scripts/tests/test_fix_registration_unclaimed_records.py b/scripts/tests/test_fix_registration_unclaimed_records.py index 4d67b70f492..8f3cb98536d 100644 --- a/scripts/tests/test_fix_registration_unclaimed_records.py +++ b/scripts/tests/test_fix_registration_unclaimed_records.py @@ -4,6 +4,7 @@ from osf_tests.factories import PreprintFactory, UserFactory, ProjectFactory from scripts.fix_registration_unclaimed_records import main as fix_records_script from osf_tests.utils import mock_archive +from tests.utils import capture_notifications pytestmark = pytest.mark.django_db @@ -13,10 +14,6 @@ class TestFixRegistrationUnclaimedRecords: def user(self): return UserFactory() - @pytest.fixture() - def project(self, user, auth, fake): - return ret - @pytest.fixture() def auth(self, user): return Auth(user) @@ -49,8 +46,9 @@ def contributor_unregistered_no_email(self, user, auth, project): @pytest.fixture() def registration(self, project, contributor_unregistered, contributor_unregistered_no_email): - with mock_archive(project, autoapprove=True) as registration: - return registration + with capture_notifications(): + with mock_archive(project, autoapprove=True) as registration: + return registration @pytest.mark.usefixtures('mock_gravy_valet_get_verified_links') def test_migrate_bad_data(self, user, project, registration, contributor_unregistered, contributor_unregistered_no_email): diff --git a/tests/test_registrations/test_views.py b/tests/test_registrations/test_views.py index 394af84e3cb..052a54ff11c 100644 --- a/tests/test_registrations/test_views.py +++ b/tests/test_registrations/test_views.py @@ -174,7 +174,8 @@ def test_new_draft_registration_POST(self): } url = target.web_url_for('new_draft_registration') - res = self.app.post(url, data=payload, auth=self.user.auth) + with capture_notifications(): + res = self.app.post(url, data=payload, auth=self.user.auth) assert res.status_code == http_status.HTTP_302_FOUND target.reload() draft = DraftRegistration.objects.get(branched_from=target) From 2552d7e5b500134aa1e39b7b8ca07d2dd90193b7 Mon Sep 17 00:00:00 2001 From: Ostap Zherebetskyi Date: Wed, 31 Dec 2025 19:31:10 +0200 Subject: [PATCH 3/3] fix unit tests --- osf_tests/test_draft_node.py | 3 +-- osf_tests/test_registration_moderation_notifications.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/osf_tests/test_draft_node.py b/osf_tests/test_draft_node.py index 9f549fd2a12..2ed26e4e7d3 100644 --- a/osf_tests/test_draft_node.py +++ b/osf_tests/test_draft_node.py @@ -148,8 +148,7 @@ def test_register_draft_node(self, user, draft_node, draft_registration): def test_draft_registration_fields_are_copied_back_to_draft_node(self, user, institution, subject, write_contrib, title, description, category, license, make_complex_draft_registration): - with capture_notifications(): - draft_registration = make_complex_draft_registration() + draft_registration = make_complex_draft_registration() draft_node = draft_registration.branched_from with disconnected_from_listeners(after_create_registration): diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index a18e5c97da0..ca16b4eb993 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -140,7 +140,7 @@ def test_submit_notifications(self, registration, moderator, admin, contrib, pro assert notification['emits'][1]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION assert notification['emits'][1]['kwargs']['user'] == contrib assert notification['emits'][2]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS - assert NotificationSubscription.objects.count() == 6 + assert NotificationSubscription.objects.count() == 7 digest = NotificationSubscription.objects.last() assert digest.user == moderator