Problem/Motivation

Follow-up to #2725533: Add experimental content_moderation module.

ModerationInformationInterface and it's implementation are doing quite a bit and expose API that is not explicitly about moderation information. It is also difficult to tell what each method does from the name - isModeratableBundle() - does that mean the bundle is moderated or the entities that use it?

Also, Moderatable is not a word in any dictionary I found online so far. That said, since moderate is a transitive verb it could be argued that adding -able is allowed.

Proposed resolution

The only thing that can be moderated are content entities - therefore bundles and entity types can not be moderated or moderatable.

Looking at the three renamed methods:

  • ::isModeratedEntityType() when this returns TRUE we add the base fields - but the entity can have bundles which are not moderated so ::canModerateEntitiesOfEntityType() is appropriate.
  • ::isModeratableBundle() is used to add the manage moderation tab to entity operations. At this point, all entities with this bundle have to be moderated so ::shouldModerateEntitiesOfBundle() is appropriate
  • ::isModeratableEntity() is used to add the manage moderation on an entity. At this point the entity is definitely being moderated so ::isModeratedEntity() is appropriate

Remaining tasks

User interface changes

None

API changes

Changes:

  • Rename ModerationInformationInterface::isModeratableEntity() to ::isModeratedEntity()
  • Rename ModerationInformationInterface::isModeratableBundle() to ::shouldModerateEntitiesOfBundle()
  • Rename ModerationInformationInterface::isModeratableEntityType() to ::canModerateEntitiesOfEntityType()

Removals:

  • Remove ModerationInformationInterface::isBundleForModeratableEntity() it is unnecessary and removed
  • Remove ModerationInformationInterface::selectRevisionableEntityTypes() it is unnecessary and removed - this is replaced by an internal method for \Drupal\content_moderation\EntityTypeInfo() - less public API that does not have much to do with moderation information. Moderation information uses the alters that EntityTypeInfo puts in place.
  • Remove ModerationInformationInterface::selectRevisionableEntities() it is unnecessary and removed - this is replaced by just filtering lists of entity types using the ::canModerateEntitiesOfEntityType() method

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Here's the patch to replace 'moderatable' with 'moderated'. What this does not tease out is the difference when looking at something that can be moderated or is moderated.

alexpott’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
25.03 KB

Updated issue summary to detail approach taken.

timmillwood’s picture

Maybe I'm weird, but I like "moderatable" even though, like "revisionable", it's not a word.

alexpott’s picture

@timmillwood here's the thing. In at least isModeratableBundle and isModeratableEntity, the use of -able as in making something possible but it might not be, is wrong. It is not that these things can be moderated it is that they are moderated.

timmillwood’s picture

ok, that makes sense:
Moderatable - something that can be moderated (moderation isn't enabled, but could be)
moderated - something that is moderated (moderation is enabled)

Also:

+++ b/core/modules/content_moderation/src/ModerationInformationInterface.php
@@ -26,29 +26,29 @@
+   *   TRUE if this is an entity tis moderated, FALSE otherwise.

Typo

alexpott’s picture

Fixing the typo. Imo we should just avoid moderatable since it is only potentially valid for one method... no need for a neologism for that.

cilefen’s picture

Moderatable is not a word in many dictionaries

"Performant" is also not a word, but people put it on slides as if hoping it will become one. "Moderatable", to its credit, is a recognizable part of speech.

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

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joachim’s picture

Status: Needs review » Needs work
+++ b/core/modules/content_moderation/src/Entity/Handler/ModerationHandlerInterface.php
@@ -28,7 +28,7 @@
-   * Operates on the bundle definition that has been marked as moderatable.
+   * Operates on the bundle definition that has been marked as moderated.

That doesn't make sense. The bundle is not being moderated! It's being set up to allow *its entities* to be moderated.

I don't see a problem with 'moderatable'. It's pretty clear what it means. But if we must remove it, please don't make things make less sense!

alexpott’s picture

Status: Needs work » Needs review
FileSize
5.07 KB
25.11 KB

@joachim I think your point is just as valid against HEAD since the bundle is not moderatable either. In the sense it that moderatable = can be moderated.

Here's a new suggestion...

Status: Needs review » Needs work

The last submitted patch, 11: 2779939-11.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
19.27 KB
35.08 KB

Patch attached fixes the patch and goes a bit further wrt to the API exposed by ModerationInformationInterface trying to make it responsible for less and easier to grok.

alexpott’s picture

Title: Moderatable is not a word in many dictionaries » Cleanup the ModerationInformationInterface
Issue summary: View changes
timmillwood’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/content_moderation/src/Plugin/Derivative/DynamicLocalTasks.php
@@ -76,18 +76,20 @@ public static function create(ContainerInterface $container, $base_plugin_id) {
+          // @todo - are we sure they all have an edit_form?

Can we have an issue open for this now?

Otherwise I think we're good to go.

The last submitted patch, 2: 2779939.2.patch, failed testing.

timmillwood’s picture

Just noticed that @todo isn't actually added in this patch.

Also I think it's pretty safe to assume they all have an edit form, so maybe we can remove the todo?

catch’s picture

Status: Reviewed & tested by the community » Fixed
+++ b/core/modules/content_moderation/src/EntityTypeInfo.php
@@ -169,8 +178,10 @@ protected function addModerationToEntityType(ConfigEntityTypeInterface $type) {
+    if ($this->currentUser->hasPermission('administer moderation states') && $bundle_of &&

It seemed odd to require the current user service, so I looked to see if hook_entity_operation() has an $account parameter. It doesn't, and field_ui for example does permission checks in its implementation, so fair enough.

https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Entity!entity.api...

  • catch committed ce17b23 on 8.3.x
    Issue #2779939 by alexpott: Cleanup the ModerationInformationInterface
    
alexpott’s picture

@catch well this patch just moved the permission check to implementation and out of ModerationInformation so there was no logical change. Just trying to make the hook implementations more obvious and the public API a little slimmer.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.