This patch will change using hook_entity_delete() to using hook_field_attach_delete().
Why is this needed?
Because of this bug:
EntityMalformedException: Missing bundle property on entity of type relation. in entity_extract_ids()
The way to reproduce it:
- Modules to have installed on a clean Drupal (including their dependencies):
- Field collection
- Relation add
- Create a relation type.
- Create a content type with relation_add and field_collection fields.
- Add any field to a newly created field collection (a text field, for instance).
- Create a node and fill all the fields (relation_add and field_collection ones are important) with any values.
- Try to delete the node.
How does that fix it?
Well, a little of prehistory. Field collections module deletes its items on entity delete - obvious behaviour. After items are deleted, field collection module invokes entity_save in order to save the entity with unsetting references to those items. That's weird to me, but I can't judge, maybe it has to be so, if it's done.
The problem is that, that Relation module deletes relation entities before this mess with Field collections happens. So, when a relation is deleted, but there is still a reference to it in the field, Field collections module calls entity_save - then we get the mentioned error, while trying to update the relation which has been deleted.
Field collections delete themselves on hook_field_attach_delete(), so we have to do the same after them. That's how it's solved.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | relation-hook-field-attach-delete-2581913-12.patch | 608 bytes | daveferrara1 |
| #2 | relation-hook-field-attach-delete-delete-2581913-0.patch | 608 bytes | mvlabat |
Comments
Comment #2
mvlabat commentedIncluded the patch.
Comment #3
mvlabat commentedComment #4
mvlabat commentedComment #5
drupalway commentedLooks good to me.
Comment #6
mikran commentedGreat issue description!
Does this work with non-fieldable entities too, and do we have tests for deleting non-fieldable entity?
Comment #7
valentine94Looks great! +1 to RTBC
Comment #8
mvlabat commentedmikran, thank you for your reply!
Could you, please, clarify, what do you mean with non-fieldable entities?
Comment #9
mikran commentedI did a quick search to field_collection issues and I found this #1781190: Field Collection saved on presave which I think would solve this problem too.
Comment #10
mvlabat commentedNo, that, unfortunately, doesn't relate to our problem, because this is another level where those hooks are invoked.
This is how functions were called before:
hook_entity_delete() invokes:
hook_field_attach_delete() invokes:
If they swap some functionality between those hooks it won't affect our story, so that's how it is now:
hook_field_attach_delete() invokes:
I'll remind what is the point of my fix: we have to delete relations after entity_save is called (which is caused by mysterious Field collections nature), in order to prevent saving an entity pointing to already deleted relation. All of these is happening on deleting a host entity.
Comment #11
mikran commentedOh and with non-fieldable entity I mean entities that have
'fieldable' => FALSEset in entity info. taxonomy_vocabulary is this kind of example from core.Comment #12
daveferrara1 commentedPatch for 7.x-1.2
Comment #14
jeff h commentedIt's frustrating that this patch isn't rolled in. In my opinion, it's false logic holding it back only because it doesn't have tests for non-fieldable entities. Holding it back means the module is known to be broken for (surely) its most common use case (fieldable entities), but won't be fixed because it might break an edge case (unfieldable entities).
Comment #15
jeff h commented