Let's assume you have a node view. Then add a relationship to user, and add the user: uid argument.

Once you have done this you get the following nice error: 

( ! ) Fatal error: Call to a member function getArgumentText() on a non-object in /var/www/d8/core/modules/views/lib/Drupal/views/Plugin/views/argument/ArgumentPluginBase.php on line 165
Call Stack

There are several reasons for that:

  • To get always a required relationships we try to guess the relationship, and then force a change of the view in the tempstore
  • This currently acts on the actual objects, so any changes in there reflects back to the original value, let's clone that
  • To be honest in this case we should actually not try to guess the relationship because we know the right one already.
CommentFileSizeAuthor
#13 1877114-13.patch5.39 KBdamiankloip
#11 1877114-10.patch5.56 KBdamiankloip
#9 interdiff.txt1.08 KBdamiankloip
#8 1877114-8.patch5.56 KBdamiankloip
#7 1877114-7.patch5.66 KBdamiankloip
#6 drupal-1877114-6.patch2.04 KBdawehner
#2 drupal-1877114-2.patch1.93 KBdawehner
#1 drupal-1877114-1.patch999 bytesdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new drupal-1877114-1.patch999 bytes

This at least fixes the failure.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new drupal-1877114-2.patch1.93 KB

So views doesn't really know relationships when adding an item, so it set up the right relationship on first time visiting the config item form. Even this seems to be fundamentally wrong, let's fix the general issue first, as it is quite a major bug to not be able to use any kind of handlers with relationships.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/modules/views/views_ui/views_ui.moduleundefined
@@ -372,11 +372,21 @@ function views_ui_cache_set(ViewUI $view) {
   // Unset handlers; we don't want to write these into the cache

Might as well add a . here

+++ b/core/modules/views/views_ui/views_ui.moduleundefined
@@ -372,11 +372,21 @@ function views_ui_cache_set(ViewUI $view) {
+  // Readd the different variables to be able to call views_ui_cache_set
+  // without fearing data lost.

Re-add, views_ui_cache_set(), data loss

+++ b/core/modules/views/views_ui/views_ui.moduleundefined
@@ -372,11 +372,21 @@ function views_ui_cache_set(ViewUI $view) {
+  $executable->displayHandlers = $display_handlers;

I'm not sure that this will work with the PluginBag patch.

dawehner’s picture

dawehner
Primary language German
commented

Will fix the other problems next year :)

I'm not sure that this will work with the PluginBag patch.

Yeah no idea, but I tell you this code fixes the problem. As d7 does something similar like our current code i don't fully understand why we need this.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: +Needs tests

Fair enough! :)

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new drupal-1877114-6.patch2.04 KB

Here is the new version.

damiankloip’s picture

damiankloip commented
Status: Active » Needs review
StatusFileSize
new 1877114-7.patch5.66 KB

Ok, here is the start of some tests. I have created an ArgumentUI test class. This exposes another issue with the current fix, this is why I'm uploading this now.

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1877114-8.patch5.56 KB

I think it's the default_display part we don't want, I think we only use this on display plugins nowadays? I might be wrong...

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff.txt1.08 KB

These were removed.

dawehner’s picture

dawehner
Primary language German
commented

Thank you for working on that!

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/ArgumentUITest.phpundefined
@@ -0,0 +1,51 @@
+    $this->drupalPost("admin/structure/views/nojs/add-item/$view_name/default/argument", $edit, 'Add and configure contextual filters');
...
+    $this->drupalPost(NULL, array(), 'Apply');

Aren't we using t() around the values of submit elements?

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1877114-10.patch5.56 KB

Good point, this is done everywhere else, although I'm not sure why. I thought this would be similar to assertion messages etc.. as tests are pretty much not translated unless testing translation. oh well :)

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Let's get this in now, as it's quite a big bug for users.

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1877114-13.patch5.39 KB

Just removed a couple of rogue lines, still good to go.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

I reviewed this already, and the patch is definitely RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1877114-13.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review

#13: 1877114-13.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1877114-13.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review

#13: 1877114-13.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1877114-13.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review

#13: 1877114-13.patch queued for re-testing.

dawehner’s picture

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

So we fixed that already in a way better way: #1906794: Using a relationship in the UI fails completly