Problem/Motivation

\Drupal\Core\Entity\EntityStorageBase::doPreSave does not reload the 'original' node into $entity->original when workflows are enabled for the bundle and the entity being updated is not the 'Default Revision'.

Steps to reproduce

  1. Enable the Workflows module
  2. Add Basic Page as applying to the 'Editorial' workflow
  3. Create and move a basic page through the workflow to published, then set to draft
  4. Finally, edit the non-default revision (draft) basic page again and set it to ready for review
  5. In a presave hook, $entity->original will be set to the published revision instead of the draft revision.

Proposed resolution

\Drupal\Core\Entity\EntityStorageBase::doPreSave uses \Drupal\Core\Entity\EntityStorageBase::loadUnchanged instead of loading by the node's revision ID when the entity is not the default revision.

Workaround

Load the 'original' yourself in hook presave via the entity's revision ID.

CommentFileSizeAuthor
#16 presave-fix-revisions.patch985 bytesmakbay
#10 presave_wrong_original-3201209-10.patch4.07 KBkala4ek

Issue fork drupal-3201209

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mattsqd created an issue. See original summary.

mattsqd opened merge request !375

mattsqd’s picture

mattsqd commented
Status: Needs work » Needs review

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
joachim’s picture

joachim commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Looks good, but needs tests.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

sachin.bansal27’s picture

sachin.bansal27 commented

Still facing this issue in Drupal 9.5.0. Anyway to fix it? Thanks

kala4ek’s picture

kala4ek
Primary language Russian
Location 🇷🇺 Novosibirsk
commented
Priority: Normal » Major
StatusFileSize
new presave_wrong_original-3201209-10.patch4.07 KB

Increasing the priority of that ticket, because a lot of code do compare entities in the wrong way now.

Also attached the actual changes from MR as patch, so it can be safely used with composer.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

makbay’s picture

makbay commented

Thanks for the MR and patch but this is still working incorrectly when the latest saved entity is the default version.

Since the patch is checking if the latest version is default or not, in case the latest revision is default (published), the $entity->original again gets the previous default (published) revision instead of the latest revision.

Steps to reproduce after patch

  1. Enable the Workflows module
  2. Add Basic Page as applying to the 'Editorial' workflow
  3. Create and move a basic page through the workflow to published, then set to draft
  4. Finally, edit the non-default revision (draft) basic page again and set it to ready for review
  5. In a presave hook, $entity->original will be set to the published revision instead of the draft revision.
  6. Publish the basic page again.
  7. Create a new non-default revision (draft). The patch is ok for now, $entity->original will be set to the published version.
  8. Create another new non-default revision (e.g in review) after the published one. The patch is still ok, $entity->original will be set to the draft version on the previous step.
  9. Now publish the basic page. The patch will not work here since there is !$entity->isDefaultRevision() check in the condition and $entity->original will be set to the previous published version on step 6.

So, I think we need to remove !$entity->isDefaultRevision() check from the if block.

I'll create another MR for this.

makbay changed the visibility of the branch 9.2.x to hidden.

makbay changed the visibility of the branch 11.x to hidden.

makbay opened merge request !6655

makbay’s picture

makbay commented
StatusFileSize
new presave-fix-revisions.patch985 bytes
tobiasb’s picture

tobiasb
Primary language German
Location Berlin
commented
Related issues: +#3346430: Wrong revision can be loaded into original in \Drupal\Core\Entity\EntityStorageBase::doPreSave
brunoalmeida’s picture

brunoalmeida commented

I tested the #16 patch using the 10.3.7 core version and it's working.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Closed (duplicate)

I knew there was another one. This is fixed now in 11.x by #2839195: Add a method to access the original property