Skip to content

Commit

Permalink
[ENG-4255] update to public true on ham just the objects that were pu…
Browse files Browse the repository at this point in the history
…blic 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
  • Loading branch information
mkovalua authored Feb 24, 2025
1 parent 8a9dc39 commit 5313a38
Show file tree
Hide file tree
Showing 7 changed files with 217 additions and 3 deletions.
4 changes: 3 additions & 1 deletion admin/nodes/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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())
Expand Down
28 changes: 27 additions & 1 deletion admin_tests/nodes/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from osf.models import AdminLogEntry, NodeLog, AbstractNode, RegistrationApproval
from admin.nodes.views import (
NodeConfirmSpamView,
NodeDeleteView,
NodeRemoveContributorView,
NodeView,
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down
20 changes: 19 additions & 1 deletion admin_tests/preprints/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Empty file.
153 changes: 153 additions & 0 deletions admin_tests/registrations/test_registrations.py
Original file line number Diff line number Diff line change
@@ -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'
7 changes: 7 additions & 0 deletions admin_tests/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 8 additions & 0 deletions osf/models/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 5313a38

Please sign in to comment.