datetime_type is not set correctly when migrating datetime fields from D7

Assigning to vicky to review this week.

@davidsickmiller, thanks for patch!

Can we add the D7 field type 'Date (ISO format)' field and thus cover all the date fields?

After running a migration I found that for field_date_with_end_time only the start date is migrated, not the end date. Is this a case for the daterange type?

1. +++ b/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
   @@ -118,6 +119,12 @@ public function testFields() {
   +    $this->assertIdentical('datetime', $field->getSetting('datetime_type'));
   ...
   +    $this->assertIdentical('date', $field->getSetting('datetime_type'));
   

   assertIdentical is deprecated. Should now be assertSame

2. +++ b/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
   @@ -118,6 +119,12 @@ public function testFields() {
   +    $field = FieldStorageConfig::load('node.field_date');
   ...
   +    $field = FieldStorageConfig::load('node.field_date_without_time');
   

   Why is field_date_with_end_time not tested here

Known test bot failure: #2828143: Stop tests like LocaleConfigTranslationImportTest from failing if l.d.o becomes unavailable. Back to NR.

All looks good to me (ignoring the testbot failure) and ready for RTBC. Have run re-tests for 8.4.x and a test against 8.5.x, but not expecting any surprises in the results.

Uploading a test-only version of the patch in #11 (for completeness as much as anything).

If all the tests come back as expected then this can be set to RTBC.

The d7_field_settings plugin was not catching fields of type "datestamp" (e.g. field_date_with_end_time) and updating the datetime_type setting when appropriate so I have corrected that.

Even so, the datetime_type setting for field_date_with_end_time should not be set because the hour/minute granularities are not 0 so I have also updated the expected to Null in the test.

So we're not quite ready for RTBC, but I think this should take us a step closer.

Back to NW for the remaining tasks. What are those?

The only remaining task was to understand why field_date_with_end_time did not have a datetime_type setting. This is because it is a datestamp field which becomes a timestamp field (which does not have a datetime_type setting).

Re-rolled the patch (because the patch in #17 no longer applies) and added a comment to explain why we are asserting that the datetime_type setting for field_date_with_end_time is null (I thought this was better than simply ignoring it).

Great test coverage, comments in #19 makes sense. Reviewed the IS again, do we actually need to collect any settings about 'year, month, day, hour, minute, and/or second' or just collect the type? Back to NW for that, either update the IS to reflect what is going on in this issue or update the patch. And if we do need to collect the settings, should that move to a follow-up? Or take place here? If it is simple, let's do it here. If it isn't, punt to a follow-up.

The only datetime granularity options in D8 are datetime_type of "datetime" (fields including date & time) and "date" (fields including only date & no time) - there is not the same granularity as D7. I have edited to IS to reflect this.

Ah yes, all that is stored on the field formatter now and is in contrib as core doesn't support more fine grain settings. I should have remembered. Great. This is good to go then.

Rerolled the patch #19 & because it's failed to apply on 8.5.x branch.

This is a bug, so moving it back to 8.4. And added a test run as well for good measure.

This patch only increases the number of field_storage_config by 2 so correcting that.

This patch only increases the number of field_config by 2 so correcting that. We'll get there in the end...

Ooops, empty patch file! This is what I meant to upload.

Straight re-roll here, so back to RTBC.

Crediting @heddn and @quietone as reviewers.

Committed and pushed a151cef151 to 8.5.x and d975b8df0f to 8.4.x. Thanks!

This seems to have broken HEAD, so I've reverted it:
https://www.drupal.org/pift-ci-job/784751
https://www.drupal.org/pift-ci-job/784827
https://www.drupal.org/pift-ci-job/784828
Etc.

Here's a re-roll of #33. Let's see what errors are getting thrown.

This is a db-other-mysql issue. If I run Drupal\Tests\block\Kernel\Migrate\d7\MigrateBlockTest locally using postgres the test fails...

So dates in D7 had some fun stuff for different DBs :) See date.install in D7 - http://cgit.drupalcode.org/date/tree/date.install

Hi, I'm not sure if there is a bug on this or if it is a migrate_plus module issue.

Trying to migrate from Drupal 7.56 to Drupal 8.4.0

On the Drupal 8 site I have installed: migrate, migrate_plus, migrate_tools, migrate_upgrade.

After applying #44 patch, I ran:

drush migrate-upgrade --legacy-db-url=mysql://user:password@server/db --legacy-root=http://example.com --configure-only

drush mi --all

Content was migrated and date field is migrated as only date (the way I want it), but I can't see any date when editing a migrated node. I mean, the field is there but seems to be empty.

So I rollback the migration and ran drush cex to check the migration configuration. I have a content type "Norma" (Norma translates as "norm", for law content creation) and this content type has a date field "fecha" which is the spanish word for date.

So, in migrate_plus.migration.upgrade_d7_node_norma.yml file I can see:

field_fecha:
    plugin: iterator
    source: field_fecha
    process:
      value:
        plugin: format_date
        from_format: 'Y-m-d\TH:i:s'
        to_format: 'Y-m-d\TH:i:s'
        source: value

So I changed to_format: 'Y-m-d\TH:i:s' to be like to_format: 'Y-m-d' and then drush cim and ran again the upgrade, now the dates are correctly shown when editing a migrated node but (and this is the weird part) I get this error (when editing or creating a node of type "norma"): Notice: Undefined index: main-menu in menu_ui_form_node_form_alter() (line 267 of core/modules/menu_ui/menu_ui.module).

So, I went and edit the content type "norma" to asign a menu to see if it fixes and it does... except that the date field "fecha" has now disapeared from the content type, I don't see it listed even in /admin/reports/fields

Not sure if I'm doing it wrong or where to look at.

Regards.

Hi, I can confirm that Notice: Undefined index: main-menu in menu_ui_form_node_form_alter() (line 267 of core/modules/menu_ui/menu_ui.module) is not related to this issue (sorry for the noise) but is related to https://www.drupal.org/node/2592829 because even if I don't apply #44 I get the error. This happens because I have selected main-menu as an available menu for the content type that I'm migrating (on D7) and in D8 that menu is now 'main', so 'main-menu' not exists from the D8 perspective. Updating the other issue.

Anyway, It's still the need to change the to_format: 'Y-m-d\TH:i:s' to to_format: 'Y-m-d' in the configuration. I made it as I described before: exporting the configuration drush cex, changing the yaml files and then importing the configuration changes drush cim but people can do that on a custom module too. Will be great if the to_format is generated correctly from the beginning.

Regards!

Assigning to myself to review in the next week.

This looks good now. It was already RTBC, just failing on postgress. That is now accommodated in the re-rolled patch. It isn't clear if #45 was with or without this patch. But I can only imagine it was without. Because if it were with the patch, then it would correctly map the fields.

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

@Yaazkal: I posted a patch to fix what you described at #45 at #2822801: Date field empty after migration d7 d8.