Merge lp:~jml/launchpad/searchTasks-modified-since-590535 into lp:launchpad

[Jonathan Lange]

This branch fixes bug 590535 by adding a new parameter to searchTasks to allow people to search for bugs that have been modified since a certain date.

I put the tests in test_bugtask.py, since it seemed the most sensible place. I may have duplicated tests elsewhere, since I wanted to be extra confident with these, since I didn't know what I was doing.

The test_suite method change simply makes the test runner find all of the tests in the module, without them having to be picked out and named individually.

The change to xx-bug.txt is kind of a cheat, in that it doesn't test much of the behaviour. However, since I know the method works, all I'm really testing is that passing in modified_since actually has an effect on the results. I think the test works for that.

I didn't really know whether the query should be for date_last_updated >= modified_since or date_last_updated > modified_since. I didn't think hard about it, but I couldn't really see the difference, so I opted for the one needing the fewest key presses.

I'm beside myself with scrumptious anticipation as I wait dotingly on your review.

jml

[Deryck Hodge (deryck)]

Hi, Jono.

This branch looks good to me. I was going to suggest changing the test name to be less generic about search and more about the specific case you were covering, but realizing we have no unit test coverage for searchTasks, I'm fine with this now, even if there is some tiny duplication. I would like to see us eventually move tests out of doc/bugtask-search.txt and into this class, where appropriate. Given that, keeping the name seems appropriate.

Also, limiting to tasks after but not including modified_since feels right to me, given the naming. The way you've done it here is what I would expect when using the parameter.

Finally, I agree with your choice to simply demonstrate the webservice and rely on unit test coverage. I have taken this approach myself recently.

Thanks for the work!

Cheers,
deryck

review: Approve (code)

=== modified file 'lib/lp/bugs/interfaces/bugtarget.py'
--- lib/lp/bugs/interfaces/bugtarget.py	2010-06-15 19:36:24 +0000
+++ lib/lp/bugs/interfaces/bugtarget.py	2010-06-24 14:20:50 +0000
@@ -21,7 +21,7 @@
     ]
 
 from zope.interface import Interface, Attribute
-from zope.schema import Bool, Choice, List, Object, Text, TextLine
+from zope.schema import Bool, Choice, Datetime, List, Object, Text, TextLine
 
 from canonical.launchpad import _
 from canonical.launchpad.fields import Tag
@@ -164,7 +164,13 @@
             title=(
                 u"Search for bugs that are linked to branches or for bugs "
                 "that are not linked to branches."),
-            vocabulary=BugBranchSearch, required=False))
+            vocabulary=BugBranchSearch, required=False),
+        modified_since=Datetime(
+            title=(
+                u"Search for bugs that have been modified since the given "
+                "date."),
+            required=False),
+        )
     @operation_returns_collection_of(IBugTask)
     @export_read_operation()
     def searchTasks(search_params, user=None,
@@ -186,7 +192,7 @@
                     hardware_owner_is_affected_by_bug=False,
                     hardware_owner_is_subscribed_to_bug=False,
                     hardware_is_linked_to_bug=False, linked_branches=None,
-                    structural_subscriber=None):
+                    structural_subscriber=None, modified_since=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	2010-06-07 19:48:29 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2010-06-24 14:20:50 +0000
@@ -1075,8 +1075,8 @@
                  hardware_owner_is_affected_by_bug=False,
                  hardware_owner_is_subscribed_to_bug=False,
                  hardware_is_linked_to_bug=False,
-                 linked_branches=None, structural_subscriber=None
-                 ):
+                 linked_branches=None, structural_subscriber=None,
+                 modified_since=None):
 
         self.bug = bug
         self.searchtext = searchtext
@@ -1121,6 +1121,7 @@
         self.hardware_is_linked_to_bug = hardware_is_linked_to_bug
         self.linked_branches = linked_branches
         self.structural_subscriber = structural_subscriber
+        self.modified_since = None
 
     def setProduct(self, product):
         """Set the upstream context on which to filter the search."""
@@ -1194,7 +1195,7 @@
                        hardware_owner_is_affected_by_bug=False,
                        hardware_owner_is_subscribed_to_bug=False,
                        hardware_is_linked_to_bug=False, linked_branches=None,
-                       structural_subscriber=None):
+                       structural_subscriber=None, modified_since=None):
         """Create and return a new instance using the parameter list."""
         search_params = cls(user=user, orderby=order_by)
 
@@ -1263,6 +1264,7 @@
             hardware_is_linked_to_bug)
         search_params.linked_branches=linked_branches
         search_params.structural_subscriber = structural_subscriber
+        search_params.modified_since = modified_since
 
         return search_params
 

=== modified file 'lib/lp/bugs/model/bugtarget.py'
--- lib/lp/bugs/model/bugtarget.py	2010-05-21 04:38:42 +0000
+++ lib/lp/bugs/model/bugtarget.py	2010-06-24 14:20:50 +0000
@@ -65,7 +65,8 @@
                     hardware_owner_is_bug_reporter=None,
                     hardware_owner_is_affected_by_bug=False,
                     hardware_owner_is_subscribed_to_bug=False,
-                    hardware_is_linked_to_bug=False, linked_branches=None):
+                    hardware_is_linked_to_bug=False, linked_branches=None,
+                    modified_since=None):
         """See `IHasBugs`."""
         if status is None:
             # If no statuses are supplied, default to the

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2010-06-22 17:08:23 +0000
+++ lib/lp/bugs/model/bugtask.py	2010-06-24 14:20:50 +0000
@@ -1833,6 +1833,11 @@
             # we don't need to add any clause.
             pass
 
+        if params.modified_since:
+            extra_clauses.append(
+                "Bug.date_last_updated > %s" % (
+                    sqlvalues(params.modified_since,)))
+
         orderby_arg = self._processOrderBy(params)
 
         query = " AND ".join(extra_clauses)

=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
--- lib/lp/bugs/stories/webservice/xx-bug.txt	2010-06-10 18:55:22 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt	2010-06-24 14:20:50 +0000
@@ -1436,6 +1436,15 @@
     total_size: 0
     ---
 
+It can also be used to find bugs modified since a certain date.
+
+    >>> pprint_collection(webservice.named_get(
+    ...     '/ubuntu', 'searchTasks',
+    ...     modified_since=u'2011-01-01T00:00:00+00:00').jsonBody())
+    start: None
+    total_size: 0
+    ---
+
 It is possible to search for bugs targeted to a milestone within a
 project group.
 

=== modified file 'lib/lp/bugs/tests/test_bugtask.py'
--- lib/lp/bugs/tests/test_bugtask.py	2010-06-21 18:09:47 +0000
+++ lib/lp/bugs/tests/test_bugtask.py	2010-06-24 14:20:50 +0000
@@ -3,6 +3,7 @@
 
 __metaclass__ = type
 
+from datetime import timedelta
 import unittest
 
 from zope.component import getUtility
@@ -14,8 +15,10 @@
 from lp.hardwaredb.interfaces.hwdb import HWBus, IHWDeviceSet
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.searchbuilder import all, any
-from canonical.testing import LaunchpadFunctionalLayer, LaunchpadZopelessLayer
+from canonical.testing import (
+    DatabaseFunctionalLayer, LaunchpadFunctionalLayer, LaunchpadZopelessLayer)
 
+from lp.bugs.interfaces.bugtarget import IBugTarget
 from lp.bugs.interfaces.bugtask import (
     BugTaskImportance, BugTaskSearchParams, BugTaskStatus)
 from lp.bugs.model.bugtask import build_tag_search_clause
@@ -28,7 +31,7 @@
 
 class TestBugTaskDelta(TestCaseWithFactory):
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         super(TestBugTaskDelta, self).setUp()
@@ -66,7 +69,6 @@
 
     def test_get_bugwatch_delta(self):
         # Exercise getDelta() with a change to bugwatch.
-        user = self.factory.makePerson()
         bug_task = self.factory.makeBugTask()
         bug_task_before_modification = Snapshot(
             bug_task, providing=providedBy(bug_task))
@@ -501,9 +503,9 @@
             [bugtask.bug.id for bugtask in bugtasks])
 
 
-class TestBugTaskPermissionsToSetAssigneeBase(TestCaseWithFactory):
+class TestBugTaskPermissionsToSetAssigneeMixin:
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         """Create the test setup.
@@ -516,7 +518,7 @@
           owners, bug supervisors, drivers
         - bug tasks for the targets
         """
-        super(TestBugTaskPermissionsToSetAssigneeBase, self).setUp()
+        super(TestBugTaskPermissionsToSetAssigneeMixin, self).setUp()
         self.target_owner_member = self.factory.makePerson()
         self.target_owner_team = self.factory.makeTeam(
             owner=self.target_owner_member)
@@ -556,6 +558,14 @@
         self.target_bugtask.transitionToAssignee(self.regular_user)
         logout()
 
+    def makeTarget(self):
+        """Create a target and a series.
+
+        The target and series must be assigned as attributes of self:
+        'self.target' and 'self.series'.
+        """
+        raise NotImplementedError(self.makeTarget)
+
     def test_userCanSetAnyAssignee_anonymous_user(self):
         # Anonymous users cannot set anybody as an assignee.
         login(ANONYMOUS)
@@ -700,7 +710,7 @@
 
 
 class TestProductBugTaskPermissionsToSetAssignee(
-    TestBugTaskPermissionsToSetAssigneeBase):
+    TestBugTaskPermissionsToSetAssigneeMixin, TestCaseWithFactory):
 
     def makeTarget(self):
         """Create a product and a product series."""
@@ -709,7 +719,7 @@
 
 
 class TestDistributionBugTaskPermissionsToSetAssignee(
-    TestBugTaskPermissionsToSetAssigneeBase):
+    TestBugTaskPermissionsToSetAssigneeMixin, TestCaseWithFactory):
 
     def makeTarget(self):
         """Create a distribution and a distroseries."""
@@ -718,14 +728,73 @@
         self.series = self.factory.makeDistroSeries(self.target)
 
 
+class TestBugTaskSearch(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def login(self):
+        # Log in as an arbitrary person.
+        person = self.factory.makePerson()
+        login_person(person)
+        self.addCleanup(logout)
+        return person
+
+    def makeBugTarget(self):
+        """Make an arbitrary bug target with no tasks on it."""
+        return IBugTarget(self.factory.makeProduct())
+
+    def test_no_tasks(self):
+        # A brand new bug target has no tasks.
+        target = self.makeBugTarget()
+        self.assertEqual([], list(target.searchTasks(None)))
+
+    def test_new_task_shows_up(self):
+        # When we create a new bugtask on the target, it shows up in
+        # searchTasks.
+        target = self.makeBugTarget()
+        self.login()
+        task = self.factory.makeBugTask(target=target)
+        self.assertEqual([task], list(target.searchTasks(None)))
+
+    def test_modified_since_excludes_earlier_bugtasks(self):
+        # When we search for bug tasks that have been modified since a certain
+        # time, tasks for bugs that have not been modified since then are
+        # excluded.
+        target = self.makeBugTarget()
+        self.login()
+        task = self.factory.makeBugTask(target=target)
+        date = task.bug.date_last_updated + timedelta(days=1)
+        result = target.searchTasks(None, modified_since=date)
+        self.assertEqual([], list(result))
+
+    def test_modified_since_includes_later_bugtasks(self):
+        # When we search for bug tasks that have been modified since a certain
+        # time, tasks for bugs that have been modified since then are
+        # included.
+        target = self.makeBugTarget()
+        self.login()
+        task = self.factory.makeBugTask(target=target)
+        date = task.bug.date_last_updated - timedelta(days=1)
+        result = target.searchTasks(None, modified_since=date)
+        self.assertEqual([task], list(result))
+
+    def test_modified_since_includes_later_bugtasks_excludes_earlier(self):
+        # When we search for bugs that have been modified since a certain
+        # time, tasks for bugs that have been modified since then are
+        # included, tasks that have not are excluded.
+        target = self.makeBugTarget()
+        self.login()
+        task1 = self.factory.makeBugTask(target=target)
+        date = task1.bug.date_last_updated
+        task1.bug.date_last_updated -= timedelta(days=1)
+        task2 = self.factory.makeBugTask(target=target)
+        task2.bug.date_last_updated += timedelta(days=1)
+        result = target.searchTasks(None, modified_since=date)
+        self.assertEqual([task2], list(result))
+
+
 def test_suite():
     suite = unittest.TestSuite()
-    suite.addTest(unittest.makeSuite(TestBugTaskDelta))
-    suite.addTest(unittest.makeSuite(TestBugTaskTagSearchClauses))
-    suite.addTest(unittest.makeSuite(TestBugTaskHardwareSearch))
-    suite.addTest(unittest.makeSuite(
-        TestProductBugTaskPermissionsToSetAssignee))
-    suite.addTest(unittest.makeSuite(
-        TestDistributionBugTaskPermissionsToSetAssignee))
+    suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))
     suite.addTest(DocTestSuite('lp.bugs.model.bugtask'))
     return suite