Problem/Motivation

FieldConfigStorageBase::mapToStorageRecord() calls $class::fieldSettingsFromConfigData() instead of $class::fieldSettingsToConfigData() :/

Proposed resolution

Fix it.

Remaining tasks

Write some tests.

User interface changes

Nope.

API changes

Nope.

CommentFileSizeAuthor
#3 2437785-3.patch6.88 KBamateescu
#3 2437785-3-test-only.patch6.16 KBamateescu
#1 2437785.patch744 bytesamateescu

Comments

amateescu’s picture

amateescu commented
Status: Active » Needs review
Issue tags: +Needs tests
StatusFileSize
new 2437785.patch744 bytes

This is the fix.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

How is this not exploding for list field types?

amateescu’s picture

amateescu commented
Issue tags: -Needs tests
StatusFileSize
new 2437785-3-test-only.patch6.16 KB
new 2437785-3.patch6.88 KB

How is this not exploding for list field types?

Because ListItemBase implements only storageSettingsToConfigData() and storageSettingsFromConfigData() and this bug is for fieldSettingsToConfigData()/fieldSettingsFromConfigData() :)

Wrote some tests for all of them.

The last submitted patch, 3: 2437785-3-test-only.patch, failed testing.

yched’s picture

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

Yay - we already had another issue to fix that wrong call, but it was still "needs tests". Will mark it as duplicate if I can dig it back :-)

amateescu’s picture

amateescu commented

This is the issue: #2343483: Doc cleanup after FieldConfig

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 74f9435 and pushed to 8.0.x. Thanks!

  • alexpott committed 74f9435 on 8.0.x 
    Issue #2437785 by amateescu: FieldConfigStorageBase::mapToStorageRecord...
yched’s picture

yched commented

@amateescu : thanks :-) Reuploaded the patch over there, it's now just a minor doc fix.

Status: Fixed » Closed (fixed)

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