Add the ability to create sub-workspaces in order to enable the dev -> stage -> live workflow for content

Rebased the implementation on #3062434-7: Track the workspace of a revision in a base field and convert the workspace_association entity type to a custom index. The failing test on uninstall stems from there.

Ported over test fixes from #3062434: Track the workspace of a revision in a base field and convert the workspace_association entity type to a custom index

Added an API and basic UI for re-building the associations index.

Added missing link type definition for association forms.

As discussed with @amateescu, index rebuilding will move somewhere else to keep this patch smaller. Setting patch #7 as the current one.

Adding intermediate 8.7.3 backport patch.

Fixed backport patch.

Great effort on this one. I've reviewed the code, have done some testing and here are my findings.

>>> \Drupal\workspaces\Entity\Workspace::load('live')->label();
=> "Live"
>>> \Drupal\workspaces\Entity\Workspace::load('live')->parent->entity->label();
=> "Live"

The live workspace was its parent on a clean installation. This brought a few more issues. On this line in the WorkspaceStorage::loadTree()

$graph[$id]['edges'][$entity->parent->target_id] = TRUE;

a vertex from 'live' to 'live' was added which caused the graph to no longer be acyclic. This violated the input conditions of the Graph component and results in unexpected output

>>> $tree = \Drupal::entityTypeManager()->getStorage('workspace')->loadTree();
=> [
     "live" => Drupal\workspaces\Entity\Workspace {#4087},
     "stage" => Drupal\workspaces\Entity\Workspace {#4085},
   ]
>>> $tree['stage']->parent->entity->label();
=> "Live"
>>> $tree['stage']->_depth;
=> 0

The _depth property was zero regardless of the position in the tree.

Here's a summary of the changes introduced in this comment. This patch:

1. Adds a generic NoSelfReference constraint for entity_reference fields that prevents an entity from referencing itself.
2. Sets that constraint on the workspace parent field.
3. Makes sure the root workspace never has a parent, even when it's created programmatically bypassing the validation, by setting it to null in preSave.
4. Triggers a re-save of the existing default workspace in a post_update hook.
5. Adds test coverage for the constraint and the root workspace parent field.
6. Changes the output structure of WorkspaceStorage::loadTree to improve DX. The dynamic properties added to the full-blown objects were invisible in the debugger.
7. Adds ::getAncestors(), ::getDescendants() and ::getDepth() to the workspace class and interface.
8. Fixes the entry indentation on the Workspaces list page.
9. Adds test coverage for the loadTree method.

The combined patch contains is based on the most recent patch in #3062434: Track the workspace of a revision in a base field and convert the workspace_association entity type to a custom index (#16).

We had a discussion in slack with @amateescu and he said that none of the changes in the patch in #14 are worth pursuing, so reverting the display files back to the state from #13.

Here is an initial review/questions at code level (didn't apply and check manually yet).

1. +++ b/core/modules/workspaces/src/Plugin/EntityReferenceSelection/WorkspaceSelection.php
   @@ -0,0 +1,64 @@
   + *   id = "default:workspace",
   

   don't feel it matches with what this field is for?

2. +++ b/core/modules/workspaces/src/Plugin/EntityReferenceSelection/WorkspaceSelection.php
   @@ -0,0 +1,64 @@
   +    $form['sort']['#access'] = FALSE;
   

   Do we have to do anything similar for $limit as well?

3. +++ b/core/modules/workspaces/src/Plugin/EntityReferenceSelection/WorkspaceSelection.php
   @@ -0,0 +1,64 @@
   +    if ($match || $limit) {
   

   could we add a comment why we don't do anything special if $match or $limit set here?

4. +++ b/core/modules/workspaces/src/WorkspaceAssociation.php
   @@ -47,16 +47,59 @@ public function __construct(Connection $connection, EntityTypeManagerInterface $
   +    // Insert a new index entry for each workspace that is not tracking this
   +    // entity yet.
   

   It means, from the next revision update, these workspaces also considered tracking the entity?

I've done some testing and it looks good in general.

My concerns:

1. Currently there is not possible to deploy from a child workspace neither to parent or to Live workspace. The user is allowed to access the Deploy page, can see the items that can be deployed, but when tries to deploy them, is redirected to the list of workspaces and this error message is displayed: Only top-level workspaces can be published.. This is not a good user experience and after discussing with @amateescu, he said the plan is to make possible to deploy from child to parent workspaces. I think that would be the correct behavior.
2. There should be added a test for the case when deleting a parent workspace.

1. Currently there is not possible to deploy from a child workspace neither to parent or to Live workspace. The user is allowed to access the Deploy page, can see the items that can be deployed, but when tries to deploy them, is redirected to the list of workspaces and this error message is displayed: Only top-level workspaces can be published.. This is not a good user experience and after discussing with @amateescu, he said the plan is to make possible to deploy from child to parent workspaces. I think that would be the correct behavior.

The issue that will address this is: #3070788: Add support for merging the contents of two workspaces

2. There should be added a test for the case when deleting a parent workspace.

Sounds indeed like good tests to add.

Andrei and I walked through the entire patch and did a whole round of manual testing and found a few things to tweak. I'll be back in a few hours to give this one another review and full end-to-end test.

Thanks @amateescu! The patch looks good and functionality works as expected!

It's not trivial to describe or let alone imagine all possibilities this patch brings to Drupal 8! Content inheritance between workspaces is simply AMAZING! But if I try summarise this in a few bullet points;

• Editorial experience is greatly enhanced for workspace sites that have a lot of "living" content. Instead of constantly having to "pull down" content into lower workspaces published content in higher workspace is now automagically inherited by lower workspaces.
• Entirely new editorial workflows are possible – sub campaign workspaces, regional or country workspaces that inherit content, etc.
• Expose a JSON API per workspace and you suddenly have an incredibly competent content-hub solution!
• Domain module anyone?

Apart from the solid test coverage in this patch I have done a lot of manual testing and everything works as expected. Below are some demos of a few notable things.

Basic demo

In this first video I demo a basic workflow that shows the content inheritance from Live down through two campaign workspace ("Top campaign" and "Sub campaign"):

https://www.youtube.com/watch?v=AjKPC3PYXVk

Conflicting drafts demo

This is an important (but logical) gotcha. A node can only have unpublished drafts in a single workspace. The user is currently prevented in the UI to create conflicting drafts. This is the simplest and most logical solution until we get this into Drupal 9: #2725523: Add a revision_parent field to revisionable entities

https://www.youtube.com/watch?v=UBuF33zc0ts

Deleting content demo

While revision changes are contained in separate workspaces, deleting a node affects all workspaces. Ideally deleting a node in a sub-workspace would create a new "softly deleted" revision that should be merged and published upstream as any other revision change. However, that's to be considered separate from this issue as we need to extend the delete API with some significant changes in order to support this in Drupal 9. For background on this see: #2786135: WI: Phase E: Introduce Trash module

https://www.youtube.com/watch?v=waKELEDkKSI

Just got off a call with @webchick where I walked her through the functionality end-to-end. She'll drop in her own comments later, but here's the main take-away from our conversation:

Webchick rightfully pointed out that deleting content in a workspace or sub-workspace which also wipes the content from all other workspaces is a no-go. Workspaces should be considered "a safe space" for making any changes and potentially destructive changes should not be allowed. I agree with this.

What we came up with is the following:

• Content with a published revision in live can only be deleted in the live workspace
  • The user interface should not let the user take the delete action, i.e. disable the delete confirmation form and bulk operation forms
  • This would also make the behaviour kind of consistent with us already not allowing conflicting draft revisions in workspaces as we disable the content editing form in that case
• Only content that's not been published to any other workspace should be able to be deleted in that very workspace

Wow, that's a great summary! :)

I will say, with my "product manager" hat on, that this truly seems like a really amazing feature, and I'd love to see it make it into Drupal 8 still, though we are now past alpha deadline so this isn't my call alone.

The entire point of Workspaces is to provide a "true" preview of changes before they happen. That currently is not possible in core, because once you've triggered a workspace, there's no way to update it with changes that happen subsequently. So while you can preview the new things you are adding within the workspace on their own, you can't preview them in context, like how they'll look on the front page (which now has so much new content it'll move yours out of the "featured" block entirely once you've deploy it, for example).

In contrib, @dixon_ mentioned that there's a solution that uses a manual "pull" content model, but editors find this a confusing model to work with (which makes sense; it's a developer's model). The current patch is much more intuitive, because it handles the "pulling" for you automatically.

But yes, unfortunately I do think the current delete logic is too dangerous, and works against user expectations. They expect a workspace to be a "private sandbox" for playing in, and if deleting content from there also deleted it from other peoples' workspaces or—even worse—from Live, that would be very unexpected/shocking behaviour, I feel. :\

I'm really hoping that since this "deny" logic is already figured out for editing, it will hopefully not be too much more work to do something similar for delete.

Also, I have to say that I really appreciate all of the thought/care that went into preparing the issue summary, preparing the video walkthroughs of how it works (and also the being up until stupid o'clock to run through it, but :P). That's SUPER helpful for orienting people who are new to the issue (like me!).

So yes, sadly back to needs work, but thanks so much for pushing on this!

Expose a JSON API per workspace and you suddenly have an incredibly competent content-hub solution!

😲🥳🤯

I'd sure love to hear more about this, and I think it'd really help if the issue summary explained this :)

Took this for a quick spin, and overall it looks really good. The functionality works as expected. I'll post a longer review tomorrow with a demo video.

There's one issue with the patch, it throws a type error in certain circumstances because WorkspaceManager::getActiveWorkspace() can return FALSE. Here's a quick fix I implemented while testing.

And attaching the fix I talked about as an interdiff to the patch in comment 41.

Gone through and done plenty of manual scenario testing on this latest patch, which also fixes the type error I discovered.

IMO we're good to go! :)

Did a review mainly for data integrity stuff, questions below:

1. +++ b/core/modules/workspaces/src/Plugin/EntityReferenceSelection/WorkspaceSelection.php
   @@ -0,0 +1,72 @@
   + * Provides specific access control for the workspace entity type.
   

   This class isn't providing access control afaict.

2. +++ b/core/modules/workspaces/src/WorkspaceAssociation.php
   @@ -48,16 +58,60 @@ public function __construct(Connection $connection, EntityTypeManagerInterface $
   +    }
   +
   +    // Update all affected workspaces that were tracking the current revision.
   +    // This means they are inheriting content and should be updated.
   +    if ($tracked_revision) {
   +      $this->database->update(static::TABLE)
   +        ->fields([
   +          'target_entity_revision_id' => $entity->getRevisionId(),
   +        ])
   +        ->condition('workspace', $affected_workspaces, 'IN')
   +        ->condition('target_entity_type_id', $entity->getEntityTypeId())
   +        ->condition('target_entity_id', $entity->id())
   +        // Only update descendant workspaces if they have the same initial
   +        // revision, which means they are currently inheriting content.
   +        ->condition('target_entity_revision_id', $tracked_revision)
   +        ->execute();
   +    }
   +
   +    // Insert a new index entry for each workspace that is not tracking this
   +    // entity yet.
   +    $missing_workspaces = array_diff($affected_workspaces, $this->getEntityTrackingWorkspaceIds($entity));
   +    if ($missing_workspaces) {
   +      $insert_query = $this->database->insert(static::TABLE)
   +        ->fields([
   +          'workspace',
   +          'target_entity_revision_id',
   +          'target_entity_type_id',
   +          'target_entity_id',
   +        ]);
   +      foreach ($missing_workspaces as $workspace_id) {
   +        $insert_query->values([
   +          'workspace' => $workspace_id,
   +          'target_entity_type_id' => $entity->getEntityTypeId(),
   +          'target_entity_id' => $entity->id(),
   +          'target_entity_revision_id' => $entity->getRevisionId(),
   +        ]);
   +      }
   +      $insert_query->execute();
   +    }
   +
   

   Does this need to be in a transaction now it's more than one query?

3. +++ b/core/modules/workspaces/src/WorkspaceAssociation.php
   @@ -158,6 +212,78 @@ public function deleteAssociations($workspace_id, $entity_type_id = NULL, $entit
   +  public function isEntityDeletable(RevisionableInterface $entity, WorkspaceInterface $workspace) {
   +    if (!isset($this->deletableEntityIds[$workspace->id()][$entity->getEntityTypeId()])) {
   +      // First, get all the unpublished default revisions.
   +      $unpublished_default_revisions = $this->entityTypeManager->getStorage($entity->getEntityTypeId())
   +        ->getQuery()
   +        ->accessCheck(FALSE)
   +        ->condition($entity->getEntityType()->getKey('published'), 0)
   +        ->addMetaData('skip_workspaces_alter', TRUE)
   +        ->execute();
   +
   +      // If there are no unpublished default revisions, the entity can not be
   +      // deleted.
   +      if (!$unpublished_default_revisions) {
   +        $this->deletableEntityIds[$workspace->id()][$entity->getEntityTypeId()] = [];
   +        return FALSE;
   +      }
   +
   

   Reading this through, I was really confused why we're checking all entities when we only need the information for one. This could do with an explicit comment that we're initializing the property for all entities up front for performance reasons or similar. Although also are we really checking deletability for multiple entities during the same request much?

   Also what about moving the pre-load initialization stuff into a protected method?

4. +++ b/core/modules/workspaces/src/WorkspaceMerger.php
   @@ -0,0 +1,209 @@
   +
   +    $transaction = $this->database->startTransaction();
   +    try {
   

   Thanks for adding a transaction here.

5. +++ b/core/modules/workspaces/src/WorkspaceMerger.php
   @@ -0,0 +1,209 @@
   +   */
   +  public function checkConflictsOnTarget() {
   +    // Nothing to do for now, we can not get to a conflicting state because an
   +    // entity which is being edited in a workspace can not be edited in any
   +    // other workspace.
   +  }
   +
   

   Given we're merging entity changes from one workspace to another how does this remain true now?

6. +++ b/core/modules/workspaces/src/WorkspaceStorage.php
   @@ -0,0 +1,179 @@
   +   * @internal
   +   */
   +  public function loadTree() {
   +    if (!isset($this->tree)) {
   +      $cache = $this->cache->get('workspace_tree');
   +      if ($cache) {
   +        $this->tree = $cache->data;
   +        return $this->tree;
   +      }
   +
   +      $entities = $this->loadMultiple();
   +
   +      // First, sort everything alphabetically.
   

   Published workspaces get deleted still right? Just wondering how finite every workspace on the site is going to be.

7. +++ b/core/modules/workspaces/src/WorkspaceStorage.php
   @@ -0,0 +1,179 @@
   +    Cache::invalidateTags(['workspace_list']);
   

   Should this come from the entity type or similar?

This is an awesome addition and code looks very good to me. There are a couple of fixes still needed and a few clarifications pending (mainly the deletion logic), but I think we're very close!

1. +++ b/core/modules/content_moderation/tests/src/Kernel/WorkspacesContentModerationStateTest.php
   @@ -164,6 +164,17 @@ public function basicModerationTestCases() {
   +    // ::testContentModerationStateRevisionDataRemoval().
   

   I didn't immediately realize this method is defined on the parent class: I'd add parent:: here to make it more obvious.

2. +++ b/core/modules/workspaces/config/install/core.entity_form_display.workspace.workspace.deploy.yml
   @@ -11,4 +11,5 @@ bundle: workspace
   +  parent: true
   

   Don't we need a (post) update for this?

+++ b/core/modules/workspaces/src/Plugin/Validation/Constraint/EntityWorkspaceConflictConstraintValidator.php
@@ -69,17 +69,26 @@ public static function create(ContainerInterface $container) {
+      // If the latest revision of the entity is tracked in a workspace, it can
+      // only be edited in that workspace or one of its descendants.
+      if ($latest_revision_workspace = $latest_revision->workspace->entity) {

I might be wrong but I didn't spot any explicit test coverage for this. I think it would be important to have both functional and integration test coverage to check both the form and the validator itself.

3. +++ b/core/modules/workspaces/src/Entity/Workspace.php
   @@ -141,6 +153,26 @@ public function setCreatedTime($created) {
   +    // When a new workspace has been saved, we need to copy all the associations
   +    // of its parent.
   +    if (!$update && !$this->parent->isEmpty()) {
   +      \Drupal::service('workspaces.association')->initializeWorkspace($this);
   +    }
   

   Personally I'd prefer if this kind of changes happened in the storage: I realize that this may imply duplicated logic when writing an alternative storage backend, however we keep introducing dependencies from domain objects to services, which doesn't feel right. IMO these methods should be used only to manipulate the entity/field data/state.

   Just saying, since we're already doing something similar in ::postDelete(), so nothing really new added by this.
   Rant over :)

4. +++ b/core/modules/workspaces/src/Form/WorkspaceDeleteForm.php
   @@ -66,17 +66,29 @@ public function __construct(EntityRepositoryInterface $entity_repository, Worksp
   +    foreach ($this->workspaceAssociation->getTrackedEntities($this->entity->id()) as $entity_type_id => $entity_ids) {
   +      if ($revision_ids = $this->workspaceAssociation->getAssociatedRevisions($this->entity->id(), $entity_type_id, $entity_ids)) {
   +        $label = $this->entityTypeManager->getDefinition($entity_type_id)->getLabel();
   +        $items[] = $this->formatPlural(count($revision_ids), '1 @label revision.', '@count @label revisions.', ['@label' => $label]);
   +      }
   ...
   +      '#access' => !empty($items),
   

   Is this change strictly related/required?

5. +++ b/core/modules/workspaces/src/Plugin/Validation/Constraint/EntityWorkspaceConflictConstraintValidator.php
   @@ -69,17 +69,26 @@ public static function create(ContainerInterface $container) {
   +        $descendants_and_self = array_merge([$latest_revision_workspace->id()], $workspace_tree[$latest_revision_workspace->id()]['descendants']);
   

   I'm wondering whether we could add a helper method to the storage to do this, something like ::loadSubTree(). This would of course rely on ::loadTree() internally.

6. +++ b/core/modules/workspaces/src/WorkspaceAssociation.php
   @@ -31,6 +31,16 @@ class WorkspaceAssociation implements WorkspaceAssociationInterface {
   +  protected $deletableEntityIds = [];
   

   Would it make sense to rely on MemoryCache for this?

7. +++ b/core/modules/workspaces/src/WorkspaceAssociation.php
   @@ -48,16 +58,60 @@ public function __construct(Connection $connection, EntityTypeManagerInterface $
   +    $tracked_revision = NULL;
   

   Nit: I'd call this $tracked_revision_id to improve readability.

8. +++ b/core/modules/workspaces/src/WorkspaceAssociation.php
   @@ -158,6 +212,78 @@ public function deleteAssociations($workspace_id, $entity_type_id = NULL, $entit
   +        if ((int) $revision_count_value[$revision_count_key] - 1 === $associated_revisions[$revision_count_value['nid']]) {
   

   s/'nid'/$id_key :)

9. +++ b/core/modules/workspaces/src/WorkspaceMerger.php
   @@ -0,0 +1,209 @@
   +   * Constructs a new WorkspacePublisher.
   

   WorkspaceMerger :)

10. +++ b/core/modules/workspaces/src/WorkspaceMerger.php
    @@ -0,0 +1,209 @@
    +        // Since we're not saving entity objects, we need to invalidate the list
    +        // cache tags manually.
    +        $entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
    +        $this->cacheTagsInvalidator->invalidateTags($entity_type->getListCacheTags());
    

    This seems an implementation detail to me, I'd move it into ::trackEntity. If invalidating at each entity tracking is expensive, we could add a multiple version of track entity (::trackEntities()?) invalidating only at the end of the process.

11. +++ b/core/modules/workspaces/src/WorkspaceMerger.php
    @@ -0,0 +1,209 @@
    +  public function getDifferringRevisionIdsOnTarget() {
    

    It seems to me that this logic could be factored out to a (protected) helper method that both ::getDifferringRevisionIdsOnTarget() and ::getDifferringRevisionIdsOnTarget() could rely on.
    (Even if we only have two of them ;)

12. +++ b/core/modules/workspaces/src/WorkspaceStorage.php
    @@ -0,0 +1,179 @@
    +      $entities = $this->loadMultiple();
    

    Nit: I'd rename this to $workspaces and $id to $workspace_id to improve readability: in the middle of the review I was wondering whether there were also tracked entities involved in the process besides workspaces.

13. +++ b/core/modules/workspaces/src/WorkspaceStorage.php
    @@ -0,0 +1,179 @@
    +      uasort($entities, function ($a, $b) {
    

    Missing type hints.

14. +++ b/core/modules/workspaces/src/WorkspaceStorage.php
    @@ -0,0 +1,179 @@
    +      // Keeps track of the parents we have to process, the last entry is used
    +      // for the next processing step.
    

    I'd explicitly mention that roots are stored with a NULL parent: I had to read this a few times before grasping the logic.

15. +++ b/core/modules/workspaces/src/WorkspaceStorage.php
    @@ -0,0 +1,179 @@
    +          $child = current($tree_children[$parent]);
    

    $child_id :)

16. +++ b/core/modules/workspaces/tests/src/Functional/EntityResource/WorkspaceResourceTestBase.php
    @@ -111,6 +111,7 @@ protected function getExpectedNormalizedEntity() {
    +      'parent' => [],
    

    Is this a BC break for API clients? Not sure what the policies are in this case.

17. +++ b/core/modules/workspaces/tests/src/Functional/WorkspaceEntityDeleteTest.php
    @@ -0,0 +1,173 @@
    +    $assert_session->linkByHrefExists($unpublished_live->toUrl('delete-form')->toString());
    ...
    +    $assert_session->linkByHrefExists($unpublished_live->toUrl('delete-form')->toString());
    ...
    +    $assert_session->linkByHrefExists($unpublished_stage->toUrl('delete-form')->toString());
    

    Based on @webchick's feedback I was assuming this would work quite differently: I'd expect an entity to be deletable only if it was created in the active workspace and if descendant workspaces have no pending changes for it, regardless of its publishing state. Otherwise I could create an unpublished default article in live, plannning to edit it later, just to see it disappear because it was deleted in the stage workspace by an editor assuming to act only in the stage scope.

18. +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceCRUDTest.php
    @@ -294,4 +294,29 @@ public function testDeletingPublishedWorkspace() {
    +    $dev->delete();
    +    $stage->delete();
    

    Can we explicitly assert that deletion was successful?

19. +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceMergerTest.php
    @@ -0,0 +1,209 @@
    +    // Check that Local 1 can not be merged directly into Stage, since it can
    +    // only be merged into its direct parent.
    

    This may seem minor, but it would have saved me a minute or so of head scratching: can we move this comment below, right before the assertion block, and replace it with something like "// Check that there is no content in Stage that's not also in Local 1."? I spent some time wondering why an empty result implied that the merge could not happen :D

20. +++ b/core/modules/workspaces/workspaces.install
    @@ -158,3 +159,24 @@ function workspaces_update_8801() {
    +  // Update the storage class of the 'workspace' entity type.
    +  $entity_type = $entity_definition_update_manager->getEntityType('workspace');
    +  $entity_type->setStorageClass(WorkspaceStorage::class);
    +  $entity_definition_update_manager->updateEntityType($entity_type);
    

    We should do this only if the default storage class is set.

While manually testing the patch, I found this (minor) UX issue that seems to be pre-existing, since it happens also with the deploy action: operation links are not hidden when the operation is not allowed. Maybe there's already an issue for this, didn't check.

Edit: typos

Based on @webchick's feedback I was assuming this would work quite differently: I'd expect an entity to be deletable only if it was created in the active workspace and if descendant workspaces have no pending changes for it, regardless of its publishing state. Otherwise I could create an unpublished default article in live, and plan to edit it later, just to see it disappear because it was deleted in the stage workspace by an editor assuming to act only in the stage scope.

I got a bit confused when reviewing for the wrong reasons - I was thinking about other workspaces, which of course we prevent editing in so that shouldn't happen, and thought 'oh OK then', but yes definitely creating an unpublished entity is still independently valid on live so seems like an extra check would be useful.

Based on @webchick's feedback I was assuming this would work quite differently: I'd expect an entity to be deletable only if it was created in the active workspace and if descendant workspaces have no pending changes for it, regardless of its publishing state. Otherwise I could create an unpublished default article in live, plannning to edit it later, just to see it disappear because it was deleted in the stage workspace by an editor assuming to act only in the stage scope.

That is an accurate representation for how I expected the delete logic design to work.

:)

Interdiffs look great!

@amateescu, #53:

2: Ok, I hope it won't be forgot ;)
3: Good call!
6: Ok, we can always switch to MemoryCache later, if needed.
10: Makes sense, thanks for clarifying
11: 👍
16: 👍
20: 👍

1. +++ b/core/modules/workspaces/src/WorkspaceAssociation.php
   @@ -122,6 +128,17 @@ public function trackEntity(RevisionableInterface $entity, WorkspaceInterface $w
   +  public function workspaceInsert(WorkspaceInterface $workspace) {
   +    // When a new workspace has been saved, we need to copy all the associations
   +    // of its parent.
   +    if ($workspace->hasParent()) {
   +      \Drupal::service('workspaces.association')->initializeWorkspace($workspace);
   +    }
   +  }
   

   $this->initializeWorkspace() :)

2. +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceMergerTest.php
   @@ -184,9 +184,9 @@ public function testWorkspaceMerger() {
   -    // Check that Local 1 can not be merged directly into Stage, since it can
   -    // only be merged into its direct parent.
   

   Nit: we could restore this comment right before the final assertions.

Reviewed all the changes since my initial patch and also ran the tests of our projects that build on top of it.

Good to go from my side!

@amateescu It makes sense to address the delete behaviour in a separate patch now that we've decided to keep Workspaces module as experimental.

I gave this latest patch a whole new round of testing (now disregarding the fact that deletes affect all workspaces). All is looking good. All comments have been addressed. Back to RTBC!

I spoke with @webchick and she is ok with committing this as-is, given that the issue is pre-existing, which #3092247: Prevent content from being deleted when there is an active workspace nicely proves. The follow-up should be a stable-blocker, though.

Given that, I went ahead and committed/pushed f10a397 to 9.0.x and friends. Congratulations!

Marking this as needing work, since the release notes snippet is missing as well as a CR announcing this new functionality to site builders and editors.

Thanks everyone who's been involved here! 🎉 This is a major piece of functionality that just landed!

Thank you all!

Exciting stuff, folks! Thanks for all your hard work!

+++ b/core/modules/workspaces/src/Entity/Workspace.php
@@ -100,6 +100,17 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
+    $fields['parent'] = BaseFieldDefinition::create('entity_reference')
+      ->setLabel(new TranslatableMarkup('Parent'))
+      ->setDescription(new TranslatableMarkup('The parent workspace.'))
+      ->setSetting('target_type', 'workspace')
+      ->setReadOnly(TRUE)
+      ->setDisplayConfigurable('form', TRUE)
+      ->setDisplayOptions('form', [
+        'type' => 'options_select',
+        'weight' => 10,
+      ]);

+++ b/core/modules/workspaces/workspaces.install
@@ -158,3 +158,19 @@ function workspaces_update_8801() {
+  $storage_definition = BaseFieldDefinition::create('entity_reference')
+    ->setLabel(t('Parent'))
+    ->setDescription(t('The parent workspace.'))
+    ->setSetting('target_type', 'workspace')
+    ->setReadOnly(TRUE);

I was wondering should we enforce max_length here as workflow ID has ->setSetting('max_length', 128) but the question is can we enforce lenght on ER field target_id field?