Changing workflow definition can leave a ticket in a locked state

[osimons <simon-code@…>]

Here is how to recreate on a clean Trac env using Trac revision 5384 from trunk:

1. Use the 'enterprise' workflow in trac.ini
2. Set a ticket from new to infoneeded_new
3. Edit trac.ini and replace the workflow section with the 'trivial' example
4. Try to edit the ticket state - it has only a 'leave it' alternative

Editing workflows are likely to be rare, but they do happen - like for components and other ticket meta data. Unlike workflow states, these other data types can always be edited to new valid content directly on the ticket page.

My personal preference would be to add a simple logic: If the ticket has a state that is no longer defined, add the always available 'New' state as an option to reset the workflow for that ticket.

[osimons <simon-code@…>]

I have now tried the StatusFixer sample plugin, and it does provide a way to fix the situation - if installed and enabled. I would still recommend for something along the lines of my suggestion to provide an always available 'bail out' in case of mismatch between data and config.

[Christian Boos]

Replying to osimons <simon-code@bvnetwork.no>:

My personal preference would be to add a simple logic: If the ticket has a state that is no longer defined, add the always available 'New' state as an option to reset the workflow for that ticket.

As you've found out, there's the StatusFixer for these cases, but I nevertheless agree with you on the above suggestion.

[Eli Carter]

Agreed.

[Christian Boos]

(eli's exercising the old workflow warts ;-) )

[Christian Boos]

Replying to osimons <simon-code@bvnetwork.no>:

… I would still recommend for something along the lines of my suggestion to provide an always available 'bail out' in case of mismatch between data and config.

A consistency check would be a good thing, probably during the TicketSystem component initialization (especially in light of #3833).

[osimons]

Patch adding a 'reset' action (for set to 'new') if ticket status is no longer a valid state.

[osimons]

First stab at a solution to this problem - t5307-r6323-reset_workflow-a.diff​

Basically, it will add an internal 'reset' action to whatever default workflow is used, that gets activated whenever the ticket has a status that is no longer an existing status - due to workflow configuration changes.

I looked at various ways of doing this, and doing an implicit reset was not a good option as the workflow change could be temporary due to config mistakes and the like. My (current) feeling is therefore that using the action mechanism explicitly best matches how tickets change whenever any settings become invalid for some reason. And, as the state is outside the current workflow there is no other natural state than to reset it to 'new' so that the workflow can be restarted based on current configuration.

Reviews and comments are most welcome.

[Eli Carter]

I think the 'reset' action should be restricted to admins; the admin needs to know that something has gone wrong with the config and have an opportunity to fix all the tickets in the invalid state, or fix the config, before users start resetting tickets to 'new', at which point cleanup gets more difficult.

I have some misgivings about how the 'reset' action is inserted into the workflow; but they're vague, and other than doing this in a separate plugin, I'm not sure how I'd address them, so discount them accordingly.

[osimons]

Any further comments on a) should we do it, and b) if so should we do it like the patch suggests?

With a general availablility of 0.11.0 not too far away, configuration and reconfiguration of workflows will make this problem appear quite often (personal opinion). I've tested the patch again using current 0.11dev, and it seems to do the job nicely. I see Eli's misgivings of course as we 'inject' into the workflows this 'reset' action, but I can't see how else we can do it in a non-intrusive manner - apart from sticking with the Status Fixer plugin as recommended approach (obviously a working alternative).

'Go' or 'No go'?

[Christian Boos]

Can't the Status Fixer plugin do that?

i.e. only add the force status action (defaulting to 'new') if:

• the current status is not a valid one
• the user has TICKET_ADMIN perm

[osimons]

Replying to cboos:

Can't the Status Fixer plugin do that?

Of course it can - and more as it can set any old state to new state of choice. If it is installed and enabled - it is only a sample plugin. The topic of this tickets is if Trac should include 'bail out' functionality in case an invalid state is detected.

[Christian Boos]

Ok, then. Maybe just give some more explicit "special" status to that code, like using _reset for the action key and writing a comment in __init__ telling what this special action is for.

[r.wolf@…]

I can add a real life example we had.

We first used the default workflow, with "accepted" having the meaning of "I am right now working on it". I changed the default workflow in a way that I removed the state "accepted" and introduced "analyzing", "implementing" and "testing" instead. We also removed lots of the transtions to closed. As we did it while normal work was ongoing, several tickets were in accepted state.

To prevent that tickets got stranded in "accepted", we defined an intermediate work flow, which was the desired one plus a transition from "accepted" to "analyzing". There was no transition TO "accepted" anymore and this worked fine. After all "accepted" tickets were set to analyzing (and maybe to "implementing" or "testing" within the next seconds), we removed this migration transition again and use the final workflow without "accepted".

From a quality point of view, I would prefer to have some well defined tranisitions, which are removed later, over a general "reset" functionality. For me, it's fine as it is in 0.11dev.

Regards, Robert

[Christian Boos]

Thanks for explaining this alternative approach, which indeed sounds better. Implementing migration transitions as you did is even better but is perfectly compatible with having this "reset" fallback when no transition is available, so I'm still in favor of the patch.

[osimons]

Thanks for feedback.

Committed in [6538] with the following changes from the patch:

• Only available for TICKET_ADMIN permission.
• Changed action name to _reset.
• Better comments in code.

[anonymous]

Hi Teams,

We have downloaded the t5307-r6323-reset_workflow-a.diff patch, please share the details for installation and configuration on TRAC

Regards TRAC admin