diff --git a/docs/source/settings_reference.rst b/docs/source/settings_reference.rst index 7fd277fb..87bde22e 100644 --- a/docs/source/settings_reference.rst +++ b/docs/source/settings_reference.rst @@ -392,3 +392,43 @@ Use this to specify the 'depth' value of a project's 'section root' pages. For m Default value: ``False`` By default, menu items linking to custom URLs are attributed with the 'active' class only if their ``link_url`` value matches the path of the current request _exactly_. Setting this to `True` in your project's settings will enable a smarter approach to active class attribution for custom URLs, where only the 'path' part of the ``link_url`` value is used to determine what active class should be used. The new approach will also attribute the 'ancestor' class to menu items if the ``link_url`` looks like an ancestor of the current request URL. + + +.. _LOCALIZE_MENU_ITEMS: + +``WAGTAILMENUS_LOCALIZE_MENU_ITEMS`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Default value: ``False`` (or ``True`` when ``WAGTAIL_I18N_ENABLED = True``) + +When enabled, wagtailmenus will resolve each menu item's ``link_page`` to its +translation in the currently active locale at render time, so that menu item +titles and URLs automatically reflect the visitor's language without any +additional configuration. + +If ``WAGTAIL_I18N_ENABLED`` is set to ``True`` in your project's settings, +this feature is **enabled automatically** — you do not need to set +``WAGTAILMENUS_LOCALIZE_MENU_ITEMS`` explicitly. You can still opt out by +setting it to ``False`` even when ``WAGTAIL_I18N_ENABLED`` is ``True``. + +.. code-block:: python + + # e.g. settings/base.py + + # Opt in explicitly (not required if WAGTAIL_I18N_ENABLED = True): + WAGTAILMENUS_LOCALIZE_MENU_ITEMS = True + + # Opt out even when WAGTAIL_I18N_ENABLED = True: + WAGTAILMENUS_LOCALIZE_MENU_ITEMS = False + +When active, menu items stored against a default-locale page will be displayed +using the equivalent page in the active locale (if a translation exists). +If no translation is found for the active locale, the original page is used as +a fallback. The localization is applied **at render time only** — the stored +``link_page`` foreign key on the menu item is never modified. + +.. NOTE:: + Menu items are configured once, against the default-locale page. For this feature to do + anything, your pages need translated versions. `wagtail-localize + `_ is the usual way to produce them, but any + Wagtail translation workflow that creates linked ``Locale`` copies will work. diff --git a/wagtailmenus/conf/defaults.py b/wagtailmenus/conf/defaults.py index a86746e6..b2893a34 100644 --- a/wagtailmenus/conf/defaults.py +++ b/wagtailmenus/conf/defaults.py @@ -79,6 +79,8 @@ ACTIVE_ANCESTOR_CLASS = 'ancestor' +LOCALIZE_MENU_ITEMS = False + PAGE_FIELD_FOR_MENU_ITEM_TEXT = 'title' SECTION_ROOT_DEPTH = 3 diff --git a/wagtailmenus/conf/settings.py b/wagtailmenus/conf/settings.py index 0433a921..b8d82e2c 100644 --- a/wagtailmenus/conf/settings.py +++ b/wagtailmenus/conf/settings.py @@ -1,10 +1,19 @@ import sys +from django.conf import settings as django_settings from cogwheels import BaseAppSettingsHelper class WagtailmenusSettingsHelper(BaseAppSettingsHelper): deprecations = () + def __getattr__(self, name): + value = super().__getattr__(name) + # Auto-detect locale-awareness from Wagtail's own i18n flag when the + # wagtailmenus-specific setting has not been explicitly overridden. + if name == 'LOCALIZE_MENU_ITEMS' and not self.is_overridden('LOCALIZE_MENU_ITEMS'): + return bool(getattr(django_settings, 'WAGTAIL_I18N_ENABLED', False)) + return value + sys.modules[__name__] = WagtailmenusSettingsHelper() diff --git a/wagtailmenus/models/menus.py b/wagtailmenus/models/menus.py index 505dcb45..85912f60 100644 --- a/wagtailmenus/models/menus.py +++ b/wagtailmenus/models/menus.py @@ -287,6 +287,21 @@ def pages_for_display(self): # using OrderedDict to preserve ordering in Python < 3.6 return OrderedDict((p.id, p) for p in self.get_pages_for_display()) + @cached_property + def localize_page_map(self): + """Mapping of stored (default-locale) page IDs to their active-locale + page counterparts. Empty dict when LOCALIZE_MENU_ITEMS is disabled.""" + if not settings.LOCALIZE_MENU_ITEMS: + return {} + menu_items = getattr(self, '_raw_menu_items', None) or self.get_base_menuitem_queryset() + mapping = {} + for item in menu_items: + if item.link_page: + localized = item.link_page.localized + if localized and localized.pk != item.link_page.pk: + mapping[item.link_page.pk] = localized + return mapping + def get_page_children_dict(self, page_qs=None): """ Returns a dictionary of lists, where the keys are 'path' values for @@ -963,6 +978,9 @@ def get_top_level_items(self): # allow this query result to be reused by get_pages_for_display() self._raw_menu_items = menu_items + pages = self.pages_for_display + localize_map = self.localize_page_map + top_level_items = [] for item in menu_items: @@ -971,10 +989,14 @@ def get_top_level_items(self): top_level_items.append(item) continue + # Translate the stored (default-locale) FK to the active-locale page + # when localization is active, without mutating item.link_page_id. + effective_page = localize_map.get(item.link_page_id, item.link_page) + # But, we only want to include links to pages if the page was # in the get_pages_for_display() result try: - item.link_page = self.pages_for_display[item.link_page_id] + item.link_page = pages[effective_page.pk] top_level_items.append(item) except KeyError: continue @@ -997,21 +1019,32 @@ def get_pages_for_display(self): queryset = Page.objects.none() for item in (item for item in menu_items if item.link_page): + effective_page = self.localize_page_map.get(item.link_page.pk, item.link_page) + if( item.allow_subnav and - item.link_page.depth >= settings.SECTION_ROOT_DEPTH + effective_page.depth >= settings.SECTION_ROOT_DEPTH ): # Add this branch to the overall `queryset` queryset = queryset | Page.objects.filter( - path__startswith=item.link_page.path, - depth__lt=item.link_page.depth + self.max_levels, + path__startswith=effective_page.path, + depth__lt=effective_page.depth + self.max_levels, ) else: # Add this page only to the overall `queryset` - queryset = queryset | Page.objects.filter(id=item.link_page_id) - - # Filter out pages unsutable display - queryset = self.get_base_page_queryset() & queryset + queryset = queryset | Page.objects.filter(id=effective_page.pk) + + if settings.LOCALIZE_MENU_ITEMS: + # `queryset` already contains the localized pages. + # Apply the same live/show_in_menus/expired constraints as + # get_base_page_queryset() but only against the small set of pages + # in `queryset` (scales with menu size, not site size). + queryset = self.get_base_page_queryset().filter( + id__in=queryset.values('id') + ) + else: + # Filter out pages unsuitable for display + queryset = self.get_base_page_queryset() & queryset # Always return 'specific' page instances return queryset.specific() diff --git a/wagtailmenus/tests/test_localization.py b/wagtailmenus/tests/test_localization.py new file mode 100644 index 00000000..f9b8e89e --- /dev/null +++ b/wagtailmenus/tests/test_localization.py @@ -0,0 +1,321 @@ +"""Tests for the LOCALIZE_MENU_ITEMS feature. + +Verifies that when LOCALIZE_MENU_ITEMS is enabled, menu items resolve +to the active-locale page at render time.""" + +from unittest.mock import patch + +from django.test import TestCase, override_settings +from wagtail.models import Page + +from wagtailmenus.conf import settings +from wagtailmenus.models import MainMenu + + +class _LocalizedMapping: + """ + Descriptor that replaces ``Page.localized`` for the duration of a test. + + ``mapping`` is a dict of {original_page_id: localized_page}. Any page + whose id is not in the mapping simply returns itself (i.e. already in + the active locale). + """ + + def __init__(self, mapping): + self.mapping = mapping + + def __get__(self, obj, objtype=None): + if obj is None: + return self + return self.mapping.get(obj.pk, obj) + + +class TestLocalizationSetting(TestCase): + """LOCALIZE_MENU_ITEMS setting defaults and auto-detection.""" + + def test_localize_menu_items_disabled_by_default_and_explicit_false(self): + """LOCALIZE_MENU_ITEMS is False by default and stays False when explicitly set False.""" + self.assertFalse(settings.LOCALIZE_MENU_ITEMS) + with override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=False): + self.assertFalse(settings.LOCALIZE_MENU_ITEMS) + + @override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=True) + def test_explicit_override_enables_feature(self): + self.assertTrue(settings.LOCALIZE_MENU_ITEMS) + + @override_settings(WAGTAIL_I18N_ENABLED=True) + def test_auto_detection_from_wagtail_i18n_enabled(self): + self.assertTrue(settings.LOCALIZE_MENU_ITEMS) + + @override_settings(WAGTAIL_I18N_ENABLED=True, WAGTAILMENUS_LOCALIZE_MENU_ITEMS=False) + def test_explicit_false_overrides_wagtail_i18n_enabled(self): + """An explicit opt-out must not be overridden by WAGTAIL_I18N_ENABLED.""" + self.assertFalse(settings.LOCALIZE_MENU_ITEMS) + + @override_settings(WAGTAIL_I18N_ENABLED=False) + def test_auto_detection_returns_false_when_wagtail_i18n_disabled(self): + self.assertFalse(settings.LOCALIZE_MENU_ITEMS) + + +class TestMenuItemLocalizationRenderTime(TestCase): + """Localization is applied at render time.""" + + fixtures = ['test.json'] + + def test_top_level_items_link_page_unchanged_when_disabled(self): + """When the feature is off, link_page is whatever is stored.""" + menu = MainMenu.objects.get(pk=1) + first_item = menu.get_menu_items_manager().filter( + link_page__isnull=False + ).first() + item_pk = first_item.pk + original_page_pk = first_item.link_page_id + + items = menu.get_top_level_items() + + matched = [i for i in items if getattr(i, 'pk', None) == item_pk] + self.assertTrue(len(matched) > 0) + self.assertEqual(matched[0].link_page.pk, original_page_pk) + + @override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=True) + def test_top_level_items_link_page_swapped_when_enabled(self): + """When localization is on, item.link_page is set to the localized page.""" + menu = MainMenu.objects.get(pk=1) + first_item = menu.get_menu_items_manager().filter( + link_page__isnull=False + ).first() + item_pk = first_item.pk # stable menu item pk, not the page FK + en_page = first_item.link_page + it_page = Page.objects.exclude(pk=en_page.pk).filter( + live=True, expired=False, show_in_menus=True + ).first() + + mapping = _LocalizedMapping({en_page.pk: it_page}) + with patch.object(Page, 'localized', mapping): + items = menu.get_top_level_items() + + matched = [i for i in items if getattr(i, 'pk', None) == item_pk] + self.assertTrue(len(matched) > 0) + self.assertEqual(matched[0].link_page.pk, it_page.pk) + + @override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=True) + def test_stored_link_page_id_not_mutated_by_render(self): + """The FK column (link_page_id) must not change after get_top_level_items().""" + menu = MainMenu.objects.get(pk=1) + first_item = menu.get_menu_items_manager().filter( + link_page__isnull=False + ).first() + en_page = first_item.link_page + it_page = Page.objects.exclude(pk=en_page.pk).filter( + live=True, expired=False, show_in_menus=True + ).first() + + mapping = _LocalizedMapping({en_page.pk: it_page}) + with patch.object(Page, 'localized', mapping): + menu.get_top_level_items() + + # Re-read the item from DB: the FK must still be the original en page + stored = menu.get_menu_items_manager().get(pk=first_item.pk) + self.assertEqual(stored.link_page_id, en_page.pk) + + @override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=True) + def test_noop_when_already_in_active_locale(self): + """When localized returns the same page (already active locale), + both top_level_items and pages_for_display are unchanged.""" + menu = MainMenu.objects.get(pk=1) + mapping = _LocalizedMapping({}) + with patch.object(Page, 'localized', mapping): + items = menu.get_top_level_items() + pages = menu.get_pages_for_display() + self.assertEqual(len(items), 5) + self.assertEqual(len(pages), 12) + + @override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=True) + def test_top_level_items_no_op_when_link_page_is_none(self): + """Custom URL items (no link_page) are always included unchanged.""" + menu = MainMenu.objects.get(pk=1) + mapping = _LocalizedMapping({}) + with patch.object(Page, 'localized', mapping): + items = menu.get_top_level_items() + url_items = [i for i in items if not getattr(i, 'link_page_id', None)] + # All URL-only items must still be present + for item in url_items: + self.assertIsNone(item.link_page) + + +class TestGetPagesForDisplayLocalization(TestCase): + """ + get_pages_for_display() returns localized pages when the feature is on. + """ + + fixtures = ['test.json'] + + def _get_menu_and_first_item_pages(self): + """Return (menu, en_page, it_page) where it_page != en_page.""" + menu = MainMenu.objects.get(pk=1) + # Pick the first item that has a link_page + first_item = menu.get_menu_items_manager().filter( + link_page__isnull=False + ).first() + en_page = first_item.link_page + # Use any *other* live page as the fake localized counterpart + it_page = Page.objects.exclude(pk=en_page.pk).filter( + live=True, expired=False, show_in_menus=True + ).first() + return menu, en_page, it_page + + def test_pages_for_display_unchanged_when_disabled(self): + menu = MainMenu.objects.get(pk=1) + # The fixture has 12 pages for this menu + self.assertEqual(len(menu.pages_for_display), 12) + + @override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=True) + def test_pages_for_display_includes_localized_page(self): + menu, en_page, it_page = self._get_menu_and_first_item_pages() + + # Map: en_page → it_page; everything else stays the same + mapping = _LocalizedMapping({en_page.pk: it_page}) + + with patch.object(Page, 'localized', mapping): + # Bypass the @cached_property so we get a fresh result + pages = menu.get_pages_for_display() + page_ids = {p.id for p in pages} + + self.assertIn(it_page.pk, page_ids) + + @override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=True) + def test_pages_for_display_excludes_original_when_localized_differs(self): + menu, en_page, it_page = self._get_menu_and_first_item_pages() + + # Only the single-page (no allow_subnav) case for a clean assertion: + # find an item where allow_subnav is False + item = menu.get_menu_items_manager().filter( + link_page__isnull=False, allow_subnav=False + ).first() + if item is None: + self.skipTest("No allow_subnav=False item in fixture") + + en_page = item.link_page + # Pick a page not otherwise in the menu as the fake translation + menu_page_ids = set( + menu.get_menu_items_manager().values_list('link_page_id', flat=True) + ) + it_page = Page.objects.filter( + live=True, expired=False, show_in_menus=True + ).exclude(pk__in=menu_page_ids).first() + if it_page is None: + self.skipTest("Not enough pages in fixture for this test") + + mapping = _LocalizedMapping({en_page.pk: it_page}) + + with patch.object(Page, 'localized', mapping): + pages = menu.get_pages_for_display() + page_ids = {p.id for p in pages} + + # Original en_page should NOT appear; localized it_page SHOULD + self.assertNotIn(en_page.pk, page_ids) + self.assertIn(it_page.pk, page_ids) + + +class TestLocalizationRegressionWhenDisabled(TestCase): + """ + The full top_level_items / pages_for_display pipeline must behave + identically to before when LOCALIZE_MENU_ITEMS is False (the default). + """ + + fixtures = ['test.json'] + + def test_top_level_items_count_unchanged(self): + menu = MainMenu.objects.get(pk=1) + # Fixture has 6 menu items, one of which links to show_in_menus=False + # so 5 top-level items are returned + self.assertEqual(len(menu.top_level_items), 5) + + def test_pages_for_display_count_unchanged(self): + menu = MainMenu.objects.get(pk=1) + self.assertEqual(len(menu.pages_for_display), 12) + + def test_pages_for_display_all_live_and_show_in_menus(self): + menu = MainMenu.objects.get(pk=1) + for page in menu.pages_for_display.values(): + self.assertTrue(page.live) + self.assertFalse(page.expired) + self.assertTrue(page.show_in_menus) + + +class TestSubtreeLocalization(TestCase): + """Verify subtree localization: when a parent page is localized to a + different page with children, pages_for_display() includes the localized + subtree, not the original one.""" + + fixtures = ['test.json'] + + @override_settings(WAGTAILMENUS_LOCALIZE_MENU_ITEMS=True) + def test_subtree_includes_localized_parent_and_children(self): + """When a parent page is mapped to a different localized page with + children, pages_for_display() must include the localized parent + and its descendants, but exclude the original parent.""" + menu = MainMenu.objects.get(pk=1) + + # Find a parent page that has allow_subnav and children in the fixture + # We need a page at depth >= SECTION_ROOT_DEPTH (3) with children + # that has a menu item with allow_subnav=True + item = menu.get_menu_items_manager().filter( + link_page__isnull=False, + allow_subnav=True, + link_page__depth__gte=3, # SECTION_ROOT_DEPTH + ).first() + + if item is None: + self.skipTest("No menu item with allow_subnav and depth>=3 in fixture") + + en_page = item.link_page + en_page_children = Page.objects.filter( + path__startswith=en_page.path, + depth__gt=en_page.depth, + depth__lt=en_page.depth + menu.max_levels, + live=True, expired=False, show_in_menus=True, + ) + if not en_page_children.exists(): + self.skipTest("Parent page has no children in fixture") + + # Find a localized counterpart page with its own children + # Pick a different page at the same depth with children as the "localized" version + localized_page = Page.objects.filter( + depth=en_page.depth, + live=True, expired=False, show_in_menus=True, + ).exclude(pk=en_page.pk).first() + if localized_page is None: + self.skipTest("No alternative page at same depth in fixture") + + localized_children = Page.objects.filter( + path__startswith=localized_page.path, + depth__gt=localized_page.depth, + depth__lt=localized_page.depth + menu.max_levels, + live=True, expired=False, show_in_menus=True, + ) + if not localized_children.exists(): + self.skipTest("Localized page has no children in fixture") + + # Build a mapping: en_page → localized_page + mapping = _LocalizedMapping({en_page.pk: localized_page}) + + with patch.object(Page, 'localized', mapping): + pages = menu.get_pages_for_display() + page_ids = {p.id for p in pages} + + # The localized page and its children SHOULD be in pages_for_display + self.assertIn(localized_page.pk, page_ids, + "Localized parent page should be in pages_for_display") + for child in localized_children: + self.assertIn(child.pk, page_ids, + f"Child page {child.pk} of localized parent should be in pages_for_display") + + # The original en_page should NOT be in pages_for_display + # (unless it's also linked from another menu item — we skip in that case) + other_items_link_to_en = menu.get_menu_items_manager().filter( + link_page_id=en_page.pk + ).exclude(pk=item.pk).exists() + if not other_items_link_to_en: + self.assertNotIn(en_page.pk, page_ids, + "Original page should not be in pages_for_display when localized")