Conditions plugin validation schema is wrong

🏓 — missing some crucial info that will be necessary for this to become committable — right now, it's not clear whether this would break contrib/custom modules 😇

This was surfaced by #3525601: Personalization Segment storage: new `Segment` config entity type.

@wimleers thoughts on the comments in the MR?

There's 1 thread that's unanswered could that be addressed.

Looking at this again won't we need an update hook, using ConfigImporter, to remove the UUID from existing config.

This doesn't need that update hook, because the config currently does not reflect reality. I just grepped in my config folder for 2 projects. (both on context_mapping and visibility), in both cases the uuid does not show up.

Since both merge request comments are about the same thing, I think both are answered by this + #8.

I actually think this is rtbc.

Looks good to me too. Backported down to 10.6.x as it corrects a bug in schema.

Committed and pushed 28b81cc545f to main and fb0632b33ca to 11.x and a5831a7dc39 to 11.3.x and 25c4571c057 to 10.6.x. Thanks!

Looking at this again won't we need an update hook, using ConfigImporter, to remove the UUID from existing config.

+1

And probably this should not have been merged into 11.3.x and released in 11.3.4 because it broke our automated CI pipeline and automated tests started to fail due to Drupal\Core\Config\Schema\SchemaIncompleteException.

I haven't tested it, but I assume it could happen that site install from existing configuration could also fail in "strict" environments where \Drupal\Core\Config\Development\ConfigSchemaChecker is enabled since the beginning, and \Drupal\Core\Config\Development\LenientConfigSchemaChecker could create a bunch of noise.

This doesn't need that update hook, because the config currently does not reflect reality. I just grepped in my config folder for 2 projects. (both on context_mapping and visibility), in both cases the uuid does not show up.

It shows up in pathauto configurations.

uuid: 94138ea6-1167-4892-b511-770da955b112
langcode: en
status: true
dependencies:
  module:
    - node
id: capability_content_type
label: 'Capability content type'
type: 'canonical_entities:node'
pattern: 'capability/[node:title]'
selection_criteria:
  141d10df-0283-4b58-b275-30bcbaf561a2:
    id: 'entity_bundle:node'
    negate: false
    uuid: 141d10df-0283-4b58-b275-30bcbaf561a2
    context_mapping:
      node: node
    bundles:
      capability: capability
selection_logic: and
weight: -5
relationships: {  }

@mxr576 should we revert this and try again?

My colleague (@morvaim) just clarified the issue we experienced related to this change and it is possible that only Pathauto was doing something special and requires fixing after this change, see #3577286: Drupal core 11.3.4 removed uuid from condition plugin schema which causes schema errors in pathauto pattern configuration.

Given this has broken pathauto I am going to revert this from 10.6 and 11.3 and re-release; this fix will then go out for real in 11.4 which gives pathauto and anyone else affected time to write their update hooks.

reopening the issue

Do you think there is more to do here? ie, should we revert from 11.x and main as well, or should we leave that in place and it's up to contrib modules to handle updates? Given that I don't think core can know where this schema might be used, I am not sure what more we can do from the core side.

I agree with #30, I don't think there is more to do, I think we can move this issue back to fixed.

Re #26/30: Do we need a change record to help prepare people for 11.4/12.0?

Not sure we need a change record for every schema change, just have to remember to only make them in minor releases and not patch releases.