Hi,
I am running into problems rolling back parent entities from saving when an error occurs on child field collection. The field collection table in "field_data_[field_name]" is ending up with value in the "value" column that doesn't exist in the "field_collection_item" table.

I am using the Services module to save entities(nodes in my case) with field collections. I run into situations where the field_collections don't save correctly b/c of bad data. This works. The problem is I don't want the parent entity to save if any of the field_collections on it fail to save. Also the "field_data" table ends up with an non-existent "item_id" in the "value" column.

Here is the problem as well as I can tell:
In "field_collection_field_presave" around line #582

$item['entity']->save(TRUE);
$item = array('value' => $item['entity']->item_id);

If there is any problem in saving any of the field the FieldCollectionItemEntity->save function will rollback the transaction and return FALSE. The "field_collection_field_presave" ignores this return value and always assigns item_id to the value of the field. "item_id" in the case of multiple fields will already be set and doesn't get reset when the transaction is rolled back.

I will attach a patch that checks for the return value of the save function.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tedbow’s picture

patched attached. Basically adds:

      if ($item['entity']->save(TRUE)) {
        $item = array('value' => $item['entity']->item_id);
      }
      else
      {
        $item = array('value' => FALSE);
      }

This at least doesn't store an non-existent item_id in the table.

But I have couple questions:

Should there be flaga set on the parent entity(its passed to the function) to flag that field collections didn't all save correctly? Something like $entity->field_collection_errors = TRUE;

This would allow you to throw an error in the entity_presave hook that would rollback the parent save transaction. As it is now the parent entity actually ends up saving even if there is an error saving one of it's field_collections.

In my case and I think many others you would not want the entity to save unless all of its field_collections also saved.

Thoughts?

tedbow’s picture

Title: Field Collections save with no existent item_id in value when error occurs. » Field Collections save with non-existent item_id in field value when error occurs.

corrected title

jmuzz’s picture

Issue summary: View changes
Status: Active » Postponed (maintainer needs more info)

Could be an improvement, though the way field collection items are saved has been changed. There are now a few calls to ->save() in different places.

Does this problem still occur? I think $item['entity']->item_id should be null if it didn't save.

jmuzz’s picture

Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)