more model changes...

Author: eviljeff

src/olympia/abuse/actions.py

@@ -699,14 +699,7 @@ def should_hold_action(self):
     def process_action(self, release_hold=False):
         if self.target.status not in (amo.STATUS_DISABLED, amo.STATUS_REJECTED):
             self.target.update(status=amo.STATUS_REJECTED)
-            AddonApprovalsCounter.objects.update_or_create(
-                addon=self.target,
-                defaults={
-                    'last_content_review_status': (
-                        AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL
-                    )
-                },
-            )
+            AddonApprovalsCounter.reject_content_for_addon(self.target)
         return self.log_action(amo.LOG.REJECT_LISTING_CONTENT)
 
     def hold_action(self):
@@ -890,16 +883,8 @@ def process_action(self, release_hold=False):
                 and target.status == amo.STATUS_REJECTED
             ):
                 target_versions = target_versions.none()
-                AddonApprovalsCounter.objects.update_or_create(
-                    addon=target,
-                    defaults={
-                        'last_content_review': datetime.now(),
-                        'last_content_review_status': (
-                            AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
-                        ),
-                    },
-                )
-                # Call the function to correct it the status
+                AddonApprovalsCounter.approve_content_for_addon(target)
+                # Call update function to correct ihe status
                 target.update_status()
                 activity_log_action = amo.LOG.APPROVE_REJECTED_LISTING_CONTENT
 
@@ -973,15 +958,7 @@ def get_owners(self):
 
     def process_action(self, release_hold=False):
         if isinstance(self.target, Addon):
-            AddonApprovalsCounter.objects.update_or_create(
-                addon=self.target,
-                defaults={
-                    'last_content_review': datetime.now(),
-                    'last_content_review_status': (
-                        AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
-                    ),
-                },
-            )
+            AddonApprovalsCounter.approve_content_for_addon(self.target)
             if self.status == amo.STATUS_REJECTED:
                 self.decision.metadata['previous_status'] = self.status
                 self.decision.save(update_fields=['metadata'])

src/olympia/abuse/tests/test_actions.py

@@ -787,7 +787,7 @@ def _test_reporter_content_approved_action_taken(self):
 
         counter = AddonApprovalsCounter.objects.get(addon=self.addon)
         assert (
-            counter.last_content_review_status
+            counter.content_review_status
             == AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
         )
         self.assertCloseToNow(counter.last_content_review)
@@ -802,7 +802,7 @@ def _test_reporter_content_approved_action_taken(self):
     def test_content_approve_rejected_listing_content(self):
         AddonApprovalsCounter.objects.create(
             addon=self.addon,
-            last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REQUESTED,
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REQUESTED,
         )
         self.addon.update(status=amo.STATUS_REJECTED)
         action = DECISION_ACTIONS.AMO_APPROVE
@@ -833,7 +833,7 @@ def test_content_approve_rejected_listing_content(self):
 
         counter = AddonApprovalsCounter.objects.get(addon=self.addon)
         assert (
-            counter.last_content_review_status
+            counter.content_review_status
             == AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
         )
         self.assertCloseToNow(counter.last_content_review)
@@ -1173,7 +1173,7 @@ def _test_approve_appeal_or_override_but_listing_rejected(self, ContentActionCla
     def test_approve_appeal_success_but_listing_rejected(self):
         AddonApprovalsCounter.objects.create(
             addon=self.addon,
-            last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
         )
         self.past_negative_decision.update(appeal_job=self.cinder_job)
         self._test_approve_appeal_or_override_but_listing_rejected(
@@ -1185,7 +1185,7 @@ def test_approve_appeal_success_but_listing_rejected(self):
     def test_approve_override_success_but_listing_rejected(self):
         AddonApprovalsCounter.objects.create(
             addon=self.addon,
-            last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
         )
         self.decision.update(override_of=self.past_negative_decision)
         self._test_approve_appeal_or_override_but_listing_rejected(
@@ -1315,7 +1315,7 @@ def test_approve_appeal_success_but_listing_rejected(self):
         self.addon.update(status=amo.STATUS_REJECTED)
         AddonApprovalsCounter.objects.create(
             addon=self.addon,
-            last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
         )
         self.past_negative_decision.update(appeal_job=self.cinder_job)
         self._test_approve_appeal_or_override(ContentActionTargetAppealApprove)
@@ -1326,7 +1326,7 @@ def test_approve_override_success_but_listing_rejected(self):
         self.addon.update(status=amo.STATUS_REJECTED)
         AddonApprovalsCounter.objects.create(
             addon=self.addon,
-            last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
+            content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
         )
         self.decision.update(override_of=self.past_negative_decision)
         self._test_approve_appeal_or_override(ContentActionOverrideApprove)

…alscounter_last_content_review_status.py …pprovalscounter_content_review_status.pysrc/olympia/addons/migrations/0059_add_addonapprovalscounter_last_content_review_status.py renamed to src/olympia/addons/migrations/0059_add_addonapprovalscounter_content_review_status.py src/olympia/addons/migrations/0059_add_addonapprovalscounter_last_content_review_status.py renamed to src/olympia/addons/migrations/0059_add_addonapprovalscounter_content_review_status.py

@@ -12,7 +12,7 @@ class Migration(migrations.Migration):
     operations = [
         migrations.AddField(
             model_name='addonapprovalscounter',
-            name='last_content_review_status',
-            field=models.SmallIntegerField(choices=[(0, 'Unreviewed'), (1, 'Pass'), (2, 'Fail'), (3, 'New review requested')], db_index=True, null=True),
+            name='content_review_status',
+            field=models.SmallIntegerField(choices=[(0, 'Unreviewed'), (1, 'Pending, accepted content changed'), (2, 'Pass'), (3, 'Fail'), (4, 'Pending, New review requested')], db_index=True, null=True),
         ),
     ]

src/olympia/addons/migrations/0060_backfill_last_content_review_status.py

@@ -1,33 +1,37 @@
 # Generated by Django 4.2.28 on 2026-02-05 11:42
 
-from django.db import migrations
+from django.db import models, migrations
 
 
-def backfill_last_content_review_status(apps, schema_editor):
+def backfill_content_review_status(apps, schema_editor):
     AddonApprovalsCounter = apps.get_model('addons', 'AddonApprovalsCounter')
-    for counter in AddonApprovalsCounter.objects.filter(last_content_review_status__isnull=True):
+    for counter in AddonApprovalsCounter.objects.filter(content_review_status__isnull=True):
         # if we have a last_content_review date then we know it passed, and hasn't changed since
         if counter.last_content_review is not None:
-            counter.last_content_review_status = 1  # CONTENT_REVIEW_STATUSES.PASS
+            counter.content_review_status = 2  # CONTENT_REVIEW_STATUSES.PASS
+        # If last_content_review_pass is False we know it failed.
+        elif counter.last_content_review_pass is False:
+            counter.content_review_status = 3  # CONTENT_REVIEW_STATUSES.FAIL
         # Otherwise we don't know for sure
         # - might never have been reviewed (e.g. no listed versions).
         # - might have been reviewed and failed.
         # - might have been reviewed but then the content changed.
-        # If last_content_review_pass is False we know it failed though.
-        elif counter.last_content_review_pass is False:
-            counter.last_content_review_status = 2  # CONTENT_REVIEW_STATUSES.FAIL
         else:
-            # don't need to save if there wasn't an update.
-            continue
-        counter.save(update_fields=['last_content_review_status'])
+            counter.content_review_status = 0  # CONTENT_REVIEW_STATUSES.UNREVIEWED
+        counter.save(update_fields=['content_review_status'])
 
 
 class Migration(migrations.Migration):
 
     dependencies = [
-        ('addons', '0059_add_addonapprovalscounter_last_content_review_status'),
+        ('addons', '0059_add_addonapprovalscounter_content_review_status'),
     ]
 
     operations = [
-        migrations.RunPython(backfill_last_content_review_status, reverse_code=migrations.RunPython.noop),
+        migrations.RunPython(backfill_content_review_status, reverse_code=migrations.RunPython.noop),
+        migrations.AlterField(
+            model_name='addonapprovalscounter',
+            name='content_review_status',
+            field=models.SmallIntegerField(choices=[(0, 'Unreviewed'), (1, 'Pending, accepted content changed'), (2, 'Pass'), (3, 'Fail'), (4, 'Pending, New review requested')], db_index=True, default=0),
+        ),
     ]

src/olympia/addons/models.py

@@ -398,12 +398,14 @@ def get_content_review_queue(self, admin_reviewer=False):
             .valid()
             .filter(
                 _current_version__reviewerflags__pending_rejection__isnull=True,
-                addonapprovalscounter__last_content_review=None,
                 # Only content review extensions and dictionaries. See
                 # https://github.com/mozilla/addons-server/issues/11796 &
                 # https://github.com/mozilla/addons-server/issues/12065
                 type__in=(amo.ADDON_EXTENSION, amo.ADDON_DICT),
             )
+            .exclude(
+                addonapprovalscounter__content_review_status__in=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.COMPLETE
+            )
             .order_by('created')
         )
         return qs
@@ -1300,7 +1302,7 @@ def update_status(self, ignore_version=None):
         force_update = False
         if AddonApprovalsCounter.objects.filter(
             addon=self,
-            last_content_review_status__in=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REJECTED,
+            content_review_status__in=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REJECTED,
         ).exists():
             # If the content review failed, the status should stay as rejected
             correct_status = amo.STATUS_REJECTED
@@ -2410,27 +2412,30 @@ class AddonApprovalsCounter(ModelBase):
       add-on
     - last_content_review, the date of the last time a human fully reviewed the
       add-on content (not code).
-    - last_content_review_status, the status of the last content review.
+    - content_review_status, the status of the last content review.
     """
 
     class CONTENT_REVIEW_STATUSES(EnumChoices):
-        AWAITING = 0, 'Content awaiting review'
-        PASS = 1, 'Pass'
-        FAIL = 2, 'Fail'
-        REQUESTED = 3, 'New review requested'
-
-        __empty__ = 'Unknwon'  # i.e. null
+        UNREVIEWED = 0, 'Unreviewed'
+        CHANGED = 1, 'Pending, accepted content changed'
+        PASS = 2, 'Pass'
+        FAIL = 3, 'Fail'
+        REQUESTED = 4, 'Pending, New review requested'
 
     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)
     counter = models.PositiveIntegerField(default=0)
     last_human_review = models.DateTimeField(null=True)
     last_content_review = models.DateTimeField(null=True, db_index=True)
-    last_content_review_status = models.SmallIntegerField(
+    content_review_status = models.SmallIntegerField(
         choices=CONTENT_REVIEW_STATUSES.choices,
-        null=True,
         db_index=True,
+        default=CONTENT_REVIEW_STATUSES.UNREVIEWED,
     )
 
     def __str__(self):
@@ -2449,6 +2454,7 @@ def increment_for_addon(cls, addon):
             'counter': 1,
             'last_human_review': now,
             'last_content_review': now,
+            'content_review_status': cls.CONTENT_REVIEW_STATUSES.PASS,
         }
         # TODO: rewrite this using update_or_create when we're on django5.2
         # - it supports seperate create and update defaults.
@@ -2469,29 +2475,67 @@ def reset_for_addon(cls, addon):
     @classmethod
     def approve_content_for_addon(cls, addon):
         """
-        Set last_content_review and last_content_review_status to
+        Set last_content_review and content_review_status to
         CONTENT_REVIEW_STATUSES.PASS for this addon.
         """
         obj, _ = cls.objects.update_or_create(
             addon=addon,
             defaults={
                 'last_content_review': datetime.now(),
-                'last_content_review_status': (
+                'content_review_status': (
                     AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
                 ),
             },
         )
         return obj
 
+    @classmethod
+    def reject_content_for_addon(cls, addon):
+        """
+        Set content_review_status to CONTENT_REVIEW_STATUSES.FAIL for this
+        addon.
+        """
+        obj, _ = cls.objects.update_or_create(
+            addon=addon,
+            defaults={
+                'last_content_review': None,
+                'content_review_status': (
+                    AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL
+                ),
+            },
+        )
+        return obj
+
     @classmethod
     def reset_content_for_addon(cls, addon):
         """
         Reset the last_content_review date for this addon so it triggers
         another review.
         """
-        obj, _ = cls.objects.update_or_create(
+        obj, created = cls.objects.update_or_create(
             addon=addon, defaults={'last_content_review': None}
         )
+        if (
+            not created
+            and obj.content_review_status == cls.CONTENT_REVIEW_STATUSES.PASS
+        ):
+            obj.update(content_review_status=cls.CONTENT_REVIEW_STATUSES.CHANGED)
+        assert obj.last_content_review is None
+        return obj
+
+    @classmethod
+    def request_new_content_review_for_addon(cls, addon):
+        """
+        Set content_review_status to CONTENT_REVIEW_STATUSES.REQUESTED for this addon.
+        """
+        obj, _ = cls.objects.update_or_create(
+            addon=addon,
+            defaults={
+                'content_review_status': (
+                    AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REQUESTED
+                ),
+            },
+        )
         return obj