diff --git a/osf/external/spam/tasks.py b/osf/external/spam/tasks.py index 56f7f16aa2d..b1b4d3b579a 100644 --- a/osf/external/spam/tasks.py +++ b/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: diff --git a/osf_tests/test_node.py b/osf_tests/test_node.py index 98447188a8b..dd1e1edd991 100644 --- a/osf_tests/test_node.py +++ b/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):