Edgewall Software
Modify

Opened 8 years ago

Closed 5 years ago

Last modified 5 years ago

#10010 closed enhancement (fixed)

Allow configuring the default "retargeting" option when closing or deleting a milestone

Reported by: Yaniv Vararu <yvararu@…> Owned by: Ryan J Ollos
Priority: low Milestone: 1.1.2
Component: roadmap Version: 0.12
Severity: minor Keywords: milestone
Cc: Thijs Triemstra
Release Notes:

The option [milestone] default_retarget_to determines the default milestone to which tickets are retargeted when closing or deleting a milestone. The option can be set from the Milestone Admin page.

API Changes:

Description

It would be nice to have an option to configure the default milestone that appears in the "retargeting" option when closing a milestone.

Currently, the default is "None" (Which if not paying attention - might cause you to "lose" some tickets out in the open). I for example, would like to have my "Backlog" milestone be the default option for tickets not handled in a specific milestone. Adding such an option to the "trac.ini" file would be great.

Attachments (2)

ManageMilestones.png (31.5 KB ) - added by Ryan J Ollos 5 years ago.
ManageMilestones2.png (33.0 KB ) - added by Ryan J Ollos 5 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 8 years ago by Christian Boos

Milestone: unscheduled

Nice idea, PatchWelcome.

comment:2 Changed 8 years ago by Christian Boos

Milestone: unschedulednext-major-0.1X
Priority: normallow
Severity: normalminor

comment:3 Changed 8 years ago by Thijs Triemstra

Cc: Thijs Triemstra added

comment:4 Changed 5 years ago by Ryan J Ollos

Milestone: next-major-releasesnext-dev-1.1.x
Owner: set to Ryan J Ollos
Status: newassigned

What should the option be named? [milestone] default_retarget_to?

comment:5 Changed 5 years ago by Ryan J Ollos

Milestone: next-dev-1.1.x1.1.2
Summary: Enable to configure the default "retargeting" option when closing a milestoneAllow configuring the default "retargeting" option when closing or deleting a milestone

Proposed changes can be found in repos:rjollos.git:t10010.

comment:6 Changed 5 years ago by Ryan J Ollos

The proposed changes are pretty straightforward, but I'd be interested to hear whether others think it makes sense to have the _default_retarget_to property for the purpose of logging a warning when default_retarget_to doesn't exist. Oh, and I had better modify the patch slightly so that the warning message is not logged when [milestone] default_target_to doesn't exist in trac.ini or its value is empty. I'll make the change in a moment.

comment:7 Changed 5 years ago by Ryan J Ollos

I've fixed the logging issue in [a760b2c0/rjollos.git] and will squash the changes before committing.

comment:8 Changed 5 years ago by Jun Omae

I've tried rjollos.git:t10010 branch. It works well.

I have the following suggestions.

  • any() can accept an iterable object
  • We should pass a format string and the parameters to log.warn() instead of the formatted text.
  • trac/ticket/roadmap.py

    diff --git a/trac/ticket/roadmap.py b/trac/ticket/roadmap.py
    index 51ed6ff..488556f 100644
    a b class MilestoneModule(Component):  
    698698    @property
    699699    def _default_retarget_to(self):
    700700        if self.default_retarget_to and \
    701            not any([self.default_retarget_to == m.name
    702                     for m in Milestone.select(self.env)]):
     701           not any(self.default_retarget_to == m.name
     702                   for m in Milestone.select(self.env)):
    703703            self.log.warn('Milestone "%s" does not exist. Update the '
    704704                          '"default_retarget_to" option in the '
    705                           '[milestone] section of trac.ini'
    706                           % self.default_retarget_to)
     705                          '[milestone] section of trac.ini',
     706                          self.default_retarget_to)
    707707        return self.default_retarget_to
    708708
    709709    def _do_delete(self, req, milestone):

comment:9 Changed 5 years ago by Ryan J Ollos

Thanks, Jun. I'll make those changes.

Would it make sense to replace default_retarget_to instead of adding _default_retarget_to, in order to make it unlikely that someone extending the class might call the attribute that doesn't do the logging rather than the attribute that does the logging?

  • trac/ticket/roadmap.py

    diff --git a/trac/ticket/roadmap.py b/trac/ticket/roadmap.py
    index 51ed6ff..5c7eca0 100644
    a b class MilestoneModule(Component):  
    695695
    696696    # Internal methods
    697697
     698    _default_retarget_to = default_retarget_to
     699
    698700    @property
    699     def _default_retarget_to(self):
    700         if self.default_retarget_to and \
    701            not any([self.default_retarget_to == m.name
     701    def default_retarget_to(self):
     702        if self._default_retarget_to and \
     703           not any([self._default_retarget_to == m.name
    702704                    for m in Milestone.select(self.env)]):
    703705            self.log.warn('Milestone "%s" does not exist. Update the '
    704706                          '"default_retarget_to" option in the '
    705707                          '[milestone] section of trac.ini'
    706                           % self.default_retarget_to)
    707         return self.default_retarget_to
     708                          % self._default_retarget_to)
     709        return self._default_retarget_to
    708710
    709711    def _do_delete(self, req, milestone):
    710712        req.perm(milestone.resource).require('MILESTONE_DELETE')
    class MilestoneModule(Component):  
    803805            'milestone': milestone,
    804806            'milestone_groups': group_milestones(milestones,
    805807                'TICKET_ADMIN' in req.perm),
    806             'retarget_to': self._default_retarget_to
     808            'retarget_to': self.default_retarget_to
    807809        }
    808810        return 'milestone_delete.html', data, None
    809811
    class MilestoneModule(Component):  
    829831                          and 'MILESTONE_VIEW' in req.perm(m.resource)]
    830832            data['milestone_groups'] = group_milestones(milestones,
    831833                'TICKET_ADMIN' in req.perm)
    832             data['retarget_to'] = self._default_retarget_to
     834            data['retarget_to'] = self.default_retarget_to
    833835        else:
    834836            req.perm(milestone.resource).require('MILESTONE_CREATE')

comment:10 Changed 5 years ago by Ryan J Ollos

Latest proposed changes (including changes discussed in comment:8 and comment:9) can be found in repos:rjollos.git:t10010.2.

Last edited 5 years ago by Ryan J Ollos (previous) (diff)

comment:11 Changed 5 years ago by Ryan J Ollos

Release Notes: modified (diff)

comment:12 Changed 5 years ago by Ryan J Ollos

Oh, I should add a (since 1.1.2) to the TracIni doc string before pushing the change.

comment:13 Changed 5 years ago by Ryan J Ollos

Resolution: fixed
Status: assignedclosed

Changes committed to trunk in [12121].

Changed 5 years ago by Ryan J Ollos

Attachment: ManageMilestones.png added

Changed 5 years ago by Ryan J Ollos

Attachment: ManageMilestones2.png added

comment:14 Changed 5 years ago by Ryan J Ollos

I thought it would be useful to allow the default_retarget_to option to be set from the milestone admin page.

The text Retarget may not be self-explanatory enough, despite having a title attribute, so I expanded the text a bit:

Any preferences?

One other nuance is that the Clear Default button added in #11300 will become Clear Defaults, and will clear both values. I suppose 2 buttons could be added, but it seems like a bit of clutter for something that's not too inconvenient.

Last edited 5 years ago by Ryan J Ollos (previous) (diff)

comment:15 Changed 5 years ago by Remy Blank

I have a slight preference for the former (short labels, "Default" to the left of "Retarget"). You could add tooltips to both column headings to explain them in more details.

comment:16 Changed 5 years ago by Ryan J Ollos

Yeah, after looking at it for a while I also think that the former is better. Proposed changes can be found in log:rjollos.git:t10010.3. My primary concerns are:

  • Is it worth changing the dictionary passed to the template just for making the variable name less ambiguous since this could affect a plugin?: [568f666c/rjollos.git]
  • Are there too many tooltips? Should the tooltips be placed on the radio buttons rather or in addition to the headings?: [64a809e2/rjollos.git]

comment:17 Changed 5 years ago by Ryan J Ollos

Release Notes: modified (diff)

Committed to trunk in [12262].

comment:18 Changed 5 years ago by Ryan J Ollos

Keywords: customization removed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.