Problem/Motivation

Right now revisionable entity types are required. Content moderation is still useful without revisions, but they loose the ability to create forward revisions.

Proposed resolution

Add support for non-revisionable entities.
Make it clear the behavioural differences between the two, possibly in a follow up.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 2850627-non-revisionable-entity-types-2.patch10.67 KBSam152
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sam152 created an issue. See original summary.

Sam152’s picture

Sam152 CreditAttribution: Sam152 at PreviousNext commented
Status: Active » Needs review
FileSize
2850627-non-revisionable-entity-types-2.patch10.67 KB

I think this patch is far from ready, but these are the changes required to be able to use the basic moderation features on a non revisionable entity type. I think this will at the very least help evaluate if the added complexity is worth the benefit here.

My gut feeling right now is that it isn't.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented

I wouldn't say "far from ready", looks pretty close to me, but I think we need to work out how we enable moderation on these entity types (#2843083: Select entity type / bundle in workflow settings) and if we should limit which entity types it works with (#2851228: Warn users of what features are not available on a given entity type). I wonder how the patch in #2 handles config entities.

Sam152’s picture

Sam152 CreditAttribution: Sam152 at PreviousNext commented
Issue tags: +Workflow Initiative
timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented

I wonder if the need to moderation on non-revisionable entity types is worth the extra code and all the extra isRevisionable() checks.

tacituseu’s picture

tacituseu CreditAttribution: tacituseu commented

Alternatively, instead of trying to make ContentModeration handle all possible cases/combinations, create separate/simpler workflow types for them ?

Sam152’s picture

Sam152 CreditAttribution: Sam152 at PreviousNext commented

The workflow type is a pretty small piece of the overall puzzle. Most of the code that actually make the whole process work has lots of touch-points with the rest of Drupal. I don't think that would cleanly translate across to a simpler version, for simpler entity types.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 as a volunteer commented 
  1. +++ b/core/modules/content_moderation/src/EntityOperations.php
@@ -140,10 +142,7 @@ public function entityInsert(EntityInterface $entity) {
-    if ($this->moderationInfo->isModeratedEntity($entity)) {
-      $this->updateOrCreateFromEntity($entity);
-      $this->setLatestRevision($entity);
-    }

    Nice clean up. 

  2. +++ b/core/modules/content_moderation/src/EntityOperations.php
@@ -163,7 +162,7 @@ protected function updateOrCreateFromEntity(EntityInterface $entity) {
+    $entity_revision_id = $entity->getEntityType()->isRevisionable() ? $entity->getRevisionId() : $entity->id();

    we could use $entity_id available above.

  3. I am not very sure the introduction of isRevisionable() checks are fully covered by current tests.
timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented

Circling back to this and giving it some more thought.

I still stand by my comment in #5, I'm not sure it's really worth it. I think I would rather see us depend on "EditorialContentEntityBase" for Content Moderation to make sure entities were revisionable and publishable.

As we work to try and stabilize Content Moderation I think we need to try to guide decisions and have sensible defaults. One of the key reasons to use Content Moderation is forward revisions, this is not possible without revisions.

Can we set to "Closed (Works as designed)"?

Sam152’s picture

Sam152 CreditAttribution: Sam152 at PreviousNext commented
Status: Needs review » Closed (works as designed)

Agree, lets put this idea to bed.