Problem/Motivation

The path admin UI lives at admin/config/search/path, but the filtering uses admin/config/search/path/list/%.
That got lost in the local task conversions, but was not caught by tests, because we don't have any.

Proposed resolution

Fix the routing
Write tests

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
2.52 KB
4.6 KB

#1987802: Convert path_admin_overview() to a new style controller will port this to OO code, but we need to have a working baseline first.

tim.plunkett’s picture

+++ b/core/modules/path/lib/Drupal/path/Controller/PathController.php
@@ -15,7 +15,15 @@ class PathController {
+  public function adminOverview() {
+    module_load_include('admin.inc', 'path');
+    return path_admin_overview();
...
+  public function adminOverviewFiltered($keys) {

This just splits the filtered and unfiltered routes to separate methods, these will be utilized further in the conversion issue (right now they call the same function with and without $keys).

dawehner’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/path/lib/Drupal/path/Tests/PathAdminTest.php
    @@ -0,0 +1,92 @@
    +   * Tests the path cache.
    ...
    +  public function testPathCache() {
    ...
    +    $node1 = $this->drupalCreateNode();
    

    That test naming is a c&p

  2. +++ b/core/modules/path/path.routing.yml
    @@ -7,11 +7,18 @@ path.delete:
     path.admin_overview:
    -  path: '/admin/config/search/path/{keys}'
    +  path: '/admin/config/search/path'
       defaults:
         _title: 'URL aliases'
         _content: '\Drupal\path\Controller\PathController::adminOverview'
    -    keys: NULL
    +  requirements:
    +    _permission: 'administer url aliases'
    +
    +path.admin_overview_filter:
    +  path: '/admin/config/search/path/list/{keys}'
    +  defaults:
    +    _title: 'URL aliases'
    +    _content: '\Drupal\path\Controller\PathController::adminOverviewFiltered'
       requirements:
         _permission: 'administer url aliases'
     
    

    Keys: NULL ensures an optional part of the path.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
4.63 KB

1. Heh, laziness--

2. This is purposeful.

As discussed on IRC

in D7 it is admin/config/search/path and admin/config/search/path/list/%
someone messed up and ported it to admin/config/search/path/%
which doesn't work.
they forgot the /list/
otherwise you couldn't have an alias called 'add' or 'delete'

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Coool

The last submitted patch, 1: path-2244757-1-FAIL.patch, failed testing.

tim.plunkett’s picture

Issue tags: +Quick fix

Last fail was the tests-only patch.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit e6ac9ec on 8.x by webchick:
    Issue #2244757 by tim.plunkett: Path Admin UI is broken and has no test...

Status: Fixed » Closed (fixed)

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