Skip to content

Commit

Permalink
make sure we call log_action before calling resolve_abuse_reports (#2…
Browse files Browse the repository at this point in the history
…1869)

* make sure we call log_action before calling resolve_abuse_reports

* Apply suggestions from code review
  • Loading branch information
eviljeff committed Feb 15, 2024
1 parent 36753b3 commit 3a98da0
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
17 changes: 15 additions & 2 deletions src/olympia/reviewers/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3180,6 +3180,13 @@ def test_enable_addon_version_is_awaiting_review_fall_back_to_nominated(self):
assert len(mail.outbox) == 0

def _resolve_abuse_reports_called_everywhere_checkbox_shown(self, channel, actions):
# these two functions are to verify we call log_action before it's accessed
def log_check(decision):
assert self.helper.handler.log_entry

def log_action(*args, **kwargs):
self.helper.handler.log_entry = object()

self.grant_permission(self.user, 'Reviews:Admin')
self.grant_permission(self.user, 'Addons:Review')
self.grant_permission(self.user, 'Addons:ReviewUnlisted')
Expand All @@ -3198,8 +3205,12 @@ def _resolve_abuse_reports_called_everywhere_checkbox_shown(self, channel, actio

self.helper.handler.notify_email = lambda *arg, **kwarg: None
with (
patch.object(self.helper.handler, 'resolve_abuse_reports') as resolve_mock,
patch.object(self.helper.handler, 'log_action') as log_action_mock,
patch.object(
self.helper.handler, 'resolve_abuse_reports', wraps=log_check
) as resolve_mock,
patch.object(
self.helper.handler, 'log_action', wraps=log_action
) as log_action_mock,
):
for action_name, action in resolves_actions.items():
action['method']()
Expand All @@ -3209,7 +3220,9 @@ def _resolve_abuse_reports_called_everywhere_checkbox_shown(self, channel, actio
getattr(log_action_mock.call_args.args[0], 'hide_developer', False)
!= should_email[action_name]
)
log_action_mock.assert_called_once()
log_action_mock.reset_mock()
self.helper.handler.log_entry = None

def test_resolve_abuse_reports_called_everywhere_checkbox_shown_listed(self):
self._resolve_abuse_reports_called_everywhere_checkbox_shown(
Expand Down
5 changes: 2 additions & 3 deletions src/olympia/reviewers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1184,11 +1184,10 @@ def reject_latest_version(self):
# at.
self.clear_specific_needs_human_review_flags(self.version)
self.set_human_review_date()
self.resolve_abuse_reports(
CinderJob.DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON
)

self.log_action(amo.LOG.REJECT_VERSION)
# This call has to happen after log_action - we need self.log_entry
self.resolve_abuse_reports(CinderJob.DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON)
template = '%s_to_rejected' % self.review_type
subject = "Mozilla Add-ons: %s %s didn't pass review"
self.notify_email(template, subject)
Expand Down

0 comments on commit 3a98da0

Please sign in to comment.