Problem/Motivation

Drupal 6 text area text processing settings are not respected when migrating D6 fields to D8. Steps to reproduce:

D6 Field type: text
D6 Widget type: Text area (multiple rows)
D6 Global field settings / Text processing Plain text
Expected D8 field storage type: Text (plain, long)
Actual D8 field storage type: Text (formatted, long)

Proposed resolution

Fix getFieldType() of class \Drupal\text\Plugin\migrate\cckfield\TextField so that the text processing settings are respected.

Remaining tasks

I'll create a patch that fixes this and update the unit tests.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#17 2849861-d6-textarea-format-2-reroll.patch2.89 KBjoelpittet
#2 2849861-d6-textarea-format-2.patch2.94 KBmasipila

Comments

masipila created an issue. See original summary.

masipila’s picture

masipila commented
Status: Active » Needs review
StatusFileSize
new 2849861-d6-textarea-format-2.patch2.94 KB
jeffwpetersen’s picture

jeffwpetersen commented

In order to migrate my fields correctly for my pourpses / test case I need to load widget_settings from the d6 content_node_field_instance table

and use the "size" or "row" settings to determine which field type would be used in D8.

public function getFieldType(Row $row) {
  $widget_type = $row->getSourceProperty('widget_type');
  $settings = $row->getSourceProperty('global_settings');
  $widget_settings = $row->getSourceProperty('widget_settings');

If $widget_settings['size'] exists and is between then...

or if $widget_settings['rows'] exists and rows # than append _long...

masipila’s picture

masipila commented

Hi,

if you have rows defined in D6, then isn't it so that the D6 widget type is always Text area (multiple rows), which indicates that this field should be migrated to D8 as either Text (plain, long) or Text (formatted, long) depending on the global text processing settings in D6? In other words, we don't need to investigate the global D6 field length if the widget type is Text area (multiple rows)?

Markus

jeffwpetersen’s picture

jeffwpetersen commented 
 if (empty($settings['max_length']) || $settings['max_length'] > 255) {
        $field_type .= '_long';
      }

The problem is that "max_length" is empty in 'global_settings' so I always get text_long. And the only way to determine if the field value is of type "size" is to load 'widget_settings' from content_node_field_instance.

So to get me to plain_text I need to test for the 'widget_settings' size value.

$field_type = $settings['text_processing'] ? 'text' : 'string';
$widget_settings = $row->getSourceProperty('widget_settings');

if (!empty($widget_settings['size']) && $widget_settings['size'] < 256) {
  return $field_type;
}
masipila’s picture

masipila commented

Hi,

I just tested on my D6 test site that it is possible to updated the missing field length to the global field settings in D6. Could you test if you are able to do that? In other words

  • If the problem is that D6 single-line textfields are migrated to D8 as long text because the D6 global field length is not defined
  • Are you able to eliminate the issue by defining the global field length in D6 before you migrate?

Cheers,
Markus

jeffwpetersen’s picture

jeffwpetersen commented

My error. I did not realize there was a global field.

After updating the global field with a value the migration worked as expected.

jeffwpetersen’s picture

jeffwpetersen commented
Status: Needs review » Reviewed & tested by the community
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

Why is this issue split off from #2842222: D7 Plain text fields incorrectly migrated to D8 as Text (formatted)? This seems like a discussion we should patch and fix all and one place because there are broad implications.

masipila’s picture

masipila commented

@xjm: This is split from the parent issue so that the parent issue can focus on the D7 to D8 text field migration which is conceptually difficult due to the fact that D7 text formatting settings are defined on field instance settings. In D6 the text formatting settings are managed on global field settings, in other words on field storage settings.

This D6 textarea issue is straight forward and 1:1 with D8 and there should be nothing conceptually difficult here:

  • If the text formatting settings of D6 textarea have been set to Plain text, it should be migrated to D8 as Text (plain, long).
  • If the text formatting settings of D6 textarea have been set to Filtered text, it should be migrated to D8 as Text (formatted, long).
  • Currently both cases are migrated to D8 as Text (formatted, long), and that is wrong.
  • The patch fixes this bug and updates the unit tests so that these scenarios are covered with the tests.

Hope this helps,
Markus

jeffwpetersen’s picture

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

This has been tested and works as designed.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2849861-d6-textarea-format-2.patch, failed testing.

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Reviewed & tested by the community

Testbot error, setting back to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2849861-d6-textarea-format-2.patch, failed testing.

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Reviewed & tested by the community

At what point do we start worrying about these testbot failures? Setting back to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2849861-d6-textarea-format-2.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review
StatusFileSize
new 2849861-d6-textarea-format-2-reroll.patch2.89 KB

Here's a re-roll. Don't credit me, trying out a script I wrote to re-roll due to #2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Self-assigning for review.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Reviewed & tested by the community

There's tests. This fixes the problem. Seems strait forward enough (I don't see any need of additional code comments). RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 17: 2849861-d6-textarea-format-2-reroll.patch, failed testing.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs work » Reviewed & tested by the community

Random testbot failure.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 17: 2849861-d6-textarea-format-2-reroll.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Reviewed & tested by the community

  • Gábor Hojtsy committed 98bde1c on 8.3.x 
    Issue #2849861 by masipila, joelpittet, jeffwpetersen, xjm: D6 text area...

  • Gábor Hojtsy committed 4b8961e on 8.4.x 
    Issue #2849861 by masipila, joelpittet, jeffwpetersen, xjm: D6 text area...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +Baltimore2017

Thanks all, especially the detailed response to @xjm. Comitted!

Status: Fixed » Closed (fixed)

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