Allow choosing more than 1 visibility group per block

Working on a project that needs exactly this. We have several visibility groups and have certain blocks that should be displayed if one of those groups are matched. AFAICT, currently the only way to achieve this is to add the same block multiple times & assign each a different visibility group... which is pretty cumbersome.

Patch attached is the first attempt at this. This is how the form looks like:

You can select multiple visibility groups and choose whether each one needs to match (AND operator) or only 1 (OR operator).

I've currently implemented this as a completely separate condition ("condition_groups"), which is why you are seeing a "Condition group" and a "Condition groups" tab. The reason for this is to not break backward compatibility with existing installations.

I guess we can provide an update hook that does the conversion, but perhaps we should first decide whether we want this functionality in the module & if this way (screenshot) is the way to go forward.

Thanks @rp7 for your work on this!

The other day I was in need of this feature and I definitely think this adds value to the module. Marking this as "Needs review" and will test the patch when I get to it.

Tested patch #4, works as expected. Thank you, it is much simpler now to handle block visibility.

Change to RTBC to push this issue forward.

This is great.

I think we probably want to either try to get a good user experience under one tab, or to put this in an optional submodule for now.

Would appreciate further review (including of the wording / UX) and thoughts on one tab or two.

If this is desired functionality, I think we should definitely have 1 tab & provide an update hook that does the conversion for existing installations. Unfortunately, I currently don't have the time to work on this.

Agreed, I don't think it makes sense to have 2 tabs. I have gone ahead and added an update hook to convert the existing condition group references and the operator string to the new variant as per comment #8.

Tests are likely to fail.

Made the patch in #12 compatible with D9.

Test still failing, though.

@rp7 Any update on this patch? (#14)

This needs a re-roll now. Curious also about thoughts with regards to more generalized complex rule modules, #3302768: Plan potential future integration with context stack, ECA, and/or Rules.

Working on a reroll.

Rerolled. Hopefully.

While there is an update script for fixing the configuration, would it be worth including logic in buildConfigurationForm(), calculateDependencies(), getCacheTags(), getCacheContexts() and getCacheMaxAge() to support a single value?

(waiting for the v2 tests to be enabled, then I'll finish off any issues that arise on this issue)

This should go into a 2.1.x release, as it's a new feature.

Rerolled.

Rerolled.

Trying to fix those tests.

Trying to debug this from the runtime output as it's failing locally because of something else.

Will this fix it?

Trying again.

Now without that typo.

The config value was only partially renamed from 'block_visibility_group' to 'block_visibility_groups', a few uses of the older argument existed so let's see if this is better.

This fixes the problem locally.

This should fix it.

Let's see if this fixes the problem.

Once more.