Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem:
Currently, on the Latest Version tab, only transitions in which the “from” and “to” states are different from each other are included. We need to include transitions in which the “from” and “to” states are the same.
We need to add this capability so that:
- A workflow participant who is allowed to keep an item in its current state, but is not allowed to move an item to a different state, can still provide feedback on an item.
- A workflow participant who is allowed to both keep an item in its current state and move an item to a different state can provide feedback on an item without changing its current state.
in the code:
// Exclude self-transitions.
$transitions = array_filter($transitions, function(ModerationStateTransition $transition) use ($current_state) {
return $transition->getToState() != $current_state->id();
});
Proposed solution:
Comment | File | Size | Author |
---|---|---|---|
#20 | workbench_moderation_2768917_20.patch | 4.16 KB | pericxc |
proposed-solution.png | 74.55 KB | pericxc | |
self-transition.png | 295.06 KB | pericxc |
Comments
Comment #2
pericxc CreditAttribution: pericxc commentedComment #3
Crell CreditAttribution: Crell commentedI'm not sure I'd bother with the checkbox. If self-transitions are going to be allowed, let's just allow them.
Comment #4
pericxc CreditAttribution: pericxc commentedThis patch excludes checkbox.
Comment #5
pericxc CreditAttribution: pericxc commentedComment #6
pericxc CreditAttribution: pericxc commentedComment #7
pericxc CreditAttribution: pericxc commentedComment #8
pericxc CreditAttribution: pericxc commentedComment #9
pericxc CreditAttribution: pericxc commentedComment #10
pericxc CreditAttribution: pericxc commentedComment #11
pericxc CreditAttribution: pericxc commentedComment #12
pericxc CreditAttribution: pericxc commentedComment #13
pericxc CreditAttribution: pericxc commentedComment #14
pericxc CreditAttribution: pericxc commentedComment #15
Crell CreditAttribution: Crell commentedThis isn't going to be translatable. It needs to use t().
Comment #16
pericxc CreditAttribution: pericxc commentedComment #17
jhedstromSince there can be little context when translating strings, how about changing
%to
to something more verbose like%moderation_state
perhaps?Also, can you add a code comment here explaining the logic? I had to read it a few times to grasp how this would work.
I think adding something to one of the tests to verify this would be worthwhile too.
Comment #18
pericxc CreditAttribution: pericxc commentedComment #19
jhedstromJust 2 coding standard nits and then I think this is good to go!
These comments exceed the 80-character length, and should wrap onto another line.
Comment #20
pericxc CreditAttribution: pericxc commentedComment #21
jhedstromI think this is good to go. Thanks for working on this @pericxc!
Comment #22
scookie CreditAttribution: scookie at Workday, Inc. commentedThis is just an observation: It looks like self-transitions used to be there, but were removed.
https://www.drupal.org/node/2651196
Comment #23
larowlanThis will conflict with #2715689: Allow overridable action button labels for transitions.
@jhedstrom which would you prefer went in first, I'm voting for #2715689: Allow overridable action button labels for transitions.
Comment #24
jhedstrom@larowlan either is fine by me. I'm not sure if I'll have time to keep this up though as the focus of the projects I'm working on is now core's Content Moderation modules...