Launchpad itself

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

  1. lp-devs-can-reset-watches
  2. Merge into devel
Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 11462
Proposed branch: lp:~gmb/launchpad/lp-devs-can-reset-watches
Merge into: lp:launchpad
Diff against target: 339 lines (+212/-3)
8 files modified
lib/canonical/launchpad/security.py (+11/-0)
lib/lp/bugs/browser/bugwatch.py (+17/-0)
lib/lp/bugs/configure.zcml (+5/-2)
lib/lp/bugs/interfaces/bugwatch.py (+11/-0)
lib/lp/bugs/model/bugwatch.py (+9/-0)
lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt (+67/-0)
lib/lp/bugs/templates/bugwatch-portlet-details.pt (+1/-1)
lib/lp/bugs/tests/test_bugwatch.py (+91/-0)
To merge this branch: bzr merge lp:~gmb/launchpad/lp-devs-can-reset-watches
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+33796@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

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)
Revision history for this message
Graham Binns (gmb) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
56from lp.bugs.interfaces.bugnomination import IBugNomination56from lp.bugs.interfaces.bugnomination import IBugNomination
57from lp.bugs.interfaces.bugsubscription import IBugSubscription57from lp.bugs.interfaces.bugsubscription import IBugSubscription
58from lp.bugs.interfaces.bugtracker import IBugTracker58from lp.bugs.interfaces.bugtracker import IBugTracker
59from lp.bugs.interfaces.bugwatch import IBugWatch
59from lp.buildmaster.interfaces.builder import (60from lp.buildmaster.interfaces.builder import (
60 IBuilder,61 IBuilder,
61 IBuilderSet,62 IBuilderSet,
@@ -2572,3 +2573,13 @@
2572 user.in_janitor or2573 user.in_janitor or
2573 user.in_admin or2574 user.in_admin or
2574 user.in_launchpad_developers)2575 user.in_launchpad_developers)
2576
2577
2578class AdminBugWatch(AuthorizationBase):
2579 permission = 'launchpad.Admin'
2580 usedfor = IBugWatch
2581
2582 def checkAuthenticated(self, user):
2583 return (
2584 user.in_admin or
2585 user.in_launchpad_developers)
25752586
=== 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 @@
25 LaunchpadFormView,25 LaunchpadFormView,
26 LaunchpadView,26 LaunchpadView,
27 )27 )
28from canonical.launchpad.webapp.authorization import check_permission
28from canonical.launchpad.webapp.interfaces import ILaunchBag29from canonical.launchpad.webapp.interfaces import ILaunchBag
29from canonical.launchpad.webapp.menu import structured30from canonical.launchpad.webapp.menu import structured
30from canonical.widgets.textwidgets import URIWidget31from canonical.widgets.textwidgets import URIWidget
@@ -148,6 +149,22 @@
148 remote_bug=bugwatch.remotebug))149 remote_bug=bugwatch.remotebug))
149 bugwatch.bug.removeWatch(bugwatch, self.user)150 bugwatch.bug.removeWatch(bugwatch, self.user)
150151
152 def showResetActionCondition(self, action):
153 """Return True if the reset action can be shown to this user."""
154 return check_permission('launchpad.Admin', self.context)
155
156 @action('Reset this watch', name='reset',
157 condition=showResetActionCondition)
158 def reset_action(self, action, data):
159 bug_watch = self.context
160 bug_watch.reset()
161 self.request.response.addInfoNotification(
162 structured(
163 'The <a href="%(url)s">%(bugtracker)s #%(remote_bug)s</a>'
164 ' bug watch has been reset.',
165 url=bug_watch.url, bugtracker=bug_watch.bugtracker.name,
166 remote_bug=bug_watch.remotebug))
167
151 @property168 @property
152 def next_url(self):169 def next_url(self):
153 return canonical_url(getUtility(ILaunchBag).bug)170 return canonical_url(getUtility(ILaunchBag).bug)
154171
=== 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 @@
882 attributes="882 attributes="
883 addComment883 addComment
884 updateImportance884 updateImportance
885 updateStatus"885 updateStatus
886 reset
887 "
886 set_attributes="888 set_attributes="
887 last_error_type889 last_error_type
888 lastchanged890 lastchanged
@@ -890,7 +892,8 @@
890 needscheck892 needscheck
891 next_check893 next_check
892 remote_importance894 remote_importance
893 remotestatus"/>895 remotestatus
896 "/>
894 </class>897 </class>
895898
896 <!-- https://launchpad.net/bugs/98639 -->899 <!-- https://launchpad.net/bugs/98639 -->
897900
=== 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 @@
275 :raises: `BugWatchCannotBeRescheduled` if275 :raises: `BugWatchCannotBeRescheduled` if
276 `IBugWatch.can_be_rescheduled` is False.276 `IBugWatch.can_be_rescheduled` is False.
277 """277 """
278 def reset():
279 """Completely reset the watch.
280
281 When called, the following attributes are reset:
282 * last_error_type -> None
283 * lastchanged -> None
284 * lastchecked -> None
285 * nextcheck -> now
286 * remoteimportance -> None
287 * remotestatus -> None
288 """
278289
279290
280# Defined here because of circular imports.291# Defined here because of circular imports.
281292
=== 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 @@
364364
365 self.next_check = next_check365 self.next_check = next_check
366366
367 def reset(self):
368 """See `IBugWatch`."""
369 self.last_error_type = None
370 self.lastchanged = None
371 self.lastchecked = None
372 self.next_check = UTC_NOW
373 self.remote_importance = None
374 self.remotestatus = None
375
367376
368class BugWatchSet(BugSetBase):377class BugWatchSet(BugSetBase):
369 """A set for BugWatch"""378 """A set for BugWatch"""
370379
=== 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 @@
261 Traceback (most recent call last):261 Traceback (most recent call last):
262 ...262 ...
263 LookupError: label 'Update Now'263 LookupError: label 'Update Now'
264
265
266Resetting a watch
267-----------------
268
269It's possible to reset a watch at any time by clicking the "Reset this
270watch" button on the watch's page.
271
272 >>> from lp.testing.sampledata import ADMIN_EMAIL
273 >>> login(ADMIN_EMAIL)
274 >>> bug_watch = factory.makeBugWatch()
275 >>> bug_watch.lastchecked = datetime.now(utc)
276 >>> watch_url = (
277 ... 'http://bugs.launchpad.dev/bugs/%s/+watch/%s' %
278 ... (bug_watch.bug.id, bug_watch.id))
279 >>> logout()
280
281The "Reset this watch" button will appear for administrators.
282
283 >>> admin_browser.open(watch_url)
284 >>> admin_browser.getControl('Reset this watch')
285 <SubmitControl...>
286
287It also appears for Launchpad Developers.
288
289 >>> from zope.component import getUtility
290 >>> from canonical.launchpad.interfaces.launchpad import (
291 ... ILaunchpadCelebrities)
292 >>> from lp.registry.interfaces.person import IPersonSet
293
294 >>> login(ADMIN_EMAIL)
295 >>> admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
296 >>> new_lp_developer = factory.makePerson(password='newpassword')
297 >>> launchpad_developers = getUtility(
298 ... ILaunchpadCelebrities).launchpad_developers
299 >>> dev_added = launchpad_developers.addMember(
300 ... new_lp_developer, admin_user)
301
302 >>> lp_dev_browser = setupBrowser(
303 ... auth='Basic %s:newpassword' %
304 ... new_lp_developer.preferredemail.email)
305 >>> logout()
306
307 >>> lp_dev_browser.open(watch_url)
308 >>> reset_button = lp_dev_browser.getControl('Reset this watch')
309
310Clicking the button will reset the watch completely.
311
312 >>> reset_button.click()
313 >>> for message in find_tags_by_class(
314 ... lp_dev_browser.contents, 'informational message'):
315 ... print extract_text(message)
316 The ... bug watch has been reset.
317
318 >>> data_tag = find_tag_by_id(
319 ... user_browser.contents, 'bugwatch-lastchecked')
320 >>> print extract_text(data_tag.renderContents())
321 Checked:
322
323Should a non-admin, non-Launchpad-developer user visit the page, the
324button will not appear.
325
326 >>> user_browser.open(watch_url)
327 >>> user_browser.getControl('Reset this watch')
328 Traceback (most recent call last):
329 ...
330 LookupError: label 'Reset this watch'
264331
=== 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 @@
38 </span>38 </span>
39 </dd>39 </dd>
40 </dl>40 </dl>
41 <dl class="bugwatch-data">41 <dl class="bugwatch-data" id="bugwatch-lastchecked">
42 <dt>Checked:</dt>42 <dt>Checked:</dt>
43 <dd>43 <dd>
44 <span44 <span
4545
=== 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 @@
6__metaclass__ = type6__metaclass__ = type
77
8import unittest8import unittest
9from datetime import (
10 datetime,
11 timedelta,
12 )
13from pytz import utc
9from urlparse import urlunsplit14from urlparse import urlunsplit
1015
11import transaction16import transaction
12from zope.component import getUtility17from zope.component import getUtility
18from zope.security.interfaces import Unauthorized
13from zope.security.proxy import removeSecurityProxy19from zope.security.proxy import removeSecurityProxy
1420
21from lazr.lifecycle.snapshot import Snapshot
22
15from canonical.database.constants import UTC_NOW23from canonical.database.constants import UTC_NOW
16from canonical.launchpad.ftests import (24from canonical.launchpad.ftests import (
17 ANONYMOUS,25 ANONYMOUS,
18 login,26 login,
19 )27 )
28from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
20from canonical.launchpad.scripts.garbo import BugWatchActivityPruner29from canonical.launchpad.scripts.garbo import BugWatchActivityPruner
21from canonical.launchpad.scripts.logger import QuietFakeLogger30from canonical.launchpad.scripts.logger import QuietFakeLogger
22from canonical.launchpad.webapp import urlsplit31from canonical.launchpad.webapp import urlsplit
@@ -46,6 +55,7 @@
46 login_person,55 login_person,
47 TestCaseWithFactory,56 TestCaseWithFactory,
48 )57 )
58from lp.testing.sampledata import ADMIN_EMAIL
4959
5060
51class ExtractBugTrackerAndBugTestBase:61class ExtractBugTrackerAndBugTestBase:
@@ -672,3 +682,84 @@
672 messages = [activity.message for activity in self.bug_watch.activity]682 messages = [activity.message for activity in self.bug_watch.activity]
673 for i in range(MAX_SAMPLE_SIZE):683 for i in range(MAX_SAMPLE_SIZE):
674 self.failUnless("Activity %s" % i in messages)684 self.failUnless("Activity %s" % i in messages)
685
686
687class TestBugWatchResetting(TestCaseWithFactory):
688
689 layer = LaunchpadFunctionalLayer
690
691 def setUp(self):
692 super(TestBugWatchResetting, self).setUp(user=ADMIN_EMAIL)
693 self.bug_watch = self.factory.makeBugWatch()
694 self.bug_watch.last_error_type = BugWatchActivityStatus.BUG_NOT_FOUND
695 self.bug_watch.lastchanged = datetime.now(utc) - timedelta(days=1)
696 self.bug_watch.lastchecked = datetime.now(utc) - timedelta(days=1)
697 self.bug_watch.next_check = datetime.now(utc) + timedelta(days=7)
698 self.bug_watch.remote_importance = 'IMPORTANT'
699 self.bug_watch.remotestatus = 'FIXED'
700 self.default_bug_watch_fields = [
701 'last_error_type',
702 'lastchanged',
703 'lastchecked',
704 'next_check',
705 'remote_importance',
706 'remotestatus',
707 ]
708 self.original_bug_watch = Snapshot(
709 self.bug_watch, self.default_bug_watch_fields)
710
711 def _assertBugWatchHasBeenChanged(self, expected_changes=None):
712 """Assert that a bug watch has been changed.
713
714 :param expected_changes: A list of the attribute names that are
715 expected to have changed. If supplied, an assertion error
716 will be raised if one of the expected_changes members has
717 not changed *or* an attribute not in expected_changes has
718 changed. If not supplied, *all* attributes are expected to
719 have changed.
720 """
721 if expected_changes is None:
722 expected_changes = self.default_bug_watch_fields
723
724 actual_changes = []
725 has_changed = True
726 for expected_change in expected_changes:
727 original_value = getattr(self.original_bug_watch, expected_change)
728 current_value = getattr(self.bug_watch, expected_change)
729 if original_value != current_value:
730 has_changed = has_changed and True
731 actual_changes.append(expected_change)
732 else:
733 has_changed = False
734
735 self.assertTrue(
736 has_changed,
737 "Bug watch did not change as expected.\n"
738 "Expected changes: %s\n"
739 "Actual changes: %s" % (expected_changes, actual_changes))
740
741 def test_reset_resets(self):
742 # Calling reset() on a watch resets all of the attributes of the
743 # bug watch.
744 admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
745 login_person(admin_user)
746 self.bug_watch.reset()
747 self._assertBugWatchHasBeenChanged()
748
749 def test_unprivileged_user_cant_reset_watches(self):
750 # An unprivileged user can't call the reset() method on a bug
751 # watch.
752 unprivileged_user = self.factory.makePerson()
753 login_person(unprivileged_user)
754 self.assertRaises(Unauthorized, getattr, self.bug_watch, 'reset')
755
756 def test_lp_developer_can_reset_watches(self):
757 # A Launchpad developer can call the reset() method on a bug
758 # watch.
759 admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
760 lp_developers = getUtility(ILaunchpadCelebrities).launchpad_developers
761 lp_dev = self.factory.makePerson()
762 lp_developers.addMember(lp_dev, admin_user)
763 login_person(lp_dev)
764 self.bug_watch.reset()
765 self._assertBugWatchHasBeenChanged()