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!
| Comment | File | Size | Author |
|---|---|---|---|
| #32 | 2273731-32.patch | 10.15 KB | babruix |
| #27 | 2-2273731.png | 480.58 KB | marthinal |
| #27 | 1-2273731.png | 494.53 KB | marthinal |
| #26 | shouldbe-d8-2273731.png | 319.63 KB | marthinal |
| #26 | current-d8-2273731.png | 305.3 KB | marthinal |
Comments
Comment #1
dawehnerOh 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:
Comment #2
dawehnerThank you for finding this bug!!
Comment #3
netw3rker commentedHere'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 :)
Comment #4
dawehnerLet's first trigger the testbot.
Comment #6
netw3rker commentedtrying again..
Comment #7
netw3rker commentedComment #9
netw3rker commentedhmm i guess the testbot wants some modules defined..
Comment #11
marthinal commentedRerolled + 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.
Comment #12
marthinal commentedComment #14
marthinal commentedThe 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!
Comment #17
marthinal commentedRemoved underscore from test. Anyway needs work again.
Comment #19
marthinal commentedLet's try patch + test. Missing node_field_data table.
Comment #20
netw3rker commentedI 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.
Comment #21
dawehnerLet's be honest, this does not test anything yet.
Comment #22
netw3rker commented@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?
Comment #23
mgiffordJust a re-roll.
Comment #25
marthinal commentedComment #26
marthinal commented1) 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
Not sure if we want a views_ui test or directly add a view and test the result...
Comment #27
marthinal commentedA 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.
Comment #28
marthinal commentedComment #29
pcambraComment #30
jhedstromI 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.
Comment #31
cafuego commentedWas 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.
Comment #32
babruix commentedComment #34
kerby70 commentedI 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.
Comment #35
mrjmd commentedI'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.