Like @gbotis in https://www.drupal.org/project/inline_entity_form/issues/2953036#comment..., the changes in that issue broke my migration because they assume that $migration['process']['settings'] isn't already an array.

Example where this doesn't work:

settings:
    -
      plugin: d7_field_instance_settings
      source:
        - settings
        - widget
        - field_definition
      -
        plugin: my_example_plugin

Typically in core the settings look like this, so it works for most cases:

settings:
    plugin: d7_field_instance_settings
    source:
      - settings
      - widget
      - field_definition

As inline_entity_form adds its own key (expected):

  settings:
    0:
      plugin: d7_field_instance_settings
      source:
        - settings
        - widget
        - field_definition
    inline_entity_form:
      plugin: inline_entity_form_field_instance_settings

the current implementation would double-nest the first example!

Proposed resolution:
Only convert settings to array, if it's not a nested array already. This can be detected by checking if 'plugin' key is existing at the first level.

CommentFileSizeAuthor
#15 3213595-migrationhelperalterplugins-assumes-settings-MR54-15.diff829 bytesanybody
#4 3213595.patch671 bytesdjdevin
#2 Issue-3213595-MigrationHelper-alterPlugins-assumes-s.patch1 KBnigelcunningham

Issue fork inline_entity_form-3213595

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

NigelCunningham created an issue. See original summary.

nigelcunningham’s picture

nigelcunningham commented
Status: Active » Needs review
StatusFileSize
new Issue-3213595-MigrationHelper-alterPlugins-assumes-s.patch1 KB
damyon’s picture

damyon commented

I can reproduce this error and the patch works for me.

djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new 3213595.patch671 bytes

Rolled against latest.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs review » Reviewed & tested by the community

I also encountered this problem. The patch here fixes it for me!

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Parent issue: » #3272784: 3272784-migration-meta-issue-combined
geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

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

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Related issues: +#3221074: Implemented hook_migration_plugins_alter() breaks migrations, if shared configuration used in migration configs.

@dsnopek & @djdevin: This seems closely related to #3221074: Implemented hook_migration_plugins_alter() breaks migrations, if shared configuration used in migration configs. how do you think should we proceed here? Could you have a look at the patch in the other issue? Should we close one as duplicate or combine both somehow?

#3294481: [PP-2] Update MigrationHelper and add hook_field_migration_field_widget_info() to add widget mappings. also contains some of the fixes from here plus other fixes!

We should make progress on the migration issues to get the parent issue fixed and things resolved!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Related issues: +#3294481: [PP-2] Update MigrationHelper and add hook_field_migration_field_widget_info() to add widget mappings.
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

From the results in #3272784: 3272784-migration-meta-issue-combined I can now clearly say, that this is an independent issue and the proposed implementation in #4 fixes THIS issue. I'm creating a MR for review, so this can be committed finally.

Anybody opened merge request !54

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Postponed (maintainer needs more info) » Needs review

Okay here we go, setting this back to Needs review after the updates in the parent issue. Hope this is okay now @geek-merlin!
This is a clear and separate issue now, I updated the issue summary and prepared the tested fix.

grevil’s picture

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

We just tested it with a custom Drupal migration, works as expected!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
StatusFileSize
new 3213595-migrationhelperalterplugins-assumes-settings-MR54-15.diff829 bytes

The failing tests are unrelated and come from the module itself (see module page).

Here's the patch from MR!54 against 8.x-1.x-dev or -rc12 if anyone needs it for composer!

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

Re-run the patch test and it's green now.

  • geek-merlin committed 748816b on 8.x-1.x authored by Anybody 
    Issue #3213595 by Anybody, NigelCunningham, djdevin, geek-merlin, damyon...
geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Reviewed & tested by the community » Fixed

Merged to HEAD. Thanks everyone! And especially to the friendly Drowl folks!

grevil’s picture

grevil commented

Thank you @geek-merlin! Great addition. :)

Status: Fixed » Closed (fixed)

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