Skip to content

Commit

Permalink
fix: Unnecessary complexity in current_content query set (#417)
Browse files Browse the repository at this point in the history
* Fix: Linear in stead of quadratic complexity in `current_content` queryset method.

* Update tests

* Add test for latest_content issue in core
  • Loading branch information
fsbraun authored Jul 30, 2024
1 parent 5bc66e5 commit 9b8abd5
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 16 deletions.
15 changes: 4 additions & 11 deletions djangocms_versioning/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,16 @@ def _chain(self):
clone._group_by_key = self._group_by_key
return clone

def current_content_iterator(self, **kwargs):
"""Returns generator (not a queryset) over current content versions. Current versions are either draft
versions or published versions (in that order)"""
warnings.warn("current_content_iterator is deprecated in favour of current_conent",
DeprecationWarning, stacklevel=2)
return iter(self.current_content(**kwargs))

def current_content(self, **kwargs):
"""Returns a queryset current content versions. Current versions are either draft
versions or published versions (in that order). This optimized query assumes that
draft versions always have a higher pk than any other version type. This is true as long as
no other version type can be converted to draft without creating a new version."""
qs = self.filter(versions__state__in=(constants.DRAFT, constants.PUBLISHED), **kwargs)
pk_filter = qs.values(*self._group_by_key)\
pk_filter = self.filter(versions__state__in=(constants.DRAFT, constants.PUBLISHED))\
.values(*self._group_by_key)\
.annotate(vers_pk=models.Max("versions__pk"))\
.values_list("vers_pk", flat=True)
return qs.filter(versions__pk__in=pk_filter)
.values("vers_pk")
return self.filter(versions__pk__in=pk_filter, **kwargs)

def latest_content(self, **kwargs):
"""Returns the "latest" content object which is in this order
Expand Down
10 changes: 5 additions & 5 deletions tests/test_content_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def setUp(self) -> None:
self.create_page_content(page, "it", constants.ARCHIVED)
self.create_page_content(page, "it", constants.PUBLISHED)

def test_current_content_iterator(self):
def test_current_content(self):
# 12 PageContent versions in total
self.assertEqual(len(list(
PageContent.admin_manager.all()
Expand All @@ -79,11 +79,11 @@ def test_current_content_iterator(self):
self.assertEqual(len(qs), 4)
self.assertEqual(qs._group_by_key, ["page", "language"])
self.assertEqual(len(list(
PageContent.admin_manager.filter(page__in=self.pages1).current_content_iterator()
)), 4, f"{list(PageContent.admin_manager.filter(page__in=self.pages1).current_content_iterator())}")
PageContent.admin_manager.filter(page__in=self.pages1).current_content()
)), 4, f"{list(PageContent.admin_manager.filter(page__in=self.pages1).current_content())}")
# 2 current PageContent versions for self.pages2
self.assertEqual(len(list(
PageContent.admin_manager.filter(page__in=self.pages2).current_content_iterator()
PageContent.admin_manager.filter(page__in=self.pages2).current_content()
)), 4)

# Now unpublish all published in pages2
Expand All @@ -93,5 +93,5 @@ def test_current_content_iterator(self):

# 2 current PageContent versions for self.pages2
self.assertEqual(len(list(
PageContent.admin_manager.filter(page__in=self.pages2).current_content_iterator()
PageContent.admin_manager.filter(page__in=self.pages2).current_content()
)), 2)
45 changes: 45 additions & 0 deletions tests/test_integration_with_core.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
from unittest import skipIf

from cms import __version__ as cms_version
from cms.test_utils.testcases import CMSTestCase
from cms.toolbar.toolbar import CMSToolbar
from cms.utils.urlutils import admin_reverse
from django.template import Context

from djangocms_versioning import constants
from djangocms_versioning.plugin_rendering import VersionContentRenderer
from djangocms_versioning.test_utils.factories import (
PageFactory,
Expand Down Expand Up @@ -256,3 +261,43 @@ def test_success_url_for_cms_wizard(self):
poll_wizard.get_success_url(version.content),
version.content.get_absolute_url(),
)


class AdminManagerIntegrationTestCase(CMSTestCase):
def setUp(self):
self.page = PageFactory(node__depth=1) if TreeNode else PageFactory(depth=1)
self.en_version = PageVersionFactory(
content__page=self.page,
content__language="en",
state=constants.UNPUBLISHED,
)
self.fr_version = PageVersionFactory(
content__page=self.page,
content__language="fr",
state=constants.ARCHIVED,
)
self.page.languages = "en,fr"
self.page.save()


@skipIf(cms_version < "4.1.3", "Bug only fixed in django CMS 4.1.3")
def test_get_admin_url_for_language(self):
"""Regression fixed that made unpublished and archived versions invisivle to get_admin_url_for_language
template tag. See: https://github.com/django-cms/django-cms/pull/7967"""
from django.template import Template

# Test English page with unpublished version
context = Context({"page": self.page})
template = Template("{% load cms_admin %}{% get_admin_url_for_language page 'en' %}")

result = template.render(context)

self.assertIn(f"/admin/cms/pagecontent/{self.en_version.content.pk}/", result)

# Test French page with archived version
template = Template("{% load cms_admin %}{% get_admin_url_for_language page 'fr' %}")

result = template.render(context)

self.assertIn(f"/admin/cms/pagecontent/{self.fr_version.content.pk}/", result)

0 comments on commit 9b8abd5

Please sign in to comment.