Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Steps to reproduce:
- Install D8 in default install profile in English
- Enable all 4 multilanguage modules
- Add a second language (e.g. Dutch)
- Goto admin/config/media/file-system and submit settings
- bam! -> error (see below)
Remarks:
- This does only happen for a multilingual site and is triggered by the additional field "Interface translations directory" that appears in admin/config/media/file-system.
- Settings other than "Interface translations directory" are still saved.
Error message:
<em class="placeholder">Drupal\Core\Config\ImmutableConfigException</em>: Can not set values on immutable configuration locale.settings:translation.path. Use \Drupal\Core\Config\ConfigFactoryInterface::getEditable() to retrieve a mutable configuration object in <em class="placeholder">Drupal\Core\Config\ImmutableConfig->set()</em> (line <em class="placeholder">34</em> of <em class="placeholder">/Users/albert/Sites/drupal8-dev/core/lib/Drupal/Core/Config/ImmutableConfig.php</em>).
Comment | File | Size | Author |
---|---|---|---|
#33 | interdiff.txt | 1.14 KB | Gábor Hojtsy |
#33 | locale-settings-immutable-2414279-33.patch | 3.08 KB | Gábor Hojtsy |
#28 | interdiff.txt | 2.58 KB | Gábor Hojtsy |
#28 | locale-settings-immutable-2414279-28.patch | 3.07 KB | Gábor Hojtsy |
#26 | locale-settings-immutable-2414279-18-25.txt | 1.47 KB | victor-shelepen |
Comments
Comment #1
askibinski CreditAttribution: askibinski commentedThis is probably introduced by this change record "Configuration objects by default are immutable.":
https://www.drupal.org/node/2407153
Working on a patch.
Comment #2
askibinski CreditAttribution: askibinski commentedHere you go!
Comment #3
askibinski CreditAttribution: askibinski commentedchange status
Comment #4
Gábor HojtsyLooks good. Needs test. The test could be pretty simple, just attempt to set this directory and observe no failure :)
Comment #5
Gábor HojtsyComment #6
mon_franco CreditAttribution: mon_franco commentedComment #7
mon_franco CreditAttribution: mon_franco commentedI have checked this issue and now the directory is working well.
Let me know if you need other kind of testing :)
Comment #8
Gábor HojtsyWe would need an automated test written. Can you help with that? Just to check that setting the directory changed the configuration.
Comment #9
askibinski CreditAttribution: askibinski commentedThanks for testing :) But Gabor was referencing to an automated test which has to be written in code, see details:
https://www.drupal.org/contributor-tasks/write-tests
Comment #10
mon_franco CreditAttribution: mon_franco commentedComment #11
mon_franco CreditAttribution: mon_franco commentedSure! I'll be more than happy doing it :)
Comment #12
mon_franco CreditAttribution: mon_franco commentedComment #13
rodrigoaguileraStill needs tests
Comment #14
vijaycs85Wondering how did we miss this as part of #2392319: Config objects (but not config entities) should by default be immutable
Comment #15
Gábor HojtsyThere does not seem to be test coverage for this page, which is why we should add it here. IMHO
1. drupalGet() this page without locale module, verify the option is not there
2. Install locale module
3. drupalGet() this page, verify the option is there
3. drupalPostForm() a new setting
3. $this->config() assert that the new setting was saved
Comment #16
Gábor Hojtsy@mon_franco: how are you doing on this test? Do you need help?
Comment #17
mon_franco CreditAttribution: mon_franco commented@gábor-hojtsy sorry, I don´t need help, I just need free time to dedicate to this issue :(
Comment #18
victor-shelepen CreditAttribution: victor-shelepen commentedThe test has been added.
Comment #22
victor-shelepen CreditAttribution: victor-shelepen commentedStrange. The error is not invoked on my side.
Value true is TRUE. Other LocaleFileSystemFormTest.php 72 Drupal\locale\Tests\LocaleFileSystemFormTest->testFileConfigurationPage()
Comment #24
Gábor HojtsyThanks for providing a test, although this issue is assigned to @mon_franco. In cases like this it is important to ask the person assigned if they are working on it (like I did). I think we understood that answer differently. Sorry @mon_franco on behalf of @likin. Reviewing the patch itself:
"Contains " ... and should not have a space at the end. And should have the right class name.
NO need to set all three, just set the one you want to modify.
Use $this->config(), don't bother with the container separately.
Comment #26
victor-shelepen CreditAttribution: victor-shelepen commentedThese check the same thing by different ways.
It checks if displays the right value. It does.
Config says that the value has not been changed.
It seems it uses different containers.
Comment #28
Gábor HojtsyYeah we need to reset the config factory cache, because the one in the test does not know it changed. That fixes the fail. Also fixing all the doc lines. I think this is good to go but another quick review would be useful :)
Comment #29
vijaycs85+1 to RTBC, Just one question.
hmm, do we really need to reset this? as the save was through UI, doesn't it get reflect automatically?
Comment #30
Gábor Hojtsy@vijaycs85: this is a webtest and the config factory has a static cache in the class. Nothing would tell it that it needs to invalidate that cache given the config update happened in the simpletest browser in a different request.
Comment #31
vijaycs85cool then. Let's get this in. I guess the issue summary is clear.
Comment #32
alexpottThe post should refresh the variables. That reset should be unnecessary. See WebTestBase::refreshVariables().
After doing this you need $this->rebuildContainer() to bring the container in the parent inline with the child.
Comment #33
Gábor HojtsyAll right.
Comment #34
vijaycs85ah, nice. We are good to go for real now :)
Comment #35
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 9044a06 and pushed to 8.0.x. Thanks!
Comment #37
Gábor Hojtsyyay, thanks!