Follow-up from #1653026: [META] Use properly typed values in module configuration.

EDIT: (see comment #13) Postoned on #2130811: Use config schema in saving and validating configuration form to ensure data is consistent and correct .
Then this issue should deal with all field config shipped in core and also ensure
that the schemas are correct too.

Problem/Motivation

All integers, Booleans, and even octal numbers in config object files are converted to strings.

Proposed resolution

#1653026: [META] Use properly typed values in module configuration has fixed core, so no need to convert all data types to string anymore.

Remaining tasks

Fix and issue patch for below config files:
field.settings.yml
field.field.entity_test.field_test_import.yml
field.field.entity_test.field_test_import_2.yml
field.instance.entity_test.entity_test.field_test_import.yml
field.instance.entity_test.entity_test.field_test_import_2.yml
field.instance.entity_test.test_bundle.field_test_import_2.yml
field.field.entity_test.field_test_import_staging.yml
field.field.entity_test.field_test_import_staging_2.yml
field.instance.entity_test.entity_test.field_test_import_staging.yml
field.instance.entity_test.test_bundle.field_test_import_staging_2.yml
field.instance.entity_test.test_bundle_2.field_test_import_staging_2.yml
views.view.test_view_fieldapi.yml

User interface changes

NO

API changes

NO

Parent: #1653026: [META] Use properly typed values in module configuration

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sidharthap’s picture

Status: Active » Needs review
FileSize
9.26 KB

initial patch file.

yched’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/field/tests/modules/field_test_config/config/field.field.entity_test.field_test_import.yml
    @@ -4,11 +4,11 @@ name: field_test_import
    -cardinality: '1'
    ...
    +cardinality: true
    

    should stay as 1 (the number)
    (same in other field.field.* files)

  2. +++ b/core/modules/field/tests/modules/field_test_config/config/field.instance.entity_test.entity_test.field_test_import_2.yml
    @@ -5,15 +5,15 @@ entity_type: entity_test
     widget:
    -  weight: '-2'
    +  weight: -2
       type: text_textfield
       module: text
       settings:
    -    size: '60'
    +    size: 60
    

    What ?? We still have a 'widget' section in there ? It should be removed altogether, it's not relevant anymore.
    (same in other field.instance.* files)

sidharthap’s picture

Status: Needs work » Needs review
FileSize
11.7 KB

Thank you @yched.
Here is the corrected patch file.

yched’s picture

--- a/core/modules/content_translation/tests/modules/content_translation_test_views/test_views/views.view.test_entity_translations_link.yml
+++ b/core/modules/content_translation/tests/modules/content_translation_test_views/test_views/views.view.test_entity_translations_link.yml

Looks like this doesn't belong here ? :-)

sidharthap’s picture

FileSize
9.14 KB

oops!
The corrected patch.

yched’s picture

Status: Needs review » Reviewed & tested by the community

Looks good. Thanks !

yched’s picture

Issue tags: +Field API, +Entity Field API
alexpott’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

So doing this only makes sense if we save the files in the same way the API does... I've just created a text field through the ui

id: node.field_textfield
uuid: 38a930ad-7840-4c9c-8800-ee81e488705c
status: true
langcode: en
name: field_textfield
entity_type: node
type: text
settings:
  max_length: '240'
module: text
active: true
locked: false
cardinality: '1'
translatable: false
indexes: {  }

field.field.node.field_textfield.yml looks like this

id: node.article.field_textfield
uuid: e3abb129-2a07-4e6e-a55b-915c963c04ab
status: true
langcode: en
field_uuid: 38a930ad-7840-4c9c-8800-ee81e488705c
entity_type: node
bundle: article
label: textfield
description: ''
required: 0
default_value: {  }
default_value_function: ''
settings:
  text_processing: '0'
field_type: text

field.instance.node.article.field_textfield.yml looks like this

It looks like we're going to have to do some casting in the form submit handlers in the UI

alexpott’s picture

Also in text.schema.yml for example:

field.text.instance_settings:
  type: mapping
  label: 'Text settings'
  mapping:
    text_processing:
      type: string
      label: 'Text processing'
...
field.text_long.instance_settings:
  type: mapping
  label: 'Long text settings'
  mapping:
    text_processing:
      type: string
      label: 'Text processing'
...
field.text_with_summary.instance_settings:
  type: mapping
  label: 'Text area with a summary'
  mapping:
    text_processing:
      type: boolean
      label: 'Text processing'
    display_summary:
      type: boolean
      label: 'Summary input'

Afaics text_processing is a boolean

yched’s picture

@alexpott: I'm not sure what is the intended process behind the this issue and all the similar subtasks issued from #1653026: [META] Use properly typed values in module configuration.

I assumed (but maybe I'm wrong ?) that this issue was only about fixing the config files that are present in core, not about fixing the APIs and/or UIs so that they actually produce CMI files that comply with those typecasts (which would be a much more involved change).

This being said, I'm indeed not sure when is the right time to update the config schemas accordingly, then...

alexpott’s picture

@yched but I don't want to have to this task yet again after we've done #2130811: Use config schema in saving and validating configuration form to ensure data is consistent and correct - this change should negate the need to fix APIs and UIs as this will just get everything right :)

yched’s picture

@alexpott: OK, I'm confused then :-)
What do you think should be done in this issue (if anything) ?

alexpott’s picture

@yched I think all the "...has no type-casting to string." should be postponed on #2130811: Use config schema in saving and validating configuration form to ensure data is consistent and correct and then this issue should deal with all field config in core and ensure that the schemas are correct too.

yched’s picture

Issue summary: View changes
Status: Needs work » Postponed

OK, makes sense.

vijaycs85’s picture

Status: Postponed » Closed (duplicate)
Related issues: +#2167623: Add test for all default configuration to ensure schema exists and is correct

The patch on this issue has been updated as part of #2167623: Add test for all default configuration to ensure schema exists and is correct. As this issue doesn't have any test to confirm/validate the schema, making this change and closing this issue as duplicate of #2167623: Add test for all default configuration to ensure schema exists and is correct. The contributors of this issue (in commit message) is copied to #2167623: Add test for all default configuration to ensure schema exists and is correct.