Problem/Motivation

Noticed this while trying to write a test for #2396253: Respect format configuration on REST views display.
When creating a REST display the preview button is not reacting.

PHP error being thrown in ajax callback:

PHP Fatal error:  Method Drupal\Core\Url::__toString() must not throw an exception in /var/www/html/d8.local/drupal/core/modules/views_ui/src/ViewUI.php on line 0

When I apply the patch in #2416971: Remove Url::__toString() a more helpful error is thrown:

Uncaught PHP Exception Symfony\Component\Routing\Exception\RouteNotFoundException: "Route "views.testrest.rest_export_1" does not exist." at /var/www/html/d8.local/drupal/core/lib/Drupal/Core/Routing/RouteProvider.php line 145

Steps to reproduce

  1. Enable REST module
  2. Create a new content view
  3. Add a rest display
  4. Hit the Preview button

Proposed resolution

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

geertvd’s picture

Issue summary: View changes
dawehner’s picture

Can you please apply the patch from #2416971: Remove Url::__toString() and have a look what the thrown exception then looks like?

geertvd’s picture

Issue summary: View changes

After applying the patch from #2416971: Remove Url::__toString():

Uncaught PHP Exception Symfony\Component\Routing\Exception\RouteNotFoundException: "Route "views.testrest.rest_export_1" does not exist." at /var/www/html/d8.local/drupal/core/lib/Drupal/Core/Routing/RouteProvider.php line 145
geertvd’s picture

Issue summary: View changes
geertvd’s picture

Status: Active » Needs review
FileSize
715 bytes

Something like this fixes that, no idea if this is going to break something else. Let's see what the testbot says.

geertvd’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
+++ b/core/modules/views/src/Plugin/views/display/PathPluginBase.php
@@ -309,7 +307,7 @@ public function getMenuLinks() {
-          'route_name' => isset($view_route_names[$view_id_display]) ? $view_route_names[$view_id_display] : "view.$view_id_display",
+          'route_name' => $this->getRouteName(),

@@ -490,11 +488,28 @@ public function validate() {
+  /**
+   * {@inheritdoc}
+   */
+  public function getRouteName() {
+    $view_id = $this->view->storage->id();
+    $display_id = $this->display['id'];
+    $view_route_key = "$view_id.$display_id";
+
+    // Check for overridden route names.
+    $view_route_names = $this->getAlteredRouteNames();
+
+    return (isset($view_route_names[$view_route_key]) ? $view_route_names[$view_route_key] : "views.$view_route_key");
+  }
+

Ah, seems like a mistake in the committed patch in #2409209: Replace all _url() calls beside the one in _l(). So this is the right way to fix it.
I guess this will definitely need test coverage since this broke once and it might break again. I'll try to pick that up together with #2396253: Respect format configuration on REST views display

geertvd’s picture

Status: Needs work » Needs review
FileSize
1.08 KB
1.78 KB

The last submitted patch, 7: 2443119-7-test.patch, failed testing.

dawehner’s picture

Great catch! It would be great if you could write some unit tests on top of that, see core/modules/views/src/Plugin/views/display/PathPluginBase.php

geertvd’s picture

FileSize
2.2 KB
2.9 KB

First time writing one of those. Is something like this ok?

The last submitted patch, 10: 2443119-10-test.patch, failed testing.

koence’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
320.69 KB
157.11 KB

Tests were added for getRouteName and views interface for rest export

test result before patch

test result after patch

side note: I did notice that the views preview function for a new display only works AFTER the new display is saved.
I will log this issue if it hasn't been logged already.

koence’s picture

Sutharsan’s picture

Issue tags: -Needs tests
alexpott’s picture

Can we get a followup to prefix all the dynamic routes with views.view instead of view and fix RouteSubscriber::alterRoutes() - it does not need to remove the route from the collection since this is already handled by PathPluginBase::alterRoutes. Plus I think we should introduce a route prefix constant somewhere so that this is easier to maintain. Once the followup exists, I'll commit.

geertvd’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 7f4abc7 and pushed to 8.0.x. Thanks!

  • alexpott committed 7f4abc7 on 8.0.x
    Issue #2443119 by geertvd: Views preview not working for REST display
    

Status: Fixed » Closed (fixed)

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