From 5313a3870bc7d40df1529d2f5000d485b6c82921 Mon Sep 17 00:00:00 2001 From: mkovalua Date: Tue, 25 Feb 2025 01:04:33 +0200 Subject: [PATCH] [ENG-4255] update to public true on ham just the objects that were public before were spammed (#10970) ## Purpose When a user is is designated as Spam, and then confirmed as Ham, sometimes their preprints remain set to private. It is needed to update to public true on ham just the objects that were public before were spammed. ## Changes It is needed to was_public parameter from the latest spammed log that was after public/private changings ## Ticket https://openscience.atlassian.net/browse/ENG-4255 --- admin/nodes/views.py | 4 +- admin_tests/nodes/test_views.py | 28 +++- admin_tests/preprints/test_views.py | 20 ++- admin_tests/registrations/__init__.py | 0 .../registrations/test_registrations.py | 153 ++++++++++++++++++ admin_tests/utilities.py | 7 + osf/models/mixins.py | 8 + 7 files changed, 217 insertions(+), 3 deletions(-) create mode 100644 admin_tests/registrations/__init__.py create mode 100644 admin_tests/registrations/test_registrations.py diff --git a/admin/nodes/views.py b/admin/nodes/views.py index 53a5fc54e02..fdb3152cc2d 100644 --- a/admin/nodes/views.py +++ b/admin/nodes/views.py @@ -25,6 +25,7 @@ from api.share.utils import update_share from api.caching.tasks import update_storage_usage_cache +from framework.auth import Auth from osf.exceptions import NodeStateError from osf.models import ( @@ -673,7 +674,8 @@ class NodeMakePublic(NodeMixin, View): def post(self, request, *args, **kwargs): node = self.get_object() try: - node.set_privacy('public') + auth = Auth(request.user._wrapped, private_key=None) + node.set_privacy('public', auth=auth) except NodeStateError as e: messages.error(request, str(e)) return redirect(self.get_success_url()) diff --git a/admin_tests/nodes/test_views.py b/admin_tests/nodes/test_views.py index f696586e43e..c80eeb27e47 100644 --- a/admin_tests/nodes/test_views.py +++ b/admin_tests/nodes/test_views.py @@ -6,6 +6,7 @@ from osf.models import AdminLogEntry, NodeLog, AbstractNode, RegistrationApproval from admin.nodes.views import ( + NodeConfirmSpamView, NodeDeleteView, NodeRemoveContributorView, NodeView, @@ -21,7 +22,7 @@ ApprovalBacklogListView, ConfirmApproveBacklogView ) -from admin_tests.utilities import setup_log_view, setup_view +from admin_tests.utilities import setup_log_view, setup_view, handle_post_view_request from api_tests.share._utils import mock_update_share from website import settings from django.utils import timezone @@ -113,6 +114,31 @@ def test_correct_view_permissions(self): response = NodeView.as_view()(request, guid=guid) assert response.status_code == 200 + def test_node_spam_ham_workflow_if_node_is_private(self): + superuser = AuthUserFactory() + superuser.is_superuser = True + node = ProjectFactory() + guid = node._id + request = RequestFactory().post('/fake_path') + request.user = superuser + node = handle_post_view_request(request, NodeConfirmSpamView(), node, guid) + assert not node.is_public + node = handle_post_view_request(request, NodeConfirmHamView(), node, guid) + assert not node.is_public + + def test_node_spam_ham_workflow_if_node_is_public(self): + superuser = AuthUserFactory() + superuser.is_superuser = True + node = ProjectFactory() + node.set_privacy('public') + guid = node._id + request = RequestFactory().post('/fake_path') + request.user = superuser + node = handle_post_view_request(request, NodeConfirmSpamView(), node, guid) + assert not node.is_public + node = handle_post_view_request(request, NodeConfirmHamView(), node, guid) + assert node.is_public + class TestNodeDeleteView(AdminTestCase): def setUp(self): diff --git a/admin_tests/preprints/test_views.py b/admin_tests/preprints/test_views.py index 732a23030d9..72f399f6f72 100644 --- a/admin_tests/preprints/test_views.py +++ b/admin_tests/preprints/test_views.py @@ -22,7 +22,7 @@ from osf.models.spam import SpamStatus from osf.utils.workflows import DefaultStates, RequestTypes -from admin_tests.utilities import setup_view, setup_log_view +from admin_tests.utilities import setup_view, setup_log_view, handle_post_view_request from admin.preprints import views @@ -331,6 +331,24 @@ def test_change_preprint_provider_subjects_change_permissions(self, plain_view, assert preprint.provider == provider_one assert subject_osf in preprint.subjects.all() + def test_preprint_spam_ham_workflow_if_preprint_is_public(self, preprint, superuser): + request = RequestFactory().post('/fake_path') + request.user = superuser + preprint = handle_post_view_request(request, views.PreprintConfirmSpamView(), preprint, preprint._id) + assert not preprint.is_public + preprint = handle_post_view_request(request, views.PreprintConfirmHamView(), preprint, preprint._id) + assert preprint.is_public + + def test_preprint_spam_ham_workflow_if_preprint_is_private(self, preprint, superuser): + preprint.set_privacy('private') + preprint.refresh_from_db() + request = RequestFactory().post('/fake_path') + request.user = superuser + preprint = handle_post_view_request(request, views.PreprintConfirmSpamView(), preprint, preprint._id) + assert not preprint.is_public + preprint = handle_post_view_request(request, views.PreprintConfirmHamView(), preprint, preprint._id) + assert not preprint.is_public + @pytest.mark.urls('admin.base.urls') @pytest.mark.enable_search diff --git a/admin_tests/registrations/__init__.py b/admin_tests/registrations/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/admin_tests/registrations/test_registrations.py b/admin_tests/registrations/test_registrations.py new file mode 100644 index 00000000000..5a7431c5977 --- /dev/null +++ b/admin_tests/registrations/test_registrations.py @@ -0,0 +1,153 @@ +from scripts.approve_registrations import main as approve_registrations_runner +from datetime import timedelta +import pytest +from osf_tests.factories import AuthUserFactory, ProjectFactory, RegistrationFactory + + +@pytest.mark.django_db +class TestRegistrationSpamHam: + + @pytest.fixture() + def superuser(self): + superuser = AuthUserFactory() + superuser.is_superuser = True + superuser.save() + return superuser + + @pytest.fixture() + def public_project(self, superuser): + return ProjectFactory(title='Public Project', is_public=True, creator=superuser) + + @pytest.fixture() + def private_project(self, superuser): + return ProjectFactory(title='Private Project', is_public=False, creator=superuser) + + @pytest.fixture() + def private_project_from_public(self, public_project): + public_project.set_privacy('private', save=True) + return public_project + + @pytest.fixture() + def public_project_from_private(self, private_project): + private_project.set_privacy('public', save=True) + return private_project + + @pytest.fixture() + def public_registration_from_public_project(self, superuser, public_project): + return RegistrationFactory(project=public_project, creator=superuser, is_public=True) + + @pytest.fixture() + def public_registration_from_private_project(self, superuser, private_project): + return RegistrationFactory(project=private_project, creator=superuser, is_public=True) + + @pytest.fixture() + def private_registration_from_public_project(self, superuser, public_project): + return RegistrationFactory(project=public_project, creator=superuser, is_public=False) + + @pytest.fixture() + def public_registration_from_changed_to_public_project(self, superuser, public_project_from_private): + return RegistrationFactory(project=public_project_from_private, creator=superuser, is_public=True) + + @pytest.fixture() + def public_registration_from_changed_to_private_project(self, superuser, private_project_from_public): + return RegistrationFactory(project=private_project_from_public, creator=superuser, is_public=True) + + @pytest.fixture() + def embargoed_registration_from_public_project(self, superuser, public_project): + return RegistrationFactory(project=public_project, creator=superuser, is_embargoed=True) + + @pytest.fixture() + def embargoed_registration_from_private_project(self, superuser, private_project): + return RegistrationFactory(project=private_project, creator=superuser, is_embargoed=True) + + @pytest.fixture() + def embargoed_registration_from_changed_to_public_project(self, superuser, public_project_from_private): + return RegistrationFactory(project=public_project_from_private, creator=superuser, is_embargoed=True) + + @pytest.fixture() + def embargoed_registration_from_changed_to_private_project(self, superuser, private_project_from_public): + return RegistrationFactory(project=private_project_from_public, creator=superuser, is_embargoed=True) + + def test_embargoed_registration_from_public_project_spam_ham(self, embargoed_registration_from_public_project): + embargoed_registration_from_public_project.confirm_spam(save=True) + assert not embargoed_registration_from_public_project.is_public + embargoed_registration_from_public_project.confirm_ham(save=True) + assert not embargoed_registration_from_public_project.is_public + + def test_embargoed_registration_from_private_project_spam_ham(self, embargoed_registration_from_private_project): + embargoed_registration_from_private_project.confirm_spam(save=True) + assert not embargoed_registration_from_private_project.is_public + embargoed_registration_from_private_project.confirm_ham(save=True) + assert not embargoed_registration_from_private_project.is_public + + def test_embargoed_registration_from_changed_to_public_project_spam_ham(self, embargoed_registration_from_changed_to_public_project): + embargoed_registration_from_changed_to_public_project.confirm_spam(save=True) + assert not embargoed_registration_from_changed_to_public_project.is_public + embargoed_registration_from_changed_to_public_project.confirm_ham(save=True) + assert not embargoed_registration_from_changed_to_public_project.is_public + + def test_embargoed_registration_from_changed_to_private_project_spam_ham(self, embargoed_registration_from_changed_to_private_project): + embargoed_registration_from_changed_to_private_project.confirm_spam(save=True) + assert not embargoed_registration_from_changed_to_private_project.is_public + embargoed_registration_from_changed_to_private_project.confirm_ham(save=True) + assert not embargoed_registration_from_changed_to_private_project.is_public + + def test_public_registration_from_public_project_spam_ham(self, superuser, public_registration_from_public_project): + public_registration_from_public_project.confirm_spam(save=True) + assert not public_registration_from_public_project.is_public + public_registration_from_public_project.confirm_ham(save=True) + assert public_registration_from_public_project.is_public + + def test_public_registration_from_private_project_spam_ham(self, superuser, public_registration_from_private_project): + public_registration_from_private_project.confirm_spam(save=True) + assert not public_registration_from_private_project.is_public + public_registration_from_private_project.confirm_ham(save=True) + assert public_registration_from_private_project.is_public + + def test_private_registration_from_private_project_spam_ham(self, superuser, private_registration_from_public_project): + private_registration_from_public_project.confirm_spam(save=True) + assert not private_registration_from_public_project.is_public + private_registration_from_public_project.confirm_ham(save=True) + assert not private_registration_from_public_project.is_public + + def test_public_registration_from_changed_to_public_project_spam_ham(self, superuser, public_registration_from_changed_to_public_project): + public_registration_from_changed_to_public_project.confirm_spam(save=True) + assert not public_registration_from_changed_to_public_project.is_public + public_registration_from_changed_to_public_project.confirm_ham(save=True) + assert public_registration_from_changed_to_public_project.is_public + + def test_public_registration_from_changed_to_private_project_spam_ham(self, superuser, public_registration_from_changed_to_private_project): + public_registration_from_changed_to_private_project.confirm_spam(save=True) + assert not public_registration_from_changed_to_private_project.is_public + public_registration_from_changed_to_private_project.confirm_ham(save=True) + assert public_registration_from_changed_to_private_project.is_public + + def test_unapproved_registration_task(self, embargoed_registration_from_changed_to_public_project): + embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved' + embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3) + embargoed_registration_from_changed_to_public_project.registration_approval.save() + assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'unapproved' + approve_registrations_runner(dry_run=False) + embargoed_registration_from_changed_to_public_project.registration_approval.refresh_from_db() + assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'approved' + + def test_unapproved_registration_task_after_spam(self, embargoed_registration_from_changed_to_public_project): + embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved' + embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3) + embargoed_registration_from_changed_to_public_project.registration_approval.save() + embargoed_registration_from_changed_to_public_project.confirm_spam(save=True) + assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'unapproved' + approve_registrations_runner(dry_run=False) + embargoed_registration_from_changed_to_public_project.registration_approval.refresh_from_db() + assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'unapproved' + + def test_unapproved_registration_task_after_spam_ham(self, embargoed_registration_from_changed_to_public_project): + embargoed_registration_from_changed_to_public_project.registration_approval.state = 'unapproved' + embargoed_registration_from_changed_to_public_project.registration_approval.initiation_date -= timedelta(3) + embargoed_registration_from_changed_to_public_project.registration_approval.save() + embargoed_registration_from_changed_to_public_project.confirm_spam(save=True) + embargoed_registration_from_changed_to_public_project.confirm_ham(save=True) + assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'unapproved' + approve_registrations_runner(dry_run=False) + embargoed_registration_from_changed_to_public_project.registration_approval.refresh_from_db() + assert embargoed_registration_from_changed_to_public_project.registration_approval.state == 'approved' diff --git a/admin_tests/utilities.py b/admin_tests/utilities.py index 3a3e43009b5..44a3709fc44 100644 --- a/admin_tests/utilities.py +++ b/admin_tests/utilities.py @@ -46,3 +46,10 @@ def setup_log_view(view, request, *args, **kwargs): view.args = args view.kwargs = kwargs return view + + +def handle_post_view_request(request, view, obj, guid): + view = setup_view(view, request, guid=guid) + view.post(request) + obj.refresh_from_db() + return obj diff --git a/osf/models/mixins.py b/osf/models/mixins.py index f47f064f67a..227706e5d48 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -2094,6 +2094,14 @@ def confirm_spam(self, domains=None, save=True, train_spam_services=True): super().confirm_spam(save=save, domains=domains or [], train_spam_services=train_spam_services) self.deleted = timezone.now() was_public = self.is_public + # the approach below helps to update to public true on ham just the objects that were public before were spammed + is_public_changings = self.logs.filter(action__in=['made_public', 'made_private']) + if is_public_changings: + latest_privacy_edit_time = is_public_changings.latest('created').created + last_spam_log = self.logs.filter(action__in=['confirm_spam', 'flag_spam'], + created__gt=latest_privacy_edit_time) + if last_spam_log: + was_public = last_spam_log.latest().params.get('was_public', was_public) self.set_privacy('private', auth=None, log=False, save=False, force=True, should_hide=True) log = self.add_log(