diff --git a/src/olympia/abuse/models.py b/src/olympia/abuse/models.py index a5ce44f3082d..93d3db7e6854 100644 --- a/src/olympia/abuse/models.py +++ b/src/olympia/abuse/models.py @@ -59,10 +59,13 @@ def unresolved(self): Q(decisions__isnull=True) | Q( # i.e. the latest decision is a requeue - decisions__action=DECISION_ACTIONS.AMO_REQUEUE, + decisions__action__in=( + DECISION_ACTIONS.AMO_REQUEUE, + DECISION_ACTIONS.AMO_ESCALATE_ADDON, + ), decisions__overridden_by__isnull=True, ) - ) + ).distinct() def resolvable_in_reviewer_tools(self): return self.filter(resolvable_in_reviewer_tools=True) @@ -333,8 +336,15 @@ def process_decision( decision.send_notifications() def process_queue_move(self, *, new_queue, notes): - CinderQueueMove.objects.create(cinder_job=self, notes=notes, to_queue=new_queue) if new_queue == CinderAddonHandledByReviewers(self.target).queue: + ContentDecision.objects.create( + addon=self.target_addon, + action=DECISION_ACTIONS.AMO_ESCALATE_ADDON, + override_of=self.decision, + action_date=datetime.now(), + private_notes=notes, + cinder_job=self, + ) # now escalated entity_helper = CinderJob.get_entity_helper( self.target, resolved_in_reviewer_tools=True @@ -369,20 +379,20 @@ def clear_needs_human_review_flags(self): .unresolved() .resolvable_in_reviewer_tools() ) - if ( - (decision_actions := tuple(self.decisions.values_list('action', flat=True))) - # i.e. was the previous decision before the current a requeue - and len(decision_actions) >= 2 - and decision_actions[-2] == DECISION_ACTIONS.AMO_REQUEUE - ): + # the previous decision before this new decision + penultimate_action = tuple(self.decisions.values_list('action', flat=True))[ + -2:-1 + ] + if penultimate_action == (DECISION_ACTIONS.AMO_REQUEUE,): has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter( decisions__action=DECISION_ACTIONS.AMO_REQUEUE, decisions__overridden_by__isnull=True, ).exists() reasons = {NeedsHumanReview.REASONS.SECOND_LEVEL_REQUEUE} - elif self.queue_moves.exists(): + elif penultimate_action == (DECISION_ACTIONS.AMO_ESCALATE_ADDON,): has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter( - queue_moves__id__gt=0 + decisions__action=DECISION_ACTIONS.AMO_ESCALATE_ADDON, + decisions__overridden_by__isnull=True, ).exists() reasons = { NeedsHumanReview.REASONS.CINDER_ESCALATION, @@ -1477,11 +1487,3 @@ class CinderAppeal(ModelBase): reporter_report = models.OneToOneField( to=AbuseReport, on_delete=models.CASCADE, null=True ) - - -class CinderQueueMove(ModelBase): - cinder_job = models.ForeignKey( - to=CinderJob, on_delete=models.CASCADE, related_name='queue_moves' - ) - notes = models.TextField(max_length=1000, blank=True) - to_queue = models.CharField(max_length=128) diff --git a/src/olympia/abuse/tests/test_models.py b/src/olympia/abuse/tests/test_models.py index a52d8c40105b..f66fad10fb4e 100644 --- a/src/olympia/abuse/tests/test_models.py +++ b/src/olympia/abuse/tests/test_models.py @@ -62,7 +62,6 @@ CinderAppeal, CinderJob, CinderPolicy, - CinderQueueMove, ContentDecision, ) @@ -777,8 +776,11 @@ def test_reviewer_handled(self): assert list(qs) == [job, appeal_job] not_policy_report.cinder_job.update(resolvable_in_reviewer_tools=True) - CinderQueueMove.objects.create( - cinder_job=not_policy_report.cinder_job, to_queue='?' + ContentDecision.objects.create( + action=DECISION_ACTIONS.AMO_ESCALATE_ADDON, + action_date=datetime.now(), + cinder_job=not_policy_report.cinder_job, + addon=not_policy_report.target, ) qs = CinderJob.objects.resolvable_in_reviewer_tools() assert list(qs) == [not_policy_report.cinder_job, job, appeal_job] @@ -1401,9 +1403,10 @@ def test_process_queue_move_into_reviewer_handled(self): nhr = NeedsHumanReview.objects.get() assert nhr.reason == NeedsHumanReview.REASONS.CINDER_ESCALATION assert nhr.version == addon.current_version - assert CinderQueueMove.objects.filter( - cinder_job=cinder_job, to_queue='amo-env-addon-infringement', notes='notes!' - ).exists() + assert cinder_job.decision.private_notes == 'notes!' + assert cinder_job.decision.action == DECISION_ACTIONS.AMO_ESCALATE_ADDON + assert cinder_job.decision.cinder_job == cinder_job + assert cinder_job.decision.addon == cinder_job.target_addon def test_process_queue_move_out_of_reviewer_handled(self): # Not yet implemented, so just check it's silently ignored @@ -1421,9 +1424,7 @@ def test_process_queue_move_out_of_reviewer_handled(self): assert cinder_job.resolvable_in_reviewer_tools is True assert len(mail.outbox) == 0 assert NeedsHumanReview.objects.count() == 1 - assert CinderQueueMove.objects.filter( - cinder_job=cinder_job, to_queue='amo-env-listings', notes='out' - ).exists() + assert not cinder_job.decision def test_process_queue_move_other_queue_movement(self): # we don't need to about these other queue moves, so just check it's silently @@ -1436,9 +1437,7 @@ def test_process_queue_move_other_queue_movement(self): assert not cinder_job.resolvable_in_reviewer_tools assert len(mail.outbox) == 0 assert NeedsHumanReview.objects.count() == 0 - assert CinderQueueMove.objects.filter( - cinder_job=cinder_job, to_queue='amo-env-some-other-queue', notes='?' - ).exists() + assert not cinder_job.decision @override_switch('dsa-cinder-forwarded-review', active=True) def test_process_queue_move_with_addon_already_moderated(self): @@ -1476,7 +1475,11 @@ def test_process_queue_move_with_addon_already_moderated(self): assert mail.outbox[0].to == ['some@email.com'] assert 'already assessed' in mail.outbox[0].body assert ContentDecision.objects.exists() - decision = ContentDecision.objects.get() + assert ContentDecision.objects.count() == 2 + assert ContentDecision.objects.first().action == ( + DECISION_ACTIONS.AMO_ESCALATE_ADDON + ) + decision = ContentDecision.objects.last() assert decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION self.assertCloseToNow(decision.action_date) assert decision.cinder_job == CinderJob.objects.get() @@ -1612,14 +1615,24 @@ def test_clear_needs_human_review_flags_abuse(self): def test_clear_needs_human_review_flags_forwarded_moved_queue(self): job = self._setup_clear_needs_human_review_flags() # if the job is forwarded, we make sure that there are no other forwarded jobs - CinderQueueMove.objects.create(cinder_job=job, to_queue='wherever') + job.decision.update(action=DECISION_ACTIONS.AMO_ESCALATE_ADDON) + ContentDecision.objects.create( + action=DECISION_ACTIONS.AMO_APPROVE, + addon=job.target_addon, + cinder_job=job, + override_of=job.final_decision, + ) other_forward = CinderJob.objects.create( job_id='3', target_addon=job.target_addon, resolvable_in_reviewer_tools=True, ) - CinderQueueMove.objects.create(cinder_job=other_forward, to_queue='whoever') + ContentDecision.objects.create( + action=DECISION_ACTIONS.AMO_ESCALATE_ADDON, + addon=job.target_addon, + cinder_job=other_forward, + ) job.clear_needs_human_review_flags() assert self._nhr_exists(NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION) assert self._nhr_exists(NeedsHumanReview.REASONS.CINDER_ESCALATION) @@ -1632,6 +1645,7 @@ def test_clear_needs_human_review_flags_forwarded_moved_queue(self): action=DECISION_ACTIONS.AMO_APPROVE, addon=job.target_addon, cinder_job=other_forward, + override_of=other_forward.decision, ) job.clear_needs_human_review_flags() assert self._nhr_exists(NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION) @@ -3507,17 +3521,24 @@ def test_send_notifications_without_notifying_owners(self): def test_resolve_job_forwarded(self): addon_developer = user_factory() addon = addon_factory(users=[addon_developer]) + cinder_job = CinderJob.objects.create(job_id='999') + ContentDecision.objects.create( + addon=addon, + action=DECISION_ACTIONS.AMO_ESCALATE_ADDON, + action_date=datetime.now(), + private_notes='why moved', + cinder_job=cinder_job, + ) decision = ContentDecision.objects.create( addon=addon, action=DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON, action_date=datetime.now(), reasoning='some review text', reviewer_user=self.reviewer_user, + cinder_job=cinder_job, ) policies = [CinderPolicy.objects.create(name='policy', uuid='12345678')] decision.policies.set(policies) - cinder_job = CinderJob.objects.create(job_id='999', decision=decision) - CinderQueueMove.objects.create(cinder_job=cinder_job, to_queue='wherever') NeedsHumanReview.objects.create( version=addon.current_version, reason=NeedsHumanReview.REASONS.CINDER_ESCALATION, @@ -3563,14 +3584,20 @@ def test_resolve_job_forwarded(self): def test_execute_action_forwarded_to_legal(self): addon_developer = user_factory() addon = addon_factory(users=[addon_developer]) + cinder_job = CinderJob.objects.create(job_id='999') + ContentDecision.objects.create( + addon=addon, + action=DECISION_ACTIONS.AMO_ESCALATE_ADDON, + private_notes='why moved', + cinder_job=cinder_job, + ) decision = ContentDecision.objects.create( addon=addon, action=DECISION_ACTIONS.AMO_LEGAL_FORWARD, reasoning='some reasoning', reviewer_user=self.reviewer_user, + cinder_job=cinder_job, ) - cinder_job = CinderJob.objects.create(job_id='999', decision=decision) - CinderQueueMove.objects.create(cinder_job=cinder_job, to_queue='wherever') NeedsHumanReview.objects.create( version=addon.current_version, reason=NeedsHumanReview.REASONS.CINDER_ESCALATION, @@ -3598,8 +3625,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() diff --git a/src/olympia/constants/abuse.py b/src/olympia/constants/abuse.py index edbd21c9135a..7d7d4979bb6a 100644 --- a/src/olympia/constants/abuse.py +++ b/src/olympia/constants/abuse.py @@ -8,7 +8,7 @@ ('AMO_BAN_USER', 1, 'User ban'), ('AMO_DISABLE_ADDON', 2, 'Add-on disable'), # Used to indicate the job has been forwarded to AMO - ('AMO_ESCALATE_ADDON', 3, '(Obsolete) Forward add-on to reviewers'), + ('AMO_ESCALATE_ADDON', 3, 'Forward add-on to reviewers'), # 4 is unused ('AMO_DELETE_RATING', 5, 'Rating delete'), ('AMO_DELETE_COLLECTION', 6, 'Collection delete'), diff --git a/src/olympia/reviewers/forms.py b/src/olympia/reviewers/forms.py index 72d550f10a49..83ad9df46162 100644 --- a/src/olympia/reviewers/forms.py +++ b/src/olympia/reviewers/forms.py @@ -246,15 +246,34 @@ def create_option( # label_from_instance() on WidgetRenderedModelMultipleChoiceField returns the # full object, not a label, this is what makes this work. obj = label + + forwarded_or_requeued_decisions = list( + obj.decisions.filter( + action__in=( + DECISION_ACTIONS.AMO_REQUEUE, + DECISION_ACTIONS.AMO_ESCALATE_ADDON, + ) + ).order_by('-created') + ) + is_appeal = obj.is_appeal - queue_moves = list(obj.queue_moves.order_by('-created')) - requeued_decisions = list( - obj.decisions.filter(action=DECISION_ACTIONS.AMO_REQUEUE).order_by( - '-created' - ) + forwarded = next( + ( + decision + for decision in forwarded_or_requeued_decisions + if decision.action == DECISION_ACTIONS.AMO_ESCALATE_ADDON + ), + None, ) - forwarded = queue_moves[0].created if queue_moves else None - requeued = requeued_decisions[0].created if requeued_decisions else None + requeued = next( + ( + decision + for decision in forwarded_or_requeued_decisions + if decision.action == DECISION_ACTIONS.AMO_REQUEUE + ), + None, + ) + reports = obj.all_abuse_reports reasons_set = { (report.REASONS.for_value(report.reason).display,) for report in reports @@ -266,13 +285,12 @@ def create_option( ) for report in reports ) - forwarded_or_requeued_notes = [ - *(move.notes for move in queue_moves), - *(decision.private_notes for decision in requeued_decisions), - ] + internal_notes_gen = ( + decision.private_notes for decision in forwarded_or_requeued_decisions + ) internal_notes = ( - ((f'Reasoning: {"; ".join(forwarded_or_requeued_notes)}',),) - if forwarded_or_requeued_notes + ((f'Reasoning: {"; ".join(internal_notes_gen)}',),) + if forwarded_or_requeued_decisions else () ) appeals = ( @@ -294,10 +312,10 @@ def create_option( '', format_datetime(obj.created), '[Appeal] ' if is_appeal else '', - format_html('[Forwarded on {}] ', format_datetime(forwarded)) + format_html('[Forwarded on {}] ', format_datetime(forwarded.created)) if forwarded else '', - format_html('[Requeued on {}] ', format_datetime(requeued)) + format_html('[Requeued on {}] ', format_datetime(requeued.created)) if requeued else '', format_html_join(', ', '"{}"', reasons_set), diff --git a/src/olympia/reviewers/tests/test_forms.py b/src/olympia/reviewers/tests/test_forms.py index ed0f18843b7d..7b30d1ddde41 100644 --- a/src/olympia/reviewers/tests/test_forms.py +++ b/src/olympia/reviewers/tests/test_forms.py @@ -14,7 +14,6 @@ CinderAppeal, CinderJob, CinderPolicy, - CinderQueueMove, ContentDecision, ) from olympia.addons.models import Addon @@ -1338,11 +1337,12 @@ def test_cinder_jobs_to_resolve_choices(self): addon=self.addon, cinder_job=cinder_job_forwarded, ) - CinderQueueMove.objects.create( + ContentDecision.objects.create( created=datetime(2025, 5, 22, 11, 42, 5, 541216), + action=DECISION_ACTIONS.AMO_ESCALATE_ADDON, + private_notes='Zee de zee', + addon=self.addon, cinder_job=cinder_job_forwarded, - notes='Zee de zee', - to_queue='amo-env-content-infringment', ) AbuseReport.objects.create( **{**abuse_kw, 'location': AbuseReport.LOCATION.AMO}, @@ -1391,7 +1391,7 @@ def test_cinder_jobs_to_resolve_choices(self): '[Forwarded on May 22, 2025, 11:42 a.m.] ' '[Requeued on May 23, 2025, 10:54 p.m.] ' '"DSA: It violates Mozilla\'s Add-on Policies"\n' - 'Reasoning: Zee de zee; Why o why\n\n' + 'Reasoning: Why o why; Zee de zee\n\n' 'Show detail on 1 reports\n' 'v[]: ddd' )