Problem/Motivation
It's possible to define a custom content type by placing necessary files into a custom module's directory structure under config/[install | optional]. One might also want that content type to be translatable and may have even gone through the work to define some of the translations. 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. Currently it's not the case, because for `install` folder system ignores scalar values and processes only array values and for the `optional` folder system installs config only in default collection, ignoring all other available collections.
Proposed resolution
Process translation files correctly, when module is installed separately or along with profile installation, by processing all values in configs and taking all available collections for optional configs.
Remaining tasks
Review the patch, give feedback and fix the patch,
API changes
Within current patch function installOptionalConfig() receives 3rd parameter with collection name.
Comment | File | Size | Author |
---|
Issue fork drupal-2845437
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
Gábor HojtsyThanks! Just tagging up as a start, will flesh this out a bit later.
Comment #3
vegantriathleteComment #4
vegantriathleteSteps to reproduce:
Setup
You must have the Language module enabled, as well as Content Translation. You must also have some language installed.
Actions
Expected behavior
You see the new content type, including all its fields and the translated values.
Current behavior
You see the new content type, including all its fields. However none of the translations appear. If you go to a Translate area, you just see the installed language with the Add button.
Comment #5
vegantriathleteComment #6
vegantriathleteComment #7
vegantriathleteI think we've got the same issue with things like tour.tour.some-tour.yml. Each of those YAML files has the language key. It makes sense that translations of tours should go inside of config/[install | optional]/lanuage/[language-code] as well.
Comment #8
Gábor HojtsyJust found #2831126: Translation strings from install profile config language/[langcode]/*.yml files don't appear in locale_target which seems to be a duplicate of this one. We should decide to close that one or this one.
Comment #9
Gábor HojtsyClosed that one. Bringing over this comment from @andypost there:
So according to that the config overrides may in fact be imported but then deleted because they don't end up in locales target.
Comment #10
Gábor HojtsyComment #11
andypostRelated probably affected by the same flow
Comment #13
ckaotikAs an ugly workaround, we've been putting optional configuration's translations into
config/install/language/{langcode}
, but that of course creates other problems.Update: Configuration translation in
config/optional
works if instead of a directory structure you preprend the collection to the config name, e.g.confg/optional/language.{langcode}.{configuration name}
.After some digging around, it seems to me this might be located in ConfigInstaller:installDefaultConfig(). Files in the
config/install
directory get special collection handling (e.g forlanguage.{langcode}
)while optional config gets handled separately using ConfigInstaller::installOptionalConfig()
which then does not check for collections at all:
$config_to_create = $storage->readMultiple($list);
.Comment #14
wesleydv CreditAttribution: wesleydv at District09 commentedCross posting because I originally posted it in the duplicate of this issue
--
We encountered this issue when developing a multilingual distribution.
We copied to code from .profile file in https://www.drupal.org/project/multilingual_demo to overcome it.
We also applied this patch https://www.drupal.org/files/issues/2650434-84.patch from this issue https://www.drupal.org/node/2650434
However we’ve noticed that field label translations kept loosing their translation after we did a locale-update
The only solution we could find is to also add the strings in a .po file that is imported.
I’m not sure if it is related to this issue, if not we can still create a separate issue.
But my colleagues and I spend way too much time on this, to not document it here ;-).
We plan to opensource this distribution, so I will add a new comment as soon as that is done (normally next week) so everybody can see exactly what we did.
Comment #15
andypostComment #17
aralnoth CreditAttribution: aralnoth commentedA possible workaround when you have some override locales in 'install/language/{langcode}' folders is to add an update function in the module that has the files with the following line:
\Drupal::service('language.config_factory_override')->installLanguageOverrides( '{langcode}' );
This fixes the problem because as a commented here, the locale-update command no works. I’m not sure if it is related to this issue but I leave this possible solution here in case it is helpful to someone else.
Comment #18
mpp CreditAttribution: mpp at AmeXio for District09 commentedWhen providing translations in a module's config/install directory we encounter an error during config import on install.
See #2955457
Comment #19
alexpottThis feels like a major bug.
Comment #21
iampumaCurrently we are having the same issue with our multilingual profile installation. An additional language module is provided that enables the language modules and contains translations configuration. These are however not imported.
Comment #22
spidermanI've been tracking this bug for awhile now, trying to get to the bottom if it- I've attempted most of the fixes mentioned on this ticket and several others. I've managed to confirm (in my debugger) that Gábor's comment in #9 appears to be correct, and none of the patches/fixes mentioned here or on the (apparently duplicate) issue #2831126 seem to fix it. After quite a bit of digging on this, I'm fairly stumped!
Note that #2831126 might have been different in the sense that it relied on an install profile, whereas the original Steps to reproduce just describe enabling a custom module. In my testing, it seems that the custom modules' config translation gets installed/updated properly if I "manually" enable the relevant modules *after* the site is already installed. So it appears to be something about the installer itself, or at least the way the installer handles custom profiles. I'm unclear how much of a role Features module plays in this difference, at this point, or whether that's a clue to the fix. (see notes at the end of my comment re: test setup)
Tracing the install.php process in my debugger, I can see exactly this section of code from LocaleConfigManager::updateConfigTranslations(), right at the end of the installer, in a step called Finish Translations.
As described in the comments of that function, this is called after the initial update of interface translation strings from l.d.o. What I don't yet understand is why the config overrides seem to be deleted, after earlier being (apparently) installed properly.
The key section if that function is this loop:
As Gábor noted, the
$this->deleteTranslationOverride()
call happens for these custom config translation overrides, which removes the previously installed (apparently because they haven't yet made it tolocale_target
?). However, I'm wondering if perhaps the problem arises because the$this->saveTranslationOverride()
is never called, since$data
is empty.This seems to be the result of the
$this->filterOverride()
call above, which is where I get lost. I'm not sufficiently savvy about the CMI and Multilingual components of core here (yet!)I'd love to write a patch for this, but I'm having trouble understanding the internal logic of the key functions, so I'm hoping someone can help shed some light on it. :)
My test setup
To be clear, the test setup I have to reproduce this is like this:
[webroot]/profiles/custom/[translate-profile]/modules/
)[webroot]/profiles/custom/[custom-translate-profile]/modules/*/config/install/language/[langcode]/*.yml
Comment #23
spidermanQuick update: In the interest of making it easy to see exactly what I've done, I've posted the minimal test setup codebase here:
https://www.drupal.org/sandbox/spiderman/3018656
I'll also note that in my digging on this, I noticed this closed issue: https://www.drupal.org/project/drupal/issues/1905152
which seems to be where the LocaleConfigManager code was initially introduced (I don't think the patch committed there actually included this bug, mind you)- just in case that's helpful to someone who knows more of the history of this stuff to jog our collective memory :)
Comment #24
geek-merlinThis really needs an IS update with a thorough definition of the problem.
From the above comments i get the feeling we have 2 problems. As i understand it:
* Manually enabling MYMODULE, language overrides are picked up from MYMODULE/config/install/language/foo/some.config.yml: WORKS
* Manually enabling MYMODULE, language overrides are picked up from MYMODULE/config/optional/language/foo/some.config.yml: DoesNotWork(?)
* Installing a profile that enables MYMODULE, language overrides are picked up from MYMODULE/config/install/language/foo/some.config.yml: DoesNotWork
* Installing a profile that enables MYMODULE, language overrides are picked up from MYMODULE/config/optional/language/foo/some.config.yml: DoesNotWork
So if this is correct, we have 2 issues
* Language overrides from config/optional are not picked up
* Language overrides are not picked up (or picked up and instantly deleted) if installing a profile
Comment #25
geek-merlinTo document it, i tested the first claim above
* Manually enabling MYMODULE, language overrides are picked up from MYMODULE/config/install/language/foo/some.config.yml: WORKS
like so:
* On a clean install
* $ drush en language
* Add language de
* $ echo "$settings['extension_discovery_scan_tests'] = TRUE;" >> sites/default/settings.php # So we can enable test modules
* $ drush en config_test
* $ drush cex
Now we can verify that a german language override is installed in language/de/config_test.system.yml
Comment #26
geek-merlinAddressing the install profile issue mentioned in #22, my gut feeling is this: If for some reason the schema is not yet picked up, there is no metadate for translatable properties, and the code fragment
surely will return empty. Just a gut feeling.
Comment #28
joelstein CreditAttribution: joelstein at On Fire Media commentedI can confirm that translatable configuration in mymodule/config/install/language/LANGCODE/mymodule.yml is not being installed when installing Drupal with a config profile.
This little workaround in mymodule.install seems to at least get this exported configuration installed:
EDIT: Actually, sorry, that didn't work. Probably for the same reason that @alex.rutz mentioned in #26... but I don't know for sure.
Comment #31
CKIDOWEdit: I was totally wrong.
Comment #32
dxvargas CreditAttribution: dxvargas at European Commission and European Union Institutions, Agencies and Bodies for European Commission and European Union Institutions, Agencies and Bodies commentedUsually locale (aka interface aka string) translations and active configuration translations are up to date.
But when installing the site, we have the special case where the config can be translated but there are no updates to the locale translations. As mentioned in #22, translations don't get into locale_target and this will later cause the removal of the configuration translations in
$this->deleteTranslationOverride()
inside the loop.The solution I'm proposing is to also update the locale translations when installing the site, by removing the check to
InstallerKernel::installationAttempted()
. Please see the attached patch.There is also the need to get the langcode in the event, not sure why it's not there. I suppose this should be fixed when the event is fired but in the patch, for simplicity, I get it from the collection name.
Finally, I found this problem when installing the site with the --existing-config option, having the translations in the sync config. Patch fixes the problem with this installation process. I've also tested the patch with steps given in #9 with success.
Comment #33
ckaotikThanks @dxvargas for the patch. This gets the
config/install
translation overrides imported, however on a mono-lingual non-English site (in my case: de) things break.en
config values are installed, but incorrectly labeled with ade
langcode 😱de
config override is installed and usedThe user is then always confronted with the translated config (
de
), but the config forms always display the originalen
values. Fun fact: This is without using theconfig_translation
module, so it makes no sense to the unsuspecting user.I think there was another issue somewhere, that was talking about the automatism on non-English monolingual sites. There, the appropriate translation (if it exists) was supposed to be merged into active configuration.
Edit: Found some issues about that: #2806009: Installing a module causes translations to be overwritten, #2905295: Configuration language being overwritten during module install, #2910353: Prevent saving config entities when configuration overrides are applied, #2925203: LocaleConfigSubscriber can result in data loss during install. So my comment here is probably only partially related.
Comment #35
mxhIs there any guide out there regards how to create multilingual installation profiles? How do other distributions handle the import of config translations - only via .po files? If config translations are not supposed to be included in installation profiles, I think that this documentation needs some more love.
Edit: It seems most distributions & modules handle it with .po translation files. My use case was a site with a configuration set that used to be imported via
drush cim
, but for reusage purposes I wanted to convert this set into a installation profile. The problem was that there were a lot of translated configurations, and these were not imported properly during site installation process. For such a situation I've found two possible ways (and both ways only seem to be workarounds to me):1. Create a custom translation (.po) file out of all existing translations. Luckily, after performing a
drush cim
of that given configuration, I could run alocale:export de --types=customized > de.po
and could use that file as translation file placed inmyprofile/translations/de
. During site installation and after enabling the corresponding language, the configuration translations were created with given translations from that .po file. Note that you might need to manually import this file to make sure those translations are being used, e.g. via drushlocale:import --override=all de /path/to/translation.de.po
. After having that translation file, I could delete all translated config files (i.e. deleting the whole config/install/language/de folder).2. As mentioned in #14, the
multilingual_demo
distribution handles this situation with an installation task. Implement an installation task in your profile that takes care to ensure that the given translation is saved. If you'd only want to take care of the translations to be saved, then have a look at https://git.drupalcode.org/project/multilingual_demo/-/blob/8.x-6.x/mult.... I implemented an installation task that makes sure that every configuration (including non-translations) is being saved consistently to the configuration files of my installation profile. The code for the .profile implementation looks like this:For modules, the above example function
_myinstallprofile_reimport_configuration
could be used within the hook_install implementation instead I guess.If it was only to solve the translation problem, I'd choose option 1. For my use case I actually use both options, because I did not want to have modules override configurations during installation process.
Maybe there is still a catch on this, but if others agree with it I could add such proposals to the above mentioned documentation page. Nevertheless, both options don't feel to be real solutions for this problem.
Comment #38
tijsdeboeckI trying to get custom config translations to work on custom module, in a non-English monolingual site, and I can confirm that the behaviour described by @ckaotik in #33 still exists in Drupal 9.3.
Comment #39
dxvargas CreditAttribution: dxvargas for European Commission and European Union Institutions, Agencies and Bodies commented@tijsdeboeck can you specify if the problem you have in your non-English monolingual site occurs after or before applying the patch #32? Or it's the same before and after?
Are you sure that problem is the same being described in the description of this issue? Sorry to ask but it seems to me we are diverting into another issue, different from the initial one.
Comment #40
tijsdeboeck@dxvargas, sorry, I am indeed mixing a couple of things indeed. I didn't install patch #32, when I had the issue... Please ignore my previous comment.
Comment #43
ankithashettyUploading a new patch with a change that might fix the config overrides issue for a different language during fresh installation.
The reason some configs were removed/deleted was the scalar elements in a config were ignored.
These changes were introduced in #2212069: Non-English Drupal sites get default configuration in English, edited in English, originals not actually used if translated. Any idea why we don't consider scalar values while importing a config? I don't see any reason here, if anyone could help here, that would be great.🙌🏼
Many related issues I came across:
Comment #45
ankithashettyUploading a new patch to avoid repetitive lines in #43.
Thanks!
Comment #47
ankithashettyAdding a new patch by fixing PHPUnit test errors in
core/modules/locale/tests/src/Kernel/LocaleConfigSubscriberForeignTest.php
.Changes made:
As we are deleting the locale translation, won't change the active/override config. Hence we are supposed to
assertNoTranslation
afterdeleteLocaleTranslationData
as per my understanding. Which is similar toassertTranslation
aftersaveLocaleTranslationData
intestEnglish()
function.Before fix:
After fix:
Comment #49
ankithashettyUploading a patch by fixing PHPUnit test errors in
core/modules/locale/tests/src/Functional/LocaleConfigTranslationImportTest.php
along withcore/modules/locale/tests/src/Kernel/LocaleConfigSubscriberForeignTest.php
.There is a contradiction between both test files actually. Like
testLocaleDeleteActiveTranslation()
function inLocaleConfigSubscriberForeignTest.php
Assuming that making any change to local translations, doesn't affect the active/override config translations, have updated the patch accordingly.
Requesting a review from maintainers.
Thanks!
Comment #51
ankithashettyTest fixes.
Comment #52
smustgrave CreditAttribution: smustgrave at Mobomo commentedMoving to NW for the issue summary update. Specifically what's the proposed solution for this?
Comment #53
ankithashettyLet'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 versiontest/config/install/language/ar/test.settings.yml
=> AR versionDuring 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 (likesettings
)So after installation, try doing
drsuh cget test.settings
for both language, you would notice: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!
Comment #55
sorlov CreditAttribution: sorlov at Skilld commentedFixed code check issues on 11.x
Comment #56
smustgrave CreditAttribution: smustgrave at Mobomo commented#53 made some suggestions that I think may be worth responding.
Issue summary still needs to be updated.
Comment #57
ilya.no CreditAttribution: ilya.no as a volunteer and at Skilld for Skilld commentedI'm not sure in what way should we update summary, because for me it looks clear. Should we just add text from comment #24?
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.
Comment #58
smustgrave CreditAttribution: smustgrave at Mobomo commentedCC 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
Comment #59
Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commentedFixed CC failure. Attached interdiff for same.
Leaving NW for summary update.
Comment #60
Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commentedFixed failures, attached interdiff for same
Comment #61
ilya.no CreditAttribution: ilya.no as a volunteer and at Skilld for Skilld commentedAttaching 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.
Comment #62
smustgrave CreditAttribution: smustgrave at Mobomo commentedIssue summary updated in #61
think we need a test case
From what I can tell the current test changes don't cover that.
Comment #63
ilya.no CreditAttribution: ilya.no as a volunteer and at Skilld for Skilld commentedAttaching 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`.
Comment #64
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe 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.)
Comment #65
andypostComment #66
ilya.no CreditAttribution: ilya.no as a volunteer and at Skilld for Skilld commentedI'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.
Comment #67
smustgrave CreditAttribution: smustgrave at Mobomo commentedNew 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.
Comment #69
ilya.no CreditAttribution: ilya.no as a volunteer and at Skilld for Skilld commentedI've created MR and added typehints. I'm not sure about CR, because I have no feedback about my suggestion so far.
Comment #70
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe 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.
Comment #71
sorlov CreditAttribution: sorlov at Skilld commentedRebased fork branch and rerolled patch. Need review
Comment #72
andypostNot sure the new method is the best solution but it will need CR anyway
Comment #73
smustgrave CreditAttribution: smustgrave at Mobomo commentedFor the CR