launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/rework-bug-default-type-0 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1031148 in Launchpad itself: "Bugs default to InformationType.PUBLIC everywhere"
  https://bugs.launchpad.net/launchpad/+bug/1031148

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/rework-bug-default-type-0/+merge/117378

This branch is the first in a series of maybe four to rework bug information type defaults, which will eventually allow us to default to Proprietary when a project is so configured.

The web, mail, and XML-RPC interfaces each had their own special variety of manually overriding information types when creating bugs. And they all tended to set it to PUBLIC when the overrides didn't apply, rather than leaving it for the core createBug API to figure out what it should be. This led to createBug containing this wonderful piece of logic:

         if params.product and params.product.private_bugs:
             # If the private_bugs flag is set on a product, then
             # force the new bug report to be private.
             if params.information_type == InformationType.PUBLIC:
                 params.information_type = InformationType.USERDATA

"You wanted it public? Ha, I know better! Even if you are the bug supervisor and explicitly selected Public."

I've changed default through most of the code to None. createBug understands None to mean that the target's default should be used -- currently Private if private_bugs is set and Public otherwise, but soon to be a little more complex than that. View code that wants to override it in some circumstances (eg. because apport has asked for something to be private, or the user has selected the "This bug is a security vulnerability" checkbox) sets it explicitly, but otherwise leaves it None.

FileBugViewBase is still a little messy, but rework-bug-default-type-2 sorts that out.
-- 
https://code.launchpad.net/~wgrant/launchpad/rework-bug-default-type-0/+merge/117378
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/rework-bug-default-type-0 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2012-07-26 07:55:19 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2012-07-31 06:32:19 +0000
@@ -636,12 +636,15 @@
         title = data["title"]
         comment = data["comment"].rstrip()
         packagename = data.get("packagename")
-        information_type = data.get(
-            "information_type", InformationType.PUBLIC)
-        security_related = data.get("security_related", False)
         distribution = data.get(
             "distribution", getUtility(ILaunchBag).distribution)
 
+        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
+
         context = self.context
         if distribution is not None:
             # We're being called from the generic bug filing form, so
@@ -657,14 +660,6 @@
         if self.request.form.get("packagename_option") == "none":
             packagename = None
 
-        if not self.is_bug_supervisor:
-            # If the old UI is enabled, security bugs are always embargoed
-            # when filed, but can be disclosed after they've been reported.
-            if security_related:
-                information_type = InformationType.PRIVATESECURITY
-            else:
-                information_type = InformationType.PUBLIC
-
         linkified_ack = structured(FormattersAPI(
             self.getAcknowledgementMessage(self.context)).text_to_html(
                 last_paragraph_class="last"))
@@ -701,7 +696,7 @@
                 'Additional information was added to the bug description.')
 
         if not self.is_bug_supervisor and extra_data.private:
-            if params.information_type == InformationType.PUBLIC:
+            if params.information_type in (None, InformationType.PUBLIC):
                 params.information_type = InformationType.USERDATA
 
         # Apply any extra options given by privileged users.

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2012-07-30 00:05:44 +0000
+++ lib/lp/bugs/interfaces/bug.py	2012-07-31 06:32:19 +0000
@@ -101,9 +101,9 @@
 
     def __init__(self, owner, title, comment=None, description=None,
                  msg=None, status=None, datecreated=None,
-                 information_type=InformationType.PUBLIC, subscribers=(),
-                 tags=None, subscribe_owner=True, filed_by=None,
-                 importance=None, milestone=None, assignee=None, cve=None):
+                 information_type=None, subscribers=(), tags=None,
+                 subscribe_owner=True, filed_by=None, importance=None,
+                 milestone=None, assignee=None, cve=None):
         self.owner = owner
         self.title = title
         self.comment = comment
@@ -213,7 +213,7 @@
     information_type = exported(
         Choice(
             title=_('Information Type'), vocabulary=InformationType,
-            required=True, readonly=True, default=InformationType.PUBLIC,
+            required=True, readonly=True,
             description=_(
                 'The type of information contained in this bug report.')))
 

=== modified file 'lib/lp/bugs/mail/commands.py'
--- lib/lp/bugs/mail/commands.py	2012-07-17 06:34:59 +0000
+++ lib/lp/bugs/mail/commands.py	2012-07-31 06:32:19 +0000
@@ -46,7 +46,10 @@
     IllegalTarget,
     )
 from lp.bugs.interfaces.cve import ICveSet
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+    InformationType,
+    PUBLIC_INFORMATION_TYPES,
+    )
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage,
@@ -185,12 +188,14 @@
                 stop_processing=True)
 
         if isinstance(context, CreateBugParams):
-            if private and (
-                context.information_type == InformationType.PUBLIC):
-                context.information_type = InformationType.USERDATA
-            elif (
-                context.information_type !=
-                InformationType.PRIVATESECURITY):
+            if private:
+                # "private yes" forces it to Private if it isn't already.
+                if (context.information_type is None
+                    or context.information_type in PUBLIC_INFORMATION_TYPES):
+                    context.information_type = InformationType.USERDATA
+            elif context.information_type != InformationType.PRIVATESECURITY:
+                # "private no" forces it to Public, except we always
+                # force new security bugs to be private.
                 context.information_type = InformationType.PUBLIC
             return context, current_event
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-07-30 00:05:44 +0000
+++ lib/lp/bugs/model/bug.py	2012-07-31 06:32:19 +0000
@@ -2645,11 +2645,13 @@
         # of its attribute values below.
         params = snapshot_bug_params(bug_params)
 
-        if params.product and params.product.private_bugs:
+        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.information_type == InformationType.PUBLIC:
+            if params.product and params.product.private_bugs:
                 params.information_type = InformationType.USERDATA
+            else:
+                params.information_type = InformationType.PUBLIC
 
         bug, event = self._makeBug(params)
 

=== modified file 'lib/lp/bugs/xmlrpc/bug.py'
--- lib/lp/bugs/xmlrpc/bug.py	2012-07-17 06:34:59 +0000
+++ lib/lp/bugs/xmlrpc/bug.py	2012-07-31 06:32:19 +0000
@@ -107,7 +107,7 @@
         if security_related:
             information_type = InformationType.PRIVATESECURITY
         else:
-            information_type = InformationType.PUBLIC
+            information_type = None
 
         params = CreateBugParams(
             owner=self.user, title=summary, comment=comment,

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-07-13 14:26:36 +0000
+++ lib/lp/testing/factory.py	2012-07-31 06:32:19 +0000
@@ -1652,10 +1652,10 @@
         return branch.createBranchRevision(sequence, revision)
 
     def makeBug(self, product=None, owner=None, bug_watch_url=None,
-                information_type=InformationType.PUBLIC, date_closed=None,
-                title=None, date_created=None, description=None,
-                comment=None, status=None, distribution=None, milestone=None,
-                series=None, tags=None, sourcepackagename=None):
+                information_type=None, date_closed=None, title=None,
+                date_created=None, description=None, comment=None,
+                status=None, distribution=None, milestone=None, series=None,
+                tags=None, sourcepackagename=None):
         """Create and return a new, arbitrary Bug.
 
         The bug returned uses default values where possible. See