From 401bd662cb145b01374fc3b84c8ccc6854c6ebc1 Mon Sep 17 00:00:00 2001 From: Lee Rowlands Date: Fri, 25 Aug 2017 16:18:29 +1000 Subject: [PATCH] Issue #2767857 by Berdir, tstoeckler: Add destination to edit, delete, enable, disable links in entity list builders --- .../Config/Entity/ConfigEntityListBuilder.php | 4 ++-- .../Drupal/Core/Entity/EntityListBuilder.php | 21 +++++++++++++++-- .../src/Functional/ConfigurationTest.php | 9 ++++---- .../src/BlockContentListBuilder.php | 14 ----------- .../config/src/Tests/ConfigEntityListTest.php | 13 +++++++---- .../tests/src/Functional/FilterAdminTest.php | 14 ++++------- core/modules/node/src/NodeListBuilder.php | 20 ---------------- .../NodeActionsConfigurationTest.php | 8 +++---- .../Plugin/views/field/EntityOperations.php | 4 ++-- .../Handler/FieldEntityOperationsTest.php | 5 +++- .../Core/Entity/EntityListBuilderTest.php | 23 ++++++++++++++++--- 11 files changed, 68 insertions(+), 67 deletions(-) diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityListBuilder.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityListBuilder.php index 38b7fa87a15..e26a03ca85e 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityListBuilder.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityListBuilder.php @@ -37,14 +37,14 @@ class ConfigEntityListBuilder extends EntityListBuilder { $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')), ]; } } diff --git a/core/lib/Drupal/Core/Entity/EntityListBuilder.php b/core/lib/Drupal/Core/Entity/EntityListBuilder.php index 7f46915ae98..c1654362d0f 100644 --- a/core/lib/Drupal/Core/Entity/EntityListBuilder.php +++ b/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 @@ use Symfony\Component\DependencyInjection\ContainerInterface; */ class EntityListBuilder extends EntityHandlerBase implements EntityListBuilderInterface, EntityHandlerInterface { + use RedirectDestinationTrait; + /** * The entity storage class. * @@ -143,14 +147,14 @@ class EntityListBuilder extends EntityHandlerBase implements EntityListBuilderIn $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 @@ class EntityListBuilder extends EntityHandlerBase implements EntityListBuilderIn 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()]); + } + } diff --git a/core/modules/action/tests/src/Functional/ConfigurationTest.php b/core/modules/action/tests/src/Functional/ConfigurationTest.php index 472ba17125c..a260c079cf4 100644 --- a/core/modules/action/tests/src/Functional/ConfigurationTest.php +++ b/core/modules/action/tests/src/Functional/ConfigurationTest.php @@ -44,14 +44,15 @@ class ConfigurationTest extends BrowserTestBase { $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 @@ class ConfigurationTest extends BrowserTestBase { $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 @@ class ConfigurationTest extends BrowserTestBase { $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.'); } diff --git a/core/modules/block_content/src/BlockContentListBuilder.php b/core/modules/block_content/src/BlockContentListBuilder.php index 96259173cdd..7a4bdfc4c80 100644 --- a/core/modules/block_content/src/BlockContentListBuilder.php +++ b/core/modules/block_content/src/BlockContentListBuilder.php @@ -4,7 +4,6 @@ namespace Drupal\block_content; 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 @@ use Drupal\Core\Routing\RedirectDestinationTrait; */ class BlockContentListBuilder extends EntityListBuilder { - use RedirectDestinationTrait; - /** * {@inheritdoc} */ @@ -31,15 +28,4 @@ class BlockContentListBuilder extends EntityListBuilder { 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; - } - } diff --git a/core/modules/config/src/Tests/ConfigEntityListTest.php b/core/modules/config/src/Tests/ConfigEntityListTest.php index e9950ea424b..46c0c064b6b 100644 --- a/core/modules/config/src/Tests/ConfigEntityListTest.php +++ b/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 @@ use Drupal\Core\Entity\EntityStorageInterface; */ class ConfigEntityListTest extends WebTestBase { + use RedirectDestinationTrait; + /** * Modules to enable. * @@ -54,17 +57,17 @@ class ConfigEntityListTest extends WebTestBase { '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 @@ class ConfigEntityListTest extends WebTestBase { '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()), ], ]; diff --git a/core/modules/filter/tests/src/Functional/FilterAdminTest.php b/core/modules/filter/tests/src/Functional/FilterAdminTest.php index 21431e7969a..a6e429304e9 100644 --- a/core/modules/filter/tests/src/Functional/FilterAdminTest.php +++ b/core/modules/filter/tests/src/Functional/FilterAdminTest.php @@ -4,6 +4,7 @@ namespace Drupal\Tests\filter\Functional; 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 @@ class FilterAdminTest extends BrowserTestBase { // 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')); diff --git a/core/modules/node/src/NodeListBuilder.php b/core/modules/node/src/NodeListBuilder.php index 4dbdf27bf21..eac48fda31a 100644 --- a/core/modules/node/src/NodeListBuilder.php +++ b/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 @@ class NodeListBuilder extends EntityListBuilder { 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; - } - } diff --git a/core/modules/node/tests/src/Functional/NodeActionsConfigurationTest.php b/core/modules/node/tests/src/Functional/NodeActionsConfigurationTest.php index fbfda8f169b..c2c7617bf68 100644 --- a/core/modules/node/tests/src/Functional/NodeActionsConfigurationTest.php +++ b/core/modules/node/tests/src/Functional/NodeActionsConfigurationTest.php @@ -43,14 +43,14 @@ class NodeActionsConfigurationTest extends BrowserTestBase { $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 @@ class NodeActionsConfigurationTest extends BrowserTestBase { $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 @@ class NodeActionsConfigurationTest extends BrowserTestBase { $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.'); } diff --git a/core/modules/views/src/Plugin/views/field/EntityOperations.php b/core/modules/views/src/Plugin/views/field/EntityOperations.php index c8a307bd1db..4ca28f6d924 100644 --- a/core/modules/views/src/Plugin/views/field/EntityOperations.php +++ b/core/modules/views/src/Plugin/views/field/EntityOperations.php @@ -84,7 +84,7 @@ class EntityOperations extends FieldPluginBase { $options = parent::defineOptions(); $options['destination'] = [ - 'default' => TRUE, + 'default' => FALSE, ]; return $options; @@ -99,7 +99,7 @@ class EntityOperations extends FieldPluginBase { $form['destination'] = [ '#type' => 'checkbox', '#title' => $this->t('Include destination'), - '#description' => $this->t('Include a destination parameter in the link to return the user to the original view upon completing the link action.'), + '#description' => $this->t('Enforce a destination 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'], ]; } diff --git a/core/modules/views/tests/src/Functional/Handler/FieldEntityOperationsTest.php b/core/modules/views/tests/src/Functional/Handler/FieldEntityOperationsTest.php index 653ce11527e..7ced4f7071e 100644 --- a/core/modules/views/tests/src/Functional/Handler/FieldEntityOperationsTest.php +++ b/core/modules/views/tests/src/Functional/Handler/FieldEntityOperationsTest.php @@ -73,7 +73,10 @@ class FieldEntityOperationsTest extends ViewTestBase { $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). diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php index 44e771ceaa5..ad68e2df68e 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityListBuilderTest.php @@ -11,6 +11,7 @@ use Drupal\Core\Access\AccessResult; 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 @@ class EntityListBuilderTest extends UnitTestCase { $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 @@ class EntityListBuilderTest extends UnitTestCase { $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);