From e9323ffe08d5e465eaddeafe995819b5f80033cd Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Thu, 2 Apr 2026 17:28:32 +0100 Subject: [PATCH] Create a content review cinderjob on created webhook payload --- src/olympia/abuse/cinder.py | 5 + .../0064_cinderjob_content_review_and_more.py | 23 ++++ src/olympia/abuse/models.py | 8 +- .../job_actioned_workflow_created_job.json | 48 ++++++++ src/olympia/abuse/tests/test_models.py | 40 ++++++ src/olympia/abuse/tests/test_views.py | 67 ++++++++-- src/olympia/abuse/views.py | 116 ++++++++++++------ 7 files changed, 260 insertions(+), 47 deletions(-) create mode 100644 src/olympia/abuse/migrations/0064_cinderjob_content_review_and_more.py create mode 100644 src/olympia/abuse/tests/assets/cinder_webhook_payloads/job_actioned_workflow_created_job.json diff --git a/src/olympia/abuse/cinder.py b/src/olympia/abuse/cinder.py index 852a60926138..dcdaa9734040 100644 --- a/src/olympia/abuse/cinder.py +++ b/src/olympia/abuse/cinder.py @@ -647,6 +647,10 @@ class WorkflowEventSendMixin: workflow_name = None # Needs to be defined by subclasses + @property + def workflow_pretty_name(self): + return self.workflow_name + def change_to_v2_syntax(self, subgraph): """api/v2 syntax is a bit different, we need to change the format of the subgraph a bit before sending it.""" @@ -697,6 +701,7 @@ def send_event(self): class CinderAddonContentReview(WorkflowEventSendMixin, CinderAddon): queue_suffix = 'listing-content' workflow_name = 'amo_addon.contentreview' + workflow_pretty_name = 'Amo Addon Contentreview' def appeal(self, *, decision_cinder_id, **kwargs): # We don't flag for NHR for content review follow-ups diff --git a/src/olympia/abuse/migrations/0064_cinderjob_content_review_and_more.py b/src/olympia/abuse/migrations/0064_cinderjob_content_review_and_more.py new file mode 100644 index 000000000000..20aef6d2f92a --- /dev/null +++ b/src/olympia/abuse/migrations/0064_cinderjob_content_review_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 5.2.12 on 2026-04-02 16:04 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('abuse', '0063_create_content_review_in_cinder_switch'), + ] + + operations = [ + migrations.AddField( + model_name='cinderjob', + name='content_review', + field=models.BooleanField(default=False, null=True), + ), + migrations.AlterField( + model_name='contentdecision', + name='action', + field=models.PositiveSmallIntegerField(choices=[(1, 'User ban'), (2, 'Add-on disable'), (3, '(Obsolete) Forward add-on to reviewers'), (5, 'Rating delete'), (6, 'Collection delete'), (7, 'Approved (no action)'), (8, 'Add-on version reject'), (9, 'Add-on version delayed reject warning'), (10, 'Approved (new version approval)'), (11, 'Invalid report, so ignored'), (12, 'Content already moderated (no action)'), (13, 'Forward add-on to legal'), (14, 'Pending rejection date changed'), (15, 'No action - internal requeue'), (16, 'Add-on disable and block'), (17, 'Add-on listing content rejection')]), + ), + ] diff --git a/src/olympia/abuse/models.py b/src/olympia/abuse/models.py index bb679dc8af2e..ddea00c75b94 100644 --- a/src/olympia/abuse/models.py +++ b/src/olympia/abuse/models.py @@ -97,6 +97,7 @@ class CinderJob(ModelBase): to=Addon, blank=True, null=True, on_delete=models.deletion.SET_NULL ) resolvable_in_reviewer_tools = models.BooleanField(default=None, null=True) + content_review = models.BooleanField(default=False, null=True) objects = CinderJobManager() @@ -1280,7 +1281,11 @@ def appeal(self, *, abuse_report, appeal_text, user, is_reporter): ) if not self.can_be_appealed(is_reporter=is_reporter, abuse_report=abuse_report): raise CantBeAppealed - is_content_review = self.action == DECISION_ACTIONS.AMO_REJECT_LISTING_CONTENT + is_content_review = ( + self.cinder_job.content_review + if self.cinder_job + else self.action == DECISION_ACTIONS.AMO_REJECT_LISTING_CONTENT + ) entity_helper = CinderJob.get_entity_helper( self.target, @@ -1299,6 +1304,7 @@ def appeal(self, *, abuse_report, appeal_text, user, is_reporter): defaults={ 'target_addon': self.addon, 'resolvable_in_reviewer_tools': resolvable_in_reviewer_tools, + 'content_review': is_content_review, }, ) self.update(appeal_job=appeal_job) diff --git a/src/olympia/abuse/tests/assets/cinder_webhook_payloads/job_actioned_workflow_created_job.json b/src/olympia/abuse/tests/assets/cinder_webhook_payloads/job_actioned_workflow_created_job.json new file mode 100644 index 000000000000..3939b97fe9e1 --- /dev/null +++ b/src/olympia/abuse/tests/assets/cinder_webhook_payloads/job_actioned_workflow_created_job.json @@ -0,0 +1,48 @@ +{ + "event": "job.actioned", + "payload": { + "action": "created", + "action_made_by": { + "workflow": { + "event_id": "019d4912-4851-7242-bd0b-f16c3ab57da5", + "name": "Amo Addon Contentreview" + } + }, + "job": { + "created_at": "2026-04-01T12:43:50.782776Z", + "entity": { + "attributes": { + "average_daily_users": 0, + "created": "2026-04-01T12:19:58.646580", + "description": "en-US", + "guid": "{95a42dcb-a337-46a8-8292-d3bc8a48b647}", + "homepage": "https://addons-server.readthedocs.io/en/latest/topics/api/addons.html#create", + "id": "10000", + "last_updated": "2026-04-01T12:19:58.646580", + "name": "A string to be detected recommended", + "privacy_policy": "", + "promoted": "", + "release_notes": "EN-US Version notes added in API at addon creation time", + "requires_payment": false, + "slug": "a-string-to-be-detected-rec4", + "summary": "change summary from API from API333", + "support_email": "rusiczki.ioana@test.com", + "version": "3.0" + }, + "entity_schema": "amo_addon" + }, + "id": "0aa2e6b6-9899-4bd0-9f59-94e8762861ba", + "job_category": "standard", + "num_reports": 0, + "priority": 0, + "queue": { + "is_multi_review": false, + "slug": "amo-dev-listing-content" + }, + "status": "open" + }, + "notes": "", + "source": "workflow", + "timestamp": "2026-04-01T12:43:50.791551+00:00" + } +} \ No newline at end of file diff --git a/src/olympia/abuse/tests/test_models.py b/src/olympia/abuse/tests/test_models.py index 6ba246e5ed1d..9aa580db3004 100644 --- a/src/olympia/abuse/tests/test_models.py +++ b/src/olympia/abuse/tests/test_models.py @@ -2658,6 +2658,46 @@ def test_appeal_as_target_from_resolved_in_amo(self): addon = Addon.unfiltered.get() assert addon in Addon.unfiltered.get_queryset_for_pending_queues() + def test_appeal_as_target_on_content_review_job(self): + addon = addon_factory(status=amo.STATUS_REJECTED) + cinder_job = CinderJob.objects.create( + target_addon=addon, + decision=ContentDecision.objects.create( + cinder_id='4815162342-lost', + action_date=self.days_ago(179), + action=DECISION_ACTIONS.AMO_REJECT_LISTING_CONTENT, + addon=addon, + ), + content_review=True, + ) + appeal_response = responses.add( + responses.POST, + f'{settings.CINDER_SERVER_URL}v1/appeal', + json={'external_id': '2432615184-tsol'}, + status=201, + ) + + cinder_job.decision.appeal( + abuse_report=None, + appeal_text='appeal text', + user=user_factory(), + is_reporter=False, + ) + + 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 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 + ) + assert request_body['queue_slug'] == 'amo-env-listing-content' + def test_appeal_as_target_improperly_configured(self): addon = addon_factory() abuse_report = AbuseReport.objects.create( diff --git a/src/olympia/abuse/tests/test_views.py b/src/olympia/abuse/tests/test_views.py index 7dc2d26671af..5bb10b011398 100644 --- a/src/olympia/abuse/tests/test_views.py +++ b/src/olympia/abuse/tests/test_views.py @@ -1600,7 +1600,7 @@ def test_unknown_cinder_job_and_entity_prod(self): with override_settings(CINDER_UNIQUE_IDS=True): self._test_unkownn_cinder_job_and_entity(400) - def test_unkownn_cinder_job_but_known_entity(self): + def test_unknown_cinder_job_but_known_entity(self): assert not CinderJob.objects.exists() data = self.get_data() addon = addon_factory( @@ -1796,7 +1796,7 @@ def test_process_queue_move_called(self): assert response.status_code == 201 assert response.data == {'amo': {'received': True, 'handled': True}} - def _test_process_queue_move_no_cinder_report(self, status_code): + def _test_process_queue_move_no_cinder_job(self, status_code): req = self.get_request( data=self.get_data('job_actioned_move_to_dev_infringement.json') ) @@ -1812,20 +1812,69 @@ def _test_process_queue_move_no_cinder_report(self, status_code): } } - def test_process_queue_move_no_cinder_report_nonprod(self): + def test_process_queue_move_no_cinder_job_nonprod(self): with override_settings(CINDER_UNIQUE_IDS=False): - self._test_process_queue_move_no_cinder_report(200) + self._test_process_queue_move_no_cinder_job(200) - def test_process_queue_move_no_cinder_report_prod(self): + def test_process_queue_move_no_cinder_job_prod(self): with override_settings(CINDER_UNIQUE_IDS=True): - self._test_process_queue_move_no_cinder_report(400) + self._test_process_queue_move_no_cinder_job(400) - def test_process_queue_move_invalid_action(self): + def test_create_content_review_cinder_job_from_workflow(self): + data = self.get_data('job_actioned_workflow_created_job.json') + addon = addon_factory( + id=10000, + created=datetime.fromisoformat( + data['payload']['job']['entity']['attributes']['created'] + ), + ) + req = self.get_request(data=data) + assert not CinderJob.objects.exists() + + response = cinder_webhook(req) + assert (job := CinderJob.objects.get()) + assert job.job_id == '0aa2e6b6-9899-4bd0-9f59-94e8762861ba' + assert job.target_addon == addon + assert job.content_review is True + assert response.status_code == 201 + assert response.data == {'amo': {'received': True, 'handled': True}} + + def test_proccess_webhook_payload_job_actioned_wrong_workflow(self): + data = self.get_data('job_actioned_workflow_created_job.json') + data['payload']['action_made_by']['workflow']['name'] = 'someother workflow' + response = cinder_webhook(self.get_request(data=data)) + assert response.status_code == 200 + assert response.data == { + 'amo': { + 'received': True, + 'handled': False, + 'not_handled_reason': ( + 'Unsupported workflow (someother workflow) for job.actioned:created' + ), + } + } + + def test_process_webhook_payload_job_actioned_wrong_source(self): + data = self.get_data('job_actioned_workflow_created_job.json') + data['payload']['source'] = 'not-workflow' + response = cinder_webhook(self.get_request(data=data)) + assert response.status_code == 200 + assert response.data == { + 'amo': { + 'received': True, + 'handled': False, + 'not_handled_reason': ( + 'Unsupported source (not-workflow) for job.actioned:created' + ), + } + } + + def test_process_webhook_payload_job_actioned_invalid_action(self): data = self.get_data('job_actioned_move_to_dev_infringement.json') data['payload']['action'] = 'something_else' response = cinder_webhook(self.get_request(data=data)) - assert response.status_code == 400 + assert response.status_code == 200 assert response.data == { 'amo': { 'received': True, @@ -1836,7 +1885,7 @@ def test_process_queue_move_invalid_action(self): } } - def test_process_queue_move_not_addon(self): + def test_process_webhook_payload_job_actioned_not_addon(self): data = self.get_data('job_actioned_move_to_dev_infringement.json') data['payload']['job']['entity']['entity_schema'] = 'amo_user' diff --git a/src/olympia/abuse/views.py b/src/olympia/abuse/views.py index b90995bc6a93..aa63115c9743 100644 --- a/src/olympia/abuse/views.py +++ b/src/olympia/abuse/views.py @@ -32,7 +32,7 @@ from olympia.users.models import UserProfile from .actions import CONTENT_ACTION_FROM_DECISION_ACTION -from .cinder import CinderAddon +from .cinder import CinderAddon, CinderAddonContentReview from .forms import AbuseAppealEmailForm, AbuseAppealForm from .models import AbuseReport, CinderJob, ContentDecision from .serializers import ( @@ -228,6 +228,26 @@ def get_job_from_payload(payload): raise CinderWebhookError('Unsupported Manual decision') +def get_target_from_payload_entity(entity): + created = entity.get('attributes', {}).get('created') + target = get_instance_from_entity( + entity_schema=entity.get('entity_schema'), + entity_id=entity.get('attributes', {}).get('id'), + ) + # We trust Cinder is reporting on the correct instance by double-checking the + # created time of the instance is the same as the one in the payload. In production + # a wrong instance shouldn't happen, but in staging it could be the job was created + # from an instance from a different env. + if not target or not is_same_time(target, created): + log.debug( + 'Could not identify entity id %s with schema %s', + entity.get('attributes', {}).get('id'), + entity.get('entity_schema'), + ) + raise CinderWebhookMissingIdError('Unknown entity id received') + return target + + def process_webhook_payload_decision(payload): source = payload.get('source', {}) log.info('Valid Payload from AMO queue: %s', payload) @@ -243,26 +263,11 @@ def process_webhook_payload_decision(payload): # review), or otherwise - then we don't have a CinderJob instance. So identify the # target from the id (pk) and entity schema. cinder_job = get_job_from_payload(payload) - if cinder_job: - target = cinder_job.target - else: - entity = payload.get('entity', {}) - created = entity.get('attributes', {}).get('created') - target = get_instance_from_entity( - entity_schema=entity.get('entity_schema'), - entity_id=entity.get('attributes', {}).get('id'), - ) - # We trust Cinder is reporting a decision on the correct instance by - # double-checking the created time of the instance is the same as the one in the - # payload. In production a wrong instance shouldn't happen, but in staging it - # could be the job was created from an instance from a different env. - if not target or not is_same_time(target, created): - log.debug( - 'Could not identify entity id %s with schema %s', - entity.get('attributes', {}).get('id'), - entity.get('entity_schema'), - ) - raise CinderWebhookMissingIdError('Unknown entity id received') + target = ( + cinder_job.target + if cinder_job + else get_target_from_payload_entity(payload.get('entity', {})) + ) enforcement_actions = filter_enforcement_actions( payload.get('enforcement_actions') or [], @@ -297,27 +302,64 @@ def process_webhook_payload_decision(payload): def process_webhook_payload_job_actioned(payload): - if (action := payload.get('action')) != 'escalated': - log.debug('Cinder webhook action was invalid (not "escalated")') - raise CinderWebhookError(f'Unsupported action ({action}) for job.actioned') job = payload.get('job', {}) - entity = job.get('entity', {}).get('entity_schema') - if entity != CinderAddon.type: + entity_schema = job.get('entity', {}).get('entity_schema') + if entity_schema != CinderAddon.type: log.debug('Cinder webhook entity_schema was not %s', CinderAddon.type) raise CinderWebhookIgnoredError( - f'Unsupported entity_schema ({entity}) for job.actioned' + f'Unsupported entity_schema ({entity_schema}) for job.actioned' ) - job_id = job.get('id', '') - try: - cinder_job = CinderJob.objects.get(job_id=job_id) - except CinderJob.DoesNotExist as exc: - log.debug('CinderJob instance not found for job id %s', job_id) - raise CinderWebhookMissingIdError('No matching job id found') from exc - - new_queue = payload.get('job', {}).get('queue', {}).get('slug') - notes = payload.get('notes', '') - cinder_job.process_queue_move(new_queue=new_queue, notes=notes) + job_id = job.get('id', '') + match action := payload.get('action'): + case 'escalated': + log.info('Valid Payload from AMO queue: %s', payload) + try: + cinder_job = CinderJob.objects.get(job_id=job_id) + except CinderJob.DoesNotExist as exc: + log.debug('CinderJob instance not found for job id %s', job_id) + raise CinderWebhookMissingIdError('No matching job id found') from exc + + new_queue = payload.get('job', {}).get('queue', {}).get('slug') + notes = payload.get('notes', '') + cinder_job.process_queue_move(new_queue=new_queue, notes=notes) + + case 'created': + log.info('Valid Payload from AMO queue: %s', payload) + if (source := payload.get('source')) != 'workflow': + log.debug( + 'Cinder webhook job.actioned was created, but source is "%s", ' + 'not "workflow", so skipped.', + source, + ) + raise CinderWebhookIgnoredError( + f'Unsupported source ({source}) for job.actioned:created' + ) + elif ( + workflow_name := payload.get('action_made_by', {}) + .get('workflow', {}) + .get('name') + ) != CinderAddonContentReview.workflow_pretty_name: + log.debug( + 'Cinder webhook job.actioned was created, but workflow is "%s", ' + 'so skipped.', + workflow_name, + ) + raise CinderWebhookIgnoredError( + f'Unsupported workflow ({workflow_name}) for job.actioned:created' + ) + else: + payload.get('action_made_by', {}).get('workflow', {}).get('name') + target = get_target_from_payload_entity(job.get('entity', {})) + cinder_job, _ = CinderJob.objects.get_or_create( + job_id=job_id, + defaults={'target_addon': target, 'content_review': True}, + ) + case _: + log.debug('Cinder webhook action was invalid (was "%s")', action) + raise CinderWebhookIgnoredError( + f'Unsupported action ({action}) for job.actioned' + ) @api_view(['POST'])