Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENG-5453] don't update last updated on project's page when log.should_hide=True #10943

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions admin_tests/preprints/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def test_get_known_ham(self, superuser, preprint, ham_preprint, spam_preprint, f
assert preprint._id not in response_ids

def test_confirm_spam(self, flagged_preprint, superuser, mock_akismet):
last_logged_before_method_call = flagged_preprint.last_logged
request = RequestFactory().post('/fake_path')
request.user = superuser

Expand All @@ -160,8 +161,11 @@ def test_confirm_spam(self, flagged_preprint, superuser, mock_akismet):
assert flagged_preprint.is_spam
assert flagged_preprint.is_spam
assert not flagged_preprint.is_public
assert flagged_preprint.logs.first().action == 'confirm_spam'
assert last_logged_before_method_call == flagged_preprint.last_logged

def test_confirm_ham(self, preprint, superuser, mock_akismet):
last_logged_before_method_call = preprint.last_logged
request = RequestFactory().post('/fake_path')
request.user = superuser

Expand All @@ -172,6 +176,8 @@ def test_confirm_ham(self, preprint, superuser, mock_akismet):

assert preprint.spam_status == SpamStatus.HAM
assert preprint.is_public
assert preprint.logs.first().action == 'confirm_ham'
assert last_logged_before_method_call == preprint.last_logged

def test_valid_but_insufficient_view_permissions(self, user, preprint, plain_view):
view_permission = Permission.objects.get(codename='view_preprint')
Expand Down
30 changes: 17 additions & 13 deletions osf/models/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class Meta:
class Loggable(models.Model):
last_logged = NonNaiveDateTimeField(db_index=True, null=True, blank=True, default=timezone.now)

def add_log(self, action, params, auth, foreign_user=None, log_date=None, save=True, request=None):
def add_log(self, action, params, auth, foreign_user=None, log_date=None, save=True, request=None, should_hide=False):
AbstractNode = apps.get_model('osf.AbstractNode')
OSFUser = apps.get_model('osf.OSFUser')
user = None
Expand All @@ -120,7 +120,8 @@ def add_log(self, action, params, auth, foreign_user=None, log_date=None, save=T

log = NodeLog(
action=action, user=user, foreign_user=foreign_user,
params=params, node=self, original_node=original_node
params=params, node=self, original_node=original_node,
should_hide=should_hide
)

if log_date:
Expand All @@ -134,11 +135,14 @@ def add_log(self, action, params, auth, foreign_user=None, log_date=None, save=T
def _complete_add_log(self, log, action, user=None, save=True):
if self.logs.count() == 1:
log_date = log.date if hasattr(log, 'date') else log.created
self.last_logged = log_date.replace(tzinfo=pytz.utc)
last_logged = log_date.replace(tzinfo=pytz.utc)
else:
recent_log = self.logs.first()
recent_log = self.logs.latest('created')
log_date = recent_log.date if hasattr(log, 'date') else recent_log.created
self.last_logged = log_date
last_logged = log_date

if not log.should_hide:
self.last_logged = last_logged

if save:
self.save()
Expand Down Expand Up @@ -2090,15 +2094,15 @@ 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
self.set_privacy('private', auth=None, log=False, save=False, force=True)
self.set_privacy('private', auth=None, log=False, save=False, force=True, should_hide=True)

log = self.add_log(
action=self.log_class.CONFIRM_SPAM,
params={**self.log_params, 'was_public': was_public},
auth=None,
save=False
save=False,
should_hide=True
)
log.should_hide = True
log.save()
if save:
self.save()
Expand All @@ -2116,9 +2120,9 @@ def confirm_ham(self, save=False, train_spam_services=True):
action=self.log_class.CONFIRM_HAM,
params=self.log_params,
auth=None,
save=False
save=False,
should_hide=True
)
log.should_hide = True
log.save()
if save:
self.save()
Expand Down Expand Up @@ -2226,14 +2230,14 @@ def flag_spam(self):
super().flag_spam()
if settings.SPAM_FLAGGED_MAKE_NODE_PRIVATE:
was_public = self.is_public
self.set_privacy('private', auth=None, log=False, save=False, check_addons=False, force=True)
self.set_privacy('private', auth=None, log=False, save=False, check_addons=False, force=True, should_hide=True)
log = self.add_log(
action=self.log_class.FLAG_SPAM,
params={**self.log_params, 'was_public': was_public},
auth=None,
save=False
save=False,
should_hide=True
)
log.should_hide = True
log.save()

if settings.SPAM_THROTTLE_AUTOBAN:
Expand Down
4 changes: 2 additions & 2 deletions osf/models/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -1229,8 +1229,7 @@ def check_privacy_change_viability(self, auth=None):
raise NodeStateError(
'This project exceeds private project storage limits and thus cannot be converted into a private project.')

def set_privacy(self, permissions, auth=None, log=True, save=True, meeting_creation=False, check_addons=True,
force=False):
def set_privacy(self, permissions, auth=None, log=True, save=True, meeting_creation=False, check_addons=True, force=False, should_hide=False):
"""Set the permissions for this node. Also, based on meeting_creation, queues
an email to user about abilities of public projects.

Expand Down Expand Up @@ -1298,6 +1297,7 @@ def set_privacy(self, permissions, auth=None, log=True, save=True, meeting_creat
},
auth=auth,
save=False,
should_hide=should_hide
)
if save:
self.save()
Expand Down
7 changes: 4 additions & 3 deletions osf/models/preprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ def api_url_for(self, view_name, _absolute=False, *args, **kwargs):
def get_absolute_url(self):
return self.absolute_api_v2_url

def add_log(self, action, params, auth, foreign_user=None, log_date=None, save=True, request=None):
def add_log(self, action, params, auth, foreign_user=None, log_date=None, save=True, request=None, should_hide=False):
user = None
if auth:
user = auth.user
Expand All @@ -718,7 +718,7 @@ def add_log(self, action, params, auth, foreign_user=None, log_date=None, save=T

log = PreprintLog(
action=action, user=user, foreign_user=foreign_user,
params=params, preprint=self
params=params, preprint=self, should_hide=should_hide
)

log.save()
Expand Down Expand Up @@ -1084,7 +1084,7 @@ def get_spam_fields(self, saved_fields=None):
return self.SPAM_CHECK_FIELDS
return self.SPAM_CHECK_FIELDS.intersection(saved_fields)

def set_privacy(self, permissions, auth=None, log=True, save=True, check_addons=False, force=False):
def set_privacy(self, permissions, auth=None, log=True, save=True, check_addons=False, force=False, should_hide=False):
"""Set the permissions for this preprint - mainly for spam purposes.

:param permissions: A string, either 'public' or 'private'
Expand Down Expand Up @@ -1115,6 +1115,7 @@ def set_privacy(self, permissions, auth=None, log=True, save=True, check_addons=
},
auth=auth,
save=False,
should_hide=should_hide
)
if save:
self.save()
Expand Down
12 changes: 10 additions & 2 deletions osf_tests/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -2248,16 +2248,19 @@ def test_set_privacy_pending_registration(self, user):
registration.set_privacy('public', Auth(project.creator))

def test_set_privacy(self, node, auth):
last_logged_before_method_call = node.last_logged
node.set_privacy('public', auth=auth)
assert node.logs.first().action == NodeLog.MADE_PUBLIC
assert last_logged_before_method_call != node.last_logged
node.save()
assert bool(node.is_public) is True
assert node.logs.first().action == NodeLog.MADE_PUBLIC
assert node.keenio_read_key != ''
node.set_privacy('private', auth=auth)
node.save()
assert bool(node.is_public) is False
assert node.logs.first().action == NodeLog.MADE_PRIVATE
assert node.keenio_read_key == ''
assert node.logs.first().action == NodeLog.MADE_PRIVATE
assert last_logged_before_method_call != node.last_logged

@mock.patch('osf.models.queued_mail.queue_mail')
def test_set_privacy_sends_mail_default(self, mock_queue, node, auth):
Expand Down Expand Up @@ -2400,12 +2403,17 @@ def test_check_spam_on_private_node_does_not_ban_existing_user(self, mock_send_m
assert project.is_public is True

def test_flag_spam_make_node_private(self, project):
last_logged_before_method_call = project.last_logged
project.set_privacy('public')
last_logged_after_set_privacy = project.last_logged
assert last_logged_before_method_call != last_logged_after_set_privacy
assert project.is_public
with mock.patch.object(settings, 'SPAM_FLAGGED_MAKE_NODE_PRIVATE', True):
project.flag_spam()
assert project.is_spammy
assert project.is_public is False
assert project.logs.first().action == 'flag_spam'
assert last_logged_after_set_privacy == project.last_logged

def test_flag_spam_do_not_make_node_private(self, project):
project.set_privacy('public')
Expand Down
Loading