I don't think an upgrade path is needed. The new block_id property is only used internally by InlineBlockEntityOperations::saveInlineBlockComponent() so it doesn't require loading an entity that was just saved to get its entity ID.
This behavior is problematic for Workspaces because of the double-save that's happening when creating a new published entity in a workspace.
However, since this only applies to new content that's being created, there's no real reason to backfill this information for all existing content.
I'm a bit confused by this. If it's only used internally by InlineBlockEntityOperations::saveInlineBlockComponent() why is it in the config schema at all?
My guess is this is because theoretically, layout_builder inline blocks could be configured in block module itself, and then they would need config as defined here, but in practice that this never happens? However if they can end up in config then we would need a config upgrade path.
If it's only used internally by InlineBlockEntityOperations::saveInlineBlockComponent() why is it in the config schema at all?
That's a valid question technically, but it can also be expanded to ask "why does the inline_block plugin need a config schema at all"?
That block plugin is currently excluded from the list of plugin definitions in layout_builder_plugin_filter_block_alter(), so at the moment it can't end up in config. That might change in https://www.drupal.org/node/2979142, but we can't provide an upgrade path for config that doesn't currently exist :)
So the answer is basically: because Layout Builder provides a schema for the inline_block plugin (even though it doesn't need to), so if we're adding a configuration property to that plugin, we should provide the schema for it as well.
Now, if the next question is "why do we need to add a new configuration property", that's because InlineBlockEntityOperations has no knowledge of (or any way to access) the block_content entity that is being handled internally by the InlineBlock plugin, it can only work with that plugin object through its configuration. That's why for me it made sense to add the new block_id configuration property alongside the existing block_revision_id.
Will we need an upgrade path for this schema change? For existing sites
I don't think an upgrade path is needed. The new
block_idproperty is only used internally byInlineBlockEntityOperations::saveInlineBlockComponent()so it doesn't require loading an entity that was just saved to get its entity ID.This behavior is problematic for Workspaces because of the double-save that's happening when creating a new published entity in a workspace.
However, since this only applies to new content that's being created, there's no real reason to backfill this information for all existing content.
I'm a bit confused by this. If it's only used internally by InlineBlockEntityOperations::saveInlineBlockComponent() why is it in the config schema at all?
My guess is this is because theoretically, layout_builder inline blocks could be configured in block module itself, and then they would need config as defined here, but in practice that this never happens? However if they can end up in config then we would need a config upgrade path.
That's a valid question technically, but it can also be expanded to ask "why does the
inline_blockplugin need a config schema at all"?That block plugin is currently excluded from the list of plugin definitions in
layout_builder_plugin_filter_block_alter(), so at the moment it can't end up in config. That might change in https://www.drupal.org/node/2979142, but we can't provide an upgrade path for config that doesn't currently exist :)So the answer is basically: because Layout Builder provides a schema for the
inline_blockplugin (even though it doesn't need to), so if we're adding a configuration property to that plugin, we should provide the schema for it as well.Now, if the next question is "why do we need to add a new configuration property", that's because
InlineBlockEntityOperationshas no knowledge of (or any way to access) theblock_contententity that is being handled internally by theInlineBlockplugin, it can only work with that plugin object through its configuration. That's why for me it made sense to add the newblock_idconfiguration property alongside the existingblock_revision_id.