Problem/Motivation

During manual testing of the D6->D8 migration, I noticed that data from a D6 field of type "date" is not successfully migrated into D8. The field is created properly, but the data is not.

Possibly related to: https://drupal.org/node/2167633

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#16 interdiff.txt1.46 KBbenjy
#16 2260241-15.patch2.39 KBbenjy
#14 2260241-14.patch2.44 KBer.pushpinderrana
#8 2260241-8.patch2.42 KBultimike
#8 interdiff.txt1.73 KBultimike
#5 2260241-5.patch704 bytesultimike
#2 profile_date_field-2260241-2.patch736 bytesultimike
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
Issue summary: View changes
ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
Status: Active » Needs review
FileSize
profile_date_field-2260241-2.patch736 bytes

I made some progress on this one today.

I've attached a patch with the fix for this, I added a static_map process plugin to the d6_user_profile_field plugin that sets the datetime_type to "date" (as opposed to the default "datetime") when migrating a D6 profile "Date" field. The only thing about this patch that bothers me a little is the fact all migrated profile fields are going to have a "datetime_type" settings (all will be set to '' except for migrated "Date" fields). Is this cool?

I am unable to determine why the existing MigrateProfilesTest passed prior to this fix, and am unable to modify the test to produce a failing test...

-mike

chx’s picture

chx CreditAttribution: chx commented

Well, if we can't fail a test then that's problematic. Also, if this is necessary we should a custom plugin so that only the relevant type gets the setting.

ultimike’s picture

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

I think the issue with the non-failing test is that the value is migrated just fine, but since the D8 form widget is expecting a date and a time, and we're only migrating a date, the D8 display widget doesn't display a value (see this screenshot from a manual test I just ran: http://note.io/UYlf7b).

The MigrateProfileValuesTest test passes because the migrated value in the DB is exactly what is should be.

I'm thinking that I need to modify the MigrateUserProfileFieldTest to ensure we have the correct form widget type. I'll work on that next.

As for breaking the datetime_type into a separate plugin, I'll look at that after I get a failing test working.

Thanks,
-mike

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
FileSize
2260241-5.patch704 bytes

Well, creating a failing MigrateUserProfileFieldTest was easy. Patch attached.

Custom plugin coming later.

Thanks,
-mike

ultimike’s picture

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

@chx,

Before I start writing code, please let me know if this is what you're thinking...

I'm going to write a new d6 process plugin called Drupal\migrate_drupal\Plugin\migrate\Process\d6\ProfileFieldSettings

The configuration will look something like:

settings: 
  plugin: d6_profile_field_settings
  source: type

The transform() method will then just have a switch() statement that will set settings.datetype_type to "date" when type = datetime.

Am I missing anything?

Thanks,
-mike

chx’s picture

chx CreditAttribution: chx commented

Sounds right to me.

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
Status: Needs work » Needs review
Parent issue: » #2224001: Manual testing (D6->D8): User and User Profile
FileSize
interdiff.txt1.73 KB
2260241-8.patch2.42 KB

@chx - thanks for the help.

I've attached a patch that hopefully fixes this issue. The new test passes (consider the patch in 5 the "FAIL" patch) and a manual test looks good as well.8

Thanks,
-mike

benjy’s picture

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

I tested this patch manually and it fixes the issue. With the patch applied I successfully migrated date fields from D6 and the widget on the D8 user/edit page was simply date and not datetime.

I also commented out the fix and ran the tests and can confirm that the MigrateUserProfileFieldTest now catches this issue.

Status: Reviewed & tested by the community » Needs work

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

Status: Needs work » Needs review

benjy queued 8: 2260241-8.patch for re-testing.

benjy’s picture

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

Back to RTBC, looks like some network issues caused the failures: https://www.drupal.org/node/2298415

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll 
git ac https://www.drupal.org/files/issues/2260241-8.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2474  100  2474    0     0   2594      0 --:--:-- --:--:-- --:--:--  3092
error: patch failed: core/modules/migrate_drupal/config/install/migrate.migration.d6_user_profile_field.yml:18
error: core/modules/migrate_drupal/config/install/migrate.migration.d6_user_profile_field.yml: patch does not apply
er.pushpinderrana’s picture

er.pushpinderrana CreditAttribution: er.pushpinderrana commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2260241-14.patch2.44 KB

Rerolled patch.

Status: Needs review » Needs work

The last submitted patch, 14: 2260241-14.patch, failed testing.

benjy’s picture

benjy CreditAttribution: benjy commented
Status: Needs work » Needs review
FileSize
2260241-15.patch2.39 KB
interdiff.txt1.46 KB

Fixed the indent error from #14 and removed the message on the assertion which made this easier to debug.

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike commented
Status: Needs review » Reviewed & tested by the community

The re-roll looks fine to me, all minor changes that don't affect the overall patch logic.

Thanks,
-mike

alexpott’s picture

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

Committed 35458b5 and pushed to 8.x. Thanks!

  • alexpott committed 35458b5 on 8.x 
    Issue #2260241 by ultimike, benjy, er.pushpinderrana: Fixed D6->D8...

Status: Fixed » Closed (fixed)

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