Introducing the concept of workspaces. Content entities always belong to a workspace (there is one main exception, which is the user entity type). A workspace is a silo/container of content on a site. However, this phase only introduces the underlying concept with one single workspace available, without many supporting APIs around it (see later phases). This phase will not change any UIs or behaviours in core.

See Workspace module for the current contrib implementation.

  • Define the workspace entity itself
  • Introduce the workspace reference field for all content entity types
  • Extend storage handlers to work with workspaces

Architecture overview

  • Workspaces are content entities.
  • A "live" and "stage" workspace is created on installation.
  • The default workspace (the one anonymous users see) is set in workspace.services.yml as a parameter.
  • Workspaces depend on revisionable and publishable content, therefore only Nodes are supported.
  • No queries, entity loads, or anything are altered for the default workspace.
  • Content is linked to a workspace via a ContentWorkspace entity (similar to Content Moderation's ContentModerationState entity).
  • All content added on non-default workspaces is saved as unpublished. The real published states is saved against the ContentWorkspace entity.
  • There is a computed entity_reference field "workspace" added to all entity types that can belong to a workspace.
  • An entity query alter is performed on non-default workspaces to join the ContentWorkspace entity field revision table and only load entities which are in the active (current) or default workspace.
  • A Views query alter is done in the a similar way to the entity query alter, although here we also take into account the published status from the ContentWorksapce entity if the View has a published filter.
  • In hook_entity_load we switch out the entity revision for the correct one for the current active workspace based on data stored in the ContentWorkspace entity. The published status is also switched if needed.
  • Each workspace can have an upstream, this is like a git upstream, when deploying content is replicated to the upstream.
  • Upstreams are plugins. In the workspace module these are derived from workspaces. Contrib could hook into this to make upstreams anything, other sites, non-drupal apps, etc.
  • Replication is handled by a tagged service, again, this allows contrib to hook in and provide different replication.
  • The default replicator looks for changes between two workspaces then loops through all these revisions and saves them on the target workspace.
  • Each replication creates a replication log entity. For the default replicator this is based on an MD5 hash of the source and target workspaces, so when replicating again between those two workspaces we know what replications have already happened.
  • All entity updates create an entity in the sequence index. This index is also used in replication two find the changes since the last replication.
  • The user's active workspace is determined via a negotiator, so it can either come from the container parameter (default workspace), a session value, or a url query parameter.
  • There is complex access checks which will allow you to lock down users to only create content in specific workspaces.
  • The active workspace is switched by either using the toolbar.module integration, or enabling a switcher block.

Comments

timmillwood created an issue. See original summary.

timmillwood’s picture

Opening this issue to discuss the implementation details for workspaces.

In the Multiversion and Workspace modules we define workspaces as a content entity, then add a workspace field to all entity types to denote which workspace an entity belongs to, then every query has to check the workspace field.

I can't see this passing as a core solution, there are many alternatives, but none that get around adding a condition to each entity query.

timmillwood’s picture

Issue tags: +Workflow Initiative
catch’s picture

For CPS there is a similar concept (called 'site versions'). The way it gets around the condition on every query was the following:

1. There is a hard-coded, 'published' site version. When viewing the 'published' site (i.e. not previewing), any query alters are skipped.

2. When previewing the site in a site version, you have to join on the tracking table, can't really avoid that.

3. Additionally, only entities modified in a site version are tracked in that table - it uses a CASE IF WHEN clause in the query alter to either get the default revision for untracked entities or the draft revision for entities in the workspace.

I'm not sure what the storage should look like for core, but conceptually this means the best performance when not previewing (i.e. no change at all), and allows for workspace storage to only track changes, not every entity. (After a site version is published the full history is stored, which does/has caused problems with scaling, but there are issues discussing that in the CPS queue).

Publishing a site version is then taking the 'target' revision for each entity and making it the default revision (then changing status of the site version itself) - this means the full process of publishing is usually a handful of entity saves, which can happen in a single transaction.

Fabianx’s picture

+1 to #4.

The query alter only when not in the 'published' workspace, did work very well for us.

Also a table + COALESCE() on the join did also wonders for us.

I think COALESCE is supported also for other DB engines.

Edit:

Uhm and cloning entities per workspace is a no-go for core. Should just use revisions and track those, but always work on 'live' if not overridden in the workspace (e.g. similar to an always rebased branch).

That also worked very well and scalable for us in CPS.

timmillwood’s picture

Coming back to this (21 days later) I want to make sure I fully understand how this all works.

  • The "Live" workspace behaves exactly how standard Drupal works, all the same queries, all the same entity api.
  • When content is added to the "stage" workspace how do we prevent it from appearing on "Live"?
  • If an entity is added on Workspace "A" and another added on Workspace "B" we can join the tracking table to know which workspace should show which entity, but on live we would not join the tracking table, so what would stop it showing all entities?

Then... where do we add the under laying concept of workspace? is adding it in a module stable enough? or should it go into core, much like translations are. Yes, to do translations you need the translation modules, but the entity API can do translations without it.

catch’s picture

When content is added to the "stage" workspace how do we prevent it from appearing on "Live"?

1. Save an initial, default revision. Regardless of the 'published' status of the entity, set it to unpublished. This way it can never show up on the live site.

2. Save a draft revision, with the published status however it was set on the entity, this one is attached to the workspace. Since this will be 'published' in the workspace (if it was set to published on the form), it'll show up.

CPS does this, except the revision saving is tricky because Drupal 7 doesn't allow you to save a revision without updating the base table, but the end result is the same.

This requires published/unpublished support for all entities that can be put into workspaces.

Not sure on the where question. First inclination is to put it in a module though, its not really needed except for whole-site-preview and publishing-sets-of-content-at-once, and sites can be built without either of those.

timmillwood’s picture

So if all content is added as unpublished on all workspaces no matter which workspace the content was added, wouldn't this get confusing to people viewing /admin/content?

Fabianx’s picture

#8: A further part that CPS does here is to remove unpublished content (unpublished on the base entity) from listings while outside of a changeset / site version (e.g. in the live changeset).

And because you cannot just un-publish something, but need to go via the workflow, that works very well.

At least that is how I remember this works.

timmillwood’s picture

I think for this to work we need to be able to unpublished all of the things. #2810381: Add generic status field to ContentEntityBase is looking to introduce a generic 'status' base field, which we can then add to more entity types.

My current thinking is:

  • Workspace content entity to define workspaces
  • All content would belong in the "live" workspace
  • When content is created or updated in a "non-live" workspace we update a table (or entity) to denote which workspace the new entity/revision belongs to.
  • Replicating content from a non-live to a live workspace is just a case of removing the entry from the table (or entity), at which point the entity will "belong" to the live workspace.
catch’s picture

Replicating content from a non-live to a live workspace is just a case of removing the entry from the table (or entity), at which point the entity will "belong" to the live workspace.

For reference the way CPS does this is the following:

1. Publishing a workspace means 'make the revision associated with the workspace/site version the default one' - which either publishes a draft revision or publishes the entity in general if it's new). Ideally that should happen in a single transaction.

2. The workspace/site version gets 'archived' - so the historical information of what was in it is available. (CPS also records the state of all entities on the site at the time it was published, for historical review and rollback which I don't think we should do in core since it's an implementation nightmare).

timmillwood’s picture

Today I committed a 8.x-2.x branch of the Workspace module. I hope this will form the basis for the core experimental module.

So far I have:

  • Moved parts of Multiversion into Workspace
  • Removed all dependencies
  • Confirmed workspace types can be added
  • Confirmed workspaces can be added
  • Confirmed workspaces can be switched between using the toolbar
  • Confirmed the active workspace state is retained

Todo:

  • More tidy up
  • Make tests pass
  • Mark content as belonging to a workspace

During this time I also need to work out how replicating content will actually work. The CPS approaches above will play a big part in this. I would also like to use the services we have in the Replication module to determine differences between workspaces. This will allow us to be flexible enough for multi-site replication as well as the core single site preview functionality.

To mark content as belonging to a workspace I'd like to use the Content Moderation approach of a content entity and computed entity reference fields. Thus adding a ContentWorkspace entity which references the content entity and the workspace it belongs to.

For the CPS approach to fully work we need BlockContent, Term, and MenuLinkContent entities (at a minimum) to be unpublishable, #2810381: Add generic status field to ContentEntityBase will go a long way to making this happen.

timmillwood’s picture

The 8.2.x branch of Workspace module now has a ContentWorkspace entity type which gets updated when an entity is created or updated. The workspace field is a computed field which returns the Workspace via ContentWorkspace, or the default Workspace.

Trying to work out the best way of getting entities to only show in their correct workspace.

timmillwood’s picture

Status: Active » Needs review
FileSize
131.05 KB

Here's an initial patch for Workspace module.

It will fail on a couple of things (like hook_help), but I will work on tidy ups today.

What it does do is introduce WorkspaceType and Workspace entities, it then has toolbar integration to switch between workspaces. Every entity that is added gets a ContentWorkspace entity to map it to a workspace, much like in Content Moderation how we map an entity to it's moderation state.

One thing I'd like to get done before commit is only showing entities which belong to the current workspace, but still looking for the best way to do this. @catch? @Fabianx?

Replication is something I think should be added as a follow-up, yes, the module is pretty useless without replication, but to do it properly it has a lot of dependencies.

Status: Needs review » Needs work

The last submitted patch, 14: 2784921-14.patch, failed testing.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
132.72 KB
3.44 KB

Trying to fix some of the failures from #14

Status: Needs review » Needs work

The last submitted patch, 16: 2784921-16.patch, failed testing.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
133.77 KB
1.05 KB

This should fix the \Drupal\system\Tests\Module\InstallUninstallTest issue.

After speaking to @amateescu and @dixon there was concerns about getting Workspace module in without replication. Personally I'd like to get it in as a starting block so we can then work on multiple little patches, rather than one big one. Although I can understand the concerns with having a module in core that doesn't really do much.

dawehner’s picture

Well, one question I would ask myself here. What is the concrete usecase we solve with this additional experimental module. Can endusers somehow profit from it? Having an experimental module without any enduser features is indeed a big weird. I'm wondering whether we could implement a mini version of replication, or some other feature which workspaces would allow, even if it would be thrown away later.

timmillwood’s picture

@dawehner - I think a mini version of replication would be cool, I remember we did do this in the early days of the contrib module and it did have some big bugs, but is specific use cases it worked ok. So maybe this could be one approach.

The other approach is just develop it in contrib for now, then push to core as one big patch when ready. To do replication properly (following the couchdb protocol) there are many dependencies.

timmillwood’s picture

Status: Needs review » Needs work

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

timmillwood’s picture

Status: Needs work » Needs review

I have been working on the workspace module for core in the 8.x-2.x branch of contrib, I'd happily take reviews there https://www.drupal.org/node/11627/git-instructions/8.x-2.x/nonmaintainer

It's still in pretty early stages, but taking influence from CPS no queries are altered on the "live" workspace, on all other workspace we alter queries, and hook into the node load to only show entities that are live or belong to the workspace. Basic replication of content between workspaces is also in place with a test to back this up.

xjm’s picture

Status: Needs review » Postponed

@timmillwood commented on #2755073: WI: Content Moderation module roadmap that the Workflow team wanted to target this for 8.4.

It's a ways down the roadmap in #2721129: Workflow Initiative and hadn't been discussed with release managers yet. We'd like to see Content Moderation and Workflow stabilize before we add additional workflow features to core. We'd also like to ensure bugs like #2856363: Path alias changes for draft revisions immediately leak into live site and issues with forward revisions are addressed before this is considered for core (they are probably also blockers for Content Moderation being stable anyway).

So, marking postponed for now based on that. Thanks @timmillwood!

xjm’s picture

FWIW, #1239558: Deleting an entity with revisions and file/image field does not release file usage of non-default revisions, causing files to linger was also a must-have for Phase A and not complete, and this is Phase F. (Also the revision upgrade path is still listed as Phase A, but I'm unclear on the status of that.)

timmillwood’s picture

Status: Postponed » Needs review
FileSize
150.45 KB
203.55 KB

Time for an update, lets see if this works.

Status: Needs review » Needs work

The last submitted patch, 26: 2784921-26.patch, failed testing.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
458 bytes
203.64 KB

Let's give this another go.

Status: Needs review » Needs work

The last submitted patch, 28: 2784921-28.patch, failed testing.

timmillwood’s picture

Status: Needs work » Needs review

ok, fixed this test fail in contrib. Will add a new patch here in due course, but until then... please review!

timmillwood’s picture

Issue summary: View changes
timmillwood’s picture

Status: Needs review » Needs work

The last submitted patch, 32: 2784921-32.patch, failed testing.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
215.73 KB

#32 was missing the following files:

	core/modules/workspace/css/workspace.toolbox.css
	core/modules/workspace/icons/ffffff/ws_icon.svg
	core/modules/workspace/js/
	core/modules/workspace/templates/workspace-toolbox.html.twig
	core/modules/workspace/tests/src/Functional/ExistingContentTest.php

Status: Needs review » Needs work

The last submitted patch, 34: 2784921-34.patch, failed testing.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
3.3 KB
215.85 KB

Seems like the default workspaces added in workspace.install aren't installed during InstallUninstallTest, which I've no idea why. However, tried to make this less relevant by altering the access checks.

Hard to test locally because InstallUninstallTest runs so slowly.

Status: Needs review » Needs work

The last submitted patch, 36: 2784921-36.patch, failed testing.

dawehner’s picture

This module is gigantic, and super hard to make a mind off.

  1. +++ b/core/modules/workspace/src/Changes/ChangesInterface.php
    @@ -0,0 +1,42 @@
    +  public function includeDocs($include_docs);
    

    A classical example why having short names doesn't help at all: This could mean includeDocuments or includeDocumentation ... and well in the end its all about entities anyway, so could this be named incldueEntitiess?

  2. +++ b/core/modules/workspace/src/Entity/ContentWorkspace.php
    @@ -0,0 +1,177 @@
    +  public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
    +    $fields = parent::baseFieldDefinitions($entity_type) + self::publishedBaseFieldDefinitions($entity_type);
    +
    +    $fields['uid'] = BaseFieldDefinition::create('entity_reference')
    +      ->setLabel(t('User'))
    +      ->setDescription(t('The username of the entity creator.'))
    +      ->setSetting('target_type', 'user')
    +      ->setDefaultValueCallback('Drupal\workspace\Entity\ContentWorkspace::getCurrentUserId')
    +      ->setTranslatable(TRUE)
    +      ->setRevisionable(TRUE);
    +
    +    $fields['workspace'] = BaseFieldDefinition::create('entity_reference')
    +      ->setLabel(t('workspace'))
    +      ->setDescription(t('The workspace of the referenced content.'))
    +      ->setSetting('target_type', 'workspace')
    +      ->setRequired(TRUE)
    +      ->setTranslatable(TRUE)
    +      ->setRevisionable(TRUE)
    +      ->addConstraint('workspace', []);
    +
    +    $fields['content_entity_type_id'] = BaseFieldDefinition::create('string')
    +      ->setLabel(t('Content entity type ID'))
    +      ->setDescription(t('The ID of the content entity type this workspace is for.'))
    +      ->setRequired(TRUE)
    +      ->setRevisionable(TRUE);
    +
    +    $fields['content_entity_id'] = BaseFieldDefinition::create('integer')
    +      ->setLabel(t('Content entity ID'))
    +      ->setDescription(t('The ID of the content entity this workspace is for.'))
    +      ->setRequired(TRUE)
    +      ->setRevisionable(TRUE);
    +
    +    $fields['content_entity_revision_id'] = BaseFieldDefinition::create('integer')
    +      ->setLabel(t('Content entity revision ID'))
    +      ->setDescription(t('The revision ID of the content entity this workspace is for.'))
    +      ->setRequired(TRUE)
    +      ->setRevisionable(TRUE);
    +
    +    return $fields;
    +  }
    

    Given that I assume you cannot put translations in their own workspaces?

  3. +++ b/core/modules/workspace/src/Entity/ContentWorkspace.php
    @@ -0,0 +1,177 @@
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function getOwner() {
    +    return $this->get('uid')->entity;
    +  }
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function getOwnerId() {
    +    return $this->getEntityKey('uid');
    +  }
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function setOwnerId($uid) {
    +    $this->set('uid', $uid);
    +    return $this;
    +  }
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function setOwner(UserInterface $account) {
    +    $this->set('uid', $account->id());
    +    return $this;
    +  }
    

    Do we have an issue to have a trait for that?

  4. +++ b/core/modules/workspace/src/Negotiator/SessionWorkspaceNegotiator.php
    @@ -0,0 +1,53 @@
    +    return $workspace_id ?: $this->container->getParameter('workspace.default');
    

    Couldn't we use 'default' as default workspace parameter always?

  5. +++ b/core/modules/workspace/src/Changes/ChangesInterface.php
    @@ -0,0 +1,42 @@
    +/**
    + * Define and build a changeset for a Workspace.
    + */
    +interface ChangesInterface {
    +
    

    Documentation here should IMHO explain what a changeset means. I just assume you'll work on https://www.drupal.org/core/gates#documentation-block-requirements over time.

  6. +++ b/core/modules/workspace/src/Negotiator/WorkspaceNegotiatorBase.php
    @@ -0,0 +1,49 @@
    +abstract class WorkspaceNegotiatorBase implements WorkspaceNegotiatorInterface, ContainerAwareInterface {
    +
    +  use ContainerAwareTrait;
    +
    

    I have a hard time undertanding why these things have be to container aware by default

  7. +++ b/core/modules/workspace/src/Negotiator/WorkspaceNegotiatorInterface.php
    @@ -0,0 +1,43 @@
    +  /**
    +   * @param \Drupal\Core\Session\AccountInterface $current_user
    +   */
    +  public function setCurrentUser(AccountInterface $current_user);
    +
    +  /**
    +   * @param \Drupal\workspace\WorkspaceManagerInterface $entity_manager
    +   */
    +  public function setWorkspaceManager(WorkspaceManagerInterface $entity_manager);
    

    These things sounds like they should be injected using a constructor

  8. +++ b/core/modules/workspace/src/Plugin/Field/WorkspaceFieldItemList.php
    @@ -0,0 +1,93 @@
    +      $revisions = \Drupal::service('entity.query')->get('content_workspace')
    +        ->condition('content_entity_type_id', $entity->getEntityTypeId())
    +        ->condition('content_entity_id', $entity->id())
    +        ->condition('content_entity_revision_id', $entity->getRevisionId())
    +        ->allRevisions()
    +        ->sort('revision_id', 'DESC')
    +        ->execute();
    +
    

    IMHO these queries should opt out of access checking.

  9. +++ b/core/modules/workspace/src/Replication/ReplicationInterface.php
    @@ -0,0 +1,28 @@
    +   * @return \Drupal\workspace\Entity\ReplicationLogInterface
    +   */
    +  public function replicate(UpstreamInterface $source, UpstreamInterface $target);
    
    +++ b/core/modules/workspace/src/Replication/ReplicationManager.php
    @@ -0,0 +1,39 @@
    +   * @return mixed
    ...
    +          return $replicator->replicate($source, $target);
    

    I'm wondering whether as a first step it would be enough to just return the object, but not store it in the database?

  10. +++ b/core/modules/workspace/src/WorkspaceManager.php
    @@ -0,0 +1,253 @@
    +  public function entityTypeCanBelongToWorkspaces(EntityTypeInterface $entity_type) {
    ...
    +  public function getSupportedEntityTypes() {
    

    Given these two things do basically the same (once vs. multiple) the names are quite different.

  11. +++ b/core/modules/workspace/src/WorkspaceManager.php
    @@ -0,0 +1,253 @@
    +    // If the current user doesn't have access to view the workspace, they
    +    // shouldn't be allowed to switch to it.
    +    // @todo Could this be handled better?
    ...
    +      throw new WorkspaceAccessException('The user does not have permission to view that workspace.');
    

    You mean you want to do something else than throwing an exception?

timmillwood’s picture

@dawehner - Thank you so much for the review!

If it makes things easier, the /core/modules/workspace part of this module is the same as the 8.x-2.x branch of https://www.drupal.org/project/workspace, feel free to review / patch in the issue queue there. I'm sure I could talk @_dixon into giving out commit access to some people too.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
18.62 KB
215.58 KB

#38.1 - This is kinda inherited from CouchDB which is what the replication protocol is based on. With couchdb you can pass include_docs=true as a parameter to get changes including the documents. Changed to entities here, as it makes more sense. We'll map to include_docs when we come to rewrite Relaxed module in contrib.

#38.2 - Updated the fields to not be translatable. We have a similar issue in Content Moderation #2881645: ContentModerationState 'workflow' column empty for all non-default translations.. The entity type is still translatable (even though it has no translatable fields). Will think about if there is a need for this still.

#38.3 - #1979260: Automatically populate the author default value with the current user.

#38.4 - I'm not sure what you mean.

#38.5 - Updated.

#38.6 - This was to get the default workspace param, changed to inject it to the service.

#38.7 - Fixed.

#38.8 - Fixed.

#38.9 - The ReplicationLog is used at the start of replication to know which sequence ID we last replicated up until. Then when the next replication is done we only replicate changes since the last replication.

#38.10 - getEntityTypesThatCanBelongToWorkspaces() is a bit long isn't it? but maybe we could change the others to isEntityTypeSupported() and isEntitySupported()?

#38.11 - (For context, this was inherited from https://github.com/dickolsson/drupal-multiversion/blame/8.x-1.x/src/Work...) Ideally we should throw a 403, but it can be complex from here.

timmillwood’s picture

A few tidy ups.

Status: Needs review » Needs work

The last submitted patch, 41: 2784921-41.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
2.8 KB
214.51 KB

Got hit by the Node save button changes.

timmillwood’s picture

FileSize
8.15 KB
214.24 KB

Adding missing "administer workspaces" permission and fixing issue with two submit buttons on the deployment form, then updating replication test to use the deployment form rather than the API directly.

yoroy’s picture

Just reviewed this with the workspace team, Gabor and moi:

The "Add new workspace" buttons is missing in admin. Otherwise, this MVP looks feature complete.

Switching workspaces is jarring: it happens (too) fast and there's little feedback that it actually happened. Wondering if there's enough (visual) feedback about what just happened when you switch workspaces. A first useful step could be to have a clear visual label that distinguishes the live workspace from all the others.

Another idea was the automatically disappearing message "you are now in workspace Foo". See #2773601: Display "You are now in edit mode" prompt when user enters edit mode for a similar treatment for Edit mode. (Showing messages inside the top tray is not useful when the tray closes between workspace switches)

The top tray could be made more compact (in height) in this version where there's no search available yet.

Feel free to add the "needs usability review" tag again when needed :)

timmillwood’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/workspace/workspace.module
    @@ -0,0 +1,428 @@
    +  // @todo Don't use distinct.
    

    And we're not using distinct anymore.

  2. +++ b/core/modules/workspace/workspace.module
    @@ -0,0 +1,428 @@
    +  //$query->aggregationMethodSimple($entity_type->getDataTable(), $entity_type->getKey('id'));
    

    This can be removed too

  3. +++ b/core/modules/workspace/workspace.module
    @@ -0,0 +1,428 @@
    +  foreach ($view->displayHandlers->getInstanceIds() as $display_id) {
    +    $view->displayHandlers->get($display_id)->setOption('group_by', TRUE);
    +  }
    

    We also saw /admin/content not working, and this is why.

pk188’s picture

Status: Needs work » Needs review
FileSize
213.63 KB

Fixed #46 1 and 2.
Need more information for 3.