Over in #2224887: Language configuration overrides should have their own storage we're creating a LanguageConfigOverride object it is not using schema during save because the configuration object it saves are only partial. This means that we can not derive the schema if it is dynamic.
Consider the Italian override for tour.tour.tour-test
label: Tour test italian
tips:
tour-test-1:
label: La pioggia cade in spagna
body: Per lo più in pianura.
the tips are plugins and without a plugin: text we can not work out the schema for the label and body fields.
Proposed resolution
To validate a LanguageConfigOverride, it must be merged with the default translation. The end result itself must pass validation for the config it tried to translate. That will work for both simple config and config entities.
ℹ️ The domain contrib module implemented a subset of this: it verifies shape conformance (strings where strings are expected, same for bools, etc), but does not execute the validation constraints.
Comments
Comment #1
gábor hojtsyComment #2
gábor hojtsyPostponing on #2224887: Language configuration overrides should have their own storage.
Comment #3
mgiffordThat's fixed now.
Comment #17
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #18
borisson_While I think it has a low priority, I think it still makes sense to validate that the schema of translated items matches the original type.
I think this issue should stay open.
Comment #19
smustgrave commentedComment #20
borisson_Adding validation tag
Comment #21
mably commentedWe have the same problem when saving Domain overrides.
What is the best way to handle it?
Comment #23
wim leersI was kinda shocked to learn this is not at all validated today. I found this issue thanks to AI, after I described the current status for config translations in core and what the data integrity consequences are for Canvas: #3583854: [upstream] Validate LanguageConfigOverrides targeting Canvas config entities.
Comment #24
wim leers@borisson_++ for #18
Added proposed resolution.
Comment #25
wim leersI bet that a
LanguageConfigOverridefornode.type.articlethat looks like this:would yield interesting results.
ConfigFactoryOverrideBase::filterOverride()won't filter any of those away — because the keys all existComment #26
wim leersI see the Domain module implemented a subset of what is needed, nice!
Comment #27
borisson_It looks like the domain module is only doing this on save, is that were we want to change this as well or do we want to do it while loading the config?
#25 sounds like a fun way to break a website :D. To prevent this, I think we need to ensure that we validate after loading as well. I'm wondering if that makes performance much slower though.
Comment #28
wim leersValidate-upon-load doesn't seem feasible. We don't do that for content entities either. Validate-on-save would already be a major improvement 😅
Comment #29
borisson_I've spent the last days wondering about this, because I wasn't sure if I agree. You are correct for language overrides, and config overrides that come from config split, don't even use the same mechanism.
I was wondering if this would introduce a difference between how overrides that are coming from settings.php work compared to other overrides, but thinking about it some more - those can stay with their own way of working - they are already very different.
So yes, I agree that validate-on-save is good here.
Comment #30
wim leersCanvas added support for this yesterday in https://git.drupalcode.org/project/canvas/-/commit/40e36b738bda6d61f89b2...
\Drupal\Core\Config\Development\ConfigSchemaCheckersibling: https://git.drupalcode.org/project/canvas/-/blob/897424363cc2cc1421d313d...AFAICT both would be pretty easy to add to core (the second one could be merged into the existing
\Drupal\Core\Config\Development\ConfigSchemaChecker). Thoughts? :)Comment #31
borisson_Both the constract and the ConfigSchemaChecker equivalent look to be easy enough to read. I don't like that there's basically an array of things that we need to keep track of where the constraint is automatically added.
As soon as one part of the schema has a translatable property, should be added automatically?
This is now limited to Config entities, but there are other types that can also have language overrides?
Comment #32
smustgrave commentedDon't see an MR to review, but if it were in review to add to core I think there is enough support here to do that. :)
Comment #33
wim leers#32: yep, sorry — wanted to get a general +1 for approach.
Yes and no — the current names in Canvas do imply that, but there's nothing entity-specific about the code: it relies solely on plain config + schema:
This I did not follow 😅🙈 Could you rephrase?
Comment #34
borisson_I tried to build enough context again to understand what I wanted to say here, I don't remember.
In any case, I am +1 on this issue and I really think this a good idea to introduce load-on-save for language overrides.