The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
View modes have 3 property paths that are not yet validatable:
./vendor/bin/drush config:inspect --filter-keys=core.entity_view_mode.taxonomy_term.full --detail --list-constraints --fields=key,validatability,constraints
➜ 🤖 Analyzing…
--------------------------------------------------------------------- ------------- ---------------------------------------------------------------------------------------------
Key Validatable Validation constraints
--------------------------------------------------------------------- ------------- ---------------------------------------------------------------------------------------------
core.entity_view_mode.taxonomy_term.full 79% ValidKeys: '<infer>'
core.entity_view_mode.taxonomy_term.full: Validatable ValidKeys: '<infer>'
core.entity_view_mode.taxonomy_term.full:_core Validatable ValidKeys:
- default_config_hash
core.entity_view_mode.taxonomy_term.full:_core.default_config_hash Validatable NotNull: { }
Regex: '/^[a-zA-Z0-9\-_]+$/'
Length: 43
↣ PrimitiveType: { }
core.entity_view_mode.taxonomy_term.full:cache Validatable ↣ PrimitiveType: { }
core.entity_view_mode.taxonomy_term.full:dependencies Validatable ValidKeys: '<infer>'
core.entity_view_mode.taxonomy_term.full:dependencies.module NOT ❌ @todo Add validation constraints to ancestor type: config_dependencies
core.entity_view_mode.taxonomy_term.full:dependencies.module.0 Validatable NotBlank: { }
ExtensionName: { }
ExtensionExists: module
↣ PrimitiveType: { }
core.entity_view_mode.taxonomy_term.full:description Validatable Regex:
pattern: '/([^\PC\x09\x0a\x0d])/u'
match: false
message: 'Text is not allowed to contain control characters, only visible characters.'
↣ PrimitiveType: { }
core.entity_view_mode.taxonomy_term.full:id NOT ⚠️ @todo Add validation constraints to config entity type: core.entity_view_mode.*.*
core.entity_view_mode.taxonomy_term.full:label Validatable Regex:
pattern: '/([^\PC])/u'
match: false
message: 'Labels are not allowed to span multiple lines or contain control characters.'
NotBlank: { }
↣ PrimitiveType: { }
core.entity_view_mode.taxonomy_term.full:langcode Validatable NotNull: { }
Choice:
callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'
↣ PrimitiveType: { }
core.entity_view_mode.taxonomy_term.full:status Validatable ↣ PrimitiveType: { }
core.entity_view_mode.taxonomy_term.full:targetEntityType NOT ⚠️ @todo Add validation constraints to config entity type: core.entity_view_mode.*.*
core.entity_view_mode.taxonomy_term.full:uuid Validatable Uuid: { }
↣ PrimitiveType: { }
--------------------------------------------------------------------- ------------- ---------------------------------------------------------------------------------------------
Steps to reproduce
- Get a local git clone of Drupal core
11.x. composer require drupal/config_inspector— or manually install https://www.drupal.org/project/config_inspector/releases/2.1.5 or newer (which supports Drupal 11!)composer require drush/drushvendor/bin/drush config:inspect --filter-keys=core.entity_view_mode.taxonomy_term.full --detail --list-constraints
Proposed resolution
Add validation constraints to:
core.entity_view_mode.*.*:dependencies.modulecore.entity_view_mode.*.*:idcore.entity_view_mode.*.*:targetEntityType
This requires looking at the existing code and admin UI (if any) to understand which values could be considered valid. Eventually this needs to be reviewed by the relevant subsystem maintainer.
For examples, search *.schema.yml files for the string constraints: 😊
Reach out to @borisson_ or @wimleers in the #distributions-and-recipes.
Remaining tasks
User interface changes
None.
API changes
None.
Data model changes
More validation 🚀
Release notes snippet
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #36 | 3445150-nr-bot.txt | 20.03 KB | needs-review-queue-bot |
| #34 | 3445150-nr-bot.txt | 20.03 KB | needs-review-queue-bot |
| #28 | 3445150-nr-bot.txt | 90 bytes | needs-review-queue-bot |
| #18 | 3445150-nr-bot.txt | 14.94 KB | needs-review-queue-bot |
| #15 | Screenshot 2024-05-22 at 10.47.44 AM.png | 397.61 KB | smustgrave |
Issue fork drupal-3445150
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:
changes
Comments
Comment #4
narendrarComment #5
narendrarComment #6
wim leersNice progress here! 😄 Still needs some work, but for each of the things that still need to be addressed, there's prior art/patterns to look at 😊
Comment #7
carsoncho commentedComment #8
carsoncho commentedComment #9
carsoncho commentedComment #10
carsoncho commentedComment #11
carsoncho commentedComment #12
carsoncho commentedWhat I think is remaining is the tests updates. I see there's a few failing due to configuration tests as it's noting that the
core.entity_view_mode.*config files have been changed and updated. This is to be expected given thehook_post_update_NAME()being included here.Is the right thing to do here update the tests so they expect all those view mode configuration changes? Testing is an area I'd like to be able to contribute to more so any information folks can provide is much appreciated.
Comment #13
narendrarComment #15
smustgrave commentedReplied to the comment and applied 2 nitpicky things
On a 11.x setup with standard install there are several view modes across entity types
Applied the MR locally
Update hook ran without issue
Manually downloaded config inspector and am seeing fully validated entity_view_modes
Comment #16
catchOne note on the MR - missing a deprecation message. #3443942: Add proper deprecation notices in config entity presave bc layers has more details.
Comment #17
narendrarComment #18
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #19
narendrarComment #20
narendrarComment #21
narendrarComment #22
wim leersThanks for your continued contributions in the config validation space, @carsoncho! 🙏😊
A few nits, and one question about a comment that seems wrong.
The MR itself looks great! 👍
Comment #23
narendrarUpdating description to
'description' => NULLis giving depreciation error. Other feedback addressed.Comment #24
narendrarFeedback addressed.
Comment #25
wim leersThe deprecation error you're seeing is literally the one this MR is adding.
Why does that deprecation exist here?
node_node_type_presave()doesn't do that (introduced in #2002174: Allow vocabularies to be validated via the API, not just during form submissions.) Ah … because @catch asked for that.Well, now we've got ourselves a chicken-egg situation. 😅
The only way I see out of this is to not rely on the
presavehook to perform the update, but instead to duplicate that logic into the update hook.Comment #26
narendrarRe #25, Does it mean removing
system_entity_view_mode_presave()from system.module and doing$view_mode->set('description', NULL)->save();in system_post_update_convert_empty_string_entity_view_modes_to_null of system.post_update.php. Also where should we use @trigger_error in this case or it can be avoided?When I did as above it gives
Schema errors for core.entity_view_mode.node.teaser with the following errors: 0 [description] This value should not be blank.Comment #27
narendrarComment #28
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #30
bbralaNext up, this needs a little word making sure the fixture/config* is updated to reflect some of the config changes.
Comment #31
bbralaStill some test errors left in biuld/jsonapi and randomjs kinda test.
Comment #32
bbralaMy contribution in code was minimal, mostly rebasing and small fix to make sure it still validates. One hook needs converting to an OOP hook since this has been open for so long.
If noone else does, i can move the code, think i can even still RTBC after that since it doesnt really introduce code.
Comment #33
bbralaGoing to go out on a limb and RTBC since i only rebased and fix codestyle/phpstan issues and didnt really write new code.
Comment #34
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #35
bbralaI think the review bot is wrong, the text file also stated no errors. Still merged 11.x again though.
Comment #36
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #37
bbralaGo away nr bot.
Comment #38
borisson_The last run on the mr has a phpcs failure, let's fix that.
Comment #39
borisson_The changes that are done look good though, I think they all make sense.
Comment #40
bbralaFixed the phpcs issues.
Comment #41
borisson_I think this is good, I can't find any changes I'd make to this issue anymore.
Comment #42
alexpottWe need to update the deprecations to 11.3.x plus I think the hook this MR adds does not have the correct info the annotation.
Comment #44
bbralaI think we should skip the null changes. Me, @longwave, @phenaproxima, @berdir talked about this on slack and it will remove a lot of the hurdles to move forward with these kinda of issues. Just compare the changes of
#13904 and #7906 the difference is insane.
Gonna go our on a limb and NR this with the minimized changes. (IS seems to cover current approach still :P)
Edit: updated contribution record.
Comment #45
bbralaJust thinking, we could argue that we still need the presave to make saving either '' or null always, so it will be eventually consistent. But not sure if that is worth the hook.
Comment #46
borisson_Consistency is a big goal of increasing the strictness in my opinion, I think that's worth the hook.
Comment #47
bbralaThats a small change, i can do that.
Comment #48
bbralaWhich do you prefer as the default. '' or NULL? Id probably opt for ''
edit: added the hook
Comment #49
bbralaHmm, with '' it is all green since that has been the default for aw hile now.
With NULL we do have failing tests that expect ''.
Comment #50
smustgrave commentedMost of the tests appear to be failing with something like this. Possibly due to the presave hook?
Comment #51
bbralaYeah.
Also, very related, we need to decide on this, could use some eyes:
#3560443: [Policy, no patch] Stop enforcing NULL for text config properties that do not need to differentiate from empty string