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);}
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:

Comments

maximpodorov created an issue. See original summary.

maximpodorov’s picture

Status: Active » Needs review
StatusFileSize
new893 bytes
miro_dietiker’s picture

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

That might make sense, but we need to extend the tests to check the bug.

berdir’s picture

Thanks for the bug report.

This change was done on purpose, I actually would have expected test fails, but I guess it only fails then in combination with the symmetric translations patch.

Can you provide a failing test for your use case? I'm not sure I completely understand that.

maximpodorov’s picture

Probable test case:
- create node with paragraph A which can contain nested paragraphs
- create new revision of the node (new revision of the paragraph A is created here)
- revert to the previous revision of the node
- edit node and add nested paragraph B to the paragraph A (paragraph B has single revision here, and revision of paragraph A is not default)
- edit node and edit nested paragraph B of the paragraph A (paragraph B is set as non-default revision but actually it is the default revision - FAIL)

mikelutz’s picture

I'm also seeing an issue in this area with regard to moderation. The site I'm working with is still on the old workbench_moderation module, and I'm not sure if the same issue occurs with core content_moderation, but I wanted to mention it.

In this case when I'm saving a new draft off of a published state, this check seems to be ran before content moderation updates the defaultRevision status of $host. So if I have an existing published entity with a paragraph, and I edit and save a draft, the new node revision is not set as default revision, but the new paragraph revision is, which (combined with other code not properly checking revision ids) causing my draft paragraph to leak into the published page on display.

Again, I'm not sure if this is still an issue with content_moderation, but I'm working through a bunch of things with paragraphs, moderation, translation and all the known issues that surround them, so as I dig further I'll post patches as I find new things and work through the existing issues.

JvE’s picture

Yes, the same issue occurs with core's content_moderation.

entity_reference_revisions syncs the entity's isDefaultRevision with the host in \Drupal\entity_reference_revisions\Plugin\Field\FieldType\EntityReferenceRevisionsItem::preSave()
but moderation (both content_ and workbench_) updates the host value of isDefaultRevision in hook_entity_presave()
which always runs later.

other cases may set the isDefaultRevision in \Drupal\Core\Entity\EntityStorageBase::doSave() which is also after doPresave();

Bottom line is that in there is no way for entity_reference_revisions to know at that point if the host is going to be a default revision.

Let's try doing the sync in the postSave instead.

JvE’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: entity_reference_revisions-default_revision-2984027-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

maximpodorov’s picture

Status: Needs work » Needs review
StatusFileSize
new1.19 KB

What about this variant? I suggest to change the revision state only when new revision of the entity is created. This will not make the existing default revision non-default (with breaking the Drupal entity loading logic).
Anyway we don't cover the case when the content_moderation module forces the new revision creation for the host entity in hook_entity_presave which is executed after the field's presave method.

indytechcook’s picture

I was also able to reproduce this issue with the following workflow:

  1. Create a node with a paragraph
  2. Publish a node
  3. Create a new draft
  4. Save a new draft revision
  5. Set the "Set Current Revision" for a draft revision that occurred after the published revision.

The patch on comment #7 does work while #10 does not. It's important to note that this update will not fix bad data, it will only not change the default revision on save.

I'm not marking this as RTBC since I think it needs a data fix script and more testing (along with tests)

miro_dietiker’s picture

Issue tags: +needs update hook

Yeah... Wasn't sure about the tag, is there one for a script?

Covering this in a update hook can lead to major downtime on larger sites and we don't need to keep the site locked in maintenance mode to fix old revisions.

+++ b/src/Plugin/Field/FieldType/EntityReferenceRevisionsItem.php
@@ -273,11 +273,10 @@ class EntityReferenceRevisionsItem extends EntityReferenceItem implements Option
+          if ($host->isDefaultRevision() != $this->entity->isDefaultRevision()) {

I would assume that further indenting this below the isNewRevision() could cause a bug.

While the UI always writes a new revision on revert, i thought it is possible to set an existing previous revision as the default revision. Can / should we cover this case here as well or does such an API use also need to support ERR / Paragraphs specifically on its own?

indytechcook’s picture

I might have spoken too soon the patch at #7 as it doesn't seem to work anymore. I'm seeing what changed on our side.

I did put together a migration script I will run with drush scr which you can see here: https://github.com/department-of-veterans-affairs/va.gov-cms/pull/3409/f...

I agree about not running this in an update hook.

I'll continue to test the patches and will report back.

indytechcook’s picture

After retesting the patch at #10 it does indeed work. I must have messed up something with my testing and patch applying. Thanks for getting this fixed!

johnnydarkko’s picture

#10 works for me! Default revision is getting saved correctly for me now. Thanks!

miro_dietiker’s picture

Just out of curiosity as i see here some potential issues with content moderation..

Did anyone test this in a multilingual scenario?

I really don't like storage state inconsistencies, so I promise we will look into this if someone is helping with writing a test (or two)...

berdir’s picture

Status: Needs review » Needs work

I've been working on reproducing and fixing the special case that we noticed with duplicating sections of paragraphs: #3198091: Changes of newly replicated Paragraphs are not saved. See my latest comment there, the referenced core issue helps in our case, but that is only going to help with the special case of duplicating.

But we are not seeing the problems mentioned here with paragraphs with the default (stable) widget. the needs resave flag that is checked sounds like IEF? That's definitely not something that we can rely on, that's not a default feature of ERR.

In general, I agree that there are problems, but I'm not sure how to fix it yet. #7 seems most likely to go in the right direction, I would need to check the test fail there. Maybe attempt both? hope that the flag has been set correctly initially, optimize for that and fix it afterwards if necessary. Fixing afterwards is more expensive. Or improve core to better handle that upfront? the revision flag is set earlier because the entity form also does it by default, at least if the entity type allows to configure it.

I'm not sure if an update hook is necessary, if someones persisted data is affected by this then they could run a script manually..

dishabhadra’s picture

StatusFileSize
new1.63 KB

Yes, the same issue I am also facing when core's content_moderation is enabled and we are using the Node revert feature.

#7 comment explanation is correct.

The way content_moderation /core/modules/content_moderation/src/EntityOperations::entityPresave() checking the current state of the entity similar way I implemented logic in EntityReferenceRevisionsItem::preSave() method.

Please review my patch.

dishabhadra’s picture

StatusFileSize
new1.75 KB

Updated the patch by checking Drupal::hasService().

dishabhadra’s picture

StatusFileSize
new1.73 KB

I found an issue in the #19 patch, For nested paragraphs, it was setting the default value to 0 due to an incorrect if/else condition.

Updated the patch.

TwoD made their first commit to this issue’s fork.

twod’s picture

StatusFileSize
new5.97 KB

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.

Edit: Ok, the test-fail patch not failing is curious. I verified it just before uploading but must have done something to it...

twod’s picture

StatusFileSize
new7.35 KB

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.

timmy_cos’s picture

Note that with https://www.drupal.org/project/drupal/issues/3499181 introduced in Core 11.2 this issue is now capable of producing an exception. Attached MR seems to resolve the problem :)

timmy_cos’s picture

Status: Needs work » Needs review
rahulrasgon’s picture

StatusFileSize
new15.49 KB

Uploading MR as patch.

  • berdir committed 639d22ff on 8.x-1.x authored by twod
    fix: #2984027 Incorrect manipulation of default revision flag
    
    By:...
berdir’s picture

Status: Needs review » Fixed
Issue tags: -Needs tests, -needs update hook

Wen through the test and changes, this looks good, merged.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

safallia joseph’s picture

Regression report: Paragraph revisions deleted during draft saves with Content Moderation

We've encountered a critical regression introduced by the fix in 8.x-1.14 for this issue. After upgrading entity_reference_revisions from 1.10 to 1.14, draft paragraph content started appearing on published pages. Downgrading back to 1.10 immediately resolved the problem.

Environment:

Drupal 10.6.4
Paragraphs 1.x (latest)
Content Moderation enabled (draft / published workflow)
Nested paragraph structure: Node → band paragraph (ERR field) → text_block paragraphs (ERR field)
Steps to reproduce:

Create a node with a band paragraph containing 2+ child text_block paragraphs
Publish the node
Create a new draft, editing only the second text_block paragraph (leave the first untouched)
Save as draft
Expected: Published page continues showing published content. Draft shows updated content.

Actual (with 1.14): The published revision of the edited paragraph is deleted. The published page now shows draft content (or missing content). The paragraph's previous revision is gone from the database entirely.

Root cause analysis:

Two changes in this fix appear to interact badly with Content Moderation and nested paragraphs:

preSave() — Moving isDefaultRevision() inside the isNewRevision() block and calling it unconditionally changes the default revision flag on paragraphs that weren't edited during non-default (draft) revision saves. Previously, the standalone if only synced when there was an actual mismatch.

deleteRevision() — The old code simply returned early for default revisions (if (!$child || $child->isDefaultRevision()) { return; }). The new code now queues default-revision paragraphs for orphan purging. Combined with the preSave() change incorrectly manipulating revision flags, this causes unedited paragraphs to be purged during draft saves.

The issue is specifically triggered with nested paragraphs (2+ levels deep) and Content Moderation, when only a later-positioned child paragraph is edited. Editing the first child paragraph masks the bug because it triggers the correct save chain for all siblings.

Workaround: Pin entity_reference_revisions to 1.10.

I believe this issue should be re-opened to address this regression, or a follow-up issue should be created.

aaron.ferris’s picture

This is more of a heads up than anything, I've got a project using the patch from comment 20 and I see an issue with paragraphs on draft translations being published early.

Core 11.3.2, latest paragraphs, ERR 1.12 with the patch from 20. Content moderation and 3 languages.

1. Create a node with a paragraph - English, draft, save
2. Add Spanish translation in draft, save
3. Add French translation in draft, save
4. Publish all 3.
5. Create new draft in Spanish, save
6. Create new draft in French, save
7. Publish French
8. Spanish draft values now live

Without the patch = this doesn't happen.

With the patch from 27 = this doesn't happen.

Again, this is more a heads up for anyone else who might be using that specific patch.