Problem/Motivation

Current migrate destination plugin supports a "new_revisions" configuration option that creates a revision for the entity on migrate. However, it calculates the revision id from the old destination id values, and in a regular import (with update) those might be always filled in, so there's no way to force a revision to be created if you'd want a migrate process that always kept a track of the imported values.

My use case is importing (and regularly update) an entity that has a particular value I need to track changes over time.

Proposed resolution

Add a "force_revision" configuration option to always generate a revision.

Remaining tasks

Review on the proposed patch is needed to see if this is a change that would be interesting for the module and decide what direction to take.

CommentFileSizeAuthor
#14 reroll_diff_10_14.txt2.7 KBpookmish
#14 3004210-14.patch5.86 KBpookmish
#10 3004210-10.patch6.35 KBpcambra
#8 3004210-8.patch3.72 KBpcambra
#4 3004210.patch1.47 KBpcambra
#2 3004210.patch1.47 KBpcambra

Comments

pcambra created an issue. See original summary.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Active » Needs review
StatusFileSize
new 3004210.patch1.47 KB
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work 
+++ b/src/Plugin/migrate/destination/EntityReferenceRevisions.php
@@ -22,6 +22,8 @@ use Drupal\migrate\Row;
  * - new_revisions: (optional) Flag to indicate if a new revision should be
...
+ * - force_revision: (optional) Flag to ignore other checks and always create a

@@ -118,9 +120,14 @@ class EntityReferenceRevisions extends EntityRevision implements ConfigurablePlu
+    if ($this->getConfiguration()['new_revisions']) {

Did you accidentally mix the names?

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 3004210.patch1.47 KB

Most probably :)

Status: Needs review » Needs work

The last submitted patch, 4: 3004210.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Issue tags: +Needs tests

As the key is optional, we can not expect it being present.

Will you find time to test cover it a bit?

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

Will you find time to test cover it a bit?

Yup, just wanted to ask if you'd be OK with adding something like this.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 3004210-8.patch3.72 KB

Let's see how's this looking.

Status: Needs review » Needs work

The last submitted patch, 8: 3004210-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 3004210-10.patch6.35 KB

Had to use a slightly different data structure for this

pookmish’s picture

pookmish commented

The patch in #10 worked for me using the "force_revision: true" but i had to make some extra changes to make it work correctly with node revisions. Note, i'm using migrate plus and migrate tools. I'm not sure if theres a way to incorporate this into the plugin?

process:
  revision_timestamp:
    -
      plugin: callback
      callable: time
  revision_translation_affected:
    -
      plugin: default_value
      default_value: 1
destination:
  plugin: 'entity_reference_revisions:node'
  new_revisions: true
  force_revision: true
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

@pookmish not sure if I understand what do you mean with incorporating changes into the plugin?

For using force_revisions, you need to set new_revisions to true, I think that's in the docs of the plugin, what else would be needed?

pookmish’s picture

pookmish commented

not in the plugin defintinon, but in the process on the node. Without processing revision_timestamp and revision_translation_affected a new revision was created with a new revision ID, but it never appeared on the revisions list page for the node. Once i set the two processes i was able to see revisions on every migration update. note that i was also using the diff module.

I wasn't sure if incorporating the changes to the timestamp and translation affected would be ideal. but at least providing my experience may help others.

pookmish’s picture

pookmish commented
StatusFileSize
new 3004210-14.patch5.86 KB
new reroll_diff_10_14.txt2.7 KB

Rerolling for latest code.

merilainen’s picture

merilainen commented
Status: Needs review » Reviewed & tested by the community

This is exactly what I needed and works perfectly!

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks.

  • Berdir committed 032ad11 on 8.x-1.x authored by pookmish 
    Issue #3004210 by pcambra, pookmish: [Migrate] Include a migrate...

Status: Fixed » Closed (fixed)

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