Allow component source plugins that don't have a plugin_id setting

niharika.s → made their first commit to this issue’s fork.

@niharika.s: wow, so nice to see this almost done! 🤩

@larowlan:

I was expecting each plugin to be able to define it's own settings and hence SDC and block components could retain plugin_id, whilst JS components could use something else.

The problem with that is that that won't work with:

    id:
      # This ID intentionally does not use `type: machine_name`, because it is a composite ID that is better validated
      # using the `StringParts` constraint than the `RegEx` constraint.
      type: string
      label: 'Component'
      constraints:
        StringParts:
          separator: .
          reservedCharacters:
            - ':'
          reservedCharactersSubstitute: .
          parts:
            - '%parent.source'
            - '%parent.settings.plugin_id'

This guarantees consistent, meaningful Component config entity IDs.

See that last line there: that's immutable. There's no other way I know of in config schema to achieve this.

I think

  label: 'Block component settings'
  mapping:
    local_source_id:
      label: 'Block plugin ID'
      constraints:
        PluginExists:
          manager: plugin.manager.block
          interface: Drupal\Core\Block\BlockPluginInterface

is plenty clear: for every Block-sourced Component config entity, the local_source_id must be a block plugin ID. The validation constraint makes this firm.

And the equivalent for a component source using config entitities:

  label: 'Code component settings'
  mapping:
    local_source_id:
      label: 'JS Component ID'
      constraints:
        ConfigExists:
          prefix: experience_builder.js_component.

Thanks for taking this on, @thoward216!

Looks ready for review to me? 🤓

I was expecting each plugin to be able to define it's own settings and hence SDC and block components could retain plugin_id, whilst JS components could use something else. The thinking being there might be other source plugins in contrib that don't fit either pattern

— @larowlan at https://git.drupalcode.org/project/experience_builder/-/merge_requests/6...

Config schema limitations

While I agree that would be nice, that's AFAIK simply not possible using config schema.

We need the id for a Component config entity to be validatable.

        StringParts:
          separator: .
          reservedCharacters:
            - ':'
          reservedCharactersSubstitute: .
          parts:
            - '%parent.source'
            - '%parent.settings.plugin_id'

👆 That last bit must be a known name, there cannot be dynamicness there. At least AFAIK.

Hence:

# The structure that all ComponentSource-specific per-Component config entity settings MUST adhere to. (Because
# the `experience_builder.component_source_settings.*` type's `id` key-value pair assumes `settings.plugin_id` to exist
# to guarantee the Component config entity ID is consistent.)
# Each ComponentSource MUST provide a specific override that:
# - MUST override both labels
# - MUST add appropriate validation constraints to `plugin_id`
# - MAY add more key-value pairs to the mapping — and then add appropriate validation constraints for those
# - MUST mark the entire subtype as being fully validatable
experience_builder.component_source_settings.*:
  type: mapping
  label: 'Source-specific component settings'
  mapping:
    # @todo Rename this to `source_local_id` or something like that in https://www.drupal.org/project/experience_builder/issues/3502982
    plugin_id:
      type: string
      label: 'The intra-source ID of this component in this source'
      constraints: {}

That's why this issue summary proposed a whole range of possible names. This MR went with local_source_id.

Per-ComponentSource meaning of "what this ID points at"

It's then up to each individual ComponentSource-specific settings config schema type to define of that identifier:

Conclusion

IMHO this as good as it gets. I think it's much better than "plugin ID" because it makes no assumptions.

So: RTBC, but giving @larowlan the chance to block commit if he has a better idea 😄

Committed to 0.x - thanks!

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