AFAICT, there's no way for a menu link plugin to cause the menu link not to appear.

There is MenuLinkInterface::isEnabled(), but returning FALSE from that causes the menu link itself to become disabled.

I've also tried returning either '' or NULL from getRouteName(), but that causes a routing crash. There's route '<none>' but that creates a link to the current page.

At this point the only thing I can see to do is to create a custom route which always denies access, and return that route name.

Use case is that I am writing a custom menu link plugin to show a user's group. If the user has no group memberships, then the link should not show.

CommentFileSizeAuthor
#15 2719609-15.interdiff.txt3.33 KBclaudiu.cristea
#15 2719609-15-test-only.patch3.38 KBclaudiu.cristea
#11 2719609-10-test-only.patch2.55 KBclaudiu.cristea
#2 2719609-2.patch1.71 KBdawehner

Issue fork drupal-2719609

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Category: Feature request » Bug report
Status: Active » Needs review
FileSize
2719609-2.patch1.71 KB

Beside your proposed solution, to put that access logic into the route, IMHO isEnabled should itself totally work, but we have simply a bug.

    foreach ($tree as $data) {
      /** @var \Drupal\Core\Menu\MenuLinkInterface $link */
      $link = $data->link;
      // Generally we only deal with visible links, but just in case.
      if (!$link->isEnabled()) {
        continue;
      }
      // Gather the cacheability of every item in the menu link tree. Some links
      // may be dynamic: they may have a dynamic text (e.g. a "Hi, <user>" link
      // text, which would vary by 'user' cache context), or a dynamic route
      // name or route parameters.
      $tree_link_cacheability = $tree_link_cacheability->merge(CacheableMetadata::createFromObject($data->link));

This tree cacheability information should be gathered first, before we check whether the link is enabled or not.

joachim’s picture

joachim CreditAttribution: joachim as a volunteer commented
Issue summary: View changes

Fixed the summary -- my mention of route '<none>' wasn't showing up.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Issue tags: +Needs tests

@joachim
Did you ever managed to try out the patch? IMHO the patch is all we need, beside some additional test coverage ...

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented
Version: 8.7.x-dev » 8.6.x-dev
Issue tags: -Needs tests

This test proved that using MenuLinkInterface::isEnabled() just works, w/o the patch from #2. Or do we need an edge case in test?

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented
FileSize
2719609-10-test-only.patch2.55 KB

The test, sorry.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented

I'm wondering whether anything else adds the per route cache context automatically.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented

@dawehner, actually I can reproduce the bug in one of my projects but I cannot make this test fail to prove it. Probably it receives the route context from somewhere. Using the patch from #2, in my project is solving the issue. I have no time to investigate why the test passes (when it should not).

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented

In fact, \Drupal\system\Plugin\Block\SystemMenuBlock::getCacheContexts() is adding the route.menu_active_trails:{menu_name} context. That makes the test pass.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented
FileSize
2719609-15-test-only.patch3.38 KB
2719609-15.interdiff.txt3.33 KB

Improved the test but it's still passing w/o #2 :(

idimopoulos’s picture

idimopoulos CreditAttribution: idimopoulos at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented

This patch does work because we are testing it in our project. However, hidden links are also hidden from the menu edit form where the links are listed if the condition somehow excludes this form. I don't know if this is intended behaviour (e.g. hide the link from users that should not have access to it) but the table row is shown, without a link label next to the check box.
So, we should either not show the entire row (if the intended behaviour is to hide the menu item) or show the item in certain cases (like admin path, or administer permissions? not sure).

idimopoulos’s picture

idimopoulos CreditAttribution: idimopoulos at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented
Status: Needs review » Needs work
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented

@idimopoulos, there's no correlation between a disabled menu link and its clickable label shown in the administrative interface. Probably, you hit a JS/CSS issue that makes the link to be hidden.

idimopoulos’s picture

idimopoulos CreditAttribution: idimopoulos at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented

Actually, yeah, probably. because the patch indeed does not justify such a behaviour.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.