Merge lp:~james-w/launchpad-work-items-tracker/date-based into lp:~linaro-automation/launchpad-work-items-tracker/linaro

[James Westby]

Hi,

This branch adds a new way of viewing milestones, by target
date rather than by milestone.

This means that you can have a page that shows info for all
milestones that land on the same day.

This is desirable, as the monthly milestones we will be moving
to may be named differently in different projects, but engineers
will want to see everything that they have to do for a particular
day on one page.

Thanks,

James

[Michael Hudson-Doyle (mwhudson)]

I don't think I understand the code well enough to say whether these changes make sense, I'm afraid. Happy to talk through the changes on mumble.

The sheer number of places that have to be changed to accept a date argument suggests that the layering in this code isn't really right, but I don't think that's a surprise to learn somehow :-)

review: Abstain

[James Westby (james-w)]

Resubmitting as I missed the pre-requisite branch last time, it doesn't actually
change any of the code though.

Thanks,

James

[lp:~james-w/launchpad-work-items-tracker/date-based]

283. By James Westby on 2011-03-18

Rework to use polymorpism for milestone due dates.

284. By James Westby on 2011-03-18

Changes from review comments. Thanks Michael.

[Michael Hudson-Doyle (mwhudson)]

I think this is starting to look better, but I think you should change the name of the due_data parameter in a few places? Also I think you'll find that accessing the name of a SingleMilestone won't work, because a superclass's properties override a subclasses instance data.

Finally "This should cause a query that the fragment is used in to only consider" in milestone_select_sql.__doc__ doesn't really make sense.

[lp:~james-w/launchpad-work-items-tracker/date-based]

285. By James Westby on 2011-03-22

Comments from review. Thanks Michael.

=== modified file 'burndown-chart'
--- burndown-chart	2011-02-16 23:59:52 +0000
+++ burndown-chart	2011-03-22 21:32:31 +0000
@@ -247,6 +247,8 @@
         help='Do not show foreign totals separate', dest='noforeign')
 optparser.add_option('--group',
         help='Run for this group', dest='group')
+optparser.add_option('--date',
+        help='Run for this date', dest='date')
 
 (opts, args) = optparser.parse_args()
 if not opts.database:
@@ -260,6 +262,8 @@
     optparser.error('user and group options are mutually exclusive')
 if opts.team and opts.group:
     optparser.error('team and group options are mutually exclusive')
+if opts.milestone and opts.date:
+    optparser.error('milestone and date options are mutually exclusive')
 
 # The typing allows polymorphic behavior
 if opts.user:
@@ -267,13 +271,21 @@
 elif opts.team:
     opts.team = report_tools.team_string(opts.team)
 
+db = report_tools.get_db(opts.database)
+
+milestone_collection = None
+if opts.milestone:
+    milestone_collection = report_tools.get_milestone(db, opts.milestone)
+elif opts.date:
+    milestone_collection = report_tools.MilestoneGroup(opts.date)
+
+
 # get date -> state -> count mapping
-db = report_tools.get_db(opts.database)
-data = report_tools.workitems_over_time(db, team=opts.team, milestone=opts.milestone, group=opts.group)
+data = report_tools.workitems_over_time(db, team=opts.team, group=opts.group, milestone_collection=milestone_collection)
 
 if len(data) == 0:
-    print 'WARNING: no work items, not generating chart (team: %s, milestone: %s, group: %s)' % (
-        opts.team or 'all', opts.milestone or 'none', opts.group or 'none')
+    print 'WARNING: no work items, not generating chart (team: %s, group: %s, due date: %s)' % (
+        opts.team or 'all', opts.group or 'none', milestone_collection and milestone_collection.display_name or 'none')
     sys.exit(0)
 
 # calculate start/end date if no dates are given
@@ -283,14 +295,16 @@
     start_date=opts.start_date
 
 if opts.end_date is None:
-    cur = db.cursor()
-    end_date=report_tools.milestone_due_date(db, milestone=opts.milestone)
+    if milestone_collection is not None:
+        end_date = milestone_collection.due_date
+    else:
+        end_date=report_tools.milestone_due_date(db)
 else:
     end_date=opts.end_date
 
 if not start_date or not end_date or date_to_ordinal(start_date) > date_to_ordinal(end_date):
-    print 'WARNING: empty date range, not generating chart (team: %s, milestone: %s)' % (
-        opts.team or 'all', opts.milestone or 'none')
+    print 'WARNING: empty date range, not generating chart (team: %s, group: %s, due date: %s)' % (
+        opts.team or 'all', opts.milestone or 'none', opts.group or 'none', milestone_collection and milestone_collection.display_name or 'none')
     sys.exit(0)
 
 # title
@@ -305,8 +319,7 @@
     # It never makese sense in all teams mode
     noforeign = True
 
-if opts.milestone:
-    title += ' (%s)' % opts.milestone
+if milestone_collection is not None:
+    title += ' (%s)' % milestone_collection.name
 
 do_chart(data, start_date, end_date, opts.trendstart, title, opts.output, opts.only_weekdays, opts.inverted, noforeign)
-

=== modified file 'generate-all'
--- generate-all	2011-02-26 00:57:42 +0000
+++ generate-all	2011-03-22 21:32:31 +0000
@@ -34,11 +34,15 @@
     optparser.error('output directory does not exist')
 
 if opts.config:
-    trend_starts = report_tools.load_config(opts.config).get('trend_start', {})
-    burnup_chart_teams = report_tools.load_config(opts.config).get('burnup_chart_teams', [])
+    cfg = report_tools.load_config(opts.config)
+    trend_starts = cfg.get('trend_start', {})
+    burnup_chart_teams = cfg.get('burnup_chart_teams', [])
+    primary_team = cfg.get("primary_team", None)
 else:
+    cfg = None
     trend_starts = {}
     burnup_chart_teams = []
+    primary_team = None
 
 lock_path = opts.database + ".generate_lock"
 lock_f = open(lock_path, "wb")
@@ -104,6 +108,12 @@
     else:
         report_tools.run_reports(my_path, opts.database, basename, opts.config, milestone=m, team=None, root=opts.root)
 
+# date-based milestone view
+for mg in report_tools.milestone_groups(db, team=primary_team):
+    due_date_str = mg.due_date_str
+    basename = os.path.join(opts.output_dir, due_date_str)
+    report_tools.run_reports(my_path, opts.database, basename, opts.config, team=primary_team, root=opts.root, date=due_date_str)
+
 # groups
 for g in groups:
     basename = os.path.join(groupssubdir, g)

=== modified file 'html-report'
--- html-report	2011-03-10 16:18:45 +0000
+++ html-report	2011-03-22 21:32:31 +0000
@@ -105,8 +105,8 @@
         return self._assignee or self.blueprint.assignee
 
 
-def spec_group_completion(db, team, milestone):
-    data = report_tools.spec_group_completion(db, team, milestone)
+def spec_group_completion(db, team, milestone_collection=None):
+    data = report_tools.spec_group_completion(db, team, milestone_collection=milestone_collection)
     if not data:
         return dict(areas=[], group_completion_series=[], groups=[])
     groups = []
@@ -133,8 +133,8 @@
                 groups=sorted(groups, key=lambda x: report_tools.priority_value(x.priority), reverse=True))
 
 
-def spec_completion(db, team, milestone):
-    data = report_tools.blueprint_completion(db, team, milestone)
+def spec_completion(db, team, milestone_collection=None):
+    data = report_tools.blueprint_completion(db, team, milestone_collection=milestone_collection)
 
     blueprints = []
     all_workitems = WorkitemTarget()
@@ -189,8 +189,8 @@
         unknown_workitems=unknown_workitems,
         )
 
-def get_assignee_completion(db, team=None, milestone=None, group=None):
-    data = report_tools.assignee_completion(db, team=team, milestone=milestone, group=group)
+def get_assignee_completion(db, team=None, group=None, milestone_collection=None):
+    data = report_tools.assignee_completion(db, team=team, group=group, milestone_collection=milestone_collection)
 
     def _sort(a, b):
         # Takes [blueprint, workitem, priority, url] in a and b.
@@ -205,10 +205,10 @@
     for name, info in data.items():
         name = name or team or 'nobody'
         name_html = escape_url(name or team or 'nobody')
-        if not milestone or isinstance(team, report_tools.user_string):
+        if not milestone_collection or not milestone_collection.single_milestone or isinstance(team, report_tools.user_string):
             url = '%s/~%s/+specs?role=assignee' % (report_tools.blueprints_base_url, name_html)
         else:
-            target = name_html + '-' + milestone
+            target = name_html + '-' + milestone_collection.name
             url = 'u/%s.html' % target
         assignees.append(
             Assignee(name, url, complexity=info['complexity'],
@@ -220,8 +220,9 @@
     return assignees
 
 
-def by_assignee(db, team=None, milestone=None, group=None):
-    assignees = get_assignee_completion(db, team=team, milestone=milestone, group=group)
+def by_assignee(db, team=None, group=None, milestone_collection=None):
+    assignees = get_assignee_completion(
+        db, team=team, group=group, milestone_collection=milestone_collection)
     assignees.sort(key=lambda a: a.name, reverse=False)
 
     # avoid showing empty columns
@@ -328,6 +329,14 @@
         data.update(report_tools.days_left(db, milestone=opts.milestone))
         return data
 
+    def get_milestone_collection(self, db, opts):
+        milestone_collection = None
+        if opts.milestone:
+            milestone_collection = report_tools.get_milestone(db, opts.milestone)
+        elif opts.date:
+            milestone_collection = report_tools.MilestoneGroup(opts.date)
+        return milestone_collection
+
     def burndown(self, db, opts):
         '''Print work item status as HTML.'''
 
@@ -339,15 +348,17 @@
         else:
             title = opts.title
 
-        if opts.milestone:
-            title += ' (%s)' % opts.milestone
+        milestone_collection = self.get_milestone_collection(db, opts)
+        if milestone_collection is not None:
+            title += ' (%s)' % milestone_collection.name
 
         data = self.template_data(db, opts)
         data.update(dict(team_name=title, chart_url=opts.chart_url))
-        data.update(spec_group_completion(db, opts.team, opts.milestone))
-        data.update(spec_completion(db, opts.team, opts.milestone))
-        data.update(by_assignee(db, opts.team, opts.milestone))
-        if opts.milestone:
+        data.update(
+            spec_group_completion(db, opts.team, milestone_collection=milestone_collection))
+        data.update(spec_completion(db, opts.team, milestone_collection=milestone_collection))
+        data.update(by_assignee(db, team=opts.team, milestone_collection=milestone_collection))
+        if milestone_collection is not None:
             data.update(dict(page_type="milestones"))
         elif isinstance(opts.team, report_tools.user_string):
             data.update(dict(page_type="people"))
@@ -364,13 +375,15 @@
         else:
             title = opts.title
 
-        if opts.milestone:
-            title += ' (%s)' % opts.milestone
+        milestone_collection = self.get_milestone_collection(db, opts)
+
+        if milestone_collection is not None:
+            title += ' (%s)' % milestone_collection.name
 
         data = self.template_data(db, opts)
         data.update(dict(team_name=title, chart_url=opts.chart_url))
-        data.update(spec_group_completion(db, None, opts.milestone))
-        data.update(spec_completion(db, opts.team, opts.milestone))
+        data.update(spec_group_completion(db, None, milestone_collection=milestone_collection))
+        data.update(spec_completion(db, opts.team, milestone_collection=milestone_collection))
         data.update(blueprint_count(db, opts.team, opts.milestone))
         data.update(dict(page_type="overview"))
         print report_tools.fill_template("overview.html", data)
@@ -456,12 +469,16 @@
             help="Root URL for the charts")
     optparser.add_option('--status', dest="status",
             help="Workitem status to consider for workitem_list report-type")
+    optparser.add_option('--date', dest="date",
+            help="Include all milestones targetted to this date.")
 
     (opts, args) = optparser.parse_args()
     if not opts.database:
         optparser.error('No database given')
     if opts.user and  opts.team:
         optparser.error('team and user options are mutually exclusive')
+    if opts.milestone and opts.date:
+        optparser.error('milestone and date options are mutually exclusive')
 
     # The typing allows polymorphic behavior
     if opts.user:

=== modified file 'report_tools.py'
--- report_tools.py	2011-03-10 16:18:45 +0000
+++ report_tools.py	2011-03-22 21:32:31 +0000
@@ -24,13 +24,15 @@
 def escape_sql(text):
     return text.replace("'", "''")
 
-def report_args(args, team=None, user=None, milestone=None):
+def report_args(args, team=None, user=None, milestone=None, date=None):
     if milestone:
         args.extend(['-m',milestone])
     if team:
         args.extend(['-t',team])
     if user:
         args.extend(['-u',user])
+    if date:
+        args.extend(['--date', date])
 
 def status_overview(my_path, database, basename, config, root=None):
     cfg = load_config(config)
@@ -166,13 +168,17 @@
 
 
 def run_reports(my_path, database, basename, config, milestone=None, team=None,
-        user=None, trend_starts=None, trend_override=None, burnup=False, root=None):
+        user=None, trend_starts=None, trend_override=None, burnup=False, root=None, date=None):
 
     ident = (team or user)
     if milestone and ident:
         chartname = ident + '-' + milestone
     elif milestone:
         chartname = 'all-' + milestone
+    elif date and ident:
+        chartname = "%s-%s" % (date, milestone)
+    elif date:
+        chartname = 'all-' + date
     elif ident:
         chartname = ident
     else:
@@ -186,16 +192,17 @@
         args += ['--title', cfg['title']]
     if root:
         args += ['--root', root]
-    report_args(args, team, user, milestone)
+    report_args(args, team=team, user=user, milestone=milestone, date=date)
     hreport = Popen(args, stdout=hf)
 
-    jf = open(basename + '.json', 'w')
-    args = [os.path.join(my_path, 'json-report'), '-d', database, '-c', config]
-    report_args(args, team, user, milestone)
-    jreport = Popen(args, stdout=jf)
+    if not date: # date support not implemented in json-report yet
+        jf = open(basename + '.json', 'w')
+        args = [os.path.join(my_path, 'json-report'), '-d', database, '-c', config]
+        report_args(args, team=team, user=user, milestone=milestone)
+        jreport = Popen(args, stdout=jf)
 
     args = [os.path.join(my_path, 'burndown-chart'), '-d', database, '-o', basename + '.svg']
-    report_args(args, team, user, milestone)
+    report_args(args, team=team, user=user, milestone=milestone, date=date)
 
     if trend_override:
         args.extend(['--trend-start', trend_override])
@@ -203,7 +210,7 @@
         if milestone and ident:
             if (ident, milestone) in trend_starts:
                 args.extend(['--trend-start', str(trend_starts[(ident, milestone)])])
-        elif ident:
+        elif ident and not date:
             if ident in trend_starts:
                 args.extend(['--trend-start', str(trend_starts[ident])])
     if burnup:
@@ -259,32 +266,7 @@
     return escape(html, True)
 
 
-def get_milestone_start_sql(db, milestone):
-    if milestone:
-        # get milestone date range
-        cur = db.cursor()
-        cur.execute('SELECT due_date, project FROM milestones WHERE name = ?', (milestone,))
-        row = cur.fetchone()
-        date_end = row[0]
-        project = row[1]
-        cur.execute('SELECT MAX(due_date), MAX(due_date) < date(CURRENT_TIMESTAMP) FROM milestones WHERE due_date < ? AND project = ?', (date_end, project))
-        (date_start, in_milestone) = cur.fetchone()
-
-        # if we are already within the milestone, start at the previous
-        # milestone's end date; if we are before, start at the beginning of the
-        # entire cycle, to allow planning
-        if in_milestone:
-            ms_sql = "AND w.milestone='%s' AND date >= '%s' AND date <= '%s'" % (
-                    escape_sql(milestone), date_start, date_end)
-        else:
-            ms_sql = "AND w.milestone='%s' AND date <= '%s'" % (
-                    escape_sql(milestone), date_end)
-    else:
-        ms_sql = ''
-    return ms_sql
-
-
-def workitems_over_time(db, team=None, milestone=None, group=None):
+def workitems_over_time(db, team=None, group=None, milestone_collection=None):
     '''Calculate work item development over time.
 
     If team is given, this filters out blueprints for that particular team. If
@@ -296,10 +278,12 @@
     {todo,inprogress,done,postponed}{,_teamonly}.
     '''
     data = {}
-    ms_sql = get_milestone_start_sql(db, milestone)
+    cur = db.cursor()
+    ms_sql = ''
+    if milestone_collection is not None:
+        ms_sql = milestone_collection.milestone_start_sql(cur)
     # include both direct team assignments, as well as assignmets to
     # members of that team
-    cur = db.cursor()
     if team:
         # WIs which are assigned to team members
         if isinstance(team, user_string):
@@ -361,14 +345,16 @@
             data.setdefault(date, {})[s] = num
     return data
 
-def spec_group_completion(db, team=None, milestone=None):
+def milestone_select_sql(milestone_collection=None):
+    if milestone_collection is not None:
+        ms_sql = milestone_collection.milestone_select_sql()
+    else:
+        ms_sql = "AND w.milestone IS NULL"
+    return ms_sql
+
+def spec_group_completion(db, team=None, milestone_collection=None):
     data = {}
-    if milestone == '':
-        ms_sql = "AND w.milestone IS NULL"
-    elif milestone:
-        ms_sql = "AND w.milestone='%s'" % escape_sql(milestone)
-    else:
-        ms_sql = ''
+    ms_sql = milestone_select_sql(milestone_collection=milestone_collection)
 
     # last date
     cur = db.cursor()
@@ -451,19 +437,14 @@
     return info
 
 
-def blueprint_completion(db, team=None, milestone=None):
+def blueprint_completion(db, team=None, milestone_collection=None):
     '''Determine current blueprint completion.
 
     Return blueprint -> info mapping, with info being a map with these
     keys: todo, inprogress, done, postponed, status, priority, implementation, url.
     '''
     data = {}
-    if milestone == '':
-        ms_sql = "AND w.milestone IS NULL"
-    elif milestone:
-        ms_sql = "AND w.milestone='%s'" % escape_sql(milestone)
-    else:
-        ms_sql = ''
+    ms_sql = milestone_select_sql(milestone_collection=milestone_collection)
 
     # last date
     cur = db.cursor()
@@ -651,7 +632,8 @@
 
 
 def milestone_start_date(db, milestone=None):
-    ms_sql = get_milestone_start_sql(db, milestone)
+    cur = db.cursor()
+    ms_sql = SingleMilestone(milestone, None, None, None).milestone_start_sql(cur)
     cur = db.cursor()
     cur.execute('SELECT MIN(date) FROM work_items w '
             'WHERE 1=1 %s GROUP BY date' % ms_sql)
@@ -679,7 +661,7 @@
         )
 
 
-def assignee_completion(db, team=None, milestone=None, group=None):
+def assignee_completion(db, team=None, group=None, milestone_collection=None):
     '''Determine current by-assignee completion.
 
     Return assignee -> info mapping with info being a map with these
@@ -687,12 +669,7 @@
     workitem, priority, spec_url].
     '''
     data = {}
-    if milestone == '':
-        ms_sql = "AND w.milestone IS NULL"
-    elif milestone:
-        ms_sql = "AND w.milestone='%s'" % milestone
-    else:
-        ms_sql = ''
+    ms_sql = milestone_select_sql(milestone_collection=milestone_collection)
 
     # last date
     cur = db.cursor()
@@ -905,21 +882,119 @@
         return milestones
 
 
-class Milestone(object):
+class MilestoneCollection(object):
+
+    single_milestone = False
+
+    def __init__(self, due_date):
+        self.due_date = due_date
+
+    @property
+    def due_date_str(self):
+        return self.due_date.strftime("%Y-%m-%d")
+
+    @property
+    def display_name(self):
+        """A name suitable for display for this collection."""
+        raise NotImplementedError(self.display_name)
+
+    def milestone_start_sql(self, cur):
+        """Returns an SQL fragment that restricts workitem data to this collection.
+
+        It can restrict to workitems targeted to milestones in this collection.
+
+        It can also restrict to workitem data records that are after this milestone
+        started, and before it completed.
+
+        :param cur: a db cursor that can be used to query needed information.
+        """
+        raise NotImplementedError(self.milestone_start_sql)
+
+    def milestone_select_sql(self):
+        """Returns an SQL fragments that restricts workitems to this collection.
+
+        If the returned fragment is used in an SQL query then that query should only
+        consider workitems targeted to the milestones in this collection.
+        """
+        raise NotImplementedError(self.milestone_select_sql)
+
+
+class SingleMilestone(MilestoneCollection):
+
+    single_milestone = True
 
     def __init__(self, name, link, project, due_date):
+        super(SingleMilestone, self).__init__(due_date)
         self.name = name
         self.link = link
         self.project = project
-        self.due_date = due_date
-
-
-class MilestoneGroup(object):
+
+    @property
+    def display_name(self):
+        return "milestone %s" % self.name
+
+    def milestone_start_sql(self, cur):
+        cur.execute('SELECT due_date, project FROM milestones WHERE name = ?', (self.name,))
+        row = cur.fetchone()
+        date_end = row[0]
+        project = row[1]
+        cur.execute('SELECT MAX(due_date), MAX(due_date) < date(CURRENT_TIMESTAMP) FROM milestones WHERE due_date < ? AND project = ?', (date_end, project))
+        (date_start, in_milestone) = cur.fetchone()
+
+        # if we are already within the milestone, start at the previous
+        # milestone's end date; if we are before, start at the beginning of the
+        # entire cycle, to allow planning
+        if in_milestone:
+            ms_sql = "AND w.milestone='%s' AND date >= '%s' AND date <= '%s'" % (
+                    escape_sql(self.name), date_start, date_end)
+        else:
+            ms_sql = "AND w.milestone='%s' AND date <= '%s'" % (
+                    escape_sql(self.name), date_end)
+        return ms_sql
+
+    def milestone_select_sql(self):
+        return "AND w.milestone='%s'" % escape_sql(self.name)
+
+
+class MilestoneGroup(MilestoneCollection):
 
     def __init__(self, due_date):
-        self.due_date = due_date
+        super(MilestoneGroup, self).__init__(due_date)
         self.milestones = []
 
+    @property
+    def name(self):
+        return self.due_date_str
+
+    @property
+    def display_name(self):
+        return "any milestone on %s" % self.due_date_str
+
+    def milestone_start_sql(self, cur):
+        cur.execute('SELECT MAX(due_date), MAX(due_date) < date(CURRENT_TIMESTAMP) FROM milestones WHERE due_date < ?', (escape_sql(date), ))
+        (date_start, in_milestone) = cur.fetchone()
+        if in_milestone:
+            ms_sql = "AND date >= '%s' AND date <= '%s'" % (date_start, escape_sql(date))
+        else:
+            ms_sql = "AND date <= '%s'" % escape_sql(date)
+        ms_sql += "AND w.milestone IN (SELECT m.name from milestones m WHERE m.due_date = '%s')" % escape_sql(date)
+        return ms_sql
+
+    def milestone_select_sql(self):
+        return (
+            "AND w.milestone IN "
+            "(SELECT m.name from milestones m WHERE m.due_date = '%s')"
+                % self.due_date_str)
+
+
+def get_milestone(db, milestone_name):
+    cur = db.cursor()
+    cur.execute(
+        "SELECT project, due_date from milestones where name = ?",
+        milestone_name)
+    project, due_date = cur.fetchone()
+    return SingleMilestone(milestone_name, None, project, date_to_python(due_date))
+
 
 def milestone_groups(db, team=None):
     if team:
@@ -938,7 +1013,7 @@
         else:
             group = groups[-1]
         group.milestones.append(
-            Milestone(name, template % name, project, due_date))
+            SingleMilestone(name, template % name, project, due_date))
     return groups
 
 

=== modified file 'templates/milestone_list.html'
--- templates/milestone_list.html	2011-03-10 16:18:45 +0000
+++ templates/milestone_list.html	2011-03-22 21:32:31 +0000
@@ -30,7 +30,7 @@
 &gt;
 % endif
     </td>
-    <td class="date"><span class="connector">-</span> ${group.due_date}</td>
+    <td class="date"><span class="connector">-</span> <a href="${base.url(group.due_date_str+".html")}">${group.due_date}</a></td>
     <td class="milestones">
 % for milestone in group.milestones:
 <a href="${base.url(milestone.link)}">${milestone.name} (${milestone.project})</a>