feat: Changelist performance improvement

Author: fsbraun

djangocms_versioning/admin.py

@@ -20,7 +20,7 @@
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist, PermissionDenied
 from django.db import models
-from django.db.models import OuterRef, Subquery
+from django.db.models import OuterRef, Prefetch, Subquery
 from django.db.models.functions import Cast, Lower
 from django.forms import MediaDefiningClass
 from django.http import (
@@ -50,6 +50,7 @@
     get_current_site,
     get_editable_url,
     get_latest_admin_viewable_content,
+    get_latest_content_from_cache,
     get_object_live_url,
     get_preview_url,
     proxy_model,
@@ -182,20 +183,29 @@ def _extra_grouping_fields(self):
 
     def get_indicator_column(self, request):
         def indicator(obj):
+            versions = None
             if self._extra_grouping_fields is not None:  # Grouper Model
                 content_obj = get_latest_admin_viewable_content(
                     obj,
                     include_unpublished_archived=True,
                     **{field: getattr(self, field) for field in self._extra_grouping_fields},
                 )
+                for prefetched in getattr(obj, "_prefetched_contents", []):
+                    prefetched._prefetched_versions[0].content = prefetched  # Avoid fetching reverse
+                versions = (
+                    [content._prefetched_versions[0] for content in obj._prefetched_contents]
+                    if hasattr(obj, "_prefetched_contents")
+                    else None
+                )
             else:  # Content Model
                 content_obj = obj
-            status = content_indicator(content_obj)
+
+            status = content_indicator(content_obj, versions)
             menu = (
                 content_indicator_menu(
                     request,
                     status,
-                    content_obj._version,
+                    content_obj._versions,
                     back=f"{request.path_info}?{request.GET.urlencode()}",
                 )
                 if status
@@ -318,8 +328,37 @@ def get_queryset(self, request: HttpRequest) -> models.QuerySet:
             # cast is necessary for mysql
             content_modified=Cast(Subquery(contents.values("content_modified")[:1]), models.DateTimeField()),
         )
+        # Prefetching is not implemented here; you may want to use Prefetch for related objects if needed.
+        # To get the reverse name for self.grouper_field_name:
+        # It's usually: <related_model>_set or the related_name defined on the ForeignKey.
+        # For a ForeignKey field, you can get the reverse accessor name like this:
+        reverse_name = self.content_model._meta.get_field(self.grouper_field_name).remote_field.get_accessor_name()
+        qs = qs.prefetch_related(
+            Prefetch(
+                reverse_name,
+                to_attr="_prefetched_contents",  # Needed for state indicators
+                queryset=self.content_model.admin_manager.filter(**self.current_content_filters)
+                .prefetch_related(Prefetch("versions", to_attr="_prefetched_versions"))
+                .order_by("-pk"),
+            )
+        )
+        qs = qs.prefetch_related(
+            Prefetch(
+                reverse_name,
+                to_attr="_current_contents",  # Service for get_content_obj
+                queryset=self.content_model.admin_manager.current_content(**self.current_content_filters)
+                .prefetch_related(Prefetch("versions", to_attr="_prefetched_versions"))
+                .order_by("-pk"),
+            )
+        )
         return qs
 
+    def get_content_obj(self, obj: models.Model) -> models.Model:
+        """Returns the latest content object for the given grouper object."""
+        if obj is None or self._is_content_obj(obj) or not hasattr(obj, "_prefetched_contents"):
+            return super().get_content_obj(obj)
+        return get_latest_content_from_cache(obj._prefetched_contents, include_unpublished_archived=True)
+
     @admin.display(
         description=_("State"),
         ordering="content_state",

djangocms_versioning/cms_config.py

@@ -328,7 +328,7 @@ def get_indicator_menu(cls, request, page_content):
             status = page_content.content_indicator()
         if not status or status == "empty":  # pragma: no cover
             return super().get_indicator_menu(request, page_content)
-        versions = page_content._version  # Cache from .content_indicator()
+        versions = page_content._versions  # Cache from .content_indicator()
         back = admin_reverse("cms_pagecontent_changelist") + f"?language={request.GET.get('language')}"
         menu = indicators.content_indicator_menu(request, status, versions, back=back)
         return menu_template if menu else "", menu

djangocms_versioning/helpers.py

@@ -22,7 +22,7 @@
 
 from . import versionables
 from .conf import EMAIL_NOTIFICATIONS_FAIL_SILENTLY
-from .constants import DRAFT
+from .constants import DRAFT, PUBLISHED
 
 try:
     from djangocms_internalsearch.helpers import emit_content_change
@@ -360,6 +360,11 @@ def get_latest_admin_viewable_content(
             f"Grouping field(s) {missing_fields} required for {versionable.grouper_model}."
         )
 
+    if hasattr(grouper, "_prefetched_contents"):
+        return get_latest_content_from_cache(
+            grouper._prefetched_contents, include_unpublished_archived
+        )
+
     # Get the name of the content_set (e.g., "pagecontent_set") from the versionable
     content_set = versionable.grouper_field.remote_field.get_accessor_name()
 
@@ -373,6 +378,20 @@ def get_latest_admin_viewable_content(
     return qs.filter(**extra_grouping_fields).current_content().first()
 
 
+def get_latest_content_from_cache(cache, include_unpublished_archived: bool = False) -> models.Model | None:
+    # Evaluate prefetched contents in python
+    for content in cache:
+        if content._prefetched_versions[0].state == DRAFT:
+            return content
+    for content in cache:
+        if content._prefetched_versions[0].state == PUBLISHED:
+            return content
+    if include_unpublished_archived:
+        order_by_date = sorted(cache, key=lambda v: v._prefetched_versions[0].created, reverse=True)
+        return order_by_date[0] if order_by_date else None
+    return None
+
+
 def get_latest_admin_viewable_page_content(
     page: Page, language: str
 ) -> PageContent:  # pragma: no cover

djangocms_versioning/indicators.py

@@ -112,20 +112,20 @@ def content_indicator(
         }
         if DRAFT in signature and PUBLISHED not in signature:
             content_obj._indicator_status = "draft"
-            content_obj._version = signature[DRAFT],
+            content_obj._versions = signature[DRAFT],
         elif DRAFT in signature and PUBLISHED in signature:
             content_obj._indicator_status = "dirty"
-            content_obj._version = (signature[DRAFT], signature[PUBLISHED])
+            content_obj._versions = (signature[DRAFT], signature[PUBLISHED])
         elif PUBLISHED in signature:
             content_obj._indicator_status = "published"
-            content_obj._version = signature[PUBLISHED],
+            content_obj._versions = signature[PUBLISHED],
         elif versions[0].state == UNPUBLISHED:
             content_obj._indicator_status = "unpublished"
-            content_obj._version = signature[UNPUBLISHED],
+            content_obj._versions = signature[UNPUBLISHED],
         elif versions[0].state == ARCHIVED:
             content_obj._indicator_status = "archived"
-            content_obj._version = signature[ARCHIVED],
+            content_obj._versions = signature[ARCHIVED],
         else:  # pragma: no cover
             content_obj._indicator_status = None
-            content_obj._version = [None]
+            content_obj._versions = [None]
     return content_obj._indicator_status

djangocms_versioning/models.py

@@ -493,7 +493,7 @@ def _has_permission(self, perm: str, user) -> bool:
             # Second fallback: placeholder change permissions - works for PageContent
             return self.content.has_placeholder_change_permission(user)
         # final fallback: Django perms
-        return user.has_perm(f"{self.content_type.app_label}.change_{self.content_type.model}")
+        return user.has_perm(f"{self.content._meta.app_label}.change_{self.content._meta.model_name}")
 
     check_modify = Conditions(
         [

tests/test_admin.py

@@ -3402,3 +3402,74 @@ def test_buttons_in_archived_changeview(self):
         self.assertNotContains(response, "New Draft")
         self.assertNotContains(response, "Publish")
 
+
+class GrouperAdminPerformanceTestCase(CMSTestCase):
+    """Test to ensure Poll GrouperAdmin changelist_view queries are optimized"""
+
+    def setUp(self):
+        super().setUp()
+        # Pre-cache site to get consistent query counts across tests
+        from django.contrib.sites.models import Site
+        Site.objects.get_current()
+
+    def test_poll_grouper_changelist_queries_are_optimized(self):
+        """Pin the number of queries for Poll GrouperAdmin changelist to prevent regressions"""
+        # Create test data: 5 polls with 2 versions each (draft + published)
+        for _i in range(5):
+            # Create published version
+            published = PollVersionFactory(state=constants.PUBLISHED)
+            # Create draft version for same poll
+            PollVersionFactory(
+                content__poll=published.content.poll,
+                content__language=published.content.language,
+                state=constants.DRAFT,
+            )
+
+        from djangocms_versioning.test_utils.polls.admin import PollAdmin
+        from djangocms_versioning.test_utils.polls.models import Poll
+
+        poll_admin = PollAdmin(Poll, admin.site)
+
+        # Create request
+        factory = RequestFactory()
+        request = factory.get("/admin/polls/poll/")
+        request.user = self.get_superuser()
+
+        # Pin the number of queries
+        # Expected queries should remain constant due to prefetch optimization
+        # 1. Count query for pagination
+        # 2. Count query (duplicate from admin)
+        # 3. Main queryset with subqueries for content annotations
+        with self.assertNumQueries(3):
+            response = poll_admin.changelist_view(request)
+            # Force evaluation of queryset
+            list(response.context_data["cl"].result_list)
+
+    def test_poll_changelist_with_many_versions_scales_well(self):
+        """Ensure query count doesn't increase with more versions per poll"""
+        # Create 1 poll with many versions
+        poll_version = PollVersionFactory(state=constants.PUBLISHED)
+        poll = poll_version.content.poll
+
+        # Add 10 more versions (archived)
+        for _ in range(10):
+            PollVersionFactory(
+                content__poll=poll,
+                content__language=poll_version.content.language,
+                state=constants.ARCHIVED,
+            )
+
+        from djangocms_versioning.test_utils.polls.admin import PollAdmin
+        from djangocms_versioning.test_utils.polls.models import Poll
+
+        poll_admin = PollAdmin(Poll, admin.site)
+
+        factory = RequestFactory()
+        request = factory.get("/admin/polls/poll/")
+        request.user = self.get_superuser()
+
+        # Query count should remain the same regardless of version count
+        # because of prefetch optimization
+        with self.assertNumQueries(3):
+            response = poll_admin.changelist_view(request)
+            list(response.context_data["cl"].result_list)