make confirm ham a celery task (#11666)

Author: antkryt

admin/users/views.py

@@ -50,7 +50,7 @@
 from admin.nodes.views import NodeAddSystemTag, NodeRemoveSystemTag
 from admin.base.views import GuidView
 from api.users.services import send_password_reset_email
-from api.users.tasks import merge_users
+from api.users.tasks import merge_users, confirm_user_ham
 from website.settings import DOMAIN, OSF_SUPPORT_EMAIL
 from django.urls import reverse_lazy
 
@@ -264,14 +264,19 @@ class UserConfirmHamView(UserMixin, View):
 
     def post(self, request, *args, **kwargs):
         user = self.get_object()
-        user.is_registered = True  # back in the day spam users were de-registered
-        user.confirm_ham(save=True)
+
+        ham_task = confirm_user_ham.delay(user._id, initiator_guid=request.user._id)
         update_admin_log(
             user_id=request.user.id,
             object_id=user._id,
             object_repr='User',
-            message=f'Confirmed Ham: {user._id} when user {user._id} marked as spam',
-            action_flag=CONFIRM_SPAM
+            message=f'Queued Confirm HAM task for user {user._id} (task id: {ham_task.id}).',
+            action_flag=CONFIRM_HAM
+        )
+
+        messages.success(
+            request,
+            f'Confirm HAM started in the background for user {user._id}. Task id: {ham_task.id}.'
         )
         return redirect(self.get_success_url())
 

admin_tests/users/test_views.py

@@ -218,18 +218,22 @@ class TestHamUserRestore(AdminTestCase):
     def setUp(self):
         self.user = UserFactory()
         self.request = RequestFactory().post('/fake_path')
+        patch_messages(self.request)
         self.view = views.UserConfirmHamView
         self.view = setup_log_view(self.view, self.request, guid=self.user._id)
 
-    def test_enable_user(self):
+    @mock.patch('api.users.tasks.confirm_user_ham.delay')
+    def test_enable_user(self, mock_confirm_user_ham_delay):
         self.user.is_disabled = True
         self.user.save()
         assert self.user.is_disabled
         self.view().post(self.request)
         self.user.reload()
 
-        assert not self.user.is_disabled
-        assert self.user.spam_status == SpamStatus.HAM
+        mock_confirm_user_ham_delay.assert_called_with(
+            self.user._id,
+            initiator_guid=mock.ANY,
+        )
 
 
 class TestDisableSpamUser(AdminTestCase):

api/users/tasks.py

@@ -3,6 +3,9 @@
 from framework import sentry
 from framework.celery_tasks import app as celery_app
 
+from osf.models import OSFUser
+from osf.models.notification_type import NotificationTypeEnum
+
 logger = logging.getLogger(__name__)
 
 
@@ -32,3 +35,37 @@ def merge_users(merger_guid: str, mergee_guid: str):
     except Exception as exc:
         logger.exception(f'Unexpected error during background user merge: merger={merger_guid}, mergee={mergee_guid}')
         sentry.log_exception(exc)
+
+
+@celery_app.task(bind=True, name='api.users.tasks.confirm_user_ham')
+def confirm_user_ham(self, user_guid: str, initiator_guid: str | None = None):
+    initiator_user = OSFUser.load(initiator_guid) if initiator_guid else None
+    failed_ham = []
+    try:
+        user = OSFUser.objects.get(guids___id=user_guid)
+    except OSFUser.DoesNotExist as exc:
+        sentry.log_exception(exc)
+        return str(exc)
+    else:
+        try:
+            user.is_registered = True  # back in the day spam users were de-registered
+            failed_ham = user.confirm_ham(save=True, train_spam_services=False)
+        except Exception as exc:
+            sentry.log_exception(exc)
+
+    try:
+        if initiator_user:
+            NotificationTypeEnum.USER_CONFIRM_HAM_REPORT.instance.emit(
+                user=initiator_user,
+                message_frequency='instantly',
+                event_context={
+                    'user_guid': user._id,
+                    'failed_ham': ', '.join(failed_ham),
+                },
+                save=False,
+            )
+    except Exception as exc:
+        logger.exception('Failed to send HAM confirmation report email')
+        sentry.log_exception(exc)
+
+    return True

notifications.yaml

@@ -318,6 +318,13 @@ notification_types:
     template: 'website/templates/tou_notif.html.mako'
     tests: []
 
+  - name: user_confirm_ham_report
+    subject: 'Confirm HAM report for {user_guid}'
+    __docs__: Summary report sent to the admin who triggered Confirm HAM.
+    object_content_type_model_name: osfuser
+    template: 'website/templates/user_confirm_ham_report.html.mako'
+    tests: []
+
   - name: desk_registration_bulk_upload_product_owner
     subject: 'Registry Could Not Bulk Upload Registrations'
     object_content_type_model_name: osfuser

osf/models/notification_type.py

@@ -78,6 +78,7 @@ class NotificationTypeEnum(str, Enum):
     USER_SPAM_FILES_DETECTED = 'user_spam_files_detected'
     USER_CROSSREF_DOI_PENDING = 'user_crossref_doi_pending'
     USER_TERMS_OF_USE_UPDATED = 'user_terms_of_use_updated'  # added as a placeholder
+    USER_CONFIRM_HAM_REPORT = 'user_confirm_ham_report'
 
     # Node notifications
     NODE_FILE_UPDATED = 'node_file_updated'

osf/models/user.py

@@ -1478,11 +1478,36 @@ def confirm_ham(self, save=False, train_spam_services=False):
         self.reactivate_account()
         super().confirm_ham(save=save, train_spam_services=train_spam_services)
 
+        failed_ham_ids = []
+
         # Don't train on resources merely associated with spam user
         for node in self.nodes.filter():
-            node.confirm_ham(save=save, train_spam_services=train_spam_services)
+            try:
+                node.confirm_ham(save=save, train_spam_services=train_spam_services)
+            except Exception as exc:
+                sentry.log_exception(exc)
+                failed_ham_ids.append(node._id)
+                continue
+
+            if not node.is_ham or getattr(node, 'is_deleted', False):
+                failed_ham_ids.append(node._id)
+
         for preprint in self.preprints.filter():
-            preprint.confirm_ham(save=save, train_spam_services=train_spam_services)
+            try:
+                preprint.confirm_ham(save=save, train_spam_services=train_spam_services)
+            except Exception as exc:
+                sentry.log_exception(exc)
+                failed_ham_ids.append(preprint._id)
+                continue
+
+            if (
+                not preprint.is_ham
+                or getattr(preprint, 'is_deleted', False)
+                or getattr(preprint, 'deleted', None) is not None
+            ):
+                failed_ham_ids.append(preprint._id)
+
+        return failed_ham_ids
 
     @property
     def is_assumed_ham(self):

website/templates/user_confirm_ham_report.html.mako

@@ -0,0 +1,18 @@
+<%inherit file="notify_base.mako" />
+
+<%def name="content()">
+<tr>
+  <td style="border-collapse: collapse;">
+    <h3 style="padding: 0; margin: 30px 0 10px 0; font-weight: 400;">
+      Confirm HAM finished for user ${user_guid}
+    </h3>
+  </td>
+</tr>
+<tr>
+  <td style="border-collapse: collapse;">
+% if failed_ham:
+    <p style="margin: 0;">Failed nodes/preprints: ${failed_ham}</p>
+% endif
+  </td>
+</tr>
+</%def>