A frequent way of doing workflows is changing the entity field on a hook_entity_update or in a Rule, then saving.
So, take a commerce order as an example. Once the order is submitted at the end of checkout, we need it to go through these statuses:
checkout_complete -> fulfillment -> completed -> invoiced.
The order needs to go through each status so that all needed rules can run. We can't skip them via hook_entity_presave.
So when Commerce first completes checkout, we change the status to fulfillment in the "after an entity has been updated " rule.
The fulfillment rules run, then the last rule changes the status to completed, then the completed rules run, then the last rule changes the status to invoiced.
However, the Entity API has this in the controller:
if (!empty($entity->{$this->idKey}) && !isset($entity->original)) {
That !isset is a premature optimization and causes the original entity to be retained through next saves.
So when you respond to the order being set to invoiced, $order->original->status == 'checkout_complete'.
Here's some test code for nodes:
$node = entity_create('node', array('type' => 'page'));
$node->title = 'Drupal 6 rocks!';
node_save($node);
$node->title = 'Drupal 7 rocks!';
node_save($node);
/**
* Implements hook_node_update().
*/
function mymodule_node_update($node) {
// The node has been created with the title 'Drupal 6 rocks!'.
// Then we update it to say "Drupal 7 rocks!".
// This hook catches that update, and changes it to "Drupal 8 rocks!".
// This is a simple example that could be replaced with a presave() hook,
// but in the case of workflow changes, every status needs to be cycled
// through, no skipping allowed.
if ($node->title == 'Drupal 7 rocks!') {
$node->title = 'Drupal 8 rocks!';
node_save($node);
}
// On second update, check the value of $node->original.
elseif ($node->title == 'Drupal 8 rocks!') {
if ($node->original->title == 'Drupal 7 rocks!') {
dpm('The node has the correct original value');
}
elseif ($node->original->title == 'Drupal 6 rocks!') {
dpm('The node has an incorrect original value');
}
}
}
So, the controllers need to stop doing that isset check.
As you can see, it's the same bug in D7 and D8 core, Entity API, Commerce.
I'm opening issues one by one.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 2140171-1-always-reload-entity-original.patch | 764 bytes | bojanz |
Comments
Comment #1
bojanz commentedHere's the patch. We might want a test to go with that as well.
Core issue opened: #2140179: $entity->original gets stale between updates.
Comment #2
fagoyes, that's quite a problem with subsequent saves. With of more of those though - please see the related #1729812: Separate storage operations from reactions.
I'd suggest discussing this in the core issue and implementing the d8 core solution here afterwards.
Comment #3
chris matthews commentedThe 5 year old patch in #1 to entity.controller.inc applied cleanly to the latest entity 7.x-1.x-dev, but still needs review.
Comment #4
bojanz commentedToo late to change this.