[ENG-6292] Fixed stale registration moderation state race condition

osf/external/spam/tasks.py

@@ -207,7 +207,7 @@ def set_collected_info(resource):
 @run_postcommit(once_per_request=False, celery=True)
 @celery_app.task(ignore_results=False, max_retries=5, default_retry_delay=60)
 def check_resource_for_spam_postcommit(guid, content, author, author_email, request_headers):
-    from osf.models import Guid, OSFUser
+    from osf.models import Guid, OSFUser, Registration
 
     resource, _ = Guid.load_referent(guid)
     if not resource:
@@ -238,7 +238,12 @@ def check_resource_for_spam_postcommit(guid, content, author, author_email, requ
             request_kwargs,
         )
 
-    resource.save()
+    if isinstance(resource, Registration):
+        updated_fields = resource.get_dirty_fields().keys()
+        updated_fields_without_moderation_state = [field for field in updated_fields if field != 'moderation_state']
+        resource.save(update_fields=updated_fields_without_moderation_state)
+    else:
+        resource.save()
 
     user = OSFUser.objects.get(username=author_email)
     if hasattr(resource, 'check_spam_user') and not user.is_hammy:

osf_tests/test_node.py

@@ -22,6 +22,7 @@
 from website.views import find_bookmark_collection
 
 from osf.utils.permissions import READ, WRITE, ADMIN, DEFAULT_CONTRIBUTOR_PERMISSIONS
+from osf.utils.workflows import RegistrationModerationStates
 
 from osf.models import (
     AbstractNode,
@@ -2452,7 +2453,7 @@ def run_akismet_and_oops_tests(
             oops_spam_data - spam data returned by oopsystem
             objects_to_be_spammed - objects to be spammed instead of flagged. Example:
                 spam_object = Node
-                spam objects is flagged, its creator is flagged and the others user's public nodes/preprints must be spammed
+                spam object is flagged, its creator is flagged and the others user's public nodes/preprints must be spammed
         """
         project.set_privacy('public')
         project2.set_privacy('public')
@@ -2554,6 +2555,38 @@ def test_check_resource_for_spam_postcommit_with_spammy_domains(self, mock_check
             assert project.spam_data['domains'] == ['spam.com']
             mock_check_services.assert_not_called()
 
+    @mock.patch.object(settings, 'SPAM_SERVICES_ENABLED', False)
+    @mock.patch('osf.external.spam.tasks._check_resource_for_domains')
+    def test_check_resource_for_spam_postcommit_does_not_reset_approved_registration_moderation_state(self, mock_check_domains, user):
+        registration = RegistrationFactory(creator=user, is_public=True)
+        registration.require_approval(user)
+
+        stale_registration = Registration.objects.get(id=registration.id)
+        stale_registration.moderation_state = RegistrationModerationStates.INITIAL.db_name
+        stale_registration.description = 'stale description'
+
+        registration.registration_approval.accept()
+
+        registration.refresh_from_db()
+        assert registration.moderation_state == RegistrationModerationStates.ACCEPTED.db_name
+
+        mock_check_domains.return_value = []
+        # make load_referent return the stale state of an accepted registration to simulate a race condition
+        with mock.patch('osf.models.Guid.load_referent', return_value=(stale_registration, None)):
+            with mock.patch.object(Registration, 'check_spam_user') as mock_check_user:
+                spam_tasks.check_resource_for_spam_postcommit(
+                    guid=registration._id,
+                    content='Check me for spam',
+                    author=user.fullname,
+                    author_email=user.username,
+                    request_headers={}
+                )
+
+        registration.refresh_from_db()
+        # spam_postcommit does not override moderation state with a stale value
+        assert registration.moderation_state == RegistrationModerationStates.ACCEPTED.db_name
+        mock_check_user.assert_called_once()
+
     @mock.patch.object(settings, 'SPAM_SERVICES_ENABLED', True)
     @mock.patch('osf.external.spam.tasks._check_resource_for_domains')
     def test_check_resource_for_spam_postcommit_no_spammy_domains_checks_services(self, mock_check_domains, project, user, request_headers):