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

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
StatusFileSize
new715 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
StatusFileSize
new1.08 KB
new1.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

StatusFileSize
new2.2 KB
new2.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
StatusFileSize
new320.69 KB
new157.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.

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.