Record counts for review queues, including detail per promoted class (m…

…ozilla#22906)

src/olympia/amo/models.py

@@ -95,6 +95,21 @@ def only_translations(self):
             qs = qs.transform(transformer.get_trans)
         return qs
 
+    def optimized_count(self):
+        """
+        Slightly optimized count() for cases where there is a DISTINCT in the
+        queryset.
+
+        When a count() call is made on a queryset that has a distinct, that
+        causes django to run the full SELECT (including all fields, distinct,
+        ordering etc) in a subquery and then COUNT() on the result of that
+        subquery, which is costly/innefficient. That's tracked in
+        https://code.djangoproject.com/ticket/30685.
+        We can't easily fix the fact that there is a subquery, but we can
+        avoid selecting all fields and ordering in that subquery needlessly.
+        """
+        return self.values('pk').order_by().count()
+
 
 class RawQuerySet(models.query.RawQuerySet):
     """A RawQuerySet with __len__."""

src/olympia/lib/settings_base.py

@@ -1505,17 +1505,18 @@ def read_only_mode(env):
 # List all jobs that should be callable with cron here.
 # syntax is: job_and_method_name: full.package.path
 CRON_JOBS = {
-    'update_addon_average_daily_users': 'olympia.addons.cron',
-    'update_addon_weekly_downloads': 'olympia.addons.cron',
     'addon_last_updated': 'olympia.addons.cron',
-    'update_addon_hotness': 'olympia.addons.cron',
     'gc': 'olympia.amo.cron',
-    'write_sitemaps': 'olympia.amo.cron',
     'process_blocklistsubmissions': 'olympia.blocklist.cron',
-    'upload_mlbf_to_remote_settings': 'olympia.blocklist.cron',
+    'record_reviewer_queues_counts': 'olympia.reviewers.cron',
+    'sync_suppressed_emails_cron': 'olympia.users.cron',
+    'update_addon_average_daily_users': 'olympia.addons.cron',
+    'update_addon_hotness': 'olympia.addons.cron',
+    'update_addon_weekly_downloads': 'olympia.addons.cron',
     'update_blog_posts': 'olympia.devhub.cron',
     'update_user_ratings': 'olympia.users.cron',
-    'sync_suppressed_emails_cron': 'olympia.users.cron',
+    'upload_mlbf_to_remote_settings': 'olympia.blocklist.cron',
+    'write_sitemaps': 'olympia.amo.cron',
 }
 
 RECOMMENDATION_ENGINE_URL = env(

src/olympia/ratings/models.py

@@ -10,7 +10,7 @@
 import olympia.core.logger
 from olympia import activity, amo, core
 from olympia.amo.fields import PositiveAutoField
-from olympia.amo.models import ManagerBase, ModelBase
+from olympia.amo.models import BaseQuerySet, ManagerBase, ModelBase
 from olympia.amo.templatetags import jinja_helpers
 from olympia.amo.utils import send_mail_jinja
 from olympia.translations.templatetags.jinja_helpers import truncate
@@ -19,7 +19,7 @@
 log = olympia.core.logger.getLogger('z.ratings')
 
 
-class RatingQuerySet(models.QuerySet):
+class RatingQuerySet(BaseQuerySet):
     """
     A queryset modified for soft deletion.
     """

src/olympia/reviewers/cron.py

@@ -0,0 +1,30 @@
+from datetime import date
+
+from olympia.constants.promoted import NOT_PROMOTED, PROMOTED_GROUPS
+from olympia.reviewers.models import QueueCount
+from olympia.reviewers.utils import PendingManualApprovalQueueTable
+from olympia.reviewers.views import reviewer_tables_registry
+
+
+def record_reviewer_queues_counts():
+    today = date.today()
+    # Grab a queryset for each reviewer queue.
+    querysets = {
+        queue.name: queue.get_queryset(None)
+        for queue in reviewer_tables_registry.values()
+    }
+    # Also drill down manual review queue by promoted class (there is no real
+    # queue for each, but we still want that data).
+    for group in PROMOTED_GROUPS:
+        if group != NOT_PROMOTED:
+            querysets[f'{PendingManualApprovalQueueTable.name}/{group.api_name}'] = (
+                PendingManualApprovalQueueTable.get_queryset(
+                    None
+                ).filter(promotedaddon__group_id=group.id)
+            )
+
+    # Execute a count for each queryset and record a QueueCount instance for it
+    for key, qs in querysets.items():
+        QueueCount.objects.get_or_create(
+            name=key, date=today, defaults={'value': qs.optimized_count()}
+        )

src/olympia/reviewers/migrations/0039_alter_autoapprovalsummary_verdict_and_more.py

@@ -0,0 +1,28 @@
+# Generated by Django 4.2.16 on 2024-12-05 12:00
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('reviewers', '0038_auto_20240909_1647'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='autoapprovalsummary',
+            name='verdict',
+            field=models.PositiveSmallIntegerField(choices=[(0, 'Would *not* have been auto-approved (dry-run mode was in effect)'), (1, 'Would have been auto-approved (dry-run mode was in effect)'), (2, 'Was auto-approved'), (3, 'Was *not* auto-approved')], default=3),
+        ),
+        migrations.AlterField(
+            model_name='needshumanreview',
+            name='reason',
+            field=models.SmallIntegerField(choices=[(0, 'Unknown'), (1, 'Hit scanner rule'), (2, 'Was added to a promoted group'), (3, 'Over growth threshold for usage tier'), (4, 'Previous version in channel had needs human review set'), (5, 'Sources provided while pending rejection'), (6, 'Developer replied'), (7, 'Manually set as needing human review by a reviewer'), (8, 'Auto-approved but still had an approval delay set in the past'), (9, 'Over abuse reports threshold for usage tier'), (10, 'Escalated for an abuse report, via cinder'), (11, 'Reported for abuse within the add-on'), (12, "Appeal of a reviewer's decision about a policy violation"), (13, 'Has auto-approval disabled'), (14, 'Belongs to a promoted group')], default=0, editable=False),
+        ),
+        migrations.AlterField(
+            model_name='usagetier',
+            name='growth_threshold_before_flagging',
+            field=models.IntegerField(blank=True, default=None, help_text='Percentage point increase in growth over the average in that tier before we start automatically flagging the add-on for human review. For example, if set to 50 and the average hotness in that tier is 0.01, add-ons over 0.51 hotness get flagged.', null=True),
+        ),
+    ]

src/olympia/reviewers/migrations/0040_queuecount_queuecount_queue_count_unique_name_date.py

@@ -0,0 +1,26 @@
+# Generated by Django 4.2.16 on 2024-12-05 12:01
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('reviewers', '0039_alter_autoapprovalsummary_verdict_and_more'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='QueueCount',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('date', models.DateField(auto_now_add=True)),
+                ('name', models.CharField(max_length=255)),
+                ('value', models.IntegerField()),
+            ],
+        ),
+        migrations.AddConstraint(
+            model_name='queuecount',
+            constraint=models.UniqueConstraint(fields=('name', 'date'), name='queue_count_unique_name_date'),
+        ),
+    ]

src/olympia/reviewers/models.py

@@ -951,3 +951,19 @@ def update_due_date_for_needs_human_review_change(
 ):
     if update_fields is None or 'is_active' in update_fields:
         instance.version.reset_due_date()
+
+
+class QueueCount(models.Model):
+    date = models.DateField(auto_now_add=True)
+    name = models.CharField(max_length=255)
+    value = models.IntegerField()
+
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(
+                fields=('name', 'date'), name='queue_count_unique_name_date'
+            ),
+        ]
+
+    def __str__(self):
+        return f'{self.name} for {self.date}: {self.value}'

src/olympia/reviewers/templates/reviewers/queue.html

@@ -8,7 +8,7 @@
   {% endif %}
 {% endblock %}
 
-{% block bodyclass %}{{ super() }} {{ " content-review" if tab == "content_review" else "" }}{% endblock %}
+{% block bodyclass %}{{ super() }} {{ " content-review" if tab == "queue_content_review" else "" }}{% endblock %}
 
 {% block content %}
 
@@ -24,7 +24,7 @@
     {{ page|paginator }}
     </div>
 
-    {% if tab == 'moderated' %}
+    {% if tab == 'queue_moderated' %}
       <div id="reviews-flagged">
         <form method="post" class="item">
           <div class="review-saved">
@@ -77,7 +77,7 @@ <h3>
           {% endif %}
         </form>
       </div>
-      {% elif tab == 'held_decisions' %}
+      {% elif tab == 'queue_decisions' %}
       <table id="held-decision-queue" class="data-grid">
         <thead>
           <tr class="listing-header">

src/olympia/reviewers/templatetags/jinja_helpers.py

@@ -19,25 +19,9 @@ def queue_tabnav(context, reviewer_tables_registry):
     request = context['request']
     tabnav = []
 
-    for queue in (
-        'extension',
-        'mad',
-        'theme',
-        'moderated',
-        'content_review',
-        'pending_rejection',
-        'held_decisions',
-    ):
-        if acl.action_allowed_for(
-            request.user, reviewer_tables_registry[queue].permission
-        ):
-            tabnav.append(
-                (
-                    queue,
-                    reviewer_tables_registry[queue].urlname,
-                    reviewer_tables_registry[queue].title,
-                )
-            )
+    for tab, queue in reviewer_tables_registry.items():
+        if acl.action_allowed_for(request.user, queue.permission):
+            tabnav.append((tab, queue.name, queue.title))
 
     return tabnav
 

src/olympia/reviewers/tests/test_cron.py

@@ -0,0 +1,147 @@
+from datetime import date
+
+from django.conf import settings
+
+from freezegun import freeze_time
+
+from olympia import amo
+from olympia.amo.tests import TestCase, addon_factory, user_factory
+from olympia.constants.promoted import NOTABLE, PROMOTED_GROUPS, RECOMMENDED
+from olympia.reviewers.cron import record_reviewer_queues_counts
+from olympia.reviewers.models import NeedsHumanReview, QueueCount
+from olympia.reviewers.views import reviewer_tables_registry
+
+
+class TestQueueCount(TestCase):
+    def setUp(self):
+        user_factory(pk=settings.TASK_USER_ID)
+
+    def _test_expected_count(self, date):
+        # We are recording every queue, plus drilling down in every promoted
+        # group, minus the special not promoted group.
+        expected_count = len(reviewer_tables_registry) + len(PROMOTED_GROUPS) - 1
+        assert QueueCount.objects.filter(date=date).count() == expected_count
+
+    def test_empty(self):
+        with freeze_time('2024-12-03'):
+            expected_date = date.today()
+            record_reviewer_queues_counts()
+
+        self._test_expected_count(expected_date)
+
+        for metric in QueueCount.objects.all():
+            assert metric.date == expected_date
+            assert metric.name
+            assert metric.value == 0
+
+    def test_basic(self):
+        addon_factory()
+        addon_factory(
+            needshumanreview_kw={'reason': NeedsHumanReview.REASONS.UNKNOWN},
+        )
+        addon_factory(
+            file_kw={'status': amo.STATUS_AWAITING_REVIEW},
+            needshumanreview_kw={
+                'reason': NeedsHumanReview.REASONS.AUTO_APPROVAL_DISABLED
+            },
+        )
+        self.addon_recommended_1 = addon_factory(
+            file_kw={'status': amo.STATUS_AWAITING_REVIEW},
+            promoted=RECOMMENDED,
+            needshumanreview_kw={
+                'reason': NeedsHumanReview.REASONS.BELONGS_TO_PROMOTED_GROUP
+            },
+        )
+        addon_factory(
+            file_kw={'status': amo.STATUS_AWAITING_REVIEW},
+            promoted=RECOMMENDED,
+            needshumanreview_kw={
+                'reason': NeedsHumanReview.REASONS.BELONGS_TO_PROMOTED_GROUP
+            },
+        )
+        addon_factory(
+            file_kw={'status': amo.STATUS_AWAITING_REVIEW},
+            promoted=NOTABLE,
+            needshumanreview_kw={
+                'reason': NeedsHumanReview.REASONS.BELONGS_TO_PROMOTED_GROUP
+            },
+        )
+
+        with freeze_time('2024-12-03'):
+            expected_date = date.today()
+            record_reviewer_queues_counts()
+
+        self._test_expected_count(expected_date)
+
+        metric = QueueCount.objects.get(name='queue_extension')
+        assert metric.date == expected_date
+        assert metric.value == 5
+
+        metric = QueueCount.objects.get(name='queue_extension/recommended')
+        assert metric.date == expected_date
+        assert metric.value == 2
+
+        metric = QueueCount.objects.get(name='queue_extension/notable')
+        assert metric.date == expected_date
+        assert metric.value == 1
+
+    def test_twice_same_date_doesnt_override(self):
+        self.test_basic()
+        self.test_basic()
+
+    def test_twice_different_day(self):
+        self.test_basic()
+        previous_date = QueueCount.objects.latest('pk').date
+
+        self.addon_recommended_1.current_version.file.update(status=amo.STATUS_APPROVED)
+        self.addon_recommended_1.current_version.needshumanreview_set.all()[0].update(
+            is_active=False
+        )
+
+        with freeze_time('2024-12-04'):
+            expected_date = date.today()
+            record_reviewer_queues_counts()
+
+        # Previous date records are not affected.
+        self._test_expected_count(previous_date)
+        assert (
+            QueueCount.objects.get(date=previous_date, name='queue_extension').value
+            == 5
+        )
+        assert (
+            QueueCount.objects.get(
+                date=previous_date, name='queue_extension/recommended'
+            ).value
+            == 2
+        )
+        assert (
+            QueueCount.objects.get(
+                date=previous_date, name='queue_extension/notable'
+            ).value
+            == 1
+        )
+
+        # New date records
+        self._test_expected_count(expected_date)
+
+        # One fewer add-on in the queue.
+        assert (
+            QueueCount.objects.get(date=expected_date, name='queue_extension').value
+            == 4
+        )
+
+        # One fewer add-on in the queue that was recommended.
+        assert (
+            QueueCount.objects.get(
+                date=expected_date, name='queue_extension/recommended'
+            ).value
+            == 1
+        )
+
+        # No changes to notable.
+        assert (
+            QueueCount.objects.get(
+                date=expected_date, name='queue_extension/notable'
+            ).value
+            == 1
+        )