Steps to reproduce:

1) Fresh 8.x-dev site
2) create a view that displays users
3) add the "content: authored" relationship
4) add the "Content: Type" filter

Notice: Trying to get property of non-object in Drupal\views\Plugin\views\HandlerBase->getEntityType() (line 771 of docroot/core/modules/views/lib/Drupal/views/Plugin/views/HandlerBase.php).

The function getEntityType() in core/modules/views/lib/Drupal/views/Plugin/views/HandlerBase.php has the problem:

  public function getEntityType() {
    // If the user has configured a relationship on the handler take that into
    // account.
    if (!empty($this->options['relationship']) && $this->options['relationship'] != 'none') {
      $views_data = $this->getViewsData()->get($this->view->relationship->table);
    }
    else {
      $views_data = $this->getViewsData()->get($this->view->storage->get('base_table'));
    }

    if (isset($views_data['table']['entity type'])) {
      return $views_data['table']['entity type'];
    }
    else {
      throw new \Exception(String::format('No entity type for field @field on view @view', array('@field' => $this->options['id'], '@view' => $this->view->storage->id())));
    }
  }

As best I can tell it appears that the code is using "$this->view->relationship->table" to determine the appropriate base table entity type for this filter. This however there presents 2 problems.

1) views can have many relationships, many of which can be against the same base for different reasons: I'd assume this means that using the variable "$this->view->relationship->table" is completely wrong
2) it doesn't take into account the $this->options['relationship'] configured value at all so it can't return the right relationship to build against.

An interesting thing about this (and probably why it has escaped testing) is that things work fine when the first entity matches the expected base. For example, creating a node view, adding a "content: author" relationship, and adding content:type will work fine. additionally, adding the "user: created" filter also works. In this situation its probably because there's only exactly one relationship that can work for this view.

To break this example in the same way, you can either add the "content: author" relationship again, or add the "Content: Authored" relationship (the one that relates users to content - not content to users). and the error will show up when you simply view the configuration of one of the two filters.

This was literally the 1st thing I've ever done in D8, and while I'm pretty good at navigating the code structure of 7.x-3.x views, the D8 code is a bit beyond me right now. I'm pretty sure that this is a one line fix that involves calling some sort of "->getRelationship($this->options['relationship']) " method and getting the base_table attribute, but after tons of digging I can't find it, or any way to get the properties of the selected relationship, so I'm leaving it up to you guys.

And lastly, this is also a problem with arguments, but seems to have been resolved with fields through UI fixes in a couple of other issues (noted in the related issues field).

Hopefully this is enough to go on. Thanks for the hard work!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

FileSize
1.62 KB

Oh gosh, this one is really crazy.

The general problem is that Bundle::init() assumes that $this->view->relationship is set. I am sorry but I needed to work
so far on the patch to be even possible to help you to fix this issue. Sorry.

Steps which has to be done:

  • Add some inline documentation why we call the initHandlers method.
  • Write a testcase which does exactly what you described in the issue summary to reproduce the issue.
dawehner’s picture

Thank you for finding this bug!!

netw3rker’s picture

FileSize
8.66 KB

Here's a first stab at this patch. I'm not positive that the exception being thrown when the test is run is being properly captured. Some review will be required here. This is also my first test to write, hopefully its correct :)

dawehner’s picture

Status: Active » Needs review

Let's first trigger the testbot.

Status: Needs review » Needs work

The last submitted patch, 3: vdc-2273731-3.patch, failed testing.

netw3rker’s picture

FileSize
8.83 KB

trying again..

netw3rker’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: vdc-2273731-3.patch, failed testing.

netw3rker’s picture

Status: Needs work » Needs review
FileSize
8.88 KB

hmm i guess the testbot wants some modules defined..

Status: Needs review » Needs work

The last submitted patch, 9: vdc-2273731.patch, failed testing.

marthinal’s picture

FileSize
9.11 KB

Rerolled + new view attached. Looks like the other view is broken. Imported it to my fresh d8 and looks wrong in any way. The test may fail and needs work. Not too much time today to continue on it.

marthinal’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: 2273731-11.patch, failed testing.

marthinal’s picture

Status: Needs work » Needs review
FileSize
7.7 KB
9.35 KB

The id was not correct so a simple change to the view id and looks that the patch fixes the problem and we can reproduce the exception. So let's try again!

The last submitted patch, 14: 2273731-14-only-test.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 14: 2273731-14.patch, failed testing.

marthinal’s picture

Status: Needs work » Needs review
FileSize
708 bytes
9.11 KB

Removed underscore from test. Anyway needs work again.

Status: Needs review » Needs work

The last submitted patch, 17: 2273731-17.patch, failed testing.

marthinal’s picture

Status: Needs work » Needs review
FileSize
9.93 KB

Let's try patch + test. Missing node_field_data table.

netw3rker’s picture

Rerolled + new view attached. Looks like the other view is broken. Imported it to my fresh d8 and looks wrong in any way. The test may fail and needs work. Not too much time today to continue on it.

I haven't reviewed the new view, but I want to point out that the view I originally created should look broken if views isn't properly handling relationships right. I'm guessing you've found the same problem though, so your view is probably as good as mine.

dawehner’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Let's be honest, this does not test anything yet.

netw3rker’s picture

@dawehner, I think I've been pretty clear and honest about that :)

I'm not sure how to write the test for "ensuring that a view configured with a specific set of relationships doesn't simply break views with a fatal WSOD" other than attempting to load a view that has the set of relationships in question & seeing if it succeeds or not. I also have never written tests before & could use a bit of help since this particular test might be a lot above my test writing skill level. Do you have suggestions or an example that is similar to this that you could reference?

mgifford’s picture

Status: Needs work » Needs review
FileSize
3.95 KB

Just a re-roll.

Status: Needs review » Needs work

The last submitted patch, 23: 2273731-23.patch, failed testing.

marthinal’s picture

Assigned: Unassigned » marthinal
marthinal’s picture

1) Bug:

I was testing manually and found that the entity used when filtering by a ct (in the case that we're using content author as relationship) is wrong.

Attached the d7 form and the current d8 behavior.

The setRelationship() method probably is not working as expected.

Also missing the "Select All" checkbox.

2) Test:
about #1

Write a testcase which does exactly what you described in the issue summary to reproduce the issue.

Not sure if we want a views_ui test or directly add a view and test the result...

marthinal’s picture

FileSize
494.53 KB
480.58 KB

A quick update.

The first time we are adding the filter handler (ConfigHandler::buildForm with the $id as type and $type as filter), the relationship is missing (image 1).So... we add the relationship later (image 2). We call HandlerBase::getEntityType() before the relationship is added.

marthinal’s picture

Assigned: marthinal » Unassigned
pcambra’s picture

jhedstrom’s picture

Issue tags: +Needs reroll

I can't tell if this is a duplicate of #2322457: Error in Views Filter Nodes based on Current user that have term reference same in nodes. or not. If not, the patch needs to be rerolled since that issue landed.

cafuego’s picture

Was going to do a reroll, but I can not reproduce the problem with the current 8.0.x HEAD when following the steps as outlined. However, I get a different notice:

Notice: Undefined property: Drupal\views\Plugin\views\relationship\Standard::$alias in Drupal\views\Plugin\views\query\QueryPluginBase->getEntityTableInfo() (line 292 of core/modules/views/src/Plugin/views/query/QueryPluginBase.php).

If I reroll the patch and apply it, the Content type filter actually only lists 'User' as available filter value.

babruix’s picture

Status: Needs work » Needs review
FileSize
10.15 KB

Status: Needs review » Needs work

The last submitted patch, 32: 2273731-32.patch, failed testing.

kerby70’s picture

I tested the process and I am unable to reproduce this issue in the current 8.0.x-dev version. After step 4 I am able to save the view and see users appear within the preview without any error or notice visually or in the recent reports. Can anyone else see if this is in fact causing any notice or error on the latest version of 8.0.x-dev? Otherwise I think this can be closed.

mrjmd’s picture

Status: Needs work » Closed (cannot reproduce)

I've tested this process as well using latest dev and was able to get through all four steps to reproduce without any errors being thrown. The filter and relationship both seem to work properly, as does the output of the user view.

Marking unable to reproduce.