Problem/Motivation

Nodes that have a format of 0 which then gets mapped to NULL by the static map in the d6_node.yml file do not get imported because the filter is stubbed and we can't make NULL into a machine name so the Migration plugin throws a SkipRowException.

Proposed resolution

...

Remaining tasks

Figure out what to do.

User interface changes

n/a

API changes

n/a

CommentFileSizeAuthor
#11 interdiff.txt792 bytesultimike
#11 2363643-11.patch3.17 KBultimike
#8 interdiff.txt789 bytesultimike
#8 2363643-8.patch3.17 KBultimike
#6 interdiff.txt2.62 KBultimike
#6 2363643-6.patch3.17 KBultimike
#3 2363643-3.patch1.32 KBultimike
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented

What's the problem with setting the text format to "Basic HTML" (or maybe "Restricted HTML") in D8 when it is 0 in D6? Seems like this is an edge case, so why not make a reasonable guess and make a note of it in the docs?

-mike

benjy’s picture

benjy CreditAttribution: benjy commented

Lets figure out what 0 means in D6, eg which filter gets applied and then like you say, map it to a more suitable filter that isn't "NULL".

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
Status: Active » Needs review
FileSize
2363643-3.patch1.32 KB

Ok, so I figured out where a format = 0 can come from.

When a D6 content type has no body field, the body field is still there, just hidden. So, when nodes of this type are saved, the body field is empty, and the format is 0 (see https://api.drupal.org/api/drupal/modules%21node%21node.module/function/...).

So, I'm thinking that it is a safe assumption that any body fields with format = 0 are probably null, so why not just set it to the D6 default text format ("Filtered HTML") which will be migrated normally.

I've attached a patch to do exactly this.

Thoughts?

-mike

benjy’s picture

benjy CreditAttribution: benjy commented

Yeah I like this approach, so simple. +1 for RTBC from me.

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
Assigned: Unassigned » ultimike
Status: Needs review » Needs work

chx's idea (via our weekly meeting) is to map filter=0 to the filter_default_format value.

  1. Get the filter_default_format variable in runQuery().
  2. Add that variable to every row in prepareRow().
  3. In the source class, override the the format when 0 in prepareRow() (better) **or** we can see if we can do this with a process plugin.

-mike

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
Status: Needs work » Needs review
FileSize
2363643-6.patch3.17 KB
interdiff.txt2.62 KB

New patch based on suggestions in comment 5.

Thanks,
-mike

benjy’s picture

benjy CreditAttribution: benjy commented 
+++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/Node.php
@@ -91,6 +107,18 @@ public function fields() {
+    if ($row->getSourceProperty('format') == 0) {

Lets use strict comparison here unless we also want to map an empty format to the default as well?

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
FileSize
2363643-8.patch3.17 KB
interdiff.txt789 bytes

Benjy,

Roger that. Updated patch attached.

-mike

Status: Needs review » Needs work

The last submitted patch, 8: 2363643-8.patch, failed testing.

benjy’s picture

benjy CreditAttribution: benjy commented

Try this: 

 if ($row->getSourceProperty('format') === '0') {
ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
Status: Needs work » Needs review
FileSize
2363643-11.patch3.17 KB
interdiff.txt792 bytes

Wow - that's annoying. Updated patch attached.

Thanks,
-mike

benjy’s picture

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

Great! Not sure if this is worth a mention in the handbook?

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented

Are you thinking maybe on the Known issues page?

-mike

benjy’s picture

benjy CreditAttribution: benjy commented

Yeah, maybe just a one liner I guess? Can't do any harm :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?. Committed 392522c and pushed to 8.0.x. Thanks!

  • alexpott committed 392522c on 8.0.x 
    Issue #2363643 by ultimike | benjy: Fixed Nodes with format 0 are...
ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented

Note added to the Known issues page.

-mike

Status: Fixed » Closed (fixed)

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