Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Comment | File | Size | Author |
---|---|---|---|
#31 | interdiff.txt | 4.29 KB | dawehner |
#31 | 2238981-31.patch | 10.22 KB | dawehner |
#28 | interdiff.txt | 1.5 KB | dawehner |
#28 | 2238981-28.patch | 8.06 KB | dawehner |
#25 | 2238981-25.patch | 8.16 KB | dawehner |
Comments
Comment #1
effulgentsia CreditAttribution: effulgentsia commentedBroadening scope to also include views_ui, field_ui, and content_translation. These are all the instances I could find in core/modules.
Comment #2
effulgentsia CreditAttribution: effulgentsia commentedPatch for #1.
Comment #4
dawehnerFor views, we can just use: #2227555: Use route options instead of route defaults for view_argument_map
Comment #5
dawehnerAll the other changes looks certainly reasonable and should have been like that in the first place.
Comment #6
dawehnerReroll
Comment #8
effulgentsia CreditAttribution: effulgentsia commentedViewsUIController::edit() passes $display_id in via $form_state rather than $form_state['build_info']['args']. Not sure if that's correct or not, but here's a fix/hack to accommodate that.
Comment #10
effulgentsia CreditAttribution: effulgentsia commentedLooks like we had some dead code getting in the way.
Comment #11
dawehnerRerolled against a recent rename of a lot of classes and removed more dead code in ViewPreviewForm
Is this exception just temporary for debugging?
This seems to be a valid removal, given that the tempstore is converted as defined in the routing.yml file.
Comment #12
dawehner11: attributes-2238981-11.patch queued for re-testing.
Comment #15
tim.plunkettSee #2281619: Convert most direct usages within module code of routing related request attributes to use RouteMatchInterface instead, it also changes ViewFormBase.
Comment #16
tim.plunkettRerolled.
Comment #18
dawehnerRerolled.
Comment #20
dawehnerLet's fix it.
Comment #21
Crell CreditAttribution: Crell commentedThis seems reasonable to me. The less controllers directly access $request, the better.
Comment #22
alexpottMinor nit alert:
Let's pass the mode into the parent buildForm so that {inheritdoc} is correct.
Comment #23
dawehnerSure, let's do that.
Comment #25
dawehnerThis could be it.
Comment #26
catchWhy not $view_mode_name = 'default' and save the ternary on the next line?
$form_mode_name/$view_mode_name mismatch.
Still have to get the entity out of the Request attributes here?
Comment #28
dawehnerups
Well, we have one controller for multiple entity types here.
Comment #29
dawehnerups
Well, we have one controller for multiple entity types here.
Comment #30
effulgentsia CreditAttribution: effulgentsia commentedShould we make testAdd() and testEdit() match? Perhaps rename both to $entity_type_id (no leading underscore)?
Comment #31
dawehnerSure, this sounds like a good idea.
Comment #32
effulgentsia CreditAttribution: effulgentsia commentedLooks great.
Search HEAD for
$request->attributes->get($
as well as$route_match->getParameter($
and you'll see a few other similar usages. Opened #2356689: Decide on the proper way for core controllers to access a variably named route parameter for that.Comment #33
alexpottCommitted 6b26533 and pushed to 8.0.x. Thanks!