CTools knows how to extract relationship info from the field schema, however the 'foreign keys' in entityreference fields is always an array() so we don't get it "for free".

Attached patch adds relationship plugins.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amitaibu’s picture

oops, no white spaces.

amitaibu’s picture

Check if field doesn't exist, bail out to prevent WSOD

Damien Tournoud’s picture

Why not modifying entityreference_field_schema(), then?

amitaibu’s picture

> Why not modifying entityreference_field_schema(), then?

When I tried to do it as first, and calling entity_get_info() from within entityreference_field_schema() I got some notices. I'll re-check, and see if I can overcome the problem.

amitaibu’s picture

FileSize
872 bytes

Here's the patch I tried to do, but It gives some nasty errors on cache clear and node add.

Crell’s picture

EclipseGc’s picture

See if you can do something similar with a schema_alter instead and see if the nastiness goes away. Then no plugin would be required :-D

Crell’s picture

I don't see how the abomination that is hook_schema_alter would be any better here.

amitaibu’s picture

See comment #5 :) I couldn't do a change in schema alter -- chicken and egg errors
But yes, if someone can fix 5 it would be nicer

Damien Tournoud’s picture

Can anyone clarify the relationship between this and #1258502: Add support of Page manager contexts?

amitaibu’s picture

It's a duplication, as I somehow missed the initial issue :/ I think this patch is a bit more correct for the following reasons:

  • in n CTools' code the reverse relationship was commented out, so in this patch it wasn't added.
  • entity_from_entityreference_field_context() uses entity_metadata_wrapper()
  • making it a little more readable, IMO

With that said, I'd be happy if someone can have a look at #5, and maybe think about a what to make it work -- that will solve it in a much less verbose way.

EclipseGc’s picture

Damien,

CTools builds relationships from one entity to another automatically as long as the foreign keys of the field_schema are correct. Solve that (which is what #5 is attempting to do) and it should all work fine. Adding a whole new relationship plugin for this seems like a total fail to me. I work really hard to make sure that as long as devs kept their schema all proper, this stuff would just take care of itself.

Eclipse

amitaibu’s picture

> Solve that (which is what #5 is attempting to do) and it should all work fine

I haven't dug very hard, but I believe the the call to entity_get_info() from withing the hook_field_schema() is the problem. I wonder what other options we have?

Crell’s picture

What errors are you seeing? (I've not had a chance to try the code yet, but I have a need for this issue.)

fabsor’s picture

The patch in #2 works as expected with no problems and it will probably be way less error-prone, since a lot of modules do magical things in hook_entity_info, and we probably don't want to fix the issues that might come from this when people start combining this with a lot of different modules. I think using a special plugin is fine in this case.

EclipseGc’s picture

I still would MUCH rather see this run through the core ctools plugin built to do exactly this work. The schema just needs to be kept up to date.

amitaibu’s picture

> I still would MUCH rather see this run through the core ctools plugin built to do exactly this work

I agree, but I didn't find a way to solve the problems with the patch from #5. @ EclipseGc, maybe you can have a look at that patch and figure the errors...

Itangalo’s picture

Don't know enough about hook_schema_alter (or the alternatives) to comment on how this issue should be solved, but wanted to say that I find this issue pretty important. In fact, I think it is the last piece missing before Entity reference Really Rocks.

Thanks for the awesome work.

helior’s picture

I've spent some time yesterday looking at this and it seems there is another part to this problem:

Assuming Entity Reference does keep its foreign keys up-to-date, Ctools will not be able to create the relationship because it will not honor "target_id" -- it will instead be looking for something like "nid" (depending on the entity it references.)

#1394124: Accurately reference foreign keys in entity_from_field relationship fixes that and will allow us to move forward with what #5 is trying to do.

helior’s picture

Using entity_info() in this scope throws Drupal into a bit of a loop, so instead, the patch retrieves the existing entity_info from cache.
Also, it probably shouldn't provide foreign key info until we know which entity is being referenced via the field settings. Otherwise it will mistakenly assign foreign keys pertaining to the default target type (node), which in some cases will be incorrect.

Please note that this patch depends on #1394124: Accurately reference foreign keys in entity_from_field relationship

fago’s picture

Instead of directly providing the ctools relationship, we could also leverage entity property relationship information to provide those as ctools relationships via the entity api. So we'd avoid any schema trickery + have a storage back-end agnostic relationship support.

helior’s picture

Fago, maybe you can provide some tips on how to properly incorporate that into entityreference's hook_field_schema()? I attempted to use entity property and metadata into this, but I continued to get the same errors as before since everything still traced back to using entity_get_info(). Thoughts?

amitaibu’s picture

Status: Needs review » Fixed
Damien Tournoud’s picture

Sorry, I failed to come back because of the outage yesterday.

fabsor’s picture

There seems to be a bug in CTools that prevents the relationships from showing up. Patch here: #1418740: Entity from field relationship plugin should check the target column rather than the source column

jbrown’s picture

Status: Fixed » Needs work
FileSize
11.74 KB

I'm getting an infinite loop from this - not sure which module is to blame.

bennetteson’s picture

Status: Needs work » Fixed

Hello jbrown,

Please do not re-open a merged fixed issue.

It Seems you don't have a infinite loop, please increase your xdebug maximum nesting level to 200. Drupal has a hight level of nesting. If you still have your issue, please open a new issue.

Regards.

rogical’s picture

Seems not committed to branch 7.x-1.x yet.

Damien Tournoud’s picture

I pushed a fix to prevent recursion if a module calls drupal_get_schema() inside hook_entity_info() (as does Heartbeat).

We really need a proper fix for #1416558: hook_entity_info(), hook_schema(), and the field system are strongly bound to each other at one point.

Damien Tournoud’s picture

[Edited: wrong issue]

Status: Fixed » Closed (fixed)

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

Itangalo’s picture

I am having a small party, celebrating this issue being fixed. Great work.

B-Prod’s picture

Status: Closed (fixed) » Needs review
FileSize
916 bytes

Hi!

There is an issue when using a entity other than node, because the 'foreign key' value is stored by field_create_field() function, which uses the default entity type "node". This is done before the field settings form is displayed and it is never updated.

So for example the user relationship in cTools is displayed as "node" and does not work.

There is several approach to fix this. The most classic one would be to implement hook_field_read_field(), but that represents a extra function call for all existing fields, when field info cache is rebuilt. I found that not really efficient, so I choose another method.

I actually used the _entityreference_field_settings_validate() function to add the correct "foreign keys" inside the $form_state['values']['field'] array, which is used to update the field storage by field_ui_field_settings_form_submit() function.

Caveat: such approach means that contributed modules that intend to programmatically update an entity reference field by directly calling field_update_field() need to handle the "foreign key" changes by themselves.

Damien Tournoud’s picture

Status: Needs review » Closed (fixed)
B-Prod’s picture

Thanks for pointing this issue to me, you're right, Core should handle that.

lwalley’s picture

I ran into this foreign keys issue trying to use an entity reference field to a taxonomy_vocabulary with Panels and Views see #1831872: Entity reference relationship to taxonomy vocabulary and vid for views argument does not appear to be recognised correctly. I'm keeping an eye on #1416506: Field schema foreign keys support is broken for a fix, but in the mean time the patch in #33 appears to solve my issue so I just wanted to say thanks.

fago’s picture

I think #21 is a good idea, thus I've opened #1874006: Expose entity references as CTools relationships. Please provide your thoughts there!

emattias’s picture

Status: Closed (fixed) » Active

The current solution doesn't provide two-way relationships.

The relationships are only available on the node that has the entityreference fields. You can't get the referencing node from the node that is referenced.

amitaibu’s picture

Status: Active » Closed (fixed)

@emattias, please open a separate issue.

emattias’s picture

dagomar’s picture

#33 Seems to work for me. Couldn't this be committed as a temporary solution?

mansspams’s picture

Note on #33. If you applied patch from #33 you have to go to manage fields page click on "edit" and "save" to all entityreference fields that does not point to "node" (all term, vocabulary, user and other reference fields).

Jason Dean’s picture

#33 just saved my day... noting the useful tip in #42. Thanks!

timbrandin’s picture

Here's a new solution for the 7.x-1.x-dev branch, #33 did not apply on the latest code.

timbrandin’s picture

Status: Closed (fixed) » Needs review
bradjones1’s picture

Status: Needs review » Closed (won't fix)

Per #34 this is a core bug; while the patches here are helpful as a stopgap, this isn't really an Entity Reference issue at its, well, core.

timbrandin’s picture

Lets hope a the core bug is resolved quickly then.

Dustin Bons’s picture

Status: Closed (won't fix) » Needs review

This should be reopened as #1416506: Field schema foreign keys support is broken is now fixed.

Dustin Bons’s picture

Status: Needs review » Closed (fixed)

Never mind. Re-saving the entity reference field now already fixed this without a patch.