Merge lp://staging/~james-w/launchpad-work-items-tracker/workitem-list-pages into lp://staging/~linaro-automation/launchpad-work-items-tracker/linaro

Hi James,

I think this looks good but the new functions/methods would benefit from
some docstrings. I also have a few comments below.

 review approve

On Sat, 2011-02-26 at 01:02 +0000, James Westby wrote:
[...]
> === modified file 'generate-all'
> --- generate-all 2011-02-06 20:00:12 +0000
> +++ generate-all 2011-02-26 01:01:57 +0000
> @@ -133,6 +133,11 @@
> basename = os.path.join(opts.output_dir, "about")
> report_tools.about_page(my_path, opts.database, basename, opts.config, root=opts.root)
>
> +# workitems in status pages
> +for status in report_tools.valid_states:
> + basename = os.path.join(opts.output_dir, status)
> + report_tools.workitem_list(my_path, opts.database, basename, opts.config, status, root=opts.root)
> +
> # front page
> basename = os.path.join(opts.output_dir, 'index')
> report_tools.status_overview(my_path, opts.database, basename, opts.config, root=opts.root)
>
> === modified file 'html-report'
> --- html-report 2011-02-25 02:56:48 +0000
> +++ html-report 2011-02-26 01:01:57 +0000
> @@ -47,6 +47,7 @@
> self.priority = priority
> self.status = status
>
> +
> class Assignee(WorkitemTarget):
>
> def __init__(self, name, url, complexity=0, todo_wis=[],
> @@ -90,6 +91,19 @@
> return any([g.priority for g in self.groups])
>
>
> +class Workitem(object):
> +
> + def __init__(self, description, status, blueprint, assignee=None):
> + self.description = description
> + self.status = status
> + self.blueprint = blueprint
> + self._assignee = assignee
> +
> + @property
> + def assignee(self):
> + return self._assignee or self.blueprint.assignee
> +
> +
> def spec_group_completion(db, team, milestone):
> data = report_tools.spec_group_completion(db, team, milestone)
> if not data:
> @@ -286,6 +300,21 @@
> return group
>
>
> +def workitems_in_status(db, status, team=None, milestone=None):
> + data = report_tools.assignee_completion(
> + db, team=team, milestone=milestone)
> + workitems = []
> + for assignee in data:
> + if status in data[assignee]:
> + for wi_info in data[assignee][status]:
> + bp_name, description, priority, url = wi_info
> + blueprint = Blueprint(bp_name, url, priority=priority)
> + workitems.append(
> + Workitem(description, status, blueprint,
> + assignee=assignee))
> + return workitems
> +
> +
> def list_people(db, team=None):
> team_info = report_tools.team_information(db, team=team)
> if team is None:
> @@ -398,6 +427,17 @@
> data.update(dict(page_type="about"))
> print report_tools.fill_template("about.html", data)
>
> + def workitem_list(self, db, opts):
> + assert opts.status is not None, (
> + "Must pass --status for workitem_list report-type")
> + workitems = workitems_in_status(
> + db, opts.status, team=opts.team, milestone=opts.milestone)
> + data = self.template_data(db, opts)
> + data.update(dict(status=opts.status))
> + data.update(dict(workitems=workitems))
> + ...

> Hi James,
>
> I think this looks good but the new functions/methods would benefit from
> some docstrings.

I will add some, thanks.

> Why 3 update() calls instead of just one passing a dict combining
> everything?

No reason really, I'll change it.

> > + $(".workitems_in_status").tablesorter({
> > + sortList: [[1,1],[0,0],[2,0]],
>
> Maybe a comment explaining these magic numbers?

Good idea.

> To make this even nicer you could add something like a
> valid_states_with_labels to report_tools (by zip()ping a list containing
> the lables with the existing valid_states there). :)

I'll do that. I need to see if we can import directly, because
passing it in the data dict would be a bit of a hassle.

Thanks,

James