Follow-up to #2010132: Canonical taxonomy term link for forum vocabulary is broken

Problem/Motivation

Trying to override taxonomy term URL generation for forum pages I found that there's only a path ($route->getPath()) to identify route. But forum module needs to alter only "canonical" (default) term link (route)

Proposed resolution

Pass $route_name as third argument

Remaining tasks

TBD

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
We have manually tested the patch and confirmed that it has not been rolled. Novice Instructions Yes
Reroll the patch. Instructions Yes
Draft a change record for the API changes Instructions

User interface changes

no

API changes

new argument in interface

CommentFileSizeAuthor
#5 2283851-path-processor-5.patch6.26 KBandypost
#5 interdiff.txt3.16 KBandypost
#3 2283851-path-processor-3.patch6.21 KBandypost
#3 interdiff.txt2.51 KBandypost
#1 2283851-1.patch3.69 KBandypost

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new 2283851-1.patch3.69 KB

Status: Needs review » Needs work

The last submitted patch, 1: 2283851-1.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt2.51 KB
new 2283851-path-processor-3.patch6.21 KB

Fix broken tests

damiankloip’s picture

damiankloip commented 
+++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php
@@ -37,7 +37,7 @@ function __construct(CsrfTokenGenerator $csrf_token) {
+  public function processOutbound(Route $route, array &$parameters, $name) {

This param should probably be called $route_name, I think it is called this almost everywhere else.

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt3.16 KB
new 2283851-path-processor-5.patch6.26 KB

Yes $route_name is more consistent

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php
@@ -37,7 +37,7 @@ function __construct(CsrfTokenGenerator $csrf_token) {
+  public function processOutbound(Route $route, array &$parameters, $route_name) {

tbh I would expect route_name, route_object and parameters as last argument.

dawehner’s picture

dawehner
Primary language German
commented

Note: Earlier versions of #2302563: Access check Url objects actually expanded our route objects to contain the route names itself, which I think would be a sane idea. Did we considered to do that, instead of the "workaround" currently
proposed by this issue?

andypost’s picture

andypost
he/him
Primary language Russian
commented

@dawehner Not sure how fixed issue #7 related here but originally the issue is follow-up from #2010132: Canonical taxonomy term link for forum vocabulary is broken
That means that outbound route needs to know the name to effectively compare.
But that name does not exists now or could be optional in case route object is created manually... so no idea how to implement it right.

dawehner’s picture

dawehner
Primary language German
commented

@dawehner Not sure how fixed issue #7 related here but originally the issue is follow-up from #2010132: Convert taxonomy_term_uri() to $term->uri()

Please reread my answer. "Earlier version of ... actually expanded our route objects to contain the route names itself".

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Closed (duplicate)

Already fixed by #2346283: Add route name parameter to OutboundRouteProcessorInterface

damiankloip’s picture

damiankloip commented

@andypost, sorry - I totally forgot we had this issue!!

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Issue tags: -Novice, -Entity Field API, -DX (Developer Experience), -API cleanup +API clean-up

Removing spurious tags - https://www.drupal.org/node/1207020