From d4007faba646fce948795fdeab542d28d7b1a2e8 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Mon, 14 Sep 2026 19:55:46 +0300 Subject: [PATCH 1/4] send emails to users for approval --- osf/models/registrations.py | 14 +++++-- osf_tests/test_sanctions.py | 74 +++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/osf/models/registrations.py b/osf/models/registrations.py index 42795dc6a62..56755f06ee3 100644 --- a/osf/models/registrations.py +++ b/osf/models/registrations.py @@ -475,14 +475,17 @@ def _initiate_approval(self, user, notify_initiator_on_complete=False): notify_initiator_on_complete=notify_initiator_on_complete ) self.save() # Set foreign field reference Node.registration_approval - admins = self.get_admin_contributors_recursive(unique_users=True) + admins = list(self.get_admin_contributors_recursive(unique_users=True)) for (admin, node) in admins: self.registration_approval.add_authorizer(admin, node=node) self.registration_approval.save() # Save approval's approval_state + try: + self.registration_approval.ask(admins) + except Exception: + logger.exception(f'Failed to emit registration approval notifications for registration: {self._id}') return self.registration_approval def require_approval(self, user, notify_initiator_on_complete=False): - if not self.is_registration: raise NodeStateError('Only registrations can require registration approval') if not self.is_admin_contributor(user): @@ -520,10 +523,15 @@ def _initiate_embargo(self, user, end_date, for_existing_registration=False, ) self.update_moderation_state() self.save() # Set foreign field reference Node.embargo - admins = self.get_admin_contributors_recursive(unique_users=True) + admins = list(self.get_admin_contributors_recursive(unique_users=True)) for (admin, node) in admins: self.embargo.add_authorizer(admin, node) self.embargo.save() # Save embargo's approval_state + + try: + self.embargo.ask(admins) + except Exception: + logger.exception(f'Failed to emit registration embargo notifications for registration: {self._id}') return self.embargo def embargo_registration(self, user, end_date, for_existing_registration=False, diff --git a/osf_tests/test_sanctions.py b/osf_tests/test_sanctions.py index 02e6548958b..782fdb16f5b 100644 --- a/osf_tests/test_sanctions.py +++ b/osf_tests/test_sanctions.py @@ -176,6 +176,80 @@ def test_render_non_admin_emails( assert True # mail rendered successfully +@pytest.mark.django_db +class TestSanctionAskNotification: + + @pytest.fixture + def contributor(self): + return factories.AuthUserFactory() + + @pytest.fixture() + def registration_approval_registration(self, request, contributor): + sanction = factories.RegistrationApprovalFactory() + registration = sanction.target_registration + registration.add_contributor(contributor) + registration.save() + return registration + + @pytest.fixture() + def embargo_registration(self, request, contributor): + sanction = factories.EmbargoFactory(end_date=timezone.now()) + registration = sanction.target_registration + registration.add_contributor(contributor) + registration.save() + return registration + + @pytest.mark.parametrize('reviews_workflow', [None, 'pre-moderation']) + @pytest.mark.parametrize('branched_from_node', [True, False]) + def test_registration_approval_creates_notifications_for_approval_for_contributors(self, reviews_workflow, branched_from_node, registration_approval_registration): + registration = registration_approval_registration + provider = registration.provider + provider.reviews_workflow = reviews_workflow + provider.save() + + registration.branched_from_node = branched_from_node + registration.save() + + admin_contributor = factories.AuthUserFactory() + registration.add_contributor(admin_contributor, permissions.ADMIN) + with capture_notifications() as notifications: + registration.require_approval(registration.creator) + + # ensure RegistrationCreateSerializer sends emails to admin contributors + real_admins = {registration.creator, admin_contributor} + admins_with_notifications = set() + assert len(notifications['emits']) == 2 + for emit in notifications['emits']: + admins_with_notifications.add(emit['kwargs']['user']) + + assert real_admins == admins_with_notifications + + @pytest.mark.parametrize('reviews_workflow', [None, 'pre-moderation']) + @pytest.mark.parametrize('branched_from_node', [True, False]) + def test_embargo_creates_notifications_for_approval_for_contributors(self, reviews_workflow, branched_from_node, embargo_registration): + registration = embargo_registration + provider = registration.provider + provider.reviews_workflow = reviews_workflow + provider.save() + + registration.branched_from_node = branched_from_node + registration.save() + + admin_contributor = factories.AuthUserFactory() + registration.add_contributor(admin_contributor, permissions.ADMIN) + with capture_notifications() as notifications: + registration.embargo_registration(registration.creator, timezone.now() + datetime.timedelta(days=10)) + + # ensure RegistrationCreateSerializer sends emails to admin contributors + real_admins = {registration.creator, admin_contributor} + admins_with_notifications = set() + assert len(notifications['emits']) == 2 + for emit in notifications['emits']: + admins_with_notifications.add(emit['kwargs']['user']) + + assert real_admins == admins_with_notifications + + @pytest.mark.django_db @pytest.mark.usefixtures('mock_gravy_valet_get_verified_links') class TestDOICreation: From d6cdb70882eb8077f5e02effe7fbecedc546b43e Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Tue, 15 Sep 2026 16:28:21 +0300 Subject: [PATCH 2/4] fixed tests --- .../mailhog/provider/test_submissions.py | 33 ++++++++++++++----- ...t_registration_moderation_notifications.py | 6 +++- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/api_tests/mailhog/provider/test_submissions.py b/api_tests/mailhog/provider/test_submissions.py index 542d4410a99..b44921aaa7f 100644 --- a/api_tests/mailhog/provider/test_submissions.py +++ b/api_tests/mailhog/provider/test_submissions.py @@ -18,7 +18,7 @@ from tests.base import get_default_metaschema -from osf.models import NotificationTypeEnum +from osf.models import NotificationTypeEnum, Notification from osf.migrations import update_provider_auth_groups from tests.utils import capture_notifications, get_mailhog_messages, delete_mailhog_messages, assert_emails @@ -99,24 +99,41 @@ def test_get_provider_actions(self, app, provider_actions_url, registration, mod assert resp.status_code == 401 resp = app.get(provider_actions_url, auth=moderator.auth) - assert resp.status_code == 200 assert len(resp.json['data']) == 0 - delete_mailhog_messages() - with capture_notifications(passthrough=True) as notifications: + # registration fixture asks the creator for approval + assert Notification.objects.count() == 1 + another_contributor = AuthUserFactory() + registration.add_contributor(another_contributor, permissions='admin', visible=True) + + delete_mailhog_messages() + with capture_notifications() as notifications: + # 2 notifications: creator and another contributor are notified of node_pending_registration_admin registration.require_approval(user=registration.creator) approval = registration.registration_approval + # approve the registration to trigger the notification to the provider moderators + # 2 notifications: creator and another contributor are notified of provider_reviews_submission_confirmation approval.approve( user=registration.creator, token=approval.token_for_user(registration.creator, 'approval') ) - + approval.approve( + user=another_contributor, + token=approval.token_for_user(another_contributor, 'approval') + ) + # 1 notification after all approvals: provider moderator is notified about provider_new_pending_submissions resp = app.get(provider_actions_url, auth=moderator.auth) - assert len(notifications['emits']) == 2 - assert notifications['emits'][0]['type'] == NotificationTypeEnum.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION - assert notifications['emits'][1]['type'] == NotificationTypeEnum.PROVIDER_NEW_PENDING_SUBMISSIONS + assert len(notifications['emits']) == 5 + + notifications = [(notification['kwargs']['user'], notification['type']) for notification in notifications['emits']] + assert (registration.creator, NotificationTypeEnum.NODE_PENDING_REGISTRATION_ADMIN) in notifications + assert (another_contributor, NotificationTypeEnum.NODE_PENDING_REGISTRATION_ADMIN) in notifications + assert (registration.creator, NotificationTypeEnum.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION) in notifications + assert (another_contributor, NotificationTypeEnum.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION) in notifications + assert (moderator, NotificationTypeEnum.PROVIDER_NEW_PENDING_SUBMISSIONS) in notifications + send_users_instant_digest_email.delay() messages = get_mailhog_messages() assert messages['count'] == 1 diff --git a/osf_tests/test_registration_moderation_notifications.py b/osf_tests/test_registration_moderation_notifications.py index 5516344a2b1..b2e06e99c01 100644 --- a/osf_tests/test_registration_moderation_notifications.py +++ b/osf_tests/test_registration_moderation_notifications.py @@ -140,7 +140,11 @@ def test_submit_notifications(self, registration, moderator, admin, contrib, pro assert notification['emits'][1]['type'] == NotificationTypeEnum.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION assert notification['emits'][1]['kwargs']['user'] == contrib assert notification['emits'][2]['type'] == NotificationTypeEnum.PROVIDER_NEW_PENDING_SUBMISSIONS - assert NotificationSubscription.objects.count() == 7 + # under the hood registration fixture creates a registration and calls require_approval + # which emits NotificationTypeEnum.NODE_PENDING_REGISTRATION_ADMIN and creates a NotificationSubscription + # for the admin contributor and sends the creator a notification of type NotificationTypeEnum.NODE_PENDING_REGISTRATION_ADMIN + # so it's 8 subscriptions, not 7 + assert NotificationSubscription.objects.count() == 8 digest = NotificationSubscription.objects.last() assert digest.user == moderator From f2d5d51a39d426523f2ab1b52a45a1e99637cea7 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Tue, 15 Sep 2026 16:50:44 +0300 Subject: [PATCH 3/4] fixed missing parameter --- api_tests/mailhog/provider/test_submissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_tests/mailhog/provider/test_submissions.py b/api_tests/mailhog/provider/test_submissions.py index b44921aaa7f..fd2050cafe3 100644 --- a/api_tests/mailhog/provider/test_submissions.py +++ b/api_tests/mailhog/provider/test_submissions.py @@ -108,7 +108,7 @@ def test_get_provider_actions(self, app, provider_actions_url, registration, mod registration.add_contributor(another_contributor, permissions='admin', visible=True) delete_mailhog_messages() - with capture_notifications() as notifications: + with capture_notifications(passthrough=True) as notifications: # 2 notifications: creator and another contributor are notified of node_pending_registration_admin registration.require_approval(user=registration.creator) approval = registration.registration_approval From beab74fe788a5761bee2ac650717b5dfee16f390 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Tue, 15 Sep 2026 17:41:11 +0300 Subject: [PATCH 4/4] fixed test --- api_tests/mailhog/provider/test_submissions.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/api_tests/mailhog/provider/test_submissions.py b/api_tests/mailhog/provider/test_submissions.py index fd2050cafe3..97a9fccf250 100644 --- a/api_tests/mailhog/provider/test_submissions.py +++ b/api_tests/mailhog/provider/test_submissions.py @@ -136,8 +136,14 @@ def test_get_provider_actions(self, app, provider_actions_url, registration, mod send_users_instant_digest_email.delay() messages = get_mailhog_messages() - assert messages['count'] == 1 - assert messages['items'][0]['Content']['Headers']['To'][0] == registration.creator.username + assert messages['count'] == 4 + + # actions within capture_notifications triggered registration approval + submission confirmation emails + user_and_email_type = [(message['Content']['Headers']['To'][0], message['Content']['Headers']['Subject'][0]) for message in messages['items']] + assert (registration.creator.username, 'Pending Registration - Admin Notification') in user_and_email_type + assert (another_contributor.username, 'Pending Registration - Admin Notification') in user_and_email_type + assert (registration.creator.username, 'Submission Confirmation') in user_and_email_type + assert (another_contributor.username, 'Submission Confirmation') in user_and_email_type delete_mailhog_messages()