launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/private-bugs-bug-importer into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #457489 in Launchpad itself: "Bug importer ignores the product.private_bugs flag"
  https://bugs.launchpad.net/launchpad/+bug/457489

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/private-bugs-bug-importer/+merge/97790

Have the bug importer respect IProduct.private_bugs.

Remove the double-set of private/security_related because we want to be sure.

Remove use of unittest.TestCase.
-- 
https://code.launchpad.net/~stevenk/launchpad/private-bugs-bug-importer/+merge/97790
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/private-bugs-bug-importer into lp:launchpad.

=== modified file 'lib/lp/bugs/scripts/bugimport.py'
--- lib/lp/bugs/scripts/bugimport.py	2012-01-04 03:23:19 +0000
+++ lib/lp/bugs/scripts/bugimport.py	2012-03-16 03:27:11 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """An XML bug importer
@@ -278,6 +278,9 @@
 
         private = get_value(bugnode, 'private') == 'True'
         security_related = get_value(bugnode, 'security_related') == 'True'
+        # If the product has private_bugs, we force private to True.
+        if self.product.private_bugs:
+            private = True
 
         if owner is None:
             owner = self.bug_importer
@@ -308,9 +311,6 @@
             bug.linkMessage(msg)
             self.createAttachments(bug, msg, commentnode)
 
-        # set up bug
-        private = get_value(bugnode, 'private') == 'True'
-        security_related = get_value(bugnode, 'security_related') == 'True'
         bug.setPrivacyAndSecurityRelated(private, security_related, owner)
         bug.name = get_value(bugnode, 'nickname')
         description = get_value(bugnode, 'description')

=== modified file 'lib/lp/bugs/scripts/tests/test_bugimport.py'
--- lib/lp/bugs/scripts/tests/test_bugimport.py	2012-01-17 14:27:01 +0000
+++ lib/lp/bugs/scripts/tests/test_bugimport.py	2012-03-16 03:27:11 +0000
@@ -1,9 +1,10 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+__metaclass__ = type
+
 import os
 import re
-import unittest
 
 import pytz
 from testtools.content import text_content
@@ -51,7 +52,7 @@
 from lp.testing.layers import LaunchpadZopelessLayer
 
 
-class UtilsTestCase(unittest.TestCase):
+class UtilsTestCase(TestCase):
     """Tests for the various utility functions used by the importer."""
 
     def test_parse_date(self):
@@ -297,7 +298,7 @@
         self.assertEqual(person.preferredemail.email, 'foo@xxxxxxxxxxxxx')
 
 
-class GetMilestoneTestCase(unittest.TestCase):
+class GetMilestoneTestCase(TestCase):
     """Tests for the BugImporter.getMilestone() method."""
     layer = LaunchpadZopelessLayer
 
@@ -451,15 +452,34 @@
   </comment>
 </bug>'''
 
-
-class ImportBugTestCase(unittest.TestCase):
+public_bug = '''\
+<bug xmlns="https://launchpad.net/xmlns/2006/bugs"; id="101">
+  <private>False</private>
+  <security_related>False</security_related>
+  <datecreated>2004-10-12T12:00:00Z</datecreated>
+  <title>A non private security bug</title>
+  <description>Description</description>
+  <reporter name="foo" email="foo@xxxxxxxxxxx">Foo User</reporter>
+  <status>TRIAGED</status>
+  <importance>LOW</importance>
+  <comment>
+    <sender name="foo" email="foo@xxxxxxxxxxx">Foo User</sender>
+    <date>2004-10-12T12:00:00Z</date>
+    <text>Description</text>
+  </comment>
+</bug>'''
+
+
+class ImportBugTestCase(TestCase):
     """Test importing of a bug from XML"""
     layer = LaunchpadZopelessLayer
 
     def setUp(self):
+        super(ImportBugTestCase, self).setUp()
         login('bug-importer@xxxxxxxxxxxxx')
 
     def tearDown(self):
+        super(ImportBugTestCase, self).tearDown()
         logout()
 
     def assertNoPendingNotifications(self, bug):
@@ -653,6 +673,19 @@
         self.assertEqual(bug101.private, False)
         self.assertEqual(bug101.security_related, True)
 
+    def test_public_bug_product_private_bugs(self):
+        # Test that if we import a public bug into a product with 
+        # private_bugs, the bug is private.
+        product = getUtility(IProductSet).getByName('netapplet')
+        removeSecurityProxy(product).private_bugs = True
+        importer = bugimport.BugImporter(
+            product, 'bugs.xml', 'bug-map.pickle', verify_users=True)
+        bugnode = ET.fromstring(public_bug)
+        bug101 = importer.importBug(bugnode)
+        self.assertIsNot(None, bug101)
+        self.assertTrue(bug101.private)
+        self.assertFalse(bug101.security_related)
+
 
 class BugImportCacheTestCase(TestCase):
     """Test of bug mapping cache load/save routines."""
@@ -997,7 +1030,7 @@
             unmodified_remote_ids, server_time, self.bugtracker)
 
 
-class CheckwatchesErrorRecoveryTestCase(unittest.TestCase):
+class CheckwatchesErrorRecoveryTestCase(TestCase):
     """Test that errors in the bugwatch import process don't
     invalidate the entire run.
     """