This is just for the contrib side of https://www.drupal.org/project/drupal/issues/2934799. Once we correctly change the d6_term_node_revision migration to use d6_node_revision for its migration_lookup mapping instead of d6_node, we need to change the code here to make sure it will have the necessary migration IDs available to it when it is run.

CommentFileSizeAuthor
#4 interdiff_2-4.patch2.21 KBOlarin
#4 2935225-4.patch3.13 KBOlarin
#2 2935225-2-migrate_upgrade-fix_to_d6_term_node_revision_handling.patch2.93 KBOlarin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Olarin created an issue. See original summary.

Olarin’s picture

Olarin CreditAttribution: Olarin at Kosada commented
Status: Active » Needs review
FileSize
2935225-2-migrate_upgrade-fix_to_d6_term_node_revision_handling.patch2.93 KB

Here's my patch. Note that without the core patch linked above, it will result in all d6_term_node_revision items being ignored. (Which is only a slight regression from the existing behaviour of some/most of them being incorrectly ignored, which the two patches together should correct.)

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Needs review » Needs work 
  1. +++ b/src/MigrateUpgradeDrushRunner.php
@@ -52,6 +52,13 @@ class MigrateUpgradeDrushRunner {
+  protected $revisionMigrations = [];

@@ -123,22 +130,19 @@ class MigrateUpgradeDrushRunner {
+      $this->revisionMigrations[] = $migration->id();

@@ -146,7 +150,22 @@ class MigrateUpgradeDrushRunner {
+    elseif ($source['plugin'] == 'd6_term_node_revision') {
...
+      $process = $migration->getProcess();

    Can we name this variable d6RevisionMigrations? 

  2. +++ b/src/MigrateUpgradeDrushRunner.php
@@ -146,7 +150,22 @@ class MigrateUpgradeDrushRunner {
+            is_string($plugin_configuration['migration']) &&

    This will always be a string, no?

Olarin’s picture

Olarin CreditAttribution: Olarin at Kosada commented
FileSize
2935225-4.patch3.13 KB
interdiff_2-4.patch2.21 KB

Can we name this variable d6RevisionMigrations?

Sure, but we should probably update the existing nodeMigrations variable to d6NodeMigrations for consistency, yes? Updated patch attached.

This will always be a string, no?

Probably? I don't know. I was modeling after the existing code immediately prior that popuplates the nodeMigrations variable. The is_string call there comes from mikeryan in commit 34f29d0b0c5b35ec5854d3596ebb95b59c5ccc88, so I guess ask him. Perhaps that was just a way of ensuring that the key had not been left out, and is_string made as much sense as isset (since an invalid value wouldn't do us much good either)?

Olarin’s picture

Olarin CreditAttribution: Olarin at Kosada commented
Status: Needs work » Needs review

  • heddn committed b209394 on 8.x-3.x authored by Olarin 
    Issue #2935225 by Olarin, heddn: Fix to d6_term_node_revision handling (...
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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