Vocabulary config entities are mostly validatable using the typed config system, but not quite. There is one property (weight) that are not yet validatable:
$ ./vendor/bin/drush config:inspect --filter-keys=taxonomy.vocabulary.tags --detail --list-constraints --fields=key,validatability,constraints
➜ 🤖 Analyzing…
-------------------- ------------- -----------------------------------------------------------------
Key Validatable Validation constraints
-------------------- ------------- -----------------------------------------------------------------
taxonomy.vocabular 92% ValidKeys: '<infer>'
y.tags
taxonomy.vocabula Validatable ValidKeys: '<infer>'
ry.tags:
taxonomy.vocabula Validatable ValidKeys:
ry.tags:_core - default_config_hash
taxonomy.vocabula Validatable NotNull: { }
ry.tags:_core.defa Regex: '/^[a-zA-Z0-9\-_]+$/'
ult_config_hash Length: 43
↣ PrimitiveType: { }
taxonomy.vocabula Validatable ValidKeys: '<infer>'
ry.tags:dependenci
es
taxonomy.vocabula Validatable Regex:
ry.tags:descriptio pattern: '/([^\PC])/u'
n match: false
message: 'Labels are not allowed to span multiple lines or
contain control characters.'
↣ PrimitiveType: { }
taxonomy.vocabula Validatable NotNull: { }
ry.tags:langcode Choice:
callback:
'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAl
lValidLangcodes'
↣ PrimitiveType: { }
taxonomy.vocabula Validatable Regex:
ry.tags:name pattern: '/([^\PC])/u'
match: false
message: 'Labels are not allowed to span multiple lines or
contain control characters.'
NotBlank: { }
↣ PrimitiveType: { }
taxonomy.vocabula Validatable ↣ PrimitiveType: { }
ry.tags:new_revisi
on
taxonomy.vocabula Validatable ↣ PrimitiveType: { }
ry.tags:status
taxonomy.vocabula Validatable Uuid: { }
ry.tags:uuid ↣ PrimitiveType: { }
taxonomy.vocabula Validatable Length:
ry.tags:vid max: 32
↣ PrimitiveType: { }
↣ Regex:
pattern: '/^[a-z0-9_]+$/'
message: 'The %value machine name is not valid.'
taxonomy.vocabula NOT ⚠️ @todo Add validation constraints to config entity type:
ry.tags:weight taxonomy.vocabulary.*
-------------------- ------------- -----------------------------------------------------------------
Remarks:
weight, but that's an independent concern: #3008064: Deprecate vocabulary weight propertytype for description is wrong in two ways: A) it currently is label and should be text, because multiple lines are allowed, B) the description is optional, just like it is for NodeType for example (see #3379091: Make NodeType config entities fully validatable and its change record), so it should be marked nullable: true and needs an update path to convert an empty string to NULL11.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=announcements_feed--detail --list-constraintsFullyValidatable.Review.
None.
description is no longer allowed to be the empty string ('') — vocabularies without a description must use null. Change record: https://www.drupal.org/node/3421927
None.
N/A
Show commandsStart 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, plain diff MR !6642
Comments
Comment #1
berdirvocabularies are config entities now, not sure this is possible yet?
Comment #2
berdirTagging anyway.
Comment #3
fagoTrue, so it's postponed on #1818574: Support config entities in typed data EntityAdapter
Comment #4
ianthomas_ukThis issue is part of a D8 critical meta, but is postponed against a D9 issue.
Comment #5
fagoGiven entity validation works on typed data validation, we don't necessarily need entity fields to be able to use it. Given that I think this should be postponed on #1928868: Typed config incorrectly implements Typed Data interfaces.
Comment #6
effulgentsia commentedChild issue of #1696648: [META] Untie content entity validation from form validation, therefore critical.
Comment #7
berdirI don't think this issue should be part of that meta anymore, while there might be possibilties to do validation, it's no longer a content entity and not special in any way compared to other config entities, so they should either be all in that meta or none of them.
Comment #8
effulgentsia commentedPer #7.
Comment #9
xjm#2164373: [META] Untie config validation from form validation — enables validatable Recipes, decoupled admin UIs … is now going to be postponed to 8.1.x as is #1928868: Typed config incorrectly implements Typed Data interfaces, so postponing this issue.
Comment #10
xjmComment #11
xjmSo I'm not actually finding any validation logic in
VocabularyFormthat would be missing elsewhere; is this issue still relevant?Comment #12
xjmComment #13
effulgentsia commentedGood question. Looks to me like the last form validation of this was removed in #1978112: Convert taxonomy admin path to follow other core entity patterns, which landed 3 weeks before this issue was opened.
Comment #14
effulgentsia commentedA bit more info:
VocabularyForm::form()invokesprotectBundleIdElement(), and that is also validated at the API level viaConfigEntityBundleBase::preSave().VocabularyForm::form()sets the#maxlengthof'name'to 255, but I don't think anything validates that at the API level. I bet we have a similar lack of length validation for many configuration values throughout all of Drupal, but I don't know to what extent that's really a problem.Comment #28
wim leersAll blockers here have been solved years ago.
Since then, the
Editorconfig entity when configured to use CKEditor 5 has become the first piece of complex configuration 100% validated via the API, not via form logic.See my write-up at #2164373-23: [META] Untie config validation from form validation — enables validatable Recipes, decoupled admin UIs ….
I think vocabularies are sufficiently simple that they'd be a great starting point for starting to convert all config entities in Drupal core to use validation constraints.
Comment #31
wim leersComment #32
wim leersComment #33
wim leersComment #35
wim leersI think tests will now pass. But we'll still need an update path test + actual update path logic.
There's nothing to update in
VocabularyForm— there's zero validation in\Drupal\taxonomy\VocabularyForm::validateForm(). Besides, #3231342: [PP-2] Introduce ConfigEntityForm to standardize use of validation constraints for config entities will introduce the necessary infrastructure to make that simple and consistent.Comment #36
wim leersRE
::validateForm()being empty — @effulgentsia's #14 is relevant here:The first bullet was covered by #3382580: Add new `ImmutableProperties` constraint and solved this generically, for all config entity types 👍
The second bullet talks about the maximum length for the human-readable label for the vocabulary. That uses
type: labelwhich gained validation constraints in #3377030: Add validation constraint to `type: label`: disallow multiple lines and was refined in #3379102: Add validation constraint to `type: label` + `type: text`: disallow control characters. There is no technical limit to config entity labels at this time.But there are things that need to be validated, as this issue's MR and
VocabularyValidationTest's inherited test coverage shows.So: consider @xjm's question in #11 addressed.
Comment #37
wim leersGreen! Marking for update path.
Comment #38
wim leersUpdate path test added; should fail. CR created: https://www.drupal.org/node/3421927.
Comment #39
wim leersUpdate path test failed — just pushed the missing update path to make tests green 🥳
Comment #40
smustgrave commentedLeft some small comments.
Comment #41
fagoexciting to see this after soo many years!
I studied the MR and it seems mostly great, I found only one small remark (see above)
Comment #42
wim leersThanks for the reviews, both of you!
Comment #43
wim leersComment #44
smustgrave commentedFeedback appears to be addressed.
Should drop a comment on #3008064: Deprecate vocabulary weight property to make sure that todo gets removed.
Comment #45
wim leers@smustgrave: done: #3008064-30: Deprecate vocabulary weight property 👍
Comment #46
catchOne comment on the MR.
Comment #47
catchI'm not convinced we should do #3008064: Deprecate vocabulary weight property and once this has landed that issue should be responsible for deprecations, so think we could just remove the @todo here.
Comment #48
wim leers#47: 👍 closed that issue and re-activated its sibling to get it back on track: #1821274-20: Add back ability to sort on vocabulary weight and name.
Given that the feedback was trivial to address, self-re-RTBC'ing
Comment #51
catchCommitted/pushed to 11.x and cherry-picked to 10.3.x, thanks!
Comment #52
wim leers🥳
POST/PATCHMR: #2300677-275: JSON:API POST/PATCH support for fully validatable config entities