diff --git a/djangocms_versioning/admin.py b/djangocms_versioning/admin.py index f68b5a76..080b32e7 100644 --- a/djangocms_versioning/admin.py +++ b/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: _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", diff --git a/djangocms_versioning/cms_config.py b/djangocms_versioning/cms_config.py index dde353be..3c8e0477 100644 --- a/djangocms_versioning/cms_config.py +++ b/djangocms_versioning/cms_config.py @@ -113,6 +113,9 @@ def handle_admin_classes(self, cms_config): """Replaces admin model classes for all registered content types with an admin model class that inherits from `versionable.content_admin_mixin`. """ + from django.contrib.admin import autodiscover + + autodiscover() replace_admin_for_models( [(versionable.content_model, versionable.content_admin_mixin) for versionable in cms_config.versioning] ) @@ -328,7 +331,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 diff --git a/djangocms_versioning/helpers.py b/djangocms_versioning/helpers.py index beb70d7a..93bce577 100644 --- a/djangocms_versioning/helpers.py +++ b/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 diff --git a/djangocms_versioning/indicators.py b/djangocms_versioning/indicators.py index 6bbf7e75..5fb22e77 100644 --- a/djangocms_versioning/indicators.py +++ b/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 diff --git a/djangocms_versioning/models.py b/djangocms_versioning/models.py index 96e9adf8..20ba9218 100644 --- a/djangocms_versioning/models.py +++ b/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( [ diff --git a/tests/requirements/dj60_cms50.txt b/tests/requirements/dj60_cms50.txt index b23867c0..57cdffc1 100644 --- a/tests/requirements/dj60_cms50.txt +++ b/tests/requirements/dj60_cms50.txt @@ -2,7 +2,7 @@ django-cms>=5.0,<5.1 -Django>=6.0a1,<6.1 +Django>=6.0,<6.1 django-classy-tags django-fsm-2==4.1.0 django-sekizai diff --git a/tests/test_admin.py b/tests/test_admin.py index e41c51b1..1f13fecf 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -3402,3 +3402,76 @@ 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.session = {} + 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() + request.session = {} + + # 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)