This is to add support for staging simple configuration, e.g., site name, which is critical for Experience Builder

We'll begin with a test that looks something like this:

Four users:

  • Anonymous session - obvious
  • Non-workspaces user - authenticated user without Workspaces permissions
  • Workspaces user #1 - has all Workspaces/Workspaces Config permissions
  • Workspaces user #2 - has all Workspaces/Workspaces Config permissions

Tests:

  1. They all see the same site name in the live workspace at the start.
  2. The Workspaces users each create and activate a workspace.
  3. Workspaces user #1 modifies the site name in their workspace.
  4. Workspaces user #1 sees the new site name in their workspace.
  5. Workspaces user #2 still sees the initial site name in their workspace.
  6. Workspaces user #2 can no longer modify the site name.
  7. They all still see the same initial site name in the live workspace.
  8. Workspaces user #1 publishes their workspace.
  9. They all see the new site name in the live workspace.

Does that seem about right to start with? Anything obviously missing or extraneous?

Issue fork wse-3505900

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:

About issue forks

Comments

traviscarden created an issue. See original summary.

traviscarden’s picture

traviscarden commented
Issue summary: View changes

I've added a WIP branch to show the current state of my work while I attend to another priority. I've got a basic framework (that can probably be extracted for more generalized use later). It's going well, but wse seems to break Core's workspace test utilities, since they work until I enable it:

TypeError : Drupal\Tests\wse_config\Functional\WseSimpleConfigTest::createAndActivateWorkspaceThroughUi(): Return value must be of type Drupal\workspaces\WorkspaceInterface, null returned
 /var/www/html/web/core/modules/workspaces/tests/src/Functional/WorkspaceTestUtilities.php:73
 /var/www/html/web/modules/contrib/wse/modules/wse_config/tests/src/Functional/WseSimpleConfigTest.php:99

That's where I'll pick up when I get back.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Experience Builder

That test looks like a nice start! 👍 😄

#2: seems like the expected Workspace config entity does not exist yet, since that NULL can only come from

Workspace::load($id)
amateescu’s picture

amateescu commented
Status: Active » Needs work

I think the test case laid out in the IS is an excellent starting point, thanks for working on this!

The error from #2 is caused by wse_workspace_presave(), which overrides the workspace ID with its UUID before saving. This is needed because core's assumption that workspace IDs should be reused after publishing doesn't match the expected content editor experience on most sites.

Until this is fixed in core, you should use \Drupal\Tests\wse\Functional\WseTestUtilities::wseCreateAndActivateWorkspaceThroughUi() instead of the core trait.

traviscarden opened merge request !99

traviscarden’s picture

traviscarden commented
Title: Add basic test coverage for `wse_config` with simple config » Add support for staging simple config
Category: Task » Feature request
Priority: Normal » Major
Issue summary: View changes
Parent issue: #3497636: [META] WSE Config: Path to Stable »

Thank you, @amateescu; that was an important piece of knowledge.

When I created this issue, I thought simple config staging was supposed to already be supported, so I'm changing this to a feature request, and I'm presuming to mark it "Major" since it's in the critical path for Experience Builder.

The draft MR I created above adds failing tests that are basically complete for the happy path. They should be enough to prove that the basic functionality works once they. Then we can proceed to add tests for secondary functionality, edge cases, and error-handling, etc.

I'm going to move on to writing tests for other functionality in the meantime.

amateescu’s picture

amateescu commented
Title: Add support for staging simple config » Add test coverage for staging simple config
Version: 2.0.x-dev » 3.0.x-dev
Category: Feature request » Task
Parent issue: » #3497636: [META] WSE Config: Path to Stable

The problem here was that the system.site config object was ignored by the blanket opt-out of system.* config objects.

Changing that to opt-out specific config objects and a couple of minor changes to the test makes it pass, which proves that simple config is actually supported :)

I've opened #3563938: Refactor the way we enable/support simple config and config entities to fix this in a more general way though.

@traviscarden, do you want to continue working on this test? There are a couple of @todo's left.

traviscarden’s picture

traviscarden commented
Assigned: traviscarden » Unassigned

Hey, @amateescu. I'm afraid I no longer have allocation for this as I've been moved to another project. I'll be cheering from the sidelines. 😄

amateescu’s picture

amateescu commented
Status: Needs work » Fixed

I've opened #3564458: [PP-1] Implement workspace conflict validation for config to add the functionality needed for the remaining @todo's in this MR.

Merged into 3.0.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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