Contrib.social

Fix bugs of D7->D9 insert settings migration

Open on Drupal.org →
Created on 15 February 2022, over 2 years ago
Updated 3 March 2023, over 1 year ago

Problem/Motivation

Feature request: Field-specific insert settings should be migrated per entity type and bundle.

Bugs:

  1. If the migration d7_field_instance_widget_insert_settings is executed before d7_field_instance_widget_settings, the third party settings migrated by the latter will be lost.
  2. The current migration path migrates (an invalid) insert settings for those fields where insert was configured in the past, but then it was disabled.
  3. The Drupal 7 auto insert style should be mapped to insert__auto.
  4. The custom destination plugin assumes that the insert settings migration row always have a countable at options/third_party_settings/insert destination property, but that is actually a wrong assumption: in case of fields which aren't supported (e.g. text fields, link fields, reference fields), there won't be any insert key in options/third_party_settings.

Proposed resolution

The FR part from the IS could be solved easily: instead of providing a standalone migration + a custom destination plugin, the simplest solution is to merge the migration of insert settings into the d7_field_instance_widget_settings migration provided by Drupal core:

  1. We only have to check whether insert was installed in the source, and if it is, add a new destination property and its process pipeline to the d7_field_instance_widget_settings migration in a hook_migrate_plugins_alter implementation.
  2. And then in a hook_migrate_prepare_row implementation, add the appropriate migration Row source data.

But since the module is already in a beta phase, we must be backward compatible as well, meaning that:

  1. We shouldn't remove the standalone migration.
  2. We cannot remove the process plugin...
  3. ...nor the custom destination plugin.

So the solution is:

  1. Do the insert migrations in code>d7_field_instance_widget_settings, but at the same time,
  2. Fix the standalone migration's dependency.
  3. Fix the migrate process plugin.
  4. Fix the migrate destination plugin.
  5. If the standalone migration wasn't ever executed (so: its migrate map table is empty), then we should remove its plugin definition at runtime, so it won't be executed on new installs, or on those environments where Drupal 7 -> Drupal 9 migrations weren't ever executed.
  6. If the standalone migration was executed (so: its migrate map table populated), then keep the migration executable, but trigger a deprecation,
  7. And also trigger a deprecation whenever the process or the destination plugin gets instantiated.
  8. Remove the standalone migration, the custom migrate process plugin and the custom destination plugin in Insert 3.x

Remaining tasks

  1. ✅
  2. ✅
  3. ✅
  4. ✅
  5. Create a change record.

User interface changes

Nothing.

API changes

  1. Insert settings migration will happen during the entity form display migration.
  2. The d7_field_instance_widget_insert_settings migration becomes deprecated.
  3. The field_instance_widget_insert_settings migrate process plugin becomes deprecated.
  4. The component_entity_form_display_insert migrate destination plugin becomes deprecated.

Data model changes

Nothing.

🐛 Bug report
Status

Fixed

Version

2.0

Component

Code

Created by

🇮🇳India srishtiiee

Live updates comments and jobs are added and updated live.
  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment almost 2 years ago →
    snater

    Excuse me for not checking sooner, I have not been around for a while. I can see you have spent a lot of time on making a proper patch! That's great, very appreciated! I have looked at the code and would certainly be fine merging it. While I do not have time to test it properly, I can see you have been working on it together which I guess qualifies for reviewed and tested by the community.

    I'm sorry I don't manage finding sufficient time to properly maintain the project as of now. If there is something left to do on the change, feel free to add. In any case, I'd be super ok merging. I have seen the notes on deprecating migrations to be removed in a future beta version 3.0. I'm just not 100% sure which code is to be removed then, in addition to deleting PerComponentEntityFormDisplayInsert and FieldInstanceWidgetInsertSettings. I guess I would then also remove dealing with insert_migration_plugins_alter in insert_migration_plugins_alter and remove MigrateInsertWidgetSettings::standaloneMigrationIsOmittable as well as d7_field_instance_widget_insert_settings.yml along?

    Just a tiny thing my IDE was complaining when looking at the code:

    protected static function getTargetStyleFromSource($source_style = NULL): string {

    in MigrateInsertWidgetSettings would be missing the parameter's type declaration:

    protected static function getTargetStyleFromSource(string $source_style = NULL): string {

    I'll just add that before merging in, if there is nothing else left.

    Sorry again for not being around for a long time.

  • Comment over 1 year ago →
    System Message
  • Status changed to Fixed over 1 year ago
  • Comment over 1 year ago →
    snater
  • Comment over 1 year ago →
    System Message

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

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024