Setting a default resolution causes new tickets to have that resolution

[hyuga <hyugaricdeau@…>]

As the summary says, if I set a default resolution through the Admin interface, an option like default_resolution = fixed is created in the project's trac.ini file. Then, when a ticket is created and loaded up with its defaults, its resolution is set to 'fixed'.

Meanwhile, the template has the following lines:

<py:if test="ticket.exists">
  <span class="status">(${ticket.status} <py:if
      test="ticket.type">${ticket.type}</py:if><py:if
      test="ticket.resolution">: ${ticket.resolution}</py:if>)</span>

This causes something like Ticket #1 (new defect: fixed) to be displayed at the top of a newly-created ticket, since the template does not even check if the ticket's status is 'closed' before displaying the resolution.

[Christian Boos]

I think the template is OK, if there's a resolution set, it should be shown. What must be adapted is that a resolution shouldn't be set implicitly, i.e. in the default workflow, after any transition that has not explicitly the set_resolution in its operations.

[hyuga <hyugaricdeau@…>]

Ah, I haven't played around with the new workflow system much yet so I didn't know about that. That seems like the best solution then.

[hyuga <hyugaricdeau@…>]

Gives a more explicit meaning to 'default resolution'

[hyuga <hyugaricdeau@…>]

I was looking into fixing this and found that checking the operations for each action would be somewhat involved, as it would mean having to look at the ticket actions when creating new tickets as well as modifying existing ones.

I thought instead it might be easier to give 'default resolution' a more explicit meaning along the lines of "default resolution to select when resolving a ticket" and not have it give the ticket itself a default resolution, which really doesn't make much sense to me to begin with. The attached patch takes care of the problem that way.

If Trac devs agree with this I wouldn't mind seeing it applied in 0.11, since I can see this becoming a nuisance.

[Goldan <denis.golomazov@…>]

What can Trac developers say about this patch? If it does the job, why not to commit it and close the ticket? Or there is something wrong in it?

@hyuga: I have nothing against your solution, just wonder why it hasn't been implemented yet in official repository.

[Goldan <denis.golomazov@…>]

What about to change trac.ini:

default_resolution = 

? Then default resolution will be None.

[hyuga <hyugaricdeau@…>]

Replying to Goldan <denis.golomazov@gmail.com>:

What about to change trac.ini:

default_resolution = 

? Then default resolution will be None.

That's not the point. Of course if there is no default resolution then there's no problem. But in 0.11 the admin interface allows the setting of a default resolution. Also, the handling of default values for enums is such that nothing would stop one from manuallying adding a default_resolution to trac.ini. So as long as it's possible to even do such a thing, it might as well behave well.

[osimons]

Got strange behavior on the 'inside' using default_status = closed as well. That does not make much sense either with workflow and all new tickets being 'new' by definition. However, as new ticket did not respect this setting anyway due to workflow, it just makes it a bit strange.

Anyway, I've poked around here, and think the best way is to trap it when a new Ticket() is created - through the _init_defaults() method. Here is a patch that just ignores settings for 'resolution' and 'status' for empty ticket:

trac/ticket/model.py

@@ -62 +62 @@
     def _init_defaults(self, db=None):
         for field in self.fields:
             default = None
-            if not field.get('custom'):
+            if field['name'] in ['resolution', 'status']:
+                # Ignore for new - only change through workflow
+                pass
+            elif not field.get('custom'):
                 default = self.env.config.get('ticket',
                                               'default_' + field['name'])
             else:

Seems to work well. Testing and comments welcome as usual.

[hyuga <hyugaricdeau@…>]

Ignoring default resolution and status would be fine, as they are 'special' fields in the context of ticket workflow. But any solution that just ignores such defaults should also remove the ability to set default resolutions and status in the admin pages. My solution got around that by assigning some meaning to what a 'default' resolution should do, in this case being just the default resolution that should be selected in the resolution field.

Either way is fine—whatever would confuse the fewest users. But having the ability to set a default resolution and then not doing *anything* with it is definitely confusing to them (I know because they were already doing it).

[osimons]

Replying to hyuga <hyugaricdeau@gmail.com>:

Ignoring default resolution and status would be fine, as they are 'special' fields in the context of ticket workflow. But any solution that just ignores such defaults should also remove the ability to set default resolutions and status in the admin pages. My solution got around that by assigning some meaning to what a 'default' resolution should do, in this case being just the default resolution that should be selected in the resolution field.

Right, I just focused on the reason for why it appeard in the ticket header - and found a much more important problem in that it actually gets stored on the tickets. Any query for resolutions against milestone would pick it up as 'fixed' and display it as such regardless of status. Not good as it breaks with the core the ticket system whereby a resolution should only exist on a ticket if status is 'closed'.

Either way is fine—whatever would confuse the fewest users. But having the ability to set a default resolution and then not doing *anything* with it is definitely confusing to them (I know because they were already doing it).

I agree - and did not even look at that for my patch (missed that point when I was reading the ticket earlier). I'll update my patch to implement your suggestion as well - it makes sense that it then becomes the default option in the drop-down.

[osimons]

Patch that a) makes sure status and resolution is not set on new tickets, and b) makes default_resoltion work as default in drop-down.

[osimons]

Replying to osimons:

I'll update my patch to implement your suggestion as well - it makes sense that it then becomes the default option in the drop-down.

Updated patch t6164-r6333-default_resolution-a.diff​. Please test & review.

[osimons]

Actually, the last drop-down fix is more correct just doing it like this:

trac/ticket/default_workflow.py

@@ -244 +244 @@
                              resolutions[0])
             else:
                 id = action + '_resolve_resolution'
-                selected_option = req.args.get(id, 'fixed')
+                selected_option = req.args.get(id,
+                        self.config.get('ticket', 'default_resolution'))
                 control.append(tag(['as ', tag.select(
                     [tag.option(x, selected=(x == selected_option or None))
                      for x in resolutions],

[Christian Boos]

The [ticket] default_resolution should also have its documented Option in TicketModule, like the others, and maybe still fixed as the default value.

Otherwise, patch (with comment:11) looks good and works fine.

[osimons]

That would be something like this, then:

trac/ticket/web_ui.py

@@ -76 +76 @@
     default_component = Option('ticket', 'default_component', '',
         """Default component for newly created tickets""")
 
+    default_resolution = Option('ticket', 'default_resolution', 'fixed',
+        """Default resolution for resolving (closing) tickets
+        (''since 0.11'').""")
+
     timeline_details = BoolOption('timeline', 'ticket_show_details', 'false',
         """Enable the display of all ticket changes in the timeline
         (''since 0.9'').""")

Ready for commit?

[Christian Boos]

Yep.

[osimons]

Patch(es) committed as [6334].

[hyuga <hyugaricdeau@…>]

Great! This is almost identical to my patch save for the fact that it also makes a consideration for default status. Works well for me.

[osimons]

Replying to hyuga <hyugaricdeau@gmail.com>:

Great! This is almost identical to my patch save for the fact that it also makes a consideration for default status. Works well for me.

Re-reading your patch, you're sure right :-) I ended up doing my own as I needed to make sense of the issues as I went along. Knowing what I now know, I would be much better off studying your patch more carefully and using it as a starting point. Catch-22 of having to understand the code of others I suppose…

Thanks for helping out!

[Christian Boos]

See also r11154.