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.
Exclude from Ingore textarea example

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.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

gambry created an issue. See original summary.

tlyngej’s picture

Hi @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.

gambry’s picture

Hi @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.

gambry’s picture

Status: Active » Needs work
Issue tags: +Needs tests
FileSize
2.15 KB

This is an initial work. It needs test coverage of course, but it works on my local tests.

tlyngej’s picture

I 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!

gambry’s picture

Assigned: Unassigned » gambry

Working on test coverage

gambry’s picture

Assigned: gambry » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
5.86 KB
4.93 KB

In this patch:

  • Using ConfigImporterIgnore::FORCE_EXCLUSION_PREFIX instead of hardcoding '~' on the Settings form
  • Adding test coverage. The first test is mainly for avoiding regressions, as test passes with or without path. It makes sure we don't use the same character for something else.
    The 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.

scott_euser’s picture

Status: Needs review » Needs work

This 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.

/**
* Verify Force Import syntax is working.
*
* This test makes sure we avoid regression issues.
*/

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.

~webform.webform.contact (will force import for this configuration, even if ignored)

I think this should be something like:

~webform.webform.contact (will force import for this configuration, even if ignored by a wildcard)

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.

gambry’s picture

Status: Needs work » Needs review
FileSize
2 KB
5.97 KB

Addressing issues from #8.

gambry’s picture

Issue tags: +DrupalCampLDN
FileSize
1.08 KB
5.96 KB

Just a small change on the tests, replacing array() with [].

scott_euser’s picture

Status: Needs review » Reviewed & tested by the community

Thanks 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.

tlyngej’s picture

Status: Reviewed & tested by the community » Fixed

Guys, 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.

  • tlyngej committed c848944 on 8.x-1.x authored by gambry
    Issue #2855322 by gambry, tlyngej, scott_euser: Exclude specific...

  • bircher committed 33a5137 on 8.x-2.x authored by gambry
    Issue #2855322 by gambry, tlyngej, scott_euser: Exclude specific...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.