[PP-1] Add validation constraints to claro.settings

Created on 20 October 2023, about 1 year ago
Updated 29 January 2024, 11 months ago

Problem/Motivation

Claro's settings have 1 property paths that are not yet validatable:

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

 ----------------------------------------------------------- ------------- ------------------------------------------ 
  Key                                                         Validatable   Validation constraints                    
 ----------------------------------------------------------- ------------- ------------------------------------------ 
  claro.settings                                              75%           ValidKeys: '<infer>'                      
                                                                            RequiredKeys: '<infer>'                   
   claro.settings:                                            Validatable   ValidKeys: '<infer>'                      
                                                                            RequiredKeys: '<infer>'                   
   claro.settings:third_party_settings                        NOT           โš ๏ธ  @todo Add validation constraints here  
   claro.settings:third_party_settings.shortcut               Validatable   ValidKeys: '<infer>'                      
                                                                            RequiredKeys: '<infer>'                   
   claro.settings:third_party_settings.shortcut.module_link   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=olivero.settings --detail --list-constraints

Proposed resolution

Add validation constraints to:

  1. third_party_settings

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

  1. third_party_settings

User interface changes

None.

API changes

None.

Data model changes

More validation ๐Ÿš€

Release notes snippet

None.

๐Ÿ“Œ Task
Status

Postponed

Version

11.0 ๐Ÿ”ฅ

Component
Claroย  โ†’

Last updated about 10 hours ago

Created by

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

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

Merge Requests

Comments & Activities

  • Issue created by @borisson_
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium borisson_ Mechelen, ๐Ÿ‡ง๐Ÿ‡ช

    The config should be updated at the source of the problem here, which is in shortcut.schema.yml

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia prashant.c Dharamshala

    Prashant.c โ†’ made their first commit to this issueโ€™s fork.

  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia prashant.c Dharamshala

    1. Added mapping for third_party_settings in claro.schema.yml
    2. Executed ddev drush config:inspect --filter-keys=claro.settings --detail --list-constraintsgave all keys validated.

    โžœ  ๐Ÿค– Analyzingโ€ฆ
    
     Legend for Data: 
      โœ…โ“  โ†’ Correct primitive type, detailed validation impossible.
      โœ…โœ…  โ†’ Correct primitive type, passed all validation constraints.
     ----------------------------------------------------------- --------- ------------- ------ ------------------------ 
      Key                                                         Status    Validatable   Data   Validation constraints  
     ----------------------------------------------------------- --------- ------------- ------ ------------------------ 
      claro.settings                                              Correct   100%          โœ…โœ…   ValidKeys: '<infer>'    
       claro.settings:                                            Correct   Validatable   โœ…โœ…   ValidKeys: '<infer>'    
       claro.settings:third_party_settings                        Correct   Validatable   โœ…โœ…   ValidKeys: '<infer>'    
       claro.settings:third_party_settings.shortcut               Correct   Validatable   โœ…โœ…   ValidKeys: '<infer>'    
       claro.settings:third_party_settings.shortcut.module_link   Correct   Validatable   โœ…โœ…   โ†ฃ PrimitiveType: {  }   
     ----------------------------------------------------------- --------- ------------- ------ ------------------------ 

    Please review.

    Thanks!

  • Pipeline finished with Success
    about 1 year ago
    Total: 625s
    #64259
  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium borisson_ Mechelen, ๐Ÿ‡ง๐Ÿ‡ช

    Should be in shortcut schema, see #2.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia prashant.c Dharamshala

    But shortcut.schema.yml already has this section:

    theme_settings.third_party.shortcut:
      type: mapping
      label: 'Shortcut settings'
      mapping:
        module_link:
          type: boolean
          label: 'Add shortcut link'

    Could you please guide here?

    Thanks

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

    I looked into this, I see that olivero does this in the same way. I'm not sure how we can tell the third party settings to apply to this.

    Since I'm not sure how to proceed here, I asked in slack: https://drupal.slack.com/archives/C2THUBAVA/p1702728107642189

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

    Actually, this should be fixed in the same way as ๐Ÿ› [PP-1] olivero.settings config schema is wrong Postponed

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia prashant.c Dharamshala

    okay then I guess, we can follow the same approach based on the conclusion in https://www.drupal.org/project/drupal/issues/3402885 ๐Ÿ› [PP-1] olivero.settings config schema is wrong Postponed .

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    This now needs the FullyValidatable constraint to be added to claro.settings. See https://www.drupal.org/node/3404425 โ†’ .

  • First commit to issue fork.
  • Pipeline finished with Failed
    11 months ago
    Total: 641s
    #83156
  • Pipeline finished with Failed
    11 months ago
    #83175
  • Pipeline finished with Failed
    11 months ago
    Total: 580s
    #83267
  • Status changed to Postponed 11 months ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium svendecabooter Gent

    This is postponed on https://www.drupal.org/project/drupal/issues/3416178 ๐Ÿ“Œ Add validation constraints to `type: theme_settings` Active , because (amongst other issues) the "favicon" property contains properties that are dependent on the value of "use_default".

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium svendecabooter Gent
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    @svendecabooter in #13: Yep, you're right โ€” thanks!

Production build 0.71.5 2024