Split ValidKeysConstraint::$dynamicInvalidKeyMessage in 2: one for a specific matched type, one for a fallback type

Created on 6 December 2023, about 1 year ago
Updated 27 December 2023, about 1 year ago

Problem/Motivation

Split off from πŸ“Œ Configuration schema & required keys Fixed .

This is blocked on either πŸ“Œ Configuration schema & required keys Fixed or πŸ“Œ Refine ValidKeysConstraintValidator's messages: use #3401883's new Mapping infrastructure Needs review landing.

Either of those 2 issues will make us go from

'block_count' is not a supported key.

to

'block_count' is an unknown key because plugin is aggregator_feed_block (see config schema type block.settings.*).

This is a clear improvement.

But what would be even better per @phenaproxima is:

'block_count' is an unknown key because plugin is aggregator_feed_block (see config schema type block.settings.* β€” no more specific type was found).

Steps to reproduce

Run MigrateblockTest and see its assertions.

Proposed resolution

Detect when the matched type is a fallback type and in that case use a variant of $dynamicInvalidKeyMessage.

Remaining tasks

TBD

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component
ConfigurationΒ  β†’

Last updated about 17 hours ago

Created by

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @wim leers
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    @phenaproxima proposed:

    'block_count' is an unknown key because plugin is aggregator_feed_block (see config schema type block.settings.* β€” no more specific type was found).
    

    I propose:

    'block_count' is an unknown key because plugin is aggregator_feed_block (see config schema type block.settings.* β€” this is a fallback type, no more specific type matches).
    
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    Sure, I like your wording too. I would maybe alter it ever so slightly:

    'block_count' is an unknown key because plugin is aggregator_feed_block (see config schema type block.settings.* β€” this is a fallback type, no more specific type was found).

    I would not use the word "matches" because it's unclear if we're talking about the verb, "to match", or a noun, like "there is such a thing as specific type matches and there aren't any". (I know these mean the same thing, but I think it's worth removing any trace of ambiguity from the wording.)

  • πŸ‡§πŸ‡ͺBelgium borisson_ Mechelen, πŸ‡§πŸ‡ͺ

    I link the wording in #4, I would be really happy to get an error like this.

    Maybe we should also help people figure out what it is they have to do to resolve it? Is that too much?
    Either remove the key from your config, or add it to the schema.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    πŸ‘ WFM!

  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    Nice idea, @borisson_. Telling developers what they can do about an error can't hurt!

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    #5: That's not accurate. It's fine to not have a detailed config schema type. For example, a filter or a block may have no settings at all, in which case the fallback name block.settings.* or filter.settings.* is entirely appropriate β€” no additional config schema type needed!

  • Assigned to wim leers
  • Status changed to Active about 1 year ago
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • Issue was unassigned.
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Doesn't need to be me doing this actually :)

  • πŸ‡§πŸ‡ͺBelgium borisson_ Mechelen, πŸ‡§πŸ‡ͺ

    I think we settled on #4 as well, since otherwise it might get a really long error message if we want to be complete on what options there are to do.

Production build 0.71.5 2024