Problem/Motivation
As part of #2164373: [META] Untie config validation from form validation — enables validatable Recipes, decoupled admin UIs …, we are going to want to validate language codes as language codes, with their own constraints, rather than as plain strings.
Steps to reproduce
N/A
Proposed resolution
Add a new langcode data type to config schema, which implements certain validation constraints that are specific to language codes.
Remaining tasks
Figure out how to implement the feature and add robust test coverage.
User interface changes
None.
API changes
TBD, but there will probably be at least one new config data type (langcode) and possibly a new validation constraint or two.
Release notes snippet
TBD
Issue fork drupal-3373653
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 #3
andypostAdded suggestion for one more place
Comment #4
wim leers🥳 Posted a first review, but this is looking excellent already!
Comment #5
wim leersI just have a single request for clarification … other than that, this is IMHO RTBC already! 🤩
Definitely missing a change record to announce this new config schema type though! 🤓
Comment #6
phenaproximaChange record! https://www.drupal.org/node/3373721
Assigning to Wim for final review.
Comment #7
phenaproximaAdjusting credit.
Comment #8
wim leersTweaked the change record to be more similar to the one for #2920678: Add config validation for the allowed characters of machine names.
Tightened test coverage in 2 ways (which were the sole remaining concerns I raised in my review). This required adding a single additional line to the config schema definition.
IMHO this is now RTBC 😊
Comment #9
borisson_This new data type for config schemas is a good addition to make configuration schema's more strict. I had one remark on the pull request but @Wim Leers pointed out that should not be an issue. I'm in the hopes that the last pull request will pass tests so already setting this to rtbc.
Comment #10
lauriiiThere's 31 test fails with the MR 🤔
Comment #11
wim leersMea culpa — forgot a single period! 🙈
Comment #12
borisson_This time I waited for the testbot to come back green. Back to RTBC.
Comment #13
wim leersThis blocks #2443991: Allow default_langcode field value to be changed, which is marked . Matching priority.
Comment #14
bnjmnmSetting to NW based on a few items I added to the MR. What I'm asking about is likely pretty trivial so if that proves to be the case, whoever addresses this can switch it back to RTBC without waiting for another NR cycle.
An additional question: are there contrib/custom cases that might result in errors on update? It seems possible since this constraint is applied to the
langcode:property of the root configuration object. It gets a "The value you selected is not a valid choice." error, but perhaps at least the CR could include a section on how to most easily address the error if a site encounters it? Ideally it's phrased in a search-engine friendly way for those who don't read change records but get the error.Comment #15
phenaproximaComment #16
smustgrave commentedApplied the MR to 11.x
Searched for "label: 'Language code' and all the instances appear to have been captured.
CR makes sense to me.
Think this is good.
Comment #17
penyaskito+1 I reviewed that this took into account the custom langcode configuration languages.
Comment #18
larowlanAsked some questions on the merge request
Comment #19
phenaproximaBack to review!
Comment #20
penyaskitoI'd use reset() instead of key(), but that's too nitpicky for not RTBCing this. All feedback so far has been considered already.
Do we need release notes for this? I don't think so.
Change record? Exists at https://www.drupal.org/node/3373721.
So IMHO RTBC
Comment #21
borisson_+1 on the rtbc.
Comment #22
bnjmnmBad news: Back to NW because of a test failure
Good news: looks like the failure might be because this validation works/
Comment #23
phenaproximaHuh, go figure. I guess I forgot to add a couple of special internal language codes.
Comment #24
borisson_Back to rtbc now that the test coverage looks complete.
Comment #26
bnjmnmLove to see schemas get smarter and more leveraging of those handy Symfony validation constraints. Committed to 11.x