Issue #2107531 by pwolanin, neclimdul, cilefen, dawehner, YesCT, tim.plunkett:...

Issue #2107531 by pwolanin, neclimdul, cilefen, dawehner, YesCT, tim.plunkett: Improve DX of local task YAML definitions.

core/lib/Drupal/Core/Menu/LocalTaskDefault.php

@@ -92,7 +92,7 @@ public function getTitle() {
   public function getWeight() {
     // By default the weight is 0, or -10 for the root tab.
     if (!isset($this->pluginDefinition['weight'])) {
-      if ($this->pluginDefinition['tab_root_id'] == $this->pluginId) {
+      if ($this->pluginDefinition['base_route'] == $this->pluginDefinition['route_name']) {
         $this->pluginDefinition['weight'] = -10;
       }
       else {

core/lib/Drupal/Core/Menu/LocalTaskDerivativeBase.php

-<?php
-
-/**
- * @file
- * Contains \Drupal\Core\Menu\LocalTaskDerivativeBase.
- */
-
-namespace Drupal\Core\Menu;
-
-use Drupal\Component\Plugin\Derivative\DerivativeBase;
-
-/**
- * Provides a getPluginIdFromRoute method for local task derivatives.
- */
-class LocalTaskDerivativeBase extends DerivativeBase {
-
-  /**
-   * Finds the local task ID of a route given the route name.
-   *
-   * @param string $route_name
-   *   The route name.
-   * @param array $local_tasks
-   *   An array of all local task definitions.
-   *
-   * @return string|null
-   *   Returns the local task ID of the given route or NULL if none is found.
-   */
-  protected function getPluginIdFromRoute($route_name, &$local_tasks) {
-    foreach ($local_tasks as $plugin_id => $local_task) {
-      if ($local_task['route_name'] == $route_name) {
-        return $plugin_id;
-        break;
-      }
-    }
-  }
-
-}

core/lib/Drupal/Core/Menu/LocalTaskManager.php

@@ -40,10 +40,10 @@ class LocalTaskManager extends DefaultPluginManager {
     'route_parameters' => array(),
     // The static title for the local task.
     'title' => '',
-    // The plugin ID of the root tab.
-    'tab_root_id' => '',
+    // The route name where the root tab appears.
+    'base_route' => '',
     // The plugin ID of the parent tab (or NULL for the top-level tab).
-    'tab_parent_id' => NULL,
+    'parent_id' => NULL,
     // The weight of the tab.
     'weight' => NULL,
     // The default link options.
@@ -171,7 +171,7 @@ public function getLocalTasksForRoute($route_name) {
     if (!isset($this->instances[$route_name])) {
       $this->instances[$route_name] = array();
       if ($cache = $this->cacheBackend->get($this->cacheKey . ':' . $route_name)) {
-        $tab_root_ids = $cache->data['tab_root_ids'];
+        $base_routes = $cache->data['base_routes'];
         $parents = $cache->data['parents'];
         $children = $cache->data['children'];
       }
@@ -179,47 +179,56 @@ public function getLocalTasksForRoute($route_name) {
         $definitions = $this->getDefinitions();
         // We build the hierarchy by finding all tabs that should
         // appear on the current route.
-        $tab_root_ids = array();
+        $base_routes = array();
         $parents = array();
         $children = array();
         foreach ($definitions as $plugin_id => $task_info) {
+          // Fill in the base_route from the parent to insure consistency.
+          if (!empty($task_info['parent_id']) && !empty($definitions[$task_info['parent_id']])) {
+            $task_info['base_route'] = $definitions[$task_info['parent_id']]['base_route'];
+            // Populate the definitions we use in the next loop. Using a
+            // reference like &$task_info causes bugs.
+            $definitions[$plugin_id]['base_route'] = $definitions[$task_info['parent_id']]['base_route'];
+          }
           if ($route_name == $task_info['route_name']) {
-            $tab_root_ids[$task_info['tab_root_id']] = $task_info['tab_root_id'];
+            if(!empty($task_info['base_route'])) {
+              $base_routes[$task_info['base_route']] = $task_info['base_route'];
+            }
             // Tabs that link to the current route are viable parents
             // and their parent and children should be visible also.
             // @todo - this only works for 2 levels of tabs.
             // instead need to iterate up.
             $parents[$plugin_id] = TRUE;
-            if (!empty($task_info['tab_parent_id'])) {
-              $parents[$task_info['tab_parent_id']] = TRUE;
+            if (!empty($task_info['parent_id'])) {
+              $parents[$task_info['parent_id']] = TRUE;
             }
           }
         }
-        if ($tab_root_ids) {
+        if ($base_routes) {
           // Find all the plugins with the same root and that are at the top
           // level or that have a visible parent.
           foreach ($definitions  as $plugin_id => $task_info) {
-            if (!empty($tab_root_ids[$task_info['tab_root_id']]) && (empty($task_info['tab_parent_id']) || !empty($parents[$task_info['tab_parent_id']]))) {
+            if (!empty($base_routes[$task_info['base_route']]) && (empty($task_info['parent_id']) || !empty($parents[$task_info['parent_id']]))) {
               // Concat '> ' with root ID for the parent of top-level tabs.
-              $parent = empty($task_info['tab_parent_id']) ? '> ' . $task_info['tab_root_id'] : $task_info['tab_parent_id'];
+              $parent = empty($task_info['parent_id']) ? '> ' . $task_info['base_route'] : $task_info['parent_id'];
               $children[$parent][$plugin_id] = $task_info;
             }
           }
         }
         $data = array(
-          'tab_root_ids' => $tab_root_ids,
+          'base_routes' => $base_routes,
           'parents' => $parents,
           'children' => $children,
         );
         $this->cacheBackend->set($this->cacheKey . ':' . $route_name, $data, CacheBackendInterface::CACHE_PERMANENT, $this->cacheTags);
       }
       // Create a plugin instance for each element of the hierarchy.
-      foreach ($tab_root_ids as $root_id) {
+      foreach ($base_routes as $base_route) {
         // Convert the tree keyed by plugin IDs into a simple one with
         // integer depth.  Create instances for each plugin along the way.
         $level = 0;
         // We used this above as the top-level parent array key.
-        $next_parent = '> ' . $root_id;
+        $next_parent = '> ' . $base_route;
         do {
           $parent = $next_parent;
           $next_parent = FALSE;
@@ -283,8 +292,7 @@ public function getTasksBuild($current_route_name) {
           // The plugin may have been set active in getLocalTasksForRoute() if
           // one of its child tabs is the active tab.
           $active = $active || $child->getActive();
-          // @todo It might make sense to use menu link entities instead of
-          //   arrays.
+          // @todo It might make sense to use link render elements instead.
 
           $link = array(
             'title' => $this->getTitle($child),

core/modules/action/action.local_tasks.yml

 action.admin:
   route_name: action.admin
   title: 'Manage actions'
-  tab_root_id: action.admin
+  base_route: action.admin

core/modules/aggregator/aggregator.local_tasks.yml

 aggregator.admin_overview:
   route_name: aggregator.admin_overview
   title: 'List'
-  tab_root_id: aggregator.admin_overview
+  base_route: aggregator.admin_overview
 aggregator.admin_settings:
   route_name: aggregator.admin_settings
   title: 'Settings'
   weight: 100
-  tab_root_id: aggregator.admin_overview
+  base_route: aggregator.admin_overview
 
 aggregator.feed_view:
   route_name: aggregator.feed_view
-  tab_root_id: aggregator.feed_view
+  base_route: aggregator.feed_view
   title: View
 aggregator.feed_configure:
   route_name: aggregator.feed_configure
-  tab_root_id: aggregator.feed_view
+  base_route: aggregator.feed_view
   title: 'Configure'
   weight: 10

core/modules/block/block.local_tasks.yml

 block.admin_edit:
   title: 'Configure block'
   route_name: block.admin_edit
-  tab_root_id: block.admin_edit
+  base_route: block.admin_edit
 
 # Per theme block layout pages.
 block.admin_display:
   title: 'Block Layout'
   route_name: block.admin_display
-  tab_root_id: block.admin_display
+  base_route: block.admin_display
 block.admin_display_theme:
   title: 'Block Layout'
   route_name: block.admin_display_theme
-  tab_root_id: block.admin_display
-  tab_parent_id: block.admin_display
+  parent_id: block.admin_display
   derivative: 'Drupal\block\Plugin\Derivative\ThemeLocalTask'

core/modules/block/custom_block/custom_block.local_tasks.yml

 custom_block.list:
   title: 'Custom block library'
   route_name: custom_block.list
-  tab_root_id: block.admin_display
+  base_route: block.admin_display
 custom_block.list_sub:
   title: Blocks
   route_name: custom_block.list
-  tab_root_id: block.admin_display
-  tab_parent_id: custom_block.list
+  parent_id: custom_block.list
 custom_block.type_list:
   title: Types
   route_name: custom_block.type_list
-  tab_root_id: block.admin_display
-  tab_parent_id: custom_block.list
+  parent_id: custom_block.list
 
 custom_block.edit:
   title: Edit
   route_name: custom_block.edit
-  tab_root_id: custom_block.edit
+  base_route: custom_block.edit
 custom_block.delete:
   title: Delete
   route_name: custom_block.delete
-  tab_root_id: custom_block.edit
+  base_route: custom_block.edit
 
 # Default tab for custom block type editing.
 custom_block.type_edit:
   title: 'Edit'
   route_name: custom_block.type_edit
-  tab_root_id: custom_block.type_edit
+  base_route: custom_block.type_edit

core/modules/block/lib/Drupal/block/Plugin/Derivative/ThemeLocalTask.php

@@ -56,8 +56,8 @@ public function getDerivativeDefinitions(array $base_plugin_definition) {
       // Default task!
       if ($default_theme == $theme_name) {
         $this->derivatives[$theme_name]['route_name'] = 'block.admin_display';
-        // Emulate default logic because without the base plugin id we can't set the
-        // change the tab_root_id.
+        // Emulate default logic because without the base plugin id we can't
+        // change the base_route.
         $this->derivatives[$theme_name]['weight'] = -10;
 
         unset($this->derivatives[$theme_name]['route_parameters']);

core/modules/book/book.local_tasks.yml

 book.admin:
   route_name: book.admin
   title: 'List'
-  tab_root_id: book.admin
+  base_route: book.admin
 book.settings:
   route_name: book.settings
   title: 'Settings'
-  tab_root_id: book.admin
+  base_route: book.admin
   weight: 100
 
 book.outline:
   route_name: book.outline
-  tab_root_id: node.view
+  base_route: node.view
   title: Outline
   weight: 2

core/modules/comment/comment.local_tasks.yml

 comment.permalink_tab:
   route_name: comment.permalink
   title: 'View comment'
-  tab_root_id: comment.permalink_tab
+  base_route: comment.permalink
 comment.edit_page_tab:
   route_name: comment.edit_page
   title: 'Edit'
-  tab_root_id: comment.permalink_tab
+  base_route: comment.permalink
   weight: 0
 comment.confirm_delete_tab:
   route_name: comment.confirm_delete
   title: 'Delete'
-  tab_root_id: comment.permalink_tab
+  base_route: comment.permalink
   weight: 10
 
 comment.admin:
   title: Comments
   route_name: comment.admin
-  tab_root_id: node.content_overview
+  base_route: node.content_overview
 
 comment.admin_new:
   title: 'Published comments'
   route_name: comment.admin
-  tab_root_id: node.content_overview
-  tab_parent_id: comment.admin
+  parent_id: comment.admin
 
 comment.admin_approval:
   title: 'Unapproved comments'
   route_name: comment.admin_approval
   class: Drupal\comment\Plugin\Menu\LocalTask\UnapprovedComments
-  tab_root_id: node.content_overview
-  tab_parent_id: comment.admin
+  parent_id: comment.admin
   weight: 1

core/modules/config/config.local_tasks.yml

 config.sync:
   route_name: config.sync
-  tab_root_id: config.sync
+  base_route: config.sync
   title: 'Synchronize'
 
 config.full:
   route_name: config.import_full
   title: 'Full Import/Export'
-  tab_root_id: config.sync
+  base_route: config.sync
 
 config.single:
   route_name: config.import_single
   title: 'Single Import/Export'
-  tab_root_id: config.sync
+  base_route: config.sync
 
 config.export_full:
   route_name: config.export_full
   title: Export
-  tab_root_id: config.sync
-  tab_parent_id: config.full
+  parent_id: config.full
 
 config.import_full:
   route_name: config.import_full
   title: Import
-  tab_root_id: config.sync
-  tab_parent_id: config.full
+  parent_id: config.full
 
 config.export_single:
   route_name: config.export_single
   title: Export
-  tab_root_id: config.sync
-  tab_parent_id: config.single
+  parent_id: config.single
 
 config.import_single:
   route_name: config.import_single
   title: Import
-  tab_root_id: config.sync
-  tab_parent_id: config.single
+  parent_id: config.single

core/modules/config/tests/config_test/config_test.local_tasks.yml

 config_test.entity_tab:
   route_name: config_test.entity
   title: 'Edit'
-  tab_root_id: config_test.entity_tab
+  base_route: config_test.entity

core/modules/config_translation/config_translation.module

@@ -175,12 +175,3 @@ function config_translation_library_info() {
   );
   return $libraries;
 }
-
-/**
- * Implements hook_local_tasks_alter().
- */
-function config_translation_local_tasks_alter(&$local_tasks) {
-  // Alters in tab_root_ids onto the config translation local tasks.
-  $derivative = ConfigTranslationLocalTasks::create(\Drupal::getContainer(), 'config_translation.local_tasks');
-  $derivative->alterLocalTasks($local_tasks);
-}

core/modules/config_translation/lib/Drupal/config_translation/Plugin/Derivative/ConfigTranslationLocalTasks.php

@@ -8,14 +8,14 @@
 namespace Drupal\config_translation\Plugin\Derivative;
 
 use Drupal\config_translation\ConfigMapperManagerInterface;
-use Drupal\Core\Menu\LocalTaskDerivativeBase;
+use Drupal\Component\Plugin\Derivative\DerivativeBase;
 use Drupal\Core\Plugin\Discovery\ContainerDerivativeInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Provides dynamic local tasks for config translation.
  */
-class ConfigTranslationLocalTasks extends LocalTaskDerivativeBase implements ContainerDerivativeInterface {
+class ConfigTranslationLocalTasks extends DerivativeBase implements ContainerDerivativeInterface {
 
   /**
    * The mapper plugin discovery service.
@@ -62,32 +62,16 @@ public function getDerivativeDefinitions(array $base_plugin_definition) {
     foreach ($mappers as $plugin_id => $mapper) {
       /** @var \Drupal\config_translation\ConfigMapperInterface $mapper */
       $route_name = $mapper->getOverviewRouteName();
-      $this->derivatives[$route_name] = $base_plugin_definition;
-      $this->derivatives[$route_name]['config_translation_plugin_id'] = $plugin_id;
-      $this->derivatives[$route_name]['class'] = '\Drupal\config_translation\Plugin\Menu\LocalTask\ConfigTranslationLocalTask';
-      $this->derivatives[$route_name]['route_name'] = $route_name;
-    }
-    return parent::getDerivativeDefinitions($base_plugin_definition);
-  }
-
-  /**
-   * Alters the local tasks to find the proper tab_root_id for each task.
-   */
-  public function alterLocalTasks(array &$local_tasks) {
-    $mappers = $this->mapperManager->getMappers();
-    foreach ($mappers as $mapper) {
-      /** @var \Drupal\config_translation\ConfigMapperInterface $mapper */
-      $route_name = $mapper->getOverviewRouteName();
-      $translation_tab = $this->basePluginId . ':' . $route_name;
-      $tab_root_id = $this->getPluginIdFromRoute($mapper->getBaseRouteName(), $local_tasks);
-      if (!empty($tab_root_id)) {
-        $local_tasks[$translation_tab]['tab_root_id'] = $tab_root_id;
-      }
-      else {
-        unset($local_tasks[$translation_tab]);
+      $base_route = $mapper->getBaseRouteName();
+      if (!empty($base_route)) {
+        $this->derivatives[$route_name] = $base_plugin_definition;
+        $this->derivatives[$route_name]['config_translation_plugin_id'] = $plugin_id;
+        $this->derivatives[$route_name]['class'] = '\Drupal\config_translation\Plugin\Menu\LocalTask\ConfigTranslationLocalTask';
+        $this->derivatives[$route_name]['route_name'] = $route_name;
+        $this->derivatives[$route_name]['base_route'] = $base_route;
       }
     }
+    return parent::getDerivativeDefinitions($base_plugin_definition);
   }
 
-
 }

core/modules/contact/contact.local_tasks.yml

 contact.category_edit:
   title: 'Edit'
   route_name: contact.category_edit
-  tab_root_id: contact.category_edit
+  base_route: contact.category_edit
 
 contact.personal_page:
   title: 'Contact'
   route_name: contact.personal_page
   weight: 2
-  tab_root_id: user.view
+  base_route: user.view

core/modules/content_translation/content_translation.module

@@ -261,15 +261,6 @@ function content_translation_menu_alter(array &$items) {
   }
 }
 
-/**
- * Implements hook_local_tasks_alter().
- */
-function content_translation_local_tasks_alter(&$local_tasks) {
-  // Alters in tab_root_id onto the content translation local task.
-  $derivative = ContentTranslationLocalTasks::create(\Drupal::getContainer(), 'content_translation.local_tasks');
-  $derivative->alterLocalTasks($local_tasks);
-}
-
 /**
  * Convert an entity canonical link to a router path.
  *

core/modules/content_translation/lib/Drupal/content_translation/Plugin/Derivative/ContentTranslationLocalTasks.php

@@ -8,14 +8,14 @@
 namespace Drupal\content_translation\Plugin\Derivative;
 
 use Drupal\content_translation\ContentTranslationManagerInterface;
-use Drupal\Core\Menu\LocalTaskDerivativeBase;
+use Drupal\Component\Plugin\Derivative\DerivativeBase;
 use Drupal\Core\Plugin\Discovery\ContainerDerivativeInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Provides dynamic local tasks for content translation.
  */
-class ContentTranslationLocalTasks extends LocalTaskDerivativeBase implements ContainerDerivativeInterface {
+class ContentTranslationLocalTasks extends DerivativeBase implements ContainerDerivativeInterface {
 
   /**
    * The base plugin ID
@@ -67,25 +67,10 @@ public function getDerivativeDefinitions(array $base_plugin_definition) {
         'entity_type' => $entity_type,
         'title' => 'Translate',
         'route_name' => $translation_route_name,
+        'base_route' => $entity_info['links']['canonical'],
       ) + $base_plugin_definition;
     }
     return parent::getDerivativeDefinitions($base_plugin_definition);
   }
 
-  /**
-   * Alters the local tasks to find the proper tab_root_id for each task.
-   */
-  public function alterLocalTasks(array &$local_tasks) {
-    foreach ($this->contentTranslationManager->getSupportedEntityTypes() as $entity_info) {
-      // Find the route name for the entity page.
-      $entity_route_name = $entity_info['links']['canonical'];
-
-      // Find the route name for the translation overview.
-      $translation_route_name = $entity_info['links']['drupal:content-translation-overview'];
-      $translation_tab = $this->basePluginId . ':' . $translation_route_name;
-
-      $local_tasks[$translation_tab]['tab_root_id'] = $this->getPluginIdFromRoute($entity_route_name, $local_tasks);
-    }
-  }
-
 }

core/modules/content_translation/tests/Drupal/content_translation/Tests/Menu/ContentTranslationLocalTasksTest.php

@@ -8,7 +8,6 @@
 namespace Drupal\content_translation\Tests\Menu;
 
 use Drupal\Tests\Core\Menu\LocalTaskIntegrationTest;
-use Drupal\content_translation\Plugin\Derivative\ContentTranslationLocalTasks;;
 
 /**
  * Tests existence of block local tasks.
@@ -48,24 +47,6 @@ public function setUp() {
     \Drupal::getContainer()->set('content_translation.manager', $content_translation_manager);
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function getLocalTaskManager($modules, $route_name, $route_params) {
-    $manager = parent::getLocalTaskManager($modules, $route_name, $route_params);
-
-    // Duplicate content_translation_local_tasks_alter()'s code here to avoid
-    // having to load the .module file.
-    $this->moduleHandler->expects($this->once())
-      ->method('alter')
-      ->will($this->returnCallback(function ($hook, &$local_tasks) {
-          // Alters in tab_root_id onto the content translation local task.
-          $derivative = ContentTranslationLocalTasks::create(\Drupal::getContainer(), 'content_translation.local_tasks');
-          $derivative->alterLocalTasks($local_tasks);
-      }));
-    return $manager;
-  }
-
   /**
    * Tests the block admin display local tasks.
    *

core/modules/entity/entity.local_tasks.yml

 entity.view_mode_edit:
   title: 'Edit'
   route_name: entity.view_mode_edit
-  tab_root_id: entity.view_mode_edit
+  base_route: entity.view_mode_edit
 
 entity.form_mode_edit:
   title: 'Edit'
   route_name: entity.form_mode_edit
-  tab_root_id: entity.form_mode_edit
+  base_route: entity.form_mode_edit
 
 entity.view_mode_list:
   title: List
   route_name: entity.view_mode_list
-  tab_root_id: entity.view_mode_list
+  base_route: entity.view_mode_list
 
 entity.form_mode_list:
   title: List
   route_name: entity.form_mode_list
-  tab_root_id: entity.form_mode_list
+  base_route: entity.form_mode_list

core/modules/field_ui/field_ui.local_tasks.yml

 field_ui.list:
   title: Entities
   route_name: field_ui.list
-  tab_root_id: field_ui.list
+  base_route: field_ui.list
 field_ui.fields:
   class: \Drupal\Core\Menu\LocalTaskDefault
   derivative: \Drupal\field_ui\Plugin\Derivative\FieldUiLocalTask