Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Comment type entity is not yet fully validatable:
./vendor/bin/drush config:inspect --filter-keys=comment.type.comment --detail --list-constraints --fields=key,validatability,constraints
> 🤖 Analyzing…
------------------------------------------------- ------------- ----------------------------------------------------------------------
Key Validatable Validation constraints
------------------------------------------------- ------------- ----------------------------------------------------------------------
comment.type.comment 91% ValidKeys: '<infer>'
comment.type.comment: Validatable ValidKeys: '<infer>'
comment.type.comment:_core Validatable ValidKeys:
- default_config_hash
comment.type.comment:_core.default_config_hash Validatable NotNull: { }
Regex: '/^[a-zA-Z0-9\-_]+$/'
Length: 43
↣ PrimitiveType: { }
comment.type.comment:dependencies Validatable ValidKeys: '<infer>'
comment.type.comment:description Validatable Regex:
pattern: '/([^\PC\x09\x0a\x0d])/u'
match: false
message: 'Text is not allowed to contain control characters, only
visible characters.'
↣ PrimitiveType: { }
comment.type.comment:id Validatable Regex:
pattern: '/^[a-z0-9_]+$/'
message: 'The %value machine name is not valid.'
Length:
max: 166
↣ PrimitiveType: { }
comment.type.comment:label Validatable Regex:
pattern: '/([^\PC])/u'
match: false
message: 'Labels are not allowed to span multiple lines or contain
control characters.'
NotBlank: { }
↣ PrimitiveType: { }
comment.type.comment:langcode Validatable NotNull: { }
Choice:
callback:
'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllVali
dLangcodes'
↣ PrimitiveType: { }
comment.type.comment:status Validatable ↣ PrimitiveType: { }
comment.type.comment:target_entity_type_id NOT ⚠️ @todo Add validation constraints to config entity type:
comment.type.*
comment.type.comment: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=comment.type.comment --detail --list-constraints
Proposed resolution
Add validation constraints to missing properties.
Remaining tasks
User interface changes
None.
API changes
None.
Data model changes
More validation 🚀
Release notes snippet
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | 3455066-nr-bot.txt | 90 bytes | needs-review-queue-bot |
| #20 | 3455066-nr-bot.txt | 90 bytes | needs-review-queue-bot |
| #19 | Screenshot 2025-03-03 at 11.42.34 AM.png | 25.64 KB | smustgrave |
| #19 | Screenshot 2025-03-03 at 11.26.37 AM.png | 23.28 KB | smustgrave |
Issue fork drupal-3455066
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 #2
mtiftComment #4
mtiftComment #5
borisson_This requires us to add something like EntityTypeExistsValidator, currently working on that.
Comment #7
borisson_Spellcheck fails, but not sure how to better name this.
Comment #8
borisson_Comment #9
smustgrave commentedMR appears to have phpcs errors.
Comment #10
pooja_sharma commentedI have observed the test failures, may be we can use:
"commentAccepted/commentEligible/commentAllowed" in place of "Commentable"
As "commentAccepted/commentEligible/commentAllowed" word would be more closer/relative to the "Commentable" word to fix cspell test failures.
Left suggestion not updating MR, as most of the work done already.
Comment #12
bbralaCommentable makes so much sense i dont want to change it, just added to the drupal words. THink this can go back to NR
Comment #13
bbralaSeems a test is still fialing since it expects a string, not null.
Comment #14
bbralaedit: Seems the message is more generic, when an entity doesnt use interger id, it doesn't validate as commentable. Which makes sense, the error is just not adjusted to that situation
Comment #15
bbralaOk, so the issue remaining is the fact that in
ConfigEntityValidationTestBase::testRequiredPropertyValuesMissingit tries to test what happens when a property is set to null. WhenEntityTypeExistsConstraintValidatoris validated it also executes the code:Which is then thrown. When looking at other implementations where this eror is thrown i see ContentLanguageSettings with
EntityBundleExists. This sems to be an immutable property.I'm now guessing there is 2 possible solutions:
1. The target_entity_type_id property of the comment type should be immutable.
2. It is not immutable, but instead of throwing an UnexpectedTypeException is might just need to add a validation error that the property shhould be string.
What the right solution is, i dont know. Hopefully someone does :x
Comment #16
bbralaDid some digging. Ended up removing the exception and checking for null and '' in IsCommentable and EntityTypeExists. Seems to make more sense. Also adjusted the test so it expects extra validation errors when looking for null.
Comment #17
bbralaAll green.
Comment #18
borisson_I think this looks good, there's no way I can rtbc this issue, because I wrote part of it.
Comment #19
smustgrave commentedBefore
After
"EntityTypeExists" We actually used a similar constraint in book contrib so glad to see this is going to be part of core.
Did a review of the code changes and nothing stands out and comments appear to be functioning just fine (11.x standard profile install locally).
LGTM
Comment #20
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 #21
bbralaConflict was on cspell word list. Rebased, keeping rtbc
Comment #22
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 #23
bbralaThis was a clean rebase. Not sure why its confused.
Comment #24
bbralaAdded CR
Comment #25
longwaveAdded some nits to the MR.
Comment #26
bbralaFixed a few issues, but still need to make the constraint container aware to inject the entitty type manager and use that in the validator.
Comment #27
bbralaThink nullanble is not allow, but willl cycle back later.
Comment #28
bbralaAll green after cleanup. But to much to set rtbc right away
Comment #29
bbralaComment #30
bbralaUnrealted failure, all good imo.
Comment #31
borisson_Looks like all the remarks by longwave have been resolved.
Comment #32
longwaveLooks good, one question about the placement of IsCommentableConstraint.