Merge lp:~bjornt/lp2kanban/bugs-to-cards into lp:~launchpad/lp2kanban/trunk

[Björn Tillenius]

Support creating new cards from Launchpad bugs.

The workflow is:

  1) Tag the bug you want to create a card for with the
     configured tag.
  2) bugs2cards.py gets all bugs with the configured tag
     and creates new cards from them, if there aren't already
     a card linking to that bug.
  3) bugs2cards.py removes the configured tag from the bug, in
     order to make the next runs faster.

I'm not completely happy with 3), since it adds noise to the bug and
requires authenticated use of the API, but it's needed, since otherwise
the list of bugs to process would just grow and grow.

I'm also not too happy of the name of the config options, but I couldn't
come up with anything better. I'm open to suggestions.

Something that I would like to do in the future is to have the type of the
card be determined by what tags the bug has, but for now it always creates
card of the same type.

[Gary Poster (gary)]

Hi. This sounds nice. I agree that #3 is not ideal but I can't think of another approach yet. I've asked someone in yellow squad to give you a review soon. I'd love to have this landed.

Thank you!

Gary

[lp:~bjornt/lp2kanban/bugs-to-cards]

70. By Björn Tillenius on 2012-11-28

Pass in the project, not the project group.

[Benji York (benji)]

The branch looks good. I didn't see anything that should prevent this
from landing but I had several comments/suggestions while reading the
code.

> I'm not completely happy with 3), since it adds noise to the bug and
> requires authenticated use of the API, but it's needed, since
> otherwise the list of bugs to process would just grow and grow.

An idea how to tag cards as synced with kanban without having to remove
the tags later: Tag bugs that should be synced, the script then queries
for non-closed, tagged bugs and compares against existing cards,
creating those that do not yet exist.

We will be processing a few more bugs than is strictly necessary, but
once a bug is closed it will fall out of the set of bugs being
inspected, so the number of bugs examined will be bounded.

This approach also means that "semantic" tags can be used to manage
cards. For example, if you have a project to implement the "gizmo"
feature and all the "gizmo" bugs are already tagged, you can just start
running lp2kanban with the new tag name and all the bugs will have cards
created and the tags won't be removed (as they would now).

What do you think about that?

Config option brainstorming
---------------------------

bug_new_card_tag:

- card_creation_bug_tag
- new_card_bug_tag
- bug_to_card_tag

bug_new_card_type:

- bug_to_card_type

I like the last two suggestions in each category because they share a
prefix and "bug_to_card" is relatively obvious in meaning.

Is my reading correct in that create_new_cards creates all the bugs in
the left-most lane? It would be nice for that to be configurable, many
boards aren't structured that way. In fact, I can see people running
multiple passes; creating cards from one tag in one lane and cards from
another tag in another.

Hmm, maybe that doesn't matter because the cards are then synced after
they are created, thus moving them into the correct lanes. If that is
correct, then a comment to that effect in create_new_cards would be
helpful.

Shouldn't the newly created cards have the "Synced with Launchpad"
marker added to the description? Wait, since the cards are "dirty",
that will be done by save_board, right?

The tests are appreciated.

We could use some tests for create_new_cards too. I would include tests
for the lane lookup code (possibly breaking it out into its own
function) and for the card type name to card type ID translation
function (it would probably benefit from being its own function too).

review: Approve (code)

[Björn Tillenius (bjornt)]

On Wed, Nov 28, 2012 at 02:39:57PM -0000, Benji York wrote:
> Review: Approve code
>
> The branch looks good. I didn't see anything that should prevent this
> from landing but I had several comments/suggestions while reading the
> code.
>
> > I'm not completely happy with 3), since it adds noise to the bug and
> > requires authenticated use of the API, but it's needed, since
> > otherwise the list of bugs to process would just grow and grow.
>
> An idea how to tag cards as synced with kanban without having to remove
> the tags later: Tag bugs that should be synced, the script then queries
> for non-closed, tagged bugs and compares against existing cards,
> creating those that do not yet exist.
>
> We will be processing a few more bugs than is strictly necessary, but
> once a bug is closed it will fall out of the set of bugs being
> inspected, so the number of bugs examined will be bounded.
>
> This approach also means that "semantic" tags can be used to manage
> cards. For example, if you have a project to implement the "gizmo"
> feature and all the "gizmo" bugs are already tagged, you can just start
> running lp2kanban with the new tag name and all the bugs will have cards
> created and the tags won't be removed (as they would now).
>
> What do you think about that?

Hmm. Maybe. I'm going to try it and do some timings. I currently run the
sync script to create new bugs once every minute, and would like to keep
doing so.

It also raises the question, how to delete cards from the board.
Although, I guess I could make it a config option whether to remove the
tag...

> Config option brainstorming
> ---------------------------
>
> bug_new_card_tag:
>
> - card_creation_bug_tag
> - new_card_bug_tag
> - bug_to_card_tag
>
> bug_new_card_type:
>
> - bug_to_card_type
>
>
> I like the last two suggestions in each category because they share a
> prefix and "bug_to_card" is relatively obvious in meaning.

Yeah, those are good suggestion. Let's see, I will postpone merging this
branch a while, to see whether I have time to implement some other
features I want. One would be to have a default card type for new bugs,
but then being able to map specific tags to specific types. For example,
the default could be 'Feature', but if a bug is marked with a 'defect'
tag, the card type will be 'Bug'.

> Is my reading correct in that create_new_cards creates all the bugs in
> the left-most lane? It would be nice for that to be configurable, many
> boards aren't structured that way. In fact, I can see people running
> multiple passes; creating cards from one tag in one lane and cards from
> another tag in another.

Ah, that is correct, that's something I forgot to address before
proposing the merge. I'll ponder on how to do that in a nice way.

> Hmm, maybe that doesn't matter because the cards are then synced after
> they are created, thus moving them into the correct lanes. If that is
> correct, then a comment to that effect in create_new_cards would be
> helpful.

No, I don't think that will work. As far I as can see, the logic for
moving lanes are highly specific to a certain board layout, and it will
move cards only within the same horizontal lane...

[Gary Poster (gary)]

FWIW, I encourage landing something soon and making a separate branch for as many of the new features as you think you can postpone reasonably. There is some more work coming soon on lp2kanban AIUI from Graham's team.

[lp:~bjornt/lp2kanban/bugs-to-cards]

71. By Björn Tillenius on 2012-12-07

Rename config options.

[Björn Tillenius (bjornt)]

Ok, let's get this branch landed then?

I've renamed the config options, since that's easy to do.

I will soon propose a branch that enables card moves to work for Landscape's
board layout, and after that I will add another branch for choosing which
lane to create new cards in.

I will also add more tests in later branches as well.

I don't have permission to commit to lp:lp2kanban, so please someone either
land the branch for me, or give me permission.

=== modified file 'examples/config.ini'
--- examples/config.ini	2012-09-03 13:35:17 +0000
+++ examples/config.ini	2012-12-07 09:40:24 +0000
@@ -13,6 +13,17 @@
 # for cards you don't want synced).
 autosync = off
 
+# Bug tag for which new cards should be created. If a bug in Launchpad
+# has this tag, a new card will be created for it in the first kanban lane.
+# If this config option isn't set, no cards will be created.
+# After the card has been created, the tag will be removed from the bug, so a
+# credentials_file has to be specified as well, to authenticate against LP.
+#bug_to_card_tag = kanban
+
+# Which card type should cards created from bugs have. If not set, the default
+# card type is used.
+#bug_to_card_type = Defect
+
 # Lanes are assigned a "role" by looking for sublanes matching
 # values given here.  "Coding" will match all lanes which have
 # a title of "Coding", and "QA::Ready" will match all lanes

=== modified file 'src/lp2kanban/bugs2cards.py'
--- src/lp2kanban/bugs2cards.py	2012-09-27 13:06:03 +0000
+++ src/lp2kanban/bugs2cards.py	2012-12-07 09:40:24 +0000
@@ -213,6 +213,15 @@
     CardStatus.DONE: 'done_lanes',
     }
 
+def get_new_bugs(board, bugs):
+    """Return the bugs that aren't already in the board."""
+    existing_bugs = set(
+        int(card.external_card_id) for card in board.cards
+        if card.external_card_id)
+    for bug in bugs:
+        if bug.id not in existing_bugs:
+            yield bug
+
 
 def get_card_status(bug_status, bug_tags, branch_info):
     """Return the status of the card as one of CardStatus values."""
@@ -277,6 +286,34 @@
                 return target_lane
         return None
 
+def create_new_cards(board, launchpad_project, bug_tag, cardtype_name=None):
+    """Create cards for bugs tagged in Launchpad.
+
+    If a bug is tagged with the given tag, a new card will be created in
+    the board and the tag will be removed from the bug.
+    """
+    new_cardtype = board.default_cardtype.id
+    if cardtype_name is not None:
+        for type_id, cardtype in board.cardtypes.items():
+            if cardtype.name == cardtype_name:
+                new_cardtype = type_id
+    kanban_bugtasks = launchpad_project.searchTasks(tags=[bug_tag])
+    kanban_bugs = get_new_bugs(
+        board, [bugtask.bug for bugtask in kanban_bugtasks])
+    next_lane = board.root_lane
+    while next_lane.child_lanes:
+        next_lane = next_lane.child_lanes[0]
+    for bug in kanban_bugs:
+        print "    Creating card for bug %s in %s." % (bug.id, next_lane.title)
+        card = next_lane.addCard()
+        card.title = bug.title
+        card.description = bug.description
+        card.external_card_id = str(bug.id)
+        card.type_id = new_cardtype
+        card.save()
+        bug.tags = [tag for tag in bug.tags if tag != bug_tag]
+        bug.lp_save()
+
 
 def get_lp(config):
     """Get a lp instance for a logged in user or anonymous access.
@@ -315,6 +352,12 @@
             if project:
                 all_projects.append(project)
     lp_users = LaunchpadUsersForBoard(lp, board)
+    new_bug_tag = bconf.get("bug_to_card_tag")
+    if new_bug_tag is not None:
+        for project in all_projects:
+            print "   Creating new cards for %s:" % project.name
+            create_new_cards(
+                board, project, new_bug_tag, bconf.get("bug_to_card_type"))
     print "   Syncing cards:"
     for card in board.getCardsWithExternalIds():
         if should_sync_card(card, bconf):

=== modified file 'src/lp2kanban/tests/test_bugs2cards.py'
--- src/lp2kanban/tests/test_bugs2cards.py	2012-10-02 14:02:04 +0000
+++ src/lp2kanban/tests/test_bugs2cards.py	2012-12-07 09:40:24 +0000
@@ -14,6 +14,7 @@
     get_branch_info,
     get_bug_status,
     get_card_status,
+    get_new_bugs,
     parse_config,
     should_sync_card,
     )
@@ -192,10 +193,15 @@
 
 
 class FauxBug:
-    def __init__(self, tags=[]):
+    def __init__(self, tags=[], title="Faux Bug",
+                 description="This is a faux bug.",
+                 bug_id=None):
         self.bug_tasks_collection = []
         self.linked_branches = []
         self.tags = tags
+        self.title = title
+        self.description = description
+        self.id = bug_id
 
     def addTask(self, target, status='New', assignee=None):
         self.bug_tasks_collection.append(
@@ -477,3 +483,48 @@
         slow_land.addNextLane(rollout)
 
         self.assertEqual(rollout, find_card_target_lane(qa, 'Rollout'))
+
+
+class FauxBoard:
+
+    def __init__(self, cards):
+        self.cards = cards
+
+
+class FauxCard:
+
+    def __init__(self, external_card_id=None):
+        self.external_card_id = external_card_id
+
+
+class GetNewBugsTest(unittest.TestCase):
+
+    def test_empty_board_no_bugs(self):
+        # If the board is empty, and there are no new bugs, no
+        # bugs are returned.
+        board = FauxBoard(cards=[])
+        bugs = []
+        self.assertEqual([], list(get_new_bugs(board, bugs)))
+
+    def test_nonempty_board_no_bugs(self):
+        # If the board has bugs, but there are no new bugs in the passed
+        # in list, no bugs are returned.
+        board = FauxBoard(
+            cards=[FauxCard(), FauxCard(external_card_id="123")])
+        bugs = []
+        self.assertEqual([], list(get_new_bugs(board, bugs)))
+
+    def test_no_new_bugs(self):
+        # If all the passed in bugs are already in the board, no bugs
+        # are returned.
+        board = FauxBoard(
+            cards=[FauxCard(), FauxCard(external_card_id="123")])
+        bugs = [FauxBug(bug_id=123)]
+        self.assertEqual([], list(get_new_bugs(board, bugs)))
+
+    def test_new_bugs(self):
+        # The passed in bugs that aren't in the board are returned.
+        board = FauxBoard(
+            cards=[FauxCard(), FauxCard(external_card_id="123")])
+        bugs = [FauxBug(bug_id=123), FauxBug(bug_id=457)]
+        self.assertEqual([bugs[1]], list(get_new_bugs(board, bugs)))