Add validation constraints to user.role.*

Created on 4 May 2024, 2 months ago
Updated 2 July 2024, 11 days ago

Problem/Motivation

User role entity is not yet fully validatable:

./vendor/bin/drush config:inspect --filter-keys=user.role.anonymous --detail --list-constraints --fields=key,validatability,constraints    
โžœ  ๐Ÿค– Analyzingโ€ฆ

 ------------------------------------------------ ------------- --------------------------------------------------------------------------------------------- 
  Key                                              Validatable   Validation constraints                                                                       
 ------------------------------------------------ ------------- --------------------------------------------------------------------------------------------- 
  user.role.anonymous                              66%           ValidKeys: '<infer>'                                                                         
   user.role.anonymous:                            Validatable   ValidKeys: '<infer>'                                                                         
   user.role.anonymous:_core                       Validatable   ValidKeys:                                                                                   
                                                                   - default_config_hash                                                                      
   user.role.anonymous:_core.default_config_hash   Validatable   NotNull: {  }                                                                                
                                                                 Regex: '/^[a-zA-Z0-9\-_]+$/'                                                                 
                                                                 Length: 43                                                                                   
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:dependencies                Validatable   ValidKeys: '<infer>'                                                                         
   user.role.anonymous:dependencies.config         NOT           โŒ @todo Add validation constraints to ancestor type: config_dependencies                    
   user.role.anonymous:dependencies.config.0       Validatable   NotBlank: {  }                                                                               
                                                                 ConfigExists: {  }                                                                           
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:dependencies.module         NOT           โŒ @todo Add validation constraints to ancestor type: config_dependencies                    
   user.role.anonymous:dependencies.module.0       Validatable   NotBlank: {  }                                                                               
                                                                 ExtensionName: {  }                                                                          
                                                                 ExtensionExists: module                                                                      
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:dependencies.module.1       Validatable   NotBlank: {  }                                                                               
                                                                 ExtensionName: {  }                                                                          
                                                                 ExtensionExists: module                                                                      
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:dependencies.module.2       Validatable   NotBlank: {  }                                                                               
                                                                 ExtensionName: {  }                                                                          
                                                                 ExtensionExists: module                                                                      
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:dependencies.module.3       Validatable   NotBlank: {  }                                                                               
                                                                 ExtensionName: {  }                                                                          
                                                                 ExtensionExists: module                                                                      
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:dependencies.module.4       Validatable   NotBlank: {  }                                                                               
                                                                 ExtensionName: {  }                                                                          
                                                                 ExtensionExists: module                                                                      
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:dependencies.module.5       Validatable   NotBlank: {  }                                                                               
                                                                 ExtensionName: {  }                                                                          
                                                                 ExtensionExists: module                                                                      
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:dependencies.module.6       Validatable   NotBlank: {  }                                                                               
                                                                 ExtensionName: {  }                                                                          
                                                                 ExtensionExists: module                                                                      
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:id                          Validatable   Regex:                                                                                       
                                                                   pattern: '/^[a-z0-9_]+$/'                                                                  
                                                                   message: 'The %value machine name is not valid.'                                           
                                                                 Length:                                                                                      
                                                                   max: 166                                                                                   
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:is_admin                    Validatable   โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:label                       Validatable   Regex:                                                                                       
                                                                   pattern: '/([^\PC])/u'                                                                     
                                                                   match: false                                                                               
                                                                   message: 'Labels are not allowed to span multiple lines or contain control characters.'    
                                                                 NotBlank: {  }                                                                               
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:langcode                    Validatable   NotNull: {  }                                                                                
                                                                 Choice:                                                                                      
                                                                   callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'  
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:permissions                 NOT           โš ๏ธ  @todo Add validation constraints to config entity type: user.role.*                      
   user.role.anonymous:permissions.0               NOT           โš ๏ธ  @todo Add validation constraints to config entity type: user.role.*                      
   user.role.anonymous:permissions.1               NOT           โš ๏ธ  @todo Add validation constraints to config entity type: user.role.*                      
   user.role.anonymous:permissions.2               NOT           โš ๏ธ  @todo Add validation constraints to config entity type: user.role.*                      
   user.role.anonymous:permissions.3               NOT           โš ๏ธ  @todo Add validation constraints to config entity type: user.role.*                      
   user.role.anonymous:permissions.4               NOT           โš ๏ธ  @todo Add validation constraints to config entity type: user.role.*                      
   user.role.anonymous:permissions.5               NOT           โš ๏ธ  @todo Add validation constraints to config entity type: user.role.*                      
   user.role.anonymous:permissions.6               NOT           โš ๏ธ  @todo Add validation constraints to config entity type: user.role.*                      
   user.role.anonymous:status                      Validatable   โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:uuid                        Validatable   Uuid: {  }                                                                                   
                                                                 โ†ฃ PrimitiveType: {  }                                                                        
   user.role.anonymous:weight                      Validatable   Range:                                                                                       
                                                                   min: -2147483648                                                                           
                                                                   max: 2147483647                                                                            
                                                                 FullyValidatable: null                                                                       
                                                                 โ†ฃ 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=user.role.anonymous --detail --list-constraints

Proposed resolution

Add validation constraints to missing properties.

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

Fixed

Version

11.0 ๐Ÿ”ฅ

Component
User moduleย  โ†’

Last updated about 21 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 !7910User role validation โ†’ (Open) created by narendraR
  • Pipeline finished with Failed
    2 months ago
    Total: 737s
    #164071
  • Pipeline finished with Failed
    2 months ago
    Total: 593s
    #164612
  • Pipeline finished with Failed
    2 months ago
    Total: 488s
    #164650
  • Pipeline finished with Failed
    2 months ago
    Total: 504s
    #165321
  • Pipeline finished with Failed
    2 months ago
    Total: 600s
    #165437
  • Pipeline finished with Success
    2 months ago
    Total: 498s
    #165470
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to RTBC 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Using configuration inspector on a standard install I see

    So believe this one is good.

  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    I don't see any justification for making is_admin and weight optional values? I kinda get weight being optional โ€ฆ but not is_admin.

    Plus, if they're optional, then that should be reflected on the class properties too, and that's not the case:

      /**
       * The weight of this role in administrative listings.
       *
       * @var int
       */
      protected $weight;
    

    +

      /**
       * An indicator whether the role has all permissions.
       *
       * @var bool
       */
      protected $is_admin;
    
  • First commit to issue fork.
  • Pipeline finished with Failed
    2 months ago
    Total: 575s
    #168986
  • First commit to issue fork.
  • Pipeline finished with Failed
    2 months ago
    Total: 643s
    #169163
  • Pipeline finished with Success
    2 months ago
    #169201
  • Pipeline finished with Success
    2 months ago
    Total: 575s
    #169225
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States mtift Minnesota, USA
  • Status changed to Needs work about 2 months ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • Status changed to Needs review about 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States mtift Minnesota, USA
  • Status changed to Needs work about 2 months ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • Status changed to Needs review about 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States mikelutz Michigan, USA

    Done.

  • Pipeline finished with Success
    about 2 months ago
    Total: 688s
    #184365
  • Status changed to RTBC about 1 month ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Feedback appears to be addressed.

  • Status changed to Needs work about 1 month ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    Added some review comments. I'm not sure about the nullability being added here.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States mtift Minnesota, USA

    @alexpott I don't see any comments. Did you add them on https://git.drupalcode.org/project/drupal/-/merge_requests/7910?

  • Pipeline finished with Failed
    about 1 month ago
    Total: 612s
    #197288
  • Status changed to Needs review about 1 month ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States mtift Minnesota, USA
  • Status changed to Needs work 18 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    MR appears to have a large number of failures.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium borisson_ Mechelen, ๐Ÿ‡ง๐Ÿ‡ช

    Looks like a lot of the current failures are because of invalid configuration in tests.

  • Status changed to Needs review 17 days ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium borisson_ Mechelen, ๐Ÿ‡ง๐Ÿ‡ช

    Discussed with @alexpott at drupal dev days, setting default values on the entity.

  • Pipeline finished with Failed
    17 days ago
    Total: 493s
    #208590
  • Pipeline finished with Failed
    17 days ago
    Total: 553s
    #208602
  • Status changed to Needs work 17 days ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium borisson_ Mechelen, ๐Ÿ‡ง๐Ÿ‡ช

    The remaining failures are all in rest, but I don't understand how to fix them.

  • Pipeline finished with Failed
    17 days ago
    Total: 605s
    #208696
  • Pipeline finished with Failed
    17 days ago
    Total: 485s
    #208717
  • Pipeline finished with Success
    16 days ago
    Total: 513s
    #209480
  • Status changed to Needs review 16 days ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium borisson_ Mechelen, ๐Ÿ‡ง๐Ÿ‡ช

    Hurrah, it's green.

  • Status changed to RTBC 11 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Feedback appears to be addressed for this one.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    Committed f4ae13d and pushed to 11.x. Thanks!

  • Status changed to Fixed 11 days ago
    • alexpott โ†’ committed f4ae13d0 on 11.x
      Issue #3445215 by narendraR, borisson_, mtift, mikelutz, smustgrave, Wim...
Production build 0.69.0 2024