Merge lp:~wgrant/launchpad/rework-bug-default-type-2 into lp:launchpad

[William Grant]

Introduce {Product,Distribution}.getDefaultBugInformationType, use it in place of Product.private_bugs in most places.

This branch introduces {Product,Distribution}.getDefaultBugInformationType, and replaces most callsites of Product.private_bugs. This simplifies model and view code, and lets us be a bit more flexible.

bugimport still needs porting, as it's a little more complex.

[j.c.sackett (jcsackett)]

Thanks, this looks good.

review: Approve

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2012-08-02 08:31:08 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2012-08-03 00:28:29 +0000
@@ -120,6 +120,7 @@
 from lp.registry.enums import (
     InformationType,
     PRIVATE_INFORMATION_TYPES,
+    PUBLIC_INFORMATION_TYPES,
     SECURITY_INFORMATION_TYPES,
     )
 from lp.registry.interfaces.distribution import IDistribution
@@ -276,7 +277,8 @@
         cache.objects['private_types'] = [
             type.name for type in PRIVATE_INFORMATION_TYPES]
         cache.objects['bug_private_by_default'] = (
-            IProduct.providedBy(self.context) and self.context.private_bugs)
+            self.context.pillar.getDefaultBugInformationType() in
+            PRIVATE_INFORMATION_TYPES)
         cache.objects['information_type_data'] = [
             {'value': term.name, 'description': term.description,
             'name': term.title,
@@ -365,14 +367,18 @@
         return field_names
 
     @property
+    def default_information_type(self):
+        value = self.context.pillar.getDefaultBugInformationType()
+        if (self.extra_data
+            and self.extra_data.private
+            and value in PUBLIC_INFORMATION_TYPES):
+            value = InformationType.USERDATA
+        return value
+
+    @property
     def initial_values(self):
         """Give packagename a default value, if applicable."""
-        if (self.context and IProduct.providedBy(self.context)
-            and self.context.private_bugs):
-            type = InformationType.USERDATA
-        else:
-            type = InformationType.PUBLIC
-        values = {'information_type': type}
+        values = {'information_type': self.default_information_type}
 
         if IDistributionSourcePackage.providedBy(self.context):
             values['packagename'] = self.context.name
@@ -512,10 +518,14 @@
         packagename = data.get("packagename")
 
         information_type = data.get("information_type")
-        # If the old UI is enabled, security bugs are always embargoed
-        # when filed, but can be disclosed after they've been reported.
-        if information_type is None and data.get("security_related"):
-            information_type = InformationType.PRIVATESECURITY
+        if information_type is None:
+            # If the old UI is enabled, security bugs are always embargoed
+            # when filed, but can be disclosed after they've been reported.
+            # Otherwise we use the default.
+            if data.get("security_related"):
+                information_type = InformationType.PRIVATESECURITY
+            else:
+                information_type = self.default_information_type
 
         context = self.context
 
@@ -559,10 +569,6 @@
             notifications.append(
                 'Additional information was added to the bug description.')
 
-        if not self.is_bug_supervisor and extra_data.private:
-            if params.information_type in (None, InformationType.PUBLIC):
-                params.information_type = InformationType.USERDATA
-
         # Apply any extra options given by privileged users.
         if self.is_bug_supervisor:
             if 'assignee' in data:

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-08-03 00:18:27 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-08-03 00:28:29 +0000
@@ -42,7 +42,10 @@
     find_main_content,
     find_tag_by_id,
     )
-from lp.testing.views import create_initialized_view
+from lp.testing.views import (
+    create_initialized_view,
+    create_view,
+    )
 
 
 class TestBugTargetFileBugConfirmationMessage(TestCaseWithFactory):
@@ -350,7 +353,8 @@
             }]
         self.assertEqual(expected_guidelines, view.bug_reporting_guidelines)
 
-    def filebug_via_view(self, private_bugs=False, information_type=None):
+    def filebug_via_view(self, private_bugs=False, information_type=None,
+                         extra_data_token=None):
         form = {
             'field.title': 'A bug',
             'field.comment': 'A comment',
@@ -362,28 +366,53 @@
         if private_bugs:
             removeSecurityProxy(product).private_bugs = True
         with person_logged_in(product.owner):
-            view = create_initialized_view(
-                product, '+filebug', form=form, principal=product.owner)
+            view = create_view(
+                product, '+filebug', method='POST', form=form,
+                principal=product.owner)
+            if extra_data_token is not None:
+                view = view.publishTraverse(view.request, extra_data_token)
+            view.initialize()
             bug_url = view.request.response.getHeader('Location')
             bug_number = bug_url.split('/')[-1]
-            return getUtility(IBugSet).getByNameOrID(bug_number)
+            return (getUtility(IBugSet).getByNameOrID(bug_number), view)
 
     def test_filebug_default_information_type(self):
         # If we don't specify the bug's information_type, it is PUBLIC for
         # products with private_bugs=False.
-        bug = self.filebug_via_view()
+        bug, view = self.filebug_via_view()
+        self.assertEqual(
+            InformationType.PUBLIC, view.default_information_type)
         self.assertEqual(InformationType.PUBLIC, bug.information_type)
 
     def test_filebug_set_information_type(self):
         # When we specify the bug's information_type, it is set.
-        bug = self.filebug_via_view(information_type='PRIVATESECURITY')
+        bug, view = self.filebug_via_view(information_type='PRIVATESECURITY')
         self.assertEqual(
             InformationType.PRIVATESECURITY, bug.information_type)
 
     def test_filebug_information_type_with_private_bugs(self):
         # If we don't specify the bug's information_type, it is USERDATA for
         # products with private_bugs=True.
-        bug = self.filebug_via_view(private_bugs=True)
+        bug, view = self.filebug_via_view(private_bugs=True)
+        self.assertEqual(
+            InformationType.USERDATA, view.default_information_type)
+        self.assertEqual(InformationType.USERDATA, bug.information_type)
+
+    def test_filebug_information_type_with_public_blob(self):
+        # Bugs filed with an apport blob that doesn't request privacy
+        # are public by default.
+        blob = self.factory.makeProcessedApportBlob({})
+        bug, view = self.filebug_via_view(extra_data_token=blob.uuid)
+        self.assertEqual(
+            InformationType.PUBLIC, view.default_information_type)
+        self.assertEqual(InformationType.PUBLIC, bug.information_type)
+
+    def test_filebug_information_type_with_private_blob(self):
+        # An apport blob can ask for the bug to be private.
+        blob = self.factory.makeProcessedApportBlob({'private': True})
+        bug, view = self.filebug_via_view(extra_data_token=blob.uuid)
+        self.assertEqual(
+            InformationType.USERDATA, view.default_information_type)
         self.assertEqual(InformationType.USERDATA, bug.information_type)
 
     def test_filebug_information_type_normal_projects(self):

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-08-01 07:10:54 +0000
+++ lib/lp/bugs/model/bug.py	2012-08-03 00:28:29 +0000
@@ -2638,11 +2638,12 @@
         # of its attribute values below.
         params = snapshot_bug_params(bug_params)
 
+        context = params.product or params.distribution
+
         if params.information_type is None:
-            # If the private_bugs flag is set on a product, then
-            # force the new bug report to be private.
-            if params.product and params.product.private_bugs:
-                params.information_type = InformationType.USERDATA
+            if context is not None:
+                params.information_type = (
+                    context.getDefaultBugInformationType())
             else:
                 params.information_type = InformationType.PUBLIC
 
@@ -2662,11 +2663,6 @@
                 bug, params.owner, target, status=params.status)
 
         if params.information_type in SECURITY_INFORMATION_TYPES:
-            if params.product:
-                context = params.product
-            else:
-                context = params.distribution
-
             if context.security_contact:
                 bug.subscribe(context.security_contact, params.owner)
             else:

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2012-07-26 04:36:03 +0000
+++ lib/lp/registry/interfaces/distribution.py	2012-08-03 00:28:29 +0000
@@ -625,6 +625,12 @@
         :return: A sequence of `InformationType`s.
         """
 
+    def getDefaultBugInformationType():
+        """Get the default information type of a new bug in this distro.
+
+        :return: The `InformationType`.
+        """
+
     def userCanEdit(user):
         """Can the user edit this distribution?"""
 

=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py	2012-07-26 04:36:03 +0000
+++ lib/lp/registry/interfaces/product.py	2012-08-03 00:28:29 +0000
@@ -794,11 +794,17 @@
         """Mutator for private_bugs that checks entitlement."""
 
     def getAllowedBugInformationTypes():
-        """Get the information types that a bug in this distribution can have.
+        """Get the information types that a bug in this project can have.
 
         :return: A sequence of `InformationType`s.
         """
 
+    def getDefaultBugInformationType():
+        """Get the default information type of a new bug in this project.
+
+        :return: The `InformationType`.
+        """
+
     def getVersionSortedSeries(statuses=None, filter_statuses=None):
         """Return all the series sorted by the name field as a version.
 

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2012-08-01 07:10:54 +0000
+++ lib/lp/registry/model/distribution.py	2012-08-03 00:28:29 +0000
@@ -1603,6 +1603,10 @@
         types.discard(InformationType.PROPRIETARY)
         return types
 
+    def getDefaultBugInformationType(self):
+        """See `IDistribution.`"""
+        return InformationType.PUBLIC
+
     def userCanEdit(self, user):
         """See `IDistribution`."""
         if user is None:

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-08-01 07:10:54 +0000
+++ lib/lp/registry/model/product.py	2012-08-03 00:28:29 +0000
@@ -571,6 +571,13 @@
         types.discard(InformationType.PROPRIETARY)
         return types
 
+    def getDefaultBugInformationType(self):
+        """See `IDistribution.`"""
+        if self.private_bugs:
+            return InformationType.USERDATA
+        else:
+            return InformationType.PUBLIC
+
     def _ensurePolicies(self, information_types):
         # Ensure that the product has access policies for the specified
         # information types.

=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py	2012-07-26 04:36:03 +0000
+++ lib/lp/registry/tests/test_distribution.py	2012-08-03 00:28:29 +0000
@@ -282,7 +282,13 @@
         self.assertContentEqual(
             [InformationType.PUBLIC, InformationType.PUBLICSECURITY,
              InformationType.PRIVATESECURITY, InformationType.USERDATA],
-            self.factory.makeProduct().getAllowedBugInformationTypes())
+            self.factory.makeDistribution().getAllowedBugInformationTypes())
+
+    def test_getDefaultBugInformationType(self):
+        # The default information type for distributions is always PUBLIC.
+        self.assertEqual(
+            InformationType.PUBLIC,
+            self.factory.makeDistribution().getDefaultBugInformationType())
 
 
 class TestDistributionCurrentSourceReleases(

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-07-26 04:36:03 +0000
+++ lib/lp/registry/tests/test_product.py	2012-08-03 00:28:29 +0000
@@ -409,6 +409,18 @@
              InformationType.PRIVATESECURITY, InformationType.USERDATA],
             self.factory.makeProduct().getAllowedBugInformationTypes())
 
+    def test_getDefaultBugInformationType_public(self):
+        # The default information type for normal projects is PUBLIC.
+        product = self.factory.makeProduct()
+        self.assertEqual(
+            InformationType.PUBLIC, product.getDefaultBugInformationType())
+
+    def test_getDefaultBugInformationType_private(self):
+        # private_bugs overrides the default information type to USERDATA.
+        product = self.factory.makeProduct(private_bugs=True)
+        self.assertEqual(
+            InformationType.USERDATA, product.getDefaultBugInformationType())
+
 
 class TestProductFiles(TestCase):
     """Tests for downloadable product files."""

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-07-31 01:36:44 +0000
+++ lib/lp/testing/factory.py	2012-08-03 00:28:29 +0000
@@ -43,6 +43,7 @@
 import sys
 from textwrap import dedent
 from types import InstanceType
+import uuid
 import warnings
 
 from bzrlib.plugins.builder.recipe import BaseRecipeBranch
@@ -76,6 +77,7 @@
     )
 from lp.blueprints.interfaces.specification import ISpecificationSet
 from lp.blueprints.interfaces.sprint import ISprintSet
+from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
 from lp.bugs.interfaces.bug import (
     CreateBugParams,
     IBugSet,
@@ -90,6 +92,7 @@
     CveStatus,
     ICveSet,
     )
+from lp.bugs.model.bug import FileBugData
 from lp.buildmaster.enums import (
     BuildFarmJobType,
     BuildStatus,
@@ -244,6 +247,9 @@
 from lp.services.temporaryblobstorage.interfaces import (
     ITemporaryStorageManager,
     )
+from lp.services.temporaryblobstorage.model import (
+    TemporaryBlobStorage,
+    )
 from lp.services.utils import AutoDecorate
 from lp.services.webapp.dbpolicy import MasterDatabasePolicy
 from lp.services.webapp.interfaces import (
@@ -4155,6 +4161,20 @@
 
         return getUtility(ITemporaryStorageManager).fetch(new_uuid)
 
+    def makeProcessedApportBlob(self, metadata):
+        """Create a processed ApportJob with the specified metadata dict.
+
+        It doesn't actually run the job. It fakes it, and uses a fake
+        librarian file so as to work without the librarian.
+        """
+        blob = TemporaryBlobStorage(uuid=str(uuid.uuid1()), file_alias=1)
+        job = getUtility(IProcessApportBlobJobSource).create(blob)
+        job.job.start()
+        removeSecurityProxy(job).metadata = {
+            'processed_data': FileBugData(**metadata).asDict()}
+        job.job.complete()
+        return blob
+
     def makeLaunchpadService(self, person=None, version="devel"):
         if person is None:
             person = self.makePerson()