Updated: Comment 0

Problem/Motivation

Let's get rid of request attributes.

Proposed resolution

Move _view_argument_map to route options instead of attributes. Attributes are information derived from the request, while _view_argument_map is just metainformation on top of a route.

Move _view_argument_map from the 'defaults' information of view routes into the 'options' of the routes.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#1 vdc_route_options-2227555-1.patch6.26 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review
FileSize
vdc_route_options-2227555-1.patch6.26 KB

Here we go.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Looks good to me. Looks like it needs an issue summary though.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Parent issue: » #2124749: [meta] Stop using $request->attributes->get(MAGIC_KEY) as a public API
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

/me gets so sad if people don't review small patches

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Assigned: dawehner » Unassigned

.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue summary: View changes
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php
@@ -200,7 +200,7 @@ protected function getRoute($view_id, $display_id) {
-    $route->setDefault('_view_argument_map', $argument_map);
+    $route->setOption('_view_argument_map', $argument_map);

+++ b/core/modules/views/lib/Drupal/views/Routing/ViewPageController.php
@@ -74,7 +75,7 @@ public function handle(Request $request) {
-    $map = $request->attributes->get('_view_argument_map', array());
+    $map = $request->attributes->get(RouteObjectInterface::ROUTE_OBJECT)->getOption('_view_argument_map', array());

This is a big improvement semantically, and its all internal anyway, so the extra verbosity is not a problem.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Assigned: Unassigned » alexpott

Sorry, I can't even begin to fathom how to read that new code, so passing this one onto alexpott.

If this kind of code is what #2124749: [meta] Stop using $request->attributes->get(MAGIC_KEY) as a public API is going to bring us all over, though, colour me concerned. :(

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Well, this usecase would never be used by any custom/contrib module besides view so I don't give a shit about what people consider as DX.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Makes sense to me - and yep this is not a dx issue since it is all internal to views. I agree with Tim this is much more semantically correct.

Committed 52da190 and pushed to 8.x. Thanks!

  • Commit 52da190 on 8.x by alexpott: 
    Issue #2227555 by dawehner: Use route options instead of route defaults...

Status: Fixed » Closed (fixed)

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