Add validation constraints to user.settings, use/introduce symfony/expression-language to improve config validation DX

This needs a review from one of the user.module maintainers, tagging it like that, but this change is great. Thanks @Eli-T!

What if a site altered these settings already (or if using a module which is altering these)? Is this a BC break? The same applies for all other issues where we are hardcoding values (dblog, olivero, ...). I think we should consider this very carefully.

LGTM. I think this needs input from config validation maintainers as well (leaving tag in for this).

Thanks @moshe weitzman for the fastest subsystem maintainer review sign-off I can recall in years 🥳 🙏

#8: it's up to that module to then alter the validation constraints in the config schema.

Looks great! 😊

it's up to that module to then alter the validation constraints in the config schema.

So is there a draft change record/release notes snippet for all these "Add validation constraints to ..." changes somewhere?

@poker10: Please note that NOTHING in Drupal core or contrib is strictly validating config yet — only tests are, but contrib modules have no effect on that.

Change record: https://www.drupal.org/node/3362879 → . I've just updated it to document this, because you're right, that was missing: https://www.drupal.org/node/3362879/revisions/view/13198950/13294591 →

That made me realize that in this particular example, it may be better to follow the example set by ✨ Add a `langcode` data type to config schema Fixed , which did:

  constraints:
    Choice:
      callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'

Because in this case, we could say that all valid values for cancel_method are array_keys(user_cancel_methods().
(All of this would have been trivial if these were plugin IDs because for that we have the PluginExists validation constraint, but the user cancelation methods system predates the plugin system!)

That way, there is no need for altering the config schema anymore! 😊

I agree that this makes more sense. The idea that alex mentioned at drupalcon Lille, was that config could be validatable without having drupal running will be more difficult with this change, but since we already have that example in the langcode data type that shouldn't hold this back.

It's not possible to define a callback like this, so we would have create a new function just to define this callback? I'm not sure that makes sens to do.
callback: 'array_keys(user_cancel_methods)'

#13:

config could be validatable without having drupal running will be more difficult with this change

That is already the case for any configuration that is configuring e.g. plugins: only the Drupal site itself can only ever know which plugins are in fact installed 😅

#14: correct, what you describe there is not yet possible. Well, technically it is, because closures are allowed — see https://symfony.com/doc/current/reference/constraints/Choice.html#supply.... But … PHP closures cannot be defined in YAML files 😅

ACTUALLY! I just discovered https://symfony.com/doc/current/reference/constraints/Expression.html. Combine that with https://stackoverflow.com/questions/50782963/symfony-expression-validation and we should be able to do exactly what you wrote in #14, @borisson_!

I think this would be worth opening a blocking issue for? OTOH this would be the first use in core (because \Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes() actually is quite a bit more complex), so maybe this would be the appropriate issue?

O wow, that is great, this is a perfect usecase for that. I think we can add it in this issue, that would get the example next to the implementation.

To use this, we would also need symfony/expression-language, that feels like it would be something we need to do in another issue (and we'd need to do dependency evaluation for it as well).

I'm not sure if that is something we'd want to do; would there be other places where we could use expression language as well? What do you all think?

But this issue is the one that justifies that addition. I think we should add that dependency here, and add the dependency evaluation to the issue summary.

This would mean a massive DX improvement for making a lot of config validatable! 🤩

Let's first prove that this works by implementing it and then worry about adding a dependency evaluation? 🤓 Tentatively tagging 😇

What is left to do on this issue?