Problem/Motivation
When previewing nodes that use a Views Reference Field with a view that includes a node contextual filter configured to use the default argument using the content id from the URL, the view does not render any results as expected and seen when rendering the node.
The node views argument default expects the node parameter, as seen here.
\Drupal\node\Plugin\views\argument_default\Node::getArgument()
public function getArgument() {
if (($node = $this->routeMatch->getParameter('node')) && $node instanceof NodeInterface) {
return $node->id();
}
}
However, the entity.node.preview route provides the node_preview parameter, which is upcasted to the node entity via the Drupal\node\ParamConverter\NodePreviewConverter()
See line 31 of node.routing.yml
entity.node.preview:
path: '/node/preview/{node_preview}/{view_mode_id}'
defaults:
_controller: '\Drupal\node\Controller\NodePreviewController::view'
_title_callback: '\Drupal\node\Controller\NodePreviewController::title'
requirements:
_node_preview_access: '{node_preview}'
options:
parameters:
node_preview:
type: 'node_preview'
Proposed resolution
Can we simply convert the parameter name in entity.node.preview from node_preview to node?
Remaining tasks
Patch, tests, ?
User interface changes
None
API changes
Unsure
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | drupal-node_preview_parameter-2942054-12.patch | 2.08 KB | fabianfiorotto |
| #5 | drupal-node_preview_parameter-2942054-5.patch | 896 bytes | jasonawant |
| #2 | drupal-node_preview_parameter-2942054-2.patch | 5.02 KB | jasonawant |
Comments
Comment #2
jasonawantHere.s a patch that changes the parameter name and many places where its used for previewing to work.
Comment #4
timmillwoodThis is one of those bugs that if we fix it correctly it's going to introduce a BC break. Would it not be possible to update
\Drupal\node\Plugin\views\argument_default\Node::getArgumentto expectnodeandnode_preview?Comment #5
jasonawantGotcha, yeah, here's a patch for that approach.
Comment #6
timmillwoodWorks for me!
Also, this was not picked up until now because we don't have test coverage, so I guess the next step is... add a test.
Comment #7
jasonawant@timmillwood,
Could you provide some direction on what this test would looks like? A Kernel or Functional test? Jason
Comment #9
maxstarkenburgAlso chiming in since I'm seeing this issue still in 9.1.x, and patch 5 is working to fix this for me all these years later, though I know next to nothing about automated tests in order to help get this into core.
Comment #10
maxstarkenburgFWIW, also adding a non-patch workaround suggested by my colleague instead of a core patch to address this on our site in the meanwhile (targeted in this case to work on just a specific view):
Comment #11
vikashsoni commented@jasonawant patch working for me thanks for the patch
Comment #12
fabianfiorotto commentedI modified #5 to make it work with node_revision. I also changed the Tid argument because it wasn't working when the option "Load default filter from node page" is selected.
Comment #16
smustgrave commentedWas previous tagged for tests in #6 which still needs to happen
Thanks.
Comment #18
riyas_nr commentedThis patch addresses an additional scenario where adding a block with a View reference field containing a contextual filter in Layout Builder results in the preview displaying 'Placeholder for the name Views block.'
Comment #19
acbramley commentedI think this should definitely be explored. I have no background on why it's the way it is but it seems like changing the parameter name could be a better way forward and reduce some other code duplication we have (e.g in template_preprocess_page). This would however be quite disruptive for BC purposes.