Merge lp:~gmb/launchpad/lp-devs-can-reset-watches into lp:launchpad

[Graham Binns]

A button has been added to bug watch pages to allow admins to completely reset a bug watch.

This branch adds a button to the BugWatch +edit page, visible to LP devs and admins. The button allows the user to completely reset the bug watch so that checkwatches treats it as a brand new watch. This is distinct from the "reschedule this watch" button, which merely re-queues the watch for updating.

I've added a new AuthorizationBase for IBugWatch, defining the people who have launchpad.Admin permission on a BugWatch as Admins and Launchpad developers.

I've added a reset() method to BugWatch and a related action to BugWatchEditView. I've added unittests for BugWatch.reset() to lp.bugs.tests.test_bugwatch and a story to cover the use of the 'Reset this watch' button.

[Abel Deuring (adeuring)]

Hi Graham,

nice work! Just one suggestion:

> === modified file 'lib/lp/bugs/browser/bugwatch.py'
> --- lib/lp/bugs/browser/bugwatch.py 2010-08-20 20:31:18 +0000
> +++ lib/lp/bugs/browser/bugwatch.py 2010-08-26 15:46:37 +0000
> @@ -25,6 +25,7 @@
> LaunchpadFormView,
> LaunchpadView,
> )
> +from canonical.launchpad.webapp.authorization import check_permission
> from canonical.launchpad.webapp.interfaces import ILaunchBag
> from canonical.launchpad.webapp.menu import structured
> from canonical.widgets.textwidgets import URIWidget
> @@ -148,6 +149,22 @@
> remote_bug=bugwatch.remotebug))
> bugwatch.bug.removeWatch(bugwatch, self.user)
>
> + def resetBugWatchCondition(self, action):
> + """Return True if the reset action can be shown to this user."""
> + return check_permission('launchpad.Admin', self.context)
> +

Perhaps I did not have enough coffee, but I think this method name could also mean "is the bug watch in a state that it could/should be reset?". What about "userCanResetBugWatch"?

review: Approve (code)

[Graham Binns (gmb)]

From IRC:

<gmb> adeuring, foo*Condition is the usual naming idiom for methods used to decide whether or not we can display an action. Maybe showResetActionCondition() would be clearer.
<adeuring> gmb: right, that's better!
<gmb> Right, I'll do that, then.

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-08-26 12:33:03 +0000
+++ lib/canonical/launchpad/security.py	2010-08-27 10:03:47 +0000
@@ -56,6 +56,7 @@
 from lp.bugs.interfaces.bugnomination import IBugNomination
 from lp.bugs.interfaces.bugsubscription import IBugSubscription
 from lp.bugs.interfaces.bugtracker import IBugTracker
+from lp.bugs.interfaces.bugwatch import IBugWatch
 from lp.buildmaster.interfaces.builder import (
     IBuilder,
     IBuilderSet,
@@ -2572,3 +2573,13 @@
             user.in_janitor or
             user.in_admin or
             user.in_launchpad_developers)
+
+
+class AdminBugWatch(AuthorizationBase):
+    permission = 'launchpad.Admin'
+    usedfor = IBugWatch
+
+    def checkAuthenticated(self, user):
+        return (
+            user.in_admin or
+            user.in_launchpad_developers)

=== modified file 'lib/lp/bugs/browser/bugwatch.py'
--- lib/lp/bugs/browser/bugwatch.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/browser/bugwatch.py	2010-08-27 10:03:47 +0000
@@ -25,6 +25,7 @@
     LaunchpadFormView,
     LaunchpadView,
     )
+from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.interfaces import ILaunchBag
 from canonical.launchpad.webapp.menu import structured
 from canonical.widgets.textwidgets import URIWidget
@@ -148,6 +149,22 @@
             remote_bug=bugwatch.remotebug))
         bugwatch.bug.removeWatch(bugwatch, self.user)
 
+    def showResetActionCondition(self, action):
+        """Return True if the reset action can be shown to this user."""
+        return check_permission('launchpad.Admin', self.context)
+
+    @action('Reset this watch', name='reset',
+            condition=showResetActionCondition)
+    def reset_action(self, action, data):
+        bug_watch = self.context
+        bug_watch.reset()
+        self.request.response.addInfoNotification(
+            structured(
+            'The <a href="%(url)s">%(bugtracker)s #%(remote_bug)s</a>'
+            ' bug watch has been reset.',
+            url=bug_watch.url, bugtracker=bug_watch.bugtracker.name,
+            remote_bug=bug_watch.remotebug))
+
     @property
     def next_url(self):
         return canonical_url(getUtility(ILaunchBag).bug)

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2010-08-26 12:33:03 +0000
+++ lib/lp/bugs/configure.zcml	2010-08-27 10:03:47 +0000
@@ -882,7 +882,9 @@
                 attributes="
                     addComment
                     updateImportance
-                    updateStatus"
+                    updateStatus
+                    reset
+                    "
                 set_attributes="
                     last_error_type
                     lastchanged
@@ -890,7 +892,8 @@
                     needscheck
                     next_check
                     remote_importance
-                    remotestatus"/>
+                    remotestatus
+                    "/>
         </class>
 
         <!-- https://launchpad.net/bugs/98639 -->

=== modified file 'lib/lp/bugs/interfaces/bugwatch.py'
--- lib/lp/bugs/interfaces/bugwatch.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/interfaces/bugwatch.py	2010-08-27 10:03:47 +0000
@@ -275,6 +275,17 @@
         :raises: `BugWatchCannotBeRescheduled` if
                  `IBugWatch.can_be_rescheduled` is False.
         """
+    def reset():
+        """Completely reset the watch.
+
+        When called, the following attributes are reset:
+         * last_error_type -> None
+         * lastchanged -> None
+         * lastchecked -> None
+         * nextcheck -> now
+         * remoteimportance -> None
+         * remotestatus -> None
+        """
 
 
 # Defined here because of circular imports.

=== modified file 'lib/lp/bugs/model/bugwatch.py'
--- lib/lp/bugs/model/bugwatch.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/model/bugwatch.py	2010-08-27 10:03:47 +0000
@@ -364,6 +364,15 @@
 
         self.next_check = next_check
 
+    def reset(self):
+        """See `IBugWatch`."""
+        self.last_error_type = None
+        self.lastchanged = None
+        self.lastchecked = None
+        self.next_check = UTC_NOW
+        self.remote_importance = None
+        self.remotestatus = None
+
 
 class BugWatchSet(BugSetBase):
     """A set for BugWatch"""

=== modified file 'lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt'
--- lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt	2010-05-17 15:10:01 +0000
+++ lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt	2010-08-27 10:03:47 +0000
@@ -261,3 +261,70 @@
     Traceback (most recent call last):
       ...
     LookupError: label 'Update Now'
+
+
+Resetting a watch
+-----------------
+
+It's possible to reset a watch at any time by clicking the "Reset this
+watch" button on the watch's page.
+
+    >>> from lp.testing.sampledata import ADMIN_EMAIL
+    >>> login(ADMIN_EMAIL)
+    >>> bug_watch = factory.makeBugWatch()
+    >>> bug_watch.lastchecked = datetime.now(utc)
+    >>> watch_url = (
+    ...     'http://bugs.launchpad.dev/bugs/%s/+watch/%s' %
+    ...     (bug_watch.bug.id, bug_watch.id))
+    >>> logout()
+
+The "Reset this watch" button will appear for administrators.
+
+    >>> admin_browser.open(watch_url)
+    >>> admin_browser.getControl('Reset this watch')
+    <SubmitControl...>
+
+It also appears for Launchpad Developers.
+
+    >>> 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(watch_url)
+    >>> reset_button = lp_dev_browser.getControl('Reset this watch')
+
+Clicking the button will reset the watch completely.
+
+    >>> reset_button.click()
+    >>> for message in find_tags_by_class(
+    ...     lp_dev_browser.contents, 'informational message'):
+    ...     print extract_text(message)
+    The ... bug watch has been reset.
+
+    >>> data_tag = find_tag_by_id(
+    ...     user_browser.contents, 'bugwatch-lastchecked')
+    >>> print extract_text(data_tag.renderContents())
+    Checked:
+
+Should a non-admin, non-Launchpad-developer user visit the page, the
+button will not appear.
+
+    >>> user_browser.open(watch_url)
+    >>> user_browser.getControl('Reset this watch')
+    Traceback (most recent call last):
+      ...
+    LookupError: label 'Reset this watch'

=== modified file 'lib/lp/bugs/templates/bugwatch-portlet-details.pt'
--- lib/lp/bugs/templates/bugwatch-portlet-details.pt	2010-04-15 15:14:21 +0000
+++ lib/lp/bugs/templates/bugwatch-portlet-details.pt	2010-08-27 10:03:47 +0000
@@ -38,7 +38,7 @@
            </span>
         </dd>
       </dl>
-      <dl class="bugwatch-data">
+      <dl class="bugwatch-data" id="bugwatch-lastchecked">
         <dt>Checked:</dt>
         <dd>
           <span

=== modified file 'lib/lp/bugs/tests/test_bugwatch.py'
--- lib/lp/bugs/tests/test_bugwatch.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/tests/test_bugwatch.py	2010-08-27 10:03:47 +0000
@@ -6,17 +6,26 @@
 __metaclass__ = type
 
 import unittest
+from datetime import (
+    datetime,
+    timedelta,
+    )
+from pytz import utc
 from urlparse import urlunsplit
 
 import transaction
 from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
+from lazr.lifecycle.snapshot import Snapshot
+
 from canonical.database.constants import UTC_NOW
 from canonical.launchpad.ftests import (
     ANONYMOUS,
     login,
     )
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.scripts.garbo import BugWatchActivityPruner
 from canonical.launchpad.scripts.logger import QuietFakeLogger
 from canonical.launchpad.webapp import urlsplit
@@ -46,6 +55,7 @@
     login_person,
     TestCaseWithFactory,
     )
+from lp.testing.sampledata import ADMIN_EMAIL
 
 
 class ExtractBugTrackerAndBugTestBase:
@@ -672,3 +682,84 @@
         messages = [activity.message for activity in self.bug_watch.activity]
         for i in range(MAX_SAMPLE_SIZE):
             self.failUnless("Activity %s" % i in messages)
+
+
+class TestBugWatchResetting(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestBugWatchResetting, self).setUp(user=ADMIN_EMAIL)
+        self.bug_watch = self.factory.makeBugWatch()
+        self.bug_watch.last_error_type = BugWatchActivityStatus.BUG_NOT_FOUND
+        self.bug_watch.lastchanged = datetime.now(utc) - timedelta(days=1)
+        self.bug_watch.lastchecked = datetime.now(utc) - timedelta(days=1)
+        self.bug_watch.next_check = datetime.now(utc) + timedelta(days=7)
+        self.bug_watch.remote_importance = 'IMPORTANT'
+        self.bug_watch.remotestatus = 'FIXED'
+        self.default_bug_watch_fields = [
+            'last_error_type',
+            'lastchanged',
+            'lastchecked',
+            'next_check',
+            'remote_importance',
+            'remotestatus',
+            ]
+        self.original_bug_watch = Snapshot(
+            self.bug_watch, self.default_bug_watch_fields)
+
+    def _assertBugWatchHasBeenChanged(self, expected_changes=None):
+        """Assert that a bug watch has been changed.
+
+        :param expected_changes: A list of the attribute names that are
+            expected to have changed. If supplied, an assertion error
+            will be raised if one of the expected_changes members has
+            not changed *or* an attribute not in expected_changes has
+            changed. If not supplied, *all* attributes are expected to
+            have changed.
+        """
+        if expected_changes is None:
+            expected_changes = self.default_bug_watch_fields
+
+        actual_changes = []
+        has_changed = True
+        for expected_change in expected_changes:
+            original_value = getattr(self.original_bug_watch, expected_change)
+            current_value = getattr(self.bug_watch, expected_change)
+            if original_value != current_value:
+                has_changed = has_changed and True
+                actual_changes.append(expected_change)
+            else:
+                has_changed = False
+
+        self.assertTrue(
+            has_changed,
+            "Bug watch did not change as expected.\n"
+            "Expected changes: %s\n"
+            "Actual changes: %s" % (expected_changes, actual_changes))
+
+    def test_reset_resets(self):
+        # Calling reset() on a watch resets all of the attributes of the
+        # bug watch.
+        admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+        login_person(admin_user)
+        self.bug_watch.reset()
+        self._assertBugWatchHasBeenChanged()
+
+    def test_unprivileged_user_cant_reset_watches(self):
+        # An unprivileged user can't call the reset() method on a bug
+        # watch.
+        unprivileged_user = self.factory.makePerson()
+        login_person(unprivileged_user)
+        self.assertRaises(Unauthorized, getattr, self.bug_watch, 'reset')
+
+    def test_lp_developer_can_reset_watches(self):
+        # A Launchpad developer can call the reset() method on a bug
+        # watch.
+        admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+        lp_developers = getUtility(ILaunchpadCelebrities).launchpad_developers
+        lp_dev = self.factory.makePerson()
+        lp_developers.addMember(lp_dev, admin_user)
+        login_person(lp_dev)
+        self.bug_watch.reset()
+        self._assertBugWatchHasBeenChanged()