Problem/Motivation

In Drupal 6 you could have a text field and then select the single on/off checkbox widget. The data was stored in the database as text. In Drupal 8 a boolean field can only be stored as an integer, on/off.

Proposed resolution

We need to process text - single on/off checkboxes values so that we store only 0 or 1. We need to store the "On" value as the text label for that field.

Note

There is some slight data loss here because we're losing the keys that were specified in D6. If you didn't manually specify keys then we're actually losing the values. We store the On value in the label so we lose your "Off" value which wasn't used in the UI but could have been used from code.

Remaining tasks

Write patch and tests.

User interface changes

n/a

API changes

n/a

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ishanmahajan’s picture

Issue tags: +#dcdelhi
svendecabooter’s picture

Assigned: Unassigned » svendecabooter
svendecabooter’s picture

First iteration of a patch that fixes this issue is attached.
Still needs tests written.

svendecabooter’s picture

Assigned: svendecabooter » Unassigned
Status: Active » Needs review
Issue tags: -#dcdelhi
FileSize
468.87 KB

Patch attached including tests.

What it does:

* Add a special case for migrating text field with widget optionwidgets_onoff, so it converts the D6 off/on data to a boolean field in D8.
* Updated the D6 migration database so it uses textual allowed values (for testing purposes).
* Added a test case that verifies the textual allowed values are being converted to a boolean value upon migration.

Let me know if I'm missing something.
Also: not sure if the d6.gz database diff should be included in the patch. If not, I can reattach the patch without it.

ultimike’s picture

Sven,

Regarding the d6.gz diff, as of this moment, yes, it should be included, but #2469623: Process for creating migration source DBs for automated tests will change that.

I have a couple of similar patches in the same situation - I'm hopeful that #2469623: Process for creating migration source DBs for automated tests will be resolved soon enough so I'm waiting to re-roll.

For now, will you attached a patch (call it "2405017-migrate-boolean-fields-type-text-x-do-not-test.patch") without the d6.gz for easier review?

thanks,
-mike

svendecabooter’s picture

Thanks for the clarification Mike.
Attached a patch without the d6.gz, as requested.

benjy’s picture

Thanks for working on this. In general the code looks good but I'd like to have a closer look once it's ready. For now, a few small comment/coding standard nitpicks.

  1. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/CckFieldValues.php
    @@ -241,6 +259,15 @@ protected function getSourceFieldInfo($bundle) {
    +          // Special processing for "text" field with "optionwidgets_onoff" widget type
    

    Comment needs to be 80chars and end in a full stop.

  2. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/CckFieldValues.php
    @@ -315,4 +342,24 @@ protected function cckSchemaCorrect() {
    +  /**
    +   * Parse the allowed values from the optionwidget_onoff settings
    +   */
    

    Need some proper function docs here.

  3. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/CckFieldValues.php
    @@ -315,4 +342,24 @@ protected function cckSchemaCorrect() {
    +  protected function cckOptionWidgetOnOffValues($values) {
    

    Would be good to explain a little bit about what this function does, what values it's expecting/returning etc.

  4. +++ b/core/modules/migrate_drupal/src/Tests/d6/MigrateCckFieldValuesTest.php
    @@ -180,6 +191,9 @@ public function testCckFields() {
    +    // Test that text field with optionwidgets_onoff widget gets converted to boolean value
    

    Comment too long.

anavarre’s picture

Status: Needs review » Needs work

Per #7

neclimdul’s picture

Sorry I didn't see this but #2588827: CCK migrations seem to have trouble with some checkbox types probably duplicates it and is about wrapped up.

benjy’s picture

Status: Needs work » Closed (duplicate)
Related issues: +#2405017: Cannot migrate boolean fields of type text

Yeah lets close this as a duplicate, i'll comment over there asking for the people here to be credited on that issue.

benjy’s picture