Issue #2064557 by tim.plunkett: Improve strange coupling in...

Issue #2064557 by tim.plunkett: Improve strange coupling in EntityListControllers by improving buildRow() and buildHeader().

core/lib/Drupal/Core/Entity/EntityListController.php

@@ -91,6 +91,19 @@ public function load() {
     return $this->storage->loadMultiple();
   }
 
+  /**
+   * Returns the escaped label of an entity.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface $entity
+   *   The entity being listed.
+   *
+   * @return string
+   *   The escaped entity label.
+   */
+  protected function getLabel(EntityInterface $entity) {
+    return String::checkPlain($entity->label());
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -127,8 +140,6 @@ public function getOperations(EntityInterface $entity) {
    * @see Drupal\Core\Entity\EntityListController::render()
    */
   public function buildHeader() {
-    $row['label'] = t('Label');
-    $row['id'] = t('Machine name');
     $row['operations'] = t('Operations');
     return $row;
   }
@@ -145,10 +156,7 @@ public function buildHeader() {
    * @see Drupal\Core\Entity\EntityListController::render()
    */
   public function buildRow(EntityInterface $entity) {
-    $row['label'] = String::checkPlain($entity->label());
-    $row['id'] = $entity->id();
-    $operations = $this->buildOperations($entity);
-    $row['operations']['data'] = $operations;
+    $row['operations']['data'] = $this->buildOperations($entity);
     return $row;
   }
 

core/modules/action/lib/Drupal/action/ActionListController.php

@@ -86,9 +86,9 @@ public function load() {
    */
   public function buildRow(EntityInterface $entity) {
     $row['type'] = $entity->getType();
-    $row['label'] = String::checkPlain($entity->label());
+    $row['label'] = $this->getLabel($entity);
     if ($this->hasConfigurableActions) {
-      $row['operations']['data'] = $this->buildOperations($entity);
+      $row += parent::buildRow($entity);
     }
     return $row;
   }
@@ -100,8 +100,7 @@ public function buildHeader() {
     $header = array(
       'type' => t('Action type'),
       'label' => t('Label'),
-      'operations' => t('Operations'),
-    );
+    ) + parent::buildHeader();
     return $header;
   }
 

core/modules/block/custom_block/lib/Drupal/custom_block/CustomBlockListController.php

@@ -19,19 +19,16 @@ class CustomBlockListController extends EntityListController {
    * {@inheritdoc}
    */
   public function buildHeader() {
-    $header = parent::buildHeader();
     $header['label'] = t('Block description');
-    unset($header['id']);
-    return $header;
+    return $header + parent::buildHeader();
   }
 
   /**
    * {@inheritdoc}
    */
   public function buildRow(EntityInterface $entity) {
-    $row = parent::buildRow($entity);
-    unset($row['id']);
-    return $row;
+    $row['label'] = $this->getLabel($entity);
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/block/custom_block/lib/Drupal/custom_block/CustomBlockTypeListController.php

@@ -48,22 +48,19 @@ public function getOperations(EntityInterface $entity) {
    * Overrides \Drupal\Core\Entity\EntityListController::buildHeader().
    */
   public function buildHeader() {
-    $row['type'] = t('Block type');
-    $row['description'] = t('Description');
-    $row['operations'] = t('Operations');
-    return $row;
+    $header['type'] = t('Block type');
+    $header['description'] = t('Description');
+    return $header + parent::buildHeader();
   }
 
   /**
    * Overrides \Drupal\Core\Entity\EntityListController::buildRow().
    */
   public function buildRow(EntityInterface $entity) {
-    parent::buildRow($entity);
     $uri = $entity->uri();
     $row['type'] = l($entity->label(), $uri['path'], $uri['options']);
     $row['description'] = filter_xss_admin($entity->description);
-    $row['operations']['data'] = $this->buildOperations($entity);
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/config/tests/config_test/lib/Drupal/config_test/ConfigTestListController.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\config_test\ConfigTestListController.
+ */
+
+namespace Drupal\config_test;
+
+use Drupal\Core\Config\Entity\ConfigEntityListController;
+use Drupal\Core\Entity\EntityInterface;
+
+/**
+ * Provides a list controller for config_test.
+ */
+class ConfigTestListController extends ConfigEntityListController {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildHeader() {
+    $header['label'] = t('Label');
+    $header['id'] = t('Machine name');
+    return $header + parent::buildHeader();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildRow(EntityInterface $entity) {
+    $row['label'] = $this->getLabel($entity);
+    $row['id'] = $entity->id();
+    return $row + parent::buildRow($entity);
+  }
+
+}

core/modules/config/tests/config_test/lib/Drupal/config_test/Entity/ConfigTest.php

@@ -21,7 +21,7 @@
  *   module = "config_test",
  *   controllers = {
  *     "storage" = "Drupal\config_test\ConfigTestStorageController",
- *     "list" = "Drupal\Core\Config\Entity\ConfigEntityListController",
+ *     "list" = "Drupal\config_test\ConfigTestListController",
  *     "form" = {
  *       "default" = "Drupal\config_test\ConfigTestFormController",
  *       "delete" = "Drupal\config_test\Form\ConfigTestDeleteForm"

core/modules/contact/lib/Drupal/contact/CategoryListController.php

@@ -47,18 +47,17 @@ public function getOperations(EntityInterface $entity) {
    * Overrides Drupal\Core\Entity\EntityListController::buildHeader().
    */
   public function buildHeader() {
-    $row['category'] = t('Category');
-    $row['recipients'] = t('Recipients');
-    $row['selected'] = t('Selected');
-    $row['operations'] = t('Operations');
-    return $row;
+    $header['category'] = t('Category');
+    $header['recipients'] = t('Recipients');
+    $header['selected'] = t('Selected');
+    return $header + parent::buildHeader();
   }
 
   /**
    * Overrides Drupal\Core\Entity\EntityListController::buildRow().
    */
   public function buildRow(EntityInterface $entity) {
-    $row['category'] = check_plain($entity->label());
+    $row['category'] = $this->getLabel($entity);
     // Special case the personal category.
     if ($entity->id() == 'personal') {
       $row['recipients'] = t('Selected user');
@@ -69,8 +68,7 @@ public function buildRow(EntityInterface $entity) {
       $default_category = \Drupal::config('contact.settings')->get('default_category');
       $row['selected'] = ($default_category == $entity->id() ? t('Yes') : t('No'));
     }
-    $row['operations']['data'] = $this->buildOperations($entity);
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
 }

core/modules/entity/lib/Drupal/entity/EntityDisplayModeListController.php

@@ -7,7 +7,6 @@
 
 namespace Drupal\entity;
 
-use Drupal\Component\Utility\String;
 use Drupal\Core\Config\Entity\ConfigEntityListController;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityStorageControllerInterface;
@@ -64,18 +63,16 @@ public static function createInstance(ContainerInterface $container, $entity_typ
    * {@inheritdoc}
    */
   public function buildHeader() {
-    $header = parent::buildHeader();
-    unset($header['id']);
-    return $header;
+    $header['label'] = t('Label');
+    return $header + parent::buildHeader();
   }
 
   /**
    * {@inheritdoc}
    */
   public function buildRow(EntityInterface $entity) {
-    $row = parent::buildRow($entity);
-    unset($row['id']);
-    return $row;
+    $row['label'] = $this->getLabel($entity);
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/filter/lib/Drupal/filter/FilterFormatListController.php

@@ -89,11 +89,10 @@ public function load() {
    * {@inheritdoc}
    */
   public function buildHeader() {
-    $row['name'] = t('Name');
-    $row['roles'] = t('Roles');
-    $row['weight'] = t('Weight');
-    $row['operations'] = t('Operations');
-    return $row;
+    $header['name'] = t('Name');
+    $header['roles'] = t('Roles');
+    $header['weight'] = t('Weight');
+    return $header + parent::buildHeader();
   }
 
   /**
@@ -118,7 +117,7 @@ public function buildRow(EntityInterface $entity) {
       }
     }
     else {
-      $row['name'] = array('#markup' => String::checkPlain($entity->label()));
+      $row['name'] = array('#markup' => $this->getLabel($entity));
       $roles = array_map('\Drupal\Component\Utility\String::checkPlain', filter_get_roles_by_format($entity));
       $roles_markup = $roles ? implode(', ', $roles) : t('No roles may use this format');
     }
@@ -133,8 +132,7 @@ public function buildRow(EntityInterface $entity) {
       '#attributes' => array('class' => array('text-format-order-weight')),
     );
 
-    $row['operations'] = $this->buildOperations($entity);
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/image/lib/Drupal/image/ImageStyleListController.php

@@ -75,19 +75,16 @@ public static function createInstance(ContainerInterface $container, $entity_typ
    * {@inheritdoc}
    */
   public function buildHeader() {
-    $row = parent::buildHeader();
-    unset($row['id']);
-    $row['label'] = $this->translator->translate('Style name');
-    return $row;
+    $header['label'] = $this->translator->translate('Style name');
+    return $header + parent::buildHeader();
   }
 
   /**
    * {@inheritdoc}
    */
   public function buildRow(EntityInterface $entity) {
-    $row = parent::buildRow($entity);
-    unset($row['id']);
-    return $row;
+    $row['label'] = $this->getLabel($entity);
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/language/lib/Drupal/language/LanguageListController.php

@@ -60,22 +60,16 @@ public function getOperations(EntityInterface $entity) {
    * {@inheritdoc}
    */
   public function buildHeader() {
-    $row = parent::buildHeader();
-    unset($row['id']);
-    $row['label'] = t('Name');
-    $row['weight'] = t('Weight');
-    return $row;
+    $header['label'] = t('Name');
+    $header['weight'] = t('Weight');
+    return $header + parent::buildHeader();
   }
 
-
   /**
    * {@inheritdoc}
    */
   public function buildRow(EntityInterface $entity) {
-    $row = parent::buildRow($entity);
-
     $row['#attributes']['class'][] = 'draggable';
-    unset($row['id']);
 
     $row['label'] = array(
       '#markup' => check_plain($entity->get('label')),
@@ -92,7 +86,7 @@ public function buildRow(EntityInterface $entity) {
       '#delta' => 30,
     );
 
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/menu/lib/Drupal/menu/MenuListController.php

@@ -18,13 +18,12 @@ class MenuListController extends ConfigEntityListController {
    * Overrides \Drupal\Core\Entity\EntityListController::buildHeader().
    */
   public function buildHeader() {
-    $row['title'] = t('Title');
-    $row['description'] = array(
+    $header['title'] = t('Title');
+    $header['description'] = array(
       'data' => t('Description'),
       'class' => array(RESPONSIVE_PRIORITY_MEDIUM),
     );
-    $row['operations'] = t('Operations');
-    return $row;
+    return $header + parent::buildHeader();
   }
 
   /**
@@ -32,12 +31,11 @@ public function buildHeader() {
    */
   public function buildRow(EntityInterface $entity) {
     $row['title'] = array(
-      'data' => check_plain($entity->label()),
+      'data' => $this->getLabel($entity),
       'class' => array('menu-label'),
     );
     $row['description'] = filter_xss_admin($entity->description);
-    $row['operations']['data'] = $this->buildOperations($entity);
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/node/lib/Drupal/node/NodeTypeListController.php

@@ -63,13 +63,12 @@ public static function createInstance(ContainerInterface $container, $entity_typ
    * {@inheritdoc}
    */
   public function buildHeader() {
-    $row['title'] = t('Name');
-    $row['description'] = array(
+    $header['title'] = t('Name');
+    $header['description'] = array(
       'data' => t('Description'),
       'class' => array(RESPONSIVE_PRIORITY_MEDIUM),
     );
-    $row['operations'] = t('Operations');
-    return $row;
+    return $header + parent::buildHeader();
   }
 
   /**
@@ -77,12 +76,11 @@ public function buildHeader() {
    */
   public function buildRow(EntityInterface $entity) {
     $row['title'] = array(
-      'data' => String::checkPlain($entity->label()),
+      'data' => $this->getLabel($entity),
       'class' => array('menu-label'),
     );
     $row['description'] = Xss::filterAdmin($entity->description);
-    $row['operations']['data'] = $this->buildOperations($entity);
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/picture/lib/Drupal/picture/PictureMappingListController.php

@@ -16,19 +16,21 @@
 class PictureMappingListController extends ConfigEntityListController {
 
   /**
-   * Overrides Drupal\config\EntityListControllerBase::hookMenu().
+   * {@inheritdoc}
    */
-  public function hookMenu() {
-    $path = $this->entityInfo['list path'];
-    $items = parent::hookMenu();
-
-    // Override the access callback.
-    $items[$path]['title'] = 'Picture Mappings';
-    $items[$path]['description'] = 'Manage list of picture mappings.';
-    $items[$path]['access callback'] = 'user_access';
-    $items[$path]['access arguments'] = array('administer pictures');
+  public function buildHeader() {
+    $header['label'] = t('Label');
+    $header['id'] = t('Machine name');
+    return $header + parent::buildHeader();
+  }
 
-    return $items;
+  /**
+   * {@inheritdoc}
+   */
+  public function buildRow(EntityInterface $entity) {
+    $row['label'] = $this->getLabel($entity);
+    $row['id'] = $entity->id();
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/shortcut/lib/Drupal/shortcut/ShortcutSetListController.php

@@ -19,9 +19,8 @@ class ShortcutSetListController extends ConfigEntityListController {
    * Overrides \Drupal\Core\Entity\EntityListController::buildHeader().
    */
   public function buildHeader() {
-    $row['label'] = t('Name');
-    $row['operations'] = t('Operations');
-    return $row;
+    $header['name'] = t('Name');
+    return $header + parent::buildHeader();
   }
 
   /**
@@ -47,9 +46,8 @@ public function getOperations(EntityInterface $entity) {
    * Overrides \Drupal\Core\Entity\EntityListController::buildRow().
    */
   public function buildRow(EntityInterface $entity) {
-    $row['name'] = check_plain($entity->label());
-    $row['operations']['data'] = $this->buildOperations($entity);
-    return $row;
+    $row['name'] = $this->getLabel($entity);
+    return $row + parent::buildRow($entity);
   }
 
 }

core/modules/system/lib/Drupal/system/DateFormatListController.php

@@ -76,8 +76,7 @@ public function buildHeader() {
     $header['id'] = t('Machine name');
     $header['label'] = t('Name');
     $header['pattern'] = t('Pattern');
-    $header['operations'] = t('Operations');
-    return $header;
+    return $header + parent::buildHeader();
   }
 
   /**
@@ -85,10 +84,9 @@ public function buildHeader() {
    */
   public function buildRow(EntityInterface $entity) {
     $row['id'] = $entity->id();
-    $row['label'] = String::checkPlain($entity->label());
+    $row['label'] = $this->getLabel($entity);
     $row['pattern'] = $this->dateService->format(REQUEST_TIME, $entity->id());
-    $row['operations']['data'] = $this->buildOperations($entity);
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
 }

core/modules/system/tests/modules/entity_test/lib/Drupal/entity_test/Entity/EntityTest.php

@@ -21,6 +21,7 @@
  *   module = "entity_test",
  *   controllers = {
  *     "storage" = "Drupal\entity_test\EntityTestStorageController",
+ *     "list" = "Drupal\entity_test\EntityTestListController",
  *     "access" = "Drupal\entity_test\EntityTestAccessController",
  *     "form" = {
  *       "default" = "Drupal\entity_test\EntityTestFormController"

core/modules/system/tests/modules/entity_test/lib/Drupal/entity_test/EntityTestListController.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\entity_test\EntityTestListController.
+ */
+
+namespace Drupal\entity_test;
+
+use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Entity\EntityListController;
+
+/**
+ * Provides a list controller for entity_test.
+ */
+class EntityTestListController extends EntityListController {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildHeader() {
+    $header['label'] = t('Label');
+    $header['id'] = t('Machine name');
+    return $header + parent::buildHeader();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildRow(EntityInterface $entity) {
+    $row['label'] = $this->getLabel($entity);
+    $row['id'] = $entity->id();
+    return $row + parent::buildRow($entity);
+  }
+
+}

core/modules/taxonomy/lib/Drupal/taxonomy/VocabularyListController.php

@@ -56,25 +56,20 @@ public function getOperations(EntityInterface $entity) {
    * {@inheritdoc}
    */
   public function buildHeader() {
-    $row = parent::buildHeader();
-    $row['label'] = t('Vocabulary name');
-    unset($row['id']);
-    $row['weight'] = t('Weight');
-    return $row;
+    $header['label'] = t('Vocabulary name');
+    $header['weight'] = t('Weight');
+    return $header + parent::buildHeader();
   }
 
   /**
    * {@inheritdoc}
    */
   public function buildRow(EntityInterface $entity) {
-    $row = parent::buildRow($entity);
-
     // Override default values to markup elements.
     $row['#attributes']['class'][] = 'draggable';
-    unset($row['id']);
 
     $row['label'] = array(
-      '#markup' => $row['label'],
+      '#markup' => $this->getLabel($entity),
     );
     $row['#weight'] = $entity->get('weight');
     // Add weight column.
@@ -85,7 +80,7 @@ public function buildRow(EntityInterface $entity) {
       '#default_value' => $entity->get('weight'),
       '#attributes' => array('class' => array('weight')),
     );
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
   /**
@@ -106,9 +101,8 @@ public function render() {
     );
     unset($build['#header']['weight']);
     foreach ($entities as $entity) {
-      $row = parent::buildRow($entity);
-      unset($row['id']);
-      $build['#rows'][$entity->id()] = $row;
+      $row['label'] = $this->getLabel($entity);
+      $build['#rows'][$entity->id()] = $row + parent::buildRow($entity);
     }
     return $build;
   }

core/modules/user/lib/Drupal/user/RoleListController.php

@@ -27,11 +27,9 @@ public function getFormID() {
    * {@inheritdoc}
    */
   public function buildHeader() {
-    $row = parent::buildHeader();
-    $row['label'] = t('Name');
-    unset($row['id']);
-    $row['weight'] = t('Weight');
-    return $row;
+    $header['label'] = t('Name');
+    $header['weight'] = t('Weight');
+    return $header + parent::buildHeader();
   }
 
   /**
@@ -56,14 +54,11 @@ public function getOperations(EntityInterface $entity) {
    * {@inheritdoc}
    */
   public function buildRow(EntityInterface $entity) {
-    $row = parent::buildRow($entity);
-
     // Override default values to markup elements.
     $row['#attributes']['class'][] = 'draggable';
-    unset($row['id']);
 
     $row['label'] = array(
-      '#markup' => $row['label'],
+      '#markup' => $this->getLabel($entity),
     );
     $row['#weight'] = $entity->get('weight');
     // Add weight column.
@@ -74,7 +69,7 @@ public function buildRow(EntityInterface $entity) {
       '#default_value' => $entity->get('weight'),
       '#attributes' => array('class' => array('weight')),
     );
-    return $row;
+    return $row + parent::buildRow($entity);
   }
 
   /**

core/modules/views_ui/lib/Drupal/views_ui/ViewListController.php

@@ -84,6 +84,7 @@ public function load() {
    * {@inheritdoc}
    */
   public function buildRow(EntityInterface $view) {
+    $row = parent::buildRow($view);
     return array(
       'data' => array(
         'view_name' => array(
@@ -96,9 +97,7 @@ public function buildRow(EntityInterface $view) {
         'description' => $view->get('description'),
         'tag' => $view->get('tag'),
         'path' => implode(', ', $this->getDisplayPaths($view)),
-        'operations' => array(
-          'data' => $this->buildOperations($view),
-        ),
+        'operations' => $row['operations'],
       ),
       'title' => t('Machine name: @name', array('@name' => $view->id())),
       'class' => array($view->status() ? 'views-ui-list-enabled' : 'views-ui-list-disabled'),

core/tests/Drupal/Tests/Core/Entity/EntityListControllerTest.php

@@ -14,7 +14,7 @@
  *
  * @group Entity
  *
- * @see \Drupal\Core\Entity\EntityListController
+ * @see \Drupal\entity_test\EntityTestListController
  */
 class EntityListControllerTest extends UnitTestCase {
 
@@ -78,7 +78,7 @@ protected function setUp() {
       ->disableOriginalConstructor()
       ->getMock();
 
-    $this->entityListController = $this->getMock('Drupal\Core\Entity\EntityListController', array('buildOperations'), array('user_role', static::$entityInfo, $role_storage_controller, $module_handler));
+    $this->entityListController = $this->getMock('Drupal\entity_test\EntityTestListController', array('buildOperations'), array('user_role', static::$entityInfo, $role_storage_controller, $module_handler));
 
     $this->entityListController->expects($this->any())
       ->method('buildOperations')
@@ -132,13 +132,13 @@ public function testBuildRow($input, $expected, $message, $ignorewarnings = FALS
   public function providerTestBuildRow() {
     $tests = array();
     // Checks that invalid multi-byte sequences are rejected.
-    $tests[] = array("Foo\xC0barbaz", '', 'EntityListController::buildRow() rejects invalid sequence "Foo\xC0barbaz"', TRUE);
-    $tests[] = array("\xc2\"", '', 'EntityListController::buildRow() rejects invalid sequence "\xc2\""', TRUE);
-    $tests[] = array("Fooÿñ", "Fooÿñ", 'EntityListController::buildRow() accepts valid sequence "Fooÿñ"');
+    $tests[] = array("Foo\xC0barbaz", '', 'EntityTestListController::buildRow() rejects invalid sequence "Foo\xC0barbaz"', TRUE);
+    $tests[] = array("\xc2\"", '', 'EntityTestListController::buildRow() rejects invalid sequence "\xc2\""', TRUE);
+    $tests[] = array("Fooÿñ", "Fooÿñ", 'EntityTestListController::buildRow() accepts valid sequence "Fooÿñ"');
 
     // Checks that special characters are escaped.
-    $tests[] = array("<script>", '&lt;script&gt;', 'EntityListController::buildRow() escapes &lt;script&gt;');
-    $tests[] = array('<>&"\'', '&lt;&gt;&amp;&quot;&#039;', 'EntityListController::buildRow() escapes reserved HTML characters.');
+    $tests[] = array("<script>", '&lt;script&gt;', 'EntityTestListController::buildRow() escapes &lt;script&gt;');
+    $tests[] = array('<>&"\'', '&lt;&gt;&amp;&quot;&#039;', 'EntityTestListController::buildRow() escapes reserved HTML characters.');
 
     return $tests;