Problem/Motivation
The Contact module's settings have 3 property paths that are not yet validatable:
$ ./vendor/bin/drush config:inspect --filter-keys=contact.settings --detail --list-constraints --fields=key,validatability,constraints
➜ 🤖 Analyzing…
--------------------------------------------- ------------- ------------------------------------------
Key Validatable Validation constraints
--------------------------------------------- ------------- ------------------------------------------
contact.settings 63% ValidKeys: '<infer>'
contact.settings: Validatable ValidKeys: '<infer>'
contact.settings:_core Validatable ValidKeys:
- default_config_hash
contact.settings:_core.default_config_hash Validatable NotNull: { }
Regex: '/^[a-zA-Z0-9\-_]+$/'
Length: 43
↣ PrimitiveType: { }
contact.settings:default_form NOT ⚠️ @todo Add validation constraints here
contact.settings:flood Validatable ValidKeys: '<infer>'
contact.settings:flood.interval NOT ⚠️ @todo Add validation constraints here
contact.settings:flood.limit NOT ⚠️ @todo Add validation constraints here
contact.settings:user_default_enabled Validatable ↣ 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=contact.settings --detail --list-constraints
Proposed resolution
- Add validation constraints.
- Mark
FullyValidatable.
Remaining tasks
Review.
User interface changes
None.
API changes
None.
Data model changes
None.
Release notes snippet
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | Screenshot 2024-03-13 at 4.30.02 PM.png | 344.54 KB | wim leers |
| #4 | 3422872-config_target-do-not-test.patch | 3.2 KB | wim leers |
Issue fork drupal-3422872
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:
- 3422872-contact-settings
changes, plain diff MR !6713
Comments
Comment #3
wim leersThis needed #3422398: Allow specifying a prefix for ConfigExistsConstraint, to enable using it for references to config entities.
But sadly once again, just like both
book.settings(see #3422862-3: Add validation constraints to book.settings) andfilter.settingsbefore it (see #3395562: Add validation constraints to filter.settings and #1932544: Remove all traces of fallback format concept from the (admin) UI), this simple config violates the basic rule of simple config: that it cannot depend on other config entities. 😬 Fixing that is out of scope here.Comment #4
wim leersAh, interesting — I worked to convert the weird integration of editing the
contact.settingssimple config from within theContactFormEditFormconfig entity form to use#config_target(see https://www.drupal.org/node/3373502), but didn't realize that wouldn't work, because#config_targetis only available onConfigFormBasesubclasses. So … there's nothing else to do here!Attached is the patch containing what I did before I realized it cannot work. Only if #3231342: [PP-2] Introduce ConfigEntityForm to standardize use of validation constraints for config entities adds
#config_targetsupport to config entity forms would this be possible to implement.Comment #5
wim leersLegitimate failures:
MigrateContactSettingsTest(for both D6 & D7)ContactStorageTestContactSitewideTestThis is likely because
default_formno longer is allowed to be set to'', only tonull. That means this needs an update path similar to #3379091: Make NodeType config entities fully validatable and #2002174: Allow vocabularies to be validated via the API, not just during form submissions.Comment #6
wim leersClarifying that this is blocked on #3422398: Allow specifying a prefix for ConfigExistsConstraint, to enable using it for references to config entities.
Comment #7
wim leers#3422398: Allow specifying a prefix for ConfigExistsConstraint, to enable using it for references to config entities landed, rebased MR 👍
Comment #8
wim leersNW for #5.
Comment #9
wim leersMerged in upstream. I still expect this to fail due to #5.
But … I also expect a nice green new CI job thanks to #3422641: Add a "Validatable config" tests job to GitLab CI to help core evolve towards 100% validatability 😄
Comment #10
wim leersYay:
🤩
Details: https://git.drupalcode.org/issue/drupal-3422872/-/jobs/1056803
Comment #12
kunal.sachdev commentedComment #13
wim leersThanks for pushing this further! 👍
The updated migration logic/tests are clearly incorrect though. 🙈
Comment #14
kunal.sachdev commentedComment #15
wim leersComment #16
kunal.sachdev commentedComment #17
wim leersSee https://git.drupalcode.org/project/drupal/-/merge_requests/6713#note_294465 —
\Drupal\contact\Controller\ContactController::contactSitePage()'s logic is the problem.Comment #18
kunal.sachdev commentedComment #19
wim leersComment #20
kunal.sachdev commentedComment #21
wim leersNo more concerns 😊
🚢
Comment #25
larowlanMade a minor comment edit and waited for tests - came back green.
Committed to 11.x and backported to 10.3.x
Thanks