filter_queryset_to_pending_replies to ActivityLogQuerySet.pending_rep…

…lies

src/olympia/activity/models.py

@@ -1,3 +1,4 @@
+import datetime
 import json
 import uuid
 from collections import defaultdict
@@ -38,6 +39,16 @@
 
 GENERIC_USER_NAME = gettext('Add-ons Review Team')
 
+NOT_PENDING_IDS = (
+    amo.LOG.DEVELOPER_REPLY_VERSION.id,
+    amo.LOG.APPROVE_VERSION.id,
+    amo.LOG.REJECT_VERSION.id,
+    amo.LOG.PRELIMINARY_VERSION.id,
+    amo.LOG.PRELIMINARY_ADDON_MIGRATED.id,
+    amo.LOG.NOTES_FOR_REVIEWERS_CHANGED.id,
+    amo.LOG.SOURCE_CODE_UPLOADED.id,
+)
+
 
 class GenericMozillaUser(UserProfile):
     class Meta:
@@ -284,6 +295,23 @@ class ActivityLogQuerySet(BaseQuerySet):
     def default_transformer(self, logs):
         ActivityLog.arguments_builder(logs)
 
+    def pending_for_developer(self):
+        latest_reply_date = models.functions.Coalesce(
+            models.Subquery(
+                self.filter(
+                    action__in=NOT_PENDING_IDS,
+                    versionlog__version_id=models.OuterRef('versionlog__version_id'),
+                )
+                .values('created')
+                .order_by('-created')[:1]
+            ),
+            datetime.date.min,
+        )
+        return self.filter(
+            action__in=amo.LOG_REVIEW_QUEUE_DEVELOPER,
+            created__gt=latest_reply_date,
+        )
+
 
 class ActivityLogManager(ManagerBase):
     _queryset_class = ActivityLogQuerySet

src/olympia/activity/tests/test_models.py

@@ -95,6 +95,89 @@ def test_rejected_version_still_valid(self):
         assert self.token.is_valid()
 
 
+class TestActivityLogManager(TestCase):
+    def test_pending_for_developer(self):
+        to_create = (
+            # Tests with Developer_Reply
+            (
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.DEVELOPER_REPLY_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                1,
+            ),
+            (
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.DEVELOPER_REPLY_VERSION,
+                0,
+            ),
+            # Tests with Approval
+            (
+                amo.LOG.APPROVE_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                2,
+            ),
+            (
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.APPROVE_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                1,
+            ),
+            (
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.APPROVE_VERSION,
+                0,
+            ),
+            # Tests with Rejection
+            (
+                amo.LOG.REJECT_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                2,
+            ),
+            (
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REJECT_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                1,
+            ),
+            (
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REJECT_VERSION,
+                0,
+            ),
+            # Test with no approve or reject
+            (
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                amo.LOG.REVIEWER_REPLY_VERSION,
+                3,
+            ),
+        )
+
+        user = user_factory()
+        addon = addon_factory()
+        expected = []
+        for action1, action2, action3, count in to_create:
+            version = version_factory(addon=addon)
+            logs = (
+                ActivityLog.objects.create(action1, addon, version, user=user),
+                ActivityLog.objects.create(action2, addon, version, user=user),
+                ActivityLog.objects.create(action3, addon, version, user=user),
+            )
+            logs[-3].update(created=self.days_ago(2))
+            logs[-2].update(created=self.days_ago(1))
+            logs[-1].update(created=self.days_ago(0))
+            if count:
+                expected.extend(logs[-count:])
+        results = list(ActivityLog.objects.for_addons(addon).pending_for_developer())
+        assert len(results) == len(expected)
+        assert set(results) == set(expected)
+
+
 class TestActivityLog(TestCase):
     fixtures = ['base/addon_3615']
 

src/olympia/activity/tests/test_views.py

@@ -288,7 +288,7 @@ def test_queries(self):
             'fiiiine', amo.LOG.REVIEWER_REPLY_VERSION, self.days_ago(0)
         )
         self._login_developer()
-        with self.assertNumQueries(17):
+        with self.assertNumQueries(16):
             # - 2 savepoints because of tests
             # - 2 user and groups
             # - 2 addon and its translations
@@ -301,8 +301,7 @@ def test_queries(self):
             #   enough yet to pass that to the activity log queryset, it's
             #   difficult since it's not a FK)
             # - 2 version and its translations (same issue)
-            # - 2 for highlighting (repeats the query to fetch the activity log
-            #   per version)
+            # - 1 for highlighting
             response = self.client.get(self.url)
             assert response.status_code == 200
 

src/olympia/activity/utils.py

@@ -393,28 +393,6 @@ def send_activity_mail(
         )
 
 
-NOT_PENDING_IDS = (
-    amo.LOG.DEVELOPER_REPLY_VERSION.id,
-    amo.LOG.APPROVE_VERSION.id,
-    amo.LOG.REJECT_VERSION.id,
-    amo.LOG.PRELIMINARY_VERSION.id,
-    amo.LOG.PRELIMINARY_ADDON_MIGRATED.id,
-    amo.LOG.NOTES_FOR_REVIEWERS_CHANGED.id,
-    amo.LOG.SOURCE_CODE_UPLOADED.id,
-)
-
-
-def filter_queryset_to_pending_replies(queryset, log_type_ids=NOT_PENDING_IDS):
-    latest_reply_date = (
-        queryset.filter(action__in=log_type_ids)
-        .values_list('created', flat=True)
-        .first()
-    )
-    if not latest_reply_date:
-        return queryset
-    return queryset.filter(created__gt=latest_reply_date)
-
-
 def bounce_mail(message, reason):
     recipient_header = (
         None

src/olympia/activity/views.py

@@ -21,11 +21,7 @@
     ActivityLogSerializerForComments,
 )
 from olympia.activity.tasks import process_email
-from olympia.activity.utils import (
-    action_from_user,
-    filter_queryset_to_pending_replies,
-    log_and_notify,
-)
+from olympia.activity.utils import action_from_user, log_and_notify
 from olympia.addons.views import AddonChildMixin
 from olympia.api.permissions import (
     AllowAddonAuthor,
@@ -78,9 +74,7 @@ def check_object_permissions(self, request, obj):
     def get_serializer_context(self):
         ctx = super().get_serializer_context()
         ctx['to_highlight'] = list(
-            filter_queryset_to_pending_replies(self.get_queryset()).values_list(
-                'pk', flat=True
-            )
+            self.get_queryset().pending_for_developer().values_list('pk', flat=True)
         )
         return ctx
 

src/olympia/devhub/templatetags/jinja_helpers.py

@@ -6,7 +6,6 @@
 from olympia import amo
 from olympia.access import acl
 from olympia.activity.models import ActivityLog
-from olympia.activity.utils import filter_queryset_to_pending_replies
 from olympia.amo.templatetags.jinja_helpers import format_date, new_context, page_title
 from olympia.files.models import File
 
@@ -88,10 +87,8 @@ def summarize_validation(validation):
 
 @library.global_function
 def pending_activity_log_count_for_developer(version):
-    alog = ActivityLog.objects.for_versions(version).filter(
-        action__in=amo.LOG_REVIEW_QUEUE_DEVELOPER
-    )
-    return filter_queryset_to_pending_replies(alog).count()
+    alog = ActivityLog.objects.for_versions(version).pending_for_developer()
+    return alog.count()
 
 
 @library.global_function