Merge lp:~stevenk/launchpad/bugsubscriptionfilter-itype-ui into lp:launchpad

[Steve Kowalik]

Hook up the UI to allow people to filter structural subscriptions by the information type of a bug.

Hook up the UI to allow people to filter structural subscriptions by the information type of a bug.

I have performed a little bit of drive-by, by fixing some whitespace where I noticed it, and changing a few functions, just trying to keep the LOC count down. And then the JS happened.

Screenshot at http://people.canonical.com/~stevenk/information_type_filter.jpg

[Ian Booth (wallyworld)]

The screenshot looks nice. Thanks for making the little tweaks asked for.

review: Approve

=== modified file 'lib/lp/bugs/browser/bugsubscriptionfilter.py'
--- lib/lp/bugs/browser/bugsubscriptionfilter.py	2012-03-13 00:45:33 +0000
+++ lib/lp/bugs/browser/bugsubscriptionfilter.py	2012-08-07 07:08:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """View classes for bug subscription filters."""
@@ -70,6 +70,13 @@
     # in particular, if no importances are checked, or no statuses.
     filters_everything = False
 
+    def _add_english_condition(self, conditions, variable, description):
+        if len(variable) > 0:
+            conditions.append(
+                u"the %s is %s" % (description, english_list(
+                    (kind.title for kind in sorted(variable)),
+                    conjunction=u"or")))
+
     @property
     def conditions(self):
         """Descriptions of the bug subscription filter's conditions."""
@@ -80,24 +87,18 @@
                 'the bug')
             conditions.append(
                 mapping[bug_notification_level].lower()[:-1])
-        statuses = self.context.statuses
-        if len(statuses) > 0:
-            conditions.append(
-                u"the status is %s" % english_list(
-                    (status.title for status in sorted(statuses)),
-                    conjunction=u"or"))
-        importances = self.context.importances
-        if len(importances) > 0:
-            conditions.append(
-                u"the importance is %s" % english_list(
-                    (importance.title for importance in sorted(importances)),
-                    conjunction=u"or"))
+        self._add_english_condition(
+            conditions, self.context.statuses, 'status')
+        self._add_english_condition(
+            conditions, self.context.importances, 'importance')
         tags = self.context.tags
         if len(tags) > 0:
             conditions.append(
                 u"the bug is tagged with %s" % english_list(
                     sorted(tags), conjunction=(
                         u"and" if self.context.find_all_tags else u"or")))
+        self._add_english_condition(
+            conditions, self.context.information_types, 'information type')
         return conditions
 
 
@@ -110,6 +111,7 @@
         "description",
         "statuses",
         "importances",
+        "information_types",
         "tags",
         "find_all_tags",
         )
@@ -117,6 +119,7 @@
     custom_widget("description", TextWidget, displayWidth=50)
     custom_widget("statuses", LabeledMultiCheckBoxWidget)
     custom_widget("importances", LabeledMultiCheckBoxWidget)
+    custom_widget("information_types", LabeledMultiCheckBoxWidget)
     custom_widget("tags", BugTagsFrozenSetWidget, displayWidth=35)
 
     # Define in concrete subclass to be the target of the

=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py	2012-03-18 01:29:07 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py	2012-08-07 07:08:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -52,6 +52,7 @@
     IStructuralSubscriptionTarget,
     IStructuralSubscriptionTargetHelper,
     )
+from lp.registry.enums import InformationType
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage,
@@ -210,8 +211,7 @@
         Returns True is the user is subscribed to bug notifications
         for the context target.
         """
-        subscription = self.context.getSubscription(person)
-        return subscription is not None
+        return self.context.getSubscription(person) is not None
 
     def currentUserIsSubscribed(self):
         """Return True, if the current user is subscribed."""
@@ -415,6 +415,7 @@
     expose_user_administered_teams_to_js(request, user, context)
     expose_enum_to_js(request, BugTaskImportance, 'importances')
     expose_enum_to_js(request, BugTaskStatus, 'statuses')
+    expose_enum_to_js(request, InformationType, 'information_types')
     if subscriptions is None:
         try:
             # No subscriptions, which means we are on a target
@@ -432,10 +433,7 @@
 
 def expose_enum_to_js(request, enum, name):
     """Make a list of enum titles and value available to JavaScript."""
-    info = []
-    for item in enum:
-        info.append(item.title)
-    IJSONRequestCache(request).objects[name] = info
+    IJSONRequestCache(request).objects[name] = [item.title for item in enum]
 
 
 def expose_user_administered_teams_to_js(request, user, context,

=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py'
--- lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py	2012-03-13 00:45:33 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py	2012-08-07 07:08:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 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).
 
 """Tests for bug subscription filter browser code."""
@@ -22,6 +22,7 @@
     BugTaskImportance,
     BugTaskStatus,
     )
+from lp.registry.enums import InformationType
 from lp.services.webapp.publisher import canonical_url
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
@@ -320,6 +321,17 @@
             [u"the bug is tagged with *, bar, and foo"],
             self.view.conditions)
 
+    def test_conditions_for_information_types(self):
+        # If no information types have been specified nothing is returned.
+        self.assertEqual([], self.view.conditions)
+        # If set, a description of the information type is returned.
+        with person_logged_in(self.owner):
+            self.subscription_filter.information_types = [
+                InformationType.PRIVATESECURITY, InformationType.USERDATA]
+        self.assertEqual(
+            [u"the information type is Private Security or Private"],
+            self.view.conditions)
+
     def assertRender(self, dt_content=None, dd_content=None):
         root = html.fromstring(self.view.render())
         if dt_content is not None:
@@ -435,6 +447,7 @@
             "field.description": "New description",
             "field.statuses": ["NEW", "INCOMPLETE"],
             "field.importances": ["LOW", "MEDIUM"],
+            "field.information_types": ["USERDATA"],
             "field.tags": u"foo bar",
             "field.find_all_tags": "on",
             "field.actions.update": "Update",
@@ -445,8 +458,7 @@
             self.assertEqual([], view.errors)
         # The subscription filter has been updated.
         self.assertEqual(
-            u"New description",
-            self.subscription_filter.description)
+            u"New description", self.subscription_filter.description)
         self.assertEqual(
             frozenset([BugTaskStatus.NEW, BugTaskStatus.INCOMPLETE]),
             self.subscription_filter.statuses)
@@ -454,10 +466,11 @@
             frozenset([BugTaskImportance.LOW, BugTaskImportance.MEDIUM]),
             self.subscription_filter.importances)
         self.assertEqual(
-            frozenset([u"foo", u"bar"]),
-            self.subscription_filter.tags)
-        self.assertTrue(
-            self.subscription_filter.find_all_tags)
+            frozenset([InformationType.USERDATA]),
+            self.subscription_filter.information_types)
+        self.assertEqual(
+            frozenset([u"foo", u"bar"]), self.subscription_filter.tags)
+        self.assertTrue(self.subscription_filter.find_all_tags)
 
     def test_delete(self):
         # The filter can be deleted by using the delete action.
@@ -551,6 +564,7 @@
             "field.description": "New description",
             "field.statuses": ["NEW", "INCOMPLETE"],
             "field.importances": ["LOW", "MEDIUM"],
+            "field.information_types": ["PRIVATESECURITY"],
             "field.tags": u"foo bar",
             "field.find_all_tags": "on",
             "field.actions.create": "Create",
@@ -573,7 +587,8 @@
             frozenset([BugTaskImportance.LOW, BugTaskImportance.MEDIUM]),
             subscription_filter.importances)
         self.assertEqual(
-            frozenset([u"foo", u"bar"]),
-            subscription_filter.tags)
-        self.assertTrue(
-            subscription_filter.find_all_tags)
+            frozenset([InformationType.PRIVATESECURITY]),
+            subscription_filter.information_types)
+        self.assertEqual(
+            frozenset([u"foo", u"bar"]), subscription_filter.tags)
+        self.assertTrue(subscription_filter.find_all_tags)

=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js	2012-07-20 22:48:23 +0000
+++ lib/lp/registry/javascript/structural-subscription.js	2012-08-07 07:08:19 +0000
@@ -1,4 +1,4 @@
-/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+/* Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
  * GNU Affero General Public License version 3 (see the file LICENSE).
  *
  * Form overlay widgets and subscriber handling for structural subscriptions.
@@ -103,7 +103,8 @@
         tags: [],
         find_all_tags: false,
         importances: [],
-        statuses: []
+        statuses: [],
+        information_types: []
     };
 
     // Set the notification level.
@@ -137,10 +138,14 @@
         if (form_data.statuses.length > 0) {
             patch_data.statuses = form_data.statuses;
         }
+        if (form_data.information_types.length > 0) {
+            patch_data.information_types = form_data.information_types;
+        }
     } else {
         // clear out the tags, statuses, and importances in case this is an
         // edit.
         patch_data.tags = patch_data.importances = patch_data.statuses = [];
+        patch_data.information_types = [];
     }
     return patch_data;
 }
@@ -431,6 +436,8 @@
         content_node, LP.cache.statuses, LP.cache.statuses);
     set_checkboxes(
         content_node, LP.cache.importances, LP.cache.importances);
+    set_checkboxes(
+        content_node, LP.cache.information_types, LP.cache.information_types);
     content_node.one('[name="tags"]').set('value', '');
     set_radio_buttons(
         content_node, [MATCH_ALL, MATCH_ANY], MATCH_ALL);
@@ -556,7 +563,8 @@
 
     var statuses_ai,
         importances_ai,
-        tags_ai;
+        tags_ai,
+        information_types_ai;
 
     // Build tags pane.
     tags_ai = new Y.AccordionItem( {
@@ -644,6 +652,28 @@
     var official_bug_tags = LP.cache.context.official_bug_tags || [];
     namespace.bug_tag_completer = Y.lp.bugs.tags_entry.setup_tag_complete(
         'input[name="tags"]', official_bug_tags);
+
+    // Build information_types pane.
+    information_types_ai = new Y.AccordionItem( {
+        label: "Information types",
+        expanded: false,
+        alwaysVisible: false,
+        id: "information_types_ai",
+        contentHeight: {method: "auto"}
+    } );
+    var information_types = LP.cache.information_types;
+    selectors = make_selector_controls('information_types');
+    information_types_ai.set("bodyContent",
+        Y.Node.create('<div id="information_types-wrapper"></div>')
+            .append(selectors.node)
+            .append(make_table(information_types, 'information_types', 3)));
+    accordion.addItem(information_types_ai);
+    // Wire up the 'all' and 'none' selectors.
+    node = content_node.one('#information_types-wrapper');
+    selectors.all_link.on('click',
+        make_select_handler(node, information_types, true));
+    selectors.none_link.on('click',
+        make_select_handler(node, information_types, false));
     return accordion;
 }
 
@@ -1042,10 +1072,12 @@
 function set_filter_statuses_and_importances(content_node, filter) {
     var is_lifecycle = filter.bug_notification_level==='Lifecycle',
         statuses = filter.statuses,
-        importances = filter.importances;
+        importances = filter.importances,
+        information_types = filter.information_types;
     if (is_lifecycle) {
         statuses = LP.cache.statuses;
         importances = LP.cache.importances;
+        information_types = LP.cache.information_types;
     } else {
         // An absence of values is equivalent to all values.
         if (statuses.length === 0) {
@@ -1054,10 +1086,15 @@
         if (importances.length === 0) {
             importances = LP.cache.importances;
         }
+        if (information_types.length === 0) {
+            information_types = LP.cache.information_types;
+        }
     }
     set_checkboxes(content_node, LP.cache.statuses, statuses);
     set_checkboxes(
         content_node, LP.cache.importances, importances);
+    set_checkboxes(
+        content_node, LP.cache.information_types, information_types);
 }
 
 /**
@@ -1082,6 +1119,7 @@
         has_advanced_filters = !is_lifecycle && (
             filter.statuses.length ||
                 filter.importances.length ||
+                filter.information_types.length ||
                 filter.tags.length) > 0,
         filters = has_advanced_filters ? [ADVANCED_FILTER] : [],
         event = ADDED_OR_CHANGED;
@@ -1545,14 +1583,21 @@
     // Format status conditions.
     if (filter.statuses.length !== 0) {
         filter_items.push(Y.Node.create('<li></li>')
-            .set('text', 'have the status(es): '+filter.statuses.join(', ')));
+            .set('text', 'have the status(es): ' + filter.statuses.join(', ')));
     }
 
     // Format importance conditions.
     if (filter.importances.length !== 0) {
         filter_items.push(Y.Node.create('<li></li>')
             .set('text',
-                'are of importance: '+filter.importances.join(', ')));
+                'are of importance: ' + filter.importances.join(', ')));
+    }
+
+    // Format information type conditions.
+    if (filter.information_types.length !== 0) {
+        filter_items.push(Y.Node.create('<li></li>')
+            .set('text',
+                'are of information type: ' + filter.information_types.join(', ')));
     }
 
     // Format tag conditions.

=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js	2012-07-31 06:14:21 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js	2012-08-07 07:08:19 +0000
@@ -86,6 +86,9 @@
                                'Invalid', 'Won\'t Fix', 'Expired',
                                'Confirmed', 'Triaged', 'In Progress',
                                'Fix Committed', 'Fix Released', 'Unknown'];
+          LP.cache.information_types = ['Public', 'Public Security',
+                                        'Private Security', 'Private',
+                                        'Proprietary'];
           LP.links.me = 'https://launchpad.dev/api/~someone';
           return original_lp;
     }
@@ -124,6 +127,7 @@
             LP.cache.administratedTeams = [];
             LP.cache.importances = [];
             LP.cache.statuses = [];
+            LP.cache.information_types = [];
 
             this.configuration = {
                 content_box: content_box_id
@@ -226,6 +230,7 @@
             LP.cache.administratedTeams = [];
             LP.cache.importances = [];
             LP.cache.statuses = [];
+            LP.cache.information_types = [];
 
             this.configuration = {
                 content_box: content_box_id
@@ -357,6 +362,7 @@
             LP.cache.administratedTeams = [];
             LP.cache.importances = [];
             LP.cache.statuses = [];
+            LP.cache.information_types = [];
             LP.links.me = 'https://launchpad.dev/api/~someone';
 
             var lp_client = function() {};
@@ -417,6 +423,7 @@
             LP.cache.administratedTeams = [];
             LP.cache.importances = [];
             LP.cache.statuses = [];
+            LP.cache.information_types = [];
             LP.links.me = 'https://launchpad.dev/api/~someone';
 
             var lp_client = function() {};
@@ -702,6 +709,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -737,6 +745,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -775,6 +784,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -815,6 +825,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -1015,7 +1026,8 @@
                 tags: [' one two THREE '],
                 tag_match: [''],
                 importances: [],
-                statuses: []
+                statuses: [],
+                information_types: []
             };
             var patch_data = module._extract_form_data(form_data);
             // Note that the tags are converted to lower case
@@ -1032,7 +1044,8 @@
                 tags: ['tag'],
                 tag_match: ['match-all'],
                 importances: [],
-                statuses: []
+                statuses: [],
+                information_types: []
             };
             var patch_data = module._extract_form_data(form_data);
             Assert.isTrue(patch_data.find_all_tags);
@@ -1046,7 +1059,8 @@
                 tags: ['tag'],
                 tag_match: [],
                 importances: [],
-                statuses: []
+                statuses: [],
+                information_types: []
             };
             var patch_data = module._extract_form_data(form_data);
             Assert.isFalse(patch_data.find_all_tags);
@@ -1063,7 +1077,8 @@
                 tags: ['tag'],
                 tag_match: ['match-all'],
                 importances: ['importance1'],
-                statuses: ['status1']
+                statuses: ['status1'],
+                information_types: ['informationtype1']
             };
             var patch_data = module._extract_form_data(form_data);
             // Since advanced-filter isn't set, all the advanced values should
@@ -1072,6 +1087,7 @@
             ArrayAssert.isEmpty(patch_data.tags);
             ArrayAssert.isEmpty(patch_data.importances);
             ArrayAssert.isEmpty(patch_data.statuses);
+            ArrayAssert.isEmpty(patch_data.information_types);
         }
 
     }));
@@ -1092,6 +1108,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion'
@@ -1183,6 +1200,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion'
@@ -1251,6 +1269,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -1353,6 +1372,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -1513,6 +1533,7 @@
                     return {
                         importances: [],
                         statuses: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -1561,6 +1582,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -1580,6 +1602,7 @@
                     return {
                         importances: [],
                         statuses: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -1621,6 +1644,7 @@
                     {filter: {
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',
@@ -1770,6 +1794,7 @@
                         description: 'DESCRIPTION',
                         statuses: [],
                         importances: [],
+                        information_types: [],
                         tags: [],
                         find_all_tags: true,
                         bug_notification_level: 'Discussion',