8000 Fix: Unpublished or archived versions not shown in language menu (#440) · django-cms/djangocms-versioning@59e06b9 · GitHub
[go: up one dir, main page]
Skip to content

Commit 59e06b9

Browse files
fsbraunfsbraun
authored
Fix: Unpublished or archived versions not shown in language menu (#440)
* Fix: Unpublished or archived versions not shown in language choser https://github.com/django-cms/django-cms/issues/8108 * Fix ruff issues * Add test * Fix `latest_content` admin queryset filter * Fix: stay for post-filter * test: add regression test for https://github.com/django-cms/django-cms/issues/8108 * Remove unused import * Fix: missing subquery statement * Fix QS for MySql
1 parent 76504b7 commit 59e06b9

File tree

4 files changed

+149
-57
lines changed

4 files changed

+149
-57
lines changed

‎djangocms_versioning/cms_toolbars.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ def get_page_content(self, language: Optional[str] = None) -> PageContent:
309309
return toolbar_obj
310310
else:
311311
# Get it from the DB
312-
return get_latest_admin_viewable_content(self.page, language=language)
312+
return get_latest_admin_viewable_content(self.page, language=language, include_unpublished_archived=True)
313313

314314
def populate(self):
315315
self.page = self.request.current_page
@@ -335,7 +335,7 @@ def override_language_menu(self):
335335

336336
for code, name in get_language_tuple(self.current_site.pk):
337337
# Get the page content, it could be draft too!
338-
page_content = self.get_page_content(language=code)
338+
page_content = self.page.get_admin_content(language=code)
339339
if page_content:
340340
url = get_object_preview_url(page_content, code)
341341
language_menu.add_link_item(name, url=url, active=self.current_lang == code)

‎djangocms_versioning/helpers.py

Lines changed: 68 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ def is_editable(content_obj, request):
3131
"""Check of content_obj is editable"""
3232
from .models import Version
3333

34-
return Version.objects.get_for_content(content_obj).check_modify.as_bool(request.user)
34+
return Version.objects.get_for_content(content_obj).check_modify.as_bool(
35+
request.user
36+
)
3537

3638

3739
def versioning_admin_factory(admin_class, mixin):
@@ -98,7 +100,7 @@ def register_versionadmin_proxy(versionable, admin_site=None):
98100
warnings.warn(
99101
f"{versionable.version_model_proxy!r} is already registered with admin.",
100102
UserWarning,
101-
stacklevel=2
103+
stacklevel=2,
102104
)
103105
return
104106

@@ -136,7 +138,9 @@ def manager_factory(manager, prefix, mixin):
136138
def replace_manager(model, manager, mixin, **kwargs):
137139
if hasattr(model, manager) and isinstance(getattr(model, manager), mixin):
138140
return
139-
original_manager = getattr(model, manager).__class__ if hasattr(model, manager) else models.Manager
141+
original_manager = (
142+
getattr(model, manager).__class__ if hasattr(model, manager) else models.Manager
143+
)
140144
manager_object = manager_factory(original_manager, "Versioned", mixin)()
141145
for key, value in kwargs.items():
142146
setattr(manager_object, key, value)
@@ -146,15 +150,19 @@ def replace_manager(model, manager, mixin, **kwargs):
146150
model.add_to_class(manager, manager_object)
147151
if manager == "objects":
148152
# only safe the original default manager
149-
model.add_to_class(f'_original_{"manager" if manager == "objects" else manager}', original_manager())
153+
model.add_to_class(
154+
f'_original_{"manager" if manager == "objects" else manager}',
155+
original_manager(),
156+
)
150157

151158

152159
def inject_generic_relation_to_version(model):
153160
from .models import Version
154161

155162
related_query_name = f"{model._meta.app_label}_{model._meta.model_name}"
156-
model.add_to_class("versions", GenericRelation(
157-
Version, related_query_name=related_query_name))
163+
model.add_to_class(
164+
"versions", GenericRelation(Version, related_query_name=related_query_name)
165+
)
158166
if not hasattr(model, "is_editable"):
159167
model.add_to_class("is_editable", is_editable)
160168

@@ -187,18 +195,18 @@ def nonversioned_manager(model):
187195
def _version_list_url(versionable, **params):
188196
proxy = versionable.version_model_proxy
189197
return add_url_parameters(
190-
admin_reverse(
191-
f"{proxy._meta.app_label}_{proxy._meta.model_name}_changelist"
192-
),
193-
**params
198+
admin_reverse(f"{proxy._meta.app_label}_{proxy._meta.model_name}_changelist"),
199+
**params,
194200
)
195201

196202

197203
def version_list_url(content):
198204
"""Returns a URL to list of content model versions,
199205
filtered by `content`'s grouper
200206
"""
201-
versionable = versionables._cms_extension().versionables_by_content[content.__class__]
207+
versionable = versionables._cms_extension().versionables_by_content[
208+
content.__class__
209+
]
202210
return _version_list_url(
203211
versionable, **versionable.grouping_values(content, relation_suffix=False)
204212
)
@@ -208,7 +216,9 @@ def version_list_url_for_grouper(grouper):
208216
"""Returns a URL to list of content model versions,
209217
filtered by `grouper`
210218
"""
211-
versionable = versionables._cms_extension().versionables_by_grouper[grouper.__class__]
219+
versionable = versionables._cms_extension().versionables_by_grouper[
220+
grouper.__class__
221+
]
212222
return _version_list_url(
213223
versionable, **{versionable.grouper_field_name: str(grouper.pk)}
214224
)
@@ -235,7 +245,7 @@ def is_content_editable(placeholder, user):
235245

236246
def get_editable_url(content_obj, force_admin=False):
237247
"""If the object is editable the cms editable view should be used, with the toolbar.
238-
This method provides the URL for it.
248+
This method provides the URL for it.
239249
"""
240250
if is_editable_model(content_obj.__class__) and not force_admin:
241251
language = getattr(content_obj, "language", None)
@@ -264,10 +274,12 @@ def get_content_types_with_subclasses(models, using=None):
264274
return content_types
265275

266276

267-
def get_preview_url(content_obj: models.Model, language: typing.Union[str, None] = None) -> str:
277+
def get_preview_url(
278+
content_obj: models.Model, language: typing.Union[str, None] = None
279+
) -> str:
268280
"""If the object is editable the cms preview view should be used, with the toolbar.
269-
This method provides the URL for it. It falls back the standard change view
270-
should the object not be frontend editable.
281+
This method provides the URL for it. It falls back the standard change view
282+
should the object not be frontend editable.
271283
"""
272284
versionable = versionables.for_content(content_obj)
273285
if versionable.preview_url:
@@ -300,7 +312,9 @@ def remove_published_where(queryset):
300312
that are published are returned. If you need to return the full queryset
301313
use the "admin_manager" instead of "objects"
302314
"""
303-
raise NotImplementedError("remove_published_where has been replaced by ContentObj.admin_manager")
315+
raise NotImplementedError(
316+
"remove_published_where has been replaced by ContentObj.admin_manager"
317+
)
304318

305319

306320
def get_latest_admin_viewable_content(
@@ -314,9 +328,15 @@ def get_latest_admin_viewable_content(
314328
versionable = versionables.for_grouper(grouper)
315329

316330
# Check if all required grouping fields are given to be able to select the latest admin viewable content
317-
missing_fields = [field for field in versionable.extra_grouping_fields if field not in extra_grouping_fields]
331+
missing_fields = [
332+
field
333+
for field in versionable.extra_grouping_fields
334+
if field not in extra_grouping_fields
335+
]
318336
if missing_fields:
319-
raise ValueError(f"Grouping field(s) {missing_fields} required for {versionable.grouper_model}.")
337+
raise ValueError(
338+
f"Grouping field(s) {missing_fields} required for {versionable.grouper_model}."
339+
)
320340

321341
# Get the name of the content_set (e.g., "pagecontent_set") from the versionable
322342
content_set = versionable.grouper_field.remote_field.get_accessor_name()
@@ -331,10 +351,15 @@ def get_latest_admin_viewable_content(
331351
return qs.filter(**extra_grouping_fields).current_content().first()
332352

333353

334-
def get_latest_admin_viewable_page_content(page: Page, language: str) -> PageContent: # pragma: no cover
335-
warnings.warn("get_latst_admin_viewable_page_content has ben deprecated. "
336-
"Use get_latest_admin_viewable_content(page, language=language) instead.",
337-
DeprecationWarning, stacklevel=2)
354+
def get_latest_admin_viewable_page_content(
355+
page: Page, language: str
356+
) -> PageContent: # pragma: no cover
357+
warnings.warn(
358+
"get_latst_admin_viewable_page_content has ben deprecated. "
359+
"Use get_latest_admin_viewable_content(page, language=language) instead.",
360+
DeprecationWarning,
361+
stacklevel=2,
362+
)
338363
return get_latest_admin_viewable_content(page, language=language)
339364

340365

@@ -378,14 +403,14 @@ def version_is_locked(version) -> settings.AUTH_USER_MODEL:
378403

379404

380405
def version_is_unlocked_for_user(version, user: settings.AUTH_USER_MODEL) -> bool:
381-
"""Check if lock doesn't exist for a version object or is locked to provided user.
382-
"""
406+
"""Check if lock doesn't exist for a version object or is locked to provided user."""
383407
return version.locked_by is None or version.locked_by == user
384408

385409

386-
def content_is_unlocked_for_user(content: models.Model, user: settings.AUTH_USER_MODEL) -> bool:
387-
"""Check if lock doesn't exist or object is locked to provided user.
388-
"""
410+
def content_is_unlocked_for_user(
411+
content: models.Model, user: settings.AUTH_USER_MODEL
412+
) -> bool:
413+
"""Check if lock doesn't exist or object is locked to provided user."""
389414
try:
390415
if hasattr(content, "prefetched_versions"):
391416
version = content.prefetched_versions[0]
@@ -396,7 +421,9 @@ def content_is_unlocked_for_user(content: models.Model, user: settings.AUTH_USER
396421
return True
397422

398423

399-
def placeholder_content_is_unlocked_for_user(placeholder: Placeholder, user: settings.AUTH_USER_MODEL) -> bool:
424+
def placeholder_content_is_unlocked_for_user(
425+
placeholder: Placeholder, user: settings.AUTH_USER_MODEL
426+
) -> bool:
400427
"""Check if lock doesn't exist or placeholder source object
401428
is locked to provided user.
402429
"""
@@ -405,10 +432,7 @@ def placeholder_content_is_unlocked_for_user(placeholder: Placeholder, user: set
405432

406433

407434
def send_email(
408-
recipients: list,
409-
subject: str,
410-
template: str,
411-
template_context: dict
435+
recipients: list, subject: str, template: str, template_context: dict
412436
) -> int:
413437
"""
414438
Send emails using locking templates
@@ -423,22 +447,20 @@ def send_email(
423447
from_email=settings.DEFAULT_FROM_EMAIL,
424448
to=recipients,
425449
)
426-
return message.send(
427-
fail_silently=EMAIL_NOTIFICATIONS_FAIL_SILENTLY
428-
)
450+
return message.send(fail_silently=EMAIL_NOTIFICATIONS_FAIL_SILENTLY)
429451

430452

431-
def get_latest_draft_version(version):
453+
def get_latest_draft_version(version: models.Model) -> models.Model:
432454
"""Get latest draft version of version object and caches it in the
433455
content object"""
434-
from djangocms_versioning.constants import DRAFT
435-
from djangocms_versioning.models import Version
436-
437-
if not hasattr(version.content, "_latest_draft_version"):
438-
drafts = (
439-
Version.objects
440-
.filter_by_content_grouping_values(version.content)
441-
.filter(state=DRAFT)
442-
)
456+
from .models import Version
457+
458+
if (
459+
not hasattr(version.content, "_latest_draft_version")
460+
or getattr(version.content._latest_draft_version, "state", DRAFT) != DRAFT
461+
):
462+
drafts = Version.objects.filter_by_content_grouping_values(
463+
version.content
464+
).filter(state=DRAFT)
443465
version.content._latest_draft_version = drafts.first()
444466
return version.content._latest_draft_version

‎djangocms_versioning/managers.py

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,25 @@ def with_user(self, user):
5454

5555

5656
class AdminQuerySetMixin:
57+
# Annotation for latest pk of draft or published version
58+
_DraftOrPublished = models.Max(
59+
models.Case(
60+
models.When(versions__state__in=(constants.DRAFT, constants.PUBLISHED),
61+
then="versions__pk"),
62+
default=models.Value(0),
63+
)
64+
)
65+
66+
# Annotation for latest pk of any other version
67+
_AnyOther = models.Max(
68+
models.Case(
69+
models.When(
70+
~models.Q(versions__state__in=(constants.DRAFT, constants.PUBLISHED)),
71+
then="versions__pk"),
72+
default=models.Value(0),
73+
)
74+
)
75+
5776
def _chain(self):
5877
# Also clone group by key when chaining querysets!
5978
clone = super()._chain()
@@ -65,6 +84,7 @@ def current_content(self, **kwargs):
6584
versions or published versions (in that order). This optimized query assumes that
6685
draft versions always have a higher pk than any other version type. This is true as long as
6786
no other version type can be converted to draft without creating a new version."""
87+
6888
pk_filter = self.filter(versions__state__in=(constants.DRAFT, constants.PUBLISHED))\
6989
.values(*self._group_by_key)\
7090
.annotate(vers_pk=models.Max("versions__pk"))\
@@ -80,15 +100,13 @@ def latest_content(self, **kwargs):
80100
This filter assumes that there can only be one draft created and that the draft as
81101
the highest pk of all versions (should it exist).
82102
"""
83-
current = self.filter(versions__state__in=(constants.DRAFT, constants.PUBLISHED))\
84-
.values(*self._group_by_key)\
85-
.annotate(vers_pk=models.Max("versions__pk"))
86-
pk_current = current.values("vers_pk")
87-
pk_other = self.exclude(**{key + "__in": current.values(key) for key in self._group_by_key})\
88-
.values(*self._group_by_key)\
89-
.annotate(vers_pk=models.Max("versions__pk"))\
90-
.values("vers_pk")
91-
return self.filter(versions__pk__in=pk_current | pk_other, **kwargs)
103+
104+
latest = (self.values(*self._group_by_key)
105+
.annotate(h1=self._DraftOrPublished, h2=self._AnyOther)
106+
.annotate(vers_pk=models.Case(models.When(h1__gt=0, then="h1"), default="h2"))
107+
.values("vers_pk")
108+
)
109+
return self.filter(versions__pk__in=latest, **kwargs)
92110

93111

94112
class AdminManagerMixin:

‎tests/test_managers.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
from cms.test_utils.testcases import CMSTestCase
2+
3+
from djangocms_versioning import constants
4+
from djangocms_versioning.test_utils import factories
5+
from djangocms_versioning.test_utils.polls.models import PollContent
6+
7+
8+
class TestLatestContentCurrentContent(CMSTestCase):
9+
def setUp(self):
10+
poll1 = factories.PollFactory()
11+
factories.PollVersionFactory(state=constants.PUBLISHED, content__language="de")
12+
13+
factories.PollVersionFactory(state=constants.ARCHIVED, content__poll=poll1, content__language="de")
14+
v1 = factories.PollVersionFactory(state=constants.UNPUBLISHED, content__poll=poll1, content__language="de")
15+
v2 = factories.PollVersionFactory(state=constants.ARCHIVED, content__poll=poll1, content__language="en")
16+
v3 = factories.PollVersionFactory(state=constants.DRAFT, content__poll=poll1, content__language="en")
17+
v4 = factories.PollVersionFactory(state=constants.UNPUBLISHED, content__poll=poll1, content__language="fr")
18+
19+
self.poll = poll1
20+
self.poll_content1 = v1.content
21+
self.poll_content2 = v2.content
22+
self.poll_content3 = v3.content
23+
self.poll_content4 = v4.content
24+
25+
def test_latest_content(self):
26+
"""only one version per grouper and grouping field (language) returned."""
27+
latest_content = PollContent.admin_manager.latest_content(poll=self.poll)
28+
self.assertEqual(latest_content.count(), 3)
29+
self.assertIn(self.poll_content1, latest_content)
30+
self.assertIn(self.poll_content3, latest_content)
31+
self.assertIn(self.poll_content4, latest_content)
32+
33+
def test_latest_content_by_language(self):
34+
"""only one version per grouper and grouping field (language) returned. Additional
35+
filter before or after latest_content() should **not** affect the result."""
36+
37+
latest_content = PollContent.admin_manager.latest_content().filter(poll=self.poll, language="en")
38+
self.assertEqual(latest_content.count(), 1)
39+
self.assertIn(self.poll_content3, latest_content)
40+
41+
latest_content = PollContent.admin_manager.filter(poll=self.poll, language="en").latest_content()
42+
self.assertEqual(latest_content.count(), 1)
43+
self.assertIn(self.poll_content3, latest_content)
44+
45+
latest_content = PollContent.admin_manager.latest_content().filter(poll=self.poll, language="de")
46+
self.assertEqual(latest_content.count(), 1)
47+
self.assertIn(self.poll_content1, latest_content)
48+
49+
latest_content = PollContent.admin_manager.filter(poll=self.poll, language="de").latest_content()
50+
self.assertEqual(latest_content.count(), 1)
51+
self.assertIn(self.poll_content1, latest_content)
52+

0 commit comments

Comments
 (0)
0