Merge lp:~brian-murray/launchpad/bug-320596 into lp:launchpad

[Brian Murray]

For the devel version of the API have omit_targeted searchTasks parameter default to False.

Remove the default value of True for omit_targeted so that bug tasks targeted to a milestone are no longer hidden in search results.

Tests modified:

bin/test -cvvt xx-bug.txt -t bugtask-search.txt

[Robert Collins (lifeless)]

So, I have some needs fixing and a question.

needs fixing: please don't add these things to doctests: none of your tests look like documentation, so they shouldn't be expressed as documentation.

question: looks like you're breaking the 1.0 API to me, I thought that wasn't permitted.

review: Needs Fixing

[Brian Murray (brian-murray)]

I think having the default value of omit_targeted be False in the API is actually broken. This is a different default value than the web interface and resulted in terribly confusing behavior - see bug 320596 (and its duplicates). However, I'll special case the default value of it if you think that is best.

[Robert Collins (lifeless)]

I don't hold the value to be sacred; however its my understanding that
we promised not to change the behaviour of the 1.0 API - we can change
devel, just not 1.0.

[Brian Murray (brian-murray)]

This is ready for review again. omit_targeted is left defaulting to True and in devel the default value is changed to False.

[Abel Deuring (adeuring)]

Hi Brian,

nice work! Just one minor nitpicks: I find the name "modifiable_params" and "modified_params" a bit confusing. What about "params_for_api_1_0" and "params_for_devel"? And since these variables are defined on mudole level, perhaps even "searchtasks_params_for_api_1_0"? (Having modified_params and modifiable_params for a second exported method might lead to funny results ;)

review: Approve (code)

=== modified file 'lib/lp/bugs/interfaces/bugtarget.py'
--- lib/lp/bugs/interfaces/bugtarget.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/interfaces/bugtarget.py	2010-08-26 20:46:45 +0000
@@ -29,11 +29,13 @@
     exported,
     LAZR_WEBSERVICE_EXPORTED,
     operation_parameters,
+    operation_removed_in_version,
     operation_returns_collection_of,
     REQUEST_USER,
     )
 from lazr.restful.fields import Reference
 from lazr.restful.interface import copy_field
+
 from zope.interface import (
     Attribute,
     Interface,
@@ -57,6 +59,117 @@
     )
 from lp.services.fields import Tag
 
+search_tasks_params_for_api_1_0 = {
+    "order_by": List(
+        title=_('List of fields by which the results are ordered.'),
+        value_type=Text(),
+        required=False),
+    "search_text": copy_field(IBugTaskSearch['searchtext']),
+    "status": copy_field(IBugTaskSearch['status']),
+    "importance": copy_field(IBugTaskSearch['importance']),
+    "assignee": Reference(schema=Interface),
+    "bug_reporter": Reference(schema=Interface),
+    "bug_supervisor": Reference(schema=Interface),
+    "bug_commenter": Reference(schema=Interface),
+    "bug_subscriber": Reference(schema=Interface),
+    "structural_subscriber": Reference(schema=Interface),
+    "owner": Reference(schema=Interface),
+    "affected_user": Reference(schema=Interface),
+    "has_patch": copy_field(IBugTaskSearch['has_patch']),
+    "has_cve": copy_field(IBugTaskSearch['has_cve']),
+    "tags": copy_field(IBugTaskSearch['tag']),
+    "tags_combinator": copy_field(IBugTaskSearch['tags_combinator']),
+    "omit_duplicates": copy_field(IBugTaskSearch['omit_dupes']),
+    "omit_targeted": copy_field(IBugTaskSearch['omit_targeted']),
+    "status_upstream": copy_field(IBugTaskSearch['status_upstream']),
+    "milestone_assignment": copy_field(IBugTaskSearch['milestone_assignment']),
+    "milestone": copy_field(IBugTaskSearch['milestone']),
+    "component": copy_field(IBugTaskSearch['component']),
+    "nominated_for": Reference(schema=Interface),
+    "has_no_package": copy_field(IBugTaskSearch['has_no_package']),
+    "hardware_bus": Choice(
+        title=_(u"The bus of a hardware device related to a bug"),
+        # The vocabulary should be HWBus; this is fixed in
+        # _schema_circular_imports to avoid circular imports.
+        vocabulary=DBEnumeratedType, required=False),
+    "hardware_vendor_id": TextLine(
+        title=_(
+            u"The vendor ID of a hardware device related to a bug."),
+        description=_(
+            u"Allowed values of the vendor ID depend on the bus of the "
+            "device.\n\n"
+            "Vendor IDs of PCI, PCCard and USB devices are hexadecimal "
+            "string representations of 16 bit integers in the format "
+            "'0x01ab': The prefix '0x', followed by exactly 4 digits; "
+            "where a digit is one of the characters 0..9, a..f. The "
+            "characters A..F are not allowed.\n\n"
+            "SCSI vendor IDs are strings with exactly 8 characters. "
+            "Shorter names are right-padded with space (0x20) characters."
+            "\n\n"
+            "IDs for other buses may be arbitrary strings."),
+        required=False),
+    "hardware_product_id": TextLine(
+        title=_(
+            u"The product ID of a hardware device related to a bug."),
+        description=_(
+            u"Allowed values of the product ID depend on the bus of the "
+            "device.\n\n"
+            "Product IDs of PCI, PCCard and USB devices are hexadecimal "
+            "string representations of 16 bit integers in the format "
+            "'0x01ab': The prefix '0x', followed by exactly 4 digits; "
+            "where a digit is one of the characters 0..9, a..f. The "
+            "characters A..F are not allowed.\n\n"
+            "SCSI product IDs are strings with exactly 16 characters. "
+            "Shorter names are right-padded with space (0x20) characters."
+            "\n\n"
+            "IDs for other buses may be arbitrary strings."),
+        required=False),
+    "hardware_driver_name": TextLine(
+        title=_(
+            u"The driver controlling a hardware device related to a "
+            "bug."),
+        required=False),
+    "hardware_driver_package_name": TextLine(
+        title=_(
+            u"The package of the driver which controls a hardware "
+            "device related to a bug."),
+        required=False),
+    "hardware_owner_is_bug_reporter": Bool(
+        title=_(
+            u"Search for bugs reported by people who own the given "
+            "device or who use the given hardware driver."),
+        required=False),
+    "hardware_owner_is_affected_by_bug": Bool(
+        title=_(
+            u"Search for bugs where people affected by a bug own the "
+            "given device or use the given hardware driver."),
+        required=False),
+    "hardware_owner_is_subscribed_to_bug": Bool(
+        title=_(
+            u"Search for bugs where a bug subscriber owns the "
+            "given device or uses the given hardware driver."),
+        required=False),
+    "hardware_is_linked_to_bug": Bool(
+        title=_(
+            u"Search for bugs which are linked to hardware reports "
+            "which contain the given device or whcih contain a device"
+            "controlled by the given driver."),
+        required=False),
+    "linked_branches": Choice(
+        title=_(
+            u"Search for bugs that are linked to branches or for bugs "
+            "that are not linked to branches."),
+        vocabulary=BugBranchSearch, required=False),
+    "modified_since": Datetime(
+        title=_(
+            u"Search for bugs that have been modified since the given "
+            "date."),
+        required=False),
+    }
+search_tasks_params_for_api_devel = search_tasks_params_for_api_1_0.copy()
+search_tasks_params_for_api_devel["omit_targeted"] = copy_field(
+    IBugTaskSearch['omit_targeted'], default=False)
+
 
 class IHasBugs(Interface):
     """An entity which has a collection of bug tasks."""
@@ -85,114 +198,13 @@
         "True if a BugTask has ever been reported for this target.")
 
     @call_with(search_params=None, user=REQUEST_USER)
-    @operation_parameters(
-        order_by=List(
-            title=_('List of fields by which the results are ordered.'),
-            value_type=Text(),
-            required=False),
-        search_text=copy_field(IBugTaskSearch['searchtext']),
-        status=copy_field(IBugTaskSearch['status']),
-        importance=copy_field(IBugTaskSearch['importance']),
-        assignee=Reference(schema=Interface),
-        bug_reporter=Reference(schema=Interface),
-        bug_supervisor=Reference(schema=Interface),
-        bug_commenter=Reference(schema=Interface),
-        bug_subscriber=Reference(schema=Interface),
-        structural_subscriber=Reference(schema=Interface),
-        owner=Reference(schema=Interface),
-        affected_user=Reference(schema=Interface),
-        has_patch=copy_field(IBugTaskSearch['has_patch']),
-        has_cve=copy_field(IBugTaskSearch['has_cve']),
-        tags=copy_field(IBugTaskSearch['tag']),
-        tags_combinator=copy_field(IBugTaskSearch['tags_combinator']),
-        omit_duplicates=copy_field(IBugTaskSearch['omit_dupes']),
-        omit_targeted=copy_field(IBugTaskSearch['omit_targeted']),
-        status_upstream=copy_field(IBugTaskSearch['status_upstream']),
-        milestone_assignment=copy_field(
-            IBugTaskSearch['milestone_assignment']),
-        milestone=copy_field(IBugTaskSearch['milestone']),
-        component=copy_field(IBugTaskSearch['component']),
-        nominated_for=Reference(schema=Interface),
-        has_no_package=copy_field(IBugTaskSearch['has_no_package']),
-        hardware_bus=Choice(
-            title=u'The bus of a hardware device related to a bug',
-            # The vocabulary should be HWBus; this is fixed in
-            # _schema_circular_imports to avoid circular imports.
-            vocabulary=DBEnumeratedType, required=False),
-        hardware_vendor_id=TextLine(
-            title=(
-                u"The vendor ID of a hardware device related to a bug."),
-            description=(
-                u"Allowed values of the vendor ID depend on the bus of the "
-                "device.\n\n"
-                "Vendor IDs of PCI, PCCard and USB devices are hexadecimal "
-                "string representations of 16 bit integers in the format "
-                "'0x01ab': The prefix '0x', followed by exactly 4 digits; "
-                "where a digit is one of the characters 0..9, a..f. The "
-                "characters A..F are not allowed.\n\n"
-                "SCSI vendor IDs are strings with exactly 8 characters. "
-                "Shorter names are right-padded with space (0x20) characters."
-                "\n\n"
-                "IDs for other buses may be arbitrary strings."),
-            required=False),
-        hardware_product_id=TextLine(
-            title=(
-                u"The product ID of a hardware device related to a bug."),
-            description=(
-                u"Allowed values of the product ID depend on the bus of the "
-                "device.\n\n"
-                "Product IDs of PCI, PCCard and USB devices are hexadecimal "
-                "string representations of 16 bit integers in the format "
-                "'0x01ab': The prefix '0x', followed by exactly 4 digits; "
-                "where a digit is one of the characters 0..9, a..f. The "
-                "characters A..F are not allowed.\n\n"
-                "SCSI product IDs are strings with exactly 16 characters. "
-                "Shorter names are right-padded with space (0x20) characters."
-                "\n\n"
-                "IDs for other buses may be arbitrary strings."),
-            required=False),
-        hardware_driver_name=TextLine(
-            title=(
-                u"The driver controlling a hardware device related to a "
-                "bug."),
-            required=False),
-        hardware_driver_package_name=TextLine(
-            title=(
-                u"The package of the driver which controls a hardware "
-                "device related to a bug."),
-            required=False),
-        hardware_owner_is_bug_reporter=Bool(
-            title=(
-                u"Search for bugs reported by people who own the given "
-                "device or who use the given hardware driver."),
-            required=False),
-        hardware_owner_is_affected_by_bug=Bool(
-            title=(
-                u"Search for bugs where people affected by a bug own the "
-                "given device or use the given hardware driver."),
-            required=False),
-        hardware_owner_is_subscribed_to_bug=Bool(
-            title=(
-                u"Search for bugs where a bug subscriber owns the "
-                "given device or uses the given hardware driver."),
-            required=False),
-        hardware_is_linked_to_bug=Bool(
-            title=(
-                u"Search for bugs which are linked to hardware reports "
-                "which contain the given device or whcih contain a device"
-                "controlled by the given driver."),
-            required=False),
-        linked_branches=Choice(
-            title=(
-                u"Search for bugs that are linked to branches or for bugs "
-                "that are not linked to branches."),
-            vocabulary=BugBranchSearch, required=False),
-        modified_since=Datetime(
-            title=(
-                u"Search for bugs that have been modified since the given "
-                "date."),
-            required=False),
-        )
+    @operation_parameters(**search_tasks_params_for_api_devel)
+    @operation_returns_collection_of(IBugTask)
+    @export_read_operation()
+    @operation_removed_in_version('devel')
+
+    @call_with(search_params=None, user=REQUEST_USER)
+    @operation_parameters(**search_tasks_params_for_api_1_0)
     @operation_returns_collection_of(IBugTask)
     @export_read_operation()
     def searchTasks(search_params, user=None,

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2010-08-22 18:31:30 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2010-08-26 20:46:45 +0000
@@ -892,7 +892,7 @@
         title=_('Omit bugs marked as duplicate,'), required=False,
         default=True)
     omit_targeted = Bool(
-        title=_('Omit bugs targeted to series'), required=False,
+        title=_('Omit bugs targeted to a series'), required=False,
         default=True)
     statusexplanation = TextLine(
         title=_("Status notes"), required=False)
@@ -1063,7 +1063,7 @@
 
 
 class IDistroSeriesBugTask(IBugTask):
-    """A bug needing fixing in a distrorealease, or a specific package."""
+    """A bug needing fixing in a distrorelease, or a specific package."""
     sourcepackagename = Choice(
         title=_("Source Package Name"), required=True,
         vocabulary='SourcePackageName')

=== modified file 'lib/lp/bugs/stories/webservice/xx-bug-target.txt'
--- lib/lp/bugs/stories/webservice/xx-bug-target.txt	2010-07-14 15:42:40 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug-target.txt	2010-08-26 20:46:45 +0000
@@ -57,8 +57,8 @@
     >>> product = factory.makeProduct(name='tags-test-product', owner=salgado)
     >>> logout()
 
-The webserice client is logged in as salgado, so we can add a new
-official tag.
+The webservice client is logged in as salgado, so we can add a new official
+tag.
 
     >>> print webservice.named_post(
     ...     '/tags-test-product', 'addOfficialBugTag',

=== modified file 'lib/lp/bugs/tests/test_bugtask.py'
--- lib/lp/bugs/tests/test_bugtask.py	2010-08-22 18:31:30 +0000
+++ lib/lp/bugs/tests/test_bugtask.py	2010-08-26 20:46:45 +0000
@@ -830,6 +830,15 @@
         self.assertStatementCount(1,
             lambda:[task.getConjoinedMaster for task in tasks])
 
+    def test_omit_targeted_default_is_false(self):
+        # The default value of omit_targeted is false so bugs targeted
+        # to a series are not hidden.
+        target = self.factory.makeDistroRelease()
+        self.login()
+        task1 = self.factory.makeBugTask(target=target)
+        default_result = target.searchTasks(None)
+        self.assertEqual([task1], list(default_result))
+
 
 def test_suite():
     suite = unittest.TestSuite()

=== added file 'lib/lp/bugs/tests/test_searchtasks_webservice.py'
--- lib/lp/bugs/tests/test_searchtasks_webservice.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/tests/test_searchtasks_webservice.py	2010-08-26 20:46:45 +0000
@@ -0,0 +1,43 @@
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Webservice unit tests related to Launchpad Bugs."""
+
+__metaclass__ = type
+
+from unittest import TestLoader
+
+from canonical.launchpad.ftests import login
+from lp.testing import TestCaseWithFactory
+from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
+from canonical.testing import DatabaseFunctionalLayer
+
+
+class TestOmitTargetedParameter(TestCaseWithFactory):
+    """Test all values for the omit_targeted search parameter."""
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        TestCaseWithFactory.setUp(self)
+        login('[e-mail]')
+        self.distro = self.factory.makeDistribution(name='mebuntu')
+        self.release = self.factory.makeDistroRelease(name='inkanyamba',
+            distribution=self.distro)
+        self.bug = self.factory.makeBugTask(target=self.release)
+
+        self.webservice = LaunchpadWebServiceCaller('launchpad-library',
+            'salgado-change-anything')
+
+    def test_omit_targeted_old_default_true(self):
+        response = self.webservice.named_get('/mebuntu/inkanyamba',
+            'searchTasks', api_version='1.0').jsonBody()
+        self.assertEqual(response['total_size'], 0)
+
+    def test_omit_targeted_new_default_false(self):
+        response = self.webservice.named_get('/mebuntu/inkanyamba',
+            'searchTasks', api_version='devel').jsonBody()
+        self.assertEqual(response['total_size'], 1)
+
+
+def test_suite():
+    return TestLoader().loadTestsFromName(__name__)