shortcut to "accept" a ticket when creating it, and enable the "accept" option for other statuses

[Christian Boos]

Long time ago, on the mailing list, I proposed a patch to immediately set the status of a newly created ticket to "assigned" if the reporter and the assignee are the same person.

This saves one step, as the next logical step would be to "accept" the ticket that the reporter has just created for himself.

Therefore I propose the patch once again, for discussion:

model.py

@@ -142 +142 @@
                 # Assume that no such component exists
                 pass
 
+        # If the owner creates the ticket, status is directly 'assigned'
+        owner = self.values.get('owner')
+        if owner and owner == self.values.get('reporter'):
+            self['status'] = 'assigned'
+
         # Insert ticket record
         std_fields = [f['name'] for f in self.fields if not f.get('custom')
                       and f['name'] in self.values.keys()]

[Christian Boos]

For now, I still have to accept that ticket manually :)

[Christian Boos]

By the way, I just realized that there's a similar situation when one reassigns a ticket to oneself: the ticket would also still have the status new, when assigned would have been more appropriate.

Here also, it wouldn't make sense to reassign to oneself, and then not wanting to accept this assignation.

Hence the revised patch:

model.py

@@ -142 +142 @@
                 # Assume that no such component exists
                 pass
 
+        # If the reporter assigns to himself, the status is directly 'assigned'
+        owner = self.values.get('owner')
+        if owner and owner == self.values.get('reporter'):
+            self['status'] = 'assigned'
+
         # Insert ticket record
         std_fields = [f['name'] for f in self.fields if not f.get('custom')
                       and f['name'] in self.values.keys()]
@@ -202 +207 @@
                     # just leave the owner as is.
                     pass
 
+        # If the reporter reassigns to himself, the status is directly 'assigned'
+        owner = self.values.get('owner')
+        if owner and owner == author:
+            self['status'] = 'assigned'
+
         custom_fields = [f['name'] for f in self.fields if f.get('custom')]
         for name in self._old.keys():
             if name in custom_fields:

[Christopher Lenz]

I'm not sure about this. For me, “accepting” a ticket means that I'm starting work on it, I.e. it is in-progress. Just assigning a ticket to myself doesn't necessarily mean that I am planning to start working on it immediately.

Trac is about not imposing process or policies. Different users may have different policies for accepting tickets, and Trac shouldn't make any assumptions here.

[Christopher Lenz]

And about the reassign case you note in the comment, maybe having an accept action for tickets owned by someone else may be a better approach. That way it's at least explicit.

[Matthew Good]

Well, other projects may choose to use it differently, but I believe that the "accept" option should only be used when a ticket is being actively worked on. So, there are certainly times that I would assign a ticket to myself without "accepting" it yet. This is particularly true at work where I have several projects that I'm the only developer on. So, of course I'm the owner of every ticket, but I don't "accept" it until I'm getting started on it.

I'm changing the summary of the ticket since I can see the advantage of having a shortcut (checkbox?) when creating a ticket that would immediately accept it. Also, I think that the "accept" option should be available for all statuses besides "closed". Right now it's only enabled on "new" tickets which means that accepting "reopened" or "accepted" tickets is a two step process which it doesn't need to be.

While we're on this subject, cboos, I think that it would be helpful if you didn't try to accept so many tickets since right now there are over 50 "accepted' tickets in your name, which I don't believe you're actively working on all of them. I think that this leads to confusion for both the developers and users who don't know what's actually being worked on.

[Christian Boos]

Well, according to your or cmlenz's acception of "accepted", you'd perfectly right. But I was under the impression that "accepting" a ticket would merely mean that I "accept" to work on that item.

For me, the current names chosen for the states would rather mean:

• new: not even seen by the developer yet (that's why there's currently so few new tickets owned by me, and for the few who are in that state, it's certainly because I forgot to "accept" them at some point)
• accepted: the issue is acknowledged by the developer
• fixed: done with it
• reopened: oops, not quite

Some other states would be welcomed, but I won't discuss that here.

My point is that it was not at all evident that the "accept" state would actually mean "I'm working on it".

For tracking current progress at the ticket level, something like what was suggested in #1084 is far better (cf. flyspray).

But I'm not rigid on that, and if it's the recommended way to go, I could revert the state of the tickets that I'm not working upon to the new state. It's just that this should be documented somewhere…

[direvus@…]

I use the "accepted" status in the same way as cboos. If someone reports a bug to me, I won't accept the ticket until I'm convinced the bug isn't bogus. Likewise an RFE ticket won't be accepted until I'm convinced the enhancement is a good one.

It's not about whether I am working on a ticket, it's about whether I will work on it.

That said, I totally agree with cmlenz's comment that:

Trac is about not imposing process or policies. Different users may have different policies for accepting tickets, and Trac shouldn't make any assumptions here.

Perhaps this should be a user setting, like "Auto-accept when assigning tickets to yourself? <checkbox>"

[rob]

This should be enabled as a user setting. I also want tickets I assign myself to be auto-accepted

[peter.becker@…]

If you want the scheme cmlenz proposes, I'd like to see a separate "assigned" state between "new" and "accepted". Alternatively there could be an additional boolean attribute "accepted", in which case "new" should probably be named "unassigned" and the semantics of new would be covered by the pair "unassigned"/"not accepted".

At the moment I'd consider the UI as inconsistent and we got confused since we had some tickets missing from queries where we expected them.

A particular inconsistency for me is that reassigning an accepted ticket to the empty string changes the status to "new". That is inconsistent with reassigning a new ticket to someone, where the status does not change.

[opensource@…]

How about adding an additional status like "in progress"?

So there would be:

• new
• accepted
• in progress
• closed
• reopen

[srussell@…]

I agree with some of the posts above: I prefer to use "accepted" as a tag that I'm actively working on a ticket.

To address some of the other comments, though, wouldn't a special "assigned" state be redundant? If there's a name in "Assigned to:" then it's assigned. If it's the default (somebody?) then it's not assigned.

[kelk@…]

I have recently installed trac, and I am very happy with it.

However it is NOT clear to the users of the system, that they need to "accept" a ticket. So most people newer do this. This may lead to queries with incomplete results.

In our case it would be nice, either to not have the accepted state at all - or to always auto-accept - also when assigning to others.

In my world you have the defect until you reassign it. The scheme with having bugs owned but not assigned leads to tickets in "no mans land". The idea of using "assigned" as "in progress" is to me mixing defect-management too much with project-management. Defects are either fixed or not - in progress is irrelevant when you e.g. need to decide whether to release or not…

[Alec Thomas]

Regardless of how a particular user interprets the states, I think it's quite confusing that using the reassign action makes the state new, whereas accepting the ticket makes it assigned.

WorkFlow will allow this to be customised of course, but making the current behaviour more logical would be good IMHO. This would require assign action and an accepted state.

[Christian Boos]

+1, see also my last comment on #1689, which stated essentialy the same thing.

[fumanchu@…]

For what it's worth, I implemented the same patch as cboos, and was ready to open a new ticket for y'all before I found this one.

+1 on making this configurable. I would've guessed it should be a conf-file entry, though, not a per-user setting. If it is made a per-user setting, I want to be able to default it to True site-wide.

[Christian Boos]

For the record, #2315 and #2732 are also duplicates of this ticket.

[Christian Boos]

In most BTS, the new state means not yet processed.

From the discussion above and others, it seems like a majority of the TracTeam wanted to have the accepted state (accepted as defined in comment:13) to be used as an indicator of "Someone is actively/currently working on this".

We probably need something in between, meaning "this ticket has been noticed and the team decided what to do about it". Currently, this is done by assigning a milestone, which is not that satisfying (there might be valid ticket for which no milestone are adequate). A better solution would probably an acknowledged state.

And this brings me back to the very topic of this ticket: my original request for enhancement was Ticket should be immediately 'assigned' after creation when assigned to the reporter. This could be rephrased: Ticket should be immediately 'acknowledged' after creation when assigned to the reporter, provided we have that acknowledged state.

Opposed to the acknowledged state, there should also be a needinfo state, i.e. the ticket has been seen but can't be processed further until some more details are given (#2944).

So this would give the following first steps of the workflow:

1. create new ticket → (new) state
2. (new) : seen and "triaged", the verb could be validate → (acknowledged) state
3. (new) : seen but can't decide, the verb could be question → (feedback) state

(feedback or needinfo, same thing)

So this ticket is about doing 1. and 2. in one step at ticket creation time, when the reporter assigns the ticket to himself. Now I hope that this sounds unquestionable ;)

[sid]

Oh, I can't wait for workflow. Then everyone can do it their own way.

[Christian Boos]

#4749 is also discussing the default workflow.

[anonymous]

I cannot Accept a Reopened ticket. (10.3) Is this by design ? That is, i fix a ticket, now it is closed, tester/manager reopens, now it's reopened, I should now be able to Accept this ticket, but this option is not available.

Maybe design is that tester reopens, then manager must reassign ( even though it is assigned to me already ), then it gets New status (what? that's no good, as you mention several times here, that reopened != new ).

[Eli Carter]

WorkFlow is in trunk now. Note that the default workflow was left as it was, warts and all. However, the framework is in place so that a reasonable default workflow can be chosen, along with several examples for the common use cases.

[osimons]

Didn't the discussions & decisions on the new 'fixed default workflow' committed as [5658] essentially resolve this issue for 0.11?

[osimons]

#6584 closed as a duplicate - after initial recommendation of using plugin and custom workflow.

[Eli Carter]

The part of this enhancement that remains is being able to take custom workflow actions upon ticket creation. Customizable workflow does not provide a hook into ticket creation, nor am I planning to add one for the 0.11 series. Deferring to 0.12.

[b5]

It would be cool if you would built the proposed patch in and have a config variable to enable it. It should not cost others much performance. But I am used since more than two years to create tickets for myself and then immediately accept them. Because for us "accept" came to mean "saw it, will do it, no questions now, not declined". I think it may mean this in many rougher environments actually, where avoiding blame games is part of how trac helps.

Thank you!

[Ryan J Ollos]

For the sake of consistency, I think it would be nice to have tickets in the assigned state whenever they have an owner. The issue has been raised numerous times in various tickets and on the mailing list, that many users find it confusing that a ticket can be in the new state and yet have an owner. This is confusing because we often say the ticket is assigned to the owner (see #8484).

I'm inclined to tackle this issue in a different ticket, and need to understand the constraint described in TracWorkflow that There are a couple of hard-coded constraints to the workflow. In particular, tickets are created with status new, and tickets are expected to have a closed state. I'd just want to make sure that having a ticket start in the assigned state doesn't break any of the internals of Trac. Most likely it could cause issues for plugins that are mining the ticket_change table for state transitions (e.g. it looks like the ​th:TimeVisualizerPlugin is doing this), so the change should probably be made on a major release.

[Ryan J Ollos]

It occurred to me, why don't we just add the Workflow actions to the New Ticket form? With that change, we can remove the Set Owner field, and the owner would only be set through the workflow. If the owner can only be set through the workflow, then it will behave more like the resolution field, and we remove the inconsistency of having tickets "assigned to" an owner and not in the assigned state. The ticket creator could then put the ticket directly in any state for which the Workflow supports a transition from new (or "pre-new"), and for which the creator has the appropriate permissions.

I've implemented the initial changes and will post them tomorrow. This was a pretty quick proof-of-concept test, so I'm sure there are issues to resolve still. Two questions that I'll explore next are:

• Should we have a "pre-new" state in the workflow? That would determine what state transitions are allowed when creating the ticket. I'm leaning towards implementing this because I think we can make the actions and action hints more clear (see screen captures below for current behavior).
• If a ticket is created in a state other than new, should we have an entry in the changelog? I've seen users complain on trac-hacks about plugins that create tickets with a state other than new, as it causes challenges, e.g. when trying to count the number of tickets created in a time period.

Workflow with [ticket] default_owner = < default >

Workflow with [ticket] default_owner =

[ticket-workflow]
accept = new,assigned,accepted,reopened -> accepted
accept.operations = set_owner_to_self
accept.permissions = TICKET_MODIFY
leave = * -> *
leave.default = 1
leave.operations = leave_status
reopen = closed -> reopened
reopen.operations = del_resolution
reopen.permissions = TICKET_CREATE
resolve = new,assigned,accepted,reopened -> closed
resolve.operations = set_resolution
resolve.permissions = TICKET_MODIFY
assign = new -> assigned
assign.permissions = TICKET_MODIFY
assign.operations = set_owner
reassign = assigned,accepted,reopened -> assigned
reassign.permissions = TICKET_MODIFY
reassign.operations = set_owner
unassign = assigned -> new
unassign.permissions = TICKET_MODIFY
unassign.operations = del_owner

If we add a pre-create state to the workflow, I was thinking it might look something like this:

create = none -> new
create.permissions = TICKET_CREATE
create.operations = set_default_owner
accept = none,new,assigned,accepted,reopened -> accepted
accept.operations = set_owner_to_self
accept.permissions = TICKET_MODIFY
leave = * -> *
leave.default = 1
leave.operations = leave_status
reopen = closed -> reopened
reopen.operations = del_resolution
reopen.permissions = TICKET_CREATE
resolve = new,assigned,accepted,reopened -> closed
resolve.operations = set_resolution
resolve.permissions = TICKET_MODIFY
assign = none, new -> assigned
assign.permissions = TICKET_MODIFY
assign.operations = set_owner
reassign = assigned,accepted,reopened -> assigned
reassign.permissions = TICKET_MODIFY
reassign.operations = set_owner
unassign = assigned -> new
unassign.permissions = TICKET_MODIFY
unassign.operations = del_owner

Side note: Maybe delete could be an action as well, which terminates in the none state?

The dialog might look something like this for the default workflow:

Having only create as new seems a little off because every action creates the ticket. Maybe we should have create and assign to, create and accept?

[Remy Blank]

This sounds like a good idea. It would remove the hardcoding of "new" as the initial state of every ticket (it would simply be a default), a complaint that we've had a few times in the past. I'm not sure about the deletion part, but that's largely independent and could be addressed later.

[ethan.jucovy@…]

This approach wouldn't fully prevent tickets with state="new" from having an owner set, right? [ticket] default_owner, and component owners, and ITicketChangeListener plugins, would still be able to cause a ticket to get an owner set without its state changing from "new". If true, I think this is a good thing.

I was also going to respond to comment:27 pointing out that I do often need "new" tickets with owners — in a lot of my trac sites, "new" means "this ticket has not yet been acknowledged", but it might still be a particular person's responsibility to acknowledge the ticket. (I sometimes remove the leave action from "new" tickets, so that any ticket edit at all — even just a comment or a description edit — constitutes acknowledgement, and moves the ticket into the "assigned" state.) I would still be able to configure this behavior by altering the pre-create workflow action's operations, right?

Side note: I'd love to see what your implementation looks like, and how (if at all) it alters the ITicketActionController interface; for my ​th:WorkflowNotificationPlugin I had to use some nasty hacks to trigger emails on ticket creation, since the ITicketChangeListener methods provide less context than the ITicketActionController methods. I'd be very happy if I could get rid of the ITicketChangeListener code in that plugin and only use action controller methods.

[Olemis Lang <olemis+trac@…>]

regarding comment:28 , fwiw +1

[gary.martin@…]

From what I can tell, with the suggested changes from rjollos, it should still be possible to model the current workflow by only defining the transition from "none" to "new". I think I would expect any required upgrades to conserve the existing behaviour of projects even if there is a desire to provide a new default workflow for new installations.

As a nice side-effect, dropping the hardcoding of the "new" state would give the possibility of providing the "new" state with a more meaningful name to suit the intended use of the state if desired.

On the assumption that this can be implemented, there is obviously going to be a chance that "none" is a defined state in existing installations. I was idly wondering whether it would be possible avoid the potential for conflicts by defining a special form for the definition rather than having a string defining this "none" state. Not sure if just the following would make enough sense:

create = -> new

[Ryan J Ollos]

Initial changes to add the workflow controls to the new ticket page can be found in log:rjollos.git:t2045. The next step is to add the creation workflow action and associated state, and then determine behaviours for options such as [ticket] default_action.

[Ryan J Ollos]

Replying to rblank:

… I'm not sure about the deletion part, but that's largely independent and could be addressed later.

Yeah, I don't intend to approach that in this ticket. Initially I was just thinking that it offers a nice symmetry to have create and delete in the workflow. Having delete as an action in the workflow for a ticket doesn't seem to offer any immediate advantages (and could actually be less convenient that the delete button), but it could be useful on the batch modify form. If we wanted to add the ability to delete multiple tickets in a single operation it might fit nicely into the workflow actions of the batch modify form.

One use case I can think of is, users tag tickets as spam and an administrator is responsible for deleting them - batch delete would be much more convenient.

There's a fork of the ​th:TicketMoverPlugin that integrates ticket moves into the workflow, which is also an interesting use-case for the workflow that may involve create and delete operations. I haven't looked into the details of the implementation.

[Ryan J Ollos]

#9799 was closed as a duplicate, requesting a way to translate the new and closed statuses.

[Ryan J Ollos]

​SO:24675086 presents another use case that can be solved by this ticket (also requested in #7979). The user is requesting that the ticket owner is set to the reporter when the ticket is created. That could be accomplished with something like:

create = none -> assigned
create.permissions = TICKET_CREATE
create.operations = set_owner_to_self

[Ryan J Ollos]

#11791 requests the ability to rename the new and closed statuses (but not necessarily translate them).

[Ryan J Ollos]

I'm doing some refactoring in the default_workflow module. parse_workflow_config​ should do all of the parsing rather than leaving some parsing to be done in render_ticket_action_control:

• trunk/trac/ticket/default_workflow.py@13313:248-249#L246​
• trunk/trac/ticket/default_workflow.py@13313:310-311#L308​

The two parsing statements are different though. Empty strings are allowed for set_resolution: set_resolution = , fixed, is parsed to ['', 'fixed', '']. Not for set_owner though: set_owner = , user1, , user2 is parsed to ['user1', 'user2'].

Leading and trailing empty string are stripped from set_owner but not from set_resolution.

• set_resolution = , fixed, , → [u'', u'fixed', u'', u'']
• set_owner = , user1, , → [u'user1', u'']

I suspect that particular behavior was not intentional so I'm not planning to preserve the behavior unless someone has an argument for keeping it. It seems better to just strip all empty strings.

Parsing the string using as_list, the values would be ['fixed'] and ['user']: trunk/trac/ticket/default_workflow.py@13313:63-65#L40​

[Ryan J Ollos]

In [13322]: refactored default_workflow module so that all workflow parsing occurs in parse_workflow_config. More refactoring will be done later.

[Ryan J Ollos]

Replying to gary.martin@…:

On the assumption that this can be implemented, there is obviously going to be a chance that "none" is a defined state in existing installations. I was idly wondering whether it would be possible avoid the potential for conflicts by defining a special form for the definition rather than having a string defining this "none" state. Not sure if just the following would make enough sense:

create = -> new

I experimented with using an empty string for the pre-new state. It would work well for the case -> new, but the case in which you have a list of old states it gets ugly. You'd have , new, assigned -> assigned, or new, , assigned -> assigned.

For that reason I think we need to use an actual keyword or symbol for the pre-new state. * is already used to match all actions in leave = * -> *. Besides use of none, I considered using [] or (). Other ideas? There aren't any symbols that jump out at me as an obvious indicator, which leads me to reconsider using none.

[ethan.jucovy@…]

Replying to rjollos:

For that reason I think we need to use an actual keyword or symbol for the pre-new state. * is already used to match all actions in leave = * -> *. Besides use of none, I considered using [] or (). Other ideas? There aren't any symbols that jump out at me as an obvious indicator, which leads me to reconsider using none.

In ​th:WorkflowNotificationPlugin I used @created to express something similar (though there it's mocking a workflow action instead of mocking a workflow state) — could something like that or @none be used here? I think having a symbol in front of a string like that makes it a lot easier to see that it means something special. (Of course this too is currently a valid identifier for a real state, just like "none"..)

[Ryan J Ollos]

Initial changes in log:rjollos.git:t2045.3. I plan to do more testing and revisions, but all unit/functional tests are passing and the changes could use some independent testing.

[Ryan J Ollos]

The valid parameter is no longer used in the ticket templates, so removed on trunk in [13352].

[Ryan J Ollos]

Replying to ethan.jucovy@…:

In ​th:WorkflowNotificationPlugin I used @created to express something similar (though there it's mocking a workflow action instead of mocking a workflow state) — could something like that or @none be used here? I think having a symbol in front of a string like that makes it a lot easier to see that it means something special. (Of course this too is currently a valid identifier for a real state, just like "none"..)

__none__, _none_ and @none@ seem reasonable. On the other hand none doesn't seem likely to have been used as a workflow action name, and we can detect and report conflicts during the upgrade step, so I don't see the harm in making it a reserved keyword.

[Ryan J Ollos]

Replying to rjollos:

__none__, _none_ and @none@ seem reasonable. On the other hand none doesn't seem likely to have been used as a workflow action name, and we can detect and report conflicts during the upgrade step, so I don't see the harm in making it a reserved keyword.

How about (none)?

[Christian Boos]

Don't we already have some "special" values in the ticket interface, like < default > for the owner? So perhaps we could use < none > / <none> here.

[Ryan J Ollos]

Using < none > / <none> sounds like a good idea.

TODO Modify sample workflows in trunk/contrib/workflow.

[Ryan J Ollos]

Until now TICKET_MODIFY has been required for setting the ticket owner when creating a ticket: tags/trac-1.0.2/trac/ticket/web_ui.py@:452-453#L442​ and tags/trac-1.0.2/trac/ticket/web_ui.py@:1467,1469-1470#L1447​.

The behavior isn't trivial to reproduce when adding create workflow actions. <action>.permissions = TICKET_CREATE, TICKET_MODIFY means that the user can have either TICKET_CREATE or TICKET_MODIFY: tags/trac-1.0.2/trac/ticket/default_workflow.py@:385,388#L381​. We need to decide which actions will be added to the workflows provided with Trac, and which actions will be added in the workflow upgrade.

For some environments, TICKET_MODIFY will imply TICKET_CREATE, but I don't think it's right to assume anything in that regard. We could add something that is equivalent to operators for the permissions field (e.g. TICKET_CREATE & TICKET_MODIFY would be a valid value for the permissions field. maybe TICKET_CREATE | TICKET_MODIFY is valid as well [or use the more pythonic and and or operators]), but that seems like an overly complex feature to handle in this ticket.

Unless anyone has a better idea, I'll just require TICKET_CREATE for the two "standard" workflow actions that will be added in the upgrade step and in several of the workflows that Trac provides:

create = <none> -> new
create.permissions = TICKET_CREATE

assign = <none> -> assigned
assign.permissions = TICKET_CREATE
assign.operations = may_set_owner

[Ryan J Ollos]

With the proposed changes the ticket workflow is solely responsible for changing the owner field in the same way that it's responsible for changing the status and resolution fields. That makes me think that it might make sense to get rid of the [ticket] default_owner option (documented at TracTickets#DefaultValuesforDrop-DownFields). Instead we could have:

• set_owner_to_component_owner workflow operation which would behave the same as the < default > value of default_owner (see ​th:AdvancedTicketWorkflowPlugin#Description).
• set_owner_to_reporter workflow operation (see ​th:AdvancedTicketWorkflowPlugin#Description).
• default_owner attribute of the workflow for use with may_set_owner (set_owner by definition defaults to the ticket reporter).

One nice side effect of eliminating [ticket] default_owner will be eliminating the < default > value, which is awkward due to the whitespace, and it's not obvious that the value should result in the Component owner being set: comment:7:ticket:7979, comment:8:ticket:10825. However, even with a set_owner_to_component_owner operation we'd still need to allow < default > for default_owner if we wish to have the drop-down default to the component owner (as opposed to having a workflow action that strictly assigns to the component owner).

If [ticket] default_owner is moved to the ticket workflow then [ticket] default_resolution should be handled similarly.

Also, it's slightly simpler to just use <none> rather than allowing both <none> and < none >. Getting rid of < default > would allow us to use just <none> without any syntax inconsistencies within a version of Trac.

[Ryan J Ollos]

The changes from #625 present a challenge to the aim of completely removing the requirement of having new and closed states. Not only is the new state hard-coded, but the component owner is being modified outside the workflow. I don't think there is much we can do about that for now without losing backwards compatibility: tags/trac-1.0.2/trac/ticket/model.py@:286-303#L260​. If the user renames the new state, they'll just lose the behavior of having the owner changed as the component owner is changed. The solution might be to move this behavior into the workflow, but that looks difficult, so I'm deferring that change. Btw, does it make sense to have that behavior coded in the model? It feels like the behavior should be in the realm of the controller.

[Peter Suter]

Replying to rjollos:

The changes from #625

I assume you mean #623?

I don't think there is much we can do about that for now without losing backwards compatibility: tags/trac-1.0.2/trac/ticket/model.py@:286-303#L260​. If the user renames the new state, they'll just lose the behavior of having the owner changed as the component owner is changed. The solution might be to move this behavior into the workflow, but that looks difficult, so I'm deferring that change. Btw, does it make sense to have that behavior coded in the model? It feels like the behavior should be in the realm of the controller.

I'm also surprised that this code exists in that form in the model. I don't understand why the new state should be handled specially here. Even removing this special behavior entirely seems worth considering. Moving the behavior into the workflow / controller (I assume you mean ConfigurableTicketWorkflow = ITicketActionController in both cases?) might make sense. Would you add a new operations like respect_component_default_owner?

[Peter Suter]

Replying to rjollos:

Unless anyone has a better idea, I'll just require TICKET_CREATE for the two "standard" workflow actions that will be added in the upgrade step and in several of the workflows that Trac provides:

create = <none> -> new
create.permissions = TICKET_CREATE

assign = <none> -> assigned
assign.permissions = TICKET_CREATE
assign.operations = may_set_owner

That seems entirely reasonable and sufficient to me.

[Ryan J Ollos]

Replying to psuter:

Replying to rjollos:

Unless anyone has a better idea, I'll just require TICKET_CREATE for the two "standard" workflow actions that will be added in the upgrade step and in several of the workflows that Trac provides:

create = <none> -> new
create.permissions = TICKET_CREATE

assign = <none> -> assigned
assign.permissions = TICKET_CREATE
assign.operations = may_set_owner

That seems entirely reasonable and sufficient to me.

I was just about to comment on this because I had an aha! moment. TICKET_CREATE is required to access the ticket form, so in the ticket workflow we can be sure that TICKET_CREATE has already been checked before the New Ticket form is rendered and whenever a transition from <none> is available. This allows a simpler workflow that checks TICKET_MODIFY for the assign action, which should provide even better backward-compatibility:

assign = <none> -> assigned
assign.operations = may_set_owner
assign.permissions = TICKET_MODIFY
create = <none> -> new

[Ryan J Ollos]

Replying to psuter:

Replying to rjollos:

The changes from #625

I assume you mean #623?

Right. When did I get so bad with numbers?

I'm also surprised that this code exists in that form in the model. I don't understand why the new state should be handled specially here. Even removing this special behavior entirely seems worth considering. Moving the behavior into the workflow / controller (I assume you mean ConfigurableTicketWorkflow = ITicketActionController in both cases?) might make sense.

In the first case I was thinking about adding a workflow operation. In the second case I was wondering if the behavior belonged in TicketModule.process_request (I was thinking of that as the controller in an MVC architecture, though not entirely sure I'm interpreting that piece of the pattern correctly).

Would you add a new operations like respect_component_default_owner?

Yeah, that seems like a good way to go. On the one hand the operation seems overly specialized. On the other hand we apparently have pretty specialized behavior since #623 was implemented, and at least with a dedicated workflow operation the [ticket-workflow] implementer can opt-out.

The implementation seems a bit complicated though, which is why I'll probably handle it in a follow-on ticket. To keep the current behavior for the default workflow we'd need a special leave-like action.

leave_new = new -> new
leave_new.label = leave
leave_new.default = 1
leave_new.operations = leave_status, respect_component_default_owner

We wouldn't also want the existing leave action to also be present in the new state (or whatever new is renamed to now that our workflow that doesn't require new) , so we can no longer user * -> * for the leave transition. We'd either have to add leave-like action for every state (leave_assigned = assigned -> assigned, leave_accepted = accepted -> accepted, …) or we'd need a new syntax to indicate every state except the specified state, e.g. !new -> !new.

[Ryan J Ollos]

Replying to rjollos:

Replying to gary.martin@…:

On the assumption that this can be implemented, there is obviously going to be a chance that "none" is a defined state in existing installations. I was idly wondering whether it would be possible avoid the potential for conflicts by defining a special form for the definition rather than having a string defining this "none" state. Not sure if just the following would make enough sense:

create = -> new

I experimented with using an empty string for the pre-new state. It would work well for the case -> new, but the case in which you have a list of old states it gets ugly. You'd have , new, assigned -> assigned, or new, , assigned -> assigned.

One other important point about this. The empty string is parsed to an empty list, which is already used to indicate "no valid states" for the _reset action: tags/trac-1.0.2/trac/ticket/default_workflow.py@:125#L98​.

[Ryan J Ollos]

Summarizing various points mentioned in this ticket:

My initial aim was to make owner a protected field that is modified by the workflow and not directly modified by the user. That would make owner behave like status and resolution. Another aim was to put tickets in the assigned state when an owner is assigned. These changes can't be done while preserving the existing behavior, so I've deferred that aim for now.

The obstacles to those two changes are:

• The [ticket] default_owner is set when creating a ticket, causing new tickets to have an owner. #11856 proposes to put the default_owner in the [ticket-workflow] section. The user could still create a workflow in which a new ticket has an owner, but we could modify the basic-workflow​ so that new installations have a workflow that couples the assigned state with the ticket being assigned to an owner (which I think will be more logical, but is a point that needs to be discuss).
• For new tickets, the Owner is changed in the ticket model whenever the Component is changed (comment:53 ⇒ #11858).

Also, the issue with < default > mentioned in comment:51 will be handled in #11857.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t2045.6. I think these are finally ready.

[Jun Omae]

Functional tests failing on rjollos.git@t2045.6. See de59c2d-py26-sqlite.log​.

[Ryan J Ollos]

Strange, the tests passed for me. I'll take a look.

[Ryan J Ollos]

I made a small change just before pushing branch that broke the tests. It should be fixed in log:rjollos.git:t2045.7.

[Jun Omae]

All unit and functional tests pass!

I noticed translatable message at tags/trac-1.0.2/trac/ticket/web_ui.py@:1393-1395#L1378​: %s changed "%s" to "%s", but %s changed it to "%s"..

[Ryan J Ollos]

Thanks, I'll fix that before committing.

[Jun Omae]

Minor thing: I think we should pass format string and arguments rather than formatted string, for logging api.

trac/ticket/web_ui.py

@@ -1378 +1378 @@
 
         # After saving the changes, apply the side-effects.
         for controller in controllers:
-            self.log.debug("Side effect for %s" %
-                           controller.__class__.__name__)
+            self.log.debug("Side effect for %s", controller.__class__.__name__)
             controller.apply_action_side_effects(req, ticket, action)
 
         # Notify
@@ -1440 +1439 @@
 
         # After saving the changes, apply the side-effects.
         for controller in controllers:
-            self.log.debug("Side effect for %s",
-                           controller.__class__.__name__)
+            self.log.debug("Side effect for %s", controller.__class__.__name__)
             controller.apply_action_side_effects(req, ticket, action)
 
         req.redirect(req.href.ticket(ticket.id) + fragment)

[Ryan J Ollos]

Good catch. Those lines were copy and pasted from _do_save (shown at line 1442 in your patch), and the string interpolation on logging calls was fixed in that method in [13230#file12].

[Ryan J Ollos]

Committed to trunk in [13452]. Documentation updated in TracWorkflow@6 and TracWorkflow@7.

[Ryan J Ollos]

One additional change in [13453], to satisfy the request in #7979.

[Ryan J Ollos]

#4266 closed as a duplicate.

[Ryan J Ollos]

original-workflow.ini fixed in #12824.

[Jun Omae]

I consider we should provide a method retrieves the initial status of a ticket for plugins to create new tickets (e.g. ​th:TicketImportPlugin, …).

[Ryan J Ollos]

There's no clear definition of initial status of a ticket. Even if you consider it to be the default create action for a ticket, that could depend on who is creating the ticket and what permissions that user has. Perhaps a plugin like TicketImport could have a configuration option to define the initial status.

[Ryan J Ollos]

See also #13067.