← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~stevenk/launchpad/bugs-information_type-ui-filebug into lp:launchpad

 

Steve Kowalik has proposed merging lp:~stevenk/launchpad/bugs-information_type-ui-filebug into lp:launchpad with lp:~stevenk/launchpad/bugs-information_type-ui-secrecy as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #933766 in Launchpad itself: "Update bug to use information_visibility_policy"
  https://bugs.launchpad.net/launchpad/+bug/933766

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/bugs-information_type-ui-filebug/+merge/102635

Building on the changes introduced in https://code.launchpad.net/~stevenk/launchpad/bugs-information_type-ui-secrecy/+merge/102624, and the march forward to showing information_type in the UI, show it on Bug:+filebug. As before, I've left the existing tests alone and added new ones which set the feature flag.

This branch sort of closes bug 136937, but I'd rather not do that until the feature flag is on on production.

I have not moved the JavaScript in the filebug-macros template out to a .js file since I'd rather not make any far reaching changes in these branches to increase the chances we can beta the UI on qastaging next week.
-- 
https://code.launchpad.net/~stevenk/launchpad/bugs-information_type-ui-filebug/+merge/102635
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/bugs-information_type-ui-filebug into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2012-03-28 03:24:36 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2012-04-19 06:32:24 +0000
@@ -30,14 +30,15 @@
 from urlparse import urljoin
 
 from lazr.restful.interface import copy_field
+from lazr.restful.interfaces import IJSONRequestCache
 from pytz import timezone
 from simplejson import dumps
 from sqlobject import SQLObjectNotFound
 from z3c.ptcompat import ViewPageTemplateFile
-from zope import formlib
 from zope.app.form.browser import TextWidget
 from zope.app.form.interfaces import InputErrors
 from zope.component import getUtility
+from zope.formlib.form import Fields
 from zope.interface import (
     alsoProvides,
     implements,
@@ -45,15 +46,11 @@
     )
 from zope.publisher.interfaces import NotFound
 from zope.publisher.interfaces.browser import IBrowserPublisher
-from zope.schema import (
-    Bool,
-    Choice,
-    )
+from zope.schema import Choice
 from zope.schema.interfaces import TooLong
 from zope.schema.vocabulary import SimpleVocabulary
 from zope.security.proxy import removeSecurityProxy
 
-from lp import _
 from lp.app.browser.launchpadform import (
     action,
     custom_widget,
@@ -72,6 +69,7 @@
     ILaunchpadUsage,
     )
 from lp.app.validators.name import valid_name_pattern
+from lp.app.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
 from lp.app.widgets.product import (
     GhostCheckBoxWidget,
     GhostWidget,
@@ -117,7 +115,11 @@
     ProductConfigureBase,
     ProductPrivateBugsMixin,
     )
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+    InformationType,
+    PRIVATE_INFORMATION_TYPES,
+    SECURITY_INFORMATION_TYPES,
+    )
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage,
@@ -128,8 +130,12 @@
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.interfaces.projectgroup import IProjectGroup
 from lp.registry.interfaces.sourcepackage import ISourcePackage
-from lp.registry.vocabularies import ValidPersonOrTeamVocabulary
+from lp.registry.vocabularies import (
+    InformationTypeVocabulary,
+    ValidPersonOrTeamVocabulary,
+    )
 from lp.services.config import config
+from lp.services.features import getFeatureFlag
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.propertycache import cachedproperty
@@ -238,7 +244,7 @@
 class FileBugReportingGuidelines(LaunchpadFormView):
     """Provides access to common bug reporting attributes.
 
-    Attributes provided are: security_related and bug_reporting_guidelines.
+    Attributes provided are: information_type and bug_reporting_guidelines.
 
     This view is a superclass of `FileBugViewBase` so that non-ajax browsers
     can load the file bug form, and it is also invoked directly via an XHR
@@ -248,21 +254,56 @@
     schema = IBug
 
     @property
+    def show_information_type_in_ui(self):
+        return bool(getFeatureFlag(
+            'disclosure.show_information_type_in_ui.enabled'))
+
+    @property
     def field_names(self):
         """Return the list of field names to display."""
-        return ['security_related']
+        if self.show_information_type_in_ui:
+            return ['information_type']
+        else:
+            return ['security_related']
+
+    custom_widget('information_type', LaunchpadRadioWidgetWithDescription)
+
+    def initialize(self):
+        super(FileBugReportingGuidelines, self).initialize()
+        cache = IJSONRequestCache(self.request)
+        cache.objects['private_types'] = [
+            type.name for type in PRIVATE_INFORMATION_TYPES]
+        cache.objects['show_information_type_in_ui'] = (
+            self.show_information_type_in_ui)
 
     def setUpFields(self):
         """Set up the form fields. See `LaunchpadFormView`."""
         super(FileBugReportingGuidelines, self).setUpFields()
 
-        security_related_field = Bool(
-            __name__='security_related',
-            title=_("This bug is a security vulnerability"),
-            required=False, default=False)
+        if self.show_information_type_in_ui:
+            information_type_field = copy_field(
+                IBug['information_type'], readonly=False,
+                vocabulary=InformationTypeVocabulary())
+            self.form_fields = self.form_fields.omit('information_type')
+            self.form_fields += Fields(information_type_field)
+        else:
+            security_related_field = copy_field(
+                IBug['security_related'], readonly=False)
+            self.form_fields = self.form_fields.omit('security_related')
+            self.form_fields += Fields(security_related_field)
 
-        self.form_fields = self.form_fields.omit('security_related')
-        self.form_fields += formlib.form.Fields(security_related_field)
+    @property
+    def initial_values(self):
+        """See `LaunchpadFormView`."""
+        if self.show_information_type_in_ui:
+            value = InformationType.PUBLIC
+            if (
+                self.context and IProduct.providedBy(self.context) and
+                self.context.private_bugs):
+                value = InformationType.USERDATA
+            return {'information_type': value}
+        else:
+            return {}
 
     @property
     def bug_reporting_guidelines(self):
@@ -383,9 +424,15 @@
     def field_names(self):
         """Return the list of field names to display."""
         context = self.context
-        field_names = ['title', 'comment', 'tags', 'security_related',
-                       'bug_already_reported_as', 'filecontent', 'patch',
-                       'attachment_description', 'subscribe_to_existing_bug']
+        field_names = ['title', 'comment', 'tags']
+        if bool(getFeatureFlag(
+            'disclosure.show_information_type_in_ui.enabled')):
+            field_names.append('information_type')
+        else:
+            field_names.append('security_related')
+        field_names.extend([
+            'bug_already_reported_as', 'filecontent', 'patch',
+            'attachment_description', 'subscribe_to_existing_bug'])
         if (IDistribution.providedBy(context) or
             IDistributionSourcePackage.providedBy(context)):
             field_names.append('packagename')
@@ -533,7 +580,7 @@
             required=True, default=False)
 
         self.form_fields = self.form_fields.omit('subscribe_to_existing_bug')
-        self.form_fields += formlib.form.Fields(subscribe_field)
+        self.form_fields += Fields(subscribe_field)
 
     def contextUsesMalone(self):
         """Does the context use Malone as its official bugtracker?"""
@@ -562,7 +609,12 @@
         title = data["title"]
         comment = data["comment"].rstrip()
         packagename = data.get("packagename")
-        security_related = data.get("security_related", False)
+        if bool(getFeatureFlag(
+            'disclosure.show_information_type_in_ui.enabled')):
+            information_type = data.get(
+                "information_type", InformationType.PUBLIC)
+        else:
+            security_related = data.get("security_related", False)
         distribution = data.get(
             "distribution", getUtility(ILaunchBag).distribution)
 
@@ -581,13 +633,14 @@
         if self.request.form.get("packagename_option") == "none":
             packagename = None
 
-        # Security bugs are always embargoed when filed, but can be disclosed
-        # after they've been reported.
-        # This will change when the UI does.
-        if security_related:
-            information_type = InformationType.EMBARGOEDSECURITY
-        else:
-            information_type = InformationType.PUBLIC
+        if not bool(getFeatureFlag(
+            'disclosure.show_information_type_in_ui.enabled')):
+            # 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.EMBARGOEDSECURITY
+            else:
+                information_type = InformationType.PUBLIC
 
         linkified_ack = structured(FormattersAPI(
             self.getAcknowledgementMessage(self.context)).text_to_html(
@@ -624,7 +677,9 @@
             notifications.append(
                 'Additional information was added to the bug description.')
 
-        if extra_data.private:
+        if (not bool(getFeatureFlag(
+            'disclosure.show_information_type_in_ui.enabled')) and
+            extra_data.private):
             if params.information_type == InformationType.PUBLIC:
                 params.information_type = InformationType.USERDATA
 
@@ -716,14 +771,14 @@
         # Give the user some feedback on the bug just opened.
         for notification in notifications:
             self.request.response.addNotification(notification)
-        if bug.security_related:
+        if bug.information_type in SECURITY_INFORMATION_TYPES:
             self.request.response.addNotification(
                 structured(
                 'Security-related bugs are by default private '
                 '(visible only to their direct subscribers). '
                 'You may choose to <a href="+secrecy">publicly '
                 'disclose</a> this bug.'))
-        if bug.private and not bug.security_related:
+        elif bug.information_type in PRIVATE_INFORMATION_TYPES:
             self.request.response.addNotification(
                 structured(
                 'This bug report has been marked private '

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-04-19 06:32:24 +0000
@@ -1,9 +1,11 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
 
 
+from BeautifulSoup import BeautifulSoup
+from zope.component import getUtility
 from zope.schema.interfaces import (
     TooLong,
     TooShort,
@@ -14,12 +16,18 @@
     FileBugInlineFormView,
     FileBugViewBase,
     )
-from lp.bugs.interfaces.bug import IBugAddForm
+from lp.bugs.interfaces.bug import (
+    IBugAddForm,
+    IBugSet,
+    )
 from lp.bugs.publisher import BugsLayer
+from lp.registry.enums import InformationType
+from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
     login,
     login_person,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
@@ -397,6 +405,63 @@
             }]
         self.assertEqual(expected_guidelines, view.bug_reporting_guidelines)
 
+    def filebug_via_view(self, private_bugs=False, information_type=None):
+        feature_flag = {
+            'disclosure.show_information_type_in_ui.enabled': 'on'}
+        form = {
+            'field.title': 'A bug',
+            'field.comment': 'A comment',
+            'field.actions.submit_bug': 'Submit Bug Request',
+        }
+        if information_type:
+            form['field.information_type'] = information_type
+        product = self.factory.makeProduct(official_malone=True)
+        if private_bugs:
+            removeSecurityProxy(product).private_bugs = True
+        with FeatureFixture(feature_flag):
+            with person_logged_in(product.owner):
+                view = create_initialized_view(
+                    product, '+filebug', form=form, principal=product.owner)
+                bug_url = view.request.response.getHeader('Location')
+                bug_number = bug_url.split('/')[-1]
+                return getUtility(IBugSet).getByNameOrID(bug_number)
+
+    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()
+        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='EMBARGOEDSECURITY')
+        self.assertEqual(
+            InformationType.EMBARGOEDSECURITY, 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)
+        self.assertEqual(InformationType.USERDATA, bug.information_type)
+
+    def test_filebug_information_type_vocabulary(self):
+        # The vocabulary for information_type when filing a bug is created
+        # correctly.
+        feature_flags = {
+            'disclosure.show_information_type_in_ui.enabled': 'on',
+            'disclosure.proprietary_information_type.disabled': 'on',
+            'disclosure.display_userdata_as_private.enabled': 'on'}
+        product = self.factory.makeProduct(official_malone=True)
+        with FeatureFixture(feature_flags):
+            with person_logged_in(product.owner):
+                view = create_initialized_view(
+                    product, '+filebug', principal=product.owner)
+                html = view.render()
+                soup = BeautifulSoup(html)
+        self.assertEqual(u'Private', soup.find('label', text="Private"))
+        self.assertIs(None, soup.find('label', text="User Data"))
+        self.assertIs(None, soup.find('label', text="Proprietary"))
+
 
 class TestFileBugSourcePackage(TestCaseWithFactory):
 

=== modified file 'lib/lp/bugs/templates/bugtarget-filebug-guidelines.pt'
--- lib/lp/bugs/templates/bugtarget-filebug-guidelines.pt	2011-04-19 14:11:24 +0000
+++ lib/lp/bugs/templates/bugtarget-filebug-guidelines.pt	2012-04-19 06:32:24 +0000
@@ -11,7 +11,18 @@
   </tr>
 
   <tr tal:define="security_context view/getMainContext">
+  <tal:information_type tal:condition="view/show_information_type_in_ui">
     <td colspan="2" width="100%"
+        tal:define="widget nocall: view/widgets/information_type|nothing"
+        tal:condition="widget">
+      <label tal:attributes="for widget/name">
+        This bug contains information that is:
+      </label>
+      <input tal:replace="structure widget" />
+    </td>
+  </tal:information_type>
+  <tal:security_related tal:condition="not: view/show_information_type_in_ui">
+      <td colspan="2" width="100%"
         tal:define="widget nocall: view/widgets/security_related|nothing"
         tal:condition="widget">
       <table>
@@ -46,6 +57,7 @@
         </tbody>
       </table>
     </td>
+  </tal:security_related>
   </tr>
   </tbody></table></td></tr>
 </tal:root>

=== modified file 'lib/lp/bugs/templates/bugtarget-macros-filebug.pt'
--- lib/lp/bugs/templates/bugtarget-macros-filebug.pt	2012-03-10 13:48:37 +0000
+++ lib/lp/bugs/templates/bugtarget-macros-filebug.pt	2012-04-19 06:32:24 +0000
@@ -342,29 +342,48 @@
             });
         </script>
     </tal:private>
-    <tal:not-private condition="not:view/isPrivate">
+    <tal:flip-privacy>
         <script type="text/javascript">
-            LPJS.use('lp.app.privacy', 'lp.bugs.filebug_dupefinder', function(Y) {
+            var setup_information_type = function() {
+                var itypes_table = Y.one('.radio-button-widget');
+                itypes_table.delegate('click', function() {
+                    var private_type = (Y.Array.indexOf(
+                        LP.cache.private_types, this.get('value')) >= 0);
+                    if (private_type) {
+                        Y.lp.app.privacy.display_privacy_notification();
+                    } else {
+                        Y.lp.app.privacy.hide_privacy_notification();
+                    }
+                    }, "input[name='field.information_type']");
+            };
+            var setup_security_related = function() {
+                 var cfg = {
+                      notification_text: "This report will be private "
+                          + "because it is a security vulnerability. You "
+                          + "can disclose it later."
+                  };
+                  Y.lp.app.privacy.setup_privacy_notification(cfg);
+                  var sec = Y.one('[id="field.security_related"]');
+                  sec.on('change', function() {
+                      var checked = sec.get('checked');
+                      if (checked) {
+                          Y.lp.app.privacy.display_privacy_notification();
+                      } else {
+                          Y.lp.app.privacy.hide_privacy_notification();
+                          }
+                  });
+            };
+            LPJS.use('node-event-delegate', 'lp.app.privacy', function(Y) {
                 Y.on('domready', function() {
-                   var cfg = {
-                        notification_text: "This report will be private "
-                            + "because it is a security vulnerability. You "
-                            + "can disclose it later."
-                    };
-                    Y.lp.app.privacy.setup_privacy_notification(cfg);
-                    var sec = Y.one('[id="field.security_related"]');
-                    sec.on('change', function() {
-                        var checked = sec.get('checked');
-                        if (checked) {
-                            Y.lp.app.privacy.display_privacy_notification();
-                        } else {
-                            Y.lp.app.privacy.hide_privacy_notification();
-                        }
-                    });
+                    if (LP.cache.show_information_type_in_ui) {
+                        setup_information_type();
+                    } else {
+                        setup_security_related();
+                    }
                 });
             });
         </script>
-    </tal:not-private>
+    </tal:flip-privacy>
 </metal:privacy>
 
 <metal:similar-bugs define-macro="display-similar-bugs">


Follow ups