8000 fix: Grouper admin raised AttributeError when used outside the admin views by fsbraun · Pull Request #8067 · django-cms/django-cms · GitHub
[go: up one dir, main page]

Skip to content

fix: Grouper admin raised AttributeError when used outside the admin views #8067

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

Merged
merged 6 commits into from
Nov 6, 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
1 change: 1 addition & 0 deletions cms/admin/pageadmin.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class PageAdmin(admin.ModelAdmin):
copy_form = CopyPageForm
move_form = MovePageForm
inlines = PERMISSION_ADMIN_INLINES
search_fields = ('=id', 'urls__slug', 'pagecontent_set__title', 'reverse_id')

def has_add_permission(self, request):
return False
Expand Down
14 changes: 11 additions & 3 deletions cms/admin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,16 +387,24 @@ def get_grouping_from_request(self, request: HttpRequest) -> None:
@property
def current_content_filters(self) -> dict[str, typing.Any]:
"""Filters needed to get the correct content model instance"""
return {field: getattr(self, field) for field in self.extra_grouping_fields}
return {field: getattr(self, field, self.get_extra_grouping_field(field)) for field in self.extra_grouping_fields}

def get_language(self) -> str:
"""Hook on how to get the current language. By default, Django provides it."""
return get_language()
"""Hook on how to get the current language. By default, if it is set as a
property, use the property, otherwise let Django provide it."""
return getattr(self, "language", get_language())

def get_language_tuple(self) -> tuple[tuple[str, str], ...]:
"""Hook on how to get all available languages for the language selector."""
return get_language_tuple()

def get_extra_grouping_field(self, field):
"""Retrieves the current value for grouping fields - by default by calling self.get_<field>, e.g.,
self.get_language(). If those are not implemented, this method will fail."""
if callable(getattr(self, f"get_{field}", None)):
return getattr(self, f"get_{field}")()
raise ValueError("Cannot get extra grouping field")

def get_changelist(self, request: HttpRequest, **kwargs) -> type:
"""Allow for extra grouping fields as a non-filter parameter"""
return type(
Expand Down
8 changes: 6 additions & 2 deletions cms/forms/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,15 @@ def validate_url_uniqueness(site, path, language, user_language=None, exclude_pa
if user_language:
change_url += f'?language={user_language}'

conflict_url = f'<a href="{change_url}" target="_blank">{force_str(conflict_page)}</a>'
conflict_url = f'<a href="{change_url}" target="_blank">{str(conflict_translation.title)}</a>'

if exclude_page:
message = gettext('Page %(conflict_page)s has the same url \'%(url)s\' as current page "%(instance)s".')
else:
message = gettext('Page %(conflict_page)s has the same url \'%(url)s\' as current page.')
message = message % {'conflict_page': conflict_url, 'url': path, 'instance': exclude_page}
message = message % {
'conflict_page': conflict_url,
'url': path,
'instance': exclude_page.get_title(language) if exclude_page else ''
}
raise ValidationError(mark_safe(message))
10 changes: 5 additions & 5 deletions cms/models/pagemodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,13 @@ def __init__(self, *args, **kwargs):
#: Might be larger than the page_content_cache

def __str__(self):
lang = self._get_page_content_cache(get_language(), fallback=True, force_reload=False)
page_content = self.page_content_cache.get(lang)
page_content = self.get_content_obj(get_language(), fallback=True)
if page_content:
title = page_content.menu_title or page_content.title
else:
title = _("Empty")
return force_str(title)
title = _("No available title")
path = self.get_path(get_language(), fallback=True)
return force_str(title) + ("" if path is None else f" (/{path})")

def __repr__(self):
display = f'<{self.__module__}.{self.__class__.__name__} id={self.pk} object at {hex(id(self))}>'
Expand Down Expand Up @@ -728,7 +728,7 @@ def set_translations_cache(self):
self.page_content_cache.setdefault(translation.language, translation)

def set_admin_content_cache(self):
self.admin_conent_cache = AdminCacheDict()
self.admin_content_cache = AdminCacheDict()
for translation in self.pagecontent_set(manager="admin_manager").latest_content().all():
self.admin_content_cache.setdefault(translation.language, translation)

Expand Down
2 changes: 1 addition & 1 deletion cms/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def test_search_fields(self):
if not admin_instance.search_fields:
continue
url = admin_reverse('cms_%s_changelist' % model._meta.model_name)
response = self.client.get('%s?q=1' % url)
response = self.client.get('%s?q=1' % url, follow=True) # Page redirects to PageContent
errmsg = response.content
self.assertEqual(response.status_code, 200, errmsg)

Expand Down
24 changes: 24 additions & 0 deletions cms/tests/test_grouper_admin.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.contrib.admin import site
from django.templatetags.static import static
from django.utils.crypto import get_random_string
from django.utils.translation import get_language, override as force_language

from cms.admin.utils import CONTENT_PREFIX
from cms.test_utils.project.sampleapp.models import (
Expand Down Expand Up @@ -126,6 +127,29 @@ def test_content_model_detected(self) -> None:
admin = site._registry[GrouperModel]
self.assertEqual(admin.content_model, GrouperModelContent)

def test_extra_grouping_field_fixed(self):
"""Extra grouping fields are retrieved correctly"""
with force_language("en"):
expected_language = "zh"
self.admin.language = expected_language

admin_language = self.admin.get_language()
current_content_filters = self.admin.current_content_filters

self.assertEqual(admin_language, expected_language)
self.assertEqual(current_content_filters["language"], expected_language)

def test_extra_grouping_field_current(self):
"""Extra grouping fields (language) when not set return current default correctly"""
del self.admin.language # No pre-set language
expected_language = get_language()

admin_language = self.admin.get_language()
current_content_filters = self.admin.current_content_filters

self.assertEqual(admin_language, expected_language)
self.assertEqual(current_content_filters["language"], expected_language)


class GrouperChangeListTestCase(SetupMixin, CMSTestCase):
def test_language_selector(self):
Expand Down
5 changes: 4 additions & 1 deletion cms/tests/test_log_entries.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from cms.models import Page, Placeholder, UserSettings
from cms.test_utils.testcases import URL_CMS_PAGE_MOVE, CMSTestCase
from cms.utils import get_current_site
from cms.utils.i18n import force_language
from cms.wizards.forms import WizardStep2BaseForm, step2_form_factory

# Snippet to create wizard page taken from: test_wizards.py
Expand Down Expand Up @@ -214,6 +215,8 @@ def test_log_for_delete_translation(self):
title_en = "page_a"
page = create_page(title_en, "nav_playground.html", "en")
create_page_content(language='de', title="other title %s" % title_en, page=page)
with force_language("de"): # The remaining language
expected_entry = str(page)
endpoint = self.get_page_delete_translation_uri('en', page)
post_data = {'post': 'yes', 'language': 'en'}

Expand All @@ -233,7 +236,7 @@ def test_log_for_delete_translation(self):
# Check the object id is set correctly
self.assertEqual(str(page.pk), log_entry.object_id)
# Check the object_repr is set correctly
self.assertEqual(str(page), log_entry.object_repr)
self.assertEqual(expected_entry, log_entry.object_repr)
# Check that the correct user created the log
self.assertEqual(self._admin_user.pk, log_entry.user_id)

Expand Down
4 changes: 2 additions & 2 deletions cms/utils/placeholder.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from sekizai.helpers import get_varname

from cms.exceptions import DuplicatePlaceholderWarning
from cms.models import Placeholder
from cms.models import EmptyPageContent, Placeholder
from cms.utils.conf import get_cms_setting

RANGE_START = 128
Expand Down Expand Up @@ -407,7 +407,7 @@ def rescan_placeholders_for_obj(obj):
return existing


def get_declared_placeholders_for_obj(obj: Union[models.Model, None]) -> list[Placeholder]:
def get_declared_placeholders_for_obj(obj: Union[models.Model, EmptyPageContent, None]) -> list[Placeholder]:
"""Returns declared placeholders for an object. The object is supposed to either have a method
``get_placeholder_slots`` which returns the list of placeholders or a method ``get_template``
which returns the template path as a string that renders the object. ``get_declared_placeholders`` returns
Expand Down
0