Problem/Motivation

As per #2183983: Find hidden configuration schema issues field.value.datetime and field.value.link are recurring problems in migrate tests. Here is a quick test to prove this with one of them where we should fix it.

Proposed resolution

Fix them :)

Remaining tasks

Fix. Review. Commit.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#8 2386325-field-bugs-8.patch2.27 KBgábor hojtsy
#8 interdiff.txt1.69 KBgábor hojtsy
#3 2386325-field-bugs-3.patch1.1 KBgábor hojtsy
#3 interdiff.txt444 bytesgábor hojtsy
#1 2386325-field-bugs.patch682 bytesgábor hojtsy

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
StatusFileSize
new 2386325-field-bugs.patch682 bytes

Status: Needs review » Needs work

The last submitted patch, 1: 2386325-field-bugs.patch, failed testing.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt444 bytes
new 2386325-field-bugs-3.patch1.1 KB

The link one was easy, that is a misnamed item. The datetime item I could not yet figure out...

Status: Needs review » Needs work

The last submitted patch, 3: 2386325-field-bugs-3.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

For when you reroll this one to fix the failing tests:

+++ b/core/modules/migrate_drupal/src/Tests/d6/MigrateFieldInstanceTest.php
@@ -20,6 +20,15 @@
+  ¶

Stupid whitespace problem :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Priority: Normal » Critical
Issue tags: +D8 upgrade path

Critical and D8 upgrade path based on #2183983: Find hidden configuration schema issues.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Priority: Critical » Normal

After further discussion with @xjm and @effulgentsia, keeping at normal, but keeping the tag.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.69 KB
new 2386325-field-bugs-8.patch2.27 KB

So the detetime bug is very interesting... For one, the datetime defaults are defined as such:

field.value.datetime:
  type: mapping
  label: 'Default value'
  mapping:
    default_date_type:
      type: string
      label: 'Default date type'
    default_date:
      type: string
      label: 'Default date value'

This structure is extensively tested in \Drupal\datetime\Tests\DateTimeFieldTest. However, migrations attempt to create a simple 'value' key instead of these keys with the date values. That will never pass schema validation (or work as a date default for that matter).

Here is the fix.

effulgentsia’s picture

effulgentsia commented
Status: Needs review » Reviewed & tested by the community
Parent issue: » #2183983: Find hidden configuration schema issues

Looks good.

After further discussion with @xjm and @effulgentsia, keeping at normal

To clarify, what we thought made sense was that if #2183983: Find hidden configuration schema issues uncovers a bug that requires changes to the actual structure of config (meaning something in active config or code that writes to config would need to be updated), such as #2385805: Views tests don't pass strict schema checking, then that should be a critical, since we don't want to release Drupal with a known bug in its data structure. However, this issue merely uncovers changes needed to a schema file and a test, which can happen after release, so not critical.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
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 https://www.drupal.org/core/beta-changes. Committed a029bf5 and pushed to 8.0.x. Thanks!

  • alexpott committed a029bf5 on 8.0.x 
    Issue #2386325 by Gábor Hojtsy: Recurring config schema problems with...

Status: Fixed » Closed (fixed)

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