Do not attempt to migrate Drupal 7 field instances for fields whose storage was not migrated

Created on 13 April 2023, over 1 year ago
Updated 18 July 2024, 5 months ago

Problem/Motivation

The d7_field_instance migration does not have a mechanism to skip migrating field instances for fields whose storage was not migrated by d7_field. Both d7_field_instance_widget_settings and d7_field_formatter_settings have this check at the start of their process section.

process:
  # We skip field types that don't exist because they weren't migrated by the
  # field migration.
  field_type_exists:
    -
      plugin: migration_lookup
      migration: d7_field
      source:
        - field_name
        - entity_type
    -
      plugin: extract
      index:
        - 0
    -
      plugin: skip_on_empty
      method: row

A similar check is present is also present for Drupal 6 migrations including d6_field_instance, d6_field_formatter_settings, and d6_field_instance_widget_settings to not migrate fields that were not migrated by d6_field.

D6 check was added in #2121299: Migrate in Core: Drupal 6 to Drupal 8 . D7 related code was added in #2416765: Migrate Drupal 7 Field/Instance/View mode settings .

Having this check in d7_field_instance would be useful to avoid errors that are thrown for attempting to migrate the field instance configuration of a field that does not exist.

Steps to reproduce

  1. In a Drupal 7 site, have a url field attached to the article content type.
  2. In a Drupal 10 site, with no contrib modules installed, run an automated upgrade provided by `migrate_drupal` and `migrate_drupal_ui` modules.
  3. See the recent log messages report where there are errors about trying to migrate the instance of a field type url which does not exist.

For the record, the migrate_url2link module provides an upgrade path from D7 url field to D10 link field, but that is outside the scope of this feature request.

Proposed resolution

Add the same check from d7_field_formatter_settings and d7_field_instance_widget_settings to d7_field_instance.

Remaining tasks

  • Review patch.
  • Write tests.

User interface changes

None.

API changes

Not sure if the migration plugin is a public API.

Data model changes

None.

Release notes snippet

None.

Feature request
Status

Needs work

Version

11.0 🔥

Component
Migration 

Last updated 3 days ago

Created by

🇳🇮Nicaragua dinarcon

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

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

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.

  • Issue created by @dinarcon
  • 🇳🇮Nicaragua dinarcon

    Attached is a field with the proposed changed.

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States mikelutz Michigan, USA

    So we can probably do some improvements here, but we need a little more consideration.
    Currently the situation is mostly handled by the process_field process plugin, which will throw an error if there is no field plugin available for a field, which is the main way that a field migration row can fail. It doesn't really explain itself well, and other things like bad data in the d7 database could trigger this situation (like a field config_instance entry without a corresponding field_config entry)

    Field plugins themselves can throw a skip row exception with more information on why something was skipped. namely text fields with both formatted and unformatted instance settings don't currently get migrated, and this occurrence is logged in the process_field plugin. Because you are bailing out early on any non-migrated field config though, the field plugin (which can know which fields it chooses not to migrate) does not get a chance to skip the instance row with an appropriate message. This is causing the test failure in #2 We expect and test for a few non-migrated field instances due to text format settings, and those expected error messages don't get triggered this way.

    In a sense the failing migration and error messages are intentional, as they alert developers to the issue so it can be corrected. If we quietly skip the row like we are doing here, we can lose that information, making it harder for developers to understand why fields are missing.

Production build 0.71.5 2024