Mark layout_builder.settings fully validatable

Seemed to cause a test failure

1)
    Drupal\Tests\layout_builder\Functional\SettingsFormTest::testSettingsForm
    Behat\Mink\Exception\ResponseTextException: The text "The configuration
    options have been saved" was not found anywhere in the text of the current
    page.
    
    /builds/issue/drupal-3426429/vendor/behat/mink/src/WebAssert.php:907
    /builds/issue/drupal-3426429/vendor/behat/mink/src/WebAssert.php:293
    /builds/issue/drupal-3426429/core/tests/Drupal/Tests/WebAssert.php:956
    /builds/issue/drupal-3426429/core/modules/layout_builder/tests/src/Functional/SettingsFormTest.php:43
    /builds/issue/drupal-3426429/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

Fixed. The root cause is a bug that exists for many default simple config files in core. We're fixing them one-by-one (see 🌱 [meta] Add constraints to all simple configuration Active ).

Ah this helps in a few modules.

Does this mean all config needs a landcode?

Does it need to be set in the form?

Does this mean all config needs a landcode?

Yes. As dictated by config schema:

config_object:
  type: mapping
  mapping:
    _core:
      # This only exists for merging configuration; it's not required.
      requiredKey: false
      type: _core_config_info
    langcode:
      type: langcode

— core.data_types.schema.yml.

The only reasons it hasn't been noticed before: 1) we're not validating our config, 2) our existing basic config schema checking logic at \Drupal\Core\Config\Schema\SchemaCheckTrait::checkConfigSchema() adds a langcode if it's missing 🫣

I actually didn't know that and you just answered my slack question.

Change looks good. Since this feature is on only in 11.x and 10.3.x that hasn't released yet double we need an update hook to add the langcode key.

I don't think that langcode should be added here. There's nothing translatable - it's meaningless.

I think we should make langcode optional and only require it if there is translatable config in the configuration. Interestingly if the default language is something other then 'en' then the locale module (I think) will end adding langcode (or changing) to every config file. Complex issue.

That makes sense, curious what the solution will be as the missing langcode seems to be the #1 error across multiple config from config inspector.

Quoting @alexpott from Drupal Slack:

FWIW this is going to lead to something great
Maybe…
Because the real rule around langcode is it is required when the config has translatable data in it.
And maybe we can write a constraint for that

💯

Pushed a PoC commit that implements exactly that, and even trigggers a deprecation error when langcode is inappropriately present! 🤓 Looks like that alone could be hugely disruptive though, so probably wise to accept langcode even when it is not strictly necessary.

If you step through it, you'll notice that for e.g. system.site it does say it's translatable, but for system.mail it says it's not. Which is correct.

Will extract this into a new issue, but would appreciate a +1 for overall direction 😇

@alexpott reacted enthusiastically to my PoC commit from #13.

Extracted as promised: 🐛 Require `langcode: …` only for simple config that contains translatable values Active .

This issue is now blocked on that one.

Flagging that layout_builder.settings will be removed as part of 📌 Replace "Expose all fields as blocks to Layout Builder" configuration with feature flag Active if that gets approved, so we may be able to close this one if that goes through :)

Oh, nice! Thanks :)

🐛 Require `langcode: …` only for simple config that contains translatable values Active is in!

Issues this was postponed on are complete.

MR has failures and is unmergable

layout_builder.settings was removed in 📌 Replace "Expose all fields as blocks to Layout Builder" configuration with feature flag Active . As far as I understand this is no longer required, so I'm closing this. Please feel free to reopen if I've made a mistake with that.

Thanks!