Per #2239009-29: Remove public direct usage of the '_system_path' request attribute, Raw::getArgument() and RequestPath::evaluate() are the two places where we still have functionality intrinsically tied to the internal path of the route we're on. However, we can change it from using an internal attribute (_system_path) to using a proper API by injecting both RouteMatch and UrlGenerator, and leveraging UrlGenerator's getPathFromRoute(). At least that way, we can unify all code still dealing with internal path-based functionality to this one API. We may want to change the will be removed before Drupal 8.0 portion of its current @deprecated comment, however, if we plan to continue supporting plugins like RequestPath and Raw.

CommentFileSizeAuthor
#12 interdiff-2293581-7-12.txt5.58 KBmitsuroseba
#12 drupal-use_getPathFromRoute-2293581-12.patch5.83 KBmitsuroseba
#7 drupal-use_getPathFromRoute-2293581-7.patch1.89 KBmitsuroseba

Comments

RavindraSingh’s picture

RavindraSingh commented
Assigned: Unassigned » RavindraSingh
xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Normal » Major
Issue tags: +beta target

Hi @RavindraSingh, are you planning to create a patch for this issue?

Bumping priority per #2239009: Remove public direct usage of the '_system_path' request attribute.

RavindraSingh’s picture

RavindraSingh commented

Yes, I am planing to create a patch for this issue.

xjm’s picture

xjm
she/her
Primary language English
commented

Excellent, thanks! :)

dawehner’s picture

dawehner
Primary language German
commented

#2302065: Be able to pull of the current path from the request/route match is kind of a related issue. The question is whether we really want to run the url generator, even we do have the information already available in our system.

nitvirus’s picture

nitvirus commented

Hi,
I am feeling quite noob here.

Do we need to use UrlGenerator::getPathFromRoute instead of _system_path , wherever _system_path is coming?

Thanks,
Nitish

mitsuroseba’s picture

mitsuroseba commented
Status: Active » Needs review
StatusFileSize
new drupal-use_getPathFromRoute-2293581-7.patch1.89 KB

Status: Needs review » Needs work

The last submitted patch, 7: drupal-use_getPathFromRoute-2293581-7.patch, failed testing.

Status: Needs work » Needs review

mitsuroseba queued 7: drupal-use_getPathFromRoute-2293581-7.patch for re-testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
--- a/core/modules/system/src/Plugin/Condition/RequestPath.php
+++ b/core/modules/system/src/Plugin/Condition/RequestPath.php

--- a/core/modules/views/src/Plugin/views/argument_default/Raw.php
+++ b/core/modules/views/src/Plugin/views/argument_default/Raw.php

These services should be injected.

Status: Needs review » Needs work

The last submitted patch, 7: drupal-use_getPathFromRoute-2293581-7.patch, failed testing.

mitsuroseba’s picture

mitsuroseba commented
Status: Needs work » Needs review
StatusFileSize
new drupal-use_getPathFromRoute-2293581-12.patch5.83 KB
new interdiff-2293581-7-12.txt5.58 KB

Thank you for review. Here new patch.

Status: Needs review » Needs work

The last submitted patch, 12: drupal-use_getPathFromRoute-2293581-12.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

I really dislke UrlGenerator here, in cases where we do have the path still, as it is part of the current request.
#2302065: Be able to pull of the current path from the request/route match partly solves that problem by using the route match. Once we have the route match available in those places, we could no longer
need the workaround added in this issue. ... Yes generating an URL which we kinda already know is a workaround, especially given that getPathFromRoute is marked as deprecated.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Closed (duplicate)

So yeah I think this issue is not needed. either.