Merge lp:~wallyworld/launchpad/expose-bug-infotype-search-66206 into lp:launchpad

[Ian Booth]

Update bugtask search UI and webservice API to allow searching bugtasks using information type to filter.

== Implementation ==

This branch allows to advanced bugtask search form and webservice api to be used to search for bugtasks filtering on information type. On the ui, like for Importance and Status, the Information Type checkboxes are all unclicked by default (meaning no filtering). The Proprietary option appears as necessary for projects with commercial subscriptions (if feature flag allows).

There was some whitespace to the right of the status and importance checkboxes on the advanaced bugtask search form so I added the information type checkboxes there. It seems to work well even on narrow screens.

The search backend uses the work done in the previous branch. I made some tweaks to improve the usability. eg the caller can just pass in an iterable of info types rather than having to construct an any().

== Demo ==

See screenshot:
http://people.canonical.com/~ianb/bugsearch-infotype.png

== Tests ==

I enhanced bugtask-search-views.txt to test the search view.
I added a new test case to test the search over the webservice: TestSearchByInformationType

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/browser/tests/bugtask-search-views.txt
  lib/lp/bugs/interfaces/bugtarget.py
  lib/lp/bugs/interfaces/bugtask.py
  lib/lp/bugs/model/bugtarget.py
  lib/lp/bugs/model/bugtasksearch.py
  lib/lp/bugs/templates/bugtask-macros-tableview.pt
  lib/lp/bugs/tests/test_bugtask_search.py
  lib/lp/bugs/tests/test_searchtasks_webservice.py
  lib/lp/code/model/branch.py
  lib/lp/registry/vocabularies.py

./lib/lp/bugs/browser/tests/bugtask-search-views.txt
       1: narrative uses a moin header.
     207: narrative uses a moin header.
     222: source exceeds 78 characters.
     286: narrative uses a moin header.
     326: source exceeds 78 characters.
     353: narrative uses a moin header.
     377: source exceeds 78 characters.
     380: source exceeds 78 characters.
     389: narrative uses a moin header.
     451: want exceeds 78 characters.
./lib/lp/bugs/interfaces/bugtask.py
     930: E302 expected 2 blank lines, found 1

[Curtis Hovey (sinzui)]

Thank you very much.

Can you fix the link in ./lib/lp/bugs/interfaces/bugtask.py?

review: Approve (code)

[Ian Booth (wallyworld)]

Sorry, I'm not sure I follow what you need fixed?

On 10/07/12 22:45, Curtis Hovey wrote:
> Review: Approve code
>
> Thank you very much.
>
> Can you fix the link in ./lib/lp/bugs/interfaces/bugtask.py?
>

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2012-07-10 15:39:46 +0000
+++ lib/lp/bugs/browser/bugtask.py	2012-07-11 01:24:24 +0000
@@ -241,7 +241,10 @@
 from lp.registry.interfaces.projectgroup import IProjectGroup
 from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.registry.model.personroles import PersonRoles
-from lp.registry.vocabularies import MilestoneVocabulary
+from lp.registry.vocabularies import (
+    InformationTypeVocabulary,
+    MilestoneVocabulary,
+    )
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
 from lp.services.feeds.browser import (
@@ -3103,6 +3106,11 @@
         """Return data used to render the Importance checkboxes."""
         return self.getWidgetValues(vocabulary=BugTaskImportance)
 
+    def getInformationTypeWidgetValues(self):
+        """Return data used to render the Information Type checkboxes."""
+        return self.getWidgetValues(
+            vocabulary=InformationTypeVocabulary(self.context))
+
     def getMilestoneWidgetValues(self):
         """Return data used to render the milestone checkboxes."""
         return self.getWidgetValues("Milestone")

=== modified file 'lib/lp/bugs/browser/tests/bugtask-search-views.txt'
--- lib/lp/bugs/browser/tests/bugtask-search-views.txt	2011-12-30 06:14:56 +0000
+++ lib/lp/bugs/browser/tests/bugtask-search-views.txt	2012-07-11 01:24:24 +0000
@@ -350,6 +350,42 @@
     Mozilla Firefox 1.0
 
 
+== Searching by information type ==
+
+The advanced form allows us to query for bugs matching specific
+information types.
+
+First we'll change the information type of one of the bugtasks (we are still
+logged in from earlier):
+
+    >>> from lp.registry.enums import InformationType
+    >>> previous_information_type = open_bugtasks[0].bug.information_type
+    >>> open_bugtasks[0].bug.transitionToInformationType(
+    ...     InformationType.USERDATA, getUtility(ILaunchBag).user)
+    True
+    >>> flush_database_updates()
+
+Submit the search:
+
+    >>> form_values = {
+    ...     'search': 'Search bugs in Firefox',
+    ...     'advanced': 1,
+    ...     'field.information_type': 'USERDATA',
+    ...     'field.searchtext': '',
+    ...     'field.orderby': '-importance'}
+
+    >>> mozilla_search_listingview = create_view(mozilla, '+bugs', form_values)
+    >>> userdata_bugtasks = list(mozilla_search_listingview.search().batch)
+    >>> for bugtask in userdata_bugtasks:
+    ...     print bugtask.bug.id, bugtask.product.name, bugtask.bug.information_type.name
+    15 thunderbird USERDATA
+
+    >>> open_bugtasks[0].bug.transitionToInformationType(
+    ...     previous_information_type, getUtility(ILaunchBag).user)
+    True
+    >>> flush_database_updates()
+
+
 == Constructing search filter urls ==
 
 There is a helper method, get_buglisting_search_filter_url(), which can

=== modified file 'lib/lp/bugs/interfaces/bugtarget.py'
--- lib/lp/bugs/interfaces/bugtarget.py	2012-06-14 21:50:59 +0000
+++ lib/lp/bugs/interfaces/bugtarget.py	2012-07-11 01:24:24 +0000
@@ -70,6 +70,7 @@
     "search_text": copy_field(IBugTaskSearch['searchtext']),
     "status": copy_field(IBugTaskSearch['status']),
     "importance": copy_field(IBugTaskSearch['importance']),
+    "information_type": copy_field(IBugTaskSearch['information_type']),
     "assignee": Reference(schema=Interface),
     "bug_reporter": Reference(schema=Interface),
     "bug_supervisor": Reference(schema=Interface),
@@ -258,7 +259,7 @@
                     hardware_is_linked_to_bug=False, linked_branches=None,
                     linked_blueprints=None, structural_subscriber=None,
                     modified_since=None, created_since=None,
-                    created_before=None):
+                    created_before=None, information_type=None):
         """Search the IBugTasks reported on this entity.
 
         :search_params: a BugTaskSearchParams object

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2012-07-11 01:24:24 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2012-07-11 01:24:24 +0000
@@ -924,6 +924,10 @@
     ])
 
 
+# Avoid circular imports
+from lp.registry.enums import InformationType
+
+
 class IBugTaskSearchBase(Interface):
     """The basic search controls."""
     searchtext = TextLine(title=_("Bug ID or search text."), required=False)
@@ -943,6 +947,14 @@
                       'or list of importances.'),
         value_type=IBugTask['importance'],
         required=False)
+    information_type = List(
+        title=_('Information Type'),
+        description=_('Show only bugs with the given information type '
+                      'or list of information types.'),
+        value_type=Choice(
+            title=_('Information Type'),
+            vocabulary=InformationType),
+        required=False)
     assignee = Choice(
         title=_('Assignee'),
         description=_('Person entity assigned for this bug.'),
@@ -1180,7 +1192,7 @@
                  created_since=None, exclude_conjoined_tasks=False, cve=None,
                  upstream_target=None, milestone_dateexpected_before=None,
                  milestone_dateexpected_after=None, created_before=None,
-                 information_types=None):
+                 information_type=None):
 
         self.bug = bug
         self.searchtext = searchtext
@@ -1234,12 +1246,12 @@
         self.upstream_target = upstream_target
         self.milestone_dateexpected_before = milestone_dateexpected_before
         self.milestone_dateexpected_after = milestone_dateexpected_after
-        if isinstance(information_types, collections.Iterable):
-            self.information_types = information_types
-        elif information_types:
-            self.information_types = (information_types,)
+        if isinstance(information_type, collections.Iterable):
+            self.information_type = set(information_type)
+        elif information_type:
+            self.information_type = set((information_type,))
         else:
-            self.information_types = None
+            self.information_type = None
 
     def setProduct(self, product):
         """Set the upstream context on which to filter the search."""
@@ -1385,7 +1397,7 @@
                        hardware_is_linked_to_bug=False, linked_branches=None,
                        linked_blueprints=None, structural_subscriber=None,
                        modified_since=None, created_since=None,
-                       created_before=None):
+                       created_before=None, information_type=None):
         """Create and return a new instance using the parameter list."""
         search_params = cls(user=user, orderby=order_by)
 
@@ -1457,6 +1469,7 @@
         search_params.modified_since = modified_since
         search_params.created_since = created_since
         search_params.created_before = created_before
+        search_params.information_type = information_type
 
         return search_params
 

=== modified file 'lib/lp/bugs/model/bugtarget.py'
--- lib/lp/bugs/model/bugtarget.py	2012-06-14 21:50:59 +0000
+++ lib/lp/bugs/model/bugtarget.py	2012-07-11 01:24:24 +0000
@@ -81,7 +81,8 @@
                     hardware_owner_is_subscribed_to_bug=False,
                     hardware_is_linked_to_bug=False, linked_branches=None,
                     linked_blueprints=None, modified_since=None,
-                    created_since=None, created_before=None):
+                    created_since=None, created_before=None,
+                    information_type=None):
         """See `IHasBugs`."""
         if status is None:
             # If no statuses are supplied, default to the

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2012-07-11 01:24:24 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2012-07-11 01:24:24 +0000
@@ -182,6 +182,8 @@
 
 def search_value_to_storm_where_condition(comp, search_value):
     """Convert a search value to a Storm WHERE condition."""
+    if zope_isinstance(search_value, (set, list, tuple)):
+        search_value = any(*search_value)
     if zope_isinstance(search_value, any):
         # When an any() clause is provided, the argument value
         # is a list of acceptable filter values.
@@ -734,9 +736,10 @@
         extra_clauses.append(
             BugTaskFlat.datecreated < params.created_before)
 
-    if params.information_types:
+    if params.information_type:
         extra_clauses.append(
-            BugTaskFlat.information_type.is_in(params.information_types))
+            search_value_to_storm_where_condition(
+                BugTaskFlat.information_type, params.information_type))
 
     query = And(extra_clauses)
 

=== modified file 'lib/lp/bugs/templates/bugtask-macros-tableview.pt'
--- lib/lp/bugs/templates/bugtask-macros-tableview.pt	2012-06-27 19:23:40 +0000
+++ lib/lp/bugs/templates/bugtask-macros-tableview.pt	2012-07-11 01:24:24 +0000
@@ -273,7 +273,7 @@
                 </tal:checkbox>
               </div>
             </td>
-            <td width="30%">
+            <td width="25%">
               <div><label>Importance:</label></div>
               <div tal:repeat="widget_value view/getImportanceWidgetValues">
                 <tal:checkbox
@@ -289,6 +289,22 @@
               </tal:checkbox>
             </div>
           </td>
+            <td width="35%">
+              <div><label>Information Type:</label></div>
+              <div tal:repeat="widget_value view/getInformationTypeWidgetValues">
+                <tal:checkbox
+                    define="widget_id string:information_type.${widget_value/title}">
+                    <input name="field.information_type:list"
+                      type="checkbox"
+                      tal:attributes="value widget_value/value;
+                                      checked widget_value/checked;
+                                      id widget_id"/>
+                    <label style="font-weight: normal"
+                      tal:content="widget_value/title"
+                      tal:attributes="for widget_id">Public</label>
+              </tal:checkbox>
+            </div>
+          </td>
         </tr>
       </table>
     </fieldset>

=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
--- lib/lp/bugs/tests/test_bugtask_search.py	2012-07-11 01:24:24 +0000
+++ lib/lp/bugs/tests/test_bugtask_search.py	2012-07-11 01:24:24 +0000
@@ -417,11 +417,11 @@
                 InformationType.EMBARGOEDSECURITY, self.owner)
         params = self.getBugTaskSearchParams(
             user=self.owner,
-            information_types=InformationType.EMBARGOEDSECURITY)
+            information_type=InformationType.EMBARGOEDSECURITY)
         self.assertSearchFinds(params, [self.bugtasks[2]])
         params = self.getBugTaskSearchParams(
             user=self.owner,
-            information_types=InformationType.UNEMBARGOEDSECURITY)
+            information_type=InformationType.UNEMBARGOEDSECURITY)
         self.assertSearchFinds(params, [])
 
     def test_omit_duplicate_bugs(self):

=== modified file 'lib/lp/bugs/tests/test_searchtasks_webservice.py'
--- lib/lp/bugs/tests/test_searchtasks_webservice.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/tests/test_searchtasks_webservice.py	2012-07-11 01:24:24 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+from lp.registry.enums import InformationType
 from lp.testing import (
     person_logged_in,
     TestCaseWithFactory,
@@ -81,3 +82,36 @@
         # validation is performed for the linked_blueprints parameter, and
         # thus no error is returned when we pass rubbish.
         self.search("beta", linked_blueprints="Teabags!")
+
+
+class TestSearchByInformationType(TestCaseWithFactory):
+    """Tests for the information_type parameter."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestSearchByInformationType, self).setUp()
+        self.owner = self.factory.makePerson()
+        with person_logged_in(self.owner):
+            self.product = self.factory.makeProduct()
+        self.bug = self.factory.makeBug(
+            product=self.product,
+            information_type=InformationType.EMBARGOEDSECURITY)
+        self.webservice = LaunchpadWebServiceCaller(
+            'launchpad-library', 'salgado-change-anything')
+
+    def search(self, api_version, **kwargs):
+        return self.webservice.named_get(
+            '/%s' % self.product.name, 'searchTasks',
+            api_version=api_version, **kwargs).jsonBody()
+
+    def test_search_returns_results(self):
+        # A matching search returns results.
+        response = self.search(
+            "devel", information_type="Embargoed Security")
+        self.assertEqual(response['total_size'], 1)
+
+    def test_search_returns_no_results(self):
+        # A non-matching search returns no results.
+        response = self.search("devel", information_type="User Data")
+        self.assertEqual(response['total_size'], 0)

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2012-07-11 01:24:24 +0000
+++ lib/lp/code/model/branch.py	2012-07-11 01:24:24 +0000
@@ -1318,7 +1318,7 @@
         # Branches linked to private bugs can be private.
         params = BugTaskSearchParams(
             user=user, linked_branches=self.id,
-            information_types=PRIVATE_INFORMATION_TYPES)
+            information_type=PRIVATE_INFORMATION_TYPES)
         bug_ids = getUtility(IBugTaskSet).searchBugIds(params)
         return bug_ids.count() > 0
 

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-07-11 01:24:24 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-07-11 01:24:24 +0000
@@ -402,6 +402,25 @@
         login_person(self.factory.makePerson())
         self._assert_getPillarShareesUnauthorized(product)
 
+    def _assert_sharee_data(self, expected, actual):
+        # Assert that the actual and expected sharee data is equal.
+        # Sharee data is a list of (sharee, permissions, info_types) tuples.
+        expected_list = list(expected)
+        actual_list = list(actual)
+        self.assertEqual(len(expected_list), len(list(actual_list)))
+
+        expected_sharee_map = {}
+        for data in expected_list:
+            expected_sharee_map[data[0]] = data[1:]
+        actual_sharee_map = {}
+        for data in actual_list:
+            actual_sharee_map[data[0]] = data[1:]
+
+        for sharee, expected_permissions, expected_info_types in expected:
+            actual_permissions, actual_info_types = actual_sharee_map[sharee]
+            self.assertContentEqual(expected_permissions, actual_permissions)
+            self.assertContentEqual(expected_info_types, actual_info_types)
+
     def _assert_sharePillarInformation(self, pillar, pillar_type=None):
         """sharePillarInformations works and returns the expected data."""
         sharee = self.factory.makePerson()
@@ -464,7 +483,7 @@
         expected_sharee_data = self._makeShareeData(
             sharee, expected_permissions,
             [InformationType.EMBARGOEDSECURITY, InformationType.USERDATA])
-        self.assertEqual(expected_sharee_data, sharee_data)
+        self.assertContentEqual(expected_sharee_data, sharee_data)
         # Check that getPillarSharees returns what we expect.
         if pillar_type == 'product':
             expected_sharee_grants = [
@@ -618,8 +637,7 @@
                 policy, SharingPermission.ALL) for policy in access_policies])
             owner_data = (pillar.owner, policy_permissions, [])
             expected_data.append(owner_data)
-            
-        self.assertContentEqual(
+        self._assert_sharee_data(
             expected_data, self.service.getPillarSharees(pillar))
 
     def test_deleteProductShareeAll(self):

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-07-11 01:24:24 +0000
+++ lib/lp/registry/vocabularies.py	2012-07-11 01:24:24 +0000
@@ -2276,8 +2276,8 @@
                     IProduct.providedBy(subscription_context) and
                     subscription_context.has_current_commercial_subscription)
                 already_proprietary = (
-                    safe_hasattr(context, 'information_type')
-                    and context.information_type == InformationType.PROPRIETARY)
+                    safe_hasattr(context, 'information_type') and
+                    context.information_type == InformationType.PROPRIETARY)
                 if has_commercial_subscription or already_proprietary:
                     types.append(InformationType.PROPRIETARY)
         # Disallow public items for projects with private bugs.