Skip to content

Commit

Permalink
fix: Language chooser options pointing to the same language (#7698)
Browse files Browse the repository at this point in the history
* Fix: Language chooser

* Ensure page language uniqueness

* Fix linter issue

* Add edit mode and preview mode to language chooser

* Add option to register grouper field for frontend-editable models

* Undo changes to cms_toolbars to avoid code redundancy

* Simplify toolbar utils.

* Remove not util function

* Remove unneeded imports

* fix: allow for `EmptyPageContent`
  • Loading branch information
fsbraun committed Dec 8, 2023
1 parent 5af7fc7 commit 2d17eaa
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 27 deletions.
1 change: 0 additions & 1 deletion cms/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ def _verify_apphook(apphook, namespace):
try:
assert apphook in apphook_pool.apps
except AssertionError:
print(apphook_pool.apps.values())
raise
apphook_name = apphook
else:
Expand Down
7 changes: 5 additions & 2 deletions cms/cms_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
class CMSCoreConfig(CMSAppConfig):
cms_enabled = True
cms_wizards = [cms_page_wizard, cms_subpage_wizard]
cms_toolbar_enabled_models = [(PageContent, render_pagecontent)]
cms_toolbar_enabled_models = [(PageContent, render_pagecontent, "page")]


class CMSCoreExtensions(CMSAppExtension):

def __init__(self):
self.wizards = {}
self.toolbar_enabled_models = {}
self.model_groupers = {}
self.toolbar_mixins = []

def configure_wizards(self, cms_config):
Expand All @@ -49,13 +50,15 @@ def configure_wizards(self, cms_config):
def configure_toolbar_enabled_models(self, cms_config):
if not isinstance(cms_config.cms_toolbar_enabled_models, Iterable):
raise ImproperlyConfigured("cms_toolbar_enabled_models must be iterable")
for model, render_func in cms_config.cms_toolbar_enabled_models:
for model, render_func, *grouper in cms_config.cms_toolbar_enabled_models:
if model in self.toolbar_enabled_models:
logger.warning(
"Model {} already registered for frontend rendering".format(model),
)
else:
self.toolbar_enabled_models[model] = render_func
if grouper:
self.model_groupers[model] = grouper[0]

def configure_toolbar_mixin(self, cms_config):
if not issubclass(cms_config.cms_toolbar_mixin, object):
Expand Down
13 changes: 7 additions & 6 deletions cms/cms_toolbars.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ def add_language_menu(self):
url = language_changer(code)
except NoReverseMatch:
url = DefaultLanguageChanger(self.request)(code)
self._language_menu.add_link_item(name, url=url, active=self.current_lang == code)
if url:
self._language_menu.add_link_item(name, url=url, active=self.current_lang == code)
else:
# We do not have to check every time the toolbar is created
self._language_menu = True # Pretend the language menu is already there
Expand Down Expand Up @@ -395,7 +396,7 @@ def get_page_content(self):

def has_page_change_permission(self):
if not hasattr(self, 'page_change_permission'):
self.page_change_permission = can_change_page(self.request)
self.page_change_permission = can_change_page(self.request) and self.toolbar.object_is_editable()
return self.page_change_permission

def in_apphook(self):
Expand Down Expand Up @@ -472,7 +473,6 @@ def change_language_menu(self):
)
else:
can_change = False

if can_change:
language_menu = self.toolbar.get_menu(LANGUAGE_MENU_IDENTIFIER)
if not language_menu:
Expand Down Expand Up @@ -508,9 +508,10 @@ def change_language_menu(self):
disabled = len(remove) == 1
for code, name in remove:
pagecontent = self.page.get_content_obj(code)
translation_delete_url = admin_reverse('cms_pagecontent_delete', args=(pagecontent.pk,))
url = add_url_parameters(translation_delete_url, language=code)
remove_plugins_menu.add_modal_item(name, url=url, disabled=disabled)
if pagecontent:
translation_delete_url = admin_reverse('cms_pagecontent_delete', args=(pagecontent.pk,))
url = add_url_parameters(translation_delete_url, language=code)
remove_plugins_menu.add_modal_item(name, url=url, disabled=disabled)

if copy:
copy_plugins_menu = language_menu.get_or_create_menu(
Expand Down
2 changes: 1 addition & 1 deletion cms/models/pagemodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ def remove_language(self, language):
self.update_languages(page_languages)

def update_languages(self, languages):
languages = ",".join(languages)
languages = ",".join(set(languages))
# Update current instance
self.languages = languages
# Commit. It's important to not call save()
Expand Down
36 changes: 36 additions & 0 deletions cms/tests/test_toolbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
from cms.toolbar.utils import (
add_live_url_querystring_param,
get_object_edit_url,
get_object_for_language,
get_object_preview_url,
get_object_structure_url,
)
Expand Down Expand Up @@ -690,20 +691,26 @@ def test_double_menus(self):
"""
user = self.get_staff()
page = create_page('test', 'nav_playground.html', 'en')
for code, verbose in get_language_tuple():
if code != "en":
create_page_content(code, f"test {code}", page)
page_content = self.get_page_title_obj(page)
edit_url = get_object_edit_url(page_content)
en_request = self.get_page_request(None, user, edit_url)
toolbar = CMSToolbar(en_request)
toolbar.set_object(page_content)
toolbar.populated = False
toolbar.populate()
toolbar.populated = False
toolbar.populate()
toolbar.populated = False
toolbar.post_template_populate()
get_object_for_language(page_content, "de")
admin = toolbar.get_left_items()[0]
lang = toolbar.get_left_items()[1]
self.assertEqual(len(admin.get_items()), 15)
self.assertEqual(len(lang.get_items()), len(get_language_tuple(1)))
self.assertIn(edit_url, [item.url for item in lang.get_items()]) # Edit urls returned

@override_settings(CMS_PLACEHOLDER_CONF={'col_left': {'name': 'PPPP'}})
def test_placeholder_name(self):
Expand Down Expand Up @@ -2093,6 +2100,35 @@ def test_add_live_url_querystring_param_handles_wrong_content_type(self):
self.assertEqual(edit_url.count("?"), 0)
self.assertEqual(preview_url.count("?"), 0)

def test_get_object_for_language_one_language(self):
page = create_page('Test', 'col_two.html', 'en')
page_content = self.get_page_title_obj(page, "en")

self.assertEqual(page_content, get_object_for_language(page_content, "en"))
self.assertTrue(not hasattr(page_content, "_sibling_objects_for_language_cache"))
self.assertIsNone(get_object_for_language(page_content, "de"))
self.assertTrue(hasattr(page_content, "_sibling_objects_for_language_cache"))
self.assertEqual(len(page_content._sibling_objects_for_language_cache), 1)

def test_get_object_for_language_multiple_languages(self):
page = create_page('Test', 'col_two.html', 'en')
# Additional pages to ensure not a page content of another page is returned
for code, verbose in get_language_tuple():
create_page(f"Not this page ({verbose})", "col_two.html", code)

page_content = {
"en": self.get_page_title_obj(page, "en")
}
for code, verbose in get_language_tuple():
if code != "en":
page_content[code] = create_page_content(code, verbose, page)

self.assertEqual(page_content["en"], get_object_for_language(page_content["en"], "en"))
self.assertTrue(not hasattr(page_content["en"], "_sibling_objects_for_language_cache"))
self.assertEqual(get_object_for_language(page_content["en"], "de"), page_content["de"])
self.assertTrue(hasattr(page_content["en"], "_sibling_objects_for_language_cache"))
self.assertEqual(len(page_content["en"]._sibling_objects_for_language_cache), len(get_language_tuple()))


class CharPkFrontendPlaceholderAdminTest(ToolbarTestBase):

Expand Down
9 changes: 5 additions & 4 deletions cms/toolbar/toolbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,11 +403,12 @@ def get_object_structure_url(self):
return get_object_structure_url(self.obj, language=self.request_language)
return ''

def object_is_editable(self):
if self.obj:
if hasattr(self.obj, "is_editable"):
def object_is_editable(self, obj=None):
obj = obj or self.obj
if obj:
if hasattr(obj, "is_editable"):
# Object can decide itself
return self.obj.is_editable(self.request)
return obj.is_editable(self.request)
return True
return False

Expand Down
66 changes: 63 additions & 3 deletions cms/toolbar/utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import json
from collections import defaultdict, deque
from typing import Optional

from django.apps import apps
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.urls import NoReverseMatch
from django.utils.encoding import force_str
from django.utils.translation import (
get_language,
Expand Down Expand Up @@ -145,7 +149,13 @@ def add_live_url_querystring_param(obj, url, language=None):
return url


def get_object_edit_url(obj, language=None):
def get_object_edit_url(obj: models.Model, language: str = None) -> str:
"""
Returns the url of the edit endpoint for the given object. The object must be frontend-editable
and registered as such with cms.
If the object has a language property, the language parameter is ignored.
"""
content_type = ContentType.objects.get_for_model(obj)

language = getattr(obj, "language", language) # Object trumps parameter
Expand All @@ -159,7 +169,13 @@ def get_object_edit_url(obj, language=None):
return url


def get_object_preview_url(obj, language=None):
def get_object_preview_url(obj:models.Model, language: str = None) -> str:
"""
Returns the url of the preview endpoint for the given object. The object must be frontend-editable
and registered as such with cms.
If the object has a language property, the language parameter is ignored.
"""
content_type = ContentType.objects.get_for_model(obj)

language = getattr(obj, "language", language) # Object trumps parameter
Expand All @@ -173,7 +189,14 @@ def get_object_preview_url(obj, language=None):
return url


def get_object_structure_url(obj, language=None):
def get_object_structure_url(obj: models.Model, language: str = None) -> str:
"""
Returns the url of the structure endpoint for the given object. The object must be frontend-editable
and registered as such with cms.
If the object has a language property, the language parameter is ignored.
"""

content_type = ContentType.objects.get_for_model(obj)

language = getattr(obj, "language", language) # Object trumps parameter
Expand All @@ -182,3 +205,40 @@ def get_object_structure_url(obj, language=None):

with force_language(language):
return admin_reverse('cms_placeholder_render_object_structure', args=[content_type.pk, obj.pk])

def get_object_for_language(obj: models.Model, language: str, latest: bool = False) -> Optional[models.Model]:
"""
Retrieves the correct content object for the target language. The object must be frontend-editable
and registered as such with cms.
Two cases have to be distinguished:
1. **Object has a language property:** If the language of the passed object is different,
sibling objects are retrieved from the database and cached in the object passed.
2. **Object has no language property:** The placeholders of the object contain the different
language content. The object itself is returned
"""
if getattr(obj, "language", language) == language:
# Object does not have language field or language is requested language
# Return object itself
return obj
# Does the object have a cache with sister objects
cached_object = getattr(obj, "_sibling_objects_for_language_cache", {})
if cached_object:
return cached_object.get(language)

extension = apps.get_app_config('cms').cms_extension
model = obj.__class__
field = extension.model_groupers.get(model)
if not field:
# Cannot infer sister object if grouper field is unknown
return None
# Grouper model not registered or does not have a get_content_obj method,
# or get_content_obj does not accept language parameter
# Now query db
grouper_filter = {field: getattr(obj, field)}
qs = model.admin_manager.latest_content() if latest and hasattr(model, "admin_manager") else model.objects
obj._sibling_objects_for_language_cache = {
result.language: result for result in qs.filter(**grouper_filter)
}
return obj._sibling_objects_for_language_cache.get(language)
38 changes: 28 additions & 10 deletions menus/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.conf import settings
from django.urls import NoReverseMatch, resolve, reverse
from django.urls import NoReverseMatch, Resolver404, resolve, reverse

from cms.toolbar.utils import get_object_edit_url, get_object_for_language, get_object_preview_url
from cms.utils import get_current_site, get_language_from_request
from cms.utils.i18n import (
force_language,
Expand Down Expand Up @@ -139,24 +140,41 @@ def __call__(self, lang):
with force_language(page_language):
try:
view = resolve(self.request.path_info)
except: # NOQA
except (NoReverseMatch, Resolver404): # NOQA
view = None
if hasattr(self.request, 'toolbar') and self.request.toolbar.obj:
with force_language(lang):
try:
return self.request.toolbar.obj.get_absolute_url()
except: # NOQA
pass
if (
hasattr(self.request, 'toolbar') and
self.request.toolbar.obj and
hasattr(self.request.toolbar.obj, "get_absolute_url")
):
# Toolbar object
if self.request.toolbar.edit_mode_active:
lang_obj = get_object_for_language(self.request.toolbar.obj, lang, latest=True)
return '' if lang_obj is None else get_object_edit_url(lang_obj, language=lang)
if self.request.toolbar.preview_mode_active:
lang_obj = get_object_for_language(self.request.toolbar.obj, lang, latest=True)
return '' if lang_obj is None else get_object_preview_url(lang_obj, language=lang)
try:
# First see, if object can get language-specific absolute urls (like PageContent)
return self.request.toolbar.obj.get_absolute_url(language=lang)
except (TypeError, NoReverseMatch):
# Object's get_absolute_url does not accept language parameter, set the language
with force_language(lang):
try:
url = self.request.toolbar.obj.get_absolute_url()
except NoReverseMatch:
url = None
if url:
return url
elif view and view.url_name not in ('pages-details-by-slug', 'pages-root'):
view_name = view.url_name
if view.namespace:
view_name = "%s:%s" % (view.namespace, view_name)
url = None
with force_language(lang):
try:
url = reverse(view_name, args=view.args, kwargs=view.kwargs, current_app=view.app_name)
except NoReverseMatch:
pass
url = None
if url:
return url
return '%s%s' % (self.get_page_path(lang), self.app_path)

0 comments on commit 2d17eaa

Please sign in to comment.