Updated: Comment #N

Problem/Motivation

It's not possible to use the route_name in local_actions twice. With the possibility to add route_parameters I would expect this to work:

custom.add_start_point:
  route_name: custom.event.add
  route_parameters:
    type: 'start'
  title: 'Add start point'
  appears_on:
    - custom.manage

custom.add_end_point:
  route_name: custom.event.add
  route_parameters:
    type: 'end'
  title: 'Add end point'
  appears_on:
    - custom.manage

Only the "Add end point" is visible. Removing the end_point makes the start_point visible.

Proposed resolution

Don't key the available local actions by route name but by plugin ID internally + add test coverage.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#5 interdiff.txt915 bytesdawehner
#5 local_action-2174933-5.patch15.22 KBdawehner
#1 local_action-2174933.patch15.2 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
Issue tags: +MenuSystemRevamp, +PHPUnit
StatusFileSize
new local_action-2174933.patch15.2 KB

Started to write some unit tests and fixed the bug at the same time by switching from keying by route name to plugin IDs.

dawehner’s picture

dawehner
Primary language German
commented

1: local_action-2174933.patch queued for re-testing.

h3rj4n’s picture

h3rj4n commented
Status: Needs review » Reviewed & tested by the community

I tested it and it works fine!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update 
+++ b/core/lib/Drupal/Core/Menu/LocalActionManager.php
@@ -112,12 +113,13 @@ class LocalActionManager extends DefaultPluginManager {
+   * @param \Drupal\Core\Session\AccountInterface $account
    */

If you are going to fix the docblock might as well do it properly...

Proposed resolution

I don't know how to solve this.

Looks like we need an issue summary update :)

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new local_action-2174933-5.patch15.22 KB
new interdiff.txt915 bytes

Ups, why can't phpstorm solve that as well.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Looks good, assuming testbot runs green.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs issue summary update

Committed 9848762 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.