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:

  1. Modules to have installed on a clean Drupal (including their dependencies):
    • Field collection
    • Relation add
  2. Create a relation type.
  3. Create a content type with relation_add and field_collection fields.
  4. Add any field to a newly created field collection (a text field, for instance).
  5. Create a node and fill all the fields (relation_add and field_collection ones are important) with any values.
  6. 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.

Comments

mvlabat created an issue. See original summary.

mvlabat’s picture

Included the patch.

mvlabat’s picture

Issue summary: View changes
mvlabat’s picture

drupalway’s picture

Looks good to me.

mikran’s picture

Great issue description!

Does this work with non-fieldable entities too, and do we have tests for deleting non-fieldable entity?

valentine94’s picture

Looks great! +1 to RTBC

mvlabat’s picture

mikran, thank you for your reply!

Could you, please, clarify, what do you mean with non-fieldable entities?

mikran’s picture

I 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.

mvlabat’s picture

No, 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:

  • Here was Relation's magic

hook_field_attach_delete() invokes:

  • Here is Field collections' magic
    • Here they do entity_save so hooks hook_field_update() and hook_field_presave() are invoked.

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:

  • Here is Field collections' magic
    • Here they do entity_save so hooks hook_field_update() and hook_field_presave() are invoked.
  • Here is now Relation's magic

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.

mikran’s picture

Oh and with non-fieldable entity I mean entities that have 'fieldable' => FALSE set in entity info. taxonomy_vocabulary is this kind of example from core.

daveferrara1’s picture

Patch for 7.x-1.2

Status: Needs review » Needs work

The last submitted patch, 12: relation-hook-field-attach-delete-2581913-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jeff h’s picture

It'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).

jeff h’s picture

Version: 7.x-1.x-dev » 7.x-1.2
Status: Needs work » Reviewed & tested by the community