track queue moves as a decision

Author: eviljeff

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)

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()

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'),

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(
             '</details>',
             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),

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[<script>alert()</script>]: ddd'
         )