Add validation constraints to core.entity_view_mode.*.*

Created on 3 May 2024, about 2 months ago
Updated 19 June 2024, 9 days ago

Problem/Motivation

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

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

 --------------------------------------------------------------------- ------------- --------------------------------------------------------------------------------------------- 
  Key                                                                   Validatable   Validation constraints                                                                       
 --------------------------------------------------------------------- ------------- --------------------------------------------------------------------------------------------- 
  core.entity_view_mode.taxonomy_term.full                              79%           ValidKeys: '<infer>'                                                                         
   core.entity_view_mode.taxonomy_term.full:                            Validatable   ValidKeys: '<infer>'                                                                         
   core.entity_view_mode.taxonomy_term.full:_core                       Validatable   ValidKeys:                                                                                   
                                                                                        - default_config_hash                                                                      
   core.entity_view_mode.taxonomy_term.full:_core.default_config_hash   Validatable   NotNull: {  }                                                                                
                                                                                      Regex: '/^[a-zA-Z0-9\-_]+$/'                                                                 
                                                                                      Length: 43                                                                                   
                                                                                      โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_view_mode.taxonomy_term.full:cache                       Validatable   โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_view_mode.taxonomy_term.full:dependencies                Validatable   ValidKeys: '<infer>'                                                                         
   core.entity_view_mode.taxonomy_term.full:dependencies.module         NOT           โŒ @todo Add validation constraints to ancestor type: config_dependencies                    
   core.entity_view_mode.taxonomy_term.full:dependencies.module.0       Validatable   NotBlank: {  }                                                                               
                                                                                      ExtensionName: {  }                                                                          
                                                                                      ExtensionExists: module                                                                      
                                                                                      โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_view_mode.taxonomy_term.full:description                 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_view_mode.taxonomy_term.full:id                          NOT           โš ๏ธ  @todo Add validation constraints to config entity type: core.entity_view_mode.*.*        
   core.entity_view_mode.taxonomy_term.full:label                       Validatable   Regex:                                                                                       
                                                                                        pattern: '/([^\PC])/u'                                                                     
                                                                                        match: false                                                                               
                                                                                        message: 'Labels are not allowed to span multiple lines or contain control characters.'    
                                                                                      NotBlank: {  }                                                                               
                                                                                      โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_view_mode.taxonomy_term.full:langcode                    Validatable   NotNull: {  }                                                                                
                                                                                      Choice:                                                                                      
                                                                                        callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'  
                                                                                      โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_view_mode.taxonomy_term.full:status                      Validatable   โ†ฃ PrimitiveType: {  }                                                                        
   core.entity_view_mode.taxonomy_term.full:targetEntityType            NOT           โš ๏ธ  @todo Add validation constraints to config entity type: core.entity_view_mode.*.*        
   core.entity_view_mode.taxonomy_term.full:uuid                        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_view_mode.taxonomy_term.full --detail --list-constraints

Proposed resolution

Add validation constraints to:

  1. core.entity_view_mode.*.*:dependencies.module
  2. core.entity_view_mode.*.*:id
  3. core.entity_view_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 3 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 !7906Initial validation โ†’ (Open) created by narendraR
  • Pipeline finished with Failed
    about 2 months ago
    Total: 512s
    #163725
  • Pipeline finished with Canceled
    about 2 months ago
    Total: 420s
    #165406
  • Pipeline finished with Failed
    about 2 months ago
    Total: 480s
    #165419
  • Pipeline finished with Failed
    about 2 months ago
    Total: 501s
    #165455
  • Pipeline finished with Failed
    about 2 months ago
    Total: 487s
    #166087
  • Pipeline finished with Failed
    about 2 months ago
    Total: 492s
    #166115
  • First commit to issue fork.
  • Pipeline finished with Failed
    about 2 months ago
    Total: 538s
    #166659
  • Pipeline finished with Success
    about 2 months ago
    Total: 629s
    #167071
  • Status changed to Needs review about 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to Needs work about 2 months ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Nice progress here! ๐Ÿ˜„ Still needs some work, but for each of the things that still need to be addressed, there's prior art/patterns to look at ๐Ÿ˜Š

  • Assigned to carsoncho
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States carsoncho
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States carsoncho
  • Pipeline finished with Failed
    about 2 months ago
    Total: 569s
    #167874
  • Issue was unassigned.
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States carsoncho
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States carsoncho
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States carsoncho
  • Pipeline finished with Failed
    about 2 months ago
    Total: 176s
    #168139
  • Pipeline finished with Failed
    about 2 months ago
    #168230
  • Pipeline finished with Failed
    about 2 months ago
    Total: 206s
    #168233
  • Pipeline finished with Failed
    about 2 months ago
    Total: 233s
    #168249
  • Pipeline finished with Failed
    about 2 months ago
    Total: 167s
    #169897
  • Pipeline finished with Failed
    about 2 months ago
    Total: 578s
    #169927
  • Pipeline finished with Failed
    about 2 months ago
    Total: 784s
    #171712
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States carsoncho

    What I think is remaining is the tests updates. I see there's a few failing due to configuration tests as it's noting that the core.entity_view_mode.* config files have been changed and updated. This is to be expected given the hook_post_update_NAME() being included here.

    Drupal\FunctionalTests\Installer\InstallerExistingConfigMultilingualTest::testConfigSync
    Failed asserting that two arrays are equal.
    --- Expected
    +++ Actual
    @@ @@
     Array (
         'create' => []
         'update' => Array (
    -        0 => 'system.mail'
    +        0 => 'core.entity_view_mode.node.teaser'
    +        1 => 'core.entity_view_mode.node.se...result'
    +        2 => 'core.entity_view_mode.node.se..._index'
    +        3 => 'core.entity_view_mode.node.rss'
    +        4 => 'core.entity_view_mode.node.full'
    +        5 => 'system.mail'
    +        6 => 'core.entity_view_mode.user.full'
    +        7 => 'core.entity_view_mode.user.compact'
         )
    

    Is the right thing to do here update the tests so they expect all those view mode configuration changes? Testing is an area I'd like to be able to contribute to more so any information folks can provide is much appreciated.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 878s
    #176887
  • Pipeline finished with Failed
    about 1 month ago
    Total: 959s
    #177104
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1028s
    #177302
  • Pipeline finished with Failed
    about 1 month ago
    Total: 997s
    #177356
  • Pipeline finished with Failed
    about 1 month ago
    Total: 877s
    #177372
  • Pipeline finished with Success
    about 1 month ago
    Total: 555s
    #177726
  • Status changed to Needs review about 1 month ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • First commit to issue fork.
  • Status changed to RTBC about 1 month ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Replied to the comment and applied 2 nitpicky things

    On a 11.x setup with standard install there are several view modes across entity types
    Applied the MR locally
    Update hook ran without issue

    Manually downloaded config inspector and am seeing fully validated entity_view_modes

  • Pipeline finished with Success
    about 1 month ago
    Total: 606s
    #179380
  • Status changed to Needs work about 1 month ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

    One note on the MR - missing a deprecation message. ๐Ÿ› Add proper deprecation notices in config entity presave bc layers Active has more details.

  • Pipeline finished with Success
    about 1 month ago
    Total: 508s
    #180892
  • Pipeline finished with Failed
    about 1 month ago
    Total: 508s
    #180948
  • Pipeline finished with Failed
    about 1 month ago
    Total: 655s
    #181453
  • Pipeline finished with Failed
    about 1 month ago
    Total: 2455s
    #183029
  • Pipeline finished with Failed
    about 1 month ago
    Total: 619s
    #183081
  • Pipeline finished with Success
    about 1 month ago
    Total: 590s
    #183099
  • Status changed to Needs review about 1 month ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Pipeline finished with Failed
    about 1 month ago
    #183782
  • Status changed to Needs work about 1 month ago
  • 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.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 179s
    #183803
  • Pipeline finished with Success
    about 1 month ago
    Total: 559s
    #183817
  • Status changed to Needs review about 1 month ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Pipeline finished with Success
    about 1 month ago
    Total: 576s
    #184326
  • Pipeline finished with Success
    30 days ago
    Total: 621s
    #184680
  • Status changed to Needs work 25 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Pipeline finished with Success
    23 days ago
    Total: 633s
    #191547
  • Status changed to Needs review 23 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India
  • Status changed to Needs work 17 days ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Thanks for your continued contributions in the config validation space, @carsoncho! ๐Ÿ™๐Ÿ˜Š

    A few nits, and one question about a comment that seems wrong.

    The MR itself looks great! ๐Ÿ‘

  • Pipeline finished with Canceled
    16 days ago
    Total: 184s
    #196895
  • Pipeline finished with Success
    16 days ago
    Total: 540s
    #196897
  • Status changed to Needs review 16 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India

    Updating description to 'description' => NULL is giving depreciation error. Other feedback addressed.

  • Pipeline finished with Success
    10 days ago
    Total: 504s
    #201739
  • Pipeline finished with Success
    10 days ago
    Total: 542s
    #201749
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India

    Feedback addressed.

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

    Updating description to 'description' => NULL is giving depreciation error.

    The deprecation error you're seeing is literally the one this MR is adding.

    Why does that deprecation exist here? node_node_type_presave() doesn't do that (introduced in ๐Ÿ“Œ Allow vocabularies to be validated via the API, not just during form submissions RTBC .) Ah โ€ฆ because @catch asked for that.

    Well, now we've got ourselves a chicken-egg situation. ๐Ÿ˜…

    The only way I see out of this is to not rely on the presave hook to perform the update, but instead to duplicate that logic into the update hook.

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

    Re #25, Does it mean removing system_entity_view_mode_presave() from system.module and doing $view_mode->set('description', NULL)->save(); in system_post_update_convert_empty_string_entity_view_modes_to_null of system.post_update.php. Also where should we use @trigger_error in this case or it can be avoided?

    When I did as above it gives Schema errors for core.entity_view_mode.node.teaser with the following errors: 0 [description] This value should not be blank.

Production build 0.69.0 2024