Graham Binns has proposed merging 
lp:~gmb/launchpad/choose-affected-product-oops-bug-370117 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #370117 in Launchpad itself: "BugTask:+choose-affected-product oops: 
getBugFilingAndSearchLinks needs more than 1 value to unpack"
  https://bugs.launchpad.net/launchpad/+bug/370117

For more details, see:
https://code.launchpad.net/~gmb/launchpad/choose-affected-product-oops-bug-370117/+merge/62704

This branch fixes an OOPS with Bug:+choose-affected-product. The OOPS was 
caused by code that didn't account for the fact that it might encounter data 
that wasn't valid in the context in which it was being called. I've fixed this 
by wrapping it in a try:except and have added a regression test.
-- 
https://code.launchpad.net/~gmb/launchpad/choose-affected-product-oops-bug-370117/+merge/62704
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~gmb/launchpad/choose-affected-product-oops-bug-370117 into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py	2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/model/bugtracker.py	2011-05-27 16:17:32 +0000
@@ -448,11 +448,16 @@
         description = description.encode('utf-8')
 
         if self.bugtrackertype == BugTrackerType.SOURCEFORGE:
-            # SourceForge bug trackers use a group ID and an ATID to
-            # file a bug, rather than a product name. remote_product
-            # should be an ampersand-separated string in the form
-            # 'group_id&atid'
-            group_id, at_id = remote_product.split('&')
+            try:
+                # SourceForge bug trackers use a group ID and an ATID to
+                # file a bug, rather than a product name. remote_product
+                # should be an ampersand-separated string in the form
+                # 'group_id&atid'
+                group_id, at_id = remote_product.split('&')
+            except ValueError:
+                # If remote_product contains something that's not valid
+                # in a SourceForge context we just return early.
+                return None
 
             # If this bug tracker is the SourceForge celebrity the link
             # is to the new bug tracker rather than the old one.

=== modified file 'lib/lp/bugs/tests/test_bugtracker.py'
--- lib/lp/bugs/tests/test_bugtracker.py	2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/tests/test_bugtracker.py	2011-05-27 16:17:32 +0000
@@ -351,6 +351,23 @@
         self.assertRaises(BugTrackerConnectError, tracker._csv_data)
 
 
+class TestSourceForge(TestCaseWithFactory):
+    """Tests for SourceForge-specific BugTracker code."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_getBugFilingAndSearchLinks_handles_bad_data_correctly(self):
+        # It's possible for Product.remote_product to contain data
+        # that's not valid for SourceForge BugTrackers.
+        # getBugFilingAndSearchLinks() will return None if it encounters
+        # bad data in the remote_product field.
+        remote_product = "this is not valid"
+        bug_tracker = self.factory.makeBugTracker(
+            bugtrackertype=BugTrackerType.SOURCEFORGE)
+        self.assertIs(
+            None, bug_tracker.getBugFilingAndSearchLinks(remote_product))
+
+
 class TestMakeBugtrackerName(TestCase):
     """Tests for make_bugtracker_name."""
 

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to