launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/check-series-bug-changeitype into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1234497 in Launchpad itself: "Project series bug tasks on proprietary project can target non-proprietary bugs"
  https://bugs.launchpad.net/launchpad/+bug/1234497

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/check-series-bug-changeitype/+merge/192449

Check for bugs on the product as well as any of its series when checking if a product can change information type. Drive by some comment fixes, since they should be complete sentences.
-- 
https://code.launchpad.net/~stevenk/launchpad/check-series-bug-changeitype/+merge/192449
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/check-series-bug-changeitype into lp:launchpad.

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2013-08-19 06:43:04 +0000
+++ lib/lp/registry/model/product.py	2013-10-24 01:12:08 +0000
@@ -478,20 +478,23 @@
         if not public_specs.is_empty():
             # Unlike bugs and branches, specifications cannot be USERDATA or a
             # security type.
-            yield CannotChangeInformationType(
-                'Some blueprints are public.')
+            yield CannotChangeInformationType('Some blueprints are public.')
         store = Store.of(self)
-        non_proprietary_bugs = store.find(Bug,
+        series_ids = [series.id for series in self.series]
+        non_proprietary_bugs = store.find(
+            Bug,
             Not(Bug.information_type.is_in(PROPRIETARY_INFORMATION_TYPES)),
-            BugTask.bug == Bug.id, BugTask.product == self.id)
+            BugTask.bug == Bug.id,
+            Or(
+                BugTask.product == self.id,
+                BugTask.productseriesID.is_in(series_ids)))
         if not non_proprietary_bugs.is_empty():
             yield CannotChangeInformationType(
                 'Some bugs are neither proprietary nor embargoed.')
         # Default returns all public branches.
         non_proprietary_branches = store.find(
             Branch, Branch.product == self.id,
-            Not(Branch.information_type.is_in(PROPRIETARY_INFORMATION_TYPES))
-        )
+            Not(Branch.information_type.is_in(PROPRIETARY_INFORMATION_TYPES)))
         if not non_proprietary_branches.is_empty():
             yield CannotChangeInformationType(
                 'Some branches are neither proprietary nor embargoed.')

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2013-08-19 06:43:04 +0000
+++ lib/lp/registry/tests/test_product.py	2013-10-24 01:12:08 +0000
@@ -539,7 +539,7 @@
                         raise errors[0]
 
     def test_checkInformationType_questions(self):
-        # Proprietary products must not have questions
+        # Proprietary products must not have questions.
         product = self.factory.makeProduct()
         for info_type in PROPRIETARY_INFORMATION_TYPES:
             with person_logged_in(product.owner):
@@ -549,12 +549,12 @@
         for info_type in PROPRIETARY_INFORMATION_TYPES:
             with person_logged_in(product.owner):
                 error, = list(product.checkInformationType(info_type))
-            with ExpectedException(CannotChangeInformationType,
-                                   'This project has questions.'):
+            with ExpectedException(
+                CannotChangeInformationType, 'This project has questions.'):
                 raise error
 
     def test_checkInformationType_translations(self):
-        # Proprietary products must not have translations
+        # Proprietary products must not have translations.
         productseries = self.factory.makeProductSeries()
         product = productseries.product
         for info_type in PROPRIETARY_INFORMATION_TYPES:
@@ -565,12 +565,12 @@
         for info_type in PROPRIETARY_INFORMATION_TYPES:
             with person_logged_in(product.owner):
                 error, = list(product.checkInformationType(info_type))
-            with ExpectedException(CannotChangeInformationType,
-                                   'This project has translations.'):
+            with ExpectedException(
+                CannotChangeInformationType, 'This project has translations.'):
                 raise error
 
     def test_checkInformationType_queued_translations(self):
-        # Proprietary products must not have queued translations
+        # Proprietary products must not have queued translations.
         productseries = self.factory.makeProductSeries()
         product = productseries.product
         entry = self.factory.makeTranslationImportQueueEntry(
@@ -578,8 +578,9 @@
         for info_type in PROPRIETARY_INFORMATION_TYPES:
             with person_logged_in(product.owner):
                 error, = list(product.checkInformationType(info_type))
-            with ExpectedException(CannotChangeInformationType,
-                                   'This project has queued translations.'):
+            with ExpectedException(
+                CannotChangeInformationType,
+                'This project has queued translations.'):
                 raise error
         removeSecurityProxy(entry).delete(entry.id)
         with person_logged_in(product.owner):
@@ -598,7 +599,8 @@
             productseries.translations_autoimport_mode = mode
             for info_type in PROPRIETARY_INFORMATION_TYPES:
                 error, = list(product.checkInformationType(info_type))
-                with ExpectedException(CannotChangeInformationType,
+                with ExpectedException(
+                    CannotChangeInformationType,
                     'Some product series have translation imports enabled.'):
                     raise error
         productseries.translations_autoimport_mode = (
@@ -607,6 +609,21 @@
             self.assertContentEqual(
                 [], product.checkInformationType(info_type))
 
+    def test_checkInformationType_series_only_bugs(self):
+        # A product with bugtasks that are only targetted to a series can
+        # not change information type.
+        series = self.factory.makeProductSeries()
+        bug = self.factory.makeBug(target=series.product)
+        with person_logged_in(series.owner):
+            bug.addTask(series.owner, series)
+            bug.default_bugtask.delete()
+            for info_type in PROPRIETARY_INFORMATION_TYPES:
+                error, = list(series.product.checkInformationType(info_type))
+                with ExpectedException(
+                    CannotChangeInformationType,
+                    'Some bugs are neither proprietary nor embargoed.'):
+                    raise error
+
     def test_private_forbids_translations(self):
         owner = self.factory.makePerson()
         product = self.factory.makeProduct(owner=owner)
@@ -671,7 +688,7 @@
             [ap.type for ap in aps])
 
     def test_product_information_type_default(self):
-        # Default information_type is PUBLIC
+        # Default information_type is PUBLIC.
         owner = self.factory.makePerson()
         product = getUtility(IProductSet).createProduct(
             owner, 'fnord', 'Fnord', 'Fnord', 'test 1', 'test 2')
@@ -769,7 +786,7 @@
                         product.answers_usage = usage
 
     def test_answers_for_public(self):
-        # Enabling answers is permitted while information_type is PUBLIC
+        # Enabling answers is permitted while information_type is PUBLIC.
         product = self.factory.makeProduct(
             information_type=InformationType.PUBLIC)
         self.assertEqual(ServiceUsage.UNKNOWN, product.answers_usage)