Implemented hook_migration_plugins_alter() breaks migrations, if shared configuration used in migration configs.

Thanks a lot! I added some related issues and it seems one of the latest patches did a regression.

Tbh i don't grok the migration plugin descriptor structure you're handling, but the code looks like you know what you are doing.
Tests are green. So LGTM.

Does this fix the issue for you?
I also pointed people in the related issues (with some suppress-only patches) to this one to maybe get additional feedback.

Also, if you can push to an issue branch, it will significantly make it easier to merge ;-).

Just wanted to say that I also got hit by the bug, and this patch fixed it for me. Thank you!

Still not working for me I'm afraid, the patch makes no difference at all to the error or to the process plugins.
The new function getMigrationWithSharedConfiguration runs and it seems to me that the problems is with the altered structure of migration processes.

    [inline_entity_form] => Array
        (
            [plugin] => inline_entity_form_field_instance_settings
        )

    [1] => Array
        (
            [0] => Array
                (
                    [plugin] => d7_field_instance_settings
                    [source] => Array
                        (
                            [0] => settings
                            [1] => widget
                            [2] => field_definition
                        )

                )

            [inline_entity_form] => Array
                (
                    [plugin] => inline_entity_form_field_instance_settings
                )

        )

Do I need this to work to upgrade from D7 Ubercart?

Patch from #3 also works for us to fix this issue.

• Do we really need to provide an own getMigrationWithSharedConfiguration() or does migrate module provide such method in some way?
• Should we add a isset($migration['source']['plugin']) before $source_plugin_manager->createInstance() ?

The patch is fixing the error. Thank you.

It's also breaking migrations for me in a similar way as #7.

I ran into a similar issue described in #7. Inline Entity Form module doesn't cover the case if the migration already defines the additional settings process for inline_entity_form, or more specifically whether the settings process array is a plugin or is an array of plugins. This patch checks to see if "plugin" key exists, if it does then it is wrapped inside an array prior to merging the addition rather than always wrapping the result inside an array.

Here is small improvement for #3 patch

Patch from #11 helped. Thanks!

Patch from #11 fixed the issue for me

#11 no longer helps me! I required to apply change suggested in #3208995: Alter plugins results in empty string which is invalid plugin ID instead.

> #11 no longer helps me!
What does this mean? Does it apply? What happens? What about #12?

> #11 no longer helps me!
What does this mean? Does it apply? What happens? What about #12?

It does apply, but it keeps throwing "The "" plugin does not exist." error.
Same for #12 as well.

Hi, I made this MR !49 in order to avoid creating the instance of the source plugin of the migration.
Because when we create an instance of the source plugin it inits the query to the source of the migration that
occurs issues when the migration source is not reachable.
For example, API is not available from the local machine.

The patch was created in contributing with @alt.dev it would be great if he will get a credit as well.

Hello there

Can we reroll please? This patch is no longer working for me on Drupal 9.4.4.
https://www.drupal.org/files/issues/2021-11-19/3221074-12.patch

This is the reject file. There is an error given on composer update however most of the patch is applied.
MigrationHelper.php.rej

***************
*** 31,36 ****
        /** @var \Drupal\migrate\Plugin\MigrateSourcePluginManager $source_plugin_manager */
        $source_plugin_manager = \Drupal::service('plugin.manager.migrate.source');
        $source = NULL;
        $configuration = $migration['source'];
        $source = $source_plugin_manager->createInstance($migration['source']['plugin'], $configuration, $migration_stub);
        if ($source) {
--- 31,41 ----
        /** @var \Drupal\migrate\Plugin\MigrateSourcePluginManager $source_plugin_manager */
        $source_plugin_manager = \Drupal::service('plugin.manager.migrate.source');
        $source = NULL;
+       if (!empty($migration['migration_group'])) {
+         // Integrate shared group configuration into the migration, in order to
+         // have full migration definitions in place.
+         $this->getMigrationWithSharedConfiguration($migration);
+       }
        $configuration = $migration['source'];
        $source = $source_plugin_manager->createInstance($migration['source']['plugin'], $configuration, $migration_stub);
        if ($source) {

I have updated this patch based on the newest version of the module 1.0.0@rc12

https://www.drupal.org/files/issues/2022-08-01/3221074-13.patch

Latest patch file which works with the latest version of this module.

https://www.drupal.org/files/issues/2022-08-01/3221074-14.patch

Too many migration issues, many of which look overlapping or duplicate.
Postponing on parent issue.

Hi @Axel,

settings this back to active as of #3275819: Provide useful error message for: The "" plugin does not exist on Drupal 6 / 7 migrations which we just ran into. The migrations were created with 8.x-1.0-rc12, so I'm updating the version. We can set this to 8.x-1.x-dev if they are same, @Grevil will check if 8.x-1.x-dev gives better results tomorrow.

I guess this might also be the root cause of: https://www.reddit.com/r/drupal/comments/of145l/the_plugin_does_not_exis...

This is major as it breaks migrations and is hard to find!

Ok we can safely set this to 8.x-1.x-dev. src/MigrationHelper.php is the same in rc12 and 8.x-1,x-dev (not changed since 12 months) so this is definitely still an issue!

With inline_entity_form enabled, this is what changes:

--- a/tmp/drush_tmp_1661935158_630f1e3676b3d/migrate_plus.migration.upgrade_d7_field_instance.yml
+++ b/tmp/drush_tmp_1661935158_630f1e36c8378/migrate_plus.migration.upgrade_d7_field_instance.yml
@@ -83,14 +83,12 @@ process:
           migration: upgrade_d7_taxonomy_vocabulary
           source: vid
   settings:
+    0:
-    -
       plugin: d7_field_instance_settings
       source:
         - settings
         - widget
         - field_definition
+    inline_entity_form:
+      plugin: inline_entity_form_field_instance_settings
   default_value_function:
     -
       plugin: get

Please read the parent issue before acting.

Thanks @geek-merlin, you're right. I'm going to help cleaning and clearing things up as good as possible.

@geek-merlin: From my perspective, we should set this issue "Needs review", as after checking all sibling issues from the parent task, this one is separate and there's no duplicate for this issue anymore.

If you agree, would you do that?

Next steps are to compare the approach from #28 (3221074-14.patch) to the approach from MR!49, which use very different implementations!

@Lysenko wrote in #25:

Hi, I made this MR !49 in order to avoid creating the instance of the source plugin of the migration.
Because when we create an instance of the source plugin it inits the query to the source of the migration that occurs issues when the migration source is not reachable. For example, API is not available from the local machine.

Sorry, but I don't really get the point here. Could you or someone else please compare the outcome of both patches and tell, which one is better to proceed with?

Do we have an inline_entity_form maintainer with deeper migration knowledge, we could ask for having a look?

Thanks Anybody for the thorough cleanup!

I have reviewed everything.
Short verdict: Move on with #28.

Triggered bot and #28 still applies and is green.
MRs 48/49 take the work of #28 and adds lots of unrelated changes. I doubt that something useful is in there, but if it is, it's not reviewable.
(Unrelated comment: People told me that there is a current discussion what the contribution point system encourages.)

@everyone here, receiving this notification and anyone coming here with this issue, could you please check and confirm if the patch from #28 fixes the issue for you?

We didn't run into this situation and @geek-merlin also didn't, I guess, so we need someone to try, if this is the correct solution.
All related migration fixes are now in 8.x-1.x-dev, this is currently the last open migration issue here. Please help to fix it.

@geek-merlin, what do you think about a new 8.x-1.0-rc13 release from current dev? So less people may run into the fixed issues...
I don't think this issue will be solved on the short term...

> @geek-merlin, what do you think about a new 8.x-1.0-rc13 release from current dev?

Agree once we have the other issue where you test on a migration ;-)

@Anybody, you groked all the migration issues... We have this patch #11 which looks unrelated here, but relevant.
Can you cross-check if that's in one of the other migraiton patches?

Also we have a relevant question in #8:
> Do we really need to provide an own getMigrationWithSharedConfiguration() or does migrate module provide such method in some way?

@geek-merlin: Re #40: #11 looks to me like a duplicate of the already committed https://git.drupalcode.org/project/inline_entity_form/-/commit/748816b from #3213595: MigrationHelper::alterPlugins assumes settings isn't already an array

Re #41 I'm sadly not able to answer this. We didn't run into such issues, so I think someone else has to pick this up from here... or maybe this is magically solved by the other fixes, but I don't think that's realistic from a code perspective.

Over and out here (if we shouldn't run into these issues sooner or later).

Thx! So we're at #38: Please someone(tm) test if the patch in #28 fixes the issue for them.

#28 fixed the migrated error for me. Tested with 8.x-1.0-rc12.

So the last migration bug is hereby fixed, thanks all!
Tagging rc13 release.

Thank you very much, Axel! :) I hope we'll see no regression. inline_entity_form was a huge part of our migration issues, so it's great to see this should be fixed now. Thanks for the important release!