Merge lp:~gmb/launchpad/dont-piss-the-losas-off-bug-596877 into lp:launchpad/db-devel

[Graham Binns]

A button, visible to admins and LP developers, has been added to the BugTracker +edit page to allow all of a tracker's watches to be rescheduled en masse.

A button to reschedule all watches
==================================

This branch adds a button to the BugTracker +edit page which will
allow a user with sufficient privileges to reschedule all the watches on
a given BugTracker[1]. This is useful in situations where a bug tracker
has not been updated for a long period of time, so all the watches are
out of date. In the past we've done this by asking a LOSA to run some
SQL, but since that's a drain on their time and ours, a button seemed
more useful.

One of the problems associated with a large number of out-of-date
watches, however, is that checkwatches attempts to burn through them all
at once. This can lead to us adding a great deal of load to the remote
server, which is generally a Bad Thing and can get Launchpad blocked
from communicating with the upstream bug tracker (at the very least).

In order to obviate this problem, we have in the past used SQL that
would randomise the next_check time for each bug watch within a range of
up to twenty-four hours. This would then take some of the strain off the
remote server since checkwatches wouldn't be trying to update everything
as quickly as possible. It seemed sensible to use this same SQL when
creating the reschedule button. Note that 24 hours is the optimal
maximum time frame in which to schedule watches; anything longer means
that watches remain unchecked whilst other watches go out of date and
need to be checked again, potentially creating further load issues.

Finally, I cleaned up some lint in this branch, but I'm not wholly
certain that the new linter isn't getting some things wrong about our
coding standards.

Changes made
------------

lib/lp/bugs/browser/bugtracker.py
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 - I've added an exception, UserCannotResetWatchesError, which is raised
   when a user attempts to all watches on a bug tracker without
   permission.
 - I've added a user_can_reset_watches property to BugTrackerEditView.
   This returns True if the user is either a Launchpad Admin or is a
   member of ~launchpad.
 - I've added a method, resetBugTrackerWatches() to BugTrackerEditView.
   This calls BugTracker.resetWatches(). I've made this a separate
   method so that it could be tested properly in view tests rather than
   putting all the implementation in the body of the action.
 - I've added a reschedule_action_condition() method to
   BugTrackerEditView. This returns true if there are watches to
   reschedule and user_can_reset_watches returns True. This is used to
   determine whether the reschedule button shows up on the +edit page.
 - I've added a rescheduleAction() method / action to
   BugTrackerEditView. This calls resetBugTrackerWatches() and adds an
   info notification to the page stating that all watches have been
   updated. Note that it makes no attempt to catch
   UserCannotResetWatchesErrors, since we want these to be dealt with by
   the OOPS machinery as they are indicative of someone trying to craft
   an HTTP POST to reschedule watches.

lib/lp/bugs/browser/tests/test_bugtracker_views.py
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 - I've added tests to cover the resetBugTrackerWatches() method of
   BugTrackerEditView. The tests are designed to check that
   resetBugTrackerWatches() calls BugTracker.resetWatches() correctly
   and that it also checks that the user is allowed to reset a bug
   tracker's watches.

lib/lp/bugs/configure.zcml
~~~~~~~~~~~~~~~~~~~~~~~~~~

 - I've updated the ZCML and moved BugTracker.resetWatches() out of the
   launchpad.Admin section and into launchpad.Edit (hence the need for
   stricter checking of permissions in
   BugTrackerEditView.resetBugTrackerWatches()). This move was necessary
   to allow Launchpad Developers to reset watches.

lib/lp/bugs/doc/bugtracker.txt
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 - I've updated the existing tests for BugTracker.resetWatches() to
   cover the new randomisation-over-24-hours code. I was tempted to
   refactor the whole of bugtracker.txt into unittest format, but
   decided against it for reasons of space and sanity.
 - I've fixed a bunch of lint issues with the doctest.

lib/lp/bugs/model/bugtracker.py
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 - I've updated BugTracker.resetWatches() so that it now sets next_check
   to a random time between now and one day in the future, rather than
   just setting it to now as it used to.
 - I've also fixed a minor typo.

lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 - This pagetest covers the Reschedule all watches button which will now
   appear on the BugTracker +edit page when it is viewed by admins or LP
   developers.

lib/lp/testing/factory.py
~~~~~~~~~~~~~~~~~~~~~~~~~

 - I updated makeBugTracker() so that it now accepts name and title
   arguments.

lib/lp/testing/sampledata.py
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 - I added constants for ADMIN_EMAIL (foo.bar@) and USER_EMAIL(test@).

Test commands
-------------

`bin/test -cvv -t /bugtracker.txt -t xx-reschedule-all-watches \
          -t test_bugtracker_views`

 [1] http://launchpad.net/bugs/596877

[Robert Collins (lifeless)]

Shouldn't the permission check be in the model? the LBYP idiom seems likely to miss things...

review: Needs Information

[Robert Collins (lifeless)]

Also as discussed there are no particularly special permission issues here so raising the normal permission denied stuff should be all thats needed.

review: Needs Fixing

[Graham Binns (gmb)]

I've updated the branch per our discussions. Here comes the diff:

=== modified file 'lib/lp/bugs/browser/bugtracker.py'
--- lib/lp/bugs/browser/bugtracker.py 2010-07-17 19:19:16 +0000
+++ lib/lp/bugs/browser/bugtracker.py 2010-07-20 16:17:59 +0000
@@ -27,6 +27,7 @@
 from zope.formlib import form
 from zope.schema import Choice
 from zope.schema.vocabulary import SimpleVocabulary
+from zope.security.interfaces import Unauthorized

 from canonical.cachedproperty import cachedproperty
 from canonical.database.sqlbase import flush_database_updates
@@ -63,10 +64,6 @@
     )

-class UserCannotResetWatchesError(Exception):
- """Raised when a user cannot reset a bug tracker's watches."""
-
-
 class BugTrackerSetNavigation(GetitemNavigation):

     usedfor = IBugTrackerSet
@@ -387,11 +384,13 @@
     def resetBugTrackerWatches(self):
         """Call the resetWatches() method of the current context.

- :raises `UserCannotResetWatchesError` if the user isn't an admin
+ :raises `Unauthorized` if the user isn't an admin
                 or a member of the Launchpad Developers team.
         """
         if not self.user_can_reset_watches:
- raise UserCannotResetWatchesError
+ raise Unauthorized(
+ "You don't have permission to reset all the watches for "
+ "this bug tracker.")

         self.context.resetWatches()

=== modified file 'lib/lp/bugs/browser/tests/test_bugtracker_views.py'
--- lib/lp/bugs/browser/tests/test_bugtracker_views.py 2010-07-17 16:59:12 +0000
+++ lib/lp/bugs/browser/tests/test_bugtracker_views.py 2010-07-20 16:17:59 +0000
@@ -11,12 +11,13 @@
 from pytz import utc

 from zope.component import getUtility
+from zope.security.interfaces import Unauthorized

 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.testing.layers import DatabaseFunctionalLayer

 from lp.bugs.browser.bugtracker import (
- BugTrackerEditView, UserCannotResetWatchesError)
+ BugTrackerEditView)
 from lp.registry.interfaces.person import IPersonSet
 from lp.testing import login, TestCaseWithFactory
 from lp.testing.sampledata import ADMIN_EMAIL, NO_PRIVILEGE_EMAIL
@@ -70,7 +71,7 @@
         login(NO_PRIVILEGE_EMAIL)
         view = create_initialized_view(self.bug_tracker, '+edit')
         self.assertRaises(
- UserCannotResetWatchesError, view.resetBugTrackerWatches)
+ Unauthorized, view.resetBugTrackerWatches)

     def test_admin_can_reset_watches(self):
         # Launchpad admins can reset the watches on a bugtracker.

[Robert Collins (lifeless)]

Thanks for improving it.

review: Approve

[Robert Collins (lifeless)]

Sorry, I missed that its still in the view. Shouldn't it be in the model?

review: Needs Information

[Graham Binns (gmb)]

It should, and I've moved it there (for some reason I had it in my head that we should always check stuff in the view, but that turns out not to be true).

Here's a diff:

=== modified file 'lib/lp/bugs/browser/bugtracker.py'
--- lib/lp/bugs/browser/bugtracker.py 2010-07-20 16:17:59 +0000
+++ lib/lp/bugs/browser/bugtracker.py 2010-07-23 11:11:13 +0000
@@ -374,38 +374,22 @@
     def cancel_url(self):
         return canonical_url(self.context)

- @property
- def user_can_reset_watches(self):
+ def reschedule_action_condition(self, action):
+ """Return True if the user can see the reschedule action."""
         lp_developers = getUtility(ILaunchpadCelebrities).launchpad_developers
- return (
+ user_can_reset_watches = (
             check_permission("launchpad.Admin", self.user) or
             self.user.inTeam(lp_developers))
-
- def resetBugTrackerWatches(self):
- """Call the resetWatches() method of the current context.
-
- :raises `Unauthorized` if the user isn't an admin
- or a member of the Launchpad Developers team.
- """
- if not self.user_can_reset_watches:
- raise Unauthorized(
- "You don't have permission to reset all the watches for "
- "this bug tracker.")
-
- self.context.resetWatches()
-
- def reschedule_action_condition(self, action):
- """Return True if the user can see the reschedule action."""
         return (
             self.context.watches.count() > 0 and
- self.user_can_reset_watches)
+ user_can_reset_watches)

     @action(
         'Reschedule all watches', name='reschedule',
         condition=reschedule_action_condition)
     def rescheduleAction(self, action, data):
         """Reschedule all the watches for the bugtracker."""
- self.resetBugTrackerWatches()
+ self.context.resetWatches(user=self.user)
         self.request.response.addInfoNotification(
             structured(
                 "All bug watches on %s have been rescheduled." %

=== modified file 'lib/lp/bugs/browser/tests/test_bugtracker_views.py'
--- lib/lp/bugs/browser/tests/test_bugtracker_views.py 2010-07-20 16:17:59 +0000
+++ lib/lp/bugs/browser/tests/test_bugtracker_views.py 2010-07-23 11:11:13 +0000
@@ -24,77 +24,6 @@
 from lp.testing.views import create_initialized_view

-class BugTrackerEditViewTestCase(TestCaseWithFactory):
- """A TestCase for the `BugTrackerEditView`."""
-
- layer = DatabaseFunctionalLayer
-
- def setUp(self):
- super(BugTrackerEditViewTestCase, self).setUp()
- self.bug_tracker = self.factory.makeBugTracker()
- for i in range(5):
- self.factory.makeBugWatch(bugtracker=self.bug_tracker)
-
- self.now = datetime.now(utc)
-
- def _assertBugWatchesAreCheckedInTheFuture(self):
- """Check the dates of all self.bug_tracker.watches.
-
- Raise an error if:
- * The next_check dates aren't in the future.
- * The next_check dates aren't <= 1 day in the future.
- * The lastcheck dates are not None
- * The last_error_types are not None.
- """
- for watch i...

[Gavin Panella (allenap)]

This branch is a really cool addition. I think it can be simplified by
leaning more heavily on Zope security, and there are a few other
things to consider.

I wrote the review notes in the Landscape style before I remembered
that this isn't Landscape. However, I think it's quite readable,
especially since responses need only reference the number given to
each comment, so I'll leave it. I hope you don't mind. Let me know
what you think.

[1]

+ user_can_reset_watches = (
+ check_permission("launchpad.Admin", self.user) or
+ self.user.inTeam(lp_developers))

This would perhaps be better placed on the model, something like
canResetWatches(user).

[2]

+ check_permission("launchpad.Admin", self.user) or

I may well be wrong, but this looks like it checks if the current user
has the launchpad.Admin permission on self.user, which will always be
true.

I think it might be worth defining a new AdminBugTracker authorization
adapter in canonical.launchpad.security, something like:

class AdminBugTracker(AuthorizationBase):
    permission = 'launchpad.Admin'
    usedfor = IBugTracker

    def checkAuthenticated(self, user):
        return (user.in_admin or
                user.in_launchpad_developers or
                user.in_janitor)

Then the changes to lib/lp/bugs/configure.zcml can be reverted.

[3]

=== added file 'lib/lp/bugs/browser/tests/test_bugtracker_views.py'

There are no tests in this file! :)

[4]

+ :param new_next_check: If specified, contains the datetime to
+ which to set the BugWatches' next_check times.
+ If not specified, the watches' next_check
+ times will be set to a point between now
+ and 24 hours hence.

I /think/ it's usual to indent the second and subsequent lines by 4
spaces. Doesn't matter much though.

[5]

+ def resetWatches(self, user, new_next_check=None):
         """See `IBugTracker`."""
- if now is None:
- now = datetime.now(timezone('UTC'))
+ celebrities = getUtility(ILaunchpadCelebrities)
+ launchpad_developers = celebrities.launchpad_developers
+ admin_team = celebrities.admin
+ janitor = celebrities.janitor
+
+ if (not user.inTeam(admin_team) and
+ not user.inTeam(launchpad_developers) and
+ user != janitor):
+ raise Unauthorized(
+ "You do not have permission to reset the watches for "
+ "this bug tracker.")

If you add the security adapter in [2] then the user checking stuff
here can be dropped.

[6]

+ self.request.response.addInfoNotification(
+ structured(
+ "All bug watches on %s have been rescheduled." %
+ self.context.title))

I don't think this should be a structured() string; that would allow
markup/script injection via the bug tracker title.

review: Needs Fixing

[Graham Binns (gmb)]

Diff of changes:

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py 2010-08-22 18:31:30 +0000
+++ lib/canonical/launchpad/security.py 2010-08-25 15:59:28 +0000
@@ -2561,3 +2561,14 @@
         if parent is None:
             return False
         return check_permission(self.permission, parent)
+
+
+class AdminBugTracker(AuthorizationBase):
+ permission = 'launchpad.Admin'
+ usedfor = IBugTracker
+
+ def checkAuthenticated(self, user):
+ return (
+ user.in_janitor or
+ user.in_admin or
+ user.in_launchpad_developers)

=== modified file 'lib/lp/bugs/browser/bugtracker.py'
--- lib/lp/bugs/browser/bugtracker.py 2010-08-25 10:45:59 +0000
+++ lib/lp/bugs/browser/bugtracker.py 2010-08-25 15:59:28 +0000
@@ -397,10 +397,8 @@

     def reschedule_action_condition(self, action):
         """Return True if the user can see the reschedule action."""
- lp_developers = getUtility(ILaunchpadCelebrities).launchpad_developers
- user_can_reset_watches = (
- check_permission("launchpad.Admin", self.user) or
- self.user.inTeam(lp_developers))
+ user_can_reset_watches = check_permission(
+ "launchpad.Admin", self.context)
         return (
             self.context.watches.count() > 0 and
             user_can_reset_watches)

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2010-08-25 10:45:59 +0000
+++ lib/lp/bugs/configure.zcml 2010-08-25 15:59:28 +0000
@@ -394,7 +394,7 @@
                     destroySelf
                     ensurePersonForSelf
                     linkPersonToSelf
- resetWatches"
+ "
                 set_attributes="
                     aliases
                     baseurl
@@ -408,6 +408,7 @@
                     "/>
             <require
                 permission="launchpad.Admin"
+ attributes="resetWatches"
                 set_attributes="active"/>
         </class>
         <adapter

=== modified file 'lib/lp/bugs/interfaces/bugtracker.py'
--- lib/lp/bugs/interfaces/bugtracker.py 2010-08-25 10:45:59 +0000
+++ lib/lp/bugs/interfaces/bugtracker.py 2010-08-25 16:03:56 +0000
@@ -344,14 +344,13 @@
     def destroySelf():
         """Delete this bug tracker."""

- def resetWatches(user, new_next_check=None):
+ def resetWatches(new_next_check=None):
         """Reset the next_check times of this BugTracker's `BugWatch`es.

         :param new_next_check: If specified, contains the datetime to
- which to set the BugWatches' next_check times.
- If not specified, the watches' next_check
- times will be set to a point between now
- and 24 hours hence.
+ which to set the BugWatches' next_check times. If not
+ specified, the watches' next_check times will be set to a
+ point between now and 24 hours hence.
         """

=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py 2010-08-25 10:45:59 +0000
+++ lib/lp/bugs/model/bugt...

[Gavin Panella (allenap)]

I think [6] still needs to be addressed, but the rest looks good. I do
have a couple of comments:

[7]

         return (
             self.context.watches.count() > 0 and
             user_can_reset_watches)

The order here could be reversed; user_can_reset_watches is already
calculated so it may as well short circuit on it and avoid a query if
possible:

         return (
             user_can_reset_watches and
             self.context.watches.count() > 0)

Also it would be more efficient to use is_empty():

         return (
             user_can_reset_watches and
             not self.context.watches.is_empty())

[8]

+ user_can_reset_watches = check_permission(
+ "launchpad.Admin", self.context)

This works fine, and is nothing controversial, but if, say,
resetWatches is given a new permission then it will no longer check
the right thing. Would something like the following work better?

    try:
        self.context.resetWatches
    except Unauthorized:
        return False
    else:
        return not self.context.watches.is_empty()

[Gavin Panella (allenap)]

review: Approve

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-08-24 09:51:26 +0000
+++ lib/canonical/launchpad/security.py	2010-08-26 10:08:48 +0000
@@ -2561,3 +2561,14 @@
         if parent is None:
             return False
         return check_permission(self.permission, parent)
+
+
+class AdminBugTracker(AuthorizationBase):
+    permission = 'launchpad.Admin'
+    usedfor = IBugTracker
+
+    def checkAuthenticated(self, user):
+        return (
+            user.in_janitor or
+            user.in_admin or
+            user.in_launchpad_developers)

=== modified file 'lib/lp/bugs/browser/bugtracker.py'
--- lib/lp/bugs/browser/bugtracker.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/browser/bugtracker.py	2010-08-26 10:08:48 +0000
@@ -27,6 +27,7 @@
 from zope.interface import implements
 from zope.schema import Choice
 from zope.schema.vocabulary import SimpleVocabulary
+from zope.security.interfaces import Unauthorized
 
 from canonical.cachedproperty import cachedproperty
 from canonical.database.sqlbase import flush_database_updates
@@ -79,7 +80,9 @@
 # of.
 NO_DIRECT_CREATION_TRACKERS = (
     SINGLE_INSTANCE_TRACKERS + (
-        BugTrackerType.EMAILADDRESS,))
+        BugTrackerType.EMAILADDRESS,
+        )
+    )
 
 
 class BugTrackerSetNavigation(GetitemNavigation):
@@ -185,7 +188,7 @@
             has_more_pillars = False
         return {
             'pillars': pillars[:self.pillar_limit],
-            'has_more_pillars': has_more_pillars
+            'has_more_pillars': has_more_pillars,
         }
 
 
@@ -392,6 +395,25 @@
     def cancel_url(self):
         return canonical_url(self.context)
 
+    def reschedule_action_condition(self, action):
+        """Return True if the user can see the reschedule action."""
+        user_can_reset_watches = check_permission(
+            "launchpad.Admin", self.context)
+        return (
+            user_can_reset_watches and
+            self.context.watches.count() > 0)
+
+    @action(
+        'Reschedule all watches', name='reschedule',
+        condition=reschedule_action_condition)
+    def rescheduleAction(self, action, data):
+        """Reschedule all the watches for the bugtracker."""
+        self.context.resetWatches()
+        self.request.response.addInfoNotification(
+            "All bug watches on %s have been rescheduled." %
+            self.context.title)
+        self.next_url = canonical_url(self.context)
+
 
 class BugTrackerNavigation(Navigation):
 

=== added file 'lib/lp/bugs/browser/tests/test_bugtracker_views.py'
--- lib/lp/bugs/browser/tests/test_bugtracker_views.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugtracker_views.py	2010-08-26 10:08:48 +0000
@@ -0,0 +1,29 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for BugTracker views."""
+
+__metaclass__ = type
+
+import unittest
+
+from datetime import datetime, timedelta
+from pytz import utc
+
+from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
+
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.testing.layers import DatabaseFunctionalLayer
+
+from lp.bugs.browser.bugtracker import (
+    BugTrackerEditView)
+from lp.registry.interfaces.person import IPersonSet
+from lp.testing import login, TestCaseWithFactory
+from lp.testing.sampledata import ADMIN_EMAIL, NO_PRIVILEGE_EMAIL
+from lp.testing.views import create_initialized_view
+
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2010-08-23 09:18:51 +0000
+++ lib/lp/bugs/configure.zcml	2010-08-26 10:08:48 +0000
@@ -390,7 +390,8 @@
                 attributes="
                     destroySelf
                     ensurePersonForSelf
-                    linkPersonToSelf"
+                    linkPersonToSelf
+                    "
                 set_attributes="
                     aliases
                     baseurl

=== modified file 'lib/lp/bugs/doc/bugtracker.txt'
--- lib/lp/bugs/doc/bugtracker.txt	2010-08-09 13:59:58 +0000
+++ lib/lp/bugs/doc/bugtracker.txt	2010-08-26 10:08:48 +0000
@@ -203,7 +203,8 @@
     True
 
 
-=== Aliases ===
+Aliases
+-------
 
 A bug tracker can have a number of alias URLs associated with it.
 
@@ -293,7 +294,8 @@
     >>> mozilla_bugzilla.baseurl = 'https://bugzilla.mozilla.org/'
 
 
-== Pillars for bugtrackers ===
+Pillars for bugtrackers
+-----------------------
 
     >>> trackers = list(bugtracker_set)
     >>> pillars = bugtracker_set.getPillarsForBugtrackers(trackers)
@@ -473,7 +475,7 @@
     >>> links = example_mantis.getBugFilingAndSearchLinks(
     ...     remote_product='testproduct', summary="Foo", description="Bar")
     >>> print_links(links)
-    bug_filing_url: .../bug_report_advanced_page.php?summary=Foo&description=Bar
+    bug_filing_url: .../bug_..._advanced_page.php?summary=Foo&description=Bar
     bug_search_url: .../view_all_bug_page.php
 
 The EMAILADDRESS BugTrackerType is a special case and returns None for
@@ -530,16 +532,16 @@
 ------------------------
 
 As described above, some bug trackers don't need to have a remote
-product passed to `getBugFilingAndSearchLinks()` in order to be able to return a
-bug filing URL because they use static URLs for bug filing or only track
-one product.
+product passed to `getBugFilingAndSearchLinks()` in order to be able to
+return a bug filing URL because they use static URLs for bug filing or
+only track one product.
 
 `IBugTracker` defines an attribute, `multi_product` which can be used to
 check whether a given bug tracker can return a bug filing URL without
 being passed a remote product.
 
-Our example Trac bug tracker's `multi_product` property will
-be False, since it only tracks one product at a time.
+Our example Trac bug tracker's `multi_product` property will be False,
+since it only tracks one product at a time.
 
     >>> print example_trac.multi_product
     False
@@ -586,55 +588,3 @@
     bug_search_url:
     http://.../query.cgi?product=testproduct&short_desc=Foo
 
-
-Resetting a tracker's watches
------------------------------
-
-It's possible to reset all the watches on a bug tracker by calling its
-resetWatches() method.
-
-    >>> from operator import attrgetter
-    >>> from pytz import utc
-    >>> definitely_now = datetime.now(utc)
-
-    >>> def print_tracker_watch_times(bugtracker):
-    ...     for watch in sorted(
-    ...         bugtracker.watches, key=attrgetter('id')):
-    ...         if watch.next_check != definitely_now:
-    ...             print watch.bug.id, "not now"
-    ...         else:
-    ...             print watch.bug.id, "now"
-
-    >>> print_tracker_watch_times(mozilla_bugzilla)
-    2 not now
-    1 not now
-    1 not now
-    1 not now
-
-Calling resetWatches() on mozilla_bugzilla will set all its next_check
-times to datetime.now(). For testing purposes, we'll pass it a value to
-use in place of now().
-
-    >>> mozilla_bugzilla.resetWatches(now=definitely_now)
-    >>> transaction.commit()
-    >>> print_tracker_watch_times(mozilla_bugzilla)
-    2 now
-    1 now
-    1 now
-    1 now
-
-resetWatches() also clears the last_error_type field of the watches.
-
-    >>> from lp.bugs.interfaces.bugwatch import BugWatchActivityStatus
-    >>> for watch in mozilla_bugzilla.watches:
-    ...     watch.last_error_type = BugWatchActivityStatus.UNKNOWN
-
-    >>> txn = transaction.begin()
-    >>> mozilla_bugzilla.resetWatches()
-    >>> txn.commit()
-    >>> for watch in mozilla_bugzilla.watches:
-    ...     print watch.last_error_type
-    None
-    None
-    None
-    None

=== modified file 'lib/lp/bugs/interfaces/bugtracker.py'
--- lib/lp/bugs/interfaces/bugtracker.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/interfaces/bugtracker.py	2010-08-26 10:08:48 +0000
@@ -344,12 +344,13 @@
     def destroySelf():
         """Delete this bug tracker."""
 
-    def resetWatches(now=None):
+    def resetWatches(new_next_check=None):
         """Reset the next_check times of this BugTracker's `BugWatch`es.
 
-        :param now: If specified, contains the datetime to which to set
-                    the BugWatches' next_check times. Defaults to
-                    datetime.now().
+        :param new_next_check: If specified, contains the datetime to
+            which to set the BugWatches' next_check times.  If not
+            specified, the watches' next_check times will be set to a
+            point between now and 24 hours hence.
         """
 
 

=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/model/bugtracker.py	2010-08-26 10:08:48 +0000
@@ -21,6 +21,10 @@
     splittype,
     )
 
+from zope.component import getUtility
+from zope.interface import implements
+from zope.security.interfaces import Unauthorized
+
 from lazr.uri import URI
 from pytz import timezone
 from sqlobject import (
@@ -36,6 +40,7 @@
     Count,
     Desc,
     Not,
+    SQL,
     )
 from storm.locals import Bool
 from storm.store import Store
@@ -46,7 +51,6 @@
 from canonical.database.sqlbase import (
     flush_database_updates,
     SQLBase,
-    sqlvalues,
     )
 from canonical.launchpad.helpers import shortlist
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
@@ -500,19 +504,20 @@
 
         return person
 
-    def resetWatches(self, now=None):
+    def resetWatches(self, new_next_check=None):
         """See `IBugTracker`."""
-        if now is None:
-            now = datetime.now(timezone('UTC'))
+        if new_next_check is None:
+            new_next_check = SQL(
+                "now() at time zone 'UTC' + (random() * interval '1 day')")
 
         store = Store.of(self)
-        store.execute(
-            "UPDATE BugWatch SET next_check = %s WHERE bugtracker = %s" %
-            sqlvalues(now, self))
+        store.find(BugWatch, BugWatch.bugtracker == self).set(
+            next_check=new_next_check, lastchecked=None,
+            last_error_type=None)
 
 
 class BugTrackerSet:
-    """Implements IBugTrackerSet for a container or set of BugTracker's,
+    """Implements IBugTrackerSet for a container or set of BugTrackers,
     either the full set in the db, or a subset.
     """
 

=== modified file 'lib/lp/bugs/scripts/checkwatches/core.py'
--- lib/lp/bugs/scripts/checkwatches/core.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/scripts/checkwatches/core.py	2010-08-26 10:08:48 +0000
@@ -301,7 +301,8 @@
             self.logger.info(
                 "Resetting %s bug watches for bug tracker '%s'" %
                 (bug_tracker.watches.count(), bug_tracker_name))
-            bug_tracker.resetWatches()
+            bug_tracker.resetWatches(
+                new_next_check=datetime.now(pytz.timezone('UTC')))
 
         # Loop over the bug watches in batches as specificed by
         # batch_size until there are none left to update.

=== added file 'lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt'
--- lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/stories/bugtracker/xx-reschedule-all-watches.txt	2010-08-26 10:08:48 +0000
@@ -0,0 +1,94 @@
+Rescheduling watches on a bugtracker
+====================================
+
+It's possible for all the watches on a bug tracker to be rescheduled, in
+much the same way as it's possible to reschedule a single bug watch that
+is failing to update.
+
+    >>> from canonical.launchpad.webapp import canonical_url
+    >>> from lp.testing.sampledata import ADMIN_EMAIL
+    >>> from lp.testing import login, logout
+
+    >>> login(ADMIN_EMAIL)
+    >>> bug_tracker = factory.makeBugTracker(
+    ...     name='our-bugtracker', title='Our BugTracker')
+    >>> bug_watch = factory.makeBugWatch(bugtracker=bug_tracker)
+    >>> logout()
+
+    >>> bug_tracker_edit_url = (
+    ...     canonical_url(bug_tracker) + '/+edit')
+
+The functionality to do this is available from the bug tracker's +edit
+page. It isn't visible to ordinary users, however.
+
+    >>> user_browser.open(bug_tracker_edit_url)
+    >>> user_browser.getControl('Reschedule all watches')
+    Traceback (most recent call last):
+      ...
+    LookupError: label 'Reschedule all watches'
+
+However, the reschedule button will appear to administrators.
+
+    >>> admin_browser.open(bug_tracker_edit_url)
+    >>> admin_browser.getControl('Reschedule all watches')
+    <SubmitControl...>
+
+It will also appear for non-admin members of the Launchpad Developers
+team.
+
+    >>> from zope.component import getUtility
+    >>> from canonical.launchpad.interfaces.launchpad import (
+    ...     ILaunchpadCelebrities)
+    >>> from lp.registry.interfaces.person import IPersonSet
+
+    >>> login(ADMIN_EMAIL)
+    >>> admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+    >>> new_lp_developer = factory.makePerson(password='newpassword')
+    >>> launchpad_developers = getUtility(
+    ...     ILaunchpadCelebrities).launchpad_developers
+    >>> dev_added = launchpad_developers.addMember(
+    ...     new_lp_developer, admin_user)
+
+    >>> lp_dev_browser = setupBrowser(
+    ...     auth='Basic %s:newpassword' %
+    ...         new_lp_developer.preferredemail.email)
+    >>> logout()
+
+    >>> lp_dev_browser.open(bug_tracker_edit_url)
+    >>> reschedule_button = lp_dev_browser.getControl(
+    ...     'Reschedule all watches')
+
+Clicking the button will reschedule the watches for the bug tracker for
+checking at some future date.
+
+    >>> reschedule_button.click()
+    >>> print lp_dev_browser.url
+    http://bugs.launchpad.dev/bugs/bugtrackers/our-bugtracker
+
+    >>> for message in find_tags_by_class(
+    ...     lp_dev_browser.contents, 'informational message'):
+    ...     print extract_text(message)
+    All bug watches on Our BugTracker have been rescheduled.
+
+If we look at the bug watch on our bugtracker we can see that it has
+been scheduled for checking at some point in the future.
+
+    >>> from datetime import datetime
+    >>> from pytz import utc
+
+    >>> login(ADMIN_EMAIL)
+    >>> print bug_watch.next_check >= datetime.now(utc)
+    True
+
+Should the bug watch be deleted the reschedule button will no longer
+appear on the bugtracker page.
+
+    >>> bug_watch.destroySelf()
+    >>> logout()
+
+    >>> lp_dev_browser.open(bug_tracker_edit_url)
+    >>> reschedule_button = lp_dev_browser.getControl(
+    ...     'Reschedule all watches')
+    Traceback (most recent call last):
+      ...
+    LookupError: label 'Reschedule all watches'

=== modified file 'lib/lp/bugs/tests/test_bugtracker.py'
--- lib/lp/bugs/tests/test_bugtracker.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/tests/test_bugtracker.py	2010-08-26 10:08:48 +0000
@@ -3,6 +3,8 @@
 
 __metaclass__ = type
 
+import unittest
+import transaction
 from datetime import (
     datetime,
     timedelta,
@@ -12,7 +14,6 @@
     ELLIPSIS,
     NORMALIZE_WHITESPACE,
     )
-import unittest
 from urllib2 import (
     HTTPError,
     Request,
@@ -20,10 +21,13 @@
 
 from lazr.lifecycle.snapshot import Snapshot
 from pytz import utc
-import transaction
+
+from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.ftests import login_person
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.testing import LaunchpadFunctionalLayer
 from lp.bugs.externalbugtracker import (
     BugTrackerConnectError,
@@ -35,15 +39,23 @@
     IBugTracker,
     )
 from lp.bugs.tests.externalbugtracker import Urlib2TransportTestHandler
-from lp.testing import TestCaseWithFactory
-
-
-class TestBugTracker(TestCaseWithFactory):
+from lp.registry.interfaces.person import IPersonSet
+from lp.testing import login, login_person, TestCaseWithFactory
+from lp.testing.sampledata import ADMIN_EMAIL
+
+
+class BugTrackerTestCase(TestCaseWithFactory):
+    """Unit tests for the `BugTracker` class."""
+
     layer = LaunchpadFunctionalLayer
 
     def setUp(self):
-        super(TestBugTracker, self).setUp()
-        transaction.commit()
+        super(BugTrackerTestCase, self).setUp()
+        self.bug_tracker = self.factory.makeBugTracker()
+        for i in range(5):
+            self.factory.makeBugWatch(bugtracker=self.bug_tracker)
+
+        self.now = datetime.now(utc)
 
     def test_multi_product_constraints_observed(self):
         """BugTrackers for which multi_product=True should return None
@@ -141,6 +153,82 @@
         removeSecurityProxy(bug_message).remote_comment_id = 'brains'
         self.failUnless(bug_tracker.watches_with_unpushed_comments.is_empty())
 
+    def _assertBugWatchesAreCheckedInTheFuture(self):
+        """Check the dates of all self.bug_tracker.watches.
+
+        Raise an error if:
+         * The next_check dates aren't in the future.
+         * The next_check dates aren't <= 1 day in the future.
+         * The lastcheck dates are not None
+         * The last_error_types are not None.
+        """
+        for watch in self.bug_tracker.watches:
+            self.assertTrue(
+                watch.next_check is not None,
+                "BugWatch next_check time should not be None.")
+            self.assertTrue(
+                watch.next_check >= self.now,
+                "BugWatch next_check time should be in the future.")
+            self.assertTrue(
+                watch.next_check <= self.now + timedelta(days=1),
+                "BugWatch next_check time should be one day or less in "
+                "the future.")
+            self.assertTrue(
+                watch.lastchecked is None,
+                "BugWatch lastchecked should be None.")
+            self.assertTrue(
+                watch.last_error_type is None,
+                "BugWatch last_error_type should be None.")
+
+    def test_unprivileged_user_cant_reset_watches(self):
+        # It isn't possible for a user who isn't an admin or a member of
+        # the Launchpad Developers team to reset the watches for a bug
+        # tracker.
+        unprivileged_user = self.factory.makePerson()
+        login_person(unprivileged_user)
+        self.assertRaises(
+            Unauthorized, getattr, self.bug_tracker, 'resetWatches',
+            "Unprivileged users should not be allowed to reset a "
+            "tracker's watches.")
+
+    def test_admin_can_reset_watches(self):
+        # Launchpad admins can reset the watches on a bugtracker.
+        admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+        login_person(admin_user)
+        self.bug_tracker.resetWatches()
+        self._assertBugWatchesAreCheckedInTheFuture()
+
+    def test_lp_dev_can_reset_watches(self):
+        # Launchpad developers can reset the watches on a bugtracker.
+        login(ADMIN_EMAIL)
+        admin = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+        launchpad_developers = getUtility(
+            ILaunchpadCelebrities).launchpad_developers
+        lp_dev = self.factory.makePerson()
+        launchpad_developers.addMember(lp_dev, admin)
+        login_person(lp_dev)
+        self.bug_tracker.resetWatches()
+        self._assertBugWatchesAreCheckedInTheFuture()
+
+    def test_janitor_can_reset_watches(self):
+        # The Janitor can reset the watches on a bug tracker.
+        janitor = getUtility(ILaunchpadCelebrities).janitor
+        login_person(janitor)
+        self.bug_tracker.resetWatches()
+        self._assertBugWatchesAreCheckedInTheFuture()
+
+
+class TestMantis(TestCaseWithFactory):
+    """Tests for the Mantis-specific bug tracker code."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestMantis, self).setUp()
+        # We need to commit to avoid there being errors from the
+        # checkwatches isolation protection code.
+        transaction.commit()
+
     def test_mantis_login_redirects(self):
         # The Mantis bug tracker needs a special HTTP redirect handler
         # in order to login in. Ensure that redirects to the page with
@@ -230,11 +318,9 @@
 
 
 def test_suite():
-    suite = unittest.TestSuite()
+    suite = unittest.TestLoader().loadTestsFromName(__name__)
     doctest_suite = DocTestSuite(
         'lp.bugs.model.bugtracker',
         optionflags=NORMALIZE_WHITESPACE|ELLIPSIS)
-
-    suite.addTest(unittest.makeSuite(TestBugTracker))
     suite.addTest(doctest_suite)
     return suite

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-08-23 08:12:39 +0000
+++ lib/lp/testing/factory.py	2010-08-26 10:08:48 +0000
@@ -1310,7 +1310,8 @@
 
         return removeSecurityProxy(bug).addTask(owner, target)
 
-    def makeBugTracker(self, base_url=None, bugtrackertype=None):
+    def makeBugTracker(self, base_url=None, bugtrackertype=None, title=None,
+                       name=None):
         """Make a new bug tracker."""
         owner = self.makePerson()
 
@@ -1320,7 +1321,7 @@
             bugtrackertype = BugTrackerType.BUGZILLA
 
         return getUtility(IBugTrackerSet).ensureBugTracker(
-            base_url, owner, bugtrackertype)
+            base_url, owner, bugtrackertype, title=title, name=name)
 
     def makeBugWatch(self, remote_bug=None, bugtracker=None, bug=None,
                      owner=None, bug_task=None):

=== modified file 'lib/lp/testing/sampledata.py'
--- lib/lp/testing/sampledata.py	2010-08-26 06:45:38 +0000
+++ lib/lp/testing/sampledata.py	2010-08-26 10:08:48 +0000
@@ -9,6 +9,7 @@
 
 __metaclass__ = type
 __all__ = [
+    'ADMIN_EMAIL',
     'BUILDD_ADMIN_USERNAME',
     'CHROOT_LIBRARYFILEALIAS',
     'ADMIN_EMAIL',
@@ -23,6 +24,8 @@
     'UBUNTU_DEVELOPER_ADMIN_NAME',
     'UBUNTU_DISTRIBUTION_NAME',
     'UBUNTU_UPLOAD_TEAM_NAME',
+    'UBUNTUTEST_DISTRIBUTION_NAME',
+    'USER_EMAIL',
     'WARTY_DISTROSERIES_NAME',
     'WARTY_ONLY_SOURCEPACKAGENAME',
     'WARTY_ONLY_SOURCEPACKAGEVERSION',
@@ -34,6 +37,9 @@
 # UBUNTU_DEVELOPER_NAME. Where intent is tricky to convey in the
 # name, please leave a comment as well.
 
+# A user with Launchpad Admin privileges.
+ADMIN_EMAIL = '[e-mail]'
+
 # A user with buildd admin rights and upload rights to Ubuntu.
 BUILDD_ADMIN_USERNAME = 'cprov'
 # The LibraryFileAlias of a chroot for attaching to a DistroArchSeries
@@ -43,7 +49,10 @@
 LAUNCHPAD_ADMIN = '[e-mail]'
 LAUNCHPAD_DBUSER_NAME = 'launchpad'
 MAIN_COMPONENT_NAME = 'main'
+
 NO_PRIVILEGE_EMAIL = '[e-mail]'
+USER_EMAIL = '[e-mail]'
+VCS_IMPORTS_MEMBER_EMAIL = '[e-mail]'
 COMMERCIAL_ADMIN_EMAIL = '[e-mail]'
 ADMIN_EMAIL = '[e-mail]'
 SAMPLE_PERSON_EMAIL = '[e-mail]'