Merge lp:~mabac/launchpad-work-items-tracker/linaro-roadmap-connect-hacking into lp:~linaro-automation/launchpad-work-items-tracker/linaro

Proposed by Mattias Backman
Status: Merged
Merged at revision: 307
Proposed branch: lp:~mabac/launchpad-work-items-tracker/linaro-roadmap-connect-hacking
Merge into: lp:~linaro-automation/launchpad-work-items-tracker/linaro
Diff against target: 473 lines (+188/-81)
8 files modified
collect_roadmap (+30/-11)
generate-all (+7/-3)
html-report (+15/-10)
lpworkitems/database.py (+7/-1)
lpworkitems/models_roadmap.py (+2/-0)
report_tools.py (+88/-2)
templates/roadmap_card.html (+20/-39)
templates/roadmap_lane.html (+19/-15)
To merge this branch: bzr merge lp:~mabac/launchpad-work-items-tracker/linaro-roadmap-connect-hacking
Reviewer Review Type Date Requested Status
Guilherme Salgado (community) Approve
Review via email: mp+82898@code.launchpad.net

Description of the change

Hi,

This branch contains some changes back and forth and is a bit big. I hope it is reviewable though, if it's not I'll work on breaking it down in smaller chunks.

The main thing this adds is some plumbing for card health checks and a first set of checks.

It also changes the way information is displayed after feedback at the Connect. There is more to do, but I'd like to get this landed so we can start making smaller changes step by step.

Thanks,

Mattias

To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) wrote :
Download full text (9.0 KiB)

Hi Mattias,

This looks quite good. I have a few questions below but I don't think
I've ever touched this code so please forgive me if any of them are
stupid. :)

On Mon, 2011-11-21 at 15:23 +0000, Mattias Backman wrote:
[...]
>
> This branch contains some changes back and forth and is a bit big. I
> hope it is reviewable though, if it's not I'll work on breaking it
> down in smaller chunks.

It's not too big in fact, and although a very long diff is no fun, what
is less fun is when there are lots of unrelated changes bundled together
in a single branch. I hope that's not the case here! ;)

>
> The main thing this adds is some plumbing for card health checks and a
> first set of checks.
>
> It also changes the way information is displayed after feedback at the
> Connect. There is more to do, but I'd like to get this landed so we
> can start making smaller changes step by step.

> === modified file 'collect_roadmap'
> --- collect_roadmap 2011-10-27 13:23:14 +0000
> +++ collect_roadmap 2011-11-21 15:22:45 +0000
> @@ -53,7 +53,7 @@
> def kanban_import_lanes(collector, workflow_stages):
> nodes = {}
> root_node_id = None
> - lanes_to_ignore = ['Legend', 'Deferred']
> + lanes_to_ignore = ['Legend']
>
> # Iterate over all workflow_stages which may be in any order.
> for workflow_stage in workflow_stages:
> @@ -75,7 +75,7 @@
> assert node['parent_id'] == root_node_id
> model_lane = Lane(unicode_or_None(node['name']),
> node['id'])
> - if model_lane.name == '2011Q3':
> + if model_lane.name == '2011Q4':

Would it be worth moving this to a constant?

> model_lane.is_current = True
> else:
> model_lane.is_current = False
> @@ -116,21 +116,30 @@
> unicode_or_None(task['task']['external_id']))
> model_card.status = unicode_or_None(task_status['name'])
> model_card.team = unicode_or_None(card_type_name)
> - model_card.priority = u"%s" % task['task']['priority']
> + model_card.priority = lookup_kanban_priority(task['task']['priority'])
> model_card.size = unicode_or_None(task['task']['size_estimate'])
> model_card.sponsor = unicode_or_None(task['task']['custom_field_1'])
>
> external_link = task['task']['external_link']
> - if external_link is not None:
> + if external_link is not None and external_link is not '':
> + model_card.url = unicode_or_None(external_link)
> + dbg('Getting Papyrs information from %s.' % external_link)
> papyrs_data = papyrs_import(collector, external_link)
> model_card.description = unicode_or_None(papyrs_data['description'])
> model_card.acceptance_criteria = unicode_or_None(papyrs_data['acceptance_criteria'])
> -
> collector.store_card(model_card)
> else:
> dbg("Task '%s' does not have a status." % task['task']['name'])
>
>
> +def lookup_kanban_priority(numeric_priority):
> + priority...

Read more...

Revision history for this message
Mattias Backman (mabac) wrote :
Download full text (10.8 KiB)

On Mon, Nov 21, 2011 at 10:22 PM, Guilherme Salgado
<email address hidden> wrote:
> Hi Mattias,
>
> This looks quite good. I have a few questions below but I don't think
> I've ever touched this code so please forgive me if any of them are
> stupid. :)
>
> On Mon, 2011-11-21 at 15:23 +0000, Mattias Backman wrote:
> [...]
>>
>> This branch contains some changes back and forth and is a bit big. I
>> hope it is reviewable though, if it's not I'll work on breaking it
>> down in smaller chunks.
>
> It's not too big in fact, and although a very long diff is no fun, what
> is less fun is when there are lots of unrelated changes bundled together
> in a single branch. I hope that's not the case here! ;)

As you've seen it's a bit like that. :/

>
>>
>> The main thing this adds is some plumbing for card health checks and a
>> first set of checks.
>>
>> It also changes the way information is displayed after feedback at the
>> Connect. There is more to do, but I'd like to get this landed so we
>> can start making smaller changes step by step.
>
>> === modified file 'collect_roadmap'
>> --- collect_roadmap   2011-10-27 13:23:14 +0000
>> +++ collect_roadmap   2011-11-21 15:22:45 +0000
>> @@ -53,7 +53,7 @@
>>  def kanban_import_lanes(collector, workflow_stages):
>>      nodes = {}
>>      root_node_id = None
>> -    lanes_to_ignore = ['Legend', 'Deferred']
>> +    lanes_to_ignore = ['Legend']
>>
>>      # Iterate over all workflow_stages which may be in any order.
>>      for workflow_stage in workflow_stages:
>> @@ -75,7 +75,7 @@
>>          assert node['parent_id'] == root_node_id
>>          model_lane = Lane(unicode_or_None(node['name']),
>>                            node['id'])
>> -        if model_lane.name == '2011Q3':
>> +        if model_lane.name == '2011Q4':
>
> Would it be worth moving this to a constant?

We should actually move it to an external config of some sort. That
reminds me to talk to Joey about if this can be a flag on the lanes in
Kanbantool. I'll get back to you on that one.

>
>>              model_lane.is_current = True
>>          else:
>>              model_lane.is_current = False
>> @@ -116,21 +116,30 @@
>>                                    unicode_or_None(task['task']['external_id']))
>>                  model_card.status = unicode_or_None(task_status['name'])
>>                  model_card.team = unicode_or_None(card_type_name)
>> -                model_card.priority = u"%s" % task['task']['priority']
>> +                model_card.priority = lookup_kanban_priority(task['task']['priority'])
>>                  model_card.size = unicode_or_None(task['task']['size_estimate'])
>>                  model_card.sponsor = unicode_or_None(task['task']['custom_field_1'])
>>
>>                  external_link = task['task']['external_link']
>> -                if external_link is not None:
>> +                if external_link is not None and external_link is not '':
>> +                    model_card.url = unicode_or_None(external_link)
>> +                    dbg('Getting Papyrs information from %s.' % external_link)
>>                      papyrs_data = papyrs_import(collector, external_link)
>>                      model_car...

Revision history for this message
Guilherme Salgado (salgado) wrote :
Download full text (8.7 KiB)

On Tue, 2011-11-22 at 12:47 +0000, Mattias Backman wrote:
> On Mon, Nov 21, 2011 at 10:22 PM, Guilherme Salgado
> <email address hidden> wrote:
[...]
> > It's not too big in fact, and although a very long diff is no fun, what
> > is less fun is when there are lots of unrelated changes bundled together
> > in a single branch. I hope that's not the case here! ;)
>
> As you've seen it's a bit like that. :/

It was ok, actually. :)

> >>
> >> The main thing this adds is some plumbing for card health checks and a
> >> first set of checks.
> >>
> >> It also changes the way information is displayed after feedback at the
> >> Connect. There is more to do, but I'd like to get this landed so we
> >> can start making smaller changes step by step.
> >
> >> === modified file 'collect_roadmap'
> >> --- collect_roadmap 2011-10-27 13:23:14 +0000
> >> +++ collect_roadmap 2011-11-21 15:22:45 +0000
> >> @@ -53,7 +53,7 @@
> >> def kanban_import_lanes(collector, workflow_stages):
> >> nodes = {}
> >> root_node_id = None
> >> - lanes_to_ignore = ['Legend', 'Deferred']
> >> + lanes_to_ignore = ['Legend']
> >>
> >> # Iterate over all workflow_stages which may be in any order.
> >> for workflow_stage in workflow_stages:
> >> @@ -75,7 +75,7 @@
> >> assert node['parent_id'] == root_node_id
> >> model_lane = Lane(unicode_or_None(node['name']),
> >> node['id'])
> >> - if model_lane.name == '2011Q3':
> >> + if model_lane.name == '2011Q4':
> >
> > Would it be worth moving this to a constant?
>
> We should actually move it to an external config of some sort. That
> reminds me to talk to Joey about if this can be a flag on the lanes in
> Kanbantool. I'll get back to you on that one.

Ok, cool. If that's not possible then I'd be happy with a constant
defined at the top of this module.

> >> @@ -160,12 +169,22 @@
> >> last_heading = page_item['text']
> >> if page_item['classname'] == 'Paragraph':
> >> if not has_found_description:
> >> - description = page_item['text']
> >> + description = page_item['html']
> >
> > Why are we using the html instead of the plaintext version now?
>
> We don't want to loose the formatting that whoever edits the Papyrs
> document adds. It's also a practical thing since the text version is
> just the html stripped of markup. So lists end up as a sentence that
> do not make sense and paragraph breaks are lost (since they are <br>
> which are not translated to line breaks).

Oh, ok, that makes sense.

>
> >
> >> has_found_description = True
> >> - elif last_heading == 'Acceptance Criteria':
> >> - acceptance_criteria = page_item['text']
> >> -
> >> - return {'description': description, 'acceptance_criteria': acceptance_criteria}
> >> + elif 'Acceptance Criteria' in last_heading:
> >> + acceptance_criteria = page_item['html']
> >> +
> >> + return {'description': get_first_paragraph(description),
> >> + 'acceptance_criteria': get_first_paragraph(acceptance_criteria)}
> >> +
> >> +
> >> +def get_first_par...

Read more...

Revision history for this message
Mattias Backman (mabac) wrote :
Download full text (9.8 KiB)

> On Tue, 2011-11-22 at 12:47 +0000, Mattias Backman wrote:
> > On Mon, Nov 21, 2011 at 10:22 PM, Guilherme Salgado
> > <email address hidden> wrote:
> [...]
> > > It's not too big in fact, and although a very long diff is no fun, what
> > > is less fun is when there are lots of unrelated changes bundled together
> > > in a single branch. I hope that's not the case here! ;)
> >
> > As you've seen it's a bit like that. :/
>
> It was ok, actually. :)
>
> > >>
> > >> The main thing this adds is some plumbing for card health checks and a
> > >> first set of checks.
> > >>
> > >> It also changes the way information is displayed after feedback at the
> > >> Connect. There is more to do, but I'd like to get this landed so we
> > >> can start making smaller changes step by step.
> > >
> > >> === modified file 'collect_roadmap'
> > >> --- collect_roadmap 2011-10-27 13:23:14 +0000
> > >> +++ collect_roadmap 2011-11-21 15:22:45 +0000
> > >> @@ -53,7 +53,7 @@
> > >> def kanban_import_lanes(collector, workflow_stages):
> > >> nodes = {}
> > >> root_node_id = None
> > >> - lanes_to_ignore = ['Legend', 'Deferred']
> > >> + lanes_to_ignore = ['Legend']
> > >>
> > >> # Iterate over all workflow_stages which may be in any order.
> > >> for workflow_stage in workflow_stages:
> > >> @@ -75,7 +75,7 @@
> > >> assert node['parent_id'] == root_node_id
> > >> model_lane = Lane(unicode_or_None(node['name']),
> > >> node['id'])
> > >> - if model_lane.name == '2011Q3':
> > >> + if model_lane.name == '2011Q4':
> > >
> > > Would it be worth moving this to a constant?
> >
> > We should actually move it to an external config of some sort. That
> > reminds me to talk to Joey about if this can be a flag on the lanes in
> > Kanbantool. I'll get back to you on that one.
>
> Ok, cool. If that's not possible then I'd be happy with a constant
> defined at the top of this module.

Apparently there is no data for a Lane except for it's name. So I'll go with the constant and we can change it to an external config when we need to.

>
> > >> @@ -160,12 +169,22 @@
> > >> last_heading = page_item['text']
> > >> if page_item['classname'] == 'Paragraph':
> > >> if not has_found_description:
> > >> - description = page_item['text']
> > >> + description = page_item['html']
> > >
> > > Why are we using the html instead of the plaintext version now?
> >
> > We don't want to loose the formatting that whoever edits the Papyrs
> > document adds. It's also a practical thing since the text version is
> > just the html stripped of markup. So lists end up as a sentence that
> > do not make sense and paragraph breaks are lost (since they are <br>
> > which are not translated to line breaks).
>
> Oh, ok, that makes sense.
>
> >
> > >
> > >> has_found_description = True
> > >> - elif last_heading == 'Acceptance Criteria':
> > >> - acceptance_criteria = page_item['text']
> > >> -
> > >> - return {'description': description, 'acceptance_criteria':
> acceptance_criteria}
> > >> + elif...

Revision history for this message
Guilherme Salgado (salgado) wrote :
Download full text (3.9 KiB)

On Wed, 2011-11-23 at 08:46 +0000, Mattias Backman wrote:
[...]
> > > >
> > > >> has_found_description = True
> > > >> - elif last_heading == 'Acceptance Criteria':
> > > >> - acceptance_criteria = page_item['text']
> > > >> -
> > > >> - return {'description': description, 'acceptance_criteria':
> > acceptance_criteria}
> > > >> + elif 'Acceptance Criteria' in last_heading:
> > > >> + acceptance_criteria = page_item['html']
> > > >> +
> > > >> + return {'description': get_first_paragraph(description),
> > > >> + 'acceptance_criteria':
> > get_first_paragraph(acceptance_criteria)}
> > > >> +
> > > >> +
> > > >> +def get_first_paragraph(text):
> > > >> + if text is None:
> > > >> + return None
> > > >> + # This might break, depending on what type of line breaks
> > > >> + # whoever authors the Papyrs document uses.
> > > >> + first_pararaph, _, _ = text.partition('<br>')
> > > >
> > > > Would it be possible to use something like BeautifulSoup to make this
> > > > more robust?
> > >
> > > Possibly. I can have a look at what it can do. This seemed to work but
> > > now I see that someone has added BRs just before a list too which gets
> > > ugly.
> >
> > What does the HTML you're parsing look like? On, say
> > https://linaro-public.papyrs.com/public/4552/KWG2011-AMP-IPC/, I see the
> > actual content in a javascript function, so I'm assuming you're parsing
> > something else?
>
> Here's one example with a list: http://paste.ubuntu.com/746780/
> Sorry that it's all on one line. I get it from the json API, but it's
> not much better than scraping that javascript. Sometimes we get
> whitespace with a lot of formatting around it, just like when people
> edit html using MS Word.

Ok, apparently there's not much structure on the HTML; just some lists
and <br>s to force line breaks, so I think BeautifulSoup wouldn't help
much here.

> >
> > > >
> > > >> + return first_pararaph
> > > >>
> > > >>
> > > >> ########################################################################
> > > >
> > > >
> > > >> === modified file 'report_tools.py'
> > > >> --- report_tools.py 2011-10-24 15:08:02 +0000
> > > >> +++ report_tools.py 2011-11-21 15:22:45 +0000
[...]
> > > >
> > > > Having all the classes here make for a rather long function. Is there a
> > > > reason for keeping them here other than the fact that they're not used
> > > > anywhere else?
> > >
> > > I wasn't even sure that creating subclasses would be a good idea.
> > > Since i fear that we will see a lot more checks when PMs know what
> > > they want I guess these should go on a module instead.
> > >
> > > >
> > > >> +
> > > >> + health_checks = []
> > > >> + health_checks.append(RoadmapIdHealthCheck())
> > > >> + health_checks.append(DescriptionHealthCheck())
> > > >> + health_checks.append(CriteriaHealthCheck())
> > > >> + health_checks.append(BlueprintsHealthCheck())
> > > >> + health_checks.append(BlueprintsBlockedHealthCheck())
> > > >
> > > > One neat thing we could do is use a class decorator on all HealthCheck
> > > > classes to register them on a global registry (a...

Read more...

review: Approve
Revision history for this message
Mattias Backman (mabac) wrote :

I have fixed the pep8 and constant suggestions and used Salgado's
magic to move the health checks to a separate module.

Some items (mostly visual stuff from kiko) carry over and have been
registered in a new bp.

On Thu, Nov 24, 2011 at 11:45 AM, <email address hidden> wrote:
> The proposal to merge lp:~mabac/launchpad-work-items-tracker/linaro-roadmap-connect-hacking into lp:~linaro-infrastructure/launchpad-work-items-tracker/linaro has been updated.
>
>    Status: Needs review => Merged
>
> For more details, see:
> https://code.launchpad.net/~mabac/launchpad-work-items-tracker/linaro-roadmap-connect-hacking/+merge/82898
> --
> https://code.launchpad.net/~mabac/launchpad-work-items-tracker/linaro-roadmap-connect-hacking/+merge/82898
> You are the owner of lp:~mabac/launchpad-work-items-tracker/linaro-roadmap-connect-hacking.
>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'collect_roadmap'
2--- collect_roadmap 2011-10-27 13:23:14 +0000
3+++ collect_roadmap 2011-11-21 15:22:45 +0000
4@@ -53,7 +53,7 @@
5 def kanban_import_lanes(collector, workflow_stages):
6 nodes = {}
7 root_node_id = None
8- lanes_to_ignore = ['Legend', 'Deferred']
9+ lanes_to_ignore = ['Legend']
10
11 # Iterate over all workflow_stages which may be in any order.
12 for workflow_stage in workflow_stages:
13@@ -75,7 +75,7 @@
14 assert node['parent_id'] == root_node_id
15 model_lane = Lane(unicode_or_None(node['name']),
16 node['id'])
17- if model_lane.name == '2011Q3':
18+ if model_lane.name == '2011Q4':
19 model_lane.is_current = True
20 else:
21 model_lane.is_current = False
22@@ -116,21 +116,30 @@
23 unicode_or_None(task['task']['external_id']))
24 model_card.status = unicode_or_None(task_status['name'])
25 model_card.team = unicode_or_None(card_type_name)
26- model_card.priority = u"%s" % task['task']['priority']
27+ model_card.priority = lookup_kanban_priority(task['task']['priority'])
28 model_card.size = unicode_or_None(task['task']['size_estimate'])
29 model_card.sponsor = unicode_or_None(task['task']['custom_field_1'])
30
31 external_link = task['task']['external_link']
32- if external_link is not None:
33+ if external_link is not None and external_link is not '':
34+ model_card.url = unicode_or_None(external_link)
35+ dbg('Getting Papyrs information from %s.' % external_link)
36 papyrs_data = papyrs_import(collector, external_link)
37 model_card.description = unicode_or_None(papyrs_data['description'])
38 model_card.acceptance_criteria = unicode_or_None(papyrs_data['acceptance_criteria'])
39-
40 collector.store_card(model_card)
41 else:
42 dbg("Task '%s' does not have a status." % task['task']['name'])
43
44
45+def lookup_kanban_priority(numeric_priority):
46+ priority_lookup = { -1 : "low",
47+ 0 : "normal",
48+ 1 : "high" }
49+ assert numeric_priority in priority_lookup, (
50+ "Priority '%s' is unknown." % numeric_priority)
51+ return unicode_or_None(priority_lookup[numeric_priority])
52+
53 def kanban_import(collector, cfg, board_id, api_token):
54 '''Collect roadmap items from KanbanTool into DB.'''
55 board_url = get_kanban_url('boards/%s.json' % board_id, api_token)
56@@ -150,7 +159,7 @@
57 acceptance_criteria = None
58
59 page = get_json_data(requirement_url + '?json&auth_token=e8a2801e4f0f')
60-
61+ assert page is not None, "Could not access Papyrs document %s." % requirement_url
62 page_text_items = page[0]
63 page_extra_items = page[1]
64
65@@ -160,12 +169,22 @@
66 last_heading = page_item['text']
67 if page_item['classname'] == 'Paragraph':
68 if not has_found_description:
69- description = page_item['text']
70+ description = page_item['html']
71 has_found_description = True
72- elif last_heading == 'Acceptance Criteria':
73- acceptance_criteria = page_item['text']
74-
75- return {'description': description, 'acceptance_criteria': acceptance_criteria}
76+ elif 'Acceptance Criteria' in last_heading:
77+ acceptance_criteria = page_item['html']
78+
79+ return {'description': get_first_paragraph(description),
80+ 'acceptance_criteria': get_first_paragraph(acceptance_criteria)}
81+
82+
83+def get_first_paragraph(text):
84+ if text is None:
85+ return None
86+ # This might break, depending on what type of line breaks
87+ # whoever authors the Papyrs document uses.
88+ first_pararaph, _, _ = text.partition('<br>')
89+ return first_pararaph
90
91
92 ########################################################################
93
94=== modified file 'generate-all'
95--- generate-all 2011-10-18 11:34:41 +0000
96+++ generate-all 2011-11-21 15:22:45 +0000
97@@ -159,9 +159,13 @@
98
99 # roadmap cards
100 for card in report_tools.cards(store):
101- basename = os.path.join(opts.output_dir, 'roadmap-card-%s' % \
102- card.roadmap_id)
103- report_tools.roadmap_cards(my_path, opts.database, basename, opts.config, card, root=opts.root)
104+ if card.roadmap_id != '':
105+ page_name = card.roadmap_id
106+ else:
107+ page_name = card.card_id
108+ basename = os.path.join(opts.output_dir, 'roadmap-card-%s' % page_name)
109+ report_tools.roadmap_cards(my_path, opts.database, basename, opts.config,
110+ card, root=opts.root)
111
112 def copy_files(source_dir):
113 for filename in os.listdir(source_dir):
114
115=== modified file 'html-report'
116--- html-report 2011-10-27 12:20:51 +0000
117+++ html-report 2011-11-21 15:22:45 +0000
118@@ -474,20 +474,16 @@
119 title = opts.title
120
121 data = self.template_data(store, opts)
122- lane = report_tools.lane(store, opts.lane).one()
123+ lane = report_tools.lane(store, opts.lane)
124 lanes = report_tools.lanes(store)
125 statuses = []
126 for status, cards in report_tools.statuses(store, lane):
127- cards_with_bp_counts = []
128+ cards_with_bps = []
129 for card in cards:
130+ report_tools.check_card_health(store, card)
131 blueprints = report_tools.card_blueprints_by_status(store, card.roadmap_id)
132- count_string = ''
133- for bp_status in blueprints.keys():
134- bp_count = len(blueprints[bp_status])
135- if bp_count > 0:
136- count_string += '%s: %d ' % (bp_status, bp_count)
137- cards_with_bp_counts.append({'card': card, 'count': count_string})
138- statuses.append(dict(name=status, cards=cards_with_bp_counts))
139+ cards_with_bps.append({'card': card, 'blueprints': blueprints})
140+ statuses.append(dict(name=status, cards=cards_with_bps))
141
142 data.update(dict(statuses=statuses))
143 data.update(dict(page_type="roadmap_lane"))
144@@ -503,7 +499,8 @@
145
146 data = self.template_data(store, opts)
147 card = report_tools.card(store, int(opts.card)).one()
148- lane = report_tools.lane(store, None, id=card.lane_id).one()
149+ health_checks = report_tools.check_card_health(store, card)
150+ lane = report_tools.lane(store, None, id=card.lane_id)
151
152 if not opts.title:
153 title = card.name
154@@ -511,12 +508,20 @@
155 title = opts.title
156
157 blueprints = report_tools.card_blueprints_by_status(store, card.roadmap_id)
158+ blueprint_status_count = WorkitemTarget()
159+ blueprint_status_count.todo = len(blueprints['Planned'])
160+ blueprint_status_count.blocked = len(blueprints['Blocked'])
161+ blueprint_status_count.inprogress = len(blueprints['In Progress'])
162+ blueprint_status_count.postponed = 0
163+ blueprint_status_count.done = len(blueprints['Completed'])
164
165 data.update(dict(page_type="roadmap_card"))
166 data.update(dict(card_title=title))
167 data.update(dict(card=card))
168+ data.update(dict(health_checks=health_checks))
169 data.update(dict(lane=lane.name))
170 data.update(dict(blueprints=blueprints))
171+ data.update(dict(blueprint_status_count=blueprint_status_count))
172
173 print report_tools.fill_template(
174 "roadmap_card.html", data, theme=opts.theme)
175
176=== modified file 'lpworkitems/database.py'
177--- lpworkitems/database.py 2011-10-13 15:31:42 +0000
178+++ lpworkitems/database.py 2011-11-21 15:22:45 +0000
179@@ -7,7 +7,7 @@
180 store.execute('''CREATE TABLE version (
181 db_layout_ref INT NOT NULL
182 )''')
183- store.execute('''INSERT INTO version VALUES (13)''')
184+ store.execute('''INSERT INTO version VALUES (14)''')
185
186 store.execute('''CREATE TABLE specs (
187 name VARCHAR(255) PRIMARY KEY,
188@@ -100,6 +100,8 @@
189 store.execute('''CREATE TABLE card (
190 name VARCHAR(200) NOT NULL,
191 card_id NOT NULL,
192+ url VARCHAR(200),
193+ is_healthy BOOLEAN,
194 status VARCHAR(50),
195 team VARCHAR(50),
196 priority VARCHAR(50),
197@@ -228,6 +230,10 @@
198 store.execute('ALTER TABLE card ADD COLUMN acceptance_criteria BLOB')
199 store.execute('ALTER TABLE lane ADD COLUMN is_current BOOLEAN')
200 store.execute('UPDATE version SET db_layout_ref = 13')
201+ if ver == 13:
202+ store.execute('ALTER TABLE card ADD COLUMN url VARCHAR(200)')
203+ store.execute('ALTER TABLE card ADD COLUMN is_healthy BOOLEAN')
204+ store.execute('UPDATE version SET db_layout_ref = 14')
205
206
207 def get_store(dbpath):
208
209=== modified file 'lpworkitems/models_roadmap.py'
210--- lpworkitems/models_roadmap.py 2011-10-13 15:31:42 +0000
211+++ lpworkitems/models_roadmap.py 2011-11-21 15:22:45 +0000
212@@ -21,6 +21,8 @@
213 contact = Unicode()
214 description = Unicode()
215 acceptance_criteria = Unicode()
216+ url = Unicode()
217+ is_healthy = Bool()
218
219 def __init__(self, name, card_id, lane_id, roadmap_id):
220 self.lane_id = lane_id
221
222=== modified file 'report_tools.py'
223--- report_tools.py 2011-10-24 15:08:02 +0000
224+++ report_tools.py 2011-11-21 15:22:45 +0000
225@@ -942,14 +942,15 @@
226
227 def lane(store, name, id=None):
228 if id is None:
229- return store.find(Lane, Lane.name == unicode(name))
230+ return store.find(Lane, Lane.name == unicode(name)).one()
231 else:
232- return store.find(Lane, Lane.lane_id == id)
233+ return store.find(Lane, Lane.lane_id == id).one()
234
235
236 def lane_cards(store, lane):
237 return lane.cards
238
239+
240 def statuses(store, lane):
241 result = []
242 for status in store.find(Card.status,
243@@ -995,6 +996,91 @@
244 return bp_by_status
245
246
247+def check_card_health(store, card):
248+ class HealthCheck(object):
249+ NOT_APPLICABLE = 'n/a'
250+ OK = 'OK'
251+ NOT_OK = 'Not OK'
252+ name = 'Base check, not to be used'
253+
254+ def applicable(self, card, store=None):
255+ raise NotImplementedError()
256+
257+ def check(self, card, store=None):
258+ raise NotImplementedError()
259+
260+ def execute(self, card, store=None):
261+ if self.applicable(card, store):
262+ if self.check(card, store):
263+ return self.OK
264+ else:
265+ return self.NOT_OK
266+ else:
267+ return self.NOT_APPLICABLE
268+
269+ class DescriptionHealthCheck(HealthCheck):
270+ name = 'Has description'
271+
272+ def applicable(self, card, store=None):
273+ return True
274+
275+ def check(self, card, store=None):
276+ return card.description is not None
277+
278+ class CriteriaHealthCheck(HealthCheck):
279+ name = 'Has acceptance criteria'
280+
281+ def applicable(self, card, store=None):
282+ return True
283+
284+ def check(self, card, store=None):
285+ return card.acceptance_criteria is not None
286+
287+ class BlueprintsHealthCheck(HealthCheck):
288+ name = 'Has blueprints'
289+
290+ def applicable(self, card, store):
291+ return card.status == 'Ready'
292+
293+ def check(self, card, store):
294+ return len(card_blueprints(store, card.roadmap_id)) > 0
295+
296+ class BlueprintsBlockedHealthCheck(HealthCheck):
297+ name = 'Has no Blocked blueprints'
298+
299+ def applicable(self, card, store):
300+ return card.status != 'Ready'
301+
302+ def check(self, card, store):
303+ blueprints = card_blueprints_by_status(store, card.roadmap_id)
304+ return len(blueprints['Blocked']) == 0
305+
306+ class RoadmapIdHealthCheck(HealthCheck):
307+ name = 'Has a roadmap id'
308+
309+ def applicable(self, card, store=None):
310+ return True
311+
312+ def check(self, card, store=None):
313+ return card.roadmap_id != ''
314+
315+ health_checks = []
316+ health_checks.append(RoadmapIdHealthCheck())
317+ health_checks.append(DescriptionHealthCheck())
318+ health_checks.append(CriteriaHealthCheck())
319+ health_checks.append(BlueprintsHealthCheck())
320+ health_checks.append(BlueprintsBlockedHealthCheck())
321+
322+ performed_checks = []
323+ card.is_healthy = True
324+ for check in health_checks:
325+ result = check.execute(card, store)
326+ if result == check.NOT_OK:
327+ card.is_healthy = False
328+ performed_checks.append({'name': check.name,
329+ 'result': result})
330+ return performed_checks
331+
332 def subteams(store, team):
333 result = store.execute('SELECT name from team_structure where team = ?', (unicode(team),))
334 return [i[0] for i in result]
335
336=== modified file 'templates/roadmap_card.html'
337--- templates/roadmap_card.html 2011-10-31 16:12:14 +0000
338+++ templates/roadmap_card.html 2011-11-21 15:22:45 +0000
339@@ -14,8 +14,6 @@
340 <h2>${card.status} in <a href="roadmap-${lane}.html">${lane}</a></h2>
341 <p>
342 <ul>
343- <li>Description: ${card.description[:250]}
344- <li>Acceptance criteria: ${card.acceptance_criteria[:250]}
345 <li>Sponsor: ${card.sponsor}
346 <li>Contact: ${card.contact}
347 <li>Priority: ${card.priority}
348@@ -23,12 +21,15 @@
349 <li>Team: ${card.team}
350 </ul>
351
352+<div style="text-align: center">Overall blueprint completion</div>
353+${util.progress_bar(blueprint_status_count)}
354+
355+<h3>Description</h3> ${card.description if card.description is not None else '<i>No description could be found.</i>'}
356+<p><a href="${card.url}">Read the full description</a>.
357+<h3>Acceptance criteria</h3> ${card.acceptance_criteria if card.acceptance_criteria is not None else '<i>No acceptance criteria could be found.</i>'}
358+<p><a href="${card.url}">Read the full acceptance criteria</a>.
359 <p>
360-% for status in blueprints.keys():
361-% if blueprints[status] == []:
362-<p><i>There are no ${status} blueprints linked to this card.</i>
363-% else:
364-<h3>${status}</h3>
365+% if card.status in ['Ready', 'Delivered'] or blueprints.keys() > 0:
366 <table>
367 <thead>
368 <tr><th>Title</th>
369@@ -39,47 +40,27 @@
370 <th>Health check</th>
371 </tr>
372 </thead>
373+% for status in blueprints.keys():
374 % for bp in blueprints[status]:
375 <tr><td><a href="${bp.url}">${bp.name}</a></td>
376 <td>${bp.assignee_name}</td>
377 <td>${bp.priority}</td>
378- <td>${bp.implementation}</td>
379+ <td>${status}
380 <td>${bp.milestone_name}</td>
381- <td>xxx</td>
382+ <td></td>
383 </tr>
384 % endfor
385+% endfor
386 </table>
387 % endif
388-% endfor
389
390 <h3>Health check</h3>
391-<table>
392- <tr><td>Has Description</td>
393- <td>
394-% if description == "":
395- No
396-% else:
397- Yes
398-% endif
399- </td></tr>
400- <tr><td>Has Acceptance Criteria</td>
401- <td>
402-% if acceptance_criteria == "":
403- No
404-% else:
405- Yes
406-% endif
407- </td></tr>
408- <tr><td>Has Blueprints</td>
409- <td>
410-% if blueprints == []:
411- No
412-% else:
413- Yes
414-% endif
415- </td></tr>
416- <tr><td>Is Ready</td>
417- <td>
418- TODO
419- </td></tr>
420+
421+<table>${'<tr colspan="2"><td><font color="#FF0000"><b>Needs attention!</b></font></td></tr>' if not card.is_healthy else ''}
422+ % for result in health_checks:
423+ <tr ${'bgcolor="#FFAAAA"' if result['result'] == 'Not OK' else 'bgcolor="#FFFFFF"'}>
424+ <td>${result['name']}</td>
425+ <td>${result['result']}</td>
426+</tr>
427+% endfor
428 </table>
429
430=== modified file 'templates/roadmap_lane.html'
431--- templates/roadmap_lane.html 2011-10-31 16:12:14 +0000
432+++ templates/roadmap_lane.html 2011-11-21 15:22:45 +0000
433@@ -19,22 +19,26 @@
434
435 <h1>${title()}</h1>
436 <p>
437+<table>
438+<thead><tr><th>Card</th><th>Status</th><th>Team</th><th>Priority</th><th>Blueprints</th><th>Health</th></tr></thead>
439 % for status in statuses:
440-<p><b>${status['name']}</b>
441 % for card_dict in status['cards']:
442-<table width="45%">
443-<thead>
444- <tr><th>
445- <a href="roadmap-card-${card_dict['card'].roadmap_id}.html">${card_dict['card'].name}</a>
446- </th></tr>
447-</thead>
448- <tr><td colspan=2>${card_dict['card'].description[:200]}</td></tr>
449- <tr><td>
450- ${card_dict['count']}
451- </td><td>&nbsp</td></tr>
452- <tr><td>${card_dict['card'].team}</td><td align=right>Priority ${card_dict['card'].priority}</td></tr>
453+ <tr>
454+ <td>
455+ <a href="roadmap-card-${card_dict['card'].roadmap_id if card_dict['card'].roadmap_id != '' else card_dict['card'].card_id}.html">${card_dict['card'].name}</a>
456+ </td>
457+ <td>${status['name']}</td>
458+ <!--<td>${card_dict['card'].description} <a href="roadmap-card-${card_dict['card'].roadmap_id}.html">...</a></td> -->
459+ <td>${card_dict['card'].team}</td><td align=right>${card_dict['card'].priority}</td>
460+ <td>
461+% for bp_status in card_dict['blueprints'].keys():
462+ ${'%s: %d ' % (bp_status, len(card_dict['blueprints'][bp_status])) if len(card_dict['blueprints'][bp_status]) > 0 else ''}
463+% endfor
464+ </td>
465+ <td>
466+ ${'<font color="#FF0000">Needs attention!</font>' if not card_dict['card'].is_healthy else ''}
467+ </td>
468+% endfor
469+% endfor
470 </table>
471-<p><p>
472-% endfor
473-% endfor
474

Subscribers

People subscribed via source and target branches