Unable to alter local actions prior to rendering

I definitely experienced a WTF moment with these hook naming conventions. I want to alter local actions labels and url when they are being rendered, so that I can add more contextual information like 'Add content to {this page}'.

Wow yeah that is quite sad :(

So the runtime vs. non runtime hook is completely opposite, not sure how to deal with that at all.

@jhedstrom
Do you think we can provide a little test to ensure this actually works?

Location of test has changed.

Uploading a test-only patch so we can see the test fails for drupal 8.5.x.

nevermind.

I tested this patch fine in Drupal 8.5.4. This allowed me to work around the core bug mentioned in

https://www.drupal.org/project/drupal/issues/2722237

It would be nice to have this commited.

This seems like a reasonable API addition. Thanks @k4v for testing the patch!

+++ b/core/lib/Drupal/Core/Menu/LocalActionManager.php
@@ -197,6 +197,11 @@ public function getActionsForRoute($route_appears) {
+    // @todo For consistency with local tasks, this hook should actually be
+    // named hook_menu_local_actions_alter().

If there is a @todo in the code it should have a followup issue that is included in the code comment. :) Also note the formatting in https://www.drupal.org/node/1354#todo.

If we plan to change hook_local_actions_alter() in D9, there should probably be a deprecation: https://www.drupal.org/core/deprecation#how-hook I'm not totally sure of what exactly we want to do but an explicit deprecation and followup should help. :)

Also, we should have a change record for this change. Thanks!

I wrote a change record. https://www.drupal.org/node/3028611

I've added a link/issue to the todo.

Looks good to me! Thanks

+1 to RTBC

As far as I can see we're adding a new hook for render altering called hook_local_actions_alter().

Reading the issue summary this means we'll have:

• hook_menu_local_actions_alter() - altering during discovery
• hook_local_actions_alter() - altering during render

Which is exactly the opposite of local tasks - according to the issue summary.

This will be really confusing for developers - also there is no way to deprecate to swap them around. I suggest we come up with a plan where we can make this more consistent in all cases.

I suggest doing something like this...

Actions
Discovery hook: hook_menu_local_actions_alter -> hook_local_actions_alter - need an issue to deprecate the old hook and implement the new one.
Render hook: does not exist -> hook_local_actions_render_alter - add that here

Tasks:
Discovery hook: hook_local_tasks_alter ->hook_local_tasks_alter - stays the same yay!
Render hook: hook_menu_local_tasks_alter -> hook_local_tasks_render_alter - deprecate hook_menu_local_tasks_alter here and add hook_local_tasks_render_alter for consistency.

We could choose to do the entirety of #32 and make this issue about making tasks and action hooks consistent.

1. Opened a merge request with the changes from the latest patch and minor improvements to hook example and documentation.
2. Moved to "Needs review" two related issues responsible for renaming existing hooks as suggested by @alexpott in #32: #3216564: Rename hook_menu_local_tasks_alter to hook_local_tasks_render_alter and #3028616: Rename hook_menu_local_actions_alter() to hook_local_actions_alter().
3. Updated issue summary and change record - https://www.drupal.org/node/3028611.

Would love to see this in 9.3.x :)

I'm merging #3216564: Rename hook_menu_local_tasks_alter to hook_local_tasks_render_alter into this task to make sure we keep those two hooks consistent.

It was suggested by @alexpott in Slack when I asked him for review:

Maybe merging it with https://www.drupal.org/project/drupal/issues/3216564 so at least hook_local_actions_render_alter and hook_local_tasks_render_alter are consistent

Patch is working well on my project.
Thanks a lot!

Same as #3028616: Rename hook_menu_local_actions_alter() to hook_local_actions_alter() can we update the MR for 10.1 please

Working well for us. We are using this to set a specific route parameter based on external information:

      /** @var \Drupal\Core\Url $url */
      $url = &$local_actions['local_action.id']['#link']['url'];
      $url->setRouteParameter('node', $node->id());

A word to the wise: don't be like me and remember that you may likely need to have a method of invalidating the cache for this rendered item.

Please review.

Appears this was just a copy but not updated for 10.1 but should be simple.

I rerolled !3858 into a new MR based on 11.x.
I converted procedural hooks to object ones.

Attached: patches for composer

+++ b/core/lib/Drupal/Core/Menu/LocalTaskManager.php
@@ -361,7 +361,8 @@ public function getLocalTasks($route_name, $level = 0) {
+        $this->moduleHandler->alterDeprecated('Deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. Use local_tasks_render instead. See https://www.drupal.org/node/3216578.', 'menu_local_tasks', $this->taskData[$route_name], $route_name, $cacheability);

+++ b/core/lib/Drupal/Core/Menu/menu.api.php
@@ -310,6 +310,10 @@ function hook_menu_links_discovered_alter(&$links) {
+ * @deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. Use
+ *   hook_local_tasks_render_alter instead.

version needs update to 11.2=>12.0

@andypost Feedback addressed, thanks.

Added suggestion to format deprecation message (link should not have dot at the end)

And it needs deprecation test to make sure backward compatibility works but throws deprecation

I don't have time to look at the MRs and update them, but I have re-rolled https://git.drupalcode.org/project/drupal/-/merge_requests/744 for 10.3.x. I'll upload that as a patch file for now.

Didn't meant to drop the last changes.

@andypost I accepted your suggestion an rebased the branch on 11.X
I have no idea how to write the deprecation test, though. Would you guide me, please?

Here are updated patches for composer

@duaelfr Regarding the deprecation test: https://www.drupal.org/about/core/policies/core-change-policies/how-to-d.... You can search recursively through the core folder for the string '@Group legacy' to find existing deprecation tests.

Fixed Unit test in MR!10284.

Pipeline tests all green.

Test coverage seems to be in place for the hook in MenuTestHooks.php.

We still need a deprecation test.

Patch for composer with latest changes