Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Field collection module currently sets a collection's "hostEntity" only on new pieces of content. This allows other modules that need to work with the collection to get the host entity. However, if a host entity has received changes, the hostEntity property is not updated with these changes. Because the host entity is saved all at once within a single database transaction (see node_save()), any modules needing to pull information from the hostEntity will be getting stale data during hook_field_insert/update and hook_entity_insert/update.
In our particular situation, we're working with another field module (Ooyala) that needs to get properties from a host entity and send those properties to a remote service for individual items (videos) within a field collection. On new nodes, information from the hostEntity is present. On updating nodes however, the hostEntity is stale and contains the old values, not the ones being saved in the current form.
Proposed resolution
Restore hook_field_presave() and set/update the host entity on *all* presave calls, not just when an entity has the is_new property set.
User interface changes
none
API changes
Host entity data will always be the current entity being saved, rather than the current entity from the database.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | update_host_entity_in-2220751-7.patch | 1.6 KB | redndahead |
| #3 | field_collection_presave_update-2220751.patch | 3.1 KB | quicksketch |
| #1 | field_collection_host_entity_presave-2220751.patch | 3.84 KB | quicksketch |
Comments
Comment #1
quicksketchHere's a patch the makes this change, moving the call to $entity->setHostEntity() to field_collection_field_presave(). All tests are still passing, but I haven't checked how comprehensive those tests are for checking data integrity.
This change brings up the question of why the is_new check was ever in place to begin with. Does it serve some purpose other than attempting to skip processing? The setting/updating of a few variables should be cheap, so perhaps it was in place just to prevent accidental reassigning of IDs? I'm not sure what purpose it served, but removing the check and updating the host entity all the time does not seem to have negative side effects.
Comment #2
jmuzz CreditAttribution: jmuzz commentedThank you for providing an example of why this is needed. There's a similar change in a part of the entity translation patch and there were concerns about whether that part should be included or not. The problem with simply removing the check is that field collection items are not supposed to be able to change to another host and it could cause unintentional inconsistancies in the data if it was allowed. The checks put in place in the entity translation patch version should be sufficient to prevent this problem. Consider adding these checks to your patch. If it gets committed it won't be a problem to take that part out of the entity translation patch.
#1344672: Field Collection: Field translation (entity_translation) support.
Comment #3
quicksketchI think this patch matches the approach used in #1344672: Field Collection: Field translation (entity_translation) support.. How's this look?
Comment #4
quicksketchComment #6
kristofferwiklund CreditAttribution: kristofferwiklund commentedComment #7
redndahead CreditAttribution: redndahead commentedI am not sure if this is even needed anymore, but I have rerolled it just in case. Alot of things in the previous patch have been committed. So only a little left and it feels like I am reverting something that should still be in there. But people who were having problems feel free and see if this fixes the issue you were running into.