Problem/Motivation

LocaleConfigManager provides methods to remove configuration translation overrides when modules are uninstalled. Namely deleteComponentTranslations() which uses a local deleteTranslationData() method. However, when modules are uninstalled, their configuration is removed already and LanguageConfigFactoryOverride::onConfigDelete covers that case, so needing to look up which original files belonged to the given module and attempting to remove their language overrides is double work. When the config files are removed, their overrides vanish as well.

Proposed resolution

Remove the extra unneeded code.

Remaining tasks

Review. Commit.

User interface changes

None.

API changes

Public methods LocaleConfigManager::deleteComponentTranslations() and deleteTranslationData() removed. However, deleteTranslationData() was a very thin wrapper on deleting the language override itself, which is possible by loading the language override from the language manager and deleting: \Drupal::languageManager()->getLanguageConfigOverride($langcode, $name)->delete().

Files: 
CommentFileSizeAuthor
#5 interdiff.txt2.98 KBGábor Hojtsy
#5 2429385-dont-feed-back-to-locale-5.patch1.76 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,166 pass(es). View
#1 2429385-extra-code.patch4.44 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,238 pass(es). View

Comments

Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
4.44 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,238 pass(es). View
Gábor Hojtsy’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

We should figure out that test coverage exists for uninstalling modules removing config overrides. It may not exist yet.

Gábor Hojtsy’s picture

Note that LanguageConfigOverrideInstallTest exists to test that overrides are installed. I did not find a test covering that uninstalling would remove config overrides. (Also note that the config file tested in LanguageConfigOverrideInstallTest only has an override, so actually uninstalling that module would not remove the override because it does not actually ship with a config. That is a test-only case. We can add a scenario with a real core module or a different test module that has a config file, add an override and remove the module).

Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
1.76 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,166 pass(es). View
2.98 KB

I learned a lot while building #2212069-47: Non-English Drupal sites get default configuration in English, edited in English, originals not actually used if translated. So I realize the feeding back to the locale DB may be needed when deleting a config translation manually (so it does not come back). However that may need a bit more thinking. On the other hand, when removing a module, locale should certainly NOT be updated. We don't / cannot remove t() strings either at that point and we don't really know if those strings were used at other places too. So we should not remove them from locale. The LocaleConfigManager connection is what would update the locale DB. The config overrides themselves would be removed by LocaleConfigSubscriber anyway.

Gábor Hojtsy’s picture

Title: LocaleConfigManager implements deleting config overrides even though LanguageConfigOverride::onConfigDelete does it already » locale_system_remove() should not attempt to remove strings from the locale DB
Gábor Hojtsy’s picture

Status: Needs review » Closed (duplicate)
Issue tags: -sprint, -Needs tests

Most of this same code needs to be rewritten due to #2429651: When adding English, translated default config should get English translations extracted so we better remove it there.