When a D7 site has a long text field that uses the "text_plain" format, migration errors occur.

The "text_plain" plugin does not exist

The same problem was fixed for normal textfields in #3042223: Map text_plain field formatter to string.

Proposal: implement the same fix for long text fields.

CommentFileSizeAuthor
#5 3088917-5.patch1.8 KBquietone
#5 3088917-5-fail.patch1.18 KBquietone
#2 drupal-migrate-plain-text-long-3088917-2.patch732 bytesgbirch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gbirch created an issue. See original summary.

gbirch’s picture

gbirch CreditAttribution: gbirch commented
FileSize
drupal-migrate-plain-text-long-3088917-2.patch732 bytes

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Active » Needs review
Issue tags: +Bug Smash Initiative
FileSize
3088917-5-fail.patch1.18 KB
3088917-5.patch1.8 KB

Patch needed a reroll. And a test has been added. No interdiff because this is so small.

The last submitted patch, 5: 3088917-5-fail.patch, failed testing. View results

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

marvil07’s picture

marvil07 CreditAttribution: marvil07 at Adapt commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Related issues: +#3042223: Map text_plain field formatter to string

As @gbirch mentions, this change is pretty similar to #3042223: Map text_plain field formatter to string.
The changes on #5 look good, and the extra test case added builds on top of that work.
Marking as RTBC.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers as a volunteer commented 
+++ b/core/modules/text/tests/src/Unit/Plugin/migrate/field/d7/TextFieldTest.php
@@ -39,7 +40,7 @@ public function getFieldFormatterTypeProvider() {
    * @covers ::getFieldType
    * @dataProvider getFieldFormatterTypeProvider
    */
-  public function testGetFieldType($type, $formatter_type, $expected) {
+  public function testGetFieldFormatterType($type, $formatter_type, $expected) {

Shouldn't this also update the @covers?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs review
quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented

The doc block include two @covers statements and while there isn't an assertion on getFieldType it is called in getFieldFormatterType(). Should it have two @covers or one? For background, the test was added in #3042223: Map text_plain field formatter to string

   * @covers ::getFieldFormatterType
   * @covers ::getFieldType
   * @dataProvider getFieldFormatterTypeProvider
   */
  public function testGetFieldFormatterType($type, $formatter_type, $expected) {
marvil07’s picture

marvil07 CreditAttribution: marvil07 at Adapt commented
Status: Needs review » Reviewed & tested by the community

Shouldn't this also update the @covers?

The doc block include two @covers statements and while there isn't an assertion on getFieldType it is called in getFieldFormatterType().

Strictly speaking, after the change, testGetFieldFormatterType() would be covering getFieldFormatterType(), that calls getFieldType(), as @quietone mentions, which as mentioned too, currently has two @covers in 9.3.x.

Hence, I would say it is OK as it is now on the suggested change from #5.
Marking back as RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 775223c013 to 9.3.x and ed5f0fba70 to 9.2.x. Thanks!

  • alexpott committed 775223c on 9.3.x 
    Issue #3088917 by quietone, gbirch, marvil07, Wim Leers: Map text_plain...

  • alexpott committed ed5f0fb on 9.2.x 
    Issue #3088917 by quietone, gbirch, marvil07, Wim Leers: Map text_plain...

Status: Fixed » Closed (fixed)

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