Follow-up to #2153937: Default language setting is hard to find
Problem/Motivation
Current implementation does not validate that new default language exists, this leads to notice and wrong assumption for user that settings saved.
Notice: Undefined index: af in Drupal\language\LanguageListBuilder->submitForm() (line 150 of core/modules/language/src/LanguageListBuilder.php).
Drupal\language\LanguageListBuilder->submitForm(Array, Object)
Steps to reproduce (admin/config/regional/language):
1) add second language
2) open the same page in second tab and delete it
3) on first tab select deleted language and try to submit form

Also reviewing a code:
Config factory should be properly injected.
Also form redirect should use entity url.
Proposed resolution
Add validation to form.
cover validation with tests.
Clean-up code.
Remaining tasks
Review. Test. Commit.
User interface changes
no
Comments
Comment #1
andypostSomething like that
Comment #2
andypostseems reset only needed when default language changed
Comment #5
andyposta bit more cleanup
Comment #6
gábor hojtsyFirst of all needs issue summary update on why is this a bug. No bugs are mentioned in the summary. If this is a bug, then a test is possible to write which reproduces the bug and proves this solves it.
The configuration factory. -- is what we use elsewhere for short.
Nice simplification.
Comment #7
andypostFixed IS and title.
New patch shows the bug, but no idea how to test that except manually (simpletest always replaces retrieved content)
Seems only manual testing applicable
Comment #10
gábor hojtsyOk, makes sense. Needs tests then :) Thanks!
Comment #11
Anonymous (not verified) commentedLike @andypost mentioned, it's hard to test with simpletest because
handleForm()unsets values that are not available on the form but are posted anyway.In the attached patch I try it anyway to demonstrate what I am trying to do. Any suggestion on how to improve this? Or is a totally different approach needed here?
Comment #12
gábor hojtsyThe test would fail if there was a notice, no? At least testbase has some code to augment that? Not sure where that is turned on for tests.
Comment #13
Anonymous (not verified) commentedThe test fails because I try to post "nl" and the
handleForm()method unsets that value, making the test fail to set the default language to "nl". If I could work around that, I'd be able to finish the test. I'll try to find a test that has a similar problem.Comment #14
Anonymous (not verified) commentedThis seems to work.
Comment #15
Anonymous (not verified) commentedComment #18
gábor hojtsyLooks good! Some minor notes:
Should be "Selected default language no longer exists." IMHO. We never know if someone alters the form to select a language for some other reason, so the error needs to be more specific.
Should be assertText(t()). You can remove the second string, the feedback should be fine based on the assertion.
Comment #19
Anonymous (not verified) commentedFixed feedback from #18.
Comment #20
Anonymous (not verified) commentedComment #21
gábor hojtsyLooks good, thanks!
Comment #23
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 2af2fb4 and pushed to 8.0.x. Thanks!
Comment #25
gábor hojtsyYay, thanks all!
Comment #26
andypost