fix: Efficient build menu for versioned and unversioned pages (#7807)

* Update cms menus to allow use cms 4.1 features

* Fix: Menus are only cached for preview and live urls (not edit)

* Update tests

* fix tests

* Fix fallback

* Fix get live url test

* Fix surprising lint error in cms.models.pagemodel

* Small refactor

* Wording improvement

* Fix: Remove redundant calculation of absolute URL

cms/cms_menus.py

@@ -8,6 +8,7 @@
 from cms import constants
 from cms.apphook_pool import apphook_pool
 from cms.models import EmptyPageContent, PageContent, PageUrl
+from cms.toolbar.utils import get_object_preview_url, get_toolbar_from_request
 from cms.utils.conf import get_cms_setting
 from cms.utils.i18n import (
     get_fallback_languages,
@@ -28,8 +29,8 @@ def get_visible_nodes(request, pages, site):
     `pages` contains all published pages.
     """
     user = request.user
-    public_for = get_cms_setting('PUBLIC_FOR')
-    can_see_unrestricted = public_for == 'all' or (public_for == 'staff' and user.is_staff)
+    public_for = get_cms_setting("PUBLIC_FOR")
+    can_see_unrestricted = public_for == "all" or (public_for == "staff" and user.is_staff)
 
     if not user.is_authenticated and not can_see_unrestricted:
         # User is not authenticated and can't see unrestricted pages,
@@ -48,7 +49,7 @@ def get_visible_nodes(request, pages, site):
         return list(pages) if can_see_unrestricted else []
 
     user_id = user.pk
-    user_groups = SimpleLazyObject(lambda: frozenset(user.groups.values_list('pk', flat=True)))
+    user_groups = SimpleLazyObject(lambda: frozenset(user.groups.values_list("pk", flat=True)))
     is_auth_user = user.is_authenticated
 
     def user_can_see_page(page):
@@ -69,7 +70,7 @@ def user_can_see_page(page):
     return [page for page in pages if user_can_see_page(page)]
 
 
-def get_menu_node_for_page(renderer, page, language, fallbacks=None):
+def get_menu_node_for_page(renderer, page, language, fallbacks=None, endpoint=False):
     """
     Transform a CMS page into a navigation node.
 
@@ -78,7 +79,7 @@ def get_menu_node_for_page(renderer, page, language, fallbacks=None):
         page: The page to transform.
         language: The current language used to render the menu.
         fallbacks: List of fallback languages (optional).
-
+        url: The URL to use for the node (optional) instead of page_content.get_absolute_url().
     Returns:
         A CMSNavigationNode instance.
     """
@@ -88,21 +89,21 @@ def get_menu_node_for_page(renderer, page, language, fallbacks=None):
     # These are simple to port over, since they are not calculated.
     # Other attributes will be added conditionally later.
     attr = {
-        'is_page': True,
-        'soft_root': page.get_soft_root(language),
-        'auth_required': page.login_required,
-        'reverse_id': page.reverse_id,
+        "is_page": True,
+        "soft_root": page.get_soft_root(language),
+        "auth_required": page.login_required,
+        "reverse_id": page.reverse_id,
     }
 
     limit_visibility_in_menu = page.get_limit_visibility_in_menu(language)
 
     if limit_visibility_in_menu is constants.VISIBILITY_ALL:
-        attr['visible_for_authenticated'] = True
-        attr['visible_for_anonymous'] = True
+        attr["visible_for_authenticated"] = True
+        attr["visible_for_anonymous"] = True
     else:
-        attr['visible_for_authenticated'] = limit_visibility_in_menu == constants.VISIBILITY_USERS
-        attr['visible_for_anonymous'] = limit_visibility_in_menu == constants.VISIBILITY_ANONYMOUS
-    attr['is_home'] = page.is_home
+        attr["visible_for_authenticated"] = limit_visibility_in_menu == constants.VISIBILITY_USERS
+        attr["visible_for_anonymous"] = limit_visibility_in_menu == constants.VISIBILITY_ANONYMOUS
+    attr["is_home"] = page.is_home
     # Extenders can be either navigation extenders or from apphooks.
     extenders = []
     if page.navigation_extenders:
@@ -126,35 +127,38 @@ def get_menu_node_for_page(renderer, page, language, fallbacks=None):
             # CMSAttachMenus are treated a bit differently to allow them to be
             # able to be attached to multiple points in the navigation.
             exts.append(f"{ext.__name__}:{page.pk}")
-        elif hasattr(ext, '__name__'):
+        elif hasattr(ext, "__name__"):
             exts.append(ext.__name__)
         else:
             exts.append(ext)
     if exts:
-        attr['navigation_extenders'] = exts
+        attr["navigation_extenders"] = exts
 
     for lang in [language] + fallbacks:
-        translation = page.page_content_cache[lang]
+        translation = page.page_content_cache.get(lang)
 
         if translation:
             page_url = page.urls_cache[lang]
             # Do we have a redirectURL?
-            attr['redirect_url'] = translation.redirect  # save redirect URL if any
+            attr["redirect_url"] = translation.redirect  # save redirect URL if any
 
             # Now finally, build the NavigationNode object and return it.
             # The parent_id is manually set by the menu get_nodes method.
+            if endpoint:
+                url = get_object_preview_url(translation)
+            else:
+                url = translation.get_absolute_url()
             ret_node = CMSNavigationNode(
                 title=translation.menu_title or translation.title,
-                url='',
+                url=url,
                 id=page.pk,
                 attr=attr,
                 visible=page.get_in_navigation(translation.language),
                 path=page_url.path or page_url.slug,
                 language=(translation.language if translation.language != language else None),
             )
             return ret_node
-    else:
-        raise RuntimeError('Unable to render cms menu. There is a language misconfiguration.')
+    return None
 
 
 class CMSNavigationNode(NavigationNode):
@@ -188,25 +192,17 @@ def is_selected(self, request):
             return False
         return page_id == self.id
 
-    def _get_absolute_url(self):
-        if self.attr['is_home']:
-            return reverse('pages-root')
-        return reverse('pages-details-by-slug', kwargs={"slug": self.path})
-
-    def get_absolute_url(self):
-        if self.language:
-            with force_language(self.language):
-                return self._get_absolute_url()
-        return self._get_absolute_url()
-
 
 class CMSMenu(Menu):
     """Subclass of :class:`menus.base.Menu`. Its :meth:`~menus.base.Menu.get_nodes()` creates a list of NavigationNodes
     based on a site's :class:`cms.models.pagemodel.Page` objects.
     """
+
     def get_nodes(self, request):
         site = self.renderer.site
         lang = self.renderer.request_language
+        toolbar = get_toolbar_from_request(request)
+
         pages = get_page_queryset(site)
 
         if is_valid_site_language(lang, site_id=site.pk):
@@ -231,10 +227,9 @@ def get_nodes(self, request):
             fallbacks = languages
 
         pages = (
-            pages
-            .filter(pagecontent_set__language__in=languages)
-            .select_related('node')
-            .order_by('node__path')
+            pages.filter(pagecontent_set__language__in=languages)
+            .select_related("node")
+            .order_by("node__path")
             .distinct()
         )
         pages = get_visible_nodes(request, pages, site)
@@ -248,22 +243,25 @@ def get_nodes(self, request):
             homepage = None
 
         urls_lookup = Prefetch(
-            'urls',
-            to_attr='filtered_urls',
+            "urls",
+            to_attr="filtered_urls",
             queryset=PageUrl.objects.filter(language__in=languages),
         )
+        if toolbar.edit_mode_active or toolbar.preview_mode_active:
+            # Get all translations visible in the admin for the current page
+            translations_qs = PageContent.admin_manager.current_content(language__in=languages)
+        else:
+            # Only get public translations
+            translations_qs = PageContent.objects.filter(language__in=languages)
         translations_lookup = Prefetch(
-            'pagecontent_set',
-            to_attr='filtered_translations',
-            queryset=PageContent.objects.filter(language__in=languages),
+            "pagecontent_set",
+            to_attr="filtered_translations",
+            queryset=translations_qs,
         )
         prefetch_related_objects(pages, urls_lookup, translations_lookup)
         # Build the blank title instances only once
         blank_page_content_cache = {language: EmptyPageContent(language=language) for language in languages}
 
-        if lang not in blank_page_content_cache:
-            blank_page_content_cache[lang] = EmptyPageContent(language=lang)
-
         # Maps a node id to its page id
         node_id_to_page = {}
 
@@ -283,6 +281,7 @@ def _page_to_node(page):
                 page,
                 language=lang,
                 fallbacks=fallbacks,
+                endpoint=toolbar.preview_mode_active or toolbar.edit_mode_active,
             )
             return menu_node
 
@@ -298,24 +297,25 @@ def _page_to_node(page):
                 continue
 
             menu_node = _page_to_node(page)
-            cut_homepage = homepage and not homepage.get_in_navigation(lang)
-
-            if cut_homepage and parent_id == homepage.pk:
-                # When the homepage is hidden from navigation,
-                # we need to cut all its direct children from it.
-                menu_node.parent_id = None
-            else:
-                menu_node.parent_id = parent_id
-            node_id_to_page[node.pk] = page.pk
-            menu_nodes.append(menu_node)
+            if menu_node:
+                # Only add pages with at least one page content
+                cut_homepage = homepage and not homepage.get_in_navigation(lang)
+
+                if cut_homepage and parent_id == homepage.pk:
+                    # When the homepage is hidden from navigation,
+                    # we need to cut all its direct children from it.
+                    menu_node.parent_id = None
+                else:
+                    menu_node.parent_id = parent_id
+                node_id_to_page[node.pk] = page.pk
+                menu_nodes.append(menu_node)
         return menu_nodes
 
 
 menu_pool.register_menu(CMSMenu)
 
 
 class NavExtender(Modifier):
-
     def modify(self, request, nodes, namespace, root_id, post_cut, breadcrumb):
         if post_cut:
             return nodes
@@ -347,8 +347,7 @@ def modify(self, request, nodes, namespace, root_id, post_cut, breadcrumb):
 
         # find all not assigned nodes
         for menu in self.renderer.menus.items():
-            if (hasattr(menu[1], 'cms_enabled')
-                    and menu[1].cms_enabled and menu[0] not in exts):
+            if hasattr(menu[1], "cms_enabled") and menu[1].cms_enabled and menu[0] not in exts:
                 for node in nodes:
                     if node.namespace == menu[0]:
                         removed.append(node)
@@ -484,8 +483,7 @@ def find_ancestors_and_remove_children(self, node, nodes):
                 node.parent.parent = None
                 nodes = [node.parent] + nodes
             else:
-                nodes = self.find_ancestors_and_remove_children(
-                    node.parent, nodes)
+                nodes = self.find_ancestors_and_remove_children(node.parent, nodes)
         else:
             for newnode in nodes:
                 if newnode != node and not newnode.parent:

cms/models/pagemodel.py

@@ -363,7 +363,7 @@ def get_absolute_url(self, language=None, fallback=True):
             if self.is_home:
                 return reverse('pages-root')
             path = self.get_path(language, fallback) or self.get_slug(language, fallback)  # TODO: Disallow get_slug
-            return reverse('pages-details-by-slug', kwargs={"slug": path})
+            return reverse('pages-details-by-slug', kwargs={"slug": path}) if path else None
 
     def set_tree_node(self, site, target=None, position='first-child'):
         assert position in ('last-child', 'first-child', 'left', 'right')

cms/tests/test_menu.py

@@ -425,7 +425,7 @@ def test_show_menu_num_queries(self):
             The queries should be:
                 get all page nodes
                 get all page permissions
-                get all titles
+                get all page contents
                 get all page urls
                 get the menu cache key
                 set the menu cache key

cms/toolbar/utils.py

@@ -5,6 +5,7 @@
 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,
@@ -126,7 +127,7 @@ def get_toolbar_from_request(request):
 
 def add_live_url_querystring_param(obj, url, language=None):
     """
-    Append a live url to a given Page url using a supplied url parameter configured
+    Append a live url to a given object url using a supplied url parameter configured
     by the setting: CMS_ENDPOINT_LIVE_URL_QUERYSTRING_PARAM
 
     :param obj: Placeholder source object
@@ -135,9 +136,12 @@ def add_live_url_querystring_param(obj, url, language=None):
     :returns: A url string
     """
     url_param = get_cms_setting('ENDPOINT_LIVE_URL_QUERYSTRING_PARAM')
-    if not isinstance(obj, PageContent):
+    if not hasattr(obj, "get_absolute_url"):
+        return url
+    try:
+        live_url = obj.get_absolute_url()
+    except NoReverseMatch:
         return url
-    live_url = obj.page.get_absolute_url(language=language)
     url_fragments = url.split('?')
     if len(url_fragments) > 1:
         url += f'&{url_param}={live_url}'

menus/menu_pool.py

@@ -310,9 +310,9 @@ def get_registered_modifiers(self):
         return self.modifiers
 
     def clear(self, site_id=None, language=None, all=False):
-        '''
+        """
         This invalidates the cache for a given menu (site_id and language)
-        '''
+        """
         if all:
             cache_keys = CacheKey.objects.get_keys()
         else: