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);}
| Comment | File | Size | Author |
|---|---|---|---|
| #27 | default_revision_revert-2984027-27.patch | 15.49 KB | rahulrasgon |
| #24 | err-2984027-25.TEST-ONLY-FAIL.patch | 7.35 KB | twod |
| #20 | default_revision_revert-2984027-20.patch | 1.73 KB | dishabhadra |
| #19 | default_revision_revert-2984027-19.patch | 1.75 KB | dishabhadra |
| #18 | default_revision_revert-2984027-18.patch | 1.63 KB | dishabhadra |
Issue fork entity_reference_revisions-2984027
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
Comment #2
maximpodorov commentedComment #3
miro_dietikerThat might make sense, but we need to extend the tests to check the bug.
Comment #4
berdirThanks 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.
Comment #5
maximpodorov commentedProbable 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)
Comment #6
mikelutzI'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.
Comment #7
JvE commentedYes, 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.
Comment #8
JvE commentedComment #10
maximpodorov commentedWhat 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.
Comment #11
indytechcook commentedI was also able to reproduce this issue with the following workflow:
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)
Comment #12
miro_dietikerYeah... 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.
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?
Comment #13
indytechcook commentedI 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 scrwhich 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.
Comment #14
indytechcook commentedAfter 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!
Comment #15
johnnydarkko commented#10 works for me! Default revision is getting saved correctly for me now. Thanks!
Comment #16
miro_dietikerJust 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)...
Comment #17
berdirI'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..
Comment #18
dishabhadra commentedYes, 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.
Comment #19
dishabhadra commentedUpdated the patch by checking Drupal::hasService().
Comment #20
dishabhadra commentedI 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.
Comment #22
twodWe 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
NodeModerationHandlerto 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...
Comment #24
twodI'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.
Comment #25
timmy_cos commentedNote 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 :)
Comment #26
timmy_cos commentedComment #27
rahulrasgon commentedUploading MR as patch.
Comment #29
berdirWen through the test and changes, this looks good, merged.
Comment #32
safallia joseph commentedRegression 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.
Comment #33
aaron.ferris commentedThis 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.