Problem/Motivation

Steps to reproduce:
1. Install standard and create a node with title "node1"
2. Add a view that lists content
3. Add a contextual filter "Content: Node ID" (select the first one)
4. In contextual filter settings, check "Override title" and type "%1" as the title
5. Add a page display using path test/%
6. Go to /test/1

Expected: The title should say "node1".
Actual: The title says "1"

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

olli created an issue. See original summary.

olli’s picture

Status: Active » Needs review
StatusFileSize
new616 bytes

This seems to work.

dawehner’s picture

+1 for the fix, of course!

geertvd’s picture

Added test coverage (not just for the issue described but for the whole nid argument handler)

geertvd’s picture

+++ b/core/modules/views/src/Plugin/views/PluginBase.php
@@ -396,7 +396,7 @@ function ($children, $elements) {
-      return (string) $this->getRenderer()->render($build);
+      return (string) $this->getRenderer()->renderPlain($build);

I had to borrow this line from #2492839: Views replacement token bc layer allows for Twig template injection via arguments to make my test pass.

Status: Needs review » Needs work

The last submitted patch, 4: node_id_contextual-2564495-4-complete.patch, failed testing.

The last submitted patch, 4: node_id_contextual-2564495-4-test.patch, failed testing.

The last submitted patch, 4: node_id_contextual-2564495-4-test.patch, failed testing.

The last submitted patch, 4: node_id_contextual-2564495-4-complete.patch, failed testing.

geertvd’s picture

Status: Needs work » Needs review
StatusFileSize
new3.39 KB
new3.99 KB
dawehner’s picture

+++ b/core/modules/node/src/Tests/Views/NidArgumentTest.php
@@ -0,0 +1,114 @@
+    $view = View::create([
+      'id' => $view_id,
+      'base_table' => 'node_field_data',
+      'display' => [
+        'default' => [
+          'display_plugin' => 'page',
+          'id' => 'default',
+          'display_options' => [
+            'path' => 'test/%',
+            'fields' => [
+              'nid' => [
+                'table' => 'node_field_data',
+                'field' => 'nid',
+                'id' => 'nid',
+                'plugin_id' => 'field',
+              ],
+            ],
+            'arguments' => [
+              'nid' => [
+                'table' => 'node_field_data',
+                'field' => 'nid',
+                'id' => 'nid',
+                'plugin_id' => 'numeric',
+                'title_enable' => TRUE,
+                'title' => '%1',
+              ],
+            ],
+          ],
+        ],
+      ],
+    ]);

IMHO we should use a file, if possible.

The last submitted patch, 10: node_id_contextual-2564495-10-test.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 10: node_id_contextual-2564495-10-complete.patch, failed testing.

The last submitted patch, 10: node_id_contextual-2564495-10-test.patch, failed testing.

The last submitted patch, 10: node_id_contextual-2564495-10-complete.patch, failed testing.

geertvd’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new9.4 KB
new7.8 KB
new8.4 KB

Yeah, I guess it does make more sense to use a real views export there.
Also fixed the test since we shouldn't be using the %1 syntax anymore but rather {{ arguments.nid }}

The last submitted patch, 16: 2564495-16-test.patch, failed testing.

The last submitted patch, 16: 2564495-16-test.patch, failed testing.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Yeah! As always I like if things are actually broken and we can expand our test coverage.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This is bug with a non-disruptive fix - permitted in beta. Committed 7f11ec7 and pushed to 8.0.x. Thanks!

  • alexpott committed 7f11ec7 on 8.0.x
    Issue #2564495 by geertvd, olli: Node ID contextual filter uses wrong...

Status: Fixed » Closed (fixed)

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

Status: Closed (fixed) » Needs work

The last submitted patch, 16: 2564495-16-complete.patch, failed testing.

geertvd’s picture

Status: Needs work » Closed (fixed)