- Issue created by @dinarcon
The last submitted patch, 2: d7_field_instance_field_type_exists-3354122-2.patch, failed testing. View results →
- Status changed to Needs work
over 1 year ago 2:54pm 20 April 2023 - 🇺🇸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.