report appeals for extensions to a different queue (#22851)

src/olympia/abuse/cinder.py

@@ -2,7 +2,6 @@ import mimetypes
 
 from django.conf import settings
 from django.core.files.storage import default_storage as storage
-from django.utils.functional import classproperty
 
 import requests
 import waffle
@@ -31,6 +30,11 @@ class CinderEntity:
     def queue(self):
         return f'{settings.CINDER_QUEUE_PREFIX}{self.queue_suffix}'
 
+    @property
+    def queue_appeal(self):
+        # By default it's the same queue
+        return self.queue
+
     @property
     def id(self):
         # Ideally override this in subclasses to be more efficient
@@ -151,7 +155,7 @@ class CinderEntity:
             raise NotImplementedError
         url = f'{settings.CINDER_SERVER_URL}appeal'
         data = {
-            'queue_slug': self.queue,
+            'queue_slug': self.queue_appeal,
             'appealer_entity_type': appealer.type,
             'appealer_entity': appealer.get_attributes(),
             'reasoning': self.get_str(appeal_text),
@@ -302,9 +306,8 @@ class CinderUnauthenticatedReporter(CinderEntity):
 class CinderAddon(CinderEntity):
     type = 'amo_addon'
 
-    def __init__(self, addon, version_string=None):
+    def __init__(self, addon):
         self.addon = addon
-        self.version_string = version_string
         self.related_users = self.addon.authors.all()
 
     @property
@@ -315,6 +318,14 @@ class CinderAddon(CinderEntity):
     def queue_suffix(self):
         return 'themes' if self.addon.type == amo.ADDON_STATICTHEME else 'listings'
 
+    @property
+    def queue_appeal(self):
+        return (
+            self.queue
+            if self.addon.type == amo.ADDON_STATICTHEME
+            else 'amo-escalations'
+        )
+
     def get_attributes(self):
         # We look at the promoted group to tell whether or not the add-on is
         # promoted in any way, but we don't care about the promotion being
@@ -474,9 +485,14 @@ class CinderAddonHandledByReviewers(CinderAddon):
     # This queue is not monitored on cinder - reports are resolved via AMO instead
     queue_suffix = 'addon-infringement'
 
-    @classproperty  # Special case, needs to be static for the webhook check.
-    def queue(cls):
-        return f'{settings.CINDER_QUEUE_PREFIX}{cls.queue_suffix}'
+    def __init__(self, addon, *, version_string=None):
+        super().__init__(addon)
+        self.version_string = version_string
+
+    @property
+    def queue_appeal(self):
+        # No special appeal queue for reviewer handled jobs
+        return self.queue
 
     def flag_for_human_review(
         self, *, reported_versions, appeal=False, forwarded=False

src/olympia/abuse/models.py

@@ -140,9 +140,11 @@ class CinderJob(ModelBase):
     ):
         if isinstance(target, Addon):
             if resolved_in_reviewer_tools:
-                return CinderAddonHandledByReviewers(target, addon_version_string)
+                return CinderAddonHandledByReviewers(
+                    target, version_string=addon_version_string
+                )
             else:
-                return CinderAddon(target, addon_version_string)
+                return CinderAddon(target)
         elif isinstance(target, UserProfile):
             return CinderUser(target)
         elif isinstance(target, Rating):
@@ -321,12 +323,10 @@ class CinderJob(ModelBase):
 
     def process_queue_move(self, *, new_queue, notes):
         CinderQueueMove.objects.create(cinder_job=self, notes=notes, to_queue=new_queue)
-        if new_queue == CinderAddonHandledByReviewers.queue:
+        if new_queue == CinderAddonHandledByReviewers(self.target).queue:
             # now escalated
             entity_helper = CinderJob.get_entity_helper(
-                self.target,
-                addon_version_string=None,
-                resolved_in_reviewer_tools=True,
+                self.target, resolved_in_reviewer_tools=True
             )
             entity_helper.workflow_move(job=self)
             self.update(resolvable_in_reviewer_tools=True)

src/olympia/abuse/tasks.py

@@ -206,9 +206,7 @@ def sync_cinder_policies():
 def handle_escalate_action(*, job_pk):
     old_job = CinderJob.objects.get(id=job_pk)
     entity_helper = CinderJob.get_entity_helper(
-        old_job.target,
-        addon_version_string=None,
-        resolved_in_reviewer_tools=True,
+        old_job.target, resolved_in_reviewer_tools=True
     )
     job_id = entity_helper.workflow_recreate(job=old_job)
 

src/olympia/abuse/tests/test_cinder.py

@@ -59,6 +59,11 @@ class BaseTestCinderCase:
             == f'{settings.CINDER_QUEUE_PREFIX}{cinder_entity.queue_suffix}'
         )
 
+    def test_queue_appeal(self):
+        target = self._create_dummy_target()
+        cinder_entity = self.CinderClass(target)
+        assert cinder_entity.queue == cinder_entity.queue_appeal
+
     def _create_dummy_target(self, **kwargs):
         raise NotImplementedError
 
@@ -188,7 +193,7 @@ class TestCinderAddon(BaseTestCinderCase, TestCase):
     def _create_dummy_target(self, **kwargs):
         return addon_factory(**kwargs)
 
-    def test_queue_theme(self):
+    def test_queue_with_theme(self):
         target = self._create_dummy_target(type=amo.ADDON_STATICTHEME)
         cinder_entity = self.CinderClass(target)
         expected_queue_suffix = 'themes'
@@ -198,6 +203,14 @@ class TestCinderAddon(BaseTestCinderCase, TestCase):
             == f'{settings.CINDER_QUEUE_PREFIX}{cinder_entity.queue_suffix}'
         )
 
+    def test_queue_appeal(self):
+        extension = self._create_dummy_target()
+        assert self.CinderClass(extension).queue_appeal == 'amo-escalations'
+
+        theme = self._create_dummy_target(type=amo.ADDON_STATICTHEME)
+        # we only have a special queue for extensions
+        assert self.CinderClass(theme).queue == f'{settings.CINDER_QUEUE_PREFIX}themes'
+
     def test_build_report_payload(self):
         addon = self._create_dummy_target(
             homepage='https://home.example.com',
@@ -1087,12 +1100,7 @@ class TestCinderAddonHandledByReviewers(TestCinderAddon):
     expected_queries_for_report = 2
     expected_queue_suffix = 'addon-infringement'
 
-    def test_queue(self):
-        super().test_queue()
-        # For this class the property should be guaranteed to be static.
-        assert self.CinderClass.queue == 'amo-env-addon-infringement'
-
-    def test_queue_theme(self):
+    def test_queue_with_theme(self):
         # Contrary to reports handled by Cinder moderators, for reports handled
         # by AMO reviewers the queue should remain the same regardless of the
         # addon-type.
@@ -1104,6 +1112,11 @@ class TestCinderAddonHandledByReviewers(TestCinderAddon):
             == f'{settings.CINDER_QUEUE_PREFIX}{cinder_entity.queue_suffix}'
         )
 
+    def test_queue_appeal(self):
+        # Contrary to reports handled by Cinder moderators, for reports handled
+        # by AMO reviewers there is no special queue.
+        BaseTestCinderCase.test_queue_appeal(self)
+
     def setUp(self):
         self.task_user = user_factory(id=settings.TASK_USER_ID)
 
@@ -1175,7 +1188,7 @@ class TestCinderAddonHandledByReviewers(TestCinderAddon):
             guid=addon.guid, addon_version=other_version.version
         )
         report = CinderReport(abuse_report)
-        cinder_instance = self.CinderClass(addon, other_version)
+        cinder_instance = self.CinderClass(addon, version_string=other_version)
         assert cinder_instance.report(report=report, reporter=None)
         job = CinderJob.objects.create(job_id='1234-xyz')
         assert not addon.current_version.needshumanreview_set.exists()

src/olympia/abuse/tests/test_models.py

@@ -824,18 +824,6 @@ class TestCinderJob(TestCase):
         assert isinstance(helper, CinderAddon)
         assert not isinstance(helper, CinderAddonHandledByReviewers)
         assert helper.addon == addon
-        assert helper.version_string is None
-
-        helper = CinderJob.get_entity_helper(
-            addon,
-            resolved_in_reviewer_tools=False,
-            addon_version_string=addon.current_version.version,
-        )
-        # but not if the location is not in REVIEWER_HANDLED (i.e. AMO)
-        assert isinstance(helper, CinderAddon)
-        assert not isinstance(helper, CinderAddonHandledByReviewers)
-        assert helper.addon == addon
-        assert helper.version_string == addon.current_version
 
         helper = CinderJob.get_entity_helper(addon, resolved_in_reviewer_tools=True)
         # if now reason is in REVIEWER_HANDLED it will be reported differently
@@ -2404,7 +2392,7 @@ class TestContentDecision(TestCase):
             assert ActionClass.reporter_template_path is not None
             assert ActionClass.reporter_appeal_template_path is not None
 
-    def _test_appeal_as_target(self, *, resolvable_in_reviewer_tools):
+    def _test_appeal_as_target(self, *, resolvable_in_reviewer_tools, expected_queue):
         addon = addon_factory(
             status=amo.STATUS_DISABLED,
             file_kw={'is_signed': True, 'status': amo.STATUS_DISABLED},
@@ -2424,7 +2412,7 @@ class TestContentDecision(TestCase):
                 ),
             ),
         )
-        responses.add(
+        appeal_response = responses.add(
             responses.POST,
             f'{settings.CINDER_SERVER_URL}appeal',
             json={'external_id': '2432615184-tsol'},
@@ -2449,10 +2437,22 @@ class TestContentDecision(TestCase):
         assert appeal_text_obj.text == 'appeal text'
         assert appeal_text_obj.decision == abuse_report.cinder_job.decision
         assert appeal_text_obj.reporter_report is None
+
+        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(
+            abuse_report.cinder_job.decision.cinder_id
+        )
+        assert request_body['queue_slug'] == expected_queue
+
         return abuse_report.cinder_job.decision.appeal_job.reload()
 
     def test_appeal_as_target_from_resolved_in_cinder(self):
-        appeal_job = self._test_appeal_as_target(resolvable_in_reviewer_tools=False)
+        appeal_job = self._test_appeal_as_target(
+            resolvable_in_reviewer_tools=False, expected_queue='amo-escalations'
+        )
         assert not appeal_job.resolvable_in_reviewer_tools
         assert not (
             NeedsHumanReview.objects.all()
@@ -2461,7 +2461,10 @@ class TestContentDecision(TestCase):
         )
 
     def test_appeal_as_target_from_resolved_in_amo(self):
-        appeal_job = self._test_appeal_as_target(resolvable_in_reviewer_tools=True)
+        appeal_job = self._test_appeal_as_target(
+            resolvable_in_reviewer_tools=True,
+            expected_queue='amo-env-addon-infringement',
+        )
         assert appeal_job.resolvable_in_reviewer_tools
         assert (
             NeedsHumanReview.objects.all()
@@ -2609,7 +2612,7 @@ class TestContentDecision(TestCase):
                 ),
             )
         )
-        responses.add(
+        appeal_response = responses.add(
             responses.POST,
             f'{settings.CINDER_SERVER_URL}appeal',
             json={'external_id': '2432615184-tsol'},
@@ -2635,6 +2638,15 @@ class TestContentDecision(TestCase):
         assert appeal_text_obj.decision == abuse_report.cinder_job.decision
         assert appeal_text_obj.reporter_report == abuse_report
 
+        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(
+            abuse_report.cinder_job.decision.cinder_id
+        )
+        assert request_body['queue_slug'] == 'amo-escalations'
+
     def test_appeal_as_reporter_already_appealed(self):
         addon = addon_factory()
         abuse_report = AbuseReport.objects.create(

src/olympia/abuse/tests/test_tasks.py

@@ -472,7 +472,7 @@ def test_addon_appeal_to_cinder_reporter(statsd_incr_mock):
         },
         'appealer_entity_type': 'amo_unauthenticated_reporter',
         'decision_to_appeal_id': '4815162342-abc',
-        'queue_slug': 'amo-env-listings',
+        'queue_slug': 'amo-escalations',
         'reasoning': 'I appeal',
     }
 
@@ -576,7 +576,7 @@ def test_addon_appeal_to_cinder_authenticated_reporter():
         },
         'appealer_entity_type': 'amo_user',
         'decision_to_appeal_id': '4815162342-abc',
-        'queue_slug': 'amo-env-listings',
+        'queue_slug': 'amo-escalations',
         'reasoning': 'I appeal',
     }
 

src/olympia/abuse/tests/test_views.py

@@ -1498,24 +1498,6 @@ class TestCinderWebhook(TestCase):
         data['payload']['source']['job']['queue']['slug'] = 'amo-another-queue'
         return self.test_process_decision_called(data)
 
-    def test_queue_handled_reviewer_queue_ignored(self):
-        data = self.get_data()
-        data['payload']['source']['job']['queue']['slug'] = 'amo-addon-infringement'
-        abuse_report = self._setup_reports()
-        addon_factory(guid=abuse_report.guid)
-        req = self.get_request(data=data)
-        with mock.patch.object(CinderJob, 'process_decision') as process_mock:
-            response = cinder_webhook(req)
-            process_mock.assert_not_called()
-        assert response.status_code == 200
-        assert response.data == {
-            'amo': {
-                'received': True,
-                'handled': False,
-                'not_handled_reason': 'Queue handled by AMO reviewers',
-            }
-        }
-
     def test_unknown_event(self):
         self._setup_reports()
         data = self.get_data()

src/olympia/abuse/views.py

@@ -20,7 +20,6 @@ from rest_framework.response import Response
 from rest_framework.viewsets import GenericViewSet
 
 import olympia.core.logger
-from olympia.abuse.cinder import CinderAddonHandledByReviewers
 from olympia.abuse.tasks import appeal_to_cinder
 from olympia.accounts.utils import redirect_for_login
 from olympia.accounts.views import AccountViewSet
@@ -185,16 +184,9 @@ def filter_enforcement_actions(enforcement_actions, cinder_job):
 
 def process_webhook_payload_decision(payload):
     source = payload.get('source', {})
+    log.info('Valid Payload from AMO queue: %s', payload)
     if 'job' in source:
-        job = source.get('job', {})
-        if (
-            queue_name := job.get('queue', {}).get('slug')
-        ) == CinderAddonHandledByReviewers.queue:
-            log.info('Payload from queue handled by reviewers: %s', queue_name)
-            raise ValidationError('Queue handled by AMO reviewers')
-
-        log.info('Valid Payload from AMO queue: %s', payload)
-        job_id = job.get('id', '')
+        job_id = source.get('job', {}).get('id', '')
 
         try:
             cinder_job = CinderJob.objects.get(job_id=job_id)