Duplicate node type visibility condition in block settings

+1 to this - confused me quite a bit. They seem to do the exact same thing, except I noticed that the ctools one has a "negate this condition" checkbox while the core provided one does not. I couldn't see why the core one does not include it.

Good to know, thanks!

A more generic solution is probably in order. Webform reveals the same confusing behavior. Presumably as written the current filter could run awful of any module that provides it's own block conditions based on entities defined in that module.

I can confirm this issue still exists. It's a bit confusing for "normal" users.

If both "Content types" and "Content type" are present, I assume "Content types" is preferred (the one without "Negate the condition" option), since it's from core?

Thanks for confirming my assumption @manuel.adan :-)

Patch worked for me, Drupal 8.4.4.

Although I'm now unsure where at all that ctools code is now used, since the patch just adds an extra condition to check for node type, but nodes aren't blocks (which is where I saw this) and the node edit form doesn't show this option, can anyone clear that up for me?

@AaronMcHale, blocks have visibility conditions based on the display context. One of those conditions is the current entity bundle. When accessing an entity in its canonical URL (like /node/1), the condition will apply to that blocks placed in visible areas. Drupal core provides support for this type of condition, and ctools provides the same for any entity type. That's the reason why in case of nodes (content) there is a duplicate set of entity bundle conditions.

So I understand what you're saying but where I'm not clear on is that as far as I can tell the ctools "Content type" tab (which we established seems to just be a duplicate of Core) is only shown on the Block Configuration, if that is actually the case wouldn't it be better to just remove the code all together instead of expanding the if statement?

The problem with this patch is that any existing site that uses the entity_bundle:node condition would then be broken as it wouldn't exist anymore. The problem quite simply can't really be solved in ctools as it is now stable.

However, #1932810: Add entity bundles condition plugin for entities with bundles will actually solve this by adding most of this functionality into core and hiding the old node type condition.

I'm hijacking this issue to start a patch that shows how much code can be removed after that core issue is committed.

Postponed on #1932810: Add entity bundles condition plugin for entities with bundles

The patch from #14 no longer applies cleanly. Here's a re-roll (which still needs more testing).

This overly simple (and stupid) approach helps protect users from clicking the "wrong" settings:

/* Hide CTOOLS bug */
/* https://www.drupal.org/project/ctools/issues/2857279 */
.block-form #edit-visibility-entity-bundlenode,
.block-form a[href="#edit-visibility-entity-bundlenode"] {
  display: none !Important;
}
.block-form #edit-visibility-node-type {
  display: block;
}

This way I know that I am relying on Drupal Core and not on Ctools for any visibility settings until I the patch gets ready.
Just add this on the admin-subtheme css.

Another possible approach to hide the vertical tabs for entity_bundle:x visibility plugins is via hook_plugin_filter_TYPE__CONSUMER_alter():

function my_module_plugin_filter_condition__block_ui_alter(array &$definitions, array $extra) {
  // Filter out 'entity_bundle' condition plugins. This is done to prevent
  // duplicate 'Content types', 'Webform', etc. vertical tabs.
  $definitions = array_filter($definitions, function ($def) {
    return $def['id'] !== 'entity_bundle';
  });
}

Note: this targets 'block_ui' consumer specifically, which is the consumer id used for the block form.

The workaround presented in #20 is elegant and works perfectly.

Just be sure that you don't have any blocks that are presently using the block visibility options provided by ctools. If you do, it can be quite confusing to figure out why they aren't showing/hiding correctly. Temporarily commenting out the hook and clearing cache can show the "old" ctools block visibility settings in order to remove them.

As accepted #1932810: Add entity bundles condition plugin for entities with bundles

Finally :)

added a test run of #18 against Drupal 9.3. The challenge will be to figure out what to do about this until the module requires 9.3.

Maybe keep the deriver but combine it with a version check on 9.3? And keep the duplicated code for now in EntityBundle.

Version check looks reasonable but I guess it will affect config schema

Here's a patch that does that.

@andypost: ctools didn't namespace it's plugin ID's, so they are identical to core and it should be a clean replacement. The schema definition is identical.

Attaching a patch that does the version check approach. There are no tests of this and I didn't test it manually yet.

Tested patch #26 on D9.2. Still seeing both "Content type" and "Content types"

This patch is not supposed to fix anything on 9.2, this just properly integrates with the new entity_bundle core condition in 9.3 and core then properly deprecates and hides the node_type condition if you have no existing configuration for it.

+++ b/ctools.module
@@ -91,6 +94,15 @@ function ctools_condition_info_alter(&$definitions) {
+      $definitions[$id]['class'] = EntityBundle::class;
+    }

Think we need to add

$definitions[$id]['provider'] = 'ctools';

here to ensure that the configuration now has a dependency on ctools.

I think we also need to add code to the \Drupal\ctools\Plugin\Condition\EntityBundle to ensure that the entity type provider is in the dependencies.

A typo ("Druapl") in this line:

-    // Do not define any derivates on Druapl 9.3+, instead, replace the core
+    // Do not define any derivates on Drupal 9.3+, instead, replace the core

I'd suggest "derivatives" is more common than "derivates", but that might just be me.

> $definitions[$id]['provider'] = 'ctools';

I'm actually not sure about this. The ctools implementation is basically identical and works the same way.

Uninstalling ctools *will* allow those existing configurations to continue working, it will just use a different implementation. It doesn't actually depend on the ctools module and there is no config update step necessary when uninstalling. There is extra functionality, but that's only needed when using the ctools module.

And core already defines each plugin derivative with the proper provider.

So, just fixed the typos.

That failure is in the branch not the patch for 9.2, I've committed this thank you @Berdir et al

If someone else is also missing the "negate" option for content types, here's the discussion for core: #2857282: Remove node visibility condition code from block settings form ;)