Merge lp:~dpb/lp2kanban/moar-functions into lp:lp2kanban

[David Britton]

Split apart large sync_cards into functions for some code separation.

Split apart large sync_cards into functions for some code separation. Follow-on branches will continue this.

All tests should still pass (with minimal changes).

[Chad Smith (chad.smith)]

loooks good from this side. thanks for the cleanups.

review: Approve

[🤖 Landscape Builder (landscape-builder)]

The attempt to merge lp:~davidpbritton/lp2kanban/moar-functions into lp:lp2kanban failed. Below is the output from the failed tests.

mkdir download-cache
python bootstrap.py -v 1.7.1 -f eggs -c buildout.cfg

Traceback (most recent call last):
  File "bootstrap.py", line 80, in <module>
    exec(urlopen('https://bootstrap.pypa.io/ez_setup.py').read(), ez)
  File "/usr/lib/python2.7/urllib2.py", line 127, in urlopen
    return _opener.open(url, data, timeout)
  File "/usr/lib/python2.7/urllib2.py", line 404, in open
    response = self._open(req, data)
  File "/usr/lib/python2.7/urllib2.py", line 422, in _open
    '_open', req)
  File "/usr/lib/python2.7/urllib2.py", line 382, in _call_chain
    result = func(*args)
  File "/usr/lib/python2.7/urllib2.py", line 1222, in https_open
    return self.do_open(httplib.HTTPSConnection, req)
  File "/usr/lib/python2.7/urllib2.py", line 1184, in do_open
    raise URLError(err)
urllib2.URLError: <urlopen error [Errno 110] Connection timed out>
make: *** [bin/buildout] Error 1

[🤖 Landscape Builder (landscape-builder)]

No approved revision specified.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make check
Result: Fail
Revno: 144
Branch: lp:~davidpbritton/lp2kanban/moar-functions
Jenkins: https://ci.lscape.net/job/latch-test/8155/

review: Needs Fixing (test results)

=== modified file 'src/lp2kanban/bugs2cards.py'
--- src/lp2kanban/bugs2cards.py	2016-05-02 20:32:26 +0000
+++ src/lp2kanban/bugs2cards.py	2016-08-26 17:12:09 +0000
@@ -558,8 +558,9 @@
         return None
 
 
-def create_new_cards(board, launchpad_project, bug_tag, cardtype_name=None,
-                     cardlane_path=None, feature_lanes=None):
+def create_cards_in_project(
+        board, launchpad_project, bug_tag, cardtype_name=None,
+        cardlane_path=None, feature_lanes=None):
     """Create cards for bugs tagged in Launchpad.
 
     If a bug is tagged with the given tag, a new card will be created in
@@ -641,11 +642,10 @@
     return None
 
 
-def sync_board(board, bconf, lp=None, lp_users=None):
-    if not lp:
-        lp = get_lp(bconf)
-    if not lp_users:
-        lp_users = LaunchpadUsersForBoard()
+def get_lp_data(board, bconf):
+    """Given a board and config, return lp, lp projects and lp users."""
+    lp = get_lp(bconf)
+    lp_users = LaunchpadUsersForBoard()
     all_projects = []
     project_group = bconf.get('project_group')
     if project_group:
@@ -661,14 +661,22 @@
             if project:
                 all_projects.append(project)
     lp_users.set_up_users(lp, board, bconf)
+    return lp, all_projects, lp_users
+
+
+def create_cards(board, bconf, lp_users, lp_projects):
+    """Create new cards on all projects."""
     new_bug_tag = bconf.get("bug_to_card_tag")
     if new_bug_tag is not None:
-        for project in all_projects:
-            create_new_cards(
-                board, project, new_bug_tag, bconf.get("bug_to_card_type"),
+        for lp_project in lp_projects:
+            create_cards_in_project(
+                board, lp_project, new_bug_tag, bconf.get("bug_to_card_type"),
                 bconf.get("bug_to_card_lane"), lp_users.feature_lanes)
-    print "   Syncing cards:"
-    # Process linked branch cards
+
+
+def sync_cards_linked_branches(board, bconf, lp, lp_users):
+    """Sync any cards that have linked external branches."""
+    print "   Syncing branch cards:"
     for card in board.getCardsWithExternalLinks(only_branches=True):
         if card.external_card_id:
             # Cards with external_id are processed by getCardsWithExternalIds
@@ -705,6 +713,9 @@
                 else:
                     print "   * %s (in %s)" % (card.title, card_path)
 
+
+def sync_cards_external_ids(board, bconf, lp, lp_users, lp_projects):
+    """Sync any cards that have linked external ids (bugs in launchpad)."""
     for card in board.getCardsWithExternalIds():
         if should_sync_card(card, bconf):
             lp_bug = get_lp_bug(card, lp)
@@ -712,10 +723,10 @@
                 continue
             ignore_external_projects = (
                 bconf.get("autosync") == "active-projects")
-            if ignore_external_projects and not is_bug_in_projects(lp_bug, all_projects):
+            if ignore_external_projects and not is_bug_in_projects(lp_bug, lp_projects):
                 continue  # Skip bugs in external projects
             synced, card_status, assignees, mp_url, mp_type = get_bug_status(
-                lp_bug, all_projects, lp_users.lp_to_kanban.keys())
+                lp_bug, lp_projects, lp_users.lp_to_kanban.keys())
 
             card_path = card.lane.path
             if isinstance(card.lane.board, LeankitTaskBoard):
@@ -752,6 +763,8 @@
                 print "   * %s: %s -- Not synced" % (
                     lp_bug.id, card.title)
 
+def move_cards_to_taskboards(board, bconf, lp, lp_users):
+    """Move branch cards to feature cards if they are landed."""
     if "${group}" in bconf["landing_lanes"]:
         # Replace the optional template ${group} with configured board groups.
         # This creates a list of landing_lanes, one per group on the board.
@@ -768,6 +781,9 @@
             move_cards_to_feature_taskboard(
                 feature_card, lp_bug, valid_taskboard_lanes)
 
+
+def convert_cards_to_work_items(board, bconf, lp, lp_users, lp_projects):
+    """Create work items out of cards if desired."""
     if bconf.get("convert_cards_to_work_items", False) == "on":
         # Sync cards to blueprint work items, if so configured.
         print "   Checking for cards to sync with work items..."
@@ -874,7 +890,13 @@
             if board is None:
                 print "Could not retrieve board.  Skipping."
                 continue
-            sync_board(board, boards_config[board_name])
+            bconf = boards_config[board_name]
+            lp, lp_projects, lp_users = get_lp_data(board, bconf)
+            create_cards(board, bconf, lp_users, lp_projects)
+            sync_cards_linked_branches(board, bconf, lp, lp_users)
+            sync_cards_external_ids(board, bconf, lp, lp_users, lp_projects)
+            move_cards_to_taskboards(board, bconf, lp, lp_users)
+            convert_cards_to_work_items(board, bconf, lp, lp_users)
             save_board(board)
         except (KeyError, AssertionError) as e:
             print "Exception caught.  Skipping board."

=== modified file 'src/lp2kanban/tests/test_bugs2cards.py'
--- src/lp2kanban/tests/test_bugs2cards.py	2016-05-04 14:58:20 +0000
+++ src/lp2kanban/tests/test_bugs2cards.py	2016-08-26 17:12:09 +0000
@@ -15,7 +15,7 @@
 
 from lp2kanban.bugs2cards import (
     CardStatus,
-    create_new_cards,
+    create_cards_in_project,
     find_card_target_lane_next,
     get_branch_info,
     is_bug_in_projects,
@@ -31,7 +31,8 @@
     parse_groups_config,
     should_sync_card,
     should_move_card,
-    sync_board
+    sync_cards_external_ids,
+    move_cards_to_taskboards
     )
 from lp2kanban.kanban import (
     RESOURCE_TYPE_LINK_SERIES,
@@ -980,7 +981,7 @@
         board = FauxBoard()
         bug_lane = board.addLane('bugs')
         project = FauxLaunchpadProject(bug_tasks=[])
-        create_new_cards(board, project, 'kanban')
+        create_cards_in_project(board, project, 'kanban')
         self.assertEqual([], board.cards)
 
     def test_new_bugs_card_added(self):
@@ -993,7 +994,7 @@
                            bug_id=42, title='Test bug',
                            description='Test bug description.',
                            tags=['kanban'])])
-        create_new_cards(board, project, 'kanban')
+        create_cards_in_project(board, project, 'kanban')
         [card] = board.cards
         self.assertEqual('Test bug', card.title)
         self.assertEqual('Test bug description.', card.description)
@@ -1011,7 +1012,7 @@
                            bug_id=42, title='Test bug',
                            description='Test bug description.',
                            tags=['kanban'])])
-        create_new_cards(board, project, 'kanban')
+        create_cards_in_project(board, project, 'kanban')
         [bug_task] = project.bug_tasks
         self.assertEqual([], bug_task.bug.tags)
         self.assertTrue(bug_task.bug.saved)
@@ -1026,7 +1027,7 @@
                            tags=['kanban'])])
         old_save = FauxCard.save
         FauxCard.save = lambda x: None
-        create_new_cards(board, project, 'kanban')
+        create_cards_in_project(board, project, 'kanban')
         [bug_task] = project.bug_tasks
         self.assertEqual(['kanban'], bug_task.bug.tags)
         self.assertFalse(bug_task.bug.saved)
@@ -1034,7 +1035,7 @@
 
     def test_new_bugs_tagged_only(self):
         # Cards are created only for bugs having the bug tag that is
-        # passed in to create_new_cards().
+        # passed in to create_cards_in_project().
         board = FauxBoard()
         bug_lane = board.addLane('bugs')
         project = FauxLaunchpadProject(
@@ -1047,7 +1048,7 @@
                        FauxBugTask(
                            bug_id=42, title='Tagged with no tags',
                            tags=[])])
-        create_new_cards(board, project, 'kanban')
+        create_cards_in_project(board, project, 'kanban')
         [card] = board.cards
         self.assertEqual('Tagged with right tag', card.title)
         self.assertEqual('42', card.external_card_id)
@@ -1064,7 +1065,7 @@
                            tags=['kanban'])])
         feature_type = FauxCardType(id=2, name='Feature', is_default=False)
         board.cardtypes[feature_type.id] = feature_type
-        create_new_cards(board, project, 'kanban', cardtype_name='Feature')
+        create_cards_in_project(board, project, 'kanban', cardtype_name='Feature')
         [card] = board.cards
         self.assertEqual(feature_type.id, card.type_id)
         self.assertTrue(card.saved)
@@ -1082,7 +1083,7 @@
                            tags=['kanban'])])
         feature_type = FauxCardType(id=2, name='Feature', is_default=False)
         board.cardtypes[feature_type.id] = feature_type
-        create_new_cards(
+        create_cards_in_project(
             board, project, 'kanban', cardtype_name='Feature',
             cardlane_path='incoming')
         [card] = board.cards
@@ -1098,7 +1099,7 @@
                            bug_id=42, title='Test bug',
                            description='x' * 20000,
                            tags=['kanban'])])
-        create_new_cards(board, project, 'kanban')
+        create_cards_in_project(board, project, 'kanban')
         [card] = board.cards
         self.assertEqual('Test bug', card.title)
         self.assertEqual(19500, len(card.description))
@@ -1628,7 +1629,7 @@
         self.deploy_lane = FauxLane(board=self.board, path='deploy')
         self.lp_users = FauxLaunchpadUsersForBoard()
 
-    def test_sync_board_moves_landing_cards_to_taskboard_in_landing_lane(self):
+    def test_sync_cards_moves_landing_cards_to_taskboard_in_landing_lane(self):
     	"""
         Feature-linked cards in landing lanes will move to taskboard when the
         feature card is in landing_lanes.
@@ -1641,11 +1642,12 @@
         landed_card.type = self.branch_type
         landed_card.lane = self.landing_lane
         self.board.cards.extend([coding_card, landed_card])
-        sync_board(self.board, self.bconf, lp=FauxLP(), lp_users=self.lp_users)
+        move_cards_to_taskboards(
+            self.board, self.bconf, lp=FauxLP(), lp_users=self.lp_users)
         self.assertIsNone(coding_card.moved_to)
         self.assertEqual(self.feature_card, landed_card.moved_to)
 
-    def test_sync_board_moves_landing_cards_to_taskboard_in_deploy_lane(self):
+    def test_sync_cards_moves_landing_cards_to_taskboard_in_deploy_lane(self):
     	"""
         Feature-linked cards in landing lanes will move to taskboard when the
         feature card is in deploy_lanes.
@@ -1658,11 +1660,12 @@
         landed_card.type = self.branch_type
         landed_card.lane = self.landing_lane
         self.board.cards.extend([coding_card, landed_card])
-        sync_board(self.board, self.bconf, lp=FauxLP(), lp_users=self.lp_users)
+        move_cards_to_taskboards(
+            self.board, self.bconf, lp=FauxLP(), lp_users=self.lp_users)
         self.assertIsNone(coding_card.moved_to)
         self.assertEqual(self.feature_card, landed_card.moved_to)
 
-    def test_sync_board_moves_no_cards_to_taskboard_not_landed_or_deploy(self):
+    def test_sync_cards_moves_no_cards_to_taskboard_not_landed_or_deploy(self):
     	"""
         Feature-linked cards in landing lanes will not move to taskboard when
         the feature card is not in the landing_lanes or deploy_lanes.
@@ -1672,10 +1675,11 @@
         landed_card.type = self.branch_type
         landed_card.lane = self.landing_lane
         self.board.cards.extend([landed_card])
-        sync_board(self.board, self.bconf, lp=FauxLP(), lp_users=self.lp_users)
+        move_cards_to_taskboards(
+            self.board, self.bconf, lp=FauxLP(), lp_users=self.lp_users)
         self.assertIsNone(landed_card.moved_to)
 
-    def test_sync_board_moves_landing_cards_to_taskboard_with_groups(self):
+    def test_sync_cards_moves_landing_cards_to_taskboard_with_groups(self):
     	"""
         When landing_lanes contains ${groups} template, feature-linkd cards in
         all configured  landing lanes will be moved to the feature taskboard.
@@ -1699,12 +1703,13 @@
         landed_card2.type = self.branch_type
         landed_card2.lane = landing_lane2
         self.board.cards.extend([coding_card, landed_card1, landed_card2])
-        sync_board(self.board, self.bconf, lp=FauxLP(), lp_users=lp_users)
+        move_cards_to_taskboards(
+            self.board, self.bconf, lp=FauxLP(), lp_users=lp_users)
         self.assertIsNone(coding_card.moved_to)
         self.assertEqual(self.feature_card, landed_card1.moved_to)
         self.assertEqual(self.feature_card, landed_card2.moved_to)
 
-    def test_sync_board_skips_bugs_of_external_projects_when_active_projects_set(self):
+    def test_sync_cards_skips_bugs_of_external_projects_when_active_projects_set(self):
     	"""Bugs of external projects are skipped when autosync set to active-projects."""
         # Configure bugs2cards to sync only active-projects myproject1 and myproject2
         self.bconf.update({"autosync": "active-projects",
@@ -1716,6 +1721,7 @@
         project2 = FauxTarget(resource_type_link="project2", project="myproject2")
         otherproject = FauxTarget(resource_type_link="project", project="otherproject")
         lp_users = FauxLaunchpadUsersForBoard(lp_to_kanban={"person1": "Person One"})
+        lp_projects = [project1, project2]
         bug_task1 = FauxBugTask(
             bug_id=123, target=project1, title='Test bug in myproject',
             description='Test bug description.')
@@ -1739,7 +1745,7 @@
         # Include project1 and project2 in our active 'projects' fake LP data
         lp = FauxLP({"projects": {"myproject": project1, "myproject2": project2},
                  "bugs": {123: bug_task1.bug, "456": bug_task2.bug}})
-        sync_board(board, self.bconf, lp=lp, lp_users=lp_users)
+        sync_cards_external_ids(board, self.bconf, lp, lp_users, lp_projects)
         coding_lane = FauxLane(board=board, path='coding')
         self.assertEqual(new_lane, bug_card1.moved_to)
         self.assertIsNone(bug_card2.moved_to)