Improve workflow labels and hints

[Ryan J Ollos]

Following fixes in #13087, the following action used on t.e.o did not work:

change_owner = assigned,closed -> *
change_owner.name = change ownership
change_owner.operations = set_owner
change_owner.permissions = TICKET_MODIFY

I added leave_status to the operations to make it work again. But now the control is rendered as change ownership to <new owner> as closed. It reads a bit awkwardly.

In TracWorkflow, we have this example, which is effectively the same:

reassign_reviewing = reviewing -> *
reassign_reviewing.label = reassign review
reassign_reviewing.operations = set_owner
reassign_reviewing.permissions = TICKET_MODIFY

What does a next status of * mean? The regression fixed in #13087 allows ​AdvancedTicketWorkflow's triage operation to determine the next status when the status change is -> *.

Therefore, it looks to me that a next status of * means "let the operation determine the next status". The only built-in operation that determines the next status is the leave_operation.

The documentation says:

For actions that should be available in all states, * may be used in place of the state

But that applies to the existing status, not the new status.

Maybe we only add the as next-status if leave_status is the only operation?

Also, I think we should only add Next status will be … if the status is changed by the operation.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t13199_improve_workflow_labels_and_hints.

[Ryan J Ollos]

Replying to Ryan J Ollos:

What does a next status of * mean? The regression fixed in #13087 allows ​AdvancedTicketWorkflow's triage operation to determine the next status when the status change is -> *.

Therefore, it looks to me that a next status of * means "let the operation determine the next status". The only built-in operation that determines the next status is the leave_operation.

This was also discussed in comment:1:ticket:13086:

When a new status of action is not *, that means setting a new status.

I didn't fully grasp that point previously. We should document the rule. I agree that set_status is not needed (comment:8:ticket:13013) if we have a rule that a new status of * means the implicit status change will be skipped (and presumably handled by an operation defined in some other ITicketWorkflowController).

Also (comment:7:ticket:13013):

[ticket] workflow option normally accepts one or more of workflow controllers. If one of the controllers sets new status to a ticket, which of the controllers should set the status? All controllers should set? At least, I think that other controllers expect ConfigurableTicketWorkflow sets new status to a ticket even if the operation is not defined in the ConfigurableTicketWorkflow.

Yes, I think it makes sense that we expect ConfigurableTicketWorkflow to set the status, provided we also have the rule that it skips setting status when new status is *.

It may be worth revisiting to move the status change operation out of ConfigurableTicketWorkflow (comment:7:ticket:13013).

[Ryan J Ollos]

Edited 1.0/TracWorkflow@3, TracWorkflow@78, 1.3/TracWorkflow@3.

Committed to 1.0-stable in r17141, merged in [17142:17144].

[Ryan J Ollos]

Replying to Ryan J Ollos:

This was also discussed in comment:1:ticket:13086:

When a new status of action is not *, that means setting a new status.

I didn't fully grasp that point previously. We should document the rule.

Documented in 1.3/TracWorkflow@4.