There is a logic that saves correct fields data for current revision on update:

// Field API always saves as default revision, so if the revision saved
// not default we have to restore the field values of the default
// now by invoking field_attach_update() once again.
if ($this->revisionKey && !$entity->{$this->defaultRevisionKey} && !empty($this->entityInfo['fieldable'])) {
  field_attach_update($this->entityType, $entity->original);
}

But it should be executed before hook_entity_update(). Otherwise we have wrong revision data.

CommentFileSizeAuthor
#1 entity_field_revision-2379821.patch1.66 KBa.milkovsky
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

a.milkovsky’s picture

a.milkovsky
he/him
Location UTC +2
CreditAttribution: a.milkovsky commented
Assigned: a.milkovsky » Unassigned
Status: Active » Needs review
FileSize
entity_field_revision-2379821.patch1.66 KB

Here is the patch to execute field_attach_update() earlier.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
Status: Needs review » Reviewed & tested by the community

yeah, the problem with the existing code is that during hook_entity_update() the wrong data is saved. Thus, if a module load the entity again during this hook, it gets the wrong data. Moving that code to invoke() solves this problem as it fixes the written data before any other module can load it again.

ongdesign’s picture

ongdesign CreditAttribution: ongdesign commented
Status: Reviewed & tested by the community » Needs work

When I test this using Bean by creating a non-default revision, I get some broken behavior, as follows:

  1. Define a block type (bean) that uses an image field.
  2. Create an instance of that block type using an image (it displays correctly).
  3. Edit that block, remove the image, add a new image.
  4. Save as a new revision (revision B), but not the default (so revision A is still the active one).

Before this patch, the behavior was that the Revision B's image field was not populated (which was clearly broken behavior). Revision A was still (correctly) displayed when viewing the block.

After the patch, Revision B's image field is populated, as is Revision A's, but now Revision B, while still not set to "active", is the one displayed upon viewing the block. Additionally, if I set Revision B to active, then set Revision A to active, the image gets deleted from Revision B.

Hope this makes sense...