Content Moderation and Workspace don't work together

I'm afraid we won't be able to solve this properly without #2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing.

I opened #2971779: Disallow moderation of internal entity types for disallowing moderation on the workspace association entity type, and I think that allowing workspaces themselves to be moderatable is actually a nice feature :)

Different question: Why would you need content moderation features if you have workspaces?

What you want is an per entity workflow state (and one for the whole workspace as well) - yes:

https://www.drupal.org/project/entity_workflow

but you want workspaces to manage the actual revisions for that.

And even for content moderation of just one entity, you want to still have a micro workspace, because you have:

- paragraphs
- (panelizer) layout
- translations of all that

be all in one per-entity-workspace as else that really does not make sense.

e.g. Complex workflows we have work like that:

- When all entities are in state 'review', then allow to transition the workspace to the 'review' state
- Allow translators to import translations, then check that the translation looks good in context of the site and put the state of the translated reviewed entities to ''

etc.

So my proposal to solve that would be:

- Base content_moderation on per-entity micro-workspaces WHILE not being in a workspace itself (but hide all the complexity)
- Have content_moderation only do the workflows in the future

Alternatively:

- Have workspaces conflict with content_moderation and add a new workflows module as replacement for the workflow functionality of content_moderation
- Deprecate content_moderation once workspaces is stable

As discussed in the "roadmap to Workspace beta" call, the initial step here is to disallow the Workspace module from being installed alongside Content Moderation.

Opened a followup to provide a minimal implementation which would allow them to work together: #2982937: Allow workspace integration to be enable per entity type and bundle

+++ b/core/modules/workspace/workspace.install
@@ -8,6 +8,21 @@
+function workspace_requirements($phase) {
+  $requirements = [];
+  if ($phase === 'install' && \Drupal::moduleHandler()->moduleExists('content_moderation')) {
+    $requirements['workspace_incompatibility'] = [
+      'severity' => REQUIREMENT_ERROR,
+      'description' => t('Workspace can not be installed when Content Moderation is also installed.'),
+    ];
+  }
+
+  return $requirements;
+}

What happens if you install workspace first? I think these checks are only invoked for the module you are installing and not all the modules already installed.

That's hilarious :D And it goes to show that hook_requirements() is not good fit for declaring module incompatibilities. I think we have three options:

1) mirror this hook_requirements() implementation in content_moderation

2) add a way for modules to declare the incompatibility with another module in its info.yml file, something like:

dependencies:
 - !<module_name>

3) introduce a ModuleInstallValidatorInterface, similar to the existing ModuleUninstallValidatorInterface

Let's do 1) for now since it's the simplest option.

Oops.

Added #2982954: Allow modules to declare conflicts with other modules in their info.yml file as a followup.

Not sure about changing stable code (and introducing new strings) to support experimental code. I can't think of another simple solution right now, but I'd be happier if we could find one.

A simpler solution would be #2982954: Allow modules to declare conflicts with other modules in their info.yml file, but I'm not sure that can be done before 8.6.0-alpha1.

Another solution is to implement the "workspace integration by entity type and bundle" approach, but it would require similar changes to stable code (Content Moderation), because it also has to be workspace-aware and not allow people to enable CM support on an entity type already configured with workspace support.

However, the proper long-term solution (8.7) is to allow all the modules that work with pending revisions to interact seamlessly, through the revision tree proposal/idea.

Agreed with @plach that it's better that stable modules don't have references to / code affected by experimental modules. Especially since if Workspaces doesn't make it to beta for 8.6 and ends up getting postponed to 8.7 (which, I hasten to add, we all hope won't happen!!), we'd then be left with dead code in core, which is definitely not ideal.

He and I kicked this around a bit. It seems like we need to account for the following situations:

1. I already have Content Moderation installed, now I want to play around with Workspaces.
2. I already have Workspaces installed, now I want to play around with Content Moderation.
3. I enable both Content Moderation and Workspaces at the same time.

I believe all three cases are solved by the current hook_install() approach, since case 3 will end up changing to case 1, since Content Moderation is alphabetically first and therefore will get enabled first. And agreed that introducing new functionality or dependencies on stale core issues isn't a good idea. There's a hook_modules_installed() but the best this could do is fire a warning that's easily missed/skipped over.

However, while Workspaces integration isn't per-entity type+bundle, I'm curious if we could leverage the fact that Content Moderation's integration is done that way, and have Workspaces hook_form_alter() the config form that allows you to enable Content Moderation on any content/block types, to replace the form there with a message about you not being able to run both modules at once. That seems like it solves case 1 and 3 with not-too-much-code, which could all live in the experimental module where the compatibility problem's being introduced.

What do you think?

workspace integration by entity type and bundle

This doesn't really seem viable to me. The whole model of workspaces is that you can make changes within a workspace and publish the whole thing at once. If entity types/bundles become admin configurable, then unless we prevent editing the non-workspace-enabled entities within workspaces, the behaviour will be very confusing. We already have the issue of path aliases and etc. which can't be workspace enabled.

I kind of feel like hook_requirements() (including in content_moderation) is the cleanest option here. We're not adding a feature or dependency, just avoiding an incompatibility, and it could have a @todo pointing to #2982954: Allow modules to declare conflicts with other modules in their info.yml file.

The approach suggested by @webchick in #24 and implemented in #27 seemed "viable" to do, but I have to agree with @catch that we are breaking the whole premise of workspaces if we disallow entity types from being a part of a workspace when they could have been.

Btw, #2982954: Allow modules to declare conflicts with other modules in their info.yml file is not really stale, it was just opened yesterday with an initial patch :) Someone with actual knowledge of the module (installer) system could easily make it work, IMO

Oh, I'm sorry, I'm confusing that with some much older issue.

I'm also fine with @catch's proposal, btw. :) I think @plach was just looking for a second opinion. That said, #27 looks good to me from a screenshot POV!

@catch:

I'm +1 on #27: it allows to handle all the three use cases we identified without forcing us to touch stable code to support Workspace. The fact that Content Moderation can be enabled per bundle provides us a UI to alter, so that we can state that Workspace is incompatible with it, but this does not imply that we must do #2982937: Allow workspace integration to be enable per entity type and bundle: I also believe that Workspace should act on every aspect of the site, if we are ever able to support that.

Since basically this change makes CM unusable when WS is enabled, we could also choose to grey out the UI entirely or perform something more radical, but I wouldn't spend too much time on this. Hopefully enabling WS and then CM is not the 80% use case.

Of course we need a green patch :)

I'd prefer to go with #12 with the hook_requirements() compared to #27 - it's closer to the behaviour once .info.yml files support incompatibility, and it's already green. I think we can just exclude hook_requirements() from the 'no code about experimental modules' rule since it's not even runtime.

Re-uploading an RTBC-ing. If someone has strong objections please say so but for me it's clean and it works.

Ok, sounds like we have an agreed-upon path forward! :)

Committed and pushed to 8.6.x. Thanks!

Extremely minor nitpick, but I thought it was at least worth mentioning.

Content Moderation can not be installed when Workspace is also installed.

Should can not be changed to cannot? http://grammarist.com/usage/cannot-or-can-not

You're welcome to open a (novice?) follow-up ticket to discuss/implement your suggestion :)

If you do please post the link here.

DER and Workspace both override EFQ. Right now DER v2 EFQ integration will break if the workspace module will be enabled. Related DER issue is #2835542-11: Utilize service decorator instead of taking over entire EFQ service.

Fix component following module rename.

I'm puzzled. Drupal 8.6 released with Workspace as an experimental module but it's not compatible with Content Moderation which is now stable in core, how can this be?

@timmillwood I hope you see this since the issue is closed, but I for one would love to see both modules be allowed to work together.

@webdrips, there's a very good chance for them to work together in 8.7. Follow #3023194: [PP-2] Add parallel revisioning support and #2725523: Add a revision_parent field to revisionable entities for that effort.

@amateescu thank you!