From 1656bffc6fd90af6ae704cbb06ed21708a9d93da Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Tue, 7 Jan 2025 13:28:12 +0000 Subject: [PATCH] don't set forwarded to legal job as resolvable_in_reviewer_tools (#22973) --- src/olympia/abuse/actions.py | 4 +- src/olympia/abuse/models.py | 4 +- src/olympia/abuse/tasks.py | 2 +- src/olympia/abuse/tests/test_actions.py | 5 +- src/olympia/abuse/tests/test_models.py | 84 ++++++++++++++++++++++--- 5 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/olympia/abuse/actions.py b/src/olympia/abuse/actions.py index 6f30987b4f16..1151590cecb9 100644 --- a/src/olympia/abuse/actions.py +++ b/src/olympia/abuse/actions.py @@ -344,7 +344,9 @@ def process_action(self): job_id = entity_helper.workflow_recreate(notes=self.decision.notes, job=old_job) if old_job: - old_job.handle_job_recreated(new_job_id=job_id) + old_job.handle_job_recreated( + new_job_id=job_id, resolvable_in_reviewer_tools=False + ) else: CinderJob.objects.update_or_create( job_id=job_id, diff --git a/src/olympia/abuse/models.py b/src/olympia/abuse/models.py index f1c797724f06..427041f602f2 100644 --- a/src/olympia/abuse/models.py +++ b/src/olympia/abuse/models.py @@ -255,13 +255,13 @@ def notify_reporters(self, action_helper): reporter_abuse_reports=appellants, is_appeal=True ) - def handle_job_recreated(self, new_job_id): + def handle_job_recreated(self, *, new_job_id, resolvable_in_reviewer_tools): from olympia.reviewers.models import NeedsHumanReview new_job, created = CinderJob.objects.update_or_create( job_id=new_job_id, defaults={ - 'resolvable_in_reviewer_tools': True, + 'resolvable_in_reviewer_tools': resolvable_in_reviewer_tools, 'target_addon': self.target_addon, }, ) diff --git a/src/olympia/abuse/tasks.py b/src/olympia/abuse/tasks.py index 427fd72d1d78..46200984e7cb 100644 --- a/src/olympia/abuse/tasks.py +++ b/src/olympia/abuse/tasks.py @@ -243,4 +243,4 @@ def handle_escalate_action(*, job_pk): ) job_id = entity_helper.workflow_recreate(notes=old_job.decision.notes, job=old_job) - old_job.handle_job_recreated(new_job_id=job_id) + old_job.handle_job_recreated(new_job_id=job_id, resolvable_in_reviewer_tools=True) diff --git a/src/olympia/abuse/tests/test_actions.py b/src/olympia/abuse/tests/test_actions.py index 1331167a7627..9c876d6d962f 100644 --- a/src/olympia/abuse/tests/test_actions.py +++ b/src/olympia/abuse/tests/test_actions.py @@ -838,7 +838,7 @@ def test_hold_action(self): 'cinder_action': DECISION_ACTIONS.AMO_DISABLE_ADDON, } - def test_forward_to_reviewers_no_job(self): + def test_forward_from_reviewers_no_job(self): self.decision.update(action=DECISION_ACTIONS.AMO_LEGAL_FORWARD) self.decision.cinder_job.update(decision=None) action = ContentActionForwardToLegal(self.decision) @@ -856,7 +856,7 @@ def test_forward_to_reviewers_no_job(self): assert request_body['reasoning'] == self.decision.notes assert request_body['queue_slug'] == 'legal-escalations' - def test_forward_to_reviewers_with_job(self): + def test_forward_from_reviewers_with_job(self): self.decision.update(action=DECISION_ACTIONS.AMO_LEGAL_FORWARD) action = ContentActionForwardToLegal(self.decision) responses.add_callback( @@ -884,6 +884,7 @@ def test_forward_to_reviewers_with_job(self): request_body = json.loads(responses.calls[0].request.body) assert request_body['reasoning'] == self.decision.notes assert request_body['queue_slug'] == 'legal-escalations' + assert not new_cinder_job.resolvable_in_reviewer_tools class TestContentActionCollection(BaseTestContentAction, TestCase): diff --git a/src/olympia/abuse/tests/test_models.py b/src/olympia/abuse/tests/test_models.py index 24f7e602aab2..cc50c718c24d 100644 --- a/src/olympia/abuse/tests/test_models.py +++ b/src/olympia/abuse/tests/test_models.py @@ -1006,7 +1006,7 @@ def test_report_resolvable_in_reviewer_tools(self): assert cinder_job.target_addon == abuse_report.target assert cinder_job.resolvable_in_reviewer_tools - def test_handle_job_recreated(self): + def _test_handle_job_recreated(self, *, resolvable_in_reviewer_tools): addon = addon_factory() decision = ContentDecision.objects.create( action=DECISION_ACTIONS.AMO_ESCALATE_ADDON, addon=addon, notes='blah' @@ -1017,16 +1017,24 @@ def test_handle_job_recreated(self): report = AbuseReport.objects.create(guid=addon.guid, cinder_job=job) assert not job.resolvable_in_reviewer_tools - job.handle_job_recreated(new_job_id='5678') + job.handle_job_recreated( + new_job_id='5678', resolvable_in_reviewer_tools=resolvable_in_reviewer_tools + ) job.reload() new_job = job.forwarded_to_job assert new_job.job_id == '5678' assert list(new_job.forwarded_from_jobs.all()) == [job] - assert new_job.resolvable_in_reviewer_tools + assert new_job.resolvable_in_reviewer_tools == resolvable_in_reviewer_tools assert new_job.target_addon == addon assert report.reload().cinder_job == new_job + def test_handle_job_recreated_for_reviewers(self): + self._test_handle_job_recreated(resolvable_in_reviewer_tools=True) + + def test_handle_job_recreated_for_cinder(self): + self._test_handle_job_recreated(resolvable_in_reviewer_tools=False) + def test_handle_job_recreated_existing_forwarded_job(self): addon = addon_factory() exisiting_escalation_job = CinderJob.objects.create( @@ -1052,7 +1060,9 @@ def test_handle_job_recreated_existing_forwarded_job(self): reason=NeedsHumanReview.REASONS.CINDER_ESCALATION, ) - old_job.handle_job_recreated(new_job_id='5678') + old_job.handle_job_recreated( + new_job_id='5678', resolvable_in_reviewer_tools=True + ) old_job.reload() exisiting_escalation_job.reload() @@ -1095,7 +1105,9 @@ def test_handle_job_recreated_existing_report_job(self): reason=NeedsHumanReview.REASONS.CINDER_ESCALATION, ) - old_job.handle_job_recreated(new_job_id='5678') + old_job.handle_job_recreated( + new_job_id='5678', resolvable_in_reviewer_tools=True + ) old_job.reload() exisiting_report_job.reload() @@ -1139,7 +1151,9 @@ def test_handle_job_recreated_appeal(self): ) assert not appeal_job.resolvable_in_reviewer_tools - appeal_job.handle_job_recreated(new_job_id='5678') + appeal_job.handle_job_recreated( + new_job_id='5678', resolvable_in_reviewer_tools=True + ) appeal_job.reload() new_job = appeal_job.forwarded_to_job @@ -1595,7 +1609,7 @@ def test_resolve_job_appeal_with_new_report(self): == appeal_job.decision ) - def test_resolve_job_forwarded(self): + def test_resolve_job_forwarded_from_cinder(self): addon_developer = user_factory() addon = addon_factory(users=[addon_developer]) cinder_job = CinderJob.objects.create(job_id='999') @@ -1660,6 +1674,62 @@ def test_resolve_job_forwarded(self): ).exists() assert NeedsHumanReview.objects.filter(is_active=True).count() == 2 + def test_resolve_job_forwarded_to_legal(self): + addon_developer = user_factory() + addon = addon_factory(users=[addon_developer]) + cinder_job = CinderJob.objects.create(job_id='999') + CinderJob.objects.create(forwarded_to_job=cinder_job) + NeedsHumanReview.objects.create( + version=addon.current_version, + reason=NeedsHumanReview.REASONS.CINDER_ESCALATION, + ) + abuse_report = AbuseReport.objects.create( + guid=addon.guid, + reason=AbuseReport.REASONS.POLICY_VIOLATION, + location=AbuseReport.LOCATION.ADDON, + cinder_job=cinder_job, + reporter=user_factory(), + ) + responses.add( + responses.POST, + f'{settings.CINDER_SERVER_URL}jobs/{cinder_job.job_id}/decision', + json={'uuid': uuid.uuid4().hex}, + status=201, + ) + responses.add( + responses.POST, + f'{settings.CINDER_SERVER_URL}create_report', + json={'job_id': uuid.uuid4().hex}, + status=201, + ) + + log_entry = ActivityLog.objects.create( + amo.LOG.FORCE_DISABLE, + abuse_report.target, + abuse_report.target.current_version, + details={ + 'comments': 'some reasoning', + 'cinder_action': 'AMO_LEGAL_FORWARD', + }, + user=user_factory(), + ) + + cinder_job.resolve_job(log_entry=log_entry) + + request = responses.calls[0].request + request_body = json.loads(request.body) + assert request_body['reasoning'] == 'some reasoning' + cinder_job.reload() + assert cinder_job.decision.action == DECISION_ACTIONS.AMO_LEGAL_FORWARD + self.assertCloseToNow(cinder_job.decision.action_date) + assert len(mail.outbox) == 0 + assert not NeedsHumanReview.objects.filter( + is_active=True, reason=NeedsHumanReview.REASONS.CINDER_ESCALATION + ).exists() + assert cinder_job.forwarded_to_job + new_job = cinder_job.forwarded_to_job + assert not new_job.resolvable_in_reviewer_tools + def test_all_abuse_reports(self): job = CinderJob.objects.create(job_id='fake_job_id') assert list(job.all_abuse_reports) == []