Add validation constraints to core.entity_form_mode.*.*

Created on 21 May 2024, about 2 months ago
Updated 10 July 2024, 3 days ago

Problem/Motivation

Form modes have 3 property paths that are not yet validatable:

vendor/bin/drush config:inspect --filter-keys=core.entity_form_mode.user.register --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                                                                       
 ---------------------------------------------------------------- --------- ------------- ------ --------------------------------------------------------------------------------------------- 
  core.entity_form_mode.user.register                              Correct   79%           โœ…โ“   ValidKeys: '<infer>'                                                                         
   core.entity_form_mode.user.register:                            Correct   Validatable   โœ…โœ…   ValidKeys: '<infer>'                                                                         
   core.entity_form_mode.user.register:_core                       Correct   Validatable   โœ…โœ…   ValidKeys:                                                                                   
                                                                                                    - default_config_hash                                                                      
   core.entity_form_mode.user.register:_core.default_config_hash   Correct   Validatable   โœ…โœ…   NotNull: {  }                                                                                
                                                                                                  Regex: '/^[a-zA-Z0-9\-_]+$/'                                                                 
                                                                                                  Length: 43                                                                                   
                                                                                                  โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_form_mode.user.register:cache                       Correct   Validatable   โœ…โœ…   โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_form_mode.user.register:dependencies                Correct   Validatable   โœ…โœ…   ValidKeys: '<infer>'                                                                         
   core.entity_form_mode.user.register:dependencies.module         Correct   NOT           โœ…โ“   โŒ @todo Add validation constraints to ancestor type: config_dependencies                    
   core.entity_form_mode.user.register:dependencies.module.0       Correct   Validatable   โœ…โœ…   NotBlank: {  }                                                                               
                                                                                                  ExtensionName: {  }                                                                          
                                                                                                  ExtensionExists: module                                                                      
                                                                                                  โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_form_mode.user.register:description                 Correct   Validatable   โœ…โœ…   Regex:                                                                                       
                                                                                                    pattern: '/([^\PC\x09\x0a\x0d])/u'                                                         
                                                                                                    match: false                                                                               
                                                                                                    message: 'Text is not allowed to contain control characters, only visible characters.'     
                                                                                                  โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_form_mode.user.register:id                          Correct   NOT           โœ…โ“   โš ๏ธ  @todo Add validation constraints to config entity type: core.entity_form_mode.*.*        
   core.entity_form_mode.user.register:label                       Correct   Validatable   โœ…โœ…   Regex:                                                                                       
                                                                                                    pattern: '/([^\PC])/u'                                                                     
                                                                                                    match: false                                                                               
                                                                                                    message: 'Labels are not allowed to span multiple lines or contain control characters.'    
                                                                                                  NotBlank: {  }                                                                               
                                                                                                  โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_form_mode.user.register:langcode                    Correct   Validatable   โœ…โœ…   NotNull: {  }                                                                                
                                                                                                  Choice:                                                                                      
                                                                                                    callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'  
                                                                                                  โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_form_mode.user.register:status                      Correct   Validatable   โœ…โœ…   โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_form_mode.user.register:targetEntityType            Correct   NOT           โœ…โ“   โš ๏ธ  @todo Add validation constraints to config entity type: core.entity_form_mode.*.*        
   core.entity_form_mode.user.register:uuid                        Correct   Validatable   โœ…โœ…   Uuid: {  }                                                                                   
                                                                                                  โ†ฃ 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=core.entity_form_mode.user.register --detail --list-constraints

Proposed resolution

Add validation constraints to:

  1. core.entity_form_mode.*.*:dependencies.module
  2. core.entity_form_mode.*.*:id
  3. core.entity_form_mode.*.*:targetEntityType

This requires looking at the existing code and admin UI (if any) to understand which values could be considered valid. Eventually this needs to be reviewed by the relevant subsystem maintainer.

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

None.

Data model changes

More validation ๐Ÿš€

Release notes snippet

None.

๐Ÿ“Œ Task
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Baseย  โ†’

Last updated about 5 hours ago

Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @narendraR
  • Merge request !8135Config validation added โ†’ (Open) created by narendraR
  • Pipeline finished with Failed
    about 2 months ago
    Total: 700s
    #177826
  • Pipeline finished with Success
    about 2 months ago
    Total: 694s
    #178093
  • Pipeline finished with Success
    about 2 months ago
    Total: 567s
    #178982
  • Status changed to Needs review about 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • First commit to issue fork.
  • Status changed to RTBC about 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Applied some nitpicky typehint return voids

    Applied the MR to a 11.x install using standard profile
    Believe the only entity_form_mode is for core.entity_form_mode.user.register
    Ran updb and hook ran without issue

    Manually downloaded config_inspector to 11.x setup and confirmed seeing fully validatable now

  • Pipeline finished with Success
    about 2 months ago
    Total: 671s
    #179348
  • Status changed to Needs work about 2 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

    One issue with the presave hook on the MR - missing a deprecation notice.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 572s
    #180953
  • Pipeline finished with Failed
    about 2 months ago
    Total: 607s
    #183186
  • Pipeline finished with Failed
    about 2 months ago
    Total: 177s
    #183218
  • Pipeline finished with Success
    about 2 months ago
    Total: 562s
    #183223
  • Status changed to Needs review about 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to Needs work about 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Left some comments on MR.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 187s
    #183780
  • Pipeline finished with Success
    about 2 months ago
    Total: 626s
    #183819
  • Status changed to Needs review about 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to RTBC about 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Deprecation appears to be correct to me.

  • Status changed to Needs work about 2 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

    One small comment on the deprecation message. Hard to fit this sort of thing into the standard format.

  • Pipeline finished with Success
    about 2 months ago
    Total: 537s
    #184322
  • Status changed to Needs review about 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to RTBC about 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Message seems good

  • Status changed to Needs review about 2 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia larowlan ๐Ÿ‡ฆ๐Ÿ‡บ๐Ÿ.au GMT+10

    Left one question on the MR, great work here - bit more involved with the update hook!

  • Pipeline finished with Success
    about 1 month ago
    Total: 545s
    #184678
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India

    This issue is ready for re-review. I have added the return FALSE. Thanks

  • Status changed to Needs work about 1 month ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States phenaproxima Massachusetts

    Looks okay but I have a couple of questions here.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 634s
    #185834
  • Pipeline finished with Success
    about 1 month ago
    Total: 602s
    #185908
  • Pipeline finished with Success
    about 1 month ago
    Total: 543s
    #189663
  • Status changed to Needs review about 1 month ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to Needs work about 1 month ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States phenaproxima Massachusetts

    Only one tiny thing and then I think this looks good.

  • Pipeline finished with Canceled
    about 1 month ago
    Total: 102s
    #191374
  • Pipeline finished with Failed
    about 1 month ago
    Total: 548s
    #191375
  • Pipeline finished with Success
    about 1 month ago
    Total: 515s
    #191506
  • Status changed to Needs review about 1 month ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to RTBC about 1 month ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Applied MR on a standard install for 11.x with config inspector and getting all green checks.

    Looking at MR 8135 and from what I can tell all feedback has been addressed.

  • Pipeline finished with Success
    25 days ago
    Total: 507s
    #201781
  • Status changed to Needs work 16 days ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    If I go to admin/structure/display-modes/form/ and edit the user registration form and save the form mode with no description then the system_entity_form_mode_presave() will trigger a deprecation notice.

    We need to add something to the form to convert empty strings to NULLs... normally we'd say use #config_target but we've not worked out how to using #config_target like stuff with config entities yet.

  • Pipeline finished with Success
    10 days ago
    Total: 517s
    #214686
  • Status changed to Needs review 10 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to RTBC 9 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Tested the scenario in #21 has been addressed with https://git.drupalcode.org/project/drupal/-/merge_requests/8135/diffs?co...

  • Status changed to Needs review 9 days ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    I'm not sure that a NULL description is any better then an empty string. Do we think that this part of the change is actually worth it?

  • Status changed to Needs work 3 days ago
  • The 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.

  • Assigned to Wim Leers
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States phenaproxima Massachusetts

    I'm not sure that a NULL description is any better then an empty string. Do we think that this part of the change is actually worth it?

    I don't feel strongly one way or the other, so I'm assigning this to Wim for input. I suspect he has a clearer reason why NULL would be useful here.

Production build 0.69.0 2024