Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Adding an optional feature which limits a user's ability to transition content based on the same logic that applies to edit/delete operations would be a great addition to this module.
Proposed resolution
- Use a ServiceProvider or other means to override the workbench_moderation.state_transition_validation service and add our own which extends workbench_moderation's StateTransitionValidation class.
- Override the getValidTransitions function (this is what the ModerationForm uses to build out the available transitions, and potentially others to check whether the user has a section that matches up as per edit/delete access checking in this module.
- Use a separate module for this integration so workbench_moderation is not added as a dependency to workbench_access
Remaining tasks
Tests
Code
User interface changes
None.
API changes
None.
Data model changes
None.
Comments
Comment #2
acbramley CreditAttribution: acbramley commentedComment #3
acbramley CreditAttribution: acbramley commentedHere's a patch which adds a new submodule to integrate workbench_access with workbench_moderation. It alters the state transition validation service to also check for edit access via the WorkbenchAccessManager.
I am not opposed to having this in an entirely separate module but thought I'd add it here first and see what maintainers think.
Comment #4
acbramley CreditAttribution: acbramley commentedComment #5
acbramley CreditAttribution: acbramley commentedComment #6
agentrickardI think my real question here is: Under what conditions can someone perform a transition without being able to edit the node?
Comment #7
acbramley CreditAttribution: acbramley commented@agentrickard that's a good question! Workbench Moderation has a form which you can expose via the display settings on a content type. It means that users with access to the transitions can moderate content without going to the edit screen. The service this module overrides is what checks for the access to those transitions.
Comment #8
fenstratI'm not sure this makes sense as a separate module? It really seems expected behaviour if you've enabled workbench_moderation with workbench_access.
Nit: Whitespace at end of line.
Comment #9
acbramley CreditAttribution: acbramley commented@fenstrat I thought the same too but then I noticed that workbench_access doesn't actually depend on workbench_moderation at all. Not wanting to introduce that dependency, I've gone with a separate module.
Comment #10
acbramley CreditAttribution: acbramley commentedRemoves whitespace.
Comment #12
acbramley CreditAttribution: acbramley commentedAdd test dependencies.
Comment #14
acbramley CreditAttribution: acbramley commentedMessed that up, moving test_dependencies to parent module.
Comment #16
acbramley CreditAttribution: acbramley commentedI believe the failures are due to adding test_dependencies in a patch (i.e DrupalCI won't install those dependencies unless they're already part of the module)
Comment #17
larowlanLooks great, a couple of comments/questions below:
nit: Could we use the existing wba test trait to simplify this?
Comment #18
agentrickardTo be fair, that test trait was just committed. Yes, we should use it.
I have no problem adding this to the main module, but I do wonder if there is a bigger issue here.
State transitions need to respect editorial access controls. So this issue isn't limited to Workbench Access. Say you are using Organic Groups or similar, wouldn't the same problem occur?
If so, then we should fix this upstream at the Workbench Moderation / Content Moderation level.
I think the real problem is here in the StateTransitionValidation service:
That is not a sufficient permission check. It also needs to check entity access.
Unless that's a design decision by Workbench Moderation, in which case, this would be By Design.
Comment #19
agentrickardComment #20
mlhess CreditAttribution: mlhess as a volunteer commentedComment #21
acbramley CreditAttribution: acbramley commentedSo now that we have come to the conclusion to move this into Workbench Moderation instead, I'm happy to write a patch but I'd like to know how we envisage this working.
It could be:
* A global config setting on the module
* A sub module which alters the workbench_moderation.state_transition_validation service to add various checks on entity edit access
* A config setting per content type on the moderation tab?
Comment #22
larowlanComment #23
larowlanHow about this
\Drupal\workbench_moderation\EntityOperations::entityView
to include#access
on the render array like so:- would require injecting the
current_user
service into EntityOperations, but no biggieBecause I think that's the only way you can change the moderation state without edit access.
Comment #24
larowlanwould require a change notice/release notes as its a change in behaviour
Comment #25
acbramley CreditAttribution: acbramley commented@larowlan sounds good to me!
Comment #26
acbramley CreditAttribution: acbramley commentedComment #27
acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commentedNew patch against WBM as per recommendations in #23, let's see what tests fail.
Comment #28
larowlanLooking good
Should this be AccountInterface? not the proxy?
I think we need to work on the title, not sure what we should make it though
Comment #29
larowlanhow about 'Moderate without edit access' then the description to clarify what it means?
Comment #30
acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commented@larowlan thanks, fixed that up.
Comment #31
larowlanOk, we just need a change record here.
Comment #32
larowlanComment #33
acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commentedDraft CR created https://www.drupal.org/node/2863096
Comment #34
acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commentedFound a bug with this on the node's canonical page where the block has been cached for the second user, need to add cache context on the user. Related to #2733127: Can't moderate new content - moderation controls don't appear on new nodes
Comment #35
acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commentedHere's an update to the test to check the canonical url also shows the form to prove the failure.
Comment #36
acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commentedMy bad, this was a nuance with our custom set up to display the form on a node's canonical page. Back to RTBC with the same patch from #30
Comment #38
larowlanThanks @acbramley!