Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: get_page_content retrieved non page-content objects from the toolbar #423

Merged
merged 4 commits into from
Sep 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions djangocms_versioning/cms_toolbars.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,16 +295,18 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

def get_page_content(self, language: Optional[str] = None) -> PageContent:
# This method overwrites the method in django CMS core. Not necessary
# for django CMS 4.2+
if not language:
language = self.current_lang

if self.page_content and self.page_content.language == language:
if isinstance(self.page_content, PageContent) and self.page_content.language == language:
# Already known - no need to query it again
return self.page_content
toolbar_obj = self.toolbar.get_object()
if toolbar_obj and toolbar_obj.language == language:
if isinstance(toolbar_obj, PageContent) and toolbar_obj.language == language:
# Already in the toolbar, then use it!
return self.toolbar.get_object()
return toolbar_obj
else:
# Get it from the DB
return get_latest_admin_viewable_content(self.page, language=language)
Expand Down
2 changes: 1 addition & 1 deletion djangocms_versioning/test_utils/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def get_toolbar(content_obj, user=None, **kwargs):
request = kwargs.get("request", RequestFactory().get("/"))
request.user = user
request.session = kwargs.get("session", {})
request.current_page = getattr(content_obj, "page", None)
request.current_page = kwargs.get("current_page", getattr(content_obj, "page", None))
request.toolbar = CMSToolbar(request)
# Set the toolbar class
if kwargs.get("toolbar_class", False):
Expand Down
17 changes: 17 additions & 0 deletions tests/test_toolbars.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
from django.utils.text import slugify

from djangocms_versioning.cms_config import VersioningCMSConfig
from djangocms_versioning.cms_toolbars import VersioningPageToolbar
from djangocms_versioning.constants import ARCHIVED, DRAFT, PUBLISHED
from djangocms_versioning.helpers import version_list_url
from djangocms_versioning.test_utils.factories import (
BlogPostVersionFactory,
FancyPollFactory,
PageContentWithVersionFactory,
PageFactory,
PageUrlFactory,
PageVersionFactory,
PollVersionFactory,
Expand Down Expand Up @@ -615,3 +617,18 @@ def test_page_toolbar_wo_language_menu(self):

language_menu = request.toolbar.get_menu(LANGUAGE_MENU_IDENTIFIER, _("Language"))
self.assertIsNone(language_menu)

def test_toolbar_only_catches_page_content_objects(self):
"""Regression test to ensure that the toolbar only catches PageContent objects and not
other toolbar objects."""

version = PollVersionFactory() # Not a page content model
page = PageFactory() # Get a page, e.g. where an apphook is configured
toolbar = get_toolbar(version.content, edit_mode=True, toolbar_class=VersioningPageToolbar, current_page=page)

# Did page get detected? Otherwise, page_content never will be detected
self.assertIs(toolbar.page, page)
# Check regression does not happen
self.assertNotIsInstance(toolbar.page_content, version.content.__class__)
# Check for correct result
self.assertIsNone(toolbar.page_content)
Loading