olivero.settings config schema is wrong

Surprised that removing that cause all those failures. Is it a dependency thing where those other schemas aren't getting loaded?

Is it a dependency thing where those other schemas aren't getting loaded?

Quite possibly the Shortcut module must be installed in all those failing tests.

I see lots of migration test failures. 90% likely that the migration is wrong (i.e. assumes the Shortcut module is installed, which is not guaranteed).

Ah good find!

What about existing sites? Do we need to clean up if Olivero is installed but Shortcut is not?

You are correct. Did a standard install, uninstalled shortcut, config export and still see that third_party_setting in the yml file.

_core:
  default_config_hash: 1TswGK46jyu77aIM7Z-0JVQs5bxHmo-gtgrvrQGMXxc
favicon:
  use_default: true
features:
  comment_user_picture: true
  comment_user_verification: true
  favicon: true
  node_user_picture: false
logo:
  use_default: false
third_party_settings:
  shortcut:
    module_link: true
mobile_menu_all_widths: 0
site_branding_bg_color: default
base_primary_color: '#1b9ae4'

Most sites don't need an update path, for two reasons:

1. 99% of sites have the Shortcut module installed, which means they'll have type: theme_settings.third_party.shortcut, and hence their config is valid
2. We should keep 100% of the sites that have Shortcut installed working the exact same way in Olivero, which means: not modifying their configuration

That means we need to write an update path for the remaining 1% of sites that use Olivero but have the Shortcut module uninstalled, because in those cases, the third party settings in olivero.settings are meaningless noise.

(Sorry for maybe stating the obvious, but I was thinking it through so might as well write it up.)

olivero_post_update_add_olivero_primary_color() and OliveroPostUpdateTest already exist and provide a decent example :) At the start of the post-update function, we'd need something like:

  $module_handler = \Drupal::moduleHandler();
  if ($module_handler->moduleExists('shortcut')) {
    // No config to update.
    return;
  }

(example of that: help_post_update_help_topics_search())

Postponing this on 📌 [PP-1] Add validation constraints to olivero.settings Postponed , which adds more validation.

This bug is a bit annoying.

Contrib (e.g. Gin) is now copying this bug into their own codebase, just to make Config inspector shut up. And this again breaks theme 3rd party settings for everybody else :(

Issue itr was postponed on was fixed, this can actually be worked on.

Added an update hook and test, also tested it locally on a clean install.

So tested this one with 2 separate sites, 1 I had shortcuts installed the other I did not

First site
> Shortcut remained

Second site
> Shortcut was removed

Problem though, on the site that remained the validation went down because the third_party_settings schema is gone.

What was the message there?

The validation should still be OK, since:

olivero.settings:
  type: theme_settings

Which means core.data_types.schema.yml

theme_settings:
  type: config_object
  mapping:
...
    third_party_settings:
      # Third party settings are always optional: they're an optional extension
      # point.
      requiredKey: false
      type: sequence
      label: 'Third party settings'
      sequence:
        type: theme_settings.third_party.[%key]

Which then should resolve into:

shortcut.schema.yml

# Schema for theme settings.
theme_settings.third_party.shortcut:
  type: mapping
  label: 'Shortcut settings'
  mapping:
    module_link:
      type: boolean
      label: 'Add shortcut link'

I also tried to proove the issue in the test, but seems fine. I'm confused.

Don't have that branch up right now but validation had gone down for me because third_party_settings was no longer validated

Can you help me reproduce locally?

So I apply the MR and run the update when I go to olivero.settings

I see that the percentage went down to 94% and

Ahh, there.

Hmm, ok, seems weird but can look into that. I'd assume that is validated since it is not overwritten for the theme_settings type.

Thanks, I cab work with this.

I should confess i first read your message as the validation crashed, but you mean it is not 100% :)

This goed a little deeper. I dont see any third_party_settings validated in other config, only the actualyl settings when set, so this seems expected. There is also;

🐛 third_party_settings in THEME.settings.yml cannot be dependency managed Needs work
✨ 3rd party should be allowed to add config dependencies Active

Which expose some issues around third part settings. So i don't think we should block on the fact the parent key is not validated.

Alright that was the only issue I found