Merge lp:~wallyworld/launchpad/filebug-choice-widget-infotype-1006799 into lp:launchpad

[Ian Booth]

Enhance information type radio buttons on filebug page - replace with choice popup widget

== Implementation ==

Start with the js addPopupChoice() method added previously and create a version to handle radio button groups. Refactor out the common code. So now filebug will use choice popups for info type, importance, status. For info type, the description of the selected value is displayed as well.

The branch also contains a coupl eof drive bys. The main one is for the information_type_description() method in app/browser/informationtype.py. A previous branch fixed the description text but it is being generated in 2 places and I missed this one.

== Demo ==

http://people.canonical.com/~ianb/filebug-infotype.png

== Tests ==

Add new tests to file_bug yui tests
Add new tests to test_bugtarget_filebug

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/browser/informationtype.py
  lib/lp/app/javascript/choice.js
  lib/lp/bugs/browser/bugtarget.py
  lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
  lib/lp/bugs/javascript/filebug.js
  lib/lp/bugs/javascript/tests/test_filebug.js
  lib/lp/registry/vocabularies.py

[Abel Deuring (adeuring)]

looks good

review: Approve (code)

=== modified file 'lib/lp/app/browser/informationtype.py'
--- lib/lp/app/browser/informationtype.py	2012-05-30 20:31:37 +0000
+++ lib/lp/app/browser/informationtype.py	2012-06-02 13:58:21 +0000
@@ -59,5 +59,6 @@
         if (self.context.information_type == InformationType.USERDATA and
             self.show_userdata_as_private):
                 description = (
-                    description.replace('user data', 'private information'))
+                    'Visible only to users with whom the project has '
+                    'shared private information.')
         return description

=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js	2012-05-30 21:54:13 +0000
+++ lib/lp/app/javascript/choice.js	2012-06-02 13:58:21 +0000
@@ -56,17 +56,36 @@
   widget.render();
 };
 
-namespace.addPopupChoice = function(field_name, choices) {
-    var legacy_field_node = Y.one('[id=field.' + field_name + ']');
-    var initial_field_value = legacy_field_node.get('value');
-
+/**
+ * Replace a legacy input widget with a popup choice widget.
+ * @param legacy_node the YUI node containing the legacy widget.
+ * @param field_name the Launchpad form field name.
+ * @param choices the choices for the popup choice widget.
+ * @param show_description whether to show the selected value's description.
+ * @param get_value_fn getter for the legacy widget's value.
+ * @param set_value_fn setter for the legacy widget's value.
+ */
+var wirePopupChoice = function(legacy_node, field_name, choices,
+                               show_description, get_value_fn, set_value_fn) {
+    var choice_descriptions = {};
+    Y.Array.forEach(choices, function(item) {
+        choice_descriptions[item.value] = item.description;
+    });
+    var initial_field_value = get_value_fn(legacy_node);
     var choice_node = Y.Node.create([
         '<span id="' + field_name + '-content"><span class="value"></span>',
         '&nbsp;<a class="sprite edit editicon" href="#"></a></span>'
         ].join(' '));
+    if (show_description) {
+        choice_node.append(Y.Node.create('<div class="formHelp"></div>'));
+    }
 
-    legacy_field_node.insertBefore(choice_node, legacy_field_node);
-    legacy_field_node.addClass('unseen');
+    legacy_node.insertBefore(choice_node, legacy_node);
+    if (show_description) {
+        choice_node.one('.formHelp')
+            .set('text', choice_descriptions[initial_field_value]);
+    }
+    legacy_node.addClass('unseen');
     var field_content = Y.one('#' + field_name + '-content');
 
     var choice_edit = new Y.ChoiceSource({
@@ -91,8 +110,65 @@
     choice_edit.on('save', function(e) {
         var selected_value = choice_edit.get('value');
         update_selected_value_css(selected_value);
-        legacy_field_node.set('value', selected_value);
+        set_value_fn(legacy_node, selected_value);
+        if (show_description) {
+            choice_node.one('.formHelp')
+                .set('text', choice_descriptions[selected_value]);
+        }
     });
 };
 
-}, "0.1", {"requires": ["lazr.choiceedit", "lp.client.plugins"]});
+/**
+ * Replace a drop down combo box with a popup choice selection widget.
+ * @param field_name
+ * @param choices
+ * @param show_description
+ */
+namespace.addPopupChoice = function(field_name, choices, show_description) {
+    var legacy_node = Y.one('[id=field.' + field_name + ']');
+    var get_fn = function(node) {
+        return node.get('value');
+    };
+    var set_fn = function(node, value) {
+        node.set('value', value);
+    };
+    wirePopupChoice(
+        legacy_node, field_name, choices, show_description, get_fn, set_fn);
+};
+
+/**
+ * Replace a radio button group with a popup choice selection widget.
+ * @param field_name
+ * @param choices
+ * @param show_description
+ */
+namespace.addPopupChoiceForRadioButtons = function(field_name, choices,
+                                                   show_description) {
+
+    var legacy_node = Y.one('[name=field.' + field_name + ']')
+        .ancestor('table.radio-button-widget');
+    var get_fn = function(node) {
+        var field_value = choices[0].value;
+        node.all('input[name=field.' + field_name + ']').each(function(node) {
+            if (node.get('checked')) {
+                field_value = node.get('value');
+            }
+        });
+        return field_value;
+    };
+    var set_fn = function(node, value) {
+        node.all('input[name=field.' + field_name + ']')
+                .each(function(node) {
+            var node_selected = node.get('value') === value;
+            node.set('checked', node_selected);
+            if (node_selected) {
+                node.simulate('change');
+            }
+        });
+    };
+    wirePopupChoice(
+        legacy_node, field_name, choices, show_description, get_fn, set_fn);
+};
+
+}, "0.1", {"requires": ["lazr.choiceedit", "lp.client.plugins",
+    "node-event-simulate"]});

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2012-05-30 00:51:34 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2012-06-02 13:58:21 +0000
@@ -392,6 +392,11 @@
         cache.objects['enable_bugfiling_duplicate_search'] = (
             IProjectGroup.providedBy(self.context)
             or self.context.enable_bugfiling_duplicate_search)
+        cache.objects['information_type_data'] = [
+            {'value': term.name, 'description': term.description,
+            'name': term.title,
+            'description_css_class': 'choice-description'}
+            for term in InformationTypeVocabulary(self.context)]
         bugtask_status_data = vocabulary_to_choice_edit_items(
             BugTaskStatus, include_description=True, css_class_prefix='status',
             excluded_items=[

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-05-30 00:51:34 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-06-02 13:58:21 +0000
@@ -540,7 +540,7 @@
             'disclosure.enhanced_choice_popup.enabled': 'true'
         }))
 
-    def _assert_cache_values(self, view, duplicate_search):
+    def _assert_cache_values(self, view, duplicate_search, private_only=False):
         cache = IJSONRequestCache(view.request).objects
         self.assertEqual(
             duplicate_search, cache['enable_bugfiling_duplicate_search'])
@@ -587,6 +587,17 @@
             bugtask_importance_data.append(new_item)
         self.assertEqual(
             bugtask_importance_data, cache['bugtask_importance_data'])
+        bugtask_info_type_data = []
+        information_types = list(PRIVATE_INFORMATION_TYPES)
+        if not private_only:
+            information_types.extend(PUBLIC_INFORMATION_TYPES)
+        for item in information_types:
+            new_item = {'name': item.title, 'value': item.name,
+                        'description': item.description,
+                        'description_css_class': 'choice-description'}
+            bugtask_info_type_data.append(new_item)
+        self.assertContentEqual(
+            bugtask_info_type_data, cache['information_type_data'])
 
     def test_product(self):
         project = self.factory.makeProduct(official_malone=True)
@@ -595,6 +606,14 @@
         view = create_initialized_view(project, '+filebug', principal=user)
         self._assert_cache_values(view, True)
 
+    def test_product_private_bugs(self):
+        project = self.factory.makeProduct(
+            official_malone=True, private_bugs=True)
+        user = self.factory.makePerson()
+        login_person(user)
+        view = create_initialized_view(project, '+filebug', principal=user)
+        self._assert_cache_values(view, True, True)
+
     def test_product_no_duplicate_search(self):
         product = self.factory.makeProduct(official_malone=True)
         removeSecurityProxy(product).enable_bugfiling_duplicate_search = False

=== modified file 'lib/lp/bugs/browser/tests/test_bugview.py'
--- lib/lp/bugs/browser/tests/test_bugview.py	2012-05-17 09:45:29 +0000
+++ lib/lp/bugs/browser/tests/test_bugview.py	2012-06-02 13:58:21 +0000
@@ -79,7 +79,7 @@
             self.assertEqual('Private', view.information_type)
             self.assertTextMatchesExpressionIgnoreWhitespace(
                 'Visible only to users with whom the project has shared '
-                'information containing private information',
+                'private information.',
                 view.information_type_description)
 
     def test_proprietary_hidden(self):

=== modified file 'lib/lp/bugs/javascript/filebug.js'
--- lib/lp/bugs/javascript/filebug.js	2012-05-28 12:34:29 +0000
+++ lib/lp/bugs/javascript/filebug.js	2012-06-02 13:58:21 +0000
@@ -47,7 +47,7 @@
 
 var setup_information_type = function() {
     var itypes_table = Y.one('.radio-button-widget');
-    itypes_table.delegate('click', function() {
+    itypes_table.delegate('change', function() {
         var private_type = (Y.Array.indexOf(
             LP.cache.private_types, this.get('value')) >= 0);
         update_privacy_banner(private_type);
@@ -58,6 +58,8 @@
     Y.lp.app.choice.addPopupChoice('status', LP.cache.bugtask_status_data);
     Y.lp.app.choice.addPopupChoice(
         'importance', LP.cache.bugtask_importance_data);
+    Y.lp.app.choice.addPopupChoiceForRadioButtons(
+        'information_type', LP.cache.information_type_data, true);
 };
 
 var setup_security_related = function() {

=== modified file 'lib/lp/bugs/javascript/tests/test_filebug.js'
--- lib/lp/bugs/javascript/tests/test_filebug.js	2012-05-30 21:36:01 +0000
+++ lib/lp/bugs/javascript/tests/test_filebug.js	2012-06-02 13:58:21 +0000
@@ -14,6 +14,15 @@
                 links: {},
                 cache: {
                     private_types: ['EMBARGOEDSECURITY', 'USERDATA'],
+                    information_type_data: [
+                        {name: 'Public', value: 'PUBLIC',
+                            description: 'Public bug'},
+                        {name: 'Embargoed Security',
+                            value: 'EMBARGOEDSECURITY',
+                            description: 'Embargoed security bug'},
+                        {name: 'User Data', value: 'USERDATA',
+                            description: 'User Data bug'}
+                    ],
                     bugtask_status_data: [
                         {name: 'New', value: 'New'},
                         {name: 'Incomplete', value: 'Incomplete'}
@@ -64,8 +73,9 @@
                 'You can disclose it later.', banner_text);
         },
 
-        // Selecting a private info type turns on the privacy banner.
-        test_select_private_info_type: function () {
+        // Selecting a private info type using the legacy radio buttons
+        // turns on the privacy banner.
+        test_legacy_select_private_info_type: function () {
             window.LP.cache.show_information_type_in_ui = true;
             Y.lp.bugs.filebug.setup_filebug(true);
             var banner_hidden = Y.one('.yui3-privacybanner-hidden');
@@ -79,11 +89,13 @@
                 'You can disclose it later.', banner_text);
         },
 
-        // Selecting a public info type turns off the privacy banner.
-        test_select_public_info_type: function () {
+        // Selecting a public info type using the legacy radio buttons
+        // turns off the privacy banner.
+        test_legacy_select_public_info_type: function () {
             window.LP.cache.show_information_type_in_ui = true;
             window.LP.cache.bug_private_by_default = true;
             Y.lp.bugs.filebug.setup_filebug(true);
+            Y.one('[id=field.information_type.2]').simulate('click');
             var banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNull(banner_hidden);
             Y.one('[id=field.information_type.0]').simulate('click');
@@ -159,6 +171,72 @@
             status_choice.simulate('click');
             var legacy_dropdown = Y.one('[id=field.importance]');
             Y.Assert.areEqual('High', legacy_dropdown.get('value'));
+        },
+
+        // The bugtask information_type choice popup is rendered.
+        test_information_type_setup: function () {
+            Y.lp.bugs.filebug.setup_filebug(true);
+            var information_type_node =
+                Y.one('#information_type-content .value');
+            Y.Assert.areEqual('Public', information_type_node.get('text'));
+            var information_type_node_edit_node =
+                Y.one('#information_type-content a.sprite.edit');
+            Y.Assert.isNotNull(information_type_node_edit_node);
+            var legacy_field = Y.one('table.radio-button-widget');
+            Y.Assert.isTrue(legacy_field.hasClass('unseen'));
+        },
+
+        // The bugtask information_type choice popup updates the form.
+        test_information_type_selection: function() {
+            Y.lp.bugs.filebug.setup_filebug(true);
+            var information_type_popup = Y.one('#information_type-content a');
+            information_type_popup.simulate('click');
+            var information_type_choice = Y.one(
+                '.yui3-ichoicelist-content a[href=#USERDATA]');
+            information_type_choice.simulate('click');
+            var legacy_radio_button = Y.one('[id=field.information_type.3]');
+            Y.Assert.isTrue(legacy_radio_button.get('checked'));
+        },
+
+        // Selecting a private info type using the popup choice widget
+        // turns on the privacy banner.
+        test_select_private_info_type: function () {
+            window.LP.cache.show_information_type_in_ui = true;
+            Y.lp.bugs.filebug.setup_filebug(true);
+            var banner_hidden = Y.one('.yui3-privacybanner-hidden');
+            Y.Assert.isNotNull(banner_hidden);
+            var information_type_popup = Y.one('#information_type-content a');
+            information_type_popup.simulate('click');
+            var information_type_choice = Y.one(
+                '.yui3-ichoicelist-content a[href=#USERDATA]');
+            information_type_choice.simulate('click');
+            banner_hidden = Y.one('.yui3-privacybanner-hidden');
+            Y.Assert.isNull(banner_hidden);
+            var banner_text = Y.one('.banner-text').get('text');
+            Y.Assert.areEqual(
+                'This report will be private. ' +
+                'You can disclose it later.', banner_text);
+        },
+
+        // Selecting a public info type using the popup choice widget
+        // turns off the privacy banner.
+        test_select_public_info_type: function () {
+            window.LP.cache.show_information_type_in_ui = true;
+            window.LP.cache.bug_private_by_default = true;
+            Y.lp.bugs.filebug.setup_filebug(true);
+            var information_type_popup = Y.one('#information_type-content a');
+            information_type_popup.simulate('click');
+            var information_type_choice = Y.one(
+                '.yui3-ichoicelist-content a[href=#USERDATA]');
+            information_type_choice.simulate('click');
+            var banner_hidden = Y.one('.yui3-privacybanner-hidden');
+            Y.Assert.isNull(banner_hidden);
+            information_type_popup.simulate('click');
+            information_type_choice = Y.one(
+                '.yui3-ichoicelist-content a[href=#PUBLIC]');
+            information_type_choice.simulate('click');
+            banner_hidden = Y.one('.yui3-privacybanner-hidden');
+            Y.Assert.isNotNull(banner_hidden);
         }
     }));
 

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2012-06-01 10:17:13 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2012-06-02 13:58:21 +0000
@@ -989,9 +989,9 @@
         information_type = soup.find('strong')
         description = soup.find('div', id='information-type-description')
         self.assertEqual('User Data', information_type.renderContents())
-        self.assertEqual(
+        self.assertTextMatchesExpressionIgnoreWhitespace(
             'Visible only to users with whom the project has shared '
-            'information\ncontaining user data.\n',
+            'information containing user data.',
             description.renderContents())
 
     def test_information_type_in_ui_with_display_as_private(self):
@@ -1011,7 +1011,7 @@
         information_type = soup.find('strong')
         description = soup.find('div', id='information-type-description')
         self.assertEqual('Private', information_type.renderContents())
-        self.assertEqual(
+        self.assertTextMatchesExpressionIgnoreWhitespace(
             'Visible only to users with whom the project has shared '
-            'information\ncontaining private information.\n',
+            'private information.',
             description.renderContents())

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-06-01 02:18:27 +0000
+++ lib/lp/registry/vocabularies.py	2012-06-02 13:58:21 +0000
@@ -2250,8 +2250,8 @@
         if (context is None or
             not IProduct.providedBy(context) or
             not context.private_bugs):
-            types.insert(0, InformationType.UNEMBARGOEDSECURITY)
-            types.insert(0, InformationType.PUBLIC)
+            types = [InformationType.PUBLIC,
+                     InformationType.UNEMBARGOEDSECURITY] + types
 
         terms = []
         for type in types: