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
Comment | File | Size | Author |
---|---|---|---|
#13 | 2779939-13.patch | 35.08 KB | alexpott |
#13 | 11-13-interdiff.txt | 19.27 KB | alexpott |
#11 | 2779939-11.patch | 25.11 KB | alexpott |
#11 | 7-11-interdiff.txt | 5.07 KB | alexpott |
#7 | 2779939-7.patch | 25.03 KB | alexpott |
Comments
Comment #2
alexpottHere'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.
Comment #3
alexpottUpdated issue summary to detail approach taken.
Comment #4
timmillwoodMaybe I'm weird, but I like "moderatable" even though, like "revisionable", it's not a word.
Comment #5
alexpott@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.
Comment #6
timmillwoodok, 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:
Typo
Comment #7
alexpottFixing the typo. Imo we should just avoid moderatable since it is only potentially valid for one method... no need for a neologism for that.
Comment #8
cilefen CreditAttribution: cilefen commented"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.
Comment #10
joachim CreditAttribution: joachim commentedThat 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!
Comment #11
alexpott@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...
Comment #13
alexpottPatch 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.
Comment #14
alexpottComment #15
timmillwoodCan we have an issue open for this now?
Otherwise I think we're good to go.
Comment #17
timmillwoodJust 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?
Comment #18
catchIt 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...
Comment #20
alexpott@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.