Issue #2767857 by Berdir, tstoeckler: Add destination to edit, delete, enable,...

Issue #2767857 by Berdir, tstoeckler: Add destination to edit, delete, enable, disable links in entity list builders

core/lib/Drupal/Core/Config/Entity/ConfigEntityListBuilder.php

@@ -37,14 +37,14 @@ public function getDefaultOperations(EntityInterface $entity) {
         $operations['enable'] = [
           'title' => t('Enable'),
           'weight' => -10,
-          'url' => $entity->urlInfo('enable'),
+          'url' => $this->ensureDestination($entity->toUrl('enable')),
         ];
       }
       elseif ($entity->hasLinkTemplate('disable')) {
         $operations['disable'] = [
           'title' => t('Disable'),
           'weight' => 40,
-          'url' => $entity->urlInfo('disable'),
+          'url' => $this->ensureDestination($entity->toUrl('disable')),
         ];
       }
     }

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

@@ -2,6 +2,8 @@
 
 namespace Drupal\Core\Entity;
 
+use Drupal\Core\Routing\RedirectDestinationTrait;
+use Drupal\Core\Url;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -11,6 +13,8 @@
  */
 class EntityListBuilder extends EntityHandlerBase implements EntityListBuilderInterface, EntityHandlerInterface {
 
+  use RedirectDestinationTrait;
+
   /**
    * The entity storage class.
    *
@@ -143,14 +147,14 @@ protected function getDefaultOperations(EntityInterface $entity) {
       $operations['edit'] = [
         'title' => $this->t('Edit'),
         'weight' => 10,
-        'url' => $entity->urlInfo('edit-form'),
+        'url' => $this->ensureDestination($entity->toUrl('edit-form')),
       ];
     }
     if ($entity->access('delete') && $entity->hasLinkTemplate('delete-form')) {
       $operations['delete'] = [
         'title' => $this->t('Delete'),
         'weight' => 100,
-        'url' => $entity->urlInfo('delete-form'),
+        'url' => $this->ensureDestination($entity->toUrl('delete-form')),
       ];
     }
 
@@ -250,4 +254,17 @@ protected function getTitle() {
     return;
   }
 
+  /**
+   * Ensures that a destination is present on the given URL.
+   *
+   * @param \Drupal\Core\Url $url
+   *   The URL object to which the destination should be added.
+   *
+   * @return \Drupal\Core\Url
+   *   The updated URL object.
+   */
+  protected function ensureDestination(Url $url) {
+    return $url->mergeOptions(['query' => $this->getRedirectDestination()->getAsArray()]);
+  }
+
 }

core/modules/action/tests/src/Functional/ConfigurationTest.php

@@ -44,14 +44,15 @@ public function testActionConfiguration() {
     $this->drupalPostForm('admin/config/system/actions/add/' . Crypt::hashBase64('action_goto_action'), $edit, t('Save'));
     $this->assertResponse(200);
 
+    $action_id = $edit['id'];
+
     // Make sure that the new complex action was saved properly.
     $this->assertText(t('The action has been successfully saved.'), "Make sure we get a confirmation that we've successfully saved the complex action.");
     $this->assertText($action_label, "Make sure the action label appears on the configuration page after we've saved the complex action.");
 
     // Make another POST request to the action edit page.
     $this->clickLink(t('Configure'));
-    preg_match('|admin/config/system/actions/configure/(.+)|', $this->getUrl(), $matches);
-    $aid = $matches[1];
+
     $edit = [];
     $new_action_label = $this->randomMachineName();
     $edit['label'] = $new_action_label;
@@ -73,7 +74,7 @@ public function testActionConfiguration() {
     $this->clickLink(t('Delete'));
     $this->assertResponse(200);
     $edit = [];
-    $this->drupalPostForm("admin/config/system/actions/configure/$aid/delete", $edit, t('Delete'));
+    $this->drupalPostForm(NULL, $edit, t('Delete'));
     $this->assertResponse(200);
 
     // Make sure that the action was actually deleted.
@@ -82,7 +83,7 @@ public function testActionConfiguration() {
     $this->assertResponse(200);
     $this->assertNoText($new_action_label, "Make sure the action label does not appear on the overview page after we've deleted the action.");
 
-    $action = Action::load($aid);
+    $action = Action::load($action_id);
     $this->assertFalse($action, 'Make sure the action is gone after being deleted.');
   }
 

core/modules/block_content/src/BlockContentListBuilder.php

@@ -4,7 +4,6 @@
 
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityListBuilder;
-use Drupal\Core\Routing\RedirectDestinationTrait;
 
 /**
  * Defines a class to build a listing of custom block entities.
@@ -13,8 +12,6 @@
  */
 class BlockContentListBuilder extends EntityListBuilder {
 
-  use RedirectDestinationTrait;
-
   /**
    * {@inheritdoc}
    */
@@ -31,15 +28,4 @@ public function buildRow(EntityInterface $entity) {
     return $row + parent::buildRow($entity);
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function getDefaultOperations(EntityInterface $entity) {
-    $operations = parent::getDefaultOperations($entity);
-    if (isset($operations['edit'])) {
-      $operations['edit']['query']['destination'] = $this->getRedirectDestination()->get();
-    }
-    return $operations;
-  }
-
 }

core/modules/config/src/Tests/ConfigEntityListTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\config\Tests;
 
+use Drupal\Core\Routing\RedirectDestinationTrait;
 use Drupal\simpletest\WebTestBase;
 use Drupal\config_test\Entity\ConfigTest;
 use Drupal\Core\Entity\EntityStorageInterface;
@@ -13,6 +14,8 @@
  */
 class ConfigEntityListTest extends WebTestBase {
 
+  use RedirectDestinationTrait;
+
   /**
    * Modules to enable.
    *
@@ -54,17 +57,17 @@ public function testList() {
       'edit' => [
         'title' => t('Edit'),
         'weight' => 10,
-        'url' => $entity->urlInfo(),
+        'url' => $entity->toUrl()->setOption('query', $this->getRedirectDestination()->getAsArray()),
       ],
       'disable' => [
         'title' => t('Disable'),
         'weight' => 40,
-        'url' => $entity->urlInfo('disable'),
+        'url' => $entity->toUrl('disable')->setOption('query', $this->getRedirectDestination()->getAsArray()),
       ],
       'delete' => [
         'title' => t('Delete'),
         'weight' => 100,
-        'url' => $entity->urlInfo('delete-form'),
+        'url' => $entity->toUrl('delete-form')->setOption('query', $this->getRedirectDestination()->getAsArray()),
       ],
     ];
 
@@ -129,12 +132,12 @@ public function testList() {
       'edit' => [
         'title' => t('Edit'),
         'weight' => 10,
-        'url' => $entity->urlInfo(),
+        'url' => $entity->toUrl()->setOption('query', $this->getRedirectDestination()->getAsArray()),
       ],
       'delete' => [
         'title' => t('Delete'),
         'weight' => 100,
-        'url' => $entity->urlInfo('delete-form'),
+        'url' => $entity->toUrl('delete-form')->setOption('query', $this->getRedirectDestination()->getAsArray()),
       ],
     ];
 

core/modules/filter/tests/src/Functional/FilterAdminTest.php

@@ -4,6 +4,7 @@
 
 use Drupal\Component\Utility\Html;
 use Drupal\Component\Utility\Unicode;
+use Drupal\Core\Url;
 use Drupal\filter\Entity\FilterFormat;
 use Drupal\node\Entity\Node;
 use Drupal\node\Entity\NodeType;
@@ -135,16 +136,9 @@ public function testFormatAdmin() {
 
     // Edit text format.
     $this->drupalGet('admin/config/content/formats');
-    // Cannot use the assertNoLinkByHref method as it does partial url matching
-    // and 'admin/config/content/formats/manage/' . $format_id . '/disable'
-    // exists.
-    // @todo: See https://www.drupal.org/node/2031223 for the above.
-    $edit_link = $this->xpath('//a[@href=:href]', [
-      ':href' => \Drupal::url('entity.filter_format.edit_form', ['filter_format' => $format_id])
-    ]);
-    $this->assertNotEmpty($edit_link, format_string('Link href %href found.',
-      ['%href' => 'admin/config/content/formats/manage/' . $format_id]
-    ));
+    $destination = Url::fromRoute('filter.admin_overview')->toString();
+    $edit_href = Url::fromRoute('entity.filter_format.edit_form', ['filter_format' => $format_id], ['query' => ['destination' => $destination]])->toString();
+    $this->assertSession()->linkByHrefExists($edit_href);
     $this->drupalGet('admin/config/content/formats/manage/' . $format_id);
     $this->drupalPostForm(NULL, [], t('Save configuration'));
 

core/modules/node/src/NodeListBuilder.php

@@ -25,13 +25,6 @@ class NodeListBuilder extends EntityListBuilder {
    */
   protected $dateFormatter;
 
-  /**
-   * The redirect destination service.
-   *
-   * @var \Drupal\Core\Routing\RedirectDestinationInterface
-   */
-  protected $redirectDestination;
-
   /**
    * Constructs a new NodeListBuilder object.
    *
@@ -128,17 +121,4 @@ public function buildRow(EntityInterface $entity) {
     return $row + parent::buildRow($entity);
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function getDefaultOperations(EntityInterface $entity) {
-    $operations = parent::getDefaultOperations($entity);
-
-    $destination = $this->redirectDestination->getAsArray();
-    foreach ($operations as $key => $operation) {
-      $operations[$key]['query'] = $destination;
-    }
-    return $operations;
-  }
-
 }

core/modules/node/tests/src/Functional/NodeActionsConfigurationTest.php

@@ -43,14 +43,14 @@ public function testAssignOwnerNodeActionConfiguration() {
     $this->drupalPostForm('admin/config/system/actions/add/' . Crypt::hashBase64('node_assign_owner_action'), $edit, t('Save'));
     $this->assertResponse(200);
 
+    $action_id = $edit['id'];
+
     // Make sure that the new action was saved properly.
     $this->assertText(t('The action has been successfully saved.'), 'The node_assign_owner_action action has been successfully saved.');
     $this->assertText($action_label, 'The label of the node_assign_owner_action action appears on the actions administration page after saving.');
 
     // Make another POST request to the action edit page.
     $this->clickLink(t('Configure'));
-    preg_match('|admin/config/system/actions/configure/(.+)|', $this->getUrl(), $matches);
-    $aid = $matches[1];
     $edit = [];
     $new_action_label = $this->randomMachineName();
     $edit['label'] = $new_action_label;
@@ -68,7 +68,7 @@ public function testAssignOwnerNodeActionConfiguration() {
     $this->clickLink(t('Delete'));
     $this->assertResponse(200);
     $edit = [];
-    $this->drupalPostForm("admin/config/system/actions/configure/$aid/delete", $edit, t('Delete'));
+    $this->drupalPostForm(NULL, $edit, t('Delete'));
     $this->assertResponse(200);
 
     // Make sure that the action was actually deleted.
@@ -77,7 +77,7 @@ public function testAssignOwnerNodeActionConfiguration() {
     $this->assertResponse(200);
     $this->assertNoText($new_action_label, 'The label for the node_assign_owner_action action does not appear on the actions administration page after deleting.');
 
-    $action = Action::load($aid);
+    $action = Action::load($action_id);
     $this->assertFalse($action, 'The node_assign_owner_action action is not available after being deleted.');
   }
 

core/modules/views/src/Plugin/views/field/EntityOperations.php

@@ -84,7 +84,7 @@ public function defineOptions() {
     $options = parent::defineOptions();
 
     $options['destination'] = [
-      'default' => TRUE,
+      'default' => FALSE,
     ];
 
     return $options;
@@ -99,7 +99,7 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
     $form['destination'] = [
       '#type' => 'checkbox',
       '#title' => $this->t('Include destination'),
-      '#description' => $this->t('Include a <code>destination</code> parameter in the link to return the user to the original view upon completing the link action.'),
+      '#description' => $this->t('Enforce a <code>destination</code> parameter in the link to return the user to the original view upon completing the link action. Most operations include a destination by default and this setting is no longer needed.'),
       '#default_value' => $this->options['destination'],
     ];
   }

core/modules/views/tests/src/Functional/Handler/FieldEntityOperationsTest.php

@@ -73,7 +73,10 @@ public function testEntityOperations() {
         $this->assertTrue(count($operations) > 0, 'There are operations.');
         foreach ($operations as $operation) {
           $expected_destination = Url::fromUri('internal:/test-entity-operations')->toString();
-          $result = $this->xpath('//ul[contains(@class, dropbutton)]/li/a[@href=:path and text()=:title]', [':path' => $operation['url']->toString() . '?destination=' . $expected_destination, ':title' => (string) $operation['title']]);
+          // Update destination property of the URL as generating it in the
+          // test would by default point to the frontpage.
+          $operation['url']->setOption('query', ['destination' => $expected_destination]);
+          $result = $this->xpath('//ul[contains(@class, dropbutton)]/li/a[@href=:path and text()=:title]', [':path' => $operation['url']->toString(), ':title' => (string) $operation['title']]);
           $this->assertEqual(count($result), 1, t('Found entity @operation link with destination parameter.', ['@operation' => $operation['title']]));
           // Entities which were created in Hungarian should link to the Hungarian
           // edit form, others to the English one (which has no path prefix here).

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

@@ -11,6 +11,7 @@
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityListBuilder;
+use Drupal\Core\Routing\RedirectDestinationInterface;
 use Drupal\entity_test\EntityTestListBuilder;
 use Drupal\Tests\UnitTestCase;
 
@@ -62,6 +63,13 @@ class EntityListBuilderTest extends UnitTestCase {
    */
   protected $role;
 
+  /**
+   * The redirect destination service.
+   *
+   * @var \Drupal\Core\Routing\RedirectDestinationInterface|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $redirectDestination;
+
   /**
    * The EntityListBuilder object to test.
    *
@@ -80,7 +88,8 @@ protected function setUp() {
     $this->moduleHandler = $this->getMock('\Drupal\Core\Extension\ModuleHandlerInterface');
     $this->entityType = $this->getMock('\Drupal\Core\Entity\EntityTypeInterface');
     $this->translationManager = $this->getMock('\Drupal\Core\StringTranslation\TranslationInterface');
-    $this->entityListBuilder = new TestEntityListBuilder($this->entityType, $this->roleStorage, $this->moduleHandler);
+    $this->entityListBuilder = new TestEntityListBuilder($this->entityType, $this->roleStorage);
+    $this->redirectDestination = $this->getMock(RedirectDestinationInterface::class);
     $this->container = new ContainerBuilder();
     \Drupal::setContainer($this->container);
   }
@@ -117,12 +126,20 @@ public function testGetOperations() {
     $url->expects($this->any())
       ->method('toArray')
       ->will($this->returnValue([]));
+    $url->expects($this->atLeastOnce())
+      ->method('mergeOptions')
+      ->with(['query' => ['destination' => '/foo/bar']]);
     $this->role->expects($this->any())
-      ->method('urlInfo')
+      ->method('toUrl')
       ->will($this->returnValue($url));
 
-    $list = new EntityListBuilder($this->entityType, $this->roleStorage, $this->moduleHandler);
+    $this->redirectDestination->expects($this->atLeastOnce())
+      ->method('getAsArray')
+      ->willReturn(['destination' => '/foo/bar']);
+
+    $list = new EntityListBuilder($this->entityType, $this->roleStorage);
     $list->setStringTranslation($this->translationManager);
+    $list->setRedirectDestination($this->redirectDestination);
 
     $operations = $list->getOperations($this->role);
     $this->assertInternalType('array', $operations);