Drupal core provides schemas for the configuration of several plugins and plugin types, but fails to provide fallbacks for some of them. This means that if the schema naming format is action.configuration.*, for instance, no schema with that exact name exists to fall back to when a replacement value for that wildcard does not exist.
Add fallback schemas of type ignore. This does not change any behavior, but allows other code to safely use such schema naming formats without having to worry about whether or not a schema exists for the wildcard replacement values the code uses at that particular time. The necessary schemas are:
action.configuration.*display_variant.plugin.*condition.plugin.*ckeditor.plugin.*search.plugin.*tour.tip.*views.display.We MUST NOT provide an actual schema, not even that of the plugin type's base class, because extending base classes is not required and we cannot assume anything about individual plugins' internal configuration.
None.
None.
None.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | drupal_2633232_2.patch | 2.85 KB | xano |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
xanoComment #3
borisson_The code here looks great and I understand why this is being added.
Do we have to define the
ignoretype somewhere? Otherwise RTBC++?Comment #4
xanoignoreis the second data type defined in./core/config/schema/core.data_types.schema.ymlComment #5
borisson_In that case, this looks great.
Comment #6
alexpottLet's add some test coverage since if we don't and someone removes a fallback we'll never know it is broken.
Comment #7
xanoWe should add tests to make sure these fallback schemas exist. We'll need one test per module, and one for core, which test hardcoded lists of fallback schemas.
Comment #11
tim.plunkettIn addition the fallback of
block.settings.*should be in core.data_types.schema.yml not block.schema.yml, since it is for the core plugin not the module's config entity.Comment #26
dcam commentedI converted the patch in #2 to an MR and added the schema verification tests. Some of the original changes are irrelevant now because this issue is so old. But the CKEditor plugin schema is of particular note. I noticed there's no fallback for
ckeditor5.plugin.*, but when I attempted to add one other tests started failing with the following message:So there's an check for a specific data type here. I'm not sure if this invalidates the need for a fallback schema or not. I ended up removing that change from the MR.
Also, I don't know if there are other new plugin types in Core that need this treatment. If anyone knows of some, then let me know.
Comment #27
smustgrave commentedLeft some comments on the MR.
Comment #28
dcam commentedAttributes and a change record have been added.
Comment #30
smustgrave commentedThis one has been sitting around for 5 months and realized all feedback on the MR has been addressed. Going to go on a limb and mark.
Comment #31
borisson_While adding a schema that ignores all it's properties doesn't make it strictly validatable yet, this issue is from well before that. I think the mr here makes the situation better, so I agree with @smustgrave, this looks rtbc.
Comment #32
alexpottI'm not really sure what this change actually gives us. The issue summary says:
But core is already safely using
action.configuration.action_send_email_action:withoutaction.configuration.*existing. What problem are we solving here?Comment #33
borisson_I don't remember talking with @Xano about this, or any of the specifics from a decade ago.
I was trying to figure out a good answer to this and I don't really have one. If there is no configuration schema it currently goes to type: undefined, and with this patch it would be type: ignore.
There is nothing to be gained from strictness or validateability. Should we close this issue instead?
Comment #34
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #35
smustgrave commentedBot rebellion
Comment #36
borisson_As @alexpott said in #32, core's already doing this without issues and this doesn't provide any value. I'm going to go ahead and close this issue because of that.