Overview

JS Components don't use plugins and hence this key name doesn't make sense

Proposed resolution

Allow source components to not use a plugin ID (they're already not required).

Quoting HEAD's type: experience_builder.component_source_settings.* definition:

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: {}

So, change the name of the required setting from plugin_id to:

  1. local_source_id
  2. intra_source_id
  3. local_id
  4. lid (also used by JSON:API spec: https://jsonapi.org/format/#document-resource-objects — see https://jsonapi.org/format/#document-resource-objects:~:text=a%20client%.... specifically)
  5. … something else — IOW: TBD

User interface changes

None.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Title: Allow component source plugins that don't have a plugin_id setting » [PP-1] Allow component source plugins that don't have a plugin_id setting
Status: Active » Postponed
Related issues: +#3498889: ComponentSource plugin for code components
longwave’s picture

Title: [PP-1] Allow component source plugins that don't have a plugin_id setting » Allow component source plugins that don't have a plugin_id setting
Status: Postponed » Active
wim leers’s picture

Title: Allow component source plugins that don't have a plugin_id setting » Rename ComponentSource settings' `plugin_id` to `local_source_id` or similar to not bias towards source plugins that don't use plugins under the hood
Issue summary: View changes
Issue tags: +Configuration schema
wim leers’s picture

Issue summary: View changes

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

wim leers’s picture

Component: Data model » Config management
Status: Active » Needs work
Issue tags: +DX (Developer Experience)

@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.
wim leers’s picture

Component: Config management » Component sources
wim leers’s picture

Issue tags: +stable blocker

thoward216 made their first commit to this issue’s fork.

wim leers’s picture

Assigned: Unassigned » thoward216

Thanks for taking this on, @thoward216!

wim leers’s picture

Status: Needs work » Needs review
Parent issue: » #3520484: [META] Production-ready ComponentSource plugins

Looks ready for review to me? 🤓

wim leers’s picture

Assigned: thoward216 » larowlan
Status: Needs review » Reviewed & tested by the community

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 what the meaning is of that identifier:

SDC

    local_source_id:
      type: string
      label: 'Component ID'
      constraints:
        NotBlank: []
        Regex:
          pattern: '/^[a-z0-9_-]+:[a-z0-9_-]+$/'
          message: 'The %value machine name is not valid.'
        PluginExists:
          manager: plugin.manager.sdc
          interface: 'Drupal\Core\Plugin\PluginBase'
JS ("code components")
    local_source_id:
      label: 'JS Component ID'
      constraints:
        ConfigExists:
          prefix: experience_builder.js_component.
Block
    local_source_id:
      label: 'Block plugin ID'
      constraints:
        PluginExists:
          manager: plugin.manager.block
          interface: Drupal\Core\Block\BlockPluginInterface

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 😄

  • larowlan committed e6abcf22 on 0.x authored by niharika.s
    Issue #3502982 by thoward216, niharika.s, wim leers, larowlan: Rename...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 0.x - thanks!

Status: Fixed » Closed (fixed)

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

wim leers’s picture

Assigned: larowlan » Unassigned
Issue tags: -stable blocker +beta blocker