Merge lp:~mabac/launchpad-work-items-tracker/support-deferred-cards into lp:~linaro-automation/launchpad-work-items-tracker/linaro

[Mattias Backman]

Hi,

This branch adds support for collecting the Deferred lane.

It does this by accepting that there are Cards without status (sub lanes) and only collects cards for Lanes which have been collected. The last part is because of the Legend lane which also have no sub lanes, but we actually ignore that lane before cards are collected.

Thanks,

Mattias

[Данило Шеган (danilo)]

Looks good to me: to confirm my understanding, this basically only replaces use of 'task_status is None' when checking if a card needs to be collected by using either task_status['parent_id'] or task['task']['workflow_stage_id'] to check if a lane by that ID is collected instead using the new method lane_is_collected. All the rest is reindentation, afaics, so good to go.

(and oh my, this code does miss tests :)

review: Approve

[Mattias Backman (mabac)]

On Wed, Dec 21, 2011 at 8:57 AM, Данило Шеган <email address hidden> wrote:
> Review: Approve
>
> Looks good to me: to confirm my understanding, this basically only replaces use of 'task_status is None' when checking if a card needs to be collected by using either task_status['parent_id'] or task['task']['workflow_stage_id'] to check if a lane by that ID is collected instead using the new method lane_is_collected. All the rest is reindentation, afaics, so good to go.

Yes. The previous check made sure that we only collected cards that
where in a sub-lane below a lane. This means that they are in a
"quarter" lane and has a status so we skipped the Legend and Deferred
cards. Since we're supposed to display the Deferred lane, now we just
skip cards that belong to a lane we explicitly ignore.

>
> (and oh my, this code does miss tests :)

Probably is a good time to add those and do TDD from this point on. :)

> --
> https://code.launchpad.net/~mabac/launchpad-work-items-tracker/support-deferred-cards/+merge/86405
> You are the owner of lp:~mabac/launchpad-work-items-tracker/support-deferred-cards.

=== modified file 'collect_roadmap'
--- collect_roadmap	2011-12-15 18:45:54 +0000
+++ collect_roadmap	2011-12-20 14:18:03 +0000
@@ -97,6 +97,7 @@
 def kanban_import_cards(collector, tasks, status_list, card_types):
     types_to_ignore = ['Summits']
     for task in tasks:
+        dbg("Collecting card '%s'." % (task['task']['name']))
         status_id = task['task']['workflow_stage_id']
         assert status_id is not None
         task_status = None
@@ -104,44 +105,49 @@
             if status['id'] == status_id:
                 task_status = status
                 break
-        if task_status is not None:
-            card_type_id = task['task']['card_type_id']
-            card_type_name = None
-            for card_type in card_types:
-                if card_type['id'] == card_type_id:
-                    card_type_name = card_type['name']
-                    break
-            else:
-                dbg("Cannot find type for card '%s'." % (task['task']['name']))
-            if card_type_name in types_to_ignore:
-                dbg("Ignoring card '%s' since it\'s type is '%s'." %  \
-                        (task['task']['name'], card_type['name']))
-            else:
+        card_type_id = task['task']['card_type_id']
+        card_type_name = None
+        for card_type in card_types:
+            if card_type['id'] == card_type_id:
+                card_type_name = card_type['name']
+                break
+        else:
+            dbg("Cannot find type for card '%s'." % (task['task']['name']))
+        if card_type_name in types_to_ignore:
+            dbg("Ignoring card '%s' since it\'s type is '%s'." %  \
+                    (task['task']['name'], card_type['name']))
+        else:
+            if task_status is not None:
                 lane_id = task_status['parent_id']
                 assert lane_id is not None
-                model_card = Card(get_json_item(task['task'], 'name'),
-                                  task['task']['id'], lane_id,
-                                  get_json_item(task['task'], 'external_id'))
+            else:
+                lane_id = status_id
+            if not collector.lane_is_collected(lane_id):
+                dbg("Ignoring card '%s' since it\'s Lane is ignored." %  \
+                        (task['task']['name']))
+                continue
+            model_card = Card(get_json_item(task['task'], 'name'),
+                              task['task']['id'], lane_id,
+                              get_json_item(task['task'], 'external_id'))
+            if task_status is not None:
                 model_card.status = get_json_item(task_status, 'name')
-                model_card.team = unicode_or_None(card_type_name)
-                model_card.priority = lookup_kanban_priority(
-                    task['task']['priority'])
-                model_card.size = get_json_item(task['task'], 'size_estimate')
-                model_card.sponsor = get_json_item(task['task'],
-                                                   'custom_field_1')
+            model_card.team = unicode_or_None(card_type_name)
+            model_card.priority = lookup_kanban_priority(
+                task['task']['priority'])
+            model_card.size = get_json_item(task['task'], 'size_estimate')
+            model_card.sponsor = get_json_item(task['task'],
+                                               'custom_field_1')
 
-                external_link = task['task']['external_link']
-                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 = get_json_item(papyrs_data,
-                                                           'description')
-                    model_card.acceptance_criteria = get_json_item(
-                        papyrs_data, 'acceptance_criteria')
-                collector.store_card(model_card)
-        else:
-            dbg("Task '%s' does not have a status." % task['task']['name'])
+            external_link = task['task']['external_link']
+            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 = get_json_item(papyrs_data,
+                                                       'description')
+                model_card.acceptance_criteria = get_json_item(
+                    papyrs_data, 'acceptance_criteria')
+            collector.store_card(model_card)
 
 
 def kanban_import(collector, cfg, board_id, api_token):

=== modified file 'lpworkitems/collect_roadmap.py'
--- lpworkitems/collect_roadmap.py	2011-12-15 18:36:07 +0000
+++ lpworkitems/collect_roadmap.py	2011-12-20 14:18:03 +0000
@@ -50,6 +50,10 @@
             obj.count = count
             self.store.add(obj)
 
+    def lane_is_collected(self, lane_id):
+        return self.store.find(models_roadmap.Lane, models_roadmap.
+                               Lane.lane_id == lane_id).one() is not None
+
 
 def get_json_item(data, item_name):
     item = data[item_name]