Add triggers for new addons and metadata change to submit to Cinder

Author: eviljeff

src/olympia/abuse/tasks.py

@@ -12,7 +12,7 @@
 import olympia.core.logger
 from olympia import amo
 from olympia.activity.models import ActivityLog
-from olympia.addons.models import Addon
+from olympia.addons.models import Addon, AddonApprovalsCounter
 from olympia.amo.celery import task
 from olympia.amo.decorators import use_primary_db
 from olympia.amo.utils import to_language
@@ -287,6 +287,9 @@ def submit_addon_for_content_review(*, addon_pk):
     addon = Addon.unfiltered.get(pk=addon_pk)
     entity_helper = CinderAddonContentReview(addon)
     entity_helper.send_event()
+    AddonApprovalsCounter.reset_content_for_addon(
+        addon, initial_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PENDING
+    )
 
     # Additional context is submitted as separate api calls.
     try:

src/olympia/abuse/tests/test_tasks.py

@@ -13,6 +13,7 @@
 
 from olympia import amo
 from olympia.activity.models import ActivityLog
+from olympia.addons.models import AddonApprovalsCounter
 from olympia.amo.tests import TestCase, addon_factory, days_ago, user_factory
 from olympia.constants.abuse import (
     DECISION_ACTIONS,
@@ -1336,6 +1337,11 @@ def test_submit_addon_for_content_review():
     additional_call = json.loads(responses.calls[1].request.body)
     assert additional_call['entities'][0]['attributes']['id'] == str(author2.id)
 
+    assert (
+        AddonApprovalsCounter.objects.get(addon=addon).content_review_status
+        == AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PENDING
+    )
+
 
 @pytest.mark.django_db
 def test_submit_addon_change_for_content_review():

src/olympia/addons/models.py

@@ -262,8 +262,8 @@ def not_disabled_by_mozilla(self):
 
     def get_base_queryset_for_queue(
         self,
+        *,
         admin_reviewer=False,
-        content_review=False,
         theme_review=False,
         select_related_fields_for_listed=True,
     ):
@@ -392,9 +392,7 @@ def get_queryset_for_pending_queues(
     def get_content_review_queue(self, admin_reviewer=False):
         """Return a queryset of Addon objects that need content review."""
         qs = (
-            self.get_base_queryset_for_queue(
-                admin_reviewer=admin_reviewer, content_review=True
-            )
+            self.get_base_queryset_for_queue(admin_reviewer=admin_reviewer)
             .valid()
             .filter(
                 _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
     If a version is rejected after nomination, the developer has to upload a
     new version.
     """
+    from olympia.abuse.tasks import submit_addon_for_content_review
+
     new_status = new_attr.get('status') if new_attr else None
     old_status = old_attr.get('status') if old_attr else None
     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
             status_disabled_reason=File.STATUS_DISABLED_REASONS.DEVELOPER,
         )
         instance.update_status()
-    elif old_status == amo.STATUS_NOMINATED:
+        return
+
+    if (
+        new_status in amo.VALID_ADDON_STATUSES
+        and waffle.switch_is_active('content-review-in-cinder')
+        and not AddonApprovalsCounter.objects.filter(addon_id=instance.id)
+        .exclude(
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.UNREVIEWED
+        )
+        .exists()
+    ):
+        submit_addon_for_content_review.delay(addon_pk=instance.pk)
+
+    if old_status == amo.STATUS_NOMINATED:
         # Update latest version due date if necessary for nominated add-ons.
         inherit_due_date_if_nominated(None, latest_version)
     else:
@@ -2427,11 +2440,10 @@ class CONTENT_REVIEW_STATUSES(EnumChoices):
         PASS = 2, 'Pass'
         FAIL = 3, 'Fail'
         REQUESTED = 4, 'Pending, New review requested'
+        # This status is set when the review is sent to Cinder
+        PENDING = 5, 'Pending first review after submission'
 
     CONTENT_REVIEW_STATUSES.add_subset('REJECTED', ('FAIL', 'REQUESTED'))
-    CONTENT_REVIEW_STATUSES.add_subset(
-        'PENDING', ('UNREVIEWED', 'CHANGED', 'REQUESTED')
-    )
     CONTENT_REVIEW_STATUSES.add_subset('COMPLETE', ('PASS', 'FAIL'))
 
     addon = models.OneToOneField(Addon, primary_key=True, on_delete=models.CASCADE)
@@ -2516,13 +2528,18 @@ def reject_content_for_addon(cls, addon):
         return obj
 
     @classmethod
-    def reset_content_for_addon(cls, addon):
+    def reset_content_for_addon(
+        cls, addon, initial_status=CONTENT_REVIEW_STATUSES.UNREVIEWED
+    ):
         """
         Reset the last_content_review date for this addon so it triggers
         another review.
         """
+        defaults = {'last_content_review': None}
         obj, created = cls.objects.update_or_create(
-            addon=addon, defaults={'last_content_review': None}
+            addon=addon,
+            defaults=defaults,
+            create_defaults={**defaults, 'content_review_status': initial_status},
         )
         if (
             not created

src/olympia/addons/serializers.py

@@ -1319,6 +1319,8 @@ def _save_icon(self, uploaded_icon):
             self.instance.update(icon_type='')
 
     def log(self, instance, validated_data, changes):
+        from olympia.abuse.tasks import submit_addon_change_for_content_review
+
         validated_data = {**validated_data}  # we want to modify it, so take a copy
         user = self.context['request'].user
 
@@ -1340,13 +1342,15 @@ def log(self, instance, validated_data, changes):
             validated_data.pop('version')
 
         for field, addedremoved in changes.items():
-            ActivityLog.objects.create(
+            alog = ActivityLog.objects.create(
                 amo.LOG.EDIT_ADDON_PROPERTY,
                 instance,
                 field,
                 json.dumps(addedremoved),
                 user=user,
             )
+            if waffle.switch_is_active('content-review-in-cinder'):
+                submit_addon_change_for_content_review.delay(activity_log_pk=alog.pk)
             validated_data.pop(field)
         if validated_data:
             ActivityLog.objects.create(

src/olympia/addons/tests/test_models.py

@@ -2964,6 +2964,56 @@ def test_rejected_listing_addons_do_not_update(self):
         addon.update_status()
         assert addon.status == amo.STATUS_APPROVED
 
+    @patch('olympia.abuse.tasks.submit_addon_for_content_review.delay')
+    @override_switch('content-review-in-cinder', active=True)
+    def test_content_review_triggered_for_addon_submission(self, submit_mock):
+        addon = addon_factory()
+        submit_mock.reset_mock()
+        addon.update(status=amo.STATUS_NOMINATED)
+        submit_mock.assert_called_once_with(addon_pk=addon.pk)
+
+    @patch('olympia.abuse.tasks.submit_addon_for_content_review.delay')
+    @override_switch('content-review-in-cinder', active=False)
+    def test_content_review_not_triggered_when_waffle_not_enabled(self, submit_mock):
+        addon = addon_factory()
+        submit_mock.reset_mock()
+        addon.update(status=amo.STATUS_NOMINATED)
+        submit_mock.assert_not_called()
+
+    @patch('olympia.abuse.tasks.submit_addon_for_content_review.delay')
+    @override_switch('content-review-in-cinder', active=True)
+    def test_content_review_not_triggered_for_non_valid_status(self, submit_mock):
+        addon = addon_factory()
+        submit_mock.reset_mock()
+        addon.update(status=amo.STATUS_DISABLED)
+        submit_mock.assert_not_called()
+
+    @patch('olympia.abuse.tasks.submit_addon_for_content_review.delay')
+    @override_switch('content-review-in-cinder', active=True)
+    def test_content_review_not_triggered_when_already_pending(self, submit_mock):
+        addon = addon_factory()
+        AddonApprovalsCounter.objects.create(
+            addon=addon,
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PENDING,
+        )
+        submit_mock.reset_mock()
+        addon.update(status=amo.STATUS_NOMINATED)
+        submit_mock.assert_not_called()
+
+    @patch('olympia.abuse.tasks.submit_addon_for_content_review.delay')
+    @override_switch('content-review-in-cinder', active=True)
+    def test_content_review_not_triggered_when_content_already_reviewed(
+        self, submit_mock
+    ):
+        addon = addon_factory()
+        AddonApprovalsCounter.objects.create(
+            addon=addon,
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS,
+        )
+        submit_mock.reset_mock()
+        addon.update(status=amo.STATUS_NOMINATED)
+        submit_mock.assert_not_called()
+
 
 class TestGetVersion(TestCase):
     fixtures = [
@@ -3750,6 +3800,27 @@ def test_reset_content_existing(self):
             AddonApprovalsCounter.reset_content_for_addon(self.addon)
             assert approval_counter.reload().content_review_status == previous_status
 
+    def test_reset_content_existing_initial_status_ignored(self):
+        approval_counter = AddonApprovalsCounter.objects.create(
+            addon=self.addon,
+            counter=42,
+            last_content_review=self.days_ago(367),
+            last_human_review=self.days_ago(10),
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS,
+        )
+        # should be ignored
+        AddonApprovalsCounter.reset_content_for_addon(
+            self.addon,
+            initial_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REQUESTED,
+        )
+        assert approval_counter.reload().counter == 42  # unchanged
+        self.assertCloseToNow(approval_counter.last_human_review, now=self.days_ago(10))
+        assert approval_counter.last_content_review is None
+        assert (
+            approval_counter.content_review_status
+            == AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.CHANGED
+        )
+
     def test_reset_content_non_existing(self):
         assert not AddonApprovalsCounter.objects.filter(addon=self.addon).exists()
         AddonApprovalsCounter.reset_content_for_addon(self.addon)
@@ -3761,6 +3832,20 @@ def test_reset_content_non_existing(self):
             == AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.UNREVIEWED
         )
 
+    def test_reset_content_non_existing_with_initial_status(self):
+        assert not AddonApprovalsCounter.objects.filter(addon=self.addon).exists()
+        AddonApprovalsCounter.reset_content_for_addon(
+            self.addon,
+            initial_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PENDING,
+        )
+        approval_counter = AddonApprovalsCounter.objects.get(addon=self.addon)
+        assert approval_counter.counter == 0
+        assert approval_counter.last_human_review is None
+        assert (
+            approval_counter.content_review_status
+            == AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PENDING
+        )
+
     def test_request_review(self):
         AddonApprovalsCounter.request_new_content_review_for_addon(self.addon)
         approval_counter = AddonApprovalsCounter.objects.get(addon=self.addon)