diff --git a/api_tests/guids/views/test_guid_detail.py b/api_tests/guids/views/test_guid_detail.py index 4a6db0fbc35..f52ff10e215 100644 --- a/api_tests/guids/views/test_guid_detail.py +++ b/api_tests/guids/views/test_guid_detail.py @@ -12,6 +12,7 @@ PrivateLinkFactory, ) from website.settings import API_DOMAIN +from tests.utils import capture_notifications @pytest.mark.django_db @@ -32,13 +33,14 @@ def registration(self): @pytest.fixture() def versioned_preprint(self, user): preprint = PreprintFactory(reviews_workflow='pre-moderation') - PreprintFactory.create_version( - create_from=preprint, - creator=user, - final_machine_state='accepted', - is_published=True, - set_doi=False - ) + with capture_notifications(): + PreprintFactory.create_version( + create_from=preprint, + creator=user, + final_machine_state='accepted', + is_published=True, + set_doi=False + ) return preprint def test_redirects(self, app, project, registration, user): diff --git a/api_tests/mailhog/provider/test_preprints.py b/api_tests/mailhog/provider/test_preprints.py index db514c87c34..96d8a8c099c 100644 --- a/api_tests/mailhog/provider/test_preprints.py +++ b/api_tests/mailhog/provider/test_preprints.py @@ -2,7 +2,7 @@ from osf import features from framework.auth.core import Auth -from osf.models import NotificationType, Notification +from osf.models import NotificationType from osf_tests.factories import ( ProjectFactory, AuthUserFactory, @@ -12,7 +12,6 @@ from osf.utils.permissions import WRITE from tests.base import OsfTestCase from tests.utils import get_mailhog_messages, delete_mailhog_messages, capture_notifications, assert_emails -from notifications.tasks import send_users_instant_digest_email class TestPreprintConfirmationEmails(OsfTestCase): @@ -35,13 +34,7 @@ def test_creator_gets_email(self): assert len(notifications['emits']) == 1 assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION messages = get_mailhog_messages() - assert not messages['items'] - assert Notification.objects.all() - with capture_notifications(passthrough=True) as notifications: - send_users_instant_digest_email.delay() - - messages = get_mailhog_messages() - assert messages['count'] == len(notifications['emits']) + assert_emails(messages, notifications) delete_mailhog_messages() with capture_notifications(passthrough=True) as notifications: @@ -49,11 +42,6 @@ def test_creator_gets_email(self): assert len(notifications['emits']) == 1 assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION messages = get_mailhog_messages() - assert not messages['items'] - with capture_notifications(passthrough=True) as notifications: - send_users_instant_digest_email.delay() - massages = get_mailhog_messages() - assert massages['count'] == len(notifications['emits']) - assert_emails(massages, notifications) + assert_emails(messages, notifications) delete_mailhog_messages() diff --git a/api_tests/mailhog/provider/test_submissions.py b/api_tests/mailhog/provider/test_submissions.py index caa0abe71b0..b9d26e155df 100644 --- a/api_tests/mailhog/provider/test_submissions.py +++ b/api_tests/mailhog/provider/test_submissions.py @@ -21,7 +21,7 @@ from osf.models import NotificationType from osf.migrations import update_provider_auth_groups -from tests.utils import capture_notifications, get_mailhog_messages, delete_mailhog_messages +from tests.utils import capture_notifications, get_mailhog_messages, delete_mailhog_messages, assert_emails @pytest.mark.django_db @@ -85,8 +85,7 @@ def test_get_registration_actions(self, app, registration_actions_url, registrat assert notifications['emits'][0]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS assert notifications['emits'][1]['type'] == NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS messages = get_mailhog_messages() - assert messages['count'] == 1 - assert messages['items'][0]['Content']['Headers']['To'][0] == registration.creator.username + assert_emails(messages, notifications) delete_mailhog_messages() diff --git a/api_tests/share/test_share_preprint.py b/api_tests/share/test_share_preprint.py index cf0c8a3d92d..118abf3105b 100644 --- a/api_tests/share/test_share_preprint.py +++ b/api_tests/share/test_share_preprint.py @@ -18,6 +18,7 @@ from website import settings from website.preprints.tasks import on_preprint_updated from ._utils import expect_preprint_ingest_request +from tests.utils import capture_notifications @pytest.mark.django_db @@ -72,54 +73,59 @@ def test_save_unpublished_not_called(self, mock_share_responses, preprint): preprint.save() 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) + with capture_notifications(): + 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_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_responses, preprint, delete=True): - preprint.is_published = False - preprint.save(**{'force_update': True}) + with capture_notifications(): + with expect_preprint_ingest_request(mock_share_responses, preprint): + preprint.set_published(True, auth=auth, save=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_responses, auth, preprint, subject, subject_two): - preprint.set_published(True, auth=auth, save=True) - with expect_preprint_ingest_request(mock_share_responses, preprint): - preprint.set_subjects([[subject_two._id]], auth=auth) + with capture_notifications(): + preprint.set_published(True, auth=auth, save=True) + 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_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_responses, auth, preprint): - preprint.set_published(True, auth=auth, save=True) - with expect_preprint_ingest_request(mock_share_responses, preprint): - on_preprint_updated(preprint._id, saved_fields=['title']) + with capture_notifications(): + preprint.set_published(True, auth=auth, save=True) + 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_responses, user, auth): - preprint = PreprintFactory(is_published=True, creator=user) - preprint.set_published(True, auth=auth, save=True) - user2 = AuthUserFactory() + with capture_notifications(): + preprint = PreprintFactory(is_published=True, creator=user) + preprint.set_published(True, auth=auth, save=True) + user2 = AuthUserFactory() - 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_responses, preprint): + preprint.add_contributor(contributor=user2, auth=auth, save=True) - with expect_preprint_ingest_request(mock_share_responses, preprint): - preprint.move_contributor(contributor=user, index=0, auth=auth, save=True) + 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}] + data = [{'id': user._id, 'permissions': ADMIN, 'visible': True}, + {'id': user2._id, 'permissions': WRITE, 'visible': False}] - with expect_preprint_ingest_request(mock_share_responses, preprint): - preprint.manage_contributors(data, auth=auth, save=True) + with expect_preprint_ingest_request(mock_share_responses, preprint): + preprint.manage_contributors(data, auth=auth, save=True) - 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_responses, preprint): + preprint.update_contributor(user2, READ, True, auth=auth, save=True) - with expect_preprint_ingest_request(mock_share_responses, preprint): - preprint.remove_contributor(contributor=user2, auth=auth) + with expect_preprint_ingest_request(mock_share_responses, preprint): + preprint.remove_contributor(contributor=user2, auth=auth) @pytest.mark.skip('Synchronous retries not supported if celery >=5.0') def test_call_async_update_on_500_failure(self, mock_share_responses, preprint, auth): @@ -129,10 +135,11 @@ def test_call_async_update_on_500_failure(self, mock_share_responses, preprint, preprint.update_search() 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_responses, preprint, count=1, error_response=True): - preprint.update_search() + with capture_notifications(): + 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_responses, preprint, count=1, error_response=True): + preprint.update_search() def test_delete_from_share(self, mock_share_responses): preprint = PreprintFactory() diff --git a/notifications/listeners.py b/notifications/listeners.py index 62ad487b358..97eba256c53 100644 --- a/notifications/listeners.py +++ b/notifications/listeners.py @@ -102,7 +102,8 @@ def reviews_withdraw_requests_notification_moderators(self, timestamp, context, NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.instance.emit( subscribed_object=provider, user=user, - event_context=context + event_context=context, + is_digest=True, ) diff --git a/osf/models/notification_type.py b/osf/models/notification_type.py index fbbe2524f99..f8900ad4ec5 100644 --- a/osf/models/notification_type.py +++ b/osf/models/notification_type.py @@ -151,6 +151,30 @@ def instance(self): obj, created = NotificationType.objects.get_or_create(name=self.value) return obj + @property + def is_digest_type(self): + digest_types = { + # User types + NotificationType.Type.USER_NO_ADDON.value, + # File types + NotificationType.Type.ADDON_FILE_COPIED.value, + NotificationType.Type.ADDON_FILE_MOVED.value, + NotificationType.Type.ADDON_FILE_RENAMED.value, + NotificationType.Type.FILE_ADDED.value, + NotificationType.Type.FILE_REMOVED.value, + NotificationType.Type.FILE_UPDATED.value, + NotificationType.Type.FOLDER_CREATED.value, + NotificationType.Type.NODE_FILE_UPDATED.value, + NotificationType.Type.USER_FILE_UPDATED.value, + + # Review types + NotificationType.Type.COLLECTION_SUBMISSION_SUBMITTED.value, + NotificationType.Type.PROVIDER_NEW_PENDING_SUBMISSIONS.value, + NotificationType.Type.PROVIDER_NEW_PENDING_WITHDRAW_REQUESTS.value, + NotificationType.Type.REVIEWS_SUBMISSION_STATUS.value, + } + return self.name in digest_types + notification_interval_choices = ArrayField( base_field=models.CharField(max_length=32), default=get_default_frequency_choices, @@ -204,6 +228,12 @@ def emit( from osf.models.notification_subscription import NotificationSubscription from osf.models.provider import AbstractProvider + if is_digest != self.is_digest_type: + sentry.log_message(f'NotificationType.emit called with is_digest={is_digest} for ' + f'NotificationType {self.name} which has is_digest_type={self.is_digest_type}' + 'is_digest value will be overridden.') + is_digest = self.is_digest_type + # use concrete model for AbstractProvider to specifically get the provider content type if isinstance(subscribed_object, AbstractProvider): content_type = ContentType.objects.get_for_model(subscribed_object, for_concrete_model=False) if subscribed_object else None diff --git a/osf/models/preprint.py b/osf/models/preprint.py index 7a61b31db06..9938a0e7147 100644 --- a/osf/models/preprint.py +++ b/osf/models/preprint.py @@ -1094,7 +1094,6 @@ def _send_preprint_confirmation(self, auth): 'document_type': self.provider.preprint_word, 'notify_comment': not self.provider.reviews_comments_private }, - is_digest=True ) # FOLLOWING BEHAVIOR NOT SPECIFIC TO PREPRINTS