process jobs from legal escalations with no abuse reports (#23024)

* process jobs from legal escalations with no abuse reports

* be more explicit with NeedsHumanReviews in tests

* requested additionsl assertions

src/olympia/abuse/models.py

@@ -313,9 +313,9 @@ def process_decision(
                 if self.target_addon_id
                 else abuse_report_or_decision.addon
             ),
-            rating=abuse_report_or_decision.rating,
-            collection=abuse_report_or_decision.collection,
-            user=abuse_report_or_decision.user,
+            rating=getattr(abuse_report_or_decision, 'rating', None),
+            collection=getattr(abuse_report_or_decision, 'collection', None),
+            user=getattr(abuse_report_or_decision, 'user', None),
             cinder_id=decision_cinder_id,
             action=decision_action,
             notes=decision_notes[: ContentDecision._meta.get_field('notes').max_length],

src/olympia/abuse/tests/test_models.py

@@ -1250,6 +1250,37 @@ def test_process_decision_escalate_addon_action(self):
         assert new_job.target_addon == addon
         assert report.reload().cinder_job == new_job
 
+    def test_process_decision_for_legal_reviewed_job(self):
+        """An add-on forwarded to the legal queue for review will be a job that may not
+        contain any attached abuse reports (i.e. if an add-on was forwared without a
+        job)."""
+        target = addon_factory()
+        cinder_job = CinderJob.objects.create(job_id='1234', target_addon=target)
+        policy_a = CinderPolicy.objects.create(uuid='123-45', name='aaa', text='AAA')
+        policy_b = CinderPolicy.objects.create(uuid='678-90', name='bbb', text='BBB')
+        assert AbuseReport.objects.filter(cinder_job=cinder_job).count() == 0
+        assert ContentDecision.objects.filter(appeal_job=cinder_job).count() == 0
+
+        with mock.patch.object(
+            ContentActionDisableAddon, 'process_action'
+        ) as action_mock, mock.patch.object(
+            ContentActionDisableAddon, 'notify_owners'
+        ) as notify_mock:
+            action_mock.return_value = None
+            cinder_job.process_decision(
+                decision_cinder_id='12345',
+                decision_action=DECISION_ACTIONS.AMO_DISABLE_ADDON.value,
+                decision_notes='teh notes',
+                policy_ids=['123-45', '678-90'],
+            )
+        assert cinder_job.decision.cinder_id == '12345'
+        assert cinder_job.decision.action == DECISION_ACTIONS.AMO_DISABLE_ADDON
+        assert cinder_job.decision.notes == 'teh notes'
+        assert cinder_job.decision.addon == target
+        assert action_mock.call_count == 1
+        assert notify_mock.call_count == 1
+        assert list(cinder_job.decision.policies.all()) == [policy_a, policy_b]
+
     @override_switch('dsa-cinder-forwarded-review', active=True)
     def test_process_queue_move_into_reviewer_handled(self):
         addon = addon_factory(file_kw={'is_signed': True})

src/olympia/reviewers/tests/test_utils.py

@@ -3917,7 +3917,11 @@ def _test_request_legal_review(self, *, data=None):
             json={'job_id': '5678'},
             status=201,
         )
-
+        NeedsHumanReview.objects.create(
+            version=self.addon.current_version,
+            reason=NeedsHumanReview.REASONS.AUTO_APPROVAL_DISABLED,
+            is_active=True,
+        )
         self.helper.handler.request_legal_review()
 
         assert len(mail.outbox) == 0
@@ -3929,10 +3933,22 @@ def _test_request_legal_review(self, *, data=None):
             if data
             else self.get_data()['comments']
         )
+        assert not NeedsHumanReview.objects.filter(
+            reason=NeedsHumanReview.REASONS.AUTO_APPROVAL_DISABLED, is_active=True
+        ).exists()
 
     def test_request_legal_review_no_job(self):
+        NeedsHumanReview.objects.create(
+            version=self.addon.current_version,
+            reason=NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION,
+        )
         self._test_request_legal_review()
 
+        # is not cleared
+        assert NeedsHumanReview.objects.filter(
+            reason=NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION, is_active=True
+        ).exists()
+
     def test_request_legal_review_resolve_job(self):
         # Set up a typical job that would be handled in the reviewer tools
         job = CinderJob.objects.create(
@@ -3944,12 +3960,21 @@ def test_request_legal_review_resolve_job(self):
             f'{settings.CINDER_SERVER_URL}jobs/1234/decision',
             callback=lambda r: (201, {}, json.dumps({'uuid': uuid.uuid4().hex})),
         )
+        NeedsHumanReview.objects.create(
+            version=self.addon.current_version,
+            reason=NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION,
+        )
         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.forwarded_to_job == CinderJob.objects.get(job_id='5678')
 
+        # is cleared
+        assert not NeedsHumanReview.objects.filter(
+            reason=NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION, is_active=True
+        ).exists()
+
     def _test_single_action_remove_from_queue_history(
         self, review_action, log_action, channel=amo.CHANNEL_LISTED
     ):

src/olympia/reviewers/utils.py

@@ -1670,6 +1670,7 @@ def disable_addon(self):
     def request_legal_review(self):
         """Forward add-on and/or job to legal via Cinder."""
         log.info('Forwarding %s for legal review' % (self.addon))
+        self.clear_all_needs_human_review_flags_in_channel()
         self.record_decision(amo.LOG.REQUEST_LEGAL, action_completed=False)