Node revisions forced even if bundle not under moderation workflow

I agree that this behavior is not ideal. I recently ran up against this when reviewing the migration path from Workbench Moderation to Content Moderation. It appears that this behavior was in Workbench Moderation and was carried over into Content Moderation. However, now that moderation can be applied on a per content type basis with custom workflows, I think this deserves another look.

I have created a preliminary patch that checks the if the node type or node being editing is subjected to moderation before enforcing the revisions box to be checked. In testing on my local, this resolved the issue and appears to work well. I'm submitting it here for review by the community.

Resubmitting a new patch with the correct relative path.

Rerolled the patch against 8.5.1 and 8.5.x-dev since there was a change to the module in the meantime.

So it appears that this feature is written into the tests on Content Moderation. So the tests will also need to be patched in order to get this feature to pass build tests. Back to the drawing board.

This should fix the failing tests, but I still think this needs more test coverage.

Moving back to needs work for more test coverage, specifically around \Drupal\content_moderation\Entity\Handler\NodeModerationHandler::enforceRevisionsEntityFormAlter.

Adding a related issue.

+++ b/core/modules/content_moderation/src/Entity/Handler/NodeModerationHandler.php
@@ -44,18 +55,30 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
+    // Load the node definition and bundle name in order to verify that the
+    // current node type is assigned a content moderation workflow. If so then
+    // force the revisions checkbox to be checked and disabled.
+    $bundle = $form['type']['#default_value'];
+    if ($this->moderationInfo->shouldModerateEntitiesOfBundle($this->entityType, $bundle)) {

I think we should actually never call enforceRevisionsEntityFormAlter if the bundle is not being moderated.

That way we fix the issue in one place for all handlers, instead of just for node. Block content will still be broken for example.

We don't actually have to change enforceRevisionsEntityFormAlter, that was working already.

I did however notice we have a redundant check in \Drupal\content_moderation\EntityTypeInfo::formAlter. We check isModeratedEntityEditForm which already checks isModeratedEntity for us, so removing that.

Fixing the test case.

Whoops, removing unrelated file from patch.

I have tested it. It works perfectly for me

Reroll.

Hi @all,

I would vote for making this issue a major one, because this problem inflates the DB of a site unnecessarily and can bring massive performance problems.

Patch still applies, queuing another test.

Applied patch in #19 at my Drupal 8.7.8 and it's solving the problem.

+++ b/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTypeTest.php
@@ -65,6 +65,11 @@ public function testEnablingOnExistingContent() {
 
+    // Check that the 'Create new revision' checkbox is checked and disabled.
+    $this->drupalGet('/admin/structure/types/manage/not_moderated');
+    $this->assertSession()->checkboxChecked('options[revision]');
+    $this->assertSession()->fieldDisabled('options[revision]');
+

Shouldn't this also have a test to ensure the revision checkbox isn't checked/disabled when a bundle isn't moderated?

+1 for this fixing the issue for me, not marking as RTBC due to the question about the test above.

+1 #19 applied cleanly to 8.8.5 and seems to work just fine.

+1 functionally works great, thanks for the effort here all, not marking as RTBC as mentioned above because I agree the test should be updated to include verifying that the checkbox isn't disabled (since that is what I did manually to verify this was working).

Thanks for the reviews, adding those additional asserts and a red/green patch. Also cleaning up the issue summary a bit.

I decided not to assert the checked status of the revisions checkbox before enabling content moderation, because CM shouldn't be tweaking that and it's a default that comes from the node module.

#24 is address so back to RTBC.

Committed and pushed 30c286f45d to 9.1.x and 09fe2d3e69 to 9.0.x. Thanks!

Leaving at backport for 8.9.x and kicking off a test.

Cherry-picked back to 8.9.x