Identify roadblocks to staging config in Workspaces (e.g., via wse_config)

I haven't actually used wse_config but from speaking to @amateescu previously it works OK with config entities that have no effect on database schema, like views, and it doesn't work at all with e.g. adding or removing fields from entities for pretty obvious reasons.

If we're talking about simple config like the site name + config entities like page templates etc., then that is probably covered pretty well though.

@catch actually system.site configuration is ignored by wse_config see https://git.drupalcode.org/project/wse/-/blame/2.0.x/modules/wse_config/...

Here's a video of wse_config in action - put it on youtube because it's a big file - https://youtu.be/m4CsXtI3420

Key findings so far:

• You can stage config changes with wse_config
• Revert appears to be broken
• Conflict resolution is not in place. If you change a config on live before it is staged it will become automatically enabled on stage. Once a config change is made in the workspace it will apply over the top of any config changes made on live when the workspace is published.

Hm, those key findings seem … concerning 🤔

• Is reverting broken for deep/complex reasons or is it a trivial(ish) bugfix? I suspect and hope the latter! 😄
• The absence of conflict resolution seems pretty fundamental? 😨 It seems super plausible and reasonable to at least have multiple Site Builders modify different regions in an XB page template (PageTemplate config entity). Throwing away the work of the Site Builder who was the latest to approve/publish their changes would be a poor UX.

  (I think not allowing concurrent editing of the same region might be acceptable, although I defer to @lauriii on that.)

Workspaces doesn't have conflict resolution built in, but at DC Barcelona I talked to @hchonov and he is pretty sure https://www.drupal.org/project/conflict → 2.x branch would cover a lot of the needs for content entities - e.g. it can automatically merge two revisions of an entity when the edits are against different fields. Whether this can also be adapted to configuration entities I have no idea though.

If you change a config on live before it is staged it will become automatically enabled on stage. Once a config change is made in the workspace it will apply over the top of any config changes made on live when the workspace is published.

In the Experience Builder workflow, it should be never possible to make changes directly in the live workspace. All changes are made in non-live workspaces and then merged into staging which would be then published to live.

it works OK with config entities that have no effect on database schema, like views, and it doesn't work at all with e.g. adding or removing fields from entities for pretty obvious reasons.

Could we allow additions to database schema to take place immediately even if the change is done in a workspace? Theoretically this could work because even if we make the database changes, it shouldn't impact the live site unless they get exposed somewhere. In the case of adding a new field to a bundle, it would only get exposed if the form display and entity display uses that field.

Deletions are trickier and should probably be handled differently so that the deletion only happens on publish. In this case, we would hide the fields from the user just like it was already deleted, even though the actual deletion would only happen on publish.

I think not allowing concurrent editing of the same region might be acceptable, although I defer to @lauriii on that.

I think we can work with the assumption that we don't have to merge changes within a single region. There may be use cases that would require creating alternative versions of a region, e.g., multiple proposals or an event that requires preparing to multiple outcomes. Based on that, the behavior might be just that the region gets overridden with the new contents regardless if other changes took place in between.

For XB, I don't think it's a problem for Workspaces to not provide conflict resolution, because prior to performing a real entity save into a Workspace, we'll be performing autosaves into its own temp-store-like storage, and can implement either locking or conflict-free collaborative editing at that stage. @larowlan started jotting some ideas down for the latter in 📌 META: Conflict free concurrent editing Active .

In the Experience Builder workflow, it should be never possible to make changes directly in the live workspace. All changes are made in non-live workspaces and then merged into staging which would be then published to live.

This would be easy to implement with generic form alters (at least for config-aware forms), config entity validation, config listeners etc. either in wse_config or an additional module. I could see sites using that without workspaces anyway to prevent production changes altogether, might already exist in that sense. But it wouldn't fix the case where there is a drush cim as part of a code deployment - seems like a site/team-specific problem at that point though, we can give people different foot shooting options they can choose between.

Could we allow additions to database schema to take place immediately even if the change is done in a workspace? Theoretically this could work because even if we make the database changes, it shouldn't impact the live site unless they get exposed somewhere. In the case of adding a new field to a bundle, it would only get exposed if the form display and entity display uses that field.

So that seems theoretically possible. e.g. allow field creation in a workspace, affecting live, allow other config in the workspace to depend on that field etc. It's basically the same as if you deployed config for the supporting field (or added it in the live workspace), and then built the workspace on top of it.

The tricky bits without giving it loads of thought:
1. It will show up outside the workspace for other admins and potentially other workspaces could also add config that depends on it - maybe this is a feature rather than a limitation?
2. Discarding the entire workspace might be hard to do, but I guess it is not necessarily that much different to deleting a field for any other reason.

Possibly aspects of 1 and 2 are just informative messages in the UI that the config is 'special' and not completely isolated.

There are some places in core where you can configure fields to immediately be displayed in view and form modes, we would need to at a minimum ensure those are off.

It will show up outside the workspace for other admins and potentially other workspaces could also add config that depends on it - maybe this is a feature rather than a limitation?

Isn't this primarily an UI concern if we are able to identify that a field was added in a workspace? I would imagine we'd have to be able to identify fields that don't exist in the live workspace anyway to make sure that they don't get exported in a config export.

Discarding the entire workspace might be hard to do, but I guess it is not necessarily that much different to deleting a field for any other reason.

Given that we are isolating the use of the field to the workspace where it is being added, couldn't we delete the field safely when the workspace gets discarded?

I would imagine we'd have to be able to identify fields that don't exist in the live workspace anyway to make sure that they don't get exported in a config export.

Well the fields would exist in the live workspace though if they're new - and I think at least by default you'd want them exported too.

If you're making config changes on production (even if only via a workspace), then you would need a workflow for syncing configuration back to development environments. If you have a draft workspace with config changes, then when you sync the database back to development environments, that workspace will only work if the config supporting it exists. I could see some kind of additional workflows on top similar to config_split or similar modules where you can export with or without new config created in workspaces, but definitely I'd want the actual live state of production by default if I needed to debug something.

Isn't this primarily an UI concern if we are able to identify that a field was added in a workspace?

Yes, but don't know if wse_config already does things like filter out newly added config in config listings (vs. overriding config when it's loaded).

Given that we are isolating the use of the field to the workspace where it is being added, couldn't we delete the field safely when the workspace gets discarded?

Yep might actually be fine!

I think we should park the field creation in workspace discussion till we've work out what we're going to do. Creating fields that are not represented in live configuration is going to be fraught. I don't want to think about how the system report is going to deal with validating the entity schema definitions, for example.

I think I should have given more context to the conflict resolution remark - the moment you edit content in a workspace you can no longer edit it in the live site. The system prevents you from doing this. It's more conflict prevention than resolution I guess. The point is that the same it not true for configuration when you have wse_config installed.

I think the key decision is

we're fairly certain that we should use Workspaces as the entity storage model for this, regardless of how much of the Workspaces UI we end up using.

Given this feels nearly certain I think we need to make that certain and go all in a model that allows us to make configuration changes as part of workspace.

What confuses this issue a bit are the revert requirements on 🌱 Decide on approach to use for storing past revisions of XB config entities Active . When a workspace is applied to a live site it'll create a single revision for each content entity that is changed regardless of the number of revisions for that content entity in the workspace. I think this makes sense - each workspace is viewed as a single set of changes to make to the live site. I think config should behave the same. Unfortunately this is not the way wse_config is working at the moment. It looks like it is creating a content entity for each configuration edited in a workspace and when it is published it changes the live configuration. If you visit the WSE Config listing page (admin/config/development/configuration/config-content) you will see a list entities changed in each workspace and these changes don't really relate to the change that's happened to the live workspace. All WSE Config content entities are listed here regardless of the workspace you are on. You don't see a nice history like you do for node/n/revisions

One thing that makes me very unsure about workspaces and configuration is the possibility that the configuration change when you publish a workspace will affect more than just the content in the workspace being being published and the same goes for a revert. I think this breaks the mental model of what a workspace encapsulates.

One thing that makes me very unsure about workspaces and configuration is the possibility that the configuration change when you publish a workspace will affect more than just the content in the workspace being being published and the same goes for a revert.

I think for publishing this can be covered by some extra messaging when a workspace includes configuration changes. Anything that allows for staging of config changes on production is going to have similar issues even if it wasn't using workspaces, and the need for publishing content changes and config changes together (a new view that shows some new content etc.) wouldn't really go away even if it was implemented as linking two separate operations together.

Revert is very different though due to config/content dependencies - but again that problem exists with any kind of config revert on production after other changes have been made.

Is reverting broken for deep/complex reasons or is it a trivial(ish) bugfix?

I discussed this one with @alexpott in Slack and it seems to be a rather trivial bugfix.

I think I should have given more context to the conflict resolution remark - the moment you edit content in a workspace you can no longer edit it in the live site. The system prevents you from doing this. It's more conflict prevention than resolution I guess. The point is that the same it not true for configuration when you have wse_config installed.

Indeed, wse_config doesn't provide the same conflict prevention safeguards that content entities have, but it shouldn't be a big effort to add it.

If you visit the WSE Config listing page (admin/config/development/configuration/config-content) you will see a list entities changed in each workspace and these changes don't really relate to the change that's happened to the live workspace.

That UI is basically just the result of code scaffolding when the wse_config entity type was created, and its current role is mostly for "debugging" purposes, it was never meant to be used by a content editor or such. The UI for viewing config that is changed in a workspace is the same as for content, the "Manage workspace" page.

@catch summarized very well the current capabilities of wse_config in #3. Even though there might be some bugs here and there, any config that doesn't affect the database schema can be made editable in a workspace.

At the moment, the list of config that can be edited in a workspace is based on a "manual" allowlist, but a proper/long-term solution IMO would be to introduce something along the lines of FullyValidatable constraint from core that would work similarly to \Drupal\Core\Form\WorkspaceSafeFormInterface.

About field CRUD handling in a workspace, if that's not a strict initial requirement, I also think it would make sense to be discussed separately because it won't be nowhere as easy as a "adding a database table" discussion :) Hiding a new field from the UI would be the easiest part, IMO we should be more concerned about hiding it from the API. For example, we can't guess what kind of long-running (e.g. minutes or hours long) operations or other db schema alterations would be triggered by the "tiny" operation of adding a new field.

Apart from config changes that affect the db schema, which were "solved" by not being allowed in wse_config, the biggest challenge of that module (and any per-workspace-config solution in general), is the multitude of caching layers stacked on top of various config/services/plugin managers/etc. that have to be made workspace-aware.

That's in fact the ugliest and IMO hackiest part of the module, and I'd really love if we can come up with a better solution than what we have atm. One idea for it would be to introduce an alteration point at the very top of the caching system (in the cache backends or factories?), that would provide Workspaces an easy way to append the current workspace ID to the cache key.

Apart from config changes that affect the db schema, which were "solved" by not being allowed in wse_config, the biggest challenge of that module (and any per-workspace-config solution in general), is the multitude of caching layers stacked on top of various config/services/plugin managers/etc. that have to be made workspace-aware.

I've been pondering this quite a bit today and looking at the caching stuff in wse_config and thinking about @wim leers's config override POC from a few months ago and I'm wondering if we can using the config override system to help us here. I.e. not actually use config overrides but make it look like a config override is active for an config we've changed in the workspace. Then we might not have to do so many cache shenanigans in wse_config.

At the moment, the list of config that can be edited in a workspace is based on a "manual" allowlist, but a proper/long-term solution IMO would be to introduce something along the lines of FullyValidatable constraint from core that would work similarly to \Drupal\Core\Form\WorkspaceSafeFormInterface.

At the moment for config entities it is opt-in and uses an allow list approach. For simple configuration it's the other way around it has a list to exclude (including system.site). This mixture of approaches confused me for a bit.

Another possible approach to config overrides would be using VariationCache for a lot more things, and then adding default cache contexts similar to the render cache default cache contexts, which workspaces could then add itself to. This would be a lot of work though.

@catch in #7: Great! On the one hand, I think config entities are simpler than content entities. On the other hand, content entities have a more predictable structure (a content entity has N fields, and every field has a specific validatable structure, with almost always every field value being independently validatable — the exceptions being the validation constraints that extend \Drupal\Core\Entity\Plugin\Validation\Constraint\CompositeConstraintBase), but on the other hand config entities have an arbitrarily complex structure (mappings or sequences at any level, not just the top level, so also many levels down, and even a mapping in a mapping in a mapping in a sequence etc.).
So … that makes me strongly suspect that that project is not generalizable to arbitrary config entities… 🙈

@lauriii in #8:

• RE: first paragraph: the "then merged" part still requires conflict resolution.
• RE: second paragraph: even if a new field being added to a bundle is not exposed in the UI, it would still get exposed via validation errors (for invalid field value, or potentially even missing required value) and via APIs: REST, JSON:API, GraphQL, as well as the PHP Entity/Field API. And more complex still: what if the same field gets added twice?
• RE: third paragraph: okay, thanks! 👍

@effulgentsia in #9: But the same config entity may be modified (and auto-saved) in different ways in each workspace. Or … are you saying that that is what you are proposing to simply prevent/disallow, @effulgentsia? 🤔

IOW, are you proposing: ?

@catch in #10:

we can give people different foot shooting options they can choose between.

🤣

@alexpott in #13:

the moment you edit content in a workspace you can no longer edit it in the live site. […] It's more conflict prevention than resolution I guess.

Oh, wow!

That sure is a way to avoid it! So, when trying to edit the live version of a content entity that already has a modified version in any workspace, you'd get a message informing you editing it is not possible at this time? 🤔

@lauriii, is that acceptable to you?

@alexpott in #14:

[…] the possibility that the configuration change when you publish a workspace will affect more than just the content in the workspace being being published and the same goes for a revert. I think this breaks the mental model of what a workspace encapsulates.

I'd say it's a certainty, not a possibility. The most common examples for XB will be:

1. modifying the front-end theme's PageTemplate config entity (added in 📌 Introduce an XB `PageTemplate` config entity Active ), which will affect all HTML-serving routes, not just the ones whose content entities were modified in the workspace
2. modifying a content entity type's bundle's "content type template" (does not yet exist, but is a critical product requirement — see 🌱 [later phase] [META] 7. Content type templates — aka "default layouts" — affects the tree+props data model Active ), which will affect all content entities of that type/bundle, not just the modified/added content entities in the workspace

⚠️ This is why I've been so concerned about seeing the designs for editing the PageTemplate: while 📌 Add support for global regions Active is moving forward and that is great, I have yet to see UI designs with affordances that ensure that the user modifying the PageTemplate is they're editing the page template! See my comment #3489899-9: Add support for global regions → from >2 weeks ago.

@catch in #15: you're right that this is not technically different from modifying "live config". But the point is that XB is aiming to do better and make the Drupal UX easier to understand. If we don't pay attention to that, we don't materially move the needle on this front, which has many repercussions.

@amateescu in #16:

• RE: reverting broken being trivial: yay, exactly as anticipated! 😄
• RE: → music to my ears! 😄🎶 Is it sufficient as-is? If not, what is missing?
• RE: "hiding it from the API" — /me high-fives @amateescu for raising the exact same concern! 😁🤓
• RE: caching layers pain: I have an idea. core.services.yml contains the renderer.config container parameter. It contains required_cache_contexts: ['languages:language_interface', 'theme', 'user.permissions']. If we would generalize that to all cache bins, then it'd become simple: by default, all cache bins have [] (the empty array) as the required cache contexts, and again defined in a container parameter. The Workspace module would then be able to alter the new container parameter, and add 'workspace' to it (the \Drupal\workspaces\WorkspaceCacheContext). Done!

  I think this could even be done in a backwards-compatible way. 🤔 Though it would be non-trivial to get this through the core review process and ensure strong guarantees for backwards compatibility 😬

@alexpott in #17:

not actually use config overrides but make it look like a config override is active for an config we've changed in the workspace. Then we might not have to do so many cache shenanigans in wse_config.

That's exactly what I was thinking back then! What I did in https://git.drupalcode.org/project/experience_builder/-/commit/fca2d9b54... is agnostic of where the config is stored. And in fact, I think changing that to just use the already-existing workspace cache context instead of the new-for-PoC cache context … might mean that it could quite easily be made to work with the existing wse_config? Can't quite think that all the way through at this time though, it's been too long since I worked on that.

@catch in #18: HAH, and that is exactly what I just proposed to @amateescu above! 😁

even if a new field being added to a bundle is not exposed in the UI, it would still get exposed via validation errors (for invalid field value, or potentially even missing required value) and via APIs: REST, JSON:API, GraphQL, as well as the PHP Entity/Field API. And more complex still: what if the same field gets added twice?

We would have to bypass validation in this case because the field technically wouldn't exist in the live workspace, i.e. even if you're adding a required field, it should not trigger a validation error. The field should also not be exposed in REST, JSON:API, GraphQL until it has been deployed to the live site.

Adding the same field twice seems edge case enough that it could generate a validation error.

So, when trying to edit the live version of a content entity that already has a modified version in any workspace, you'd get a message informing you editing it is not possible at this time? 🤔

I think this would be a pretty harsh limitation. There are use cases where you may want to prepare multiple versions of a change. Also having a change to a page in one workspace even if it's stale, would mean no one else would be able to edit it.

I have an idea. core.services.yml contains the renderer.config container parameter. It contains required_cache_contexts: ['languages:language_interface', 'theme', 'user.permissions']. If we would generalize that to all cache bins

We can do this for caches that implement VariationCache, but it can only be at level, not the cache backend implementation itself. So we'd still need to make a load of caches VariationCache enabled - but these are the same sorts of things that already vary by language and similar, so it would be a good change in general, probably.