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.

Issue fork drupal-3395613

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

borisson_ created an issue. See original summary.

borisson_’s picture

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

Prashant.c made their first commit to this issue’s fork.

prashant.c’s picture

Status: Active » Needs review
Issue tags: +ClaroContributionDay2023

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!

borisson_’s picture

Status: Needs review » Needs work

Should be in shortcut schema, see #2.

prashant.c’s picture

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

borisson_’s picture

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

borisson_’s picture

Actually, can we fix this the same way as #3402885: Theme third party settings should only contain settings for modules that are enabled?

-- edit:
Wait, that issue bring is in line with the state we currently have in HEAD for claro. I don't understand this.

prashant.c’s picture

okay then I guess, we can follow the same approach based on the conclusion in https://www.drupal.org/project/drupal/issues/3402885.

wim leers’s picture

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

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

svendecabooter’s picture

Status: Needs work » Postponed

This is postponed on #3416178: Add validation constraints to `type: theme_settings`, because (amongst other issues) the "favicon" property contains properties that are dependent on the value of "use_default".

svendecabooter’s picture

wim leers’s picture

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

@svendecabooter in #13: Yep, you're right — thanks!

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.

quietone’s picture

The Claro theme was approved for removal in #3576460: [policy, no patch] Deprecate and remove Claro.

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3576668: [meta] Tasks to deprecate Claro and the removal work in #3584638: [meta] Tasks to remove the Claro theme.

smustgrave’s picture

Project: Drupal core » Claro
Version: main » 3.0.x-dev
Component: Claro theme » Code