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

fsbraun · Github Release Action · vinitkumar · web-flow · commit e1af99804564 · 2024-11-06T22:55:29.000+01:00
* Fix: GrouperModelAdmin raised an AttributeError if used outside the admin app

* Fixed: Page.__str__ returns page title with admin_manager

* feat: Format `Page.__str__` as "My title (/path/to/page/)"

* Update tests for new str method of Page

* Simplify str

---------

Co-authored-by: Github Release Action &lt;info@django-cms.org&gt;
Co-authored-by: Vinit Kumar &lt;mail@vinitkumar.me&gt;
diff --git a/cms/admin/pageadmin.py b/cms/admin/pageadmin.py
@@ -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
diff --git a/cms/admin/utils.py b/cms/admin/utils.py
@@ -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(
diff --git a/cms/forms/validators.py b/cms/forms/validators.py
@@ -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))
diff --git a/cms/models/pagemodel.py b/cms/models/pagemodel.py
@@ -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))}>'
@@ -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)
 
diff --git a/cms/tests/test_admin.py b/cms/tests/test_admin.py
@@ -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)
 
diff --git a/cms/tests/test_grouper_admin.py b/cms/tests/test_grouper_admin.py
@@ -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 (
@@ -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):
diff --git a/cms/tests/test_log_entries.py b/cms/tests/test_log_entries.py
@@ -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
@@ -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'}
 
@@ -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)
 
diff --git a/cms/utils/placeholder.py b/cms/utils/placeholder.py
@@ -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
@@ -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
