[ENG-11835] - Fix missing spam/ban notifications

admin_tests/nodes/test_views.py

@@ -52,6 +52,7 @@
 from api_tests.share._utils import mock_update_share
 from osf.models.files import Folder
 from tests.utils import capture_notifications
+from osf.models.notification_type import NotificationTypeEnum
 from website import settings
 from framework.auth.core import Auth
 
@@ -180,8 +181,11 @@ def test_node_spam_ham_workflow_if_node_is_public(self):
         guid = node._id
         request = RequestFactory().post('/fake_path')
         request.user = superuser
-        node = handle_post_view_request(request, NodeConfirmSpamView(), node, guid)
+        with capture_notifications() as notifications:
+            node = handle_post_view_request(request, NodeConfirmSpamView(), node, guid)
         assert not node.is_public
+        assert len(notifications['emits']) == 1
+        assert notifications['emits'][0]['type'] == NotificationTypeEnum.NODE_CONFIRMED_SPAM
         node = handle_post_view_request(request, NodeConfirmHamView(), node, guid)
         assert node.is_public
 

admin_tests/preprints/test_views.py

@@ -156,7 +156,8 @@ def test_confirm_spam(self, flagged_preprint, superuser, mock_akismet):
 
         view = views.PreprintConfirmSpamView()
         view = setup_view(view, request, guid=flagged_preprint._id)
-        view.post(request)
+        with assert_notification(type=NotificationTypeEnum.PREPRINT_CONFIRMED_SPAM):
+            view.post(request)
 
         assert flagged_preprint.is_public
         flagged_preprint.refresh_from_db()

admin_tests/users/test_views.py

@@ -247,7 +247,8 @@ def setUp(self):
 
     def test_disable_spam_user(self):
         settings.ENABLE_EMAIL_SUBSCRIPTIONS = False
-        self.view().post(self.request)
+        with assert_notification(type=NotificationTypeEnum.USER_SPAM_BANNED, user=self.user):
+            self.view().post(self.request)
         self.user.reload()
         self.public_node.reload()
         assert self.user.is_disabled

notifications.yaml

@@ -569,6 +569,13 @@ notification_types:
     template: 'website/templates/new_public_project.html.mako'
     tests: ['osf_tests/test_node.py']
 
+  - name: node_confirmed_spam
+    subject: 'Your OSF project has been flagged as spam'
+    __docs__: ...
+    object_content_type_model_name: abstractnode
+    template: 'website/templates/spam_resource_confirmed.html.mako'
+    tests: ['tests/test_spam_mixin.py']
+
 
   #### PREPRINT
   - name: preprint_contributor_added_preprint_node_from_osf
@@ -599,6 +606,13 @@ notification_types:
     template: 'website/templates/contributor_added_preprints.html.mako'
     tests: ['api_tests/preprints/views/test_preprint_contributors_list.py']
 
+  - name: preprint_confirmed_spam
+    subject: 'Your OSF preprint has been flagged as spam'
+    __docs__: ...
+    object_content_type_model_name: preprint
+    template: 'website/templates/spam_resource_confirmed.html.mako'
+    tests: ['tests/test_spam_mixin.py']
+
   #### SUPPORT
   #### Collection Submissions
   - name: collection_submission_removed_moderator

osf/external/spam/tasks.py

@@ -29,7 +29,9 @@ def reclassify_domain_references(notable_domain_id, current_note, previous_note)
         for item in references:
             item.is_triaged = current_note != NotableDomain.Note.UNKNOWN
             if current_note == NotableDomain.Note.EXCLUDE_FROM_ACCOUNT_CREATION_AND_CONTENT:
-                item.referrer.confirm_spam(save=False, domains=[domain.domain])
+                # Retroactive/bulk reclassification of a domain already flagged in the past --
+                # don't send a fresh "flagged as spam" email long after the fact.
+                item.referrer.confirm_spam(save=False, domains=[domain.domain], notify=False)
             elif previous_note == NotableDomain.Note.EXCLUDE_FROM_ACCOUNT_CREATION_AND_CONTENT:
                 try:
                     item.referrer.spam_data['domains'].remove(domain.domain)

osf/models/mixins.py

@@ -2285,12 +2285,14 @@ def unspam(self, save=False):
         super().unspam(save=save)
         self.undelete(save=save)
 
-    def confirm_spam(self, domains=None, save=True, train_spam_services=True):
+    def confirm_spam(self, domains=None, save=True, train_spam_services=True, notify=True):
         """
         This should add behavior specific nodes/preprints confirmed to be spam.
         :param save:
         :return:
         """
+        was_already_spam = self.spam_status == SpamStatus.SPAM
+
         super().confirm_spam(save=save, domains=domains or [], train_spam_services=train_spam_services)
         self.deleted = timezone.now()
         was_public = self.was_public_at_spam
@@ -2308,6 +2310,31 @@ def confirm_spam(self, domains=None, save=True, train_spam_services=True):
         if save:
             self.save()
 
+        if notify and not was_already_spam:
+            Preprint = apps.get_model('osf.Preprint')
+            Registration = apps.get_model('osf.Registration')
+            if isinstance(self, Preprint):
+                notification_type = NotificationTypeEnum.PREPRINT_CONFIRMED_SPAM
+                resource_type_label = 'preprint'
+            elif isinstance(self, Registration):
+                notification_type = NotificationTypeEnum.NODE_CONFIRMED_SPAM
+                resource_type_label = 'registration'
+            else:
+                notification_type = NotificationTypeEnum.NODE_CONFIRMED_SPAM
+                resource_type_label = 'project'
+            for contributor in self.contributors:
+                notification_type.instance.emit(
+                    user=contributor,
+                    subscribed_object=self,
+                    event_context={
+                        'user_fullname': contributor.fullname,
+                        'resource_title': self.title,
+                        'resource_absolute_url': self.absolute_url,
+                        'resource_type_label': resource_type_label,
+                        'osf_support_email': settings.OSF_SUPPORT_EMAIL,
+                    }
+                )
+
     def confirm_ham(self, save=False, train_spam_services=True):
         """
         This should add behavior specific nodes/preprints confirmed to be ham.
@@ -2403,29 +2430,22 @@ def suspend_spam_user(self, user, domains=None, self_spam=False):
 
         self.flag_spam(skip_user_suspension=True)
 
-        # Suspend the flagged user for spam.
-        if not user.is_disabled:
-            user.deactivate_account()
-            NotificationTypeEnum.USER_SPAM_BANNED.instance.emit(
-                user,
-                event_context={
-                    'user_fullname': user.fullname,
-                    'osf_support_email': settings.OSF_SUPPORT_EMAIL,
-                }
-            )
-
+        # Suspend the flagged user for spam. Deactivation and the account-ban
+        # notification are both handled inside user.confirm_spam().
         user.confirm_spam(domains=domains or [], save=False, skip_resources_spam=True)
         user.save()
 
-        # Make public nodes private from this contributor
+        # Make public nodes private from this contributor. These are a side effect
+        # of the account-level ban, so they don't get their own notification --
+        # the single account-ban email already covers it.
         for node in user.all_nodes:
             if (self._id != node._id or self_spam) and len(node.contributors) == 1 and node.is_public:
-                node.confirm_spam(save=True, domains=domains, train_spam_services=False)
+                node.confirm_spam(save=True, domains=domains, train_spam_services=False, notify=False)
 
         # Make preprints private from this contributor
         for preprint in user.preprints.all():
             if (self._id != preprint._id or self_spam) and len(preprint.contributors) == 1 and preprint.is_public:
-                preprint.confirm_spam(save=True, domains=domains, train_spam_services=False)
+                preprint.confirm_spam(save=True, domains=domains, train_spam_services=False, notify=False)
 
     def flag_spam(self, skip_user_suspension=False):
         """ Overrides SpamMixin#flag_spam.

osf/models/notification_type.py

@@ -107,6 +107,7 @@ class NotificationTypeEnum(str, Enum):
     NODE_WITHDRAWAl_REQUEST_APPROVED = 'node_withdrawal_request_approved'
     NODE_WITHDRAWAl_REQUEST_REJECTED = 'node_withdrawal_request_rejected'
     NODE_NEW_PUBLIC_PROJECT = 'node_new_public_project'
+    NODE_CONFIRMED_SPAM = 'node_confirmed_spam'
 
     FILE_UPDATED = 'file_updated'
     FILE_ADDED = 'file_added'
@@ -131,6 +132,7 @@ class NotificationTypeEnum(str, Enum):
     PREPRINT_REQUEST_WITHDRAWAL_DECLINED = 'preprint_request_withdrawal_declined'
     PREPRINT_CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF = 'preprint_contributor_added_preprint_node_from_osf'
     PREPRINT_CONTRIBUTOR_ADDED_DEFAULT = 'preprint_contributor_added_default'
+    PREPRINT_CONFIRMED_SPAM = 'preprint_confirmed_spam'
 
     # Collections Submission notifications
     COLLECTION_SUBMISSION_REMOVED_ADMIN = 'collection_submission_removed_admin'

osf/models/spam.py

@@ -165,7 +165,7 @@ def confirm_ham(self, save=False, train_spam_services=True):
                 )
             )
 
-    def confirm_spam(self, domains=None, save=True, train_spam_services=True):
+    def confirm_spam(self, domains=None, save=True, train_spam_services=True, notify=True):
         if domains:
             if 'domains' in self.spam_data:
                 self.spam_data['domains'].extend(domains)

osf/models/user.py

@@ -1475,18 +1475,30 @@ def confirm_email(self, token, merge=False):
 
         return True
 
-    def confirm_spam(self, domains=None, save=True, train_spam_services=False, skip_resources_spam=False):
+    def confirm_spam(self, domains=None, save=True, train_spam_services=False, skip_resources_spam=False, notify=True):
+        was_disabled = self.is_disabled
         self.deactivate_account()
         super().confirm_spam(domains=domains, save=save, train_spam_services=train_spam_services)
 
+        if notify and not was_disabled:
+            NotificationTypeEnum.USER_SPAM_BANNED.instance.emit(
+                user=self,
+                event_context={
+                    'user_fullname': self.fullname,
+                    'osf_support_email': website_settings.OSF_SUPPORT_EMAIL,
+                }
+            )
+
         if skip_resources_spam:
             return
 
-        # Don't train on resources merely associated with spam user
+        # Don't train on resources merely associated with spam user, and don't
+        # send a separate per-item notification for content caught up in the
+        # account-level ban cascade -- the single account-ban email covers it.
         for node in self.nodes.filter(is_public=True, is_deleted=False):
-            node.confirm_spam(domains=domains, train_spam_services=train_spam_services)
+            node.confirm_spam(domains=domains, train_spam_services=train_spam_services, notify=False)
         for preprint in self.preprints.filter(is_public=True, deleted__isnull=True):
-            preprint.confirm_spam(domains=domains, train_spam_services=train_spam_services)
+            preprint.confirm_spam(domains=domains, train_spam_services=train_spam_services, notify=False)
 
     def confirm_ham(self, save=False, train_spam_services=False):
         self.reactivate_account()

osf_tests/test_notable_domains.py

@@ -11,7 +11,8 @@
 from osf.models import (
     NotableDomain,
     DomainReference,
-    SpamStatus
+    SpamStatus,
+    NotificationTypeEnum,
 )
 from osf.utils.workflows import DefaultStates
 from osf_tests.factories import (
@@ -21,6 +22,7 @@
     RegistrationFactory,
     UserFactory
 )
+from tests.utils import capture_notifications
 
 
 class TestDomainExtraction:
@@ -249,6 +251,23 @@ def test_check_resource_for_duplicate_spam_domains(self, factory, spam_domain, m
             domain__domain=spam_domain.netloc
         ).count() == 1
 
+    def test_check_resource_for_spam_postcommit_bans_user_flagged_via_domain(self, spam_domain, marked_as_spam_domain):
+        user = UserFactory()
+        with mock.patch.object(spam_tasks.requests, 'head'), capture_notifications() as notifications:
+            spam_tasks.check_resource_for_spam_postcommit(
+                guid=user._id,
+                content=spam_domain.geturl(),
+                author=user.fullname,
+                author_email=user.username,
+                request_headers={},
+            )
+
+        user.reload()
+        assert user.is_disabled
+        assert user.spam_status == SpamStatus.SPAM
+        assert len(notifications['emits']) == 1
+        assert notifications['emits'][0]['type'] == NotificationTypeEnum.USER_SPAM_BANNED
+
     @pytest.mark.enable_enqueue_task
     def test_extract_domains_from_wiki__public_project_extracts_domains_on_wiki_save(self, request_context):
         assert DomainReference.objects.count() == 0

tests/test_spam_mixin.py

@@ -10,7 +10,7 @@
 
 from tests.base import DbTestCase
 from osf_tests.factories import UserFactory, CommentFactory, ProjectFactory, PreprintFactory, RegistrationFactory, AuthUserFactory
-from osf.models import NotableDomain, SpamStatus, NotificationTypeEnum
+from osf.models import NotableDomain, SpamStatus, NotificationTypeEnum, OSFUser, AbstractNode, Preprint
 from tests.utils import capture_notifications
 from website import settings
 
@@ -28,12 +28,36 @@ def test_throttled_autoban():
             projects.append(proj)
     assert len(notifications['emits']) == 1
     assert notifications['emits'][0]['type'] == NotificationTypeEnum.USER_SPAM_BANNED
+    assert not any(e['type'] == NotificationTypeEnum.NODE_CONFIRMED_SPAM for e in notifications['emits'])
     user.reload()
     assert user.is_disabled
     for project in projects:
         assert not project.is_public
 
 
+@pytest.mark.django_db
+def test_user_confirm_spam_cascade_suppresses_content_notifications():
+    user = AuthUserFactory()
+    project = ProjectFactory(creator=user, is_public=True)
+    preprint = PreprintFactory(creator=user, is_public=True)
+
+    with capture_notifications() as notifications:
+        user.confirm_spam(save=True)
+
+    assert len(notifications['emits']) == 1
+    assert notifications['emits'][0]['type'] == NotificationTypeEnum.USER_SPAM_BANNED
+    assert not any(
+        e['type'] in (NotificationTypeEnum.NODE_CONFIRMED_SPAM, NotificationTypeEnum.PREPRINT_CONFIRMED_SPAM)
+        for e in notifications['emits']
+    )
+    user.reload()
+    assert user.is_disabled
+    project.reload()
+    assert not project.is_public
+    preprint.reload()
+    assert not preprint.is_public
+
+
 @pytest.mark.enable_implicit_clean
 class TestReportAbuse(DbTestCase):
 
@@ -173,7 +197,35 @@ def test_confirm_ham(self, spammable_thing):
         assert spammable_thing.is_ham
 
     def test_confirm_spam(self, spammable_thing):
-        spammable_thing.confirm_spam(save=True)
+        if isinstance(spammable_thing, OSFUser):
+            expected_type = NotificationTypeEnum.USER_SPAM_BANNED
+        elif isinstance(spammable_thing, Preprint):
+            expected_type = NotificationTypeEnum.PREPRINT_CONFIRMED_SPAM
+        elif isinstance(spammable_thing, AbstractNode):
+            expected_type = NotificationTypeEnum.NODE_CONFIRMED_SPAM
+        else:
+            expected_type = None
+
+        with capture_notifications(allow_none=expected_type is None) as notifications:
+            spammable_thing.confirm_spam(save=True)
+
+        assert spammable_thing.is_spam
+        if expected_type is None:
+            assert notifications['emits'] == []
+        else:
+            assert len(notifications['emits']) == 1
+            assert notifications['emits'][0]['type'] == expected_type
+
+    def test_confirm_spam_is_idempotent_for_notifications(self, spammable_thing):
+        with capture_notifications(allow_none=True) as notifications:
+            spammable_thing.confirm_spam(save=True)
+            spammable_thing.confirm_spam(save=True)
+
+        assert len(notifications['emits']) <= 1
+
+    def test_confirm_spam_notify_false_suppresses_emit(self, spammable_thing):
+        with capture_notifications(expect_none=True):
+            spammable_thing.confirm_spam(save=True, notify=False)
         assert spammable_thing.is_spam
 
     @pytest.mark.parametrize('assume_ham', (True, False))

website/templates/spam_resource_confirmed.html.mako

@@ -0,0 +1,16 @@
+<%inherit file="notify_base.mako" />
+
+<%def name="content()">
+<tr>
+  <td style="border-collapse: collapse;">
+    Dear ${user_fullname}, <br>
+    <br>
+    Your ${resource_type_label} "${resource_title}" on the Open Science Framework has been flagged as spam and made private: ${resource_absolute_url}<br>
+    If this is in error, please email ${osf_support_email} for assistance.<br>
+    <br>
+    Regards,<br>
+    <br>
+    The OSF Team<br>
+
+</tr>
+</%def>