Filter Menu Tree By Language

Thanks, I've committed this to 8.x-1.x as well.

It would be great if we could add a test for it, so I'm going to leave the issue open for now.

Aha, I didn't see that this patch relies on the Menu Manipulator module. I'm going to revert these commits for now, and take another look after reading through #2466553: Untranslated menu items are displayed in menus.

With the patch from #2466553-75: Untranslated menu items are displayed in menus, the needed patch for sitemap module seems to be this one, it works on my website.

#11 would be for 8.x-1.x.

#9 is a great solution if using Menu Manipulator.

The core issue (#2466553: Untranslated menu items are displayed in menus) on its own doesn't appear to be enough yet to fix this.

This sounds more like a feature request than a task to me.

It would be good to know if this is still an issue on Drupal 10, running the latest sitemap version.

(I'm cleaning up old issues as part of a big review of open issues for #3425146: [Plan] Stable 8.x-2.0 release: thank you for your patience with me)

Hello,

yes this is still an issue on Drupal 10.2.X with the 8.x-2.0-beta6 version.
I attach new patch that fix this issue.

Moving to "Needs review" so I remember to review the new patch

The patch in #14 introduces an implicit dependency on the menu_manipulator module, which isn't ideal, so I can't merge patch as-is (see below for more information about why).

That being said, I see a way to write this patch so that menu_manipulator is an optional dependency...

diff --git a/src/Plugin/Sitemap/Menu.php b/src/Plugin/Sitemap/Menu.php
index 312b13d..5dff063 100644
--- a/src/Plugin/Sitemap/Menu.php
+++ b/src/Plugin/Sitemap/Menu.php
@@ -102,6 +102,14 @@ class Menu extends SitemapBase {
       ['callable' => 'menu.default_tree_manipulators:checkAccess'],
       ['callable' => 'menu.default_tree_manipulators:generateIndexAndSort'],
     ];
+
+    // If the menu_manipulator module is installed, use its filterLanguage
+    // manipulator to ensure that only menu items from the current language are
+    // displayed.
+    if ($this->moduleHandler->moduleExists('menu_manipulator')) {
+      $manipulators[] = ['callable' => 'menu.language_tree_manipulator:filterLanguage'];
+    }
+
     $tree = $this->menuLinkTree->transform($tree, $manipulators);
 
     // Add an alter hook so that other modules can manipulate the

... but — as mentioned before — I don't want to merge anything that doesn't include tests, otherwise we won't know if future changes to the sitemap module will break your website!

To write tests for this feature, start by adding menu_manipulator as a test dependency: this ensures that it will be available in the test environment...

diff --git a/sitemap.info.yml b/sitemap.info.yml
index 7fb4bb0..a2c7150 100644
--- a/sitemap.info.yml
+++ b/sitemap.info.yml
@@ -3,3 +3,5 @@ type: module
 description: 'Display a sitemap.'
 core_version_requirement: ^9.1 || ^10 || ^11
 configure: sitemap.settings
+test_dependencies:
+  - menu_manipulator:menu_manipulator  (>=3.0.6)

Then add a Functional test for the functionality. It will look something like the following...

<?php

namespace Drupal\Tests\sitemap\Functional;

use Drupal\Tests\language\Traits\LanguageTestTrait;

/**
 * Test menu_manipulator only shows items from the current language on sitemap.
 *
 * @group sitemap
 */
class SitemapMultilingualMenuTest extends SitemapBrowserTestBase {
  use LanguageTestTrait;

  /**
   * {@inheritdoc}
   */
  protected static $modules = [
    'content_translation',
    'menu_manipulator',
    'menu_ui',
    'node',
    'sitemap',
  ];

  /**
   * Test menu_manipulator only shows items from the current language on sitemap.
   */
  public function testMultilingualMenu() {
    // Set up languages. Note English should already be available.
    // TODO: Might have to set up language negotiation?
    self::createLanguageFromLangcode('fr');

    // TODO: Make menu_link_content translatable? (not sure if necessary)
    self::enableBundleTranslation('menu_link_content', 'menu_link_content');

    // TODO: Make nodes translatable? (not sure if necessary) Note 'page' type should already be available.
    self::enableBundleTranslation('node', 'page');

    // Log in as an admin user.
    $this->drupalLogin($this->drupalCreateUser([
      'administer sitemap',
      'administer menu',
      'administer nodes',
      'create page content',
    ]));

    // Configure sitemap to show main menu.
    $this->saveSitemapForm(['plugins[menu:main][enabled]' => TRUE]);

    $englishLinkText = 'EN' . $this->randomString();
    // TODO: Add an English-only menu-item with distinctive link text to the main menu.

    $frenchLinkText = 'FR' . $this->randomString();
    // TODO: Add a French-only menu-item with distinctive link text to the main menu.

    // Switch to a user with minimal privileges.
    $this->drupalLogout();
    $this->drupalLogin($this->drupalCreateUser([
      'access content',
      'access sitemap',
    ]));

    // Visit the sitemap in English. Check that the English-only item is
    // displayed on the page. Check that the French-only item is NOT displayed
    // on the page.
    $this->drupalGet('/en/sitemap');
    $this->assertSession()->linkExists($englishLinkText);
    $this->assertSession()->linkNotExists($frenchLinkText);

    // Visit the sitemap in French. Check that the English-only item is NOT
    // displayed on the page. Check that the French-only item is displayed on
    // the page.
    $this->drupalGet('/fr/sitemap');
    $this->assertSession()->linkNotExists($englishLinkText);
    $this->assertSession()->linkExists($frenchLinkText);
  }

}

... you'll have to fix the TODOs, and test it to make sure that the tests pass with the patch in place.

Aside, in the interest of being transparent, I considered several paths forward:

1. Rejecting this patch (I didn't seriously consider this, because that would not help the people reporting this issue)
2. Hoping that #2466553: Untranslated menu items are displayed in menus merges soon and solves the problem (2 years ago, @akalata said it would not, but 2466553 has changed quite a bit in the meantime)
3. Make sitemap explicitly depend on the menu_manipulator module (i.e.: by adding menu_manipulator to its dependencies in sitemap.info.yml)
   1. This means that everyone who uses sitemap would now have to download and install the menu_manipulator module if they want to use the Sitemap menu plugin... if they tried using the menu plugin without menu_manipulator, they would get a WSOD. Other sitemap module users might chafe at having to install a new module (for example, my client uses it only for taxonomy, and therefore wouldn't want to install another module to support a feature that they don't use)
   2. Introducing a new module dependency (i.e.: on the menu_manipulator module) would be okay in a major version release (i.e.: sitemap-8.x-2.0 -> sitemap-3.0.0), but not in a minor/patch release (i.e.: sitemap-8.x-2.0 -> sitemap-8.x-2.1). Also, a major version release is a major undertaking for me (a volunteer maintainer).
   3. Note this is different from adding it to the test_dependencies: test_dependencies are only used in the test environment, and it's okay to change those as-needed
4. Split the menu functionality into a sub-module that depends on menu_manipulator
   1. We've done this for the book module now that it has moved into contrib in D11 in #3454907: Drupal 11 compatibility fixes for sitemap
   2. There's also some discussion in #3445396: Compatibility with Domain Menus about doing this to add support for the domain_menus module features
   3. I think, again, sitemap users might chafe at having to install a module they didn't need before, because their site is only available in one language
5. Find a way to write this patch so that menu_manipulator is an optional dependency

I ultimately settled on the last option.

Adding "Needs tests" tag.

I've converted the patch in #14 into a merge request, and I made the changes I requested in the first part of #16, but, when I installed menu_manipulator-3.1.0 or menu_manipulator-4.0.0 (I tested both), when I go to /sitemap, I get a WSOD, and the error...

InvalidArgumentException: Class "menu.language_tree_manipulator" does not exist. in Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition() (line 32 of core/lib/Drupal/Core/DependencyInjection/ClassResolver.php).

... and I'm not sure where to go from here, because I don't think I understand the Steps to Reproduce!

Here's what I did to test...

I started by setting up a test environment.

1. Installed ddev-1.24.6
2. git clone --branch '8.x-2.x' https://git.drupalcode.org/project/sitemap.git && cd sitemap — cloned this module
3. git remote add sitemap-3085218 https://git.drupalcode.org/issue/sitemap-3085218.git && git fetch sitemap-3085218 && git checkout -b '3085218-filter-menu-tree-by-language' --track sitemap-3085218/'3085218-filter-menu-tree-by-language' — switched to this issue's branch
4. ddev config --project-type=drupal --docroot=web --php-version=8.3 --corepack-enable --project-name=sitemap — set up a ddev site for testing the module in isolation
5. ddev add-on get ddev/ddev-drupal-contrib — installed the very-handy DDEV Drupal Contrib add-on
6. ddev start
7. Edited composer.json, added "drupal/menu_manipulator": "^4" (later I tried ^3) to the require-dev section for testing purposes
8. ddev poser — run composer-install as per the ddev-drupal-contrib setup instructions
9. ddev symlink-project — link the module into place as per the ddev-drupal-contrib instructions
10. ddev drush -y si demo_umami — install Drupal with the Umami Food Magazine demo content — normally I use the minimal install profile, but I know the Umami demo is a multilingual site, so I figured it would be a good test case for this issue.
11. ddev drush -y en sitemap — enable the sitemap module
12. ddev drush -y uli — get a login link; pasted in browser

Then I set up the site...

12. I went to /en/admin/config/search/sitemap and enabled the Menu: Main navigation and left its configuration at the defaults.
13. I went to /en/admin/structure/menu/manage/main. I saw that the Menu language = English. It struck me that maybe you're intended to use entirely different menus in each language, but the issue summary and discussion in this ticket seemed to imply that there were menu items for more than one language in the same menu, so I went to figure out how to do that.
14. I checked the Translate menu tab, but I could only translate the title and administrative summary of the menu itself, i.e.: not the menu items.
15. Editing the items in the menu, I saw they were "special" (i.e.: managed programmatically), so I disabled them (i.e.: by unchecking them and clicking "Save"). I then re-created custom menu links to the same locations (i.e.: <front>, /en/articles, and /en/recipes).
16. While creating/editing my new custom menu links, I did not see a language selector, (nor a Translate primary action), so it was unclear to me how to get menu links from multiple languages in the same menu.
17. I know menu links are content entities, so I went to /en/admin/config/regional/content-language to check out the settings there. The Custom menu link entity-type was not set to be translatable, so I checked it to make it translatable. I left most configuration in the Custom menu link details element at the default values, but I unchecked "Changed" because it didn't seem like that needed to be translatable. I clicked Save configuration on that page.
18. I went back to /en/admin/structure/menu/manage/main and edited a custom menu link. Now I could see the language selector. I also saw a Translate tab.
19. To see what would happen, I used the Translate tab to add a Spanish translation for some of the menu links. Then I refreshed /en/sitemap, and /es/sitemap in a separate tab. To my surprise, I saw the English translations for the menu items on the English sitemap, and the Spanish translations for the menu items on the Spanish sitemap. It strikes me that making Custom Menu Links translatable, and adding translations for them might be a more-sustainable work-around for some sites. That being said, I'm aware of long-standing bug #2466553: Untranslated menu items are displayed in menus, and I have also worked on highly-localized sites where there completely different content in different languages, down to different menu structures and orders, so I'm aware this won't work for everyone.
20. I could now add menu items that were only in Spanish, and could see them on both the English and Spanish sitemaps.
21. I then went to /en/admin/modules and enabled the Menu Manipulator module.
22. Since I had already the new code in place (i.e.: from step 3), I decided to refresh /en/sitemap, and /es/sitemap to see if it made the untranslated menu links go away. This is when I got the InvalidArgumentException: Class "menu.language_tree_manipulator" does not exist error mentioned at the start of this comment.
23. I tried going to /en/admin/config/user-interface/menu-manipulator to see if there were any settings to change, but I got a different WSOD, ArgumentCountError: Too few arguments to function Drupal\Core\Form\ConfigFormBase::__construct(), 1 passed in /var/www/html/web/modules/contrib/menu_manipulator/src/Form/MenuManipulatorSettingsForm.php on line 88 and exactly 2 expected in Drupal\Core\Form\ConfigFormBase->__construct() (line 44 of core/lib/Drupal/Core/Form/ConfigFormBase.php).

Now I'm not sure what to do next!

Can someone experiencing this problem help me to figure out how to reproduce the error on a fresh site install?

For people who prefer patches, I've created a patch of the current state of the merge request.

(if you want to contribute code to this issue please contribute in the merge request: I'm only providing these patches as a convenience this time, to help me answer a question in another issue — thanks for your understanding)

Since I have asked for someone experiencing this problem help me to figure out how to reproduce the error on a fresh site install, I'm going to change this issue's status to Postponed (maintainer needs more info). If you can help me, please post it here, and change the Status back to Active or Needs work.

(thanks for your understanding and patience with me as I try to manage this module's issue queue)

I have updated the patch using menu_manipulator module v4.0.0

I have uploaded the patch and interdiff files