When using the [view:url] token when there is no URL for the current display and no linked display with a URL can be found (i.e. you only have a block display), an exception is thrown:

InvalidArgumentException: You cannot create a URL to a display without routes. in Drupal\views\ViewExecutable->getUrl()

Expected behaviour: empty token value, no exception.

CommentFileSizeAuthor
#13 view_url_token-2804375-13.patch3.69 KBLendude
#13 interdiff-2804375-7-13.txt963 bytesLendude
#7 view_url_token-2804375-7.patch3.66 KBLendude
#7 view_url_token-2804375-7-TEST_ONLY.patch2.83 KBLendude
#2 view_url_token-2804375-2.patch868 bytesLendude
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lendude created an issue. See original summary.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris for Triquanta commented
Status: Active » Needs review
Issue tags: +Needs tests
FileSize
view_url_token-2804375-2.patch868 bytes

Here is a fix for this, this still needs tests obviously.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

Should we maybe fix getUrl()?

Status: Needs review » Needs work

The last submitted patch, 2: view_url_token-2804375-2.patch, failed testing.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris for Triquanta commented

Well it seems to me that getUrl() is doing exactly what it's advertising:

   * @return \Drupal\Core\Url
   *   The URL of the current view.
   *
   * @throws \InvalidArgumentException
   *   Thrown when the current view doesn't have a route available.
   */
  public function getUrl($args = NULL, $display_id = NULL) {

So I'd say it's up to the code utilising getUrl to properly handle any exceptions. Changing getUrl would feel more like an API change, but *shrug*

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

Fair point, well then let's catch exceptions :)

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris for Triquanta commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
view_url_token-2804375-7-TEST_ONLY.patch2.83 KB
view_url_token-2804375-7.patch3.66 KB

well then let's catch exceptions :)

Hehehe, yeah lets :)

Test and a fix. No interdiff, totally different approach, it's now catching exceptions instead of trying to make sure there will be none.

The last submitted patch, 7: view_url_token-2804375-7-TEST_ONLY.patch, failed testing.

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
CreditAttribution: marcvangend as a volunteer commented

Thanks Len.

+++ b/core/modules/views/views.tokens.inc
@@ -101,9 +101,14 @@ function views_tokens($type, $tokens, array $data, array $options, BubbleableMet
+            // The view has no URL so we can't replace this token.

Do we really want to silently catch the exception and do nothing? Or would it be better to warn that the token was requested but the value could not be retrieved?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented 
  1. +++ b/core/modules/views/views.tokens.inc
@@ -101,9 +101,14 @@ function views_tokens($type, $tokens, array $data, array $options, BubbleableMet
+          catch (\InvalidArgumentException $e) {

    It is sad that we don't have a more specific domain exception :( 

  2. +++ b/core/modules/views/views.tokens.inc
@@ -101,9 +101,14 @@ function views_tokens($type, $tokens, array $data, array $options, BubbleableMet
+            // The view has no URL so we can't replace this token.

    Well, we could replace it with an empty string ... isn't that what is expect in token for this case?

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
CreditAttribution: marcvangend as a volunteer commented

I'm not sure about an empty string, the resulting string would probably not be any better than a string with a literal token in it. When I was debugging this with Len, we found that it happened only because the [view:url] token was actually being used (in the empty text, in my case). Basically it was a misconfigured view, so I think a warning message for the admin would be appropriate.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

Interesting, well we can certainly provide a logging message using watchdog_exception()

I would just follow what tokens usually do.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris for Triquanta commented
FileSize
interdiff-2804375-7-13.txt963 bytes
view_url_token-2804375-13.patch3.69 KB

I would just follow what tokens usually do.

I looked around a bit and in most cases this just means an empty token. I think a watchdog message is a little too much for a misconfigured token. It would have helped with debugging in our case but that was mostly because we were getting a fatal-500-in-the-background-ajax-call.

I think just making the token empty is a clear sign of what's going on, "we have no data to replace the token with".

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

This works for me

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Patch (to be ported)

Committed e12c685 and pushed to 8.3.x. Thanks!

Setting to patch to be ported for eventual cherry-pick to 8.2.x once 8.2.0 is released.

  • alexpott committed e12c685 on 8.3.x 
    Issue #2804375 by Lendude, dawehner: Exception thrown when using [view:...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Patch (to be ported) » Fixed

Committed 38adfc2 and pushed to 8.2.x. Thanks!

  • alexpott committed 38adfc2 on 8.2.x 
    Issue #2804375 by Lendude, dawehner: Exception thrown when using [view:...

Status: Fixed » Closed (fixed)

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