Problem/Motivation

While looking at Migration::findMigrationDependencies, I saw that migration dependencies are supposed to be automatically set when using the now deprecated 'migration' process plugin. This method has not been updated for the new 'migration_lookup' process plugin.

  /** 
   * Find migration dependencies from the migration and the iterator plugins.
   *
   * @param $process
   * @return array
   */
  protected function findMigrationDependencies($process) {
    $return = []; 
    foreach ($this->getProcessNormalized($process) as $process_pipeline) {
      foreach ($process_pipeline as $plugin_configuration) {
        if ($plugin_configuration['plugin'] == 'migration') {
          $return = array_merge($return, (array) $plugin_configuration['migration']);
        }
        if ($plugin_configuration['plugin'] == 'sub_process') {
          $return = array_merge($return, $this->findMigrationDependencies($plugin_configuration['process']));
        }
      }   
    }   
    return $return;
  }

Also, we should still support the deprecated iterator process plugin.

Proposed resolution

Add a check for the 'migration_lookup' & iterator process plugins so their dependencies are set correctly.

Remaining tasks

  • Add a failing test
  • Fix it
  • Review

User interface changes

None.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

maxocub created an issue. See original summary.

maxocub’s picture

Issue summary: View changes
Status: Active » Needs review
Issue tags: -Needs tests
FileSize
869 bytes
1.63 KB

Here's a filing test and the fix.

The last submitted patch, 2: 2945563-test-only.patch, failed testing. View results

maxocub’s picture

More test coverage and doc block improvements.

heddn’s picture

Assigned: Unassigned » heddn

Assigning to myself for review.

heddn’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
+++ b/core/modules/migrate/src/Plugin/Migration.php
@@ -620,16 +620,19 @@ public function getMigrationDependencies() {
         if ($plugin_configuration['plugin'] == 'sub_process') {

Does this need to handle iterator and sub_process, both? Either way, I'd like to see a test with iterator.

heddn’s picture

Assigned: heddn » Unassigned
maxocub’s picture

The last submitted patch, 8: 2945563-8-test-only.patch, failed testing. View results

maxocub’s picture

Title: Migration dependencies are not set when using the migration_lookup process plugin » Migration dependencies are not set when using the migration_lookup or iterator process plugins
Issue summary: View changes

IS updated.

heddn’s picture

Status: Needs review » Reviewed & tested by the community

Assuming this comes back green, I think this ready for commit. Full test coverage to fix a bug. Should be back ported to 8.5. For that I'm adding a test run, just to be safe.

The last submitted patch, 8: 2945563-8-test-only.patch, failed testing. View results

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 2945563-8.patch, failed testing. View results

quietone’s picture

Status: Needs work » Reviewed & tested by the community

Tests are passing, restoring RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 2945563-8.patch, failed testing. View results

alexpott’s picture

Status: Needs work » Reviewed & tested by the community

Testbot had a hiccup re-rtbcing

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 2945563-8.patch, failed testing. View results

maxocub’s picture

Status: Needs work » Reviewed & tested by the community

Another hiccup, back to RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed a809a15ece to 8.6.x and 6fca7ffdea to 8.5.x. Thanks!

  • alexpott committed a809a15 on 8.6.x
    Issue #2945563 by maxocub, heddn: Migration dependencies are not set...

  • alexpott committed 6fca7ff on 8.5.x
    Issue #2945563 by maxocub, heddn: Migration dependencies are not set...

Status: Fixed » Closed (fixed)

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