Allow MenuLinkTree manipulators to be altered

Re-rolling #14 for 9.5.3

Last patch was generated incorrectly and was missing files.

Is there a patch that works with D10?

Adding a patch that can be applied to D10.

Fixing my previous patch, to be used with D10

Patch for Drupal 10.2.

I've rolled ##24 onto an MR and updated the event dispatcher based on https://www.drupal.org/node/3376090 →

This was set to NW all the way back in #7 so I think this may be ready for another review.

The main thing I'm interested in is allowing menu links to have their cacheable metadata refined (i.e adding RefinableCacheableDependencyTrait to MenuLinkBase).

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Appears to have test failures

core/lib/Drupal/Core/Menu/MenuLinkBase.php will the removed functions have to be deprecated?

core/lib/Drupal/Core/Menu/MenuLinkBase.php will the removed functions have to be deprecated?
No, they are provided by the trait that's added instead.

Failures should be fixed with latest commit.

Left 2 more small comments, will keep an eye out for this one.

Resolved the comments and also applied similar changes to the rest of the new files. Also removed the reference from getManipulators as we don't need that when we have a setter.

Failing tests due to https://www.drupal.org/project/drupal/issues/3041318 →

+1 to this solution

The event does not alters the manipulators array, help!

    $this->eventDispatcher->dispatch(new MenuLinkTreeManipulatorsAlterEvent($tree, $manipulators, $this), MenuLinkTreeEvents::ALTER_MANIPULATORS);
    foreach ($manipulators as $manipulator) {
      $callable = $this->callableResolver->getCallableFromDefinition($manipulator['callable']);

My subscriber:

  /**
   * React to manipulators alter event.
   */
  public function manipulatorsAlter(MenuLinkTreeManipulatorsAlterEvent $event) {
    $manipulators = $event->getManipulators();
    if (!empty($manipulators)) {
      array_unshift($manipulators, ['callable' => 'translatable_menu_link_uri.alter_links:alterMenuLinksManipulator']);
    }
    else {
      $manipulators = [['callable' => 'translatable_menu_link_uri.alter_links:alterMenuLinksManipulator']];
    }
    $event->setManipulators($manipulators);
  }

Maybe the array should be by reference?

I would say

  $this->eventDispatcher->dispatch(new MenuLinkTreeManipulatorsAlterEvent($tree, $manipulators, $this), MenuLinkTreeEvents::ALTER_MANIPULATORS);
    foreach ($manipulators as $manipulator) {
      $callable = $this->callableResolver->getCallableFromDefinition($manipulator['callable']);

should be:

  $event = new MenuLinkTreeManipulatorsAlterEvent($tree, $manipulators, $this);
  $this->eventDispatcher->dispatch($event, MenuLinkTreeEvents::ALTER_MANIPULATORS);
    foreach ($event->getManipulators() as $manipulator) {
      $callable = $this->callableResolver->getCallableFromDefinition($manipulator['callable']);

The $manipulators array is not changed during dispatch because arrays are copied when assigned, so a call to setManipulators inside a subscriber changes the array inside the $event but not the original $manipulators array used to create the instance of $event.

We are using 10.2.7, we are not sure, if Drupal 11 treats arrays in a different way, but the @tunic comment seems to be logical.

The change proposed by @tunic works for as on Drupal 10.2.7

Tests are failing https://git.drupalcode.org/issue/drupal-3091246/-/jobs/2307611

The manipulator is not applying

    There was 1 failure:
    
    1) Drupal\KernelTests\Core\Menu\MenuLinkTreeTest::testMenuManipulatorAlter
    Failed asserting that an array contains 'menu-test-link'.
    
    /builds/issue/drupal-3091246/core/tests/Drupal/KernelTests/Core/Menu/MenuLinkTreeTest.php:220

Here came the regression, the references were removed on commit https://git.drupalcode.org/project/drupal/-/commit/552c484ad403a37e93056...

Failed Javascript tests https://git.drupalcode.org/issue/drupal-3091246/-/pipelines/275582

Ready to review!!

Posted a few comments in the MR.

The current implementation in 🐛 Untranslated menu items are displayed in menus Needs work is very similar to this

Opened ✨ Make it easier to decorate menu.default_tree_manipulators Active which proposes another method of decorating manipulators.