Closed (fixed)
Project:
Relation
Version:
7.x-1.x-dev
Component:
Views
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
10 Sep 2012 at 18:09 UTC
Updated:
7 Mar 2014 at 17:22 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
webankit commentedComment #2
steveoliver commented@webankit:
'entity_type_right' => $entity_type_leftjust seems all wrong, don't ya think?Comment #3
webankit commentedThe logic which I predicted and made this change was:
'entity_type_left' for the join should be relation (as we are using it as the base table)
and
'entity_type_right' for the join should be entity we are trying to join (i.e $entity_type_left).
I tried and it's working:)
Comment #4
mikran commentedA test to show this error (and the fix) would be really nice. I can believe you that it's working but it's likely to get broken again
Comment #5
hlykos commentedPatch seems to work.
Example query wihout the patch:
Example query with the patch:
Comment #6
alan evans commentedFWIW I also have a prototype site which was broken and is fixed by this patch. (and confirm similar query changes to hlykos)
Agreed that a test would be good, and potentially might be worth some comment and maybe some variable renaming (even if just copying the _left variable into something named better for this case).
Comment #7
gaëlgHere's a renaming patch (with no logic changes, so that previous patch is not included). It should be easier to debug with right names. Left has been replaced with source, and right with target, so that we don't get confused with left and right joins SQL considerations.
And it looks like the previous patch just partially solve the bug, the SQL of my view still have some errors. I'm working on this.
Comment #8
gaëlgMy bad, the previous patch works. I had another not related problem.
Comment #9
steveoliver commentedGaëlG: Think you could roll a git patch for this?
Comment #10
gaëlgHere's a merge of #1 and #7.
Comment #11
mikran commentedTests.
Maybe, exported view that is as small as possible in order to reproduce this bug and then we create a test from that. We're generally very light on test coverage for views support and that would be nice to fix before the stable release.
Comment #12
jhoffmcd commented#10 confirmed for me, I was dealing with this for I while before I happened upon this thread. Thanks all.
Comment #13
bsarchive commented#10 isn't working for me. Plus I have a few additional configurations that don't function, as follows:
I've got a directional relation: user->node
In a view of users (source):
- views relationship from user to node (source to target) works
- views relationship from user to relation (source to relation) works
In a view of nodes (target)
- views relationship from node to user (target to source) works
- views relationship from node to relation (target to relation) doesn't work
In a view of relations
- views relationship from relation to node (relation to target) works
- views relationship from relation to user (relation to source) doesn't work
?? This is breaking me this issue - hope someone can help!
Plus, this patch breaks all views relationships to relations. Is it possible to create a patch that upgrades gracefully?
Comment #14
bsarchive commentedI think I've worked this out now:
the patch in #1 to /relation/views/relation.views.inc changed line 334
from
'entity_type_left' => $entity_type_left,to
'entity_type_right' => $entity_type_left,which fixed views relationship: relation->source
in the same file, changing line 319
from
'entity_type_right' => $entity_type_right,to
'entity_type_left' => $entity_type_right,fixes views relationship: target->relation
I don't know how to write patches correctly so if someone could add this to patch #1, that would be great!
I've tested all views relationships (src->tar, tar->src, src->rel, rel->src, tar->rel, rel->tar) with two cases (1: same entity types in src and tar; 2; different entity types) and all seem work fine.
Can we get this committed? Seems to me now that the main thing distinguishing Relation module from Entityreference (now in D8 core) is that relations are fieldable. To access the fields in views you need the relationships to the relations so this issue is critical to the usefulness of the Relation module. This is why I've upgraded this issue to 'major'.
Comment #15
bsarchive commentedPatches #7 and #10 (which change the names of the variables and array keys) will break any views which have a views relationship from relation->source.
I'd suggest keeping the names the same. The necessary change in the code is just two words.
Comment #16
marcusx commentedCan confirm that we need to add the changes in #14 to get this working for target -> relation.
Added this change to a new patch version.
What about the name change? Should we go with left / right or source / target. Any opinions about that? Should we split this into two separate issues? Would make sense I think.
Comment #17
Paul B commentedHere's a patch that just makes the two changes in #14.
I think the renaming should be a separate issue.
Comment #18
mikran commentedViews tests added for "relation → endpoint" and "endpoint → relation" views. And yes, we shouldn't break existing views so patch #17 is better.
Comment #23
mikran commentedOh, wrong views field name.
Comment #25
mikran commentedThis one is now committed as well.