WorkFlow: customizable state transitions
Eli Carter
Ok, I'd like to get some code review and testing of the workflow branch.
There are currently 5, count them, 5 example workflows in the contrib
directory:
original - Implements what Trac has historically had for workflow
trivial - open, closed
simple - new, accepted, closed
opensource - geared to distributed volunteer projects
enterprise - geared to paid developers with QA
If there is no [ticket-workflow] section in trac.ini, the system will default
to using the original workflow. This keeps us backward compatible, and
leaves the arg^Wdiscussion about what the default workflow should be as a
separate step.
If there is a [ticket-workflow], it must specify the entire workflow; it
doesn't include just a delta/overrides. The reason for this is that it's a
lot clearer to have it all spelled out. Otherwise, we wind up unable to
eliminate states easily. And contrib/workflow_parser.py can then help you
get a graph of the workflow.
There is also the UnknownStateTicketActionController workflow component. This
provides TICKET_ADMIN users with the ability to force a ticket to any state
whatsoever. As a separate component, it can be disabled separately
with "trac.ticket.api.UnknownStateTicketActionController = disabled".
I updated the testcases, and added one for UnknownStateTicketActionController.
In the API, apply_ticket_action() should not have any side-effects because it
gets called on preview. Side effects would need to be implemented in a
ticket change listener, and key off a status change. That approach would
allow for sending email, triggering builds, etc. You're also not limited to
how many listeners trigger on a status transition.
As a demonstration of this idea, I added DeleteTicket.py to sample-plugins.
One of the interesting things here is that you could setup a workflow that
included a delete status after a deletion_review status.
I believe the configurable DefaultTicketActionController will cover 90% of the
workflows people will need. For more nuanced workflows, a plugin that
replaces it would be needed.
Current warts:
- restrict_owner handling duplicates the user listing logic stuff. Compare
DefaultTicketActionController.render_ticket_action_control vs
TicketSystem.get_ticket_fields. Pointers welcome.
- there are no provisions for adding to the list of supported operations short
of modifying DefaultTicketActionController.apply_ticket_action()
- it needs more testing
I'm thinking that once the restrict_owner stuff is addressed, this would be a
candidate for merging. I'd like feedback on this.
Thanks,
Eli
Eli Carter
>
> All,
>
> Ok, I'd like to get some code review and testing of the workflow branch.
>
> There are currently 5, count them, 5 example workflows in the contrib
> directory:
> original - Implements what Trac has historically had for workflow
> trivial - open, closed
> simple - new, accepted, closed
> opensource - geared to distributed volunteer projects
> enterprise - geared to paid developers with QA
Make that 6. There's a enterprise-review-workflow.ini to go with the
CodeReview.py plugin. This plugin implements a simplistic code review
step -- you can't review your own ticket, and you have to have permission to
review tickets.
> There is also the UnknownStateTicketActionController workflow component.
This
> provides TICKET_ADMIN users with the ability to force a ticket to any state
> whatsoever. As a separate component, it can be disabled separately
> with "trac.ticket.api.UnknownStateTicketActionController = disabled".
I've moved this out into its own plugin, "StatusFixer", with a corresponding
TICKET_STATUSFIX permission. It's in sample-plugins.
> I updated the testcases, and added one for
UnknownStateTicketActionController.
And it's gone again since it's now in a plugin.
> In the API, apply_ticket_action() should not have any side-effects because
it
> gets called on preview.
Alec pointed out that this needed a better name; so apply_ticket_action is now
get_ticket_changes.
> Side effects would need to be implemented in a
> ticket change listener, and key off a status change. That approach would
> allow for sending email, triggering builds, etc. You're also not limited to
> how many listeners trigger on a status transition.
> As a demonstration of this idea, I added DeleteTicket.py to sample-plugins.
> One of the interesting things here is that you could setup a workflow that
> included a delete status after a deletion_review status.
>
> I believe the configurable DefaultTicketActionController will cover 90% of
the
> workflows people will need. For more nuanced workflows, a plugin that
> replaces it would be needed.
With the ability to stack the workflows, having to completely replace it
becomes less likely.
> Current warts:
> - restrict_owner handling duplicates the user listing logic stuff. Compare
> DefaultTicketActionController.render_ticket_action_control vs
> TicketSystem.get_ticket_fields. Pointers welcome.
> - there are no provisions for adding to the list of supported operations
short
> of modifying DefaultTicketActionController.apply_ticket_action()
> - it needs more testing
>
> I'm thinking that once the restrict_owner stuff is addressed, this would be
a
> candidate for merging. I'd like feedback on this.
Based on feedback from Alec, I eliminated field.py. (Correctly, this time.)
I cleaned up the html, in particular the use of id= and <label for="...
(Thanks cboos)
I changed the syntax for the .ini. We now support things like this:
my_action = new,needinfo -> active
my_action.name = do something
leave = * -> *
leave.operations = leave_status
force_to_new = * -> new
force_to_new.name = force to new
force_to_new.permissions = TICKET_ADMIN
API changes:
- I removed the vestigial intercept_ticket_action method.
- get_ticket_actions now returns an ordering weight for each action so actions
can be ordered across plugins
I also fixed up the support for multiple operations on an action, so you can
actually create an action that sets the owner and the resolution.
(Known bug: if two actions want to set the owner, the first one wins... either
don't do that, or give suggestions. :) ) I believe this should work if more
than one plugin adds a control to a given action... but I haven't tested
that.
Ok, feedback welcome!
Eli
pkou
Just made a quick setup and testing of the proposed workflow branch,
r5109 (especially, in comparison with the old http://trac.edgewall.org/wiki/NewWorkflow
proposal for Trac 0.8, which we are using still).
The idea/implementation looks very good (thanks, Eli and others!).
Comments after testing:
- The 'set_resolution' action does not set the resolution to 'fixed'
by default. Actually, when resolution is empty in a ticket, the
render_ticket_action_control() in api.py sets every option in the
resolution combo box as "selected".
Basically, I vote for inclusion of the workflow branch in 0.11 since
it is one of the important and constantly required features in Trac
IMHO.
Note: The following workflow setup has been used (traditional and
simple develop-test-integrate cycle):
=====
[ticket-workflow]
; qarmt-workflow.ini
leave = * -> *
leave.operations = leave_status
leave.permissions = TICKET_MODIFY
leave.default = 195
reassign = new,resolved,verified,reopened -> *
reassign.name = reassign to
reassign.operations = set_owner
reassign.permissions = TICKET_MODIFY
reassign.default = 185
; positive flows
accept = new,reopened -> accepted
accept.name = accept ticket
accept.operations = set_owner_to_self
accept.permissions = TICKET_MODIFY
accept.default = 190
resolve = new,accepted,reopened -> resolved
resolve.name = resolve as
resolve.operations = set_resolution
resolve.permissions = TICKET_MODIFY
resolve.default = 180
verify = resolved -> verified
verify.name = verify ticket
verify.permissions = TICKET_MODIFY
verify.default = 170
close = verified -> closed
close.name = close ticket
close.permissions = TICKET_MODIFY
close.default = 160
; negative flows
unaccept = accepted -> new
unaccept.name = unaccept to
unaccept.operations = set_owner
unaccept.permissions = TICKET_MODIFY
unaccept.default = 90
reopen = resolved,verified,closed -> reopened
reopen.name = reopen ticket
reopen.operations = del_resolution
reopen.permissions = TICKET_MODIFY
reopen.default = 80
retest = verified,closed -> resolved
retest.name = retest ticket
retest.permissions = TICKET_MODIFY
retest.default = 70
=====
Regards,
Pavel
Eli Carter
>
> Hello All,
> Just made a quick setup and testing of the proposed workflow branch,
> r5109 (especially, in comparison with the old
http://trac.edgewall.org/wiki/NewWorkflow
> proposal for Trac 0.8, which we are using still).
>
> The idea/implementation looks very good (thanks, Eli and others!).
>
> Comments after testing:
> - The 'set_resolution' action does not set the resolution to 'fixed'
> by default.
Ah, good point. Fixed.
> Actually, when resolution is empty in a ticket, the
> render_ticket_action_control() in api.py sets every option in the
> resolution combo box as "selected".
Hmm... I'm not seeing this. I see it not setting any as selected. Can you
double-check and give me a detailed case where this happens?
> Basically, I vote for inclusion of the workflow branch in 0.11 since
> it is one of the important and constantly required features in Trac
> IMHO.
Thanks. :) The feedback you gave helps with that, and further testing is
appreciated.
Eli
Pavel Kourochka
>
> > Comments after testing:
> > - The 'set_resolution' action does not set the resolution to 'fixed'
> > by default.
>
> Ah, good point. Fixed.
>
> > Actually, when resolution is empty in a ticket, the
> > render_ticket_action_control() in api.py sets every option in the
> > resolution combo box as "selected".
>
> Hmm... I'm not seeing this. I see it not setting any as selected. Can you
> double-check and give me a detailed case where this happens?
It seems that the fix does not help.
It is easy to reproduce the problem if the above qarmt-workflow
settings are used. Just create a ticket, and then view HTML source for
the created ticket - the "resolve" action:
=====
<input type="radio" id="resolve" name="action" value="resolve" />
<label for="resolve">resolve as</label>
<select name="resolve_resolve_resolution"
id="resolve_resolve_resolution"><option
selected="selected">fixed</option><option
selected="selected">invalid</option><option
selected="selected">wontfix</option><option
selected="selected">duplicate</option><option
selected="selected">worksforme</option></select>
=====
When it is rendered in Firefox, it shows last selected item (e.g.
worksforme) by default.
> > Basically, I vote for inclusion of the workflow branch in 0.11 since
> > it is one of the important and constantly required features in Trac
> > IMHO.
>
> Thanks. :) The feedback you gave helps with that, and further testing is
> appreciated.
Although my free time is limited, I will spend some of my time for
reviewing/testing/playing with the new implementation.
Regards,
Pavel
Pavel Kourochka
logged user is selected) exists with the set_owner combo box.
Eli Carter
That's not what I'm getting.
=====
<input type="radio" id="resolve" name="action" value="resolve" />
<label for="resolve">resolve as</label>
<select name="resolve_resolve_resolution"
id="resolve_resolve_resolution"><option
selected="selected">fixed</option><option>invalid</option><option>wontfix</option><option>duplicate</option><option>worksforme</option></select>
=====
Double-check that your copy is up-to-date as of r5121, and has no local
modifications. Also clean out your .pyc's and .pyo's to make sure you're not
running from an old cached compilation or something.
If you are still seeing that issue, please send a copy of the entire ticket
page.
Thanks,
Eli
Pavel Kourochka
>
> On Sunday 25 March 2007, Pavel Kourochka wrote:
> >
> > On 3/23/07, Eli Carter <eli.c...@commprove.com> wrote:
> > >
> > > > Comments after testing:
> > > > - The 'set_resolution' action does not set the resolution to 'fixed'
> > > > by default.
> > >
> > > Ah, good point. Fixed.
> > >
> page.
Details have been sent to your personal email.
May be the problem occurs due to Genshi version? I am using
0.4dev-r509 because current trunk has the restriction
'Genshi>=0.4dev-r493, <0.4dev-r510'.
Regards,
Pavel
Ionut Bizau
I can reproduce on my setup when I use Pavel's config.
I use Genshi 0.4dev_r509-py2.4 and trac workflow r5121.
On Mar 26, 9:55 pm, "Pavel Kourochka" <pavel.kouroc...@gmail.com>
wrote:
> On 3/26/07, Eli Carter <eli.car...@commprove.com> wrote:
>
>
>
>
>
> > On Sunday 25 March 2007, Pavel Kourochka wrote:
>
pkou
Genshi versions 0.4dev-r493 through 0.4dev-r501 work properly.
0.4dev-r502 produces wrong HTML.
I filed ticket for Genshi: http://genshi.edgewall.org/ticket/109
So, everything fine with the workflow, let me propose to integrate it
in trunk.
Regards,
Pavel
Eli Carter
>
> The problem is in Genshi really.
>
> Genshi versions 0.4dev-r493 through 0.4dev-r501 work properly.
> 0.4dev-r502 produces wrong HTML.
>
> I filed ticket for Genshi: http://genshi.edgewall.org/ticket/109
>
> So, everything fine with the workflow, let me propose to integrate it
> in trunk.
Ah! Great, thank you for tracking that down.
Eli
Pavel Kourochka
Workflow code needs to be corrected in order to resolve the problem
with multiple selected options. See
http://genshi.edgewall.org/ticket/109#comment:1 for details.
Patch:
-----
Index: trac/ticket/api.py
===================================================================
--- trac/ticket/api.py (revision 5153)
+++ trac/ticket/api.py (working copy)
@@ -192,7 +192,7 @@
perm = PermissionSystem(self.env)
options = perm.get_users_with_permission('TICKET_MODIFY')
control.append(tag.select(
- [tag.option(x, selected=(x == selected_owner and
"selected" or ""))
+ [tag.option(x, selected=(x == selected_owner and
"selected" or None))
for x in options],
id=id, name=id))
else:
@@ -203,7 +203,7 @@
id = action + "_resolve_resolution"
selected_option = req.args.get(id, 'fixed')
control.append(tag.select(
- [tag.option(x, selected=(x == selected_option and
"selected" or ""))
+ [tag.option(x, selected=(x == selected_option and
"selected" or None))
for x in options],
id=id, name=id))
if 'leave_status' in operations:
Index: sample-plugins/workflow/CodeReview.py
===================================================================
--- sample-plugins/workflow/CodeReview.py (revision 5153)
+++ sample-plugins/workflow/CodeReview.py (working copy)
@@ -42,7 +42,7 @@
actions = get_workflow_config(self.config)
label = actions[action]['name']
control = (label, tag.select(
- [tag.option(x, selected=(x == selected_value and
"selected" or "")) for x in options],
+ [tag.option(x, selected=(x == selected_value and
"selected" or None)) for x in options],
name=id, id=id))
return control
Index: sample-plugins/workflow/VoteOperation.py
===================================================================
--- sample-plugins/workflow/VoteOperation.py (revision 5153)
+++ sample-plugins/workflow/VoteOperation.py (working copy)
@@ -20,7 +20,7 @@
selected_value = req.args.get(id, 'for')
options = ["for", "against"]
control = ('vote', tag.select(
- [tag.option(x, selected=(x == selected_value and
"selected" or "")) for x in options],
+ [tag.option(x, selected=(x == selected_value and
"selected" or None)) for x in options],
name=id, id=id))
return control
-----
Regards,
Pavel
Eli Carter
>
> Hi Eli,
> Workflow code needs to be corrected in order to resolve the problem
> with multiple selected options. See
> http://genshi.edgewall.org/ticket/109#comment:1 for details.
Great, thank you. I applied the patch to the workflow branch.
Eli