The wildcard is a really simple but powerful way of selecting multiple entity configurations, however there are often cases where you want to still push specific configurations otherwise ignored on import.
As example you want to ignore all page_manager instances (or webform) BUT the user dashboard, which should still be controlled by the developer.
The feature can consist of only a marker for excluding ignoring specific configuration, i.e. the D7 block configuration path excluder:
- - page_manager.*
- - ~page_manager.page.dashboard
- - ~page_manager.page_variant.dashboard-block_display-0
Or - even better - a second textarea listing only the excluded entities.
Code-wise we'll need to update the config schema with the excluded_from_ignore list PLUS a change on ConfigImporterIgnore::preImport():
if (self::matchConfigName($config)) {}
needs to be:
if (self::matchConfigName($config) && !self::excludeFromIgnore($config)) {}
Needs to be flagged I'm currently doing this from a custom module using the hook_config_ignore_settings_alter()
, but it's really painful to remove the wildcarded setting, looping into the configs and adding to the settings ALL from group the but the one I need to take control of. Besides it's impossible to do from the web UI.
More than happy to provide a patch if you do like my idea.
Comment | File | Size | Author |
---|---|---|---|
#10 | exclude_specific-2855322-10.patch | 5.96 KB | gambry |
| |||
#10 | interdiff.txt | 1.08 KB | gambry |
ExcludeFromIgnore.jpg | 45.03 KB | gambry |
Comments
Comment #2
tlyngej CreditAttribution: tlyngej at Annertech commentedHi @gambry
Wow, you've put some good thoughts into this issue, and I totally agree. The current solution is kind of sledgehammer'ish.
I'm in favor of the "D7 block configuration path excluder" approach. Mostly because it keeps the code logic simpler, as it would just have to take one set of configuration, from one text box, into account. The extra box might also require some changes to the alter hook, that would probably not be backward compatible. Ohh, and the UI will also need some changes with the extra box.
But that's just me, waring the maintainer cap. I'm not really sure what would make most sense from an ordinary users perspective.
Comment #3
gambryHi @tlyngej,
Thanks for your positive feedbacks!
My only concern with the "D7 block configuration path excluder" is with the feature NOT being in core anymore, even if I'm not 100%. That would introduce a new/different pattern/UX.
However we will exclude config entities and not paths, so it is something new/different anyway.
I'll work in a patch today/tomorrow using the D7 approach.
Comment #4
gambryThis is an initial work. It needs test coverage of course, but it works on my local tests.
Comment #5
tlyngej CreditAttribution: tlyngej at Annertech commentedI like the use of a constant for the character that will act an exclusion prefix. Maybe I should do that for the asterisk as well... In another issue :-)
Anywho, glancing over it, and it looks fine. I'll give it a spin later, if time allows it, and get back to you again.
But thanks so far!
Comment #6
gambryWorking on test coverage
Comment #7
gambryIn this patch:
ConfigImporterIgnore::FORCE_EXCLUSION_PREFIX
instead of hardcoding '~' on the Settings formThe second is the actual test for this patch, testing the exclusion against wildcarded config.
Happy to remove the first test if you think is useless.
Also I think most the test needs refactoring as all tests setups are reduntant and can be done from a single
setUp()
. But may be a job for a follow up.Comment #8
scott_euser CreditAttribution: scott_euser as a volunteer and at Fat Beehive commentedThis looks good to me. I understand the extra test as it test that
system.site
is caught by the setting~system.site
to avoid future conflicting changes.The comments for both tests are the same. Actually the second test verifies that the matching config is still imported despite being ignored by the wildcard.
I think this should be something like:
As it would be illogical to add both webform.webform.contact and ~webform.webform.contact to the settings.
Other than those two niggles, to me this looks ready to go and solves the issue.
Comment #9
gambryAddressing issues from #8.
Comment #10
gambryJust a small change on the tests, replacing
array()
with[]
.Comment #11
scott_euser CreditAttribution: scott_euser as a volunteer and at Fat Beehive commentedThanks for the changes, this all looks good to me and works well testing with various combinations of ignores with wildcards and forcing specific configs caught by those wildcards to be imported anyway.
Comment #12
tlyngej CreditAttribution: tlyngej at Annertech commentedGuys, what a fantastic piece of work you've done here! Much appreciated.
I'll get this bad boy committed ASAP.
PS: @gambry, I'll create a new issue that addresses the need of an overhaul of the existing tests.
Edit: Created at #2857900: Existing tests needs some refactoring.