Problem/Motivation

As we realized on #1851086: Replace admin/people with a View is that overriding existing paths in views is problematic.

Therefore selecting routes by pattern is helpful to allow altering of existing views.

Proposed resolution

Extend the RouteProviderInterface which to include a getRouteByPattern method.

Remaining tasks

DO IT, WRITE TEST, HOPE IT ACTUALLY WORKS

User interface changes

NON

API changes

Extends the interface

#1851086: Replace admin/people with a View

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
843 bytes

Do we agree so far on the interface?

Crell’s picture

Seems reasonable. I might suggest not duplicating the interface name for clarity.

I presume then our implementation would then just subcall to that method from getRoutesByRequest(), since it would be all the same logic anyway. (That's what we discussed in Portland.)

Onward[s]!

dawehner’s picture

Component: views.module » routing system
FileSize
5.27 KB

I introduced another protected method, which makes it even a bit cleaner.

Status: Needs review » Needs work

The last submitted patch, drupal-2004300-3.patch, failed testing.

Crell’s picture

+++ b/core/lib/Drupal/Core/Routing/RouteProvider.php
@@ -239,4 +219,47 @@ public function getCandidateOutlines(array $parts) {
+  /**
+   * {@inheritdoc}
+   */
+  public function getRoutesByPattern($pattern) {
+    $path = RouteCompiler::getPatternOutline($pattern);
+
+    return $this->getRoutesByPattern($path);
+  }

This looks suspiciously like an infinite loop. :-)

+++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteProviderTest.php
@@ -324,6 +324,9 @@ function testOutlinePathNoMatch() {
+      $routes = $provider->getRoutesByPattern($path);
+      $this->assertFalse($routes, 'No path found with this pattern.');
+

$routes is a RouteCollection, not array, so assertFalse() isn't going to work. If you want to ensure that it's empty you can do a count($routes) == 0.

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.25 KB
5.27 KB

Status: Needs review » Needs work
Issue tags: -WSCCI, -VDC

The last submitted patch, drupal-2004300-6.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

#6: drupal-2004300-6.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +WSCCI, +VDC

The last submitted patch, drupal-2004300-6.patch, failed testing.

xjm’s picture

Title: Extend the router provider interface » Extend the router provider interface so that Views can override routes
Category: task » bug
Priority: Normal » Critical

Clarifying why this is important. Views not being able to override routes is a critical regression.

dawehner’s picture

Status: Needs work » Needs review
FileSize
6.26 KB

Oh, I would have not thought that this could break it.

dawehner’s picture

FileSize
1007 bytes

Here is the interdiff.

Status: Needs review » Needs work
Issue tags: -WSCCI, -VDC

The last submitted patch, routing-2004300-11.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review

#11: routing-2004300-11.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, routing-2004300-11.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

#11: routing-2004300-11.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +WSCCI, +VDC

The last submitted patch, routing-2004300-11.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
977 bytes
7.21 KB

One less fail.

Status: Needs review » Needs work

The last submitted patch, routing-2004300-18.patch, failed testing.

tim.plunkett’s picture

+++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteProviderTest.phpundefined
@@ -348,7 +351,9 @@ function testSystemPathMatch() {
+    $this->assertEqual($routes_by_pattern, $routes);

This needs a message, because assertEqual is trying to var_dump the object for the message

Crell’s picture

The user password reset seems spurious. I had it crop up weirdly in the entity uri issue, too, but couldn't replicate locally. Probably transient testbot weirdness.

dawehner’s picture

Status: Needs work » Needs review
FileSize
864 bytes
7.29 KB

Thanks tim!!

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Nice, even better.

Crell’s picture

+1 from me as well.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed c707cf7 and pushed to 8.x. Thanks!

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