Problem/Motivation

The #2212069: Non-English Drupal sites get default configuration in English, edited in English, originals not actually used if translated patch introduced a way to add langcode automatically to top level configuration names. @alexpott expressed concern that that adds an implicit assumption about configuration schema that is not explicitly in the type, and we should introduce explicit configuration language codes instead.

Proposed resolution

1. Add a config_object base type. This means that for a configuration name, developers would either define it using config_object or config_entity. This makes it REALLY easy to spot definitions for high level config in schemas.
2. Use the config_object base type for top level configuration (which are not config entities). Remove explicit langcodes from the ones now extending config_object.
3. Fix language.mappings which used a top level sequence type (and therefore incompatible with adding a top level langcode).
4. Remove breakpoint schemas which are not even for configuration, they are for plugin data (not stored within configuration).
5. Fix tests.

Remaining tasks

Review. Commit.

User interface changes

None.

API changes

Configuration objects should now extend from config_object.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because config files cannot be parsed by their schema alone, unless the langcode key is explicitly defined on them.
Issue priority Major because the extent of changes required.
Disruption Disruption in how configuration schemas need to be defined.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Issue summary: View changes
Status: Active » Postponed
Gábor Hojtsy’s picture

Status: Postponed » Active
Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
3.92 KB

Not sure about the best way to figure out the top level types. This will definitely fail hard and show us some.

Gábor Hojtsy’s picture

Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 3: 2459971.patch, failed testing.

Gábor Hojtsy’s picture

Priority: Normal » Major
Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
13.35 KB
9.43 KB

With surprisingly little changes, we can eliminate all explicit langcode: keys except the newly added config_object type and one in migrate which does not seem to be a top level type. Using config_object where langcode was present before and in top level definitions of system and user. Will likely go on to produce more fails from other areas.

Status: Needs review » Needs work

The last submitted patch, 7: 2459971-7.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
18.64 KB
5.29 KB

Status: Needs review » Needs work

The last submitted patch, 9: 2459971-9.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
21.51 KB
2.87 KB

Fix ConfigImportUITest and ConfigSchemaTest by updating the expectations (label and key order).

Fix the missing config object base type for menu overrides.

Gábor Hojtsy’s picture

FileSize
22.15 KB
658 bytes

ConfigImporterTest needed the same fix of item ordering, so resaving based on the schema will not cause changes.

The last submitted patch, 11: 2459971-11.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 12: 2459971-12.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
1.37 KB
23.26 KB

ConfigImportUITest still needed some keys reordered.
The field.settings is a config object, fixed schema.

Status: Needs review » Needs work

The last submitted patch, 15: 2459971-15.patch, failed testing.

Status: Needs work » Needs review

Gábor Hojtsy queued 15: 2459971-15.patch for re-testing.

Gábor Hojtsy’s picture

Title: The top level langcode configuration key is not explicit in configuration schema » The langcode key on configuration files is not explicit in configuration schema
Gábor Hojtsy’s picture

FileSize
20.48 KB
4.75 KB

In the CMI meeting review, @alexpott asked to not move the position of the langcode of the config entities, so the change is not that dramatic. We can keep the override in config_entity for that in the place we expect it.

The last submitted patch, 15: 2459971-15.patch, failed testing.

Gábor Hojtsy’s picture

FileSize
20.9 KB
437 bytes

Continue with the file fixes in file.schema.

The last submitted patch, 19: 2459971-19.patch, failed testing.

Gábor Hojtsy’s picture

FileSize
435 bytes
20.75 KB

So from the results in #19, reordering on merge would not take it, so we need to define config_entity as a simple mapping with the langcode explicitly if we want to keep the ordering.

The last submitted patch, 21: 2459971-21.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 23: 2459971-23.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
474 bytes
21.21 KB

Next up is locale settings.

Gábor Hojtsy’s picture

FileSize
38.72 KB
18 KB

Went through all the schema files and compared entries to config objects. Found a whole bunch more.

Also found breakpoint schemas defined. But the breakpoint YAML files are not in config, they are in the theme/module. So they have no use in the config schema. (Removing here because their root type was sequence which would not be playing nice with the mapping assumed in config anyway).

Gábor Hojtsy’s picture

Issue summary: View changes

Updating issue summary.

Status: Needs review » Needs work

The last submitted patch, 27: 2459971-27.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
40.69 KB
2.23 KB

Add the missing langcode keys in the config schema test.

Gábor Hojtsy’s picture

Patch is done and complete as far as I am concerned. Please review.

vijaycs85’s picture

Status: Needs review » Reviewed & tested by the community

Changes looks good to me.

just a thought: I see the new type config_object is only applied for non-config-entity configs. Why don't we add to config entities as well?

Gábor Hojtsy’s picture

@vijaycs85: Thanks for the review. I had an encounter with trying to use config_object as a base type for config_entity from #7 to #19. @alexpott asked to keep the order of config entity keys as uuid and THEN langcode, which is not possible with this base typing due to how PHP merges arrays. (Even if we include the langcode definition in the config entity type at a different position, see #23).

Gábor Hojtsy’s picture

Also added a pretty detailed change notice draft at https://www.drupal.org/node/2460751

webchick’s picture

Assigned: Unassigned » alexpott

This feels alexpott-ish.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I think deprecating the param is the right way to go - we're giving d8 contrib a chance to upgrade by leaving the code in - we should file a follow up to remove this before 8.0.0 because I think there are many advantages to have all top level config objects extend from the same type. It makes adding any other global keys super simple. I can't think of another other than langcode but you never know. Committed 5b19144 and pushed to 8.0.x. Thanks!

  • alexpott committed 5b19144 on 8.0.x
    Issue #2459971 by Gábor Hojtsy: The langcode key on configuration files...
Gábor Hojtsy’s picture

Status: Fixed » Closed (fixed)

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