Skip to content

Commit ed2eeb2

Browse files
authored
Add triggers for new addons and metadata change to submit to Cinder (#24662)
* Add triggers for new addons and metadata change to submit to Cinder * Handle updates and review requests for REJECTED state * add waffle switch
1 parent 40c8726 commit ed2eeb2

13 files changed

Lines changed: 464 additions & 57 deletions

File tree

src/olympia/abuse/cinder.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,21 @@ def get_context_generator(self):
797797
],
798798
}
799799

800+
def send_entity(self):
801+
"""Send this entity to the Cinder graph API."""
802+
generator = self.get_context_generator()
803+
context = next(generator, self.get_empty_context())
804+
data = {
805+
'entities': [self.get_entity_data(), *context.get('entities', [])],
806+
'relationships': context.get('relationships', []),
807+
}
808+
# Note: Cinder URLS are inconsistent. Per their documentation, that
809+
# one needs a trailing slash.
810+
url = f'{settings.CINDER_SERVER_URL}v1/graph/'
811+
response = requests.post(url, json=data, headers=self.get_cinder_http_headers())
812+
if response.status_code != 202:
813+
raise HTTPError(response.content)
814+
800815
def report(self, *args, **kwargs):
801816
# It doesn't make sense to report this, it's a holder for metadata, not a real
802817
# entity.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Generated by Django 5.2.12 on 2026-03-27 10:56
2+
3+
from django.db import migrations
4+
5+
from olympia.core.db.migrations import CreateWaffleSwitch
6+
7+
8+
class Migration(migrations.Migration):
9+
10+
dependencies = [
11+
('abuse', '0062_rename_created_idx_abuse_reports_created_idx'),
12+
]
13+
14+
operations = [
15+
CreateWaffleSwitch('content-review-in-cinder'),
16+
]

src/olympia/abuse/tasks.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import olympia.core.logger
1313
from olympia import amo
1414
from olympia.activity.models import ActivityLog
15-
from olympia.addons.models import Addon
15+
from olympia.addons.models import Addon, AddonApprovalsCounter
1616
from olympia.amo.celery import task
1717
from olympia.amo.decorators import use_primary_db
1818
from olympia.amo.utils import to_language
@@ -287,6 +287,9 @@ def submit_addon_for_content_review(*, addon_pk):
287287
addon = Addon.unfiltered.get(pk=addon_pk)
288288
entity_helper = CinderAddonContentReview(addon)
289289
entity_helper.send_event()
290+
AddonApprovalsCounter.reset_content_for_addon(
291+
addon, not_reviewed_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PENDING
292+
)
290293

291294
# Additional context is submitted as separate api calls.
292295
try:
@@ -300,14 +303,29 @@ def submit_addon_for_content_review(*, addon_pk):
300303
@use_primary_db
301304
def submit_addon_change_for_content_review(*, activity_log_pk):
302305
activity = ActivityLog.objects.get(pk=activity_log_pk)
303-
if activity.action != amo.LOG.EDIT_ADDON_PROPERTY.id:
306+
if activity.action == amo.LOG.EDIT_ADDON_PROPERTY.id:
307+
addon, field, changes_blob = activity.arguments
308+
changes = json.loads(changes_blob or '{}')
309+
elif activity.action == amo.LOG.REJECTED_LISTING_REVIEW_REQUEST.id:
310+
(addon,) = activity.arguments
311+
field = 'Listing Review Requested'
312+
changes = {}
313+
else:
304314
log.error(
305315
'Activity with id %s is not an EDIT_ADDON_PROPERTY, cannot submit for '
306316
'content review.',
307317
activity_log_pk,
308318
)
309319
return
310-
addon, field, changes_blob = activity.arguments
311-
changes = json.loads(changes_blob or '{}')
320+
312321
entity_helper = CinderContentChange(addon, field, changes)
313-
entity_helper.send_event()
322+
if (
323+
activity.action == amo.LOG.EDIT_ADDON_PROPERTY.id
324+
and addon.status == amo.STATUS_REJECTED
325+
):
326+
# if the add-on is rejected, the developer needs to request a new review first
327+
# before we review it again, so we don't sent an event, which would create a
328+
# job. We just send the content change entity.
329+
entity_helper.send_entity()
330+
else:
331+
entity_helper.send_event()

src/olympia/abuse/tests/test_cinder.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,6 +2114,40 @@ def test_send_event(self):
21142114
with self.assertRaises(requests.HTTPError):
21152115
cinder_change.send_event()
21162116

2117+
def test_send_entity(self):
2118+
responses.add(
2119+
responses.POST,
2120+
f'{settings.CINDER_SERVER_URL}v1/graph/',
2121+
status=202,
2122+
)
2123+
2124+
instance = self.CinderClass(self._create_dummy_target())
2125+
instance.send_entity()
2126+
2127+
assert len(responses.calls) == 1
2128+
data = json.loads(responses.calls[0].request.body)
2129+
assert data == {
2130+
'entities': [
2131+
{
2132+
'entity_type': 'amo_content_change',
2133+
'attributes': instance.get_attributes(),
2134+
},
2135+
{
2136+
'entity_type': 'amo_addon',
2137+
'attributes': CinderAddon(instance.entity).get_attributes(),
2138+
},
2139+
],
2140+
'relationships': [
2141+
{
2142+
'source_id': instance.id,
2143+
'source_type': 'amo_content_change',
2144+
'target_id': str(instance.entity.pk),
2145+
'target_type': 'amo_addon',
2146+
'relationship_type': 'amo_content_metadata_change_of',
2147+
}
2148+
],
2149+
}
2150+
21172151
def test_build_report_payload(self):
21182152
# report isn't implemented, so this won't be called
21192153
pass

src/olympia/abuse/tests/test_tasks.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from olympia import amo
1515
from olympia.activity.models import ActivityLog
16+
from olympia.addons.models import AddonApprovalsCounter
1617
from olympia.amo.tests import TestCase, addon_factory, days_ago, user_factory
1718
from olympia.constants.abuse import (
1819
DECISION_ACTIONS,
@@ -1336,6 +1337,11 @@ def test_submit_addon_for_content_review():
13361337
additional_call = json.loads(responses.calls[1].request.body)
13371338
assert additional_call['entities'][0]['attributes']['id'] == str(author2.id)
13381339

1340+
assert (
1341+
AddonApprovalsCounter.objects.get(addon=addon).content_review_status
1342+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PENDING
1343+
)
1344+
13391345

13401346
@pytest.mark.django_db
13411347
def test_submit_addon_change_for_content_review():
@@ -1364,3 +1370,83 @@ def test_submit_addon_change_for_content_review():
13641370
assert event_call['entity']['attributes']['values_added'] == ['a new name']
13651371
assert event_call['entity']['attributes']['values_removed'] == ['an old name']
13661372
assert event_call['subgraph']['entities'][0]['attributes']['id'] == str(addon.id)
1373+
1374+
1375+
@pytest.mark.django_db
1376+
def test_submit_addon_change_for_content_review_while_rejected():
1377+
addon = addon_factory(status=amo.STATUS_REJECTED)
1378+
activity = ActivityLog.objects.create(
1379+
amo.LOG.EDIT_ADDON_PROPERTY,
1380+
addon,
1381+
'name',
1382+
'{"added": ["a new name"], "removed": ["an old name"]}',
1383+
user=user_factory(),
1384+
)
1385+
responses.add(
1386+
responses.POST,
1387+
f'{settings.CINDER_SERVER_URL}v1/graph/',
1388+
status=202,
1389+
)
1390+
1391+
with time_machine.travel(datetime.now(), tick=False):
1392+
now = datetime.now()
1393+
timestamp = int(now.timestamp())
1394+
submit_addon_change_for_content_review.delay(activity_log_pk=activity.id)
1395+
1396+
assert len(responses.calls) == 1
1397+
graph_call = json.loads(responses.calls[0].request.body)
1398+
assert graph_call == {
1399+
'entities': [
1400+
{
1401+
'entity_type': 'amo_content_change',
1402+
'attributes': {
1403+
'datetime': str(now),
1404+
'id': f'{addon.id}-name-{timestamp}',
1405+
'reason': 'Addon name change',
1406+
'values_added': ['a new name'],
1407+
'values_removed': ['an old name'],
1408+
},
1409+
},
1410+
{
1411+
'entity_type': 'amo_addon',
1412+
'attributes': CinderAddon(addon).get_attributes(),
1413+
},
1414+
],
1415+
'relationships': [
1416+
{
1417+
'relationship_type': 'amo_content_metadata_change_of',
1418+
'source_id': f'{addon.id}-name-{timestamp}',
1419+
'source_type': 'amo_content_change',
1420+
'target_id': str(addon.id),
1421+
'target_type': 'amo_addon',
1422+
}
1423+
],
1424+
}
1425+
1426+
1427+
@pytest.mark.django_db
1428+
def test_submit_addon_change_for_content_review_with_request_review():
1429+
addon = addon_factory()
1430+
activity = ActivityLog.objects.create(
1431+
amo.LOG.REJECTED_LISTING_REVIEW_REQUEST, addon, user=user_factory()
1432+
)
1433+
responses.add(
1434+
responses.POST,
1435+
f'{settings.CINDER_SERVER_URL}v2/workflows/event',
1436+
json={'status': 'ok', 'event_id': '1234-xyz'},
1437+
status=200,
1438+
)
1439+
1440+
submit_addon_change_for_content_review.delay(activity_log_pk=activity.id)
1441+
1442+
assert len(responses.calls) == 1
1443+
event_call = json.loads(responses.calls[0].request.body)
1444+
assert event_call['event_name'] == CinderAddonContentReview.workflow_name
1445+
assert event_call['entity']['entity_schema'] == 'amo_content_change'
1446+
assert (
1447+
event_call['entity']['attributes']['reason']
1448+
== 'Addon Listing Review Requested change'
1449+
)
1450+
assert event_call['entity']['attributes']['values_added'] == []
1451+
assert event_call['entity']['attributes']['values_removed'] == []
1452+
assert event_call['subgraph']['entities'][0]['attributes']['id'] == str(addon.id)

src/olympia/addons/models.py

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ def not_disabled_by_mozilla(self):
262262

263263
def get_base_queryset_for_queue(
264264
self,
265+
*,
265266
admin_reviewer=False,
266-
content_review=False,
267267
theme_review=False,
268268
select_related_fields_for_listed=True,
269269
):
@@ -392,9 +392,7 @@ def get_queryset_for_pending_queues(
392392
def get_content_review_queue(self, admin_reviewer=False):
393393
"""Return a queryset of Addon objects that need content review."""
394394
qs = (
395-
self.get_base_queryset_for_queue(
396-
admin_reviewer=admin_reviewer, content_review=True
397-
)
395+
self.get_base_queryset_for_queue(admin_reviewer=admin_reviewer)
398396
.valid()
399397
.filter(
400398
_current_version__reviewerflags__pending_rejection__isnull=True,
@@ -2108,6 +2106,8 @@ def watch_status(old_attr=None, new_attr=None, instance=None, sender=None, **kwa
21082106
If a version is rejected after nomination, the developer has to upload a
21092107
new version.
21102108
"""
2109+
from olympia.abuse.tasks import submit_addon_for_content_review
2110+
21112111
new_status = new_attr.get('status') if new_attr else None
21122112
old_status = old_attr.get('status') if old_attr else None
21132113
disabled_by_user = new_attr.get('disabled_by_user') if new_attr else None
@@ -2131,7 +2131,20 @@ def watch_status(old_attr=None, new_attr=None, instance=None, sender=None, **kwa
21312131
status_disabled_reason=File.STATUS_DISABLED_REASONS.DEVELOPER,
21322132
)
21332133
instance.update_status()
2134-
elif old_status == amo.STATUS_NOMINATED:
2134+
return
2135+
2136+
if (
2137+
new_status in amo.VALID_ADDON_STATUSES
2138+
and waffle.switch_is_active('content-review-in-cinder')
2139+
and not AddonApprovalsCounter.objects.filter(addon_id=instance.id)
2140+
.exclude(
2141+
content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.UNREVIEWED
2142+
)
2143+
.exists()
2144+
):
2145+
submit_addon_for_content_review.delay(addon_pk=instance.pk)
2146+
2147+
if old_status == amo.STATUS_NOMINATED:
21352148
# Update latest version due date if necessary for nominated add-ons.
21362149
inherit_due_date_if_nominated(None, latest_version)
21372150
else:
@@ -2427,11 +2440,10 @@ class CONTENT_REVIEW_STATUSES(EnumChoices):
24272440
PASS = 2, 'Pass'
24282441
FAIL = 3, 'Fail'
24292442
REQUESTED = 4, 'Pending, New review requested'
2443+
# This status is set when the review is sent to Cinder
2444+
PENDING = 5, 'Pending first review after submission'
24302445

24312446
CONTENT_REVIEW_STATUSES.add_subset('REJECTED', ('FAIL', 'REQUESTED'))
2432-
CONTENT_REVIEW_STATUSES.add_subset(
2433-
'PENDING', ('UNREVIEWED', 'CHANGED', 'REQUESTED')
2434-
)
24352447
CONTENT_REVIEW_STATUSES.add_subset('COMPLETE', ('PASS', 'FAIL'))
24362448

24372449
addon = models.OneToOneField(Addon, primary_key=True, on_delete=models.CASCADE)
@@ -2516,19 +2528,30 @@ def reject_content_for_addon(cls, addon):
25162528
return obj
25172529

25182530
@classmethod
2519-
def reset_content_for_addon(cls, addon):
2531+
def reset_content_for_addon(
2532+
cls, addon, *, not_reviewed_status=CONTENT_REVIEW_STATUSES.UNREVIEWED
2533+
):
25202534
"""
25212535
Reset the last_content_review date for this addon so it triggers
25222536
another review.
25232537
"""
2538+
defaults = {'last_content_review': None}
25242539
obj, created = cls.objects.update_or_create(
2525-
addon=addon, defaults={'last_content_review': None}
2540+
addon=addon,
2541+
defaults=defaults,
2542+
create_defaults={**defaults, 'content_review_status': not_reviewed_status},
25262543
)
2527-
if (
2528-
not created
2529-
and obj.content_review_status == cls.CONTENT_REVIEW_STATUSES.PASS
2530-
):
2531-
obj.update(content_review_status=cls.CONTENT_REVIEW_STATUSES.CHANGED)
2544+
if not created:
2545+
new_status = (
2546+
cls.CONTENT_REVIEW_STATUSES.CHANGED
2547+
if obj.content_review_status == cls.CONTENT_REVIEW_STATUSES.PASS
2548+
else not_reviewed_status
2549+
if obj.content_review_status == cls.CONTENT_REVIEW_STATUSES.UNREVIEWED
2550+
else None
2551+
)
2552+
# i.e. not None or cls.CONTENT_REVIEW_STATUSES.UNREVIEWED, which is 0
2553+
if new_status:
2554+
obj.update(content_review_status=new_status)
25322555
assert obj.last_content_review is None
25332556
return obj
25342557

src/olympia/addons/serializers.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,8 @@ def _save_icon(self, uploaded_icon):
13191319
self.instance.update(icon_type='')
13201320

13211321
def log(self, instance, validated_data, changes):
1322+
from olympia.abuse.tasks import submit_addon_change_for_content_review
1323+
13221324
validated_data = {**validated_data} # we want to modify it, so take a copy
13231325
user = self.context['request'].user
13241326

@@ -1340,13 +1342,15 @@ def log(self, instance, validated_data, changes):
13401342
validated_data.pop('version')
13411343

13421344
for field, addedremoved in changes.items():
1343-
ActivityLog.objects.create(
1345+
alog = ActivityLog.objects.create(
13441346
amo.LOG.EDIT_ADDON_PROPERTY,
13451347
instance,
13461348
field,
13471349
json.dumps(addedremoved),
13481350
user=user,
13491351
)
1352+
if waffle.switch_is_active('content-review-in-cinder'):
1353+
submit_addon_change_for_content_review.delay(activity_log_pk=alog.pk)
13501354
validated_data.pop(field)
13511355
if validated_data:
13521356
ActivityLog.objects.create(

0 commit comments

Comments
 (0)