Problem/Motivation
In contrib it is common for workflows to be implemented by resaving the entity in a hook_entity_update() or a matching Rule.
So if my commerce order needs to go through 4 different statuses, and can't skip them by just doing hook_entity_presave(), it is going to do do a status change and a save as the last hook_entity_update() operation / rule.
The code that initializes $entity->original is contained in EntityStorageBase::doPreSave(), the method called before hook_entity_update() implementations are invoked, which contains the following code.
// Load the original entity, if any.
if ($id_exists && !isset($entity->original)) {
$entity->original = $this->loadUnchanged($id);
}
This prevents $entity->original from being reloaded, so when I react on a fourth status change, $entity->original->status is still pointing at the first one. Ideally workflows like this would be handled in events / hooks outside of the save pipeline, but Drupal is not currently discouraging this practice.
Steps to reproduce
use Drupal\node\Entity\Node;
$node = Node::create(['type' => 'page']);
$node->set('title', 'Drupal 9 rocks!');
$node->save();
$node->set('title', 'Drupal 10 rocks!');
$node->save();
/**
* Implements hook_node_update().
*/
function mymodule_node_update(Node $node): void {
// The node has been created with the title 'Drupal 9 rocks!'.
// Then we update it to say "Drupal 10 rocks!".
// This hook catches that update, and changes it to "Drupal 11 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 10 rocks!') {
$node->set('title', 'Drupal 11 rocks!');
$node->save();
return;
}
// On second update, check the value of $node->original.
if ($node->title == 'Drupal 11 rocks!') {
if ($node->original->title == 'Drupal 10 rocks!') {
dump('The node has the correct original value');
}
if ($node->original->title == 'Drupal 9 rocks!') {
dump('The node has an incorrect original value');
}
}
}
Proposed resolution
Stop doing the isset check (it's premature optimization anyway), and then the concrete bug is fixed.
// Load the original entity, if any.
if ($id_exists) {
$entity->original = $this->loadUnchanged($id);
}
Remaining tasks
User interface changes
None.
API changes
None.
Data model changes
None.
Release notes snippet