diff --git a/src/olympia/abuse/models.py b/src/olympia/abuse/models.py index 37b3c838bc4a..a43c6afaccb5 100644 --- a/src/olympia/abuse/models.py +++ b/src/olympia/abuse/models.py @@ -105,11 +105,6 @@ class Meta: models.CheckConstraint(name='no_empty_job_id', check=~Q(job_id='')) ] - @property - def decision(self): - """This is the first decision made on the job""" - return self.decisions.first() - @property def final_decision(self): return self.decisions.last() @@ -119,9 +114,9 @@ def target(self): if self.target_addon_id: # if this was a job from an abuse report for an addon we've set target_addon return self.target_addon - elif decision := self.decision: + elif final_decision := self.final_decision: # if there is already a decision target will be set there - return decision.target + return final_decision.target elif self.is_appeal: # if this is an appeal job the decision will have the target return self.appealed_decisions.first().target @@ -393,7 +388,7 @@ def clear_needs_human_review_flags(self): # isn't resolved yet, but there is no link between NHR and jobs. # So for each possible reason, we look if there are unresolved jobs # and only clear NHR for that reason if there aren't any jobs left. - addon = self.decision.addon + addon = self.final_decision.addon base_unresolved_jobs_qs = ( self.__class__.objects.for_addon(addon) .unresolved() diff --git a/src/olympia/abuse/tests/test_actions.py b/src/olympia/abuse/tests/test_actions.py index 8ed345473d19..bc80a7bb4d87 100644 --- a/src/olympia/abuse/tests/test_actions.py +++ b/src/olympia/abuse/tests/test_actions.py @@ -494,11 +494,11 @@ def test_ban_user_after_reporter_appeal(self): user=self.user, action=DECISION_ACTIONS.AMO_APPROVE ), ) - self.cinder_job.appealed_decisions.add(original_job.decision) + self.cinder_job.appealed_decisions.add(original_job.final_decision) self.abuse_report_no_auth.update(cinder_job=original_job) self.abuse_report_auth.update(cinder_job=original_job) CinderAppeal.objects.create( - decision=original_job.decision, reporter_report=self.abuse_report_auth + decision=original_job.final_decision, reporter_report=self.abuse_report_auth ) subject = self._test_ban_user() assert len(mail.outbox) == 2 @@ -715,11 +715,11 @@ def test_execute_action_after_reporter_appeal(self): addon=self.addon, action=DECISION_ACTIONS.AMO_APPROVE ), ) - self.cinder_job.appealed_decisions.add(original_job.decision) + self.cinder_job.appealed_decisions.add(original_job.final_decision) self.abuse_report_no_auth.update(cinder_job=original_job) self.abuse_report_auth.update(cinder_job=original_job) CinderAppeal.objects.create( - decision=original_job.decision, reporter_report=self.abuse_report_auth + decision=original_job.final_decision, reporter_report=self.abuse_report_auth ) subject = self._process_action_and_notify() assert len(mail.outbox) == 2 @@ -1447,11 +1447,11 @@ def test_execute_action_after_reporter_appeal(self): addon=self.addon, action=DECISION_ACTIONS.AMO_APPROVE ), ) - self.cinder_job.appealed_decisions.add(original_job.decision) + self.cinder_job.appealed_decisions.add(original_job.final_decision) self.abuse_report_no_auth.update(cinder_job=original_job) self.abuse_report_auth.update(cinder_job=original_job) CinderAppeal.objects.create( - decision=original_job.decision, reporter_report=self.abuse_report_auth + decision=original_job.final_decision, reporter_report=self.abuse_report_auth ) subject = self._test_reject_version(content_review=False) assert len(mail.outbox) == 2 @@ -1605,11 +1605,11 @@ def test_execute_action_delayed_after_reporter_appeal(self): action=DECISION_ACTIONS.AMO_APPROVE, addon=self.addon ), ) - self.cinder_job.appealed_decisions.add(original_job.decision) + self.cinder_job.appealed_decisions.add(original_job.final_decision) self.abuse_report_no_auth.update(cinder_job=original_job) self.abuse_report_auth.update(cinder_job=original_job) CinderAppeal.objects.create( - decision=original_job.decision, reporter_report=self.abuse_report_auth + decision=original_job.final_decision, reporter_report=self.abuse_report_auth ) subject = self._test_reject_version_delayed(content_review=False) assert len(mail.outbox) == 2 @@ -2404,11 +2404,11 @@ def test_reporter_appeal_approve(self): action=DECISION_ACTIONS.AMO_APPROVE, ), ) - self.cinder_job.appealed_decisions.add(original_job.decision) + self.cinder_job.appealed_decisions.add(original_job.final_decision) self.abuse_report_no_auth.update(cinder_job=original_job) self.abuse_report_auth.update(cinder_job=original_job) CinderAppeal.objects.create( - decision=original_job.decision, reporter_report=self.abuse_report_auth + decision=original_job.final_decision, reporter_report=self.abuse_report_auth ) self.cinder_job.reload() subject = self._test_reporter_content_approved_action_taken() @@ -2813,11 +2813,11 @@ def test_delete_collection_after_reporter_appeal(self): collection=self.collection, action=DECISION_ACTIONS.AMO_APPROVE ), ) - self.cinder_job.appealed_decisions.add(original_job.decision) + self.cinder_job.appealed_decisions.add(original_job.final_decision) self.abuse_report_no_auth.update(cinder_job=original_job) self.abuse_report_auth.update(cinder_job=original_job) CinderAppeal.objects.create( - decision=original_job.decision, reporter_report=self.abuse_report_auth + decision=original_job.final_decision, reporter_report=self.abuse_report_auth ) subject = self._test_delete_collection() assert len(mail.outbox) == 2 @@ -2989,11 +2989,11 @@ def test_delete_rating_after_reporter_appeal(self): rating=self.rating, action=DECISION_ACTIONS.AMO_APPROVE ), ) - self.cinder_job.appealed_decisions.add(original_job.decision) + self.cinder_job.appealed_decisions.add(original_job.final_decision) self.abuse_report_no_auth.update(cinder_job=original_job) self.abuse_report_auth.update(cinder_job=original_job) CinderAppeal.objects.create( - decision=original_job.decision, reporter_report=self.abuse_report_auth + decision=original_job.final_decision, reporter_report=self.abuse_report_auth ) subject = self._test_delete_rating() assert len(mail.outbox) == 2 diff --git a/src/olympia/abuse/tests/test_commands.py b/src/olympia/abuse/tests/test_commands.py index fda22d06f961..87ab2f9b4131 100644 --- a/src/olympia/abuse/tests/test_commands.py +++ b/src/olympia/abuse/tests/test_commands.py @@ -104,10 +104,10 @@ def test_auto_resolve_reviewed_handled(self): assert CinderJob.objects.unresolved().count() == 1 assert CinderJob.objects.unresolved().get() == job_not_reviewed - assert job1.reload().decision - assert job1.decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION - assert job2.reload().decision - assert job2.decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION + assert job1.reload().final_decision + assert job1.final_decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION + assert job2.reload().final_decision + assert job2.final_decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION # NHRs should be cleared. assert not NeedsHumanReview.objects.filter( version__addon=addon1, is_active=True @@ -196,8 +196,10 @@ def test_auto_resolve_forwarded_job(self): job_appeal, job_second_level, ] - assert job_forwarded.reload().decision - assert job_forwarded.decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION + assert job_forwarded.reload().final_decision + assert ( + job_forwarded.final_decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION + ) # NHR should be cleared. assert not NeedsHumanReview.objects.filter( version__addon=addon1, is_active=True diff --git a/src/olympia/abuse/tests/test_models.py b/src/olympia/abuse/tests/test_models.py index 63c339de0fa0..869c3671864a 100644 --- a/src/olympia/abuse/tests/test_models.py +++ b/src/olympia/abuse/tests/test_models.py @@ -757,7 +757,7 @@ def test_reviewer_handled(self): appeal_job = CinderJob.objects.create( job_id=4, resolvable_in_reviewer_tools=True ) - job.decision.update(appeal_job=appeal_job) + job.final_decision.update(appeal_job=appeal_job) qs = CinderJob.objects.resolvable_in_reviewer_tools() assert list(qs) == [job, appeal_job] @@ -773,7 +773,7 @@ class TestCinderJob(TestCase): def setUp(self): user_factory(id=settings.TASK_USER_ID) - def test_decision_and_final_decision(self): + def final_decision(self): cinder_job = CinderJob.objects.create(job_id='1234') addon = addon_factory() first = ContentDecision.objects.create( @@ -791,7 +791,7 @@ def test_decision_and_final_decision(self): cinder_job=cinder_job, override_of=second, ) - assert cinder_job.decision == first + assert cinder_job.decisions.first() == first assert cinder_job.final_decision == third def test_target(self): @@ -809,12 +809,12 @@ def test_target(self): ContentDecision.objects.create( action=DECISION_ACTIONS.AMO_APPROVE, addon=addon, cinder_job=cinder_job ) - assert cinder_job.decision.target == cinder_job.target == addon + assert cinder_job.final_decision.target == cinder_job.target == addon # case when this is an appeal job (no decision), but the appeal had a decision appeal_job = CinderJob.objects.create(job_id='fake_appeal_job_id') - cinder_job.decision.update(appeal_job=appeal_job) - assert cinder_job.decision.target == appeal_job.target == addon + cinder_job.final_decision.update(appeal_job=appeal_job) + assert cinder_job.final_decision.target == appeal_job.target == addon # case when there is no appeal, no decision yet, no target_addon, # but an initial abuse report @@ -1182,15 +1182,15 @@ def test_create_and_execute_decision(self): policy_ids=['123-45', '678-90'], job_queue='some-cinder-queue', ) - assert cinder_job.decision.cinder_id == '12345' - assert cinder_job.decision.action == DECISION_ACTIONS.AMO_BAN_USER - assert cinder_job.decision.private_notes == 'teh notes' - assert cinder_job.decision.reasoning == '' - assert cinder_job.decision.from_job_queue == 'some-cinder-queue' - assert cinder_job.decision.user == target + assert cinder_job.final_decision.cinder_id == '12345' + assert cinder_job.final_decision.action == DECISION_ACTIONS.AMO_BAN_USER + assert cinder_job.final_decision.private_notes == 'teh notes' + assert cinder_job.final_decision.reasoning == '' + assert cinder_job.final_decision.from_job_queue == 'some-cinder-queue' + assert cinder_job.final_decision.user == target assert action_mock.call_count == 1 assert notify_mock.call_count == 1 - assert list(cinder_job.decision.policies.all()) == [policy_a, policy_b] + assert list(cinder_job.final_decision.policies.all()) == [policy_a, policy_b] def test_create_and_execute_decision_with_duplicate_parent(self): cinder_job = CinderJob.objects.create(job_id='1234') @@ -1217,14 +1217,14 @@ def test_create_and_execute_decision_with_duplicate_parent(self): policy_ids=['123-45', '678-90'], job_queue='some-cinder-queue', ) - assert cinder_job.decision.cinder_id == '12345' - assert cinder_job.decision.action == DECISION_ACTIONS.AMO_BAN_USER - assert cinder_job.decision.private_notes == 'teh notes' - assert cinder_job.decision.reasoning == '' - assert cinder_job.decision.user == target + assert cinder_job.final_decision.cinder_id == '12345' + assert cinder_job.final_decision.action == DECISION_ACTIONS.AMO_BAN_USER + assert cinder_job.final_decision.private_notes == 'teh notes' + assert cinder_job.final_decision.reasoning == '' + assert cinder_job.final_decision.user == target assert action_mock.call_count == 1 assert notify_mock.call_count == 1 - assert list(cinder_job.decision.policies.all()) == [policy] + assert list(cinder_job.final_decision.policies.all()) == [policy] def test_create_and_execute_decision_decision_already_exists(self): cinder_job = CinderJob.objects.create(job_id='1234') @@ -1292,15 +1292,15 @@ def test_create_and_execute_decision_for_legal_reviewed_job(self): policy_ids=['123-45', '678-90'], job_queue='some-cinder-queue', ) - assert cinder_job.decision.cinder_id == '12345' - assert cinder_job.decision.action == DECISION_ACTIONS.AMO_DISABLE_ADDON - assert cinder_job.decision.private_notes == 'teh notes' - assert cinder_job.decision.reasoning == '' - assert cinder_job.decision.addon == target - assert cinder_job.decision.from_job_queue == 'some-cinder-queue' + assert cinder_job.final_decision.cinder_id == '12345' + assert cinder_job.final_decision.action == DECISION_ACTIONS.AMO_DISABLE_ADDON + assert cinder_job.final_decision.private_notes == 'teh notes' + assert cinder_job.final_decision.reasoning == '' + assert cinder_job.final_decision.addon == target + assert cinder_job.final_decision.from_job_queue == 'some-cinder-queue' assert action_mock.call_count == 1 assert notify_mock.call_count == 1 - assert list(cinder_job.decision.policies.all()) == [policy_a, policy_b] + assert list(cinder_job.final_decision.policies.all()) == [policy_a, policy_b] def test_create_and_execute_decision_sets_target_versions_for_reject_version_appeal( self, @@ -1332,13 +1332,13 @@ def test_create_and_execute_decision_sets_target_versions_for_reject_version_app policy_ids=['123-45'], job_queue='some-cinder-queue', ) - assert cinder_job.decision.cinder_id == '12345' - assert cinder_job.decision.action == DECISION_ACTIONS.AMO_APPROVE - assert cinder_job.decision.addon == target - assert cinder_job.decision.target_versions.get() == target.current_version + assert cinder_job.final_decision.cinder_id == '12345' + assert cinder_job.final_decision.action == DECISION_ACTIONS.AMO_APPROVE + assert cinder_job.final_decision.addon == target + assert cinder_job.final_decision.target_versions.get() == target.current_version assert action_mock.call_count == 1 assert notify_mock.call_count == 1 - assert list(cinder_job.decision.policies.all()) == [policy_a] + assert list(cinder_job.final_decision.policies.all()) == [policy_a] def test_create_and_execute_decision_overrides_action_for_reject_version_appeals( self, @@ -1370,12 +1370,15 @@ def test_create_and_execute_decision_overrides_action_for_reject_version_appeals policy_ids=['123-45'], job_queue='some-cinder-queue', ) - assert cinder_job.decision.cinder_id == '12345' - assert cinder_job.decision.action == DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON - assert cinder_job.decision.addon == target + assert cinder_job.final_decision.cinder_id == '12345' + assert ( + cinder_job.final_decision.action + == DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON + ) + assert cinder_job.final_decision.addon == target assert action_mock.call_count == 1 assert notify_mock.call_count == 1 - assert list(cinder_job.decision.policies.all()) == [policy_a] + assert list(cinder_job.final_decision.policies.all()) == [policy_a] def test_create_and_execute_decision_no_job(self): target = user_factory() @@ -1433,7 +1436,7 @@ def test_create_and_execute_decision_override_decision(self): policy_ids=['123-45', '678-90'], job_queue='some-cinder-queue', ) - assert cinder_job.decision == earlier_decision + assert cinder_job.decisions.first() == earlier_decision assert cinder_job.final_decision.cinder_id == '12345' assert cinder_job.final_decision.action == DECISION_ACTIONS.AMO_BAN_USER assert cinder_job.final_decision.private_notes == 'teh notes' @@ -1480,22 +1483,26 @@ def test_create_and_execute_decision_with_followup_actions(self): policy_ids=['123-45', '678-90'], job_queue='some-cinder-queue', ) - assert cinder_job.decision.cinder_id == '12345' - assert cinder_job.decision.action == DECISION_ACTIONS.AMO_DISABLE_ADDON - self.assertCloseToNow(cinder_job.decision.action_date) - assert cinder_job.decision.private_notes == 'teh notes' - assert cinder_job.decision.reasoning == '' - assert cinder_job.decision.from_job_queue == 'some-cinder-queue' - assert cinder_job.decision.addon == target - assert cinder_job.decision.followup_actions.count() == 2 - self.assertCloseToNow(cinder_job.decision.followup_actions.first().action_date) - self.assertCloseToNow(cinder_job.decision.followup_actions.last().action_date) - self.assertCloseToNow(cinder_job.decision.action_date) + assert cinder_job.final_decision.cinder_id == '12345' + assert cinder_job.final_decision.action == DECISION_ACTIONS.AMO_DISABLE_ADDON + self.assertCloseToNow(cinder_job.final_decision.action_date) + assert cinder_job.final_decision.private_notes == 'teh notes' + assert cinder_job.final_decision.reasoning == '' + assert cinder_job.final_decision.from_job_queue == 'some-cinder-queue' + assert cinder_job.final_decision.addon == target + assert cinder_job.final_decision.followup_actions.count() == 2 + self.assertCloseToNow( + cinder_job.final_decision.followup_actions.first().action_date + ) + self.assertCloseToNow( + cinder_job.final_decision.followup_actions.last().action_date + ) + self.assertCloseToNow(cinder_job.final_decision.action_date) assert action_mock.call_count == 1 assert short_action_mock.call_count == 1 assert mid_action_mock.call_count == 1 assert notify_mock.call_count == 1 - assert list(cinder_job.decision.policies.all()) == [policy_a, policy_b] + assert list(cinder_job.final_decision.policies.all()) == [policy_a, policy_b] @override_switch('dsa-cinder-forwarded-review', active=True) def test_process_queue_move_into_reviewer_handled(self): @@ -1650,7 +1657,7 @@ def test_all_abuse_reports(self): action=DECISION_ACTIONS.AMO_DISABLE_ADDON, addon=addon, appeal_job=appeal_job, - override_of=job.decision, + override_of=job.final_decision, cinder_job=job, ) assert list(appeal_appeal_job.all_abuse_reports) == [ @@ -1781,7 +1788,7 @@ def test_clear_needs_human_review_flags_forwarded_requeue(self): action=DECISION_ACTIONS.AMO_REQUEUE, addon=job.target_addon, cinder_job=job, - override_of=job.decision, + override_of=job.final_decision, ) ContentDecision.objects.create( action=DECISION_ACTIONS.AMO_APPROVE, @@ -1811,7 +1818,7 @@ def test_clear_needs_human_review_flags_forwarded_requeue(self): action=DECISION_ACTIONS.AMO_APPROVE, addon=job.target_addon, cinder_job=other_forward, - override_of=other_forward.decision, + override_of=other_forward.final_decision, ) job.clear_needs_human_review_flags() assert self._nhr_exists(NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION) @@ -2059,7 +2066,7 @@ def test_reporter_can_appeal_appealed_decision(self): reporter=user_factory(), reason=AbuseReport.REASONS.ILLEGAL, ) - assert appeal_job.decision.can_be_appealed( + assert appeal_job.final_decision.can_be_appealed( is_reporter=True, abuse_report=new_report ) @@ -2156,7 +2163,7 @@ def test_author_can_appeal_appealed_decision(self): ), ) self.decision.update(appeal_job=appeal_job) - assert appeal_job.decision.can_be_appealed(is_reporter=False) + assert appeal_job.final_decision.can_be_appealed(is_reporter=False) def test_author_cant_appeal_own_appeal(self): appeal_job = CinderJob.objects.create( @@ -2171,7 +2178,7 @@ def test_author_cant_appeal_own_appeal(self): self.decision.update( action=DECISION_ACTIONS.AMO_DISABLE_ADDON, appeal_job=appeal_job ) - assert not appeal_job.decision.can_be_appealed(is_reporter=False) + assert not appeal_job.final_decision.can_be_appealed(is_reporter=False) class TestCinderPolicy(TestCase): @@ -2689,7 +2696,7 @@ def _test_appeal_as_target(self, *, resolvable_in_reviewer_tools, expected_queue status=201, ) - abuse_report.cinder_job.decision.appeal( + abuse_report.cinder_job.final_decision.appeal( abuse_report=abuse_report, appeal_text='appeal text', user=user_factory(), @@ -2697,15 +2704,18 @@ def _test_appeal_as_target(self, *, resolvable_in_reviewer_tools, expected_queue ) abuse_report.cinder_job.reload() - assert abuse_report.cinder_job.decision.appeal_job_id - assert abuse_report.cinder_job.decision.appeal_job.job_id == '2432615184-tsol' - assert abuse_report.cinder_job.decision.appeal_job.target_addon == addon + assert abuse_report.cinder_job.final_decision.appeal_job_id + assert ( + abuse_report.cinder_job.final_decision.appeal_job.job_id + == '2432615184-tsol' + ) + assert abuse_report.cinder_job.final_decision.appeal_job.target_addon == addon abuse_report.reload() assert not hasattr(abuse_report, 'cinderappeal') assert CinderAppeal.objects.count() == 1 appeal_text_obj = CinderAppeal.objects.get() assert appeal_text_obj.text == 'appeal text' - assert appeal_text_obj.decision == abuse_report.cinder_job.decision + assert appeal_text_obj.decision == abuse_report.cinder_job.final_decision assert appeal_text_obj.reporter_report is None assert appeal_response.call_count == 1 @@ -2713,11 +2723,11 @@ def _test_appeal_as_target(self, *, resolvable_in_reviewer_tools, expected_queue request_body = json.loads(request.body) assert request_body['reasoning'] == 'appeal text' assert request_body['decision_to_appeal_id'] == str( - abuse_report.cinder_job.decision.cinder_id + abuse_report.cinder_job.final_decision.cinder_id ) assert request_body['queue_slug'] == expected_queue - return abuse_report.cinder_job.decision.appeal_job.reload() + return abuse_report.cinder_job.final_decision.appeal_job.reload() def test_appeal_as_target_from_resolved_in_cinder(self): appeal_job = self._test_appeal_as_target( @@ -2763,7 +2773,7 @@ def test_appeal_as_target_on_content_review_job(self): status=201, ) - cinder_job.decision.appeal( + cinder_job.final_decision.appeal( abuse_report=None, appeal_text='appeal text', user=user_factory(), @@ -2771,16 +2781,16 @@ def test_appeal_as_target_on_content_review_job(self): ) cinder_job.reload() - assert cinder_job.decision.appeal_job.job_id == '2432615184-tsol' - assert cinder_job.decision.appeal_job.target_addon == addon - assert cinder_job.decision.appeal_job.content_review is True + assert cinder_job.final_decision.appeal_job.job_id == '2432615184-tsol' + assert cinder_job.final_decision.appeal_job.target_addon == addon + assert cinder_job.final_decision.appeal_job.content_review is True assert appeal_response.call_count == 1 request = responses.calls[0].request request_body = json.loads(request.body) assert request_body['reasoning'] == 'appeal text' assert request_body['decision_to_appeal_id'] == str( - cinder_job.decision.cinder_id + cinder_job.final_decision.cinder_id ) assert request_body['queue_slug'] == 'amo-env-listing-content' @@ -2809,7 +2819,7 @@ def test_appeal_as_target_improperly_configured(self): ) with self.assertRaises(ImproperlyConfigured): - abuse_report.cinder_job.decision.appeal( + abuse_report.cinder_job.final_decision.appeal( abuse_report=abuse_report, appeal_text='appeal text', # Can't pass user=None for a target appeal, unless it's @@ -2819,7 +2829,7 @@ def test_appeal_as_target_improperly_configured(self): ) abuse_report.cinder_job.reload() - assert not abuse_report.cinder_job.decision.appeal_job_id + assert not abuse_report.cinder_job.final_decision.appeal_job_id abuse_report.reload() assert not hasattr(abuse_report, 'cinderappeal') @@ -2851,7 +2861,7 @@ def test_appeal_as_target_ban_improperly_configured(self): ) with self.assertRaises(ImproperlyConfigured): - abuse_report.cinder_job.decision.appeal( + abuse_report.cinder_job.final_decision.appeal( abuse_report=abuse_report, appeal_text='appeal text', # user=None is allowed here since the original decision was a @@ -2864,7 +2874,7 @@ def test_appeal_as_target_ban_improperly_configured(self): ) abuse_report.cinder_job.reload() - assert not abuse_report.cinder_job.decision.appeal_job_id + assert not abuse_report.cinder_job.final_decision.appeal_job_id abuse_report.reload() assert not hasattr(abuse_report, 'cinderappeal') @@ -2891,7 +2901,7 @@ def test_appeal_as_target_banned(self): status=201, ) - abuse_report.cinder_job.decision.appeal( + abuse_report.cinder_job.final_decision.appeal( abuse_report=abuse_report, appeal_text='appeal text', # user=None is allowed here since the original decision was a ban, @@ -2902,8 +2912,11 @@ def test_appeal_as_target_banned(self): ) abuse_report.cinder_job.reload() - assert abuse_report.cinder_job.decision.appeal_job_id - assert abuse_report.cinder_job.decision.appeal_job.job_id == '2432615184-tsol' + assert abuse_report.cinder_job.final_decision.appeal_job_id + assert ( + abuse_report.cinder_job.final_decision.appeal_job.job_id + == '2432615184-tsol' + ) abuse_report.reload() assert not hasattr(abuse_report, 'cinderappeal') @@ -2933,7 +2946,7 @@ def test_appeal_as_reporter(self): status=201, ) - abuse_report.cinder_job.decision.appeal( + abuse_report.cinder_job.final_decision.appeal( abuse_report=abuse_report, appeal_text='appeal text', user=abuse_report.reporter, @@ -2941,15 +2954,18 @@ def test_appeal_as_reporter(self): ) abuse_report.cinder_job.reload() - assert abuse_report.cinder_job.decision.appeal_job - assert abuse_report.cinder_job.decision.appeal_job.job_id == '2432615184-tsol' - assert abuse_report.cinder_job.decision.appeal_job.target_addon == addon + assert abuse_report.cinder_job.final_decision.appeal_job + assert ( + abuse_report.cinder_job.final_decision.appeal_job.job_id + == '2432615184-tsol' + ) + assert abuse_report.cinder_job.final_decision.appeal_job.target_addon == addon abuse_report.reload() assert abuse_report.cinderappeal assert CinderAppeal.objects.count() == 1 appeal_text_obj = CinderAppeal.objects.get() assert appeal_text_obj.text == 'appeal text' - assert appeal_text_obj.decision == abuse_report.cinder_job.decision + assert appeal_text_obj.decision == abuse_report.cinder_job.final_decision assert appeal_text_obj.reporter_report == abuse_report assert appeal_response.call_count == 1 @@ -2957,7 +2973,7 @@ def test_appeal_as_reporter(self): request_body = json.loads(request.body) assert request_body['reasoning'] == 'appeal text' assert request_body['decision_to_appeal_id'] == str( - abuse_report.cinder_job.decision.cinder_id + abuse_report.cinder_job.final_decision.cinder_id ) assert request_body['queue_slug'] == 'amo-env-listings' @@ -2985,7 +3001,7 @@ def test_appeal_as_reporter_already_appealed(self): # to ensure the get_or_create() call that we make can't trigger an # IntegrityError because of the additional parameters (job_id must # be the only field we use to retrieve the job). - abuse_report.cinder_job.decision.update( + abuse_report.cinder_job.final_decision.update( appeal_job=CinderJob.objects.create( job_id='2432615184-tsol', target_addon=addon, @@ -2999,7 +3015,7 @@ def test_appeal_as_reporter_already_appealed(self): status=201, ) - abuse_report.cinder_job.decision.appeal( + abuse_report.cinder_job.final_decision.appeal( abuse_report=abuse_report, appeal_text='appeal text', user=abuse_report.reporter, @@ -3007,9 +3023,12 @@ def test_appeal_as_reporter_already_appealed(self): ) abuse_report.cinder_job.reload() - assert abuse_report.cinder_job.decision.appeal_job - assert abuse_report.cinder_job.decision.appeal_job.job_id == '2432615184-tsol' - assert abuse_report.cinder_job.decision.appeal_job.target_addon == addon + assert abuse_report.cinder_job.final_decision.appeal_job + assert ( + abuse_report.cinder_job.final_decision.appeal_job.job_id + == '2432615184-tsol' + ) + assert abuse_report.cinder_job.final_decision.appeal_job.target_addon == addon abuse_report.reload() assert abuse_report.cinderappeal @@ -3044,7 +3063,7 @@ def test_appeal_as_reporter_specific_version(self): ) assert not original_version.due_date - abuse_report.cinder_job.decision.appeal( + abuse_report.cinder_job.final_decision.appeal( abuse_report=abuse_report, appeal_text='appeal text', user=abuse_report.reporter, @@ -3052,15 +3071,18 @@ def test_appeal_as_reporter_specific_version(self): ) abuse_report.cinder_job.reload() - assert abuse_report.cinder_job.decision.appeal_job - assert abuse_report.cinder_job.decision.appeal_job.job_id == '2432615184-tsol' - assert abuse_report.cinder_job.decision.appeal_job.target_addon == addon + assert abuse_report.cinder_job.final_decision.appeal_job + assert ( + abuse_report.cinder_job.final_decision.appeal_job.job_id + == '2432615184-tsol' + ) + assert abuse_report.cinder_job.final_decision.appeal_job.target_addon == addon abuse_report.reload() assert abuse_report.cinderappeal assert CinderAppeal.objects.count() == 1 appeal_text_obj = CinderAppeal.objects.get() assert appeal_text_obj.text == 'appeal text' - assert appeal_text_obj.decision == abuse_report.cinder_job.decision + assert appeal_text_obj.decision == abuse_report.cinder_job.final_decision assert appeal_text_obj.reporter_report == abuse_report assert original_version.reload().due_date @@ -3075,7 +3097,7 @@ def test_appeal_improperly_configured_reporter(self): ) ) with self.assertRaises(ImproperlyConfigured): - cinder_job.decision.appeal( + cinder_job.final_decision.appeal( abuse_report=None, appeal_text='No abuse_report but is_reporter is True', user=user_factory(), @@ -3099,7 +3121,7 @@ def test_appeal_improperly_configured_author(self): ) ) with self.assertRaises(ImproperlyConfigured): - cinder_job.decision.appeal( + cinder_job.final_decision.appeal( abuse_report=abuse_report, appeal_text='No user but is_reporter is False', user=None, @@ -3902,8 +3924,8 @@ def test_execute_action_forwarded_to_legal(self): decision.execute_action() cinder_job.reload() - assert cinder_job.decision.action == DECISION_ACTIONS.AMO_LEGAL_FORWARD - self.assertCloseToNow(cinder_job.decision.action_date) + assert cinder_job.final_decision.action == DECISION_ACTIONS.AMO_LEGAL_FORWARD + self.assertCloseToNow(cinder_job.final_decision.action_date) assert not NeedsHumanReview.objects.filter( is_active=True, reason=NeedsHumanReview.REASONS.CINDER_ESCALATION ).exists() @@ -4103,7 +4125,7 @@ def test_requeue_held_action_existing_job(self): decision.requeue_held_action(user=user, notes='go!') assert job.reload().resolvable_in_reviewer_tools is True - assert job.decision == decision + assert job.decisions.first() == decision self._check_requeue_decision(job.final_decision, job, decision, user) def test_requeue_held_action_existing_job_unlisted(self): @@ -4123,7 +4145,7 @@ def test_requeue_held_action_existing_job_unlisted(self): decision.requeue_held_action(user=user, notes='go!') assert job.reload().resolvable_in_reviewer_tools is True - assert job.decision == decision + assert job.decisions.first() == decision self._check_requeue_decision(job.final_decision, job, decision, user) def test_get_target_review_url(self): diff --git a/src/olympia/abuse/tests/test_tasks.py b/src/olympia/abuse/tests/test_tasks.py index 87278cec0de1..fec59c758e4a 100644 --- a/src/olympia/abuse/tests/test_tasks.py +++ b/src/olympia/abuse/tests/test_tasks.py @@ -614,7 +614,7 @@ def test_addon_appeal_to_cinder_reporter(statsd_incr_mock): statsd_incr_mock.reset_mock() appeal_to_cinder.delay( - decision_cinder_id=cinder_job.decision.cinder_id, + decision_cinder_id=cinder_job.final_decision.cinder_id, abuse_report_id=abuse_report.id, appeal_text='I appeal', user_id=None, @@ -636,11 +636,11 @@ def test_addon_appeal_to_cinder_reporter(statsd_incr_mock): } cinder_job.reload() - assert cinder_job.decision.appeal_job_id - appeal_job = cinder_job.decision.appeal_job + assert cinder_job.final_decision.appeal_job_id + appeal_job = cinder_job.final_decision.appeal_job assert appeal_job.job_id == '2432615184-xyz' abuse_report.reload() - assert abuse_report.cinderappeal.decision == cinder_job.decision + assert abuse_report.cinderappeal.decision == cinder_job.final_decision assert statsd_incr_mock.call_count == 1 assert statsd_incr_mock.call_args[0] == ('abuse.tasks.appeal_to_cinder.success',) @@ -680,7 +680,7 @@ def test_addon_appeal_to_cinder_reporter_exception( with pytest.raises(Retry) as exc_info: appeal_to_cinder.delay( - decision_cinder_id=cinder_job.decision.cinder_id, + decision_cinder_id=cinder_job.final_decision.cinder_id, abuse_report_id=abuse_report.id, appeal_text='I appeal', user_id=None, @@ -728,7 +728,7 @@ def test_addon_appeal_to_cinder_authenticated_reporter(): ) appeal_to_cinder.delay( - decision_cinder_id=cinder_job.decision.cinder_id, + decision_cinder_id=cinder_job.final_decision.cinder_id, abuse_report_id=abuse_report.pk, appeal_text='I appeal', user_id=user.pk, @@ -752,11 +752,11 @@ def test_addon_appeal_to_cinder_authenticated_reporter(): } cinder_job.reload() - assert cinder_job.decision.appeal_job_id - appeal_job = cinder_job.decision.appeal_job + assert cinder_job.final_decision.appeal_job_id + appeal_job = cinder_job.final_decision.appeal_job assert appeal_job.job_id == '2432615184-xyz' abuse_report.reload() - assert abuse_report.cinderappeal.decision == cinder_job.decision + assert abuse_report.cinderappeal.decision == cinder_job.final_decision @pytest.mark.django_db diff --git a/src/olympia/abuse/tests/test_views.py b/src/olympia/abuse/tests/test_views.py index 7b2a2a73000e..fc858276750e 100644 --- a/src/olympia/abuse/tests/test_views.py +++ b/src/olympia/abuse/tests/test_views.py @@ -1507,7 +1507,7 @@ def test_create_and_execute_decision_triggers_emails_for_reporter_appeal_disable reporter_email='reporter@email.com', cinder_job=original_cinder_job ) CinderAppeal.objects.create( - decision=original_cinder_job.decision, reporter_report=abuse_report + decision=original_cinder_job.final_decision, reporter_report=abuse_report ) req = self.get_request(data=data) with mock.patch.object( @@ -1543,7 +1543,7 @@ def test_create_and_execute_decision_triggers_no_target_email_for_reporter_appro reporter_email='reporter@email.com', cinder_job=original_cinder_job ) CinderAppeal.objects.create( - decision=original_cinder_job.decision, reporter_report=abuse_report + decision=original_cinder_job.final_decision, reporter_report=abuse_report ) req = self.get_request(data=data) with mock.patch.object( @@ -2826,13 +2826,13 @@ def setUp(self): 'abuse.appeal_reporter', kwargs={ 'abuse_report_id': self.abuse_report.id, - 'decision_cinder_id': self.cinder_job.decision.cinder_id, + 'decision_cinder_id': self.cinder_job.final_decision.cinder_id, }, ) self.author_appeal_url = reverse( 'abuse.appeal_author', kwargs={ - 'decision_cinder_id': self.cinder_job.decision.cinder_id, + 'decision_cinder_id': self.cinder_job.final_decision.cinder_id, }, ) patcher = mock.patch('olympia.abuse.views.appeal_to_cinder') @@ -2840,7 +2840,7 @@ def setUp(self): self.appeal_mock = patcher.start() def test_no_decision_yet(self): - self.cinder_job.decision.delete() + self.cinder_job.final_decision.delete() assert self.client.get(self.reporter_appeal_url).status_code == 404 assert self.client.get(self.author_appeal_url).status_code == 404 @@ -2867,7 +2867,7 @@ def test_no_such_abuse_report(self): 'abuse.appeal_reporter', kwargs={ 'abuse_report_id': self.abuse_report.id + 1, - 'decision_cinder_id': self.cinder_job.decision.cinder_id, + 'decision_cinder_id': self.cinder_job.final_decision.cinder_id, }, ) assert self.client.get(url).status_code == 404 @@ -2931,14 +2931,14 @@ def test_appeal_approval_anonymous_report_with_email_post(self): assert self.appeal_mock.delay.call_args_list[0][0] == () assert self.appeal_mock.delay.call_args_list[0][1] == { 'appeal_text': 'I dont like this', - 'decision_cinder_id': self.cinder_job.decision.cinder_id, + 'decision_cinder_id': self.cinder_job.final_decision.cinder_id, 'abuse_report_id': self.abuse_report.id, 'user_id': None, 'is_reporter': True, } def test_appeal_approval_anonymous_report_with_email_post_cant_be_appealed(self): - self.cinder_job.decision.update(action_date=self.days_ago(200)) + self.cinder_job.final_decision.update(action_date=self.days_ago(200)) self.abuse_report.update(reporter_email='me@example.com') response = self.client.get(self.reporter_appeal_url) assert response.status_code == 200 @@ -2993,7 +2993,7 @@ def test_appeal_approval_loggued_in_user(self): assert self.appeal_mock.call_count == 0 def test_appeal_approval_logged_in_report_cant_be_appealed(self): - self.cinder_job.decision.update(action_date=self.days_ago(200)) + self.cinder_job.final_decision.update(action_date=self.days_ago(200)) self.user = user_factory() self.abuse_report.update(reporter=self.user) self.client.force_login(self.user) @@ -3008,19 +3008,19 @@ def test_appeal_approval_logged_in_report_cant_be_appealed(self): assert self.appeal_mock.call_count == 0 def test_appeal_rejection_not_logged_in(self): - self.cinder_job.decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) + self.cinder_job.final_decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) response = self.client.get(self.author_appeal_url) self.assertLoginRedirects(response, self.author_appeal_url) def test_appeal_rejection_not_author(self): - self.cinder_job.decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) + self.cinder_job.final_decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) user = user_factory() self.client.force_login(user) response = self.client.get(self.author_appeal_url) assert response.status_code == 403 def test_appeal_rejection_author(self): - self.cinder_job.decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) + self.cinder_job.final_decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) user = user_factory() self.addon.authors.add(user) self.client.force_login(user) @@ -3045,7 +3045,7 @@ def test_appeal_rejection_author(self): assert self.appeal_mock.delay.call_args_list[0][0] == () assert self.appeal_mock.delay.call_args_list[0][1] == { 'appeal_text': 'I dont like this', - 'decision_cinder_id': self.cinder_job.decision.cinder_id, + 'decision_cinder_id': self.cinder_job.final_decision.cinder_id, 'abuse_report_id': None, 'user_id': user.pk, 'is_reporter': False, @@ -3093,7 +3093,7 @@ def test_appeal_rejection_author_no_cinderjob(self): def test_appeal_banned_user(self): target = user_factory() - self.cinder_job.decision.update( + self.cinder_job.final_decision.update( action=DECISION_ACTIONS.AMO_BAN_USER, addon=None, user=target ) self.abuse_report.update(guid=None, user=target) @@ -3120,7 +3120,7 @@ def test_appeal_banned_user(self): assert self.appeal_mock.delay.call_args_list[0][0] == () assert self.appeal_mock.delay.call_args_list[0][1] == { 'appeal_text': 'I am not a bad guy', - 'decision_cinder_id': self.cinder_job.decision.cinder_id, + 'decision_cinder_id': self.cinder_job.final_decision.cinder_id, 'abuse_report_id': None, 'user_id': None, 'is_reporter': False, @@ -3128,7 +3128,7 @@ def test_appeal_banned_user(self): def test_appeal_banned_user_wrong_email(self): target = user_factory() - self.cinder_job.decision.update( + self.cinder_job.final_decision.update( action=DECISION_ACTIONS.AMO_BAN_USER, addon=None, user=target ) self.abuse_report.update(guid=None, user=target) @@ -3170,9 +3170,9 @@ def test_reporter_cant_appeal_appealed_decision_already_made_for_other_affirm(se reporter_email='otherreporter@example.com', ) appeal_job = CinderJob.objects.create(job_id='appeal job id') - self.cinder_job.decision.update(appeal_job=appeal_job) + self.cinder_job.final_decision.update(appeal_job=appeal_job) CinderAppeal.objects.create( - decision=self.cinder_job.decision, reporter_report=other_abuse_report + decision=self.cinder_job.final_decision, reporter_report=other_abuse_report ) self.client.force_login(user) @@ -3219,9 +3219,9 @@ def test_reporter_cant_appeal_appealed_decision_already_made_for_other_turned(se reporter_email='otherreporter@example.com', ) appeal_job = CinderJob.objects.create(job_id='appeal job id') - self.cinder_job.decision.update(appeal_job=appeal_job) + self.cinder_job.final_decision.update(appeal_job=appeal_job) CinderAppeal.objects.create( - decision=self.cinder_job.decision, reporter_report=other_abuse_report + decision=self.cinder_job.final_decision, reporter_report=other_abuse_report ) self.client.force_login(user) @@ -3272,7 +3272,7 @@ def test_reporter_cant_appeal_overridden_decision(self): cinder_id='appeal decision id', action=DECISION_ACTIONS.AMO_APPROVE, addon=self.addon, - override_of=self.cinder_job.decision, + override_of=self.cinder_job.final_decision, ) response = self.client.get(self.reporter_appeal_url) @@ -3287,7 +3287,7 @@ def test_reporter_cant_appeal_overridden_decision(self): ) def test_author_cant_appeal_overridden_decision(self): - self.cinder_job.decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) + self.cinder_job.final_decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) user = user_factory() self.addon.authors.add(user) self.client.force_login(user) @@ -3303,7 +3303,7 @@ def test_author_cant_appeal_overridden_decision(self): cinder_id='appeal decision id', action=DECISION_ACTIONS.AMO_APPROVE, addon=self.addon, - override_of=self.cinder_job.decision, + override_of=self.cinder_job.final_decision, ) response = self.client.get(self.author_appeal_url) @@ -3323,7 +3323,7 @@ def test_throttling_initial_email_form(self): 'Please try again after some time.' ) target = user_factory() - self.cinder_job.decision.update( + self.cinder_job.final_decision.update( action=DECISION_ACTIONS.AMO_BAN_USER, addon=None, user=target ) self.abuse_report.update(guid=None, user=target) @@ -3378,7 +3378,7 @@ def test_throttling_doesnt_reveal_validation_state_fields(self): 'Please try again after some time.' ) target = user_factory() - self.cinder_job.decision.update( + self.cinder_job.final_decision.update( action=DECISION_ACTIONS.AMO_BAN_USER, addon=None, user=target ) self.abuse_report.update(guid=None, user=target) @@ -3409,7 +3409,7 @@ def test_throttling_appeal_form(self): 'You have submitted this form too many times recently. ' 'Please try again after some time.' ) - self.cinder_job.decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) + self.cinder_job.final_decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) user = user_factory() self.addon.authors.add(user) self.client.force_login(user) diff --git a/src/olympia/reviewers/tests/test_forms.py b/src/olympia/reviewers/tests/test_forms.py index ce44d94f0271..910450dfcbc2 100644 --- a/src/olympia/reviewers/tests/test_forms.py +++ b/src/olympia/reviewers/tests/test_forms.py @@ -1197,16 +1197,16 @@ def test_cinder_jobs_to_resolve_choices(self): resolvable_in_reviewer_tools=True, target_addon=self.addon, ) - cinder_job_appealed.decision.update(appeal_job=cinder_job_appeal) + cinder_job_appealed.final_decision.update(appeal_job=cinder_job_appeal) appeal_obj = CinderAppeal.objects.create( text='some justification', - decision=cinder_job_appealed.decision, + decision=cinder_job_appealed.final_decision, ) # This wouldn't happen - a reporter can't appeal a disable decision # - but we want to test the rendering of reporter vs. developer appeal text CinderAppeal.objects.create( text='some other justification', - decision=cinder_job_appealed.decision, + decision=cinder_job_appealed.final_decision, reporter_report=appealed_abuse_report, ) diff --git a/src/olympia/reviewers/tests/test_utils.py b/src/olympia/reviewers/tests/test_utils.py index 8d80b1de505d..64572f32d608 100644 --- a/src/olympia/reviewers/tests/test_utils.py +++ b/src/olympia/reviewers/tests/test_utils.py @@ -4126,7 +4126,7 @@ def _record_decision_called_everywhere_checkbox_shown(self, actions): decision.action == actions[action_name]['cinder_action'] or policy.enforcement_actions ) - assert job.decision == decision + assert job.final_decision == decision reviewer_log_mock.reset_mock() content_action_log_spy.reset_mock() @@ -4374,12 +4374,12 @@ def test_resolve_appeal_job_policies(self): assert decision2.action == DECISION_ACTIONS.AMO_APPROVE assert decision1.activities.get() == log1 assert decision2.activities.get() == log2 - assert set(appeal_job1.reload().decision.policies.all()) == { + assert set(appeal_job1.reload().final_decision.policies.all()) == { policy_a, policy_b, policy_c, } - assert set(appeal_job2.reload().decision.policies.all()) == {policy_d} + assert set(appeal_job2.reload().final_decision.policies.all()) == {policy_d} assert decision1.metadata[ContentDecision.POLICY_DYNAMIC_VALUES] == { policy_a.uuid: { @@ -4543,7 +4543,7 @@ def test_request_legal_review_resolve_job(self): self._test_request_legal_review(data={'cinder_jobs_to_resolve': [job]}) # And check that the job was resolved in the way we expected - assert job.reload().decision.action == DECISION_ACTIONS.AMO_LEGAL_FORWARD + assert job.reload().final_decision.action == DECISION_ACTIONS.AMO_LEGAL_FORWARD # is cleared assert not NeedsHumanReview.objects.filter(