Implement a set_field workflow action

[Ryan J Ollos]

As mentioned on the ​mailing list, it would be nice to have a set_field action that would couple a specified field to a workflow action in the same way that set_owner and set_resolution couple the owner and resolution fields to the workflow action.

[ethan.jucovy@…]

For this feature to be complete (in the sense of "completely analogous to owner and resolution") I think two distinct pieces are involved:

1. Attach a field to a workflow action, so that a form field for changing that field is present next to a radio button in the Ticket Action box, and is only active when the associated action is chosen.

2. Hide that field from the Modify Fields form, so that (like Owner and Resolution) it cannot be modified except through the workflow action(s) that pertain to it. Perhaps this should be optional and specified in the workflow configuration. For example, a "code_feature_developed_in_branch" field must be filled in when taking the action "request_code_review", but may be filled in via the Modify Ticket form at any point prior to that action. Meanwhile a "qa_signoff" field must be filled in when taking the action "approve_for_release" and (like Owner and Resolution) may not be filled in under any other circumstances.

(Note: my analogies with Owner and Resolution imagine that rjollos' work on #2045 is already done and merged. Until then, there's an additional detail about whether a field is available directly on the Modify Ticket form for new tickets. It's much easier for me to think about this if #2045 brings workflow to new tickets, either pre-creation or post-creation, so that this can be folded into the general case.)

I think that this would be a very big departure from how default_workflow.py currently behaves, including some aspects (like hiding fields from the Modify Ticket form) that currently have nothing to do with ITicketActionController components at all. So, for what it's worth, I think it would be better to build this feature in a plugin, rather than trying to extend default_workflow.py to accommodate it.

I've been working on ​a plugin that would implement this, but both of the aspects I mentioned above turned out to be a lot messier to implement in a plugin than I had expected.

So my preference would be to figure out what changes could be made in Trac core that would allow plugins to implement these features and control their behaviors without too much difficulty. (Ideally I'd love to see that extended to include the Owner and Resolution fields, so that, as a Trac site administrator, I could choose to make them available directly on the Modify Ticket form instead of within workflow actions, and so that they're no longer special-cased in so many parts of Trac core. But I'm not sure whether that's possible.)

[ethan.jucovy@…]

Hide that field from the Modify Fields form, so that (like Owner and Resolution) it cannot be modified except through the workflow action(s) that pertain to it.

This isn't too bad, currently. The problem is that the ticket.html template uses a single "skip" flag (populated in trac.ticket.web_ui.TicketModule.prepare_fields() to control two behaviors: whether the field's rendered value should be displayed in the Ticket Properties box, and whether the field's input widget should be present in the Modify Ticket form.

So, if I want a field to be displayed in the Ticket Properties but absent from the Modify Ticket form, I cannot just set field["skip"] = True. Instead, I need to use an ITemplateStreamFilter — either to remove the field from the Modify Ticket form, or to re-add it to the Ticket Properties box if I do set field["skip"] = True.

In my plugin I went with the latter: first it ​sets the skip flag in IRequestFilter.post_process_request, and then it ​adds the field's rendered value to the Ticket Properties box by hand in ITemplateStreamFilter.filter_stream.

So, this would be much easier to drive from a plugin if there were two separate flags used by ticket.html, "skip_property" and "skip_modify". That said, I'm not sure if it would be useful enough to justify the added complexity / maintenance overhead.

[ethan.jucovy@…]

Attach a field to a workflow action, so that a form field for changing that field is present next to a radio button in the Ticket Action box

This ended up being very hard to do in a plugin. There is a lot of logic involved in turning a ticket field into an HTML form widget: the field's type, options, and current value are all involved. At the moment, this logic is all embedded in ticket.html, so it's very difficult to reuse. Moving those bits into a ticket_form_field.html sub-template would be better, but would still be difficult to reuse in action controllers: trying to use trac.web.chrome.render_template during ITicketActionController.render_ticket_action_control has surprising side effects involving loss of key scripts and stylesheets like trac.js. (This is similar to #9528, but I think it may be a separate issue; I haven't tracked it down well enough to file a ticket yet.)

I started ​a branch to explore issues related to this ticket. On that branch I removed the ticket.html logic for rendering fields into form widgets, and translated it into a Python function (called by ticket.html) instead:

​https://github.com/ejucovy/trac/compare/edgewall:trunk...156dca0fc739925c77323e09ff594aeaba79fb52

To reuse this in an action controller, the function would need to be extended so that the generated markup's id and name attributes can be specified by the caller. (I did this in ​a subsequent commit, but that commit tries to do too much.)

[Jun Omae]

I have not understand your use case yet….

At the moment, this logic is all embedded in ticket.html, so it's very difficult to reuse. Moving those bits into a ticket_form_field.html sub-template would be better, but would still be difficult to reuse in action controllers: trying to use trac.web.chrome.render_template during ITicketActionController.render_ticket_action_control has surprising side effects involving loss of key scripts and stylesheets like trac.js.

However, I think render_ticket_action_control should return Genshi fragment object. Therefore, it would be good to pass fragment=True to Chrome.render_template, e.g. source:tags/trac-1.0.1/trac/ticket/query.py@:1323-1324#L1301​. If it works well, I think we should use sub template for it.

[ethan.jucovy@…]

I think the two changes I proposed in comment:2 and comment:3 would be sufficient to allow plugins to implement this feature. On my refactoring branch I tried to start generalizing this to encompass "resolution" and "owner" as well, but I ended up getting bogged down. For completeness though I'll mention some of the thoughts that occurred to me during that attempt. (My apologies in advance for this very long and poorly organized comment.)

• To reuse this for set_owner and set_resolution, there would need to be more logic, and an extended function signature, for providing a set of values that the field should be restricted to. (e.g. myaction.set_resolution = fixed,wontfix) In those cases, I can't tell whether the set of possible values should be narrowed to the restricted set, or overridden by the restricted set. For example, default_workflow.py's myaction.set_resolution parameter replaces the list of known resolutions, instead of intersecting it.

• There are also some complications for the milestone field, which needs to be restricted by fine-grained MILESTONE_VIEW permissions. Currently this happens in trac.ticket.web_ui.TicketModule._prepare_fields; getting the milestone field directly from TicketSystem.fields.by_name("milestone") isn't safe for rendering without first passing it through _prepare_fields, since its options will not yet be restricted by permission. Its options will also not yet be grouped into open / due / closed until passed through _prepare_fields. And _prepare_fields also contains logic to prevent tickets from being retargeted into a closed milestone unless the ticket already exists and the user is an admin.

• The owner field also can't be rendered until it's been passed through _prepare_fields, which calls eventually_restrict_owner to check the site's configuration and turn it into a restricted dropdown if necessary. Very similar logic is duplicated in default_workflow.py.

• This makes me wonder if trac.ticket.web_ui.TicketSystem._prepare_fields should be split apart and/or made available as an API method. Right now, _prepare_fields performs a few distinct jobs: it processes each field for view-only display (setting "skip", generating "rendered") and it also modifies the field based on context-specific factors (grouping and restricting milestones based on permissions, restricting the owner field). The latter needs to occur any time the field will be used in presentation to an end user, but the former only needs to occur when the field is being rendered for a particular ticket's properties. I took a tentative stab at this on my branch ​here.

• It also makes me wish for a formalized and self-documenting TicketField object instead of a dictionary. Tracking down the various fields that are layered on in different places (type, format, options, optgroups, skip, rendered, width, height, timevalue, dateinfo, edit…) is pretty difficult. An object might also carry along some flags to say "I've been processed for context-specific use" and "I've been prepared for rendering" so that exceptions can be thrown if a field will be used in a certain way before it's "safe" to do so.

• There is very similar logic in query.html for rendering the controls in the Custom Query form. But sharing the same code there would be tricky, especially if the default_workflow.py "set_owner" and "set_resolution" logic was also moved. There are rules about what values should be available, and about what value should be selected by default, which I had trouble generalizing or abstracting. (For example, [maybe_]set_owner have some complex logic about inserting (none) and/or < default > and/or req.authname and/or ticket['owner']; the query page would use none of that, but would need to insert $USER instead.)

• A small point, but it surprised me: the "resolution" field has type "radio", even though it's a <select> dropdown in the only place that ever renders it in a default Trac installation. (default_workflow.py actions with "set_resolution") It has type "radio" because the Custom Query system renders "radio" fields as a set of checkboxes so that multiple values can be selected to create an inner "OR" clause. (The "status" field is also type:radio for the same reason, even though it's never rendered as a form field for direct modification.) I haven't thought this through (or tried to implement it) but would it be possible to decouple that logic somehow? I occasionally feel like I want the Custom Query page to give me a checkbox-set for other fields (e.g. to easily query open tickets assigned to me in milestone1, milestone2 or milestone4) — and I occasionally want a dropdown instead for the status or resolution field (e.g. I know I'm looking for a ticket with status=reopened so I don't need the mental overhead of seeing all those checkboxes)

• Lastly, another very small point: the "resolution" field is special-cased in trac.ticket.web_ui.TicketModule._query_link() (which is called by _prepare_fields to set the "rendered" attribute). The special-casing overrides the default query, which assumes you're looking for non-closed tickets, to look for tickets with status="closed" and the specified resolution. This works well for the default case, but the resulting query link would be suboptimal if a user ever overrides default_workflow.py's default behavior to provide set_resolution operations for non-closed tickets. Similar problems might arise if an ITicketActionController is able to take full responsibility for any arbitrary field. So I wonder if action controllers should somehow have a say in _query_link().

[Ryan J Ollos]

See also ​th:#9502 for the ​th:AdvancedTicketWorkflowPlugin.

[Ryan J Ollos]

#10776 closed as a duplicate, requesting ability to set a custom field from the workflow: ticket:10776:state.png​