Problem/Motivation

Right now, the Entity Reference revision field item creates a new revision from the referenced entity whenever the host entity being saved is a new revision. This is fine in 99% of the situations, but if you migrate content entity revisions, this behaviour creates new revisions from the entities referenced by an entity_reference_revisions field.

Steps to reproduce

Execute the test at #2977853-33: [PP-4] Multifield to Paragraphs migration.

Proposed resolution

Check the value of an unmapped (so unsaved) field property, e.g. needs_resave (or skip_new_revision). If it is explicitly set to FALSE (or TRUE), then don't create a new revision.

Remaining tasks

@tbd

User interface changes

Nothing

API changes

@tbd

Data model changes

Nothing.

CommentFileSizeAuthor
#2 entity_reference_revisions-opt_out_new_revision_save-3218312-2.patch1.37 KBhuzooka

Issue fork entity_reference_revisions-3218312

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

huzooka created an issue. See original summary.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: huzooka » Unassigned
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new entity_reference_revisions-opt_out_new_revision_save-3218312-2.patch1.37 KB

Let's see what it breaks (no new test(case)s, just a "fix").

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

It fails only on PHP 8 with this error in a bunch of different test cases:

Error: Call to undefined method Drupal\Tests\entity_reference_revisions\Kernel\Plugin\migrate\destination\EntityReferenceRevisionsDestinationTest::readAttribute()

That seems a general PHP 8 compatibility problem in this module, because this patch does nothing of the sort! /me checks… Yep, confirmed: https://www.drupal.org/node/2411903/qa.

Conclusion: this is passing all tests :) Which is good, because it's barely changing anything!

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

I'm not happy with the property's name, so suggestions are very welcome!

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

Any chance we can rely on the new syncing flag that core has instead of implementing our own thing. Not sure.

Also, dont use $this->getValue()[], that has to loop over all properties and stuff and might do weird things during a save. rely on __get() or just the values property directly. IMHO $this->needs_resave (or whatever we decide to use ) is fine, there is no better way right now to access that.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

@berdir, do you mean SynchronizableInterface?

junaidpv’s picture

junaidpv
Location Kannur, Kerala
commented

I am facing same issue with a migration from D7 to D9.5.

@huzooka where did you set value for "needs_resave" ?

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

mikelutz opened merge request !45

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Category: Feature request » Task
Priority: Minor » Normal

Sorry for the laxy PR with no tests, but I need the patch for a project right now. Will try to circle back around when I get a chance with tests.

fjgarlin’s picture

fjgarlin commented

#2984027: Incorrect manipulation of default revision flag changes the code around this issue, and neither the patch nor the MR is applicable now.

fjgarlin changed the visibility of the branch 8.x-1.x to hidden.

fjgarlin opened merge request !85

fjgarlin’s picture

fjgarlin commented

https://git.drupalcode.org/project/entity_reference_revisions/-/merge_re... should be applicable as it was created over a rebased branch. I created a new branch / MR because it was easier than rebasing for this small change. MR45 can be closed in favor of this one.