Field formatters with names different than their field type can not be migrated

I agree that using a static map is *very* fragile. If we can gather that data, then map it, we are going to be a whole lot better off. I think it is possible to glean the field_formatter data from the d6/d7 site. Or is that a hook_info thing that never get's stuffed in the db?

I just faced this issue today when working on #2594631: Migration integration. I had almost the same solution in mind before I found this issue. Since this is three months old, I hope you don't mind me making significant changes. Basically, I removed what I thought were out of scope changes and added a few more things like ability to specify the property name where field_type will be available in a source plugin.

Interestingly, the D6 version of field formatter migration already uses 'type' to indicate the field type and not the widget type. It is only the D7 migration which has this problem and hence I have only changed that YML file. I am wondering if we should make it consistent by setting 'type' to field_type and rename the existing 'type' to 'widget_type'. What do you think?

I forgot to mention that there is a related issue with fields in general when their plugin ids don't match the field name in source. I have create an issue for that at #2787639: MigrateCckFieldPluginManager mixes up its behaviour for creating and loading definitions and the changes in this patch will clash with that patch.

I am wondering if we should make it consistent by setting 'type' to field_type and rename the existing 'type' to 'widget_type'. What do you think?

Yes, please! This will definitely improve the readability of the code.

I am wondering if we should make it consistent by setting 'type' to field_type and rename the existing 'type' to 'widget_type'. What do you think?

I think hell freaking yes. Please do it.

+++ b/core/modules/migrate_drupal/src/Plugin/migrate/CckMigration.php
@@ -104,8 +104,12 @@ public function getProcess() {
+      $field_type_property_name = !empty($this->pluginDefinition['cck_plugin_field_type_property_name']) ?
+        $this->pluginDefinition['cck_plugin_field_type_property_name'] :
+        'type';

I see why we're doing this (backward compatibility, right?), but I'm not sure migrations should have THIS level of control over the source plugin. I'm in favor of renaming it to field_type and not making it configurable at all. That will break a lot of tests, but IMHO it'll be worth it.

Rerolling to get started again.

Fixing at least a couple of tests and then changing the name of field_type to type as per #12 and #13. There might be other failures.

We don't need this anymore. Removing the ability to specify field type column name from the YML file.

Fixing some more failures (hopefully all).

Small change to the text.

Looks good to me, thanks!

Oh well...

Re-roll.

Back to RTBC, thanks!

+++ b/core/modules/field/migration_templates/d7_field_formatter_settings.yml
@@ -51,7 +51,7 @@ process:
-      source: type
+      source: formatter_type

So our one BC issue is this change - right? As anyone who is doing a custom migration might have based things of this template. OTOH right now if this is not working for them they will have to have applied this patch (or something similar) to make it work. So what's the value in a BC layer?

+++ b/core/modules/field/src/Plugin/migrate/source/d7/FieldInstancePerViewMode.php
@@ -24,8 +24,15 @@ protected function initializeIterator() {
+        // Rename type to formatter_type in the info array.
+        $info['formatter_type'] = $info['type'];
+        unset($info['type']);

Hmm... that said - is this is going to break existing working migrations?

Or should the argument be that D7 migration is currently a little rough and this patch helps therefore let's get this in?

Jo/Hussain - can you review this and make sure I'm understanding and explaining this correctly?

So our one BC issue is this change - right? As anyone who is doing a custom migration might have based things of this template. OTOH right now if this is not working for them they will have to have applied this patch (or something similar) to make it work. So what's the value in a BC layer?

In cases where the field type and formatter type match (which is common), it works - it's only broken when they don't. So, if someone has a migration .yml using 'type', the BC layer not only keeps the old case working, by substituting 'formatter_type' it will now work better for them.

Hmm... that said - is this is going to break existing working migrations?

This is the BC layer - if someone has specified 'type', it gets converted to the new preferred 'formatter_type' and works just ducky.

@mikeryan That is my understanding of the situation - I don't envisage BC issues.

Random fail, kicked off retest, presumptively re-RTBCing.

Not reviewed yet, just restoring status.

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!