Problem/Motivation

system.site has 5 property path that are not yet validatable:

vendor/bin/drush config:inspect --filter-keys=system.site --detail --list-constraints
➜  🤖 Analyzing…

 Legend for Data: 
  ✅❓  → Correct primitive type, detailed validation impossible.
  ✅✅  → Correct primitive type, passed all validation constraints.
 ---------------------------------------- --------- ------------- ------ --------------------------------------------------------------------------------------------- 
  Key                                      Status    Validatable   Data   Validation constraints                                                                       
 ---------------------------------------- --------- ------------- ------ --------------------------------------------------------------------------------------------- 
  system.site                              Correct   69%           ✅❓   ValidKeys: '<infer>'                                                                         
                                                                          LangcodeRequiredIfTranslatableValues: null                                                   
   system.site:                            Correct   Validatable   ✅✅   ValidKeys: '<infer>'                                                                         
                                                                          LangcodeRequiredIfTranslatableValues: null                                                   
   system.site:_core                       Correct   Validatable   ✅✅   ValidKeys:                                                                                   
                                                                            - default_config_hash                                                                      
   system.site:_core.default_config_hash   Correct   Validatable   ✅✅   NotNull: {  }                                                                                
                                                                          Regex: '/^[a-zA-Z0-9\-_]+$/'                                                                 
                                                                          Length: 43                                                                                   
                                                                          ↣ PrimitiveType: {  }                                                                        
   system.site:admin_compact_mode          Correct   Validatable   ✅✅   ↣ PrimitiveType: {  }                                                                        
   system.site:default_langcode            Correct   Validatable   ✅✅   NotNull: {  }                                                                                
                                                                          Choice:                                                                                      
                                                                            callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'  
                                                                          ↣ PrimitiveType: {  }                                                                        
   system.site:langcode                    Correct   Validatable   ✅✅   NotNull: {  }                                                                                
                                                                          Choice:                                                                                      
                                                                            callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'  
                                                                          ↣ PrimitiveType: {  }                                                                        
   system.site:mail                        Correct   Validatable   ✅✅   Email:                                                                                       
                                                                            message: '%value is not a valid email address.'                                            
                                                                          ↣ PrimitiveType: {  }                                                                        
   system.site:mail_notification           Correct   NOT           ✅❓   ⚠️  @todo Add validation constraints here                                                    
   system.site:name                        Correct   Validatable   ✅✅   Regex:                                                                                       
                                                                            pattern: '/([^\PC])/u'                                                                     
                                                                            match: false                                                                               
                                                                            message: 'Labels are not allowed to span multiple lines or contain control characters.'    
                                                                          ↣ PrimitiveType: {  }                                                                        
   system.site:page                        Correct   Validatable   ✅✅   ValidKeys: '<infer>'                                                                         
   system.site:page.403                    Correct   NOT           ✅❓   ⚠️  @todo Add validation constraints here                                                    
   system.site:page.404                    Correct   NOT           ✅❓   ⚠️  @todo Add validation constraints here                                                    
   system.site:page.front                  Correct   NOT           ✅❓   ⚠️  @todo Add validation constraints here                                                    
   system.site:slogan                      Correct   Validatable   ✅✅   Regex:                                                                                       
                                                                            pattern: '/([^\PC])/u'                                                                     
                                                                            match: false                                                                               
                                                                            message: 'Labels are not allowed to span multiple lines or contain control characters.'    
                                                                          ↣ PrimitiveType: {  }                                                                        
   system.site:uuid                        Correct   Validatable   ✅✅   Uuid: {  }                                                                                   
                                                                          NotNull: {  }                                                                                
                                                                          ↣ PrimitiveType: {  }                                                                        
   system.site:weight_select_max           Correct   NOT           ✅❓   ⚠️  @todo Add validation constraints here                                                    
 ---------------------------------------- --------- ------------- ------ --------------------------------------------------------------------------------------------- 

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=system.site --detail --list-constraints

Proposed resolution

Add validation constraints to:

  1. system.site:mail_notification
  2. system.site:page.403
  3. system.site:page.404
  4. system.site:page.front
  5. system.site:weight_select_max

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

Data model changes

More validation 🚀

Release notes snippet

None.

CommentFileSizeAuthor
#12 3443432-nr-bot.txt9.12 KBneeds-review-queue-bot

Issue fork drupal-3443432

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

narendraR created an issue. See original summary.

narendrar’s picture

Version: 10.3.x-dev » 11.x-dev

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

mikelutz’s picture

Status: Active » Needs review
mikelutz’s picture

Status: Needs review » Needs work
carsoncho’s picture

This all looks correct to me. Config inspector is reporting this at 100% now. Is the reason this is back to Needs work because of the pipelines failing?

narendrar’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

So applied the MR locally.

Update hook ran just fine

I'm still able to update the settings without issue

The proposed schema changes believe make sense.

+1 from me.

quietone’s picture

Status: Reviewed & tested by the community » Needs work

I read the IS and the MR. There are new methods in the MR that should be documented.

carsoncho’s picture

Status: Needs work » Needs review

I've added docblock info for those new methods. Ready for review.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new9.12 KB

The 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.

mikelutz’s picture

Status: Needs work » Needs review

Fixed coding standards and rebased to see if the validation checker can resolve updated dependencies.

smustgrave’s picture

Left 1 question on the MR.

narendrar’s picture

Feedback addressed.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Believe feedback has been addressed.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs followup

I've added some comments to the MR and 1 that I think should have a follow up created for.

borisson_’s picture

kunal.sachdev’s picture

Status: Needs review » Reviewed & tested by the community

Looks good. Just have one minor doubt.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

removing the UUID constraint feels like a step in the wrong direction. NULL is never really a valid value for site UUID.

phenaproxima’s picture

Thanks, @narendraR; that makes sense.

However, I agree with @alexpott that NULL is never valid for the site UUID, and so it shouldn't be nullable.

alexpott’s picture

I think this issue points to us needing a new constraint which is something like NotBlankAfterInstall - so a very can be blank during site install but once the site is up and running it can not be. This is true for uuid, name and email.

I also think that none of these should be nullable. PHP 8 makes swapping between NULLs and strings hard.

narendrar’s picture

@alexpott, I tried to create a new constraint as suggested in #23. But I don't understand how to bypass existingNotNullConstraint for UUID. https://git.drupalcode.org/project/drupal/-/merge_requests/7723/diffs?commit_id=4fcb5099b2b23f22643948c901321db04b89b0db#4d658554b6066a85600dfbe03dc34c3003f8a3e1_0_32

borisson_’s picture

I don't understand having to bypass NotNullConstraint? I think we have to reimplement it in here?

We should probably start with a reroll here.

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

bbrala’s picture

Rebased thsi issue.

@alexpott

> I think this issue points to us needing a new constraint which is something like NotBlankAfterInstall - so a very can be blank during site install but once the site is up and running it can not be. This is true for uuid, name and email.

You say this, which is kinda the same problem as #3445976: Add validation constraints to contact.form.* comment #32. There it is a recipe which wants to use mail to replace a value, which is doesn't get since mail is not yet in config when installing from a recipe with the drupal command.

So there is also some sort of 'during install' issue, where during an install is appearantly possible to have invalid config.

Perhaps we even need to fix it ealier? Although i don't really feal like ignoring validation for config while installing is a good idea. Since it is pretty valuable, what if you are to install some sort of Drupal implementation (like a decoupled Drupal) and it comes with invalid config, would we want to ignore that?

wim leers’s picture

This will allow Experience Builder to remove a hack that #3503412: Allow Content Author to set site's homepage from navigator is adding.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.