Process translation config files for custom modules

Moving to NW for the issue summary update. Specifically what's the proposed solution for this?

Let's take a simple example:
We have a module called test, which gets installed during profile installation. It has a configuration for both default language (EN) and another language (for ex: AR)

test/config/install/test.settings.yml => EN version

title: 'Contact us'
// 'title' and 'description' is a scalar value.
settings:
    confirmation_message: 'Thanks for contacting us!'
    ....
// 'settings' is an array.

test/config/install/language/ar/test.settings.yml => AR version

title: 'اتصل بنا'
settings:
  confirmation_message: 'شكرا لإتصالك بنا. سنقوم بالرد عليك في أقرب وقت ممكن!'
    ....

During profile installation, when the test is getting installed, the config translations are also imported. But noticed that the scalar values of the config (i.e., title in the above example) are skipped and only array values are considered (like settings)
So after installation, try doing drsuh cget test.settings for both language, you would notice:

• Both 'title' and 'settings' in EN version (works fine)
• Only 'settings' in AR version (issue here)

Code for ref: https://git.drupalcode.org/project/drupal/-/blob/10.1.x/core/modules/locale/src/LocaleConfigManager.php#L636

Proposed solution (just a suggestion):
With the patch in #51, we are trying to process all the config values ('array' and 'string').

As I raised in #43, I still don't get why 'scalar' values are ignored.
I might be missing wrong here, but the issue is valid🤔

Thanks!

Fixed code check issues on 11.x

#53 made some suggestions that I think may be worth responding.

Issue summary still needs to be updated.

I'm not sure in what way should we update summary, because for me it looks clear. Should we just add text from comment #24 🐛 Process translation config files for custom modules Needs work ?

I'm attaching draft patch, which fixes the case, when we install module on already existing site. So current patch fixes 3/4 cases. Need to fix the case, when module is enabled during site installation.

I've noticed, that ConfigInstaller manages all available collections, when installs default config, but when it comes to optional config, then only default collection is used. So, I've added code to use all available collections.

CC failure in #57

Issue summary states the problem but doesn't clearly states the solution to the problem.

Is this changing the API or Data model? If not could leave those sections blank or put NA

Fixed CC failure. Attached interdiff for same.
Leaving NW for summary update.

Fixed failures, attached interdiff for same

Attaching the patch which fixes all 4 cases for me. I test with custom module, which has settings config in install folder and view config in optional folder. For both cases there are translations in language/fr folder.
Also updated summary.

Issue summary updated in #61

think we need a test case

If those translations are placed in config/[install | optional]/language/[language-code], then they should be pulled into the system when the module is enabled

From what I can tell the current test changes don't cover that.

Attaching patch with test. Not sure, what is the best place for it, so I've chosen locale module, because I haven't found tests for `ConfigInstaller`.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

I've found, that my new test module's name is already occupied, so I've renamed and relocated it from locale to config_translation module.

New tests should have typehints and returns

May need a CR for installOptionalConfig, just to announce the new parameter.

Can this be converted to an MR for quicker/easier reviews.

I've created MR and added typehints. I'm not sure about CR, because I have no feedback about my suggestion so far.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased fork branch and rerolled patch. Need review

Not sure the new method is the best solution but it will need CR anyway

For the CR

I've added draft CR https://www.drupal.org/node/3449092 → and rebased MR.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I've fixed tests errors, so pipeline for the MR is fine now.

Looks ready now, thank you

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Restoring previous status, as MR is rebased and pipeline is green.

I've added some comments to the MR that need addressing.

Sending issue back to review, as I've added changes for MR's comments and pipeline is green.

Question on the test changes. Are we losing coverage to get the tests to pass?

I've only added new test, because I haven't found any existing test for core/lib/Drupal/Core/Config/ConfigInstaller.php.
Concerning changing of existing tests, I find comments #47 🐛 Process translation config files for custom modules Needs work and #49 🐛 Process translation config files for custom modules Needs work explanatory, but maybe I haven't delved enough into them.