Edgewall Software
Modify

Opened 15 years ago

Last modified 7 years ago

#6232 new enhancement

Setting different completion ratios on different ticket states in [milestone-groups]

Reported by: tjorvi@… Owned by:
Priority: low Milestone: next-major-releases
Component: roadmap Version: devel
Severity: minor Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

It would make sense to be able to set [milestone-groups] ticketstate.overall_completion to a percentage rather than a boolean.

I have set up a ticket workflow that goes like this: open → programmed → reviewed → tested (closed). I want to assign completion ratios something like this:

  • programmed - 40%
  • reviewed - 20%
  • tested - 40%

That in effect means that if all tickets were programmed, but none open, reviewed or tested, the overall completion would stand at 40%.

Attachments (0)

Change History (11)

comment:1 by Noah Kantrowitz, 15 years ago

Resolution: wontfix
Status: newclosed

Duplicate of #1534. Should be a plugin as well.

comment:2 by anonymous, 15 years ago

I don't belive this is really a duplicate of #1534. #1534 talks about assigning completion ratios to individual tickets, while I mean't to assign pre-set completion ratios to ticket states.

You're right that this could be a plugin, but this plugin would be almost identical to the DefaultTicketGroupStatsProvider.

I have modified DefaultTicketGroupStatsProvider to do this on my setup, by modifying two lines:

Index: trac/ticket/roadmap.py =================================================================== —- trac/ticket/roadmap.py (revision 6080) +++ trac/ticket/roadmap.py (working copy) @@ -105,7 +105,7 @@

float(self.count) * 100))

total_percent = total_percent + intervalpercent if intervaloverall_completion:

  • self.done_percent += intervalpercent

+ self.done_percent += intervalpercent * float(interval['over all_completion'])

self.done_count += intervalcount

# We want the percentages to add up to 100%. To do that, we fudge the

@@ -240,7 +240,7 @@

query_args[k] = v

stat.add_interval(groupname, group_cnt, query_args,

group.get('css_class', groupname),

  • bool(group.get('overall_completion')))

+ group.get('overall_completion'))

stat.refresh_calcs() return stat

in reply to:  2 comment:3 by Christian Boos, 15 years ago

Replying to anonymous:

Index: trac/ticket/roadmap.py
===================================================================
--- trac/ticket/roadmap.py      (revision 6080)
+++ trac/ticket/roadmap.py      (working copy)
@@ -105,7 +105,7 @@
                                         float(self.count) * 100))
             total_percent = total_percent + interval['percent']
             if interval['overall_completion']:
-                self.done_percent += interval['percent']
+                self.done_percent += interval['percent'] * float(interval['over
all_completion'])
                 self.done_count += interval['count']

         # We want the percentages to add up to 100%.  To do that, we fudge the
@@ -240,7 +240,7 @@
                 query_args[k] = v
             stat.add_interval(group['name'], group_cnt, query_args,
                               group.get('css_class', group['name']),
-                              bool(group.get('overall_completion')))
+                              group.get('overall_completion'))
         stat.refresh_calcs()
         return stat

Fixing the patch presentation. Well, I have yet to try it myself, but that would probably need some additional work before getting accepted, like making it a bit fool-proof and compatible with the simpler bool usage.

comment:4 by tjorvi@…, 15 years ago

Thanks for that.

It would probable also need to parse percentages as a string ("25%" etc.) instead of only floating point numbers. As it is in my code it will handle "0" and "1" correctly but not "true" and "false".

comment:5 by Noah Kantrowitz, 15 years ago

I'm really going to insist this not go in core. Its clearly not a minimalist feature in my mind. The new stats provider would be very similar to the default one, thats what subclassing is for. We need to focus more time in moving features out of core, not the other way around.

comment:6 by anonymous, 15 years ago

That's a good point.

Maybe it would make sense to break down the routines in DefaultTicketGroupStatsProvider more to make subclassing a better option. Routines like get_ticket_group_stats are very long and there's no way to hook into them in a subclass, other than replacing them completely. Ideally they should call other functions to do much of the work, which could then be modified individually in a subclass.

in reply to:  6 comment:7 by Noah Kantrowitz, 15 years ago

Replying to anonymous:

That's a good point.

Maybe it would make sense to break down the routines in DefaultTicketGroupStatsProvider more to make subclassing a better option. Routines like get_ticket_group_stats are very long and there's no way to hook into them in a subclass, other than replacing them completely. Ideally they should call other functions to do much of the work, which could then be modified individually in a subclass.

That I would be +1 on, as long as it isn't too complicating. As a fallback position, a cut n' paste of the default code into the plugin doesn't bother me all that much as the code in question is so likely to change that it needs huge levels of abstraction.

comment:8 by tjorvi@…, 15 years ago

The docstrings say that ITicketGroupStatsProvider.get_ticket_group_stats() should return a valid TicketGroupStats object. For this sort of plugin to be made it requires subclassing, or re-implementing, of TicketGroupStats too.

What would be the definition of "a valid TicketGroupStats object"? An object that has exactly the same set of member variables and methods as TicketGroupStats? Or can it omit one or other of the methods?

comment:9 by tjorvi@…, 15 years ago

Here is a patch that splits get_ticket_group_stats() into more managable chunks, without changing any functionality:

Index: trac/ticket/roadmap.py
===================================================================
--- trac/ticket/roadmap.py	(revision 6085)
+++ trac/ticket/roadmap.py	(working copy)
@@ -160,6 +160,8 @@
         {'name': 'active', 'status': '*', 'css_class': 'open'}
         ]
 
+    ticket_group_stat_class = TicketGroupStats
+
     def _get_ticket_groups(self):
         """Returns a list of dict describing the ticket groups
         in the expected order of appearance in the milestone progress bars.
@@ -180,9 +182,10 @@
         else:
             return self.default_milestone_groups
 
-    def get_ticket_group_stats(self, ticket_ids):
+    def _count_status(self, all_statuses, ticket_ids):
+        """Returns a dictionary holding the count of tickets for each status
+        group"""
         total_cnt = len(ticket_ids)
-        all_statuses = set(TicketSystem(self.env).get_all_status())
         status_cnt = {}
         for s in all_statuses:
             status_cnt[s] = 0
@@ -194,8 +197,15 @@
                            ",".join(str_ids))
             for s, cnt in cursor:
                 status_cnt[s] = cnt
+        return status_cnt
 
-        stat = TicketGroupStats('ticket status', 'ticket')
+    def _parse_group_statuses(self, group_statuses_str):
+        """Takes a line of the form "status1, status2, status3" and returns
+        a set of the statuses"""
+        return set([s.strip()
+                    for s in group_statuses_str.split(',')])
+
+    def _read_ticket_groups(self, all_statuses):
         remaining_statuses = set(all_statuses)
         groups =  self._get_ticket_groups()
         catch_all_group = None
@@ -212,9 +222,8 @@
                         group1=group['name'], group2=catch_all_group['name']))
                 catch_all_group = group
             else:
-                group_statuses = set([s.strip()
-                                      for s in status_str.split(',')]) \
-                                      & all_statuses
+                group_statuses = self._parse_group_statuses(status_str) \
+                                 & all_statuses
                 if group_statuses - remaining_statuses:
                     raise TracError(_(
                         "'%(groupname)s' milestone group reused status "
@@ -227,6 +236,16 @@
                 group['statuses'] = group_statuses
         if catch_all_group:
             catch_all_group['statuses'] = remaining_statuses
+
+        return groups
+
+    def get_ticket_group_stats(self, ticket_ids):
+        all_statuses = set(TicketSystem(self.env).get_all_status())
+        status_cnt = self._count_status(all_statuses, ticket_ids)
+
+        stat = self.ticket_group_stat_class('ticket status', 'ticket')
+        groups = self._read_ticket_groups(all_statuses)
+
         for group in groups:
             group_cnt = 0
             query_args = {}

comment:10 by Christian Boos, 14 years ago

Milestone: 0.12
Priority: normallow
Resolution: wontfix
Severity: normalminor
Status: closedreopened

Re-open as a reminder for the above refactoring patch.

comment:11 by Ryan J Ollos, 7 years ago

Owner: Christopher Lenz removed
Status: reopenednew

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


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