Problem/Motivation

ConfigurableLanguageManager::updateLockedLanguageWeights() saves the configuration behind the locked language configuration entities using a raw config save.

This is the only instance in core of using ConfigFactory::loadMultiple() to return configuration objects that are then saved, therefore, this blocks #2392319: Config objects (but not config entities) should by default be immutable.

Proposed resolution

Refactor to save the configuration entity. There is good test coverage in \Drupal\language\Tests\LanguageConfigurationTest::testLanguageConfigurationWeight()

Remaining tasks

Review and commit.

User interface changes

None

API changes

None

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because configuration entities should not be updated using the raw config API
Issue priority Critical because it blocks #2392319: Config objects (but not config entities) should by default be immutable.
Disruption Zero disruption - internal refactoring only.
Files: 
CommentFileSizeAuthor
#1 2407093.1.patch3.46 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,916 pass(es). View

Comments

alexpott’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
3.46 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,916 pass(es). View
Gábor Hojtsy’s picture

Issue tags: +language-base, +sprint

Looks good. There is only one consistency question IMHO.

+++ b/core/modules/language/src/ConfigurableLanguageInterface.php
@@ -16,4 +16,15 @@
+  public function setWeight($weight);
+

Why is this in the configurable language interface? setName() is on the LanguageInterface.

alexpott’s picture

re #2 because you can not set (and save) a weight on a non configurable language. Yep there is setName() but kind of a special flower - we use it in (of all places) LanguageManager::filterLanguages() to add a default language but with a translated name.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Well, we do create Language runtime objects with dynamic calculated weights in LanguageManager::getDefaultLockedLanguages(), we just use the constructor to set the weight. Its true that we don't set the weight afterwards, and that logically we should not. As for setName(), we could just as well create a new language object in filterLanguages() based on the default language data instead of setName() but that is not for the scope of this patch anyway.

Gábor Hojtsy’s picture

alexpott’s picture

Note that #2392319: Config objects (but not config entities) should by default be immutable contains this patch so if that issue is committed first this can be set to "Closed (Duplicate)"

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Closed (duplicate)
Gábor Hojtsy’s picture

Issue tags: -sprint

Removing from D8MI sprint too.