Incorrect manipulation of default revision flag

Created on 5 July 2018, over 6 years ago
Updated 17 December 2023, about 1 year ago

The code added in #2953343: Experimental asymmetrical content translation with Paragraphs breaks concurrent drafts β†’ in EntityReferenceRevisionsItem::preSave() method:

$this->entity->isDefaultRevision($host->isDefaultRevision());

broke the correct behavior of revision saving and loading.

For example, if the host entity is not the default revision, but the referenced entity has just one revision, it is marked as non-default which prevents saving data in field tables (only field revision tables are updated). When such entity is loaded in DataType\EntityReferenceRevisions::getTarget(), the default revision is loaded (since single revision is actually the default revision), and the data we tried to save is lost.

I suggest to return the previous behavior:

if ($host->isDefaultRevision()) {$this->entity->isDefaultRevision(TRUE);}
πŸ› Bug report
Status

Needs work

Version

1.0

Component

Code

Created by

πŸ‡·πŸ‡ΊRussia maximpodorov

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    35 pass, 6 fail
  • First commit to issue fork.
  • πŸ‡ΈπŸ‡ͺSweden twod Sweden

    We have some fairly big entities and try to limit the number of revisions created for minor changes like typo fixes. We also have Content Moderation enabled, but override it to re-enable unchecking the "New revision" checkbox.

    Our overrides now work well in most cases, except when you have added a paragraph in a Draft (non-default) revision, and try to update it without creating yet another revision. Then we basically run into the scenario from #11 (but instead of reverting to a previous revision we keep modifying the latest non-default one). The main change we've made is overriding NodeModerationHandler to not create a new revision, but still toggle published/unpublished - more or less what happens if saving a moderated entity in syncing mode.

    This means the approach in #20 won't work for us, or anyone else not always using Content Moderation to always handle the default revision state.

    It think the approach in #10 was closer to what we need, except it was missing the case when you add a new referenced entity to a draft of the parent, and then change the parent to be the default revision without creating a new revision.

    The included tests first check that creating a new non-default revision does not make the referenced entity the default, preserving existing behavior, but it's also adding a new referenced entity which becomes the default revision because it's new.

    Then it checks that updating the draft does not modify the default revision state of either referenced entity. This currently fails for the new referenced entity and causes the issue we saw.

    The last part of the test "promotes" the existing draft revision to the default revision, and checks that both referenced entities are also promoted to being the default revision - ensured by the new condition added from the #10 patch.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update about 1 year ago
    39 pass
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.4 + Environment: PHP 8.2 & MySQL 8
    last update about 1 year ago
    39 pass
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update about 1 year ago
    39 pass
  • πŸ‡ΈπŸ‡ͺSweden twod Sweden

    I'm not sure how I got it to fail with a shared table field like name earlier, maybe I had accidentally changed something in core I didn't notice.
    The problem does appear there, but is masked by the shared field seemingly always being loaded from the revisions field table if it exists, while dedicated fields are not.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.4 + Environment: PHP 8.2 & MySQL 8
    last update about 1 year ago
    28 pass, 1 fail
Production build 0.71.5 2024