Split sitemap_book config schema from sitemap schema

I was using the Configuration Inspector module → to help me debug whether my config schema changes were having any effect.

Here are some things that I tried adding to a modules/sitemap_book/config/schema/sitemap_book.schema.yml that ultimately didn't resolve the "missing config schema [...] for show_expanded" error I was seeing in the config inspector.

This is based on the config in config/schema/sitemap.schema.yml at commit 551a37b

Note that in my test run, node/3 was the book, so the exact path of the config that I was trying to define the schema for was sitemap.settings:plugins.book:3.settings.show_expanded

1. sitemap.settings.plugins.book.*.settings.show_expanded:
     type: boolean
     label: 'Show expanded'
     nullable: true
   

2. plugins.book:*.settings.show_expanded:
     type: boolean
     label: 'Show expanded'
     nullable: true
   

3. sitemap.settings.plugins.book.*.settings:
     type: sitemap.settings.plugins.[%parent.settings]
     mapping:
       show_expanded:
         type: boolean
         label: 'Show expanded'
         nullable: true
   

4. sitemap.settings.plugins.book:
     type: mapping
     mapping:
       settings:
         type: mapping
         label: 'Settings'
         mapping:
           show_expanded:
             type: boolean
             label: 'Show expanded'
             nullable: true
   

5. sitemap.settings.plugins.*.settings:
     type: sitemap.plugin.settings
     label: 'Sitemap plugin'
     mapping:
       enabled:
         type: boolean
         label: 'Enabled'
       weight:
         type: integer
         label: 'Weight'
       settings:
         type: mapping
         label: 'Settings'
         mapping:
           # Global plugin settings
           title:
             type: label
             label: 'Title'
           # Book plugin settings
           show_expanded:
             type: boolean
             label: 'Show expanded'
             nullable: true
       id:
         type: string
         label: 'Plugin ID'
       provider:
         type: string
         label: 'Provider name'
   

I re-discovered ✨ Settings form takes forever to load when there are many books Needs review , which implements modules/sitemap_book/config/schema/sitemap_book.schema.yml as...

sitemap.settings.plugins.*.settings.show_expanded:
  type: boolean
  label: 'Show expanded'
  nullable: true 

... so lets try that!

Apparently config_inspector doesn't like this either... it says...

Name: plugins.book:6.settings.show_expanded
Label: Undefined
Type: undefined
Validatable: No
Value: 1
Error: missing schema
Validation error: 'show_expanded' is not a supported key.

... so back to "Needs work".

If anyone reading this has any suggestions that haven't already been tried, I'd appreciate some help!

Gotta keep 'em separated.

Give this a shot. I developed this new schema in an active dev site and unfortunately problems with Search API's schema is completely breaking the Config Inspector. The WSoD is preventing me from doing that check. But the tests are passing here and on my local.

There's too much repetition. I had to copy the title setting to every plugin schema. I tried making a base schema for settings, but it didn't work. I have theories as to why, but knowing why wouldn't necessarily solve the problem. In the end, what we really need to do this properly is for the plugins to store their type. Then I think it should be possible to do something like:

# Base schema for sitemap plugins.
sitemap_plugin:
  type: mapping
  label: 'Plugin'
  mapping:
    enabled:
      type: boolean
      label: 'Enabled'
    weight:
      type: integer
      label: 'Weight'
    settings:
      type: sitemap_plugin_settings.[%parent.type]
    id:
      type: string
      label: 'Plugin ID'
    provider:
      type: string
      label: 'Provider name'
    type:
      type: string
      label: 'Plugin type'

sitemap_plugin_settings_base:
  type:
    mapping:
    label: 'Plugin settings'
    mapping:
      title:
        type: label
        label: 'Title'

sitemap_plugin_settings.frontpage
  type: sitemap_plugin_settings_base
  mapping:
    rss:
      type: path
      label: "RSS"
      nullable: true

...

But I wouldn't worry about that right now since you're gearing up for a release. If my MR works, then I'd commit it, ship it, and open a follow-up for fixing the repetition. Because doing that will require DB updates to add the new type key.

@dcam, thank you very much! I'll take a look right now!

Hmm... unfortunately, I still get the error, even on the new branch (see attached screenshot).

I spun up a fresh dev site. I'm getting the same error in the config inspector. I'll try to diagnose the problem.

Easy fixes. I forgot to make the book schema dynamic and I still had a reference to that settings base schema in there.

Since you're waiting to put out an RC I can try to make the adjustments that I mentioned above before you commit this.

If you try to test this latest schema, then you'll get an error. You have to re-save your configuration first in order to insert the base_plugin key. This isn't ready yet because an update path is necessary.

@dcam, Awesome!! I can confirm that your latest changes fixed the error on my end!

I'll close my older merge request (!32); and leave your MR (!35) open so you can write the update path: just let me know when it's ready for final review and merging.

Thank you very much again!

mparker17 → changed the visibility of the branch 3465577-split-sitemapbook-config-schema to hidden.

mparker17 → closed merge request !32

@mparker17 I'm guessing that a lot of the nullable keys were the result of all plugin settings being lumped into a single array. You had to do that so one plugin could leave all the other settings as empty. Do you think you could go through them in the MR and remove the key from all the settings that really should be filled out?

Actually, that may be a bad idea. What we're doing so far hasn't been very disruptive. But making something that can currently be NULL not nullable might have unintended consequences. I think it would be better to do that in a follow-up issue.

There you go. Please review it.

I had to add a new database fixture in order to test the config update. It should be suitable for testing any future config changes after schema version 8202.

@dcam, this looks great to me, automated tests pass (including the new one), and manual tests work too.

I have a question (which I left in the merge request but Drupal.org has synced above), but there's nothing to be changed yet.

Marking this as RTBC since I think this is ready to merge once the question has been answered (and if the answer to the question is "let's change something", then I anticipate the change will be small and easy to review)

Thank you VERY much!

Bumping in case you don't get notified of the response in GitLab.

To prove my point I started to add a test for 8201 in the web IDE. Even with editing the fixture file to remove the path setting I was finished in a few minutes. But then I realized that this change may have further implications for the fixture since there's routing information in there. To be on the safe side, a new fixture would have to be generated for version 8200. So I backed off.

Merging! Thank you very much @dcam!

No problem. Glad I could help.

This fix has been released in sitemap-8.x-2.0-beta9 → .

Automatically closed - issue fixed for 2 weeks with no activity.