Some config do not inherit from config_object, so locale_system_set_config_langcodes() results in schema errors

Here's the failing test.

Let's try this patch. The test failed on the bot the same as for me, so that's good. Here's a patch with the test... may break everything... let's see.

Interesting. So that would be a better fix. It would be interesting to make a test profile that has every module enabled and see if this installation test would pass using that, after fixing Menu UI's config. That would smoke out any other config in config/install that doesn't inherit from config_object.

We should also document that at the top level you should always inherit from config_object. It's certainly something I wasn't aware of.

I'll take this on...

OK. As a first step towards making sure Core has all good config that inherits from config_base, I make a slightly different test -- only difference it that it uses a new testing profile that enables all the modules. Here's a patch for that.

Next I'll see about fixing it by
- patching the core config as suggested in #8 until the test passes
- also making sure the docs are updated (may be a .api.php topic needs an update, or maybe just d.o docs).

Setting to needs review so we can confirm the "test-only" patch fails with the same error reported above (it did locally).

Well that's interesting. Apparently menu_ui was the only problematic schema, and fixing its two config things to inherit from config_object made the test in #11 pass.

Just to make sure, I added all the config/install config items from the standard profile to the testing_everything profile, and ran the test again. Still passed. Here's a patch with the test, added schema for the "everything" profile, and the two lines of actual pass in menu_ui.schema.yml.

Also I checked and there are no specifics about mapping vs. config_object in the Config API topic on api.drupal.org, so I'll update the docs page under https://www.drupal.org/developing/api/8/configuration instead.

Hm... I wonder if in that existing test we could also assert that the top-level config objects either start with config_object or config_entity somehow?

Talked to berdir and Gabor in IRC. Let's try this patch:
- Test patch: Adds to SchemaCheckTrait a few lines that verify that top-level schema has a langcode element on it... Hopefully this is OK, I am assuming this method only gets called for top-level schema? If not it will definitely fail. Let's see. As a note, we'd ideally want to make sure it's either config_entity or config_object but that is not really possible. config_object just adds langcode, so that is a minimal test.
- The patch with test just fixes the one menu_ui schema element as noted in #14.

No interdiff, it would be almost the whole patch, and this patch is small. So let's see if this test finds the bad spot, doesn't break anything else, and if the patch fixes it.

whoops.
= vs. => in

        $errors[$error_key] = 'missing langcode';

I must not be awake yet. ;)

Well. So apparently when you do

    $this->schema = $typed_config->create($data_definition, $config_data);

it only generates schema for data that exists. And langcode (as with any part of config) is optional. So, this failed miserably.

Trying another tactic...

OK, now we're getting somewhere. The failing test did catch the errors that it was intended to catch. But it also located a few config schema entries in test modules whose top level was 'mapping' instead of 'config_object', which is why the with-test-22 patch also failed some tests.

So here is a new patch that fixes these as well.

Updating summary.

Doh! Right. The others in the patch all came up in the test results. Shouldn't have done that one.

Same patch without that one bit.

Because this test wasn't in place yet. This patch hasn't been committed.

I'm not sure how to fix this. Here's the schema, which is in statistics.schema.yml:

block.settings.statistics_popular_block:
  type: block_settings
  label: 'Popular content block settings'
  mapping:
    top_day_num:
...

And block_settings is defined in core/config/schema.yml:

block_settings:
  type: mapping
  label: 'Block settings'
  mapping:
    id:
      type: string
      label: 'ID'
...

So it looks to me as though block_settings is not meant to be a top-level type. But for some reason, the statistics module is putting this block's settings as a top-level schema object, and using block_settings as a top-level type. Not sure what to do here, maybe just add a 'langcode' element to this manually?

If you look at what the settings are for this block, they are really not translatable (they are all just numbers).

Thoughts?

Yeah, I agree, but I'm not sure how to fix it?

Do we know which commit this came from and can we go back and reopen that issue? And give a bop on the head to whoever did that, reviewed it, and/or committed it? :)

The other issue has been rolled back. Hitting retest.