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

  1. Get a local git clone of Drupal core 11.x.
  2. 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!)
  3. composer require drush/drush
  4. vendor/bin/drush config:inspect --filter-keys=contact.settings --detail --list-constraints

Proposed resolution

  1. Add validation constraints.
  2. Mark FullyValidatable.

Remaining tasks

Review.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

Issue fork drupal-3422872

Command icon 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:

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

This 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) and filter.settings before 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.

wim leers’s picture

Ah, interesting — I worked to convert the weird integration of editing the contact.settings simple config from within the ContactFormEditForm config entity form to use #config_target (see https://www.drupal.org/node/3373502), but didn't realize that wouldn't work, because #config_target is only available on ConfigFormBase subclasses. 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_target support to config entity forms would this be possible to implement.

wim leers’s picture

Status: Needs review » Needs work
Issue tags: +Needs update path, +Needs update path tests

Legitimate failures:

  • MigrateContactSettingsTest (for both D6 & D7)
  • ContactStorageTest
  • ContactSitewideTest

This is likely because default_form no longer is allowed to be set to '', only to null. 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.

wim leers’s picture

Title: Add validation constraints to contact.settings » [PP-1] Add validation constraints to contact.settings
wim leers’s picture

Title: [PP-1] Add validation constraints to contact.settings » Add validation constraints to contact.settings
Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Needs work

NW for #5.

wim leers’s picture

Merged 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 😄

wim leers’s picture

kunal.sachdev made their first commit to this issue’s fork.

kunal.sachdev’s picture

Status: Needs work » Needs review
Issue tags: -Needs update path, -Needs update path tests
wim leers’s picture

Status: Needs review » Needs work

Thanks for pushing this further! 👍

The updated migration logic/tests are clearly incorrect though. 🙈

kunal.sachdev’s picture

Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Needs work
kunal.sachdev’s picture

Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Needs work

See https://git.drupalcode.org/project/drupal/-/merge_requests/6713#note_294465\Drupal\contact\Controller\ContactController::contactSitePage()'s logic is the problem.

kunal.sachdev’s picture

Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Needs work
kunal.sachdev’s picture

Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Reviewed & tested by the community

No more concerns 😊

🚢

larowlan made their first commit to this issue’s fork.

  • larowlan committed 55849506 on 10.3.x
    Issue #3422872 by kunal.sachdev, Wim Leers: Add validation constraints...

  • larowlan committed c41bea2d on 11.x
    Issue #3422872 by kunal.sachdev, Wim Leers: Add validation constraints...
larowlan’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Made a minor comment edit and waited for tests - came back green.

Committed to 11.x and backported to 10.3.x

Thanks

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.