Problem/Motivation

In Drupal, extensions can provide "Language config overrides" - i.e. config translations for non-default languages.

While Config Rewrite is great at supporting default configuration overrides, if you wanted to overwrite configuration for another language, you're out of luck.

A basic example of this is an installation profile that wants to provide default configuration overrides for the site title and slogan in multiple languages. This configuration (system.site) is already provided by core, so without custom code it would be impossible to provide multilingual defaults using Config Rewrite.

Proposed resolution

Add support for language config override rewrites to Config Rewrite. Just as a module may provide config in /config/install/language/fr/system.site, they can now provide configuration in /config/rewrite/language/fr/system.site. Pretty neat!

Remaining tasks

Review the patch, port the functionality to 8.x-2.x.

User interface changes

None.

API changes

The public API remains the same for \Drupal\config_rewrite\ConfigRewriter, but some functions have been refactored to support multilingual.

Data model changes

None.

CommentFileSizeAuthor
#13 interdiff-2846515-9-13.txt5.6 KBsamuel.mortenson
#13 config-rewrite-multilingual-2846515-13.patch12.42 KBsamuel.mortenson
#9 config-rewrite-multilingual-2846515-9.patch6.05 KBsamuel.mortenson
#8 Screen Shot 2017-01-25 at 9.19.10 AM.png19.96 KBsaltednut
#4 Screen Shot 2017-01-25 at 8.31.31 AM.png74.43 KBsaltednut
config-rewrite-support-languages.patch6.05 KBsamuel.mortenson

Comments

samuel.mortenson created an issue. See original summary.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
Issue tags: +Needs tests

Nice addition!

We should extend the existing test to cover this.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented

Thanks @nedjo! We should update the testing settings for this project as well so that patches can be tested before commit.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented
StatusFileSize
new Screen Shot 2017-01-25 at 8.31.31 AM.png74.43 KB

The testing is already set up for per commit AND for issues. Not sure what else we need to do.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented
samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented
Status: Needs review » Needs work

Weird, I wonder why my patch wasn't tested then. I'll try toggling the status.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented
Status: Needs work » Needs review
saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented
StatusFileSize
new Screen Shot 2017-01-25 at 9.19.10 AM.png19.96 KB

Was the "test with" dropdown available when you uploaded the patch? We could try re-uploading.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented
StatusFileSize
new config-rewrite-multilingual-2846515-9.patch6.05 KB

Testing...

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

We should be able to leverage core's configuration collection API (which is what's used to provide language-specific configuration) to simplify this patch.

See:

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented

@nedjo I've been reviewing the links you sent a bit, thanks for sharing. Since Config Rewrite kicks in after config has been installed (and there is no event to work with) we're a bit different than Config Share, so I'm not sure how to implement config collections generically in this patch to support languages. This is as far as I have gotten:

  $config_manager = \Drupal::service('config.manager');
  $collection_info = $config_manager->getConfigCollectionInfo();
  foreach ($collection_info->getCollectionNames() as $collection_name) {
    $override_service = $collection_info->getOverrideService($collection_name);
    // Do something with the service?
  }

Any tips? I'm not sure how to easily get this working, based on our requirements for Config Rewrite. In the mean-time, I'll start writing test coverage since that coverage shouldn't change based on the implementation.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

You'll need to inject the config manager. Then look at how ConfigInstaller::installDefaultConfig() passes off the loading of config to ConfigInstaller::getConfigToCreate(). You'll want to write a similar method. You can see how this is done in Configuration Share.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented
StatusFileSize
new config-rewrite-multilingual-2846515-13.patch12.42 KB
new interdiff-2846515-9-13.txt5.6 KB

Added test coverage. @nedjo I looked into how Configuration Share is doing this but I think a lot of their code relies on hooking into the config_provider's install routines, and in Config Rewrite we're working after all config has already been installed. I'm not sure I have the knowledge to refactor the current patch to work more like Configuration Share. :O

Status: Needs review » Needs work

The last submitted patch, 13: config-rewrite-multilingual-2846515-13.patch, failed testing.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented

I agree that we maybe cannot really use the ConfigInstaller the same way as others, given we need it to run and *then* overwrite things. But I may be mistaken there.

In any case, I think if no one has objections, we can commit what we have so far to add some form of support for i18n and follow up with the specifics of writing a class that extends ConfigProviderBase and adding config_provider as a dependency.

In fact, we already have an issue reL config_provider #2828931: Integrate with Configuration Provider

Rather than making that a blocker here, I'd rather move forward and followup in that issue if we can.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented

Additionally, I do understand leverage core's configuration collection API would be good but its not a requirement for making this work for people. I have seen a fair drop off of usage and I assume a total lack of i18n support is a legitimate problem. We have been using the latest patch with much success. So though it is not the most API-driven code, nor is it extending config_provider like we'd want it to (and it will, at some point) but it is working.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented
Version: 8.x-1.x-dev » 8.x-2.x-dev
Status: Needs work » Patch (to be ported)

We should port what we have to 8.x-2.x as well.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented
Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

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