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
1=== modified file 'lib/canonical/launchpad/security.py'
2--- lib/canonical/launchpad/security.py 2010-08-26 12:33:03 +0000
3+++ lib/canonical/launchpad/security.py 2010-08-27 10:03:47 +0000
4@@ -56,6 +56,7 @@
5 from lp.bugs.interfaces.bugnomination import IBugNomination
6 from lp.bugs.interfaces.bugsubscription import IBugSubscription
7 from lp.bugs.interfaces.bugtracker import IBugTracker
8+from lp.bugs.interfaces.bugwatch import IBugWatch
9 from lp.buildmaster.interfaces.builder import (
10 IBuilder,
11 IBuilderSet,
12@@ -2572,3 +2573,13 @@
13 user.in_janitor or
14 user.in_admin or
15 user.in_launchpad_developers)
16+
17+
18+class AdminBugWatch(AuthorizationBase):
19+ permission = 'launchpad.Admin'
20+ usedfor = IBugWatch
21+
22+ def checkAuthenticated(self, user):
23+ return (
24+ user.in_admin or
25+ user.in_launchpad_developers)
26
27=== modified file 'lib/lp/bugs/browser/bugwatch.py'
28--- lib/lp/bugs/browser/bugwatch.py 2010-08-20 20:31:18 +0000
29+++ lib/lp/bugs/browser/bugwatch.py 2010-08-27 10:03:47 +0000
30@@ -25,6 +25,7 @@
31 LaunchpadFormView,
32 LaunchpadView,
33 )
34+from canonical.launchpad.webapp.authorization import check_permission
35 from canonical.launchpad.webapp.interfaces import ILaunchBag
36 from canonical.launchpad.webapp.menu import structured
37 from canonical.widgets.textwidgets import URIWidget
38@@ -148,6 +149,22 @@
39 remote_bug=bugwatch.remotebug))
40 bugwatch.bug.removeWatch(bugwatch, self.user)
41
42+ def showResetActionCondition(self, action):
43+ """Return True if the reset action can be shown to this user."""
44+ return check_permission('launchpad.Admin', self.context)
45+
46+ @action('Reset this watch', name='reset',
47+ condition=showResetActionCondition)
48+ def reset_action(self, action, data):
49+ bug_watch = self.context
50+ bug_watch.reset()
51+ self.request.response.addInfoNotification(
52+ structured(
53+ 'The <a href="%(url)s">%(bugtracker)s #%(remote_bug)s</a>'
54+ ' bug watch has been reset.',
55+ url=bug_watch.url, bugtracker=bug_watch.bugtracker.name,
56+ remote_bug=bug_watch.remotebug))
57+
58 @property
59 def next_url(self):
60 return canonical_url(getUtility(ILaunchBag).bug)
61
62=== modified file 'lib/lp/bugs/configure.zcml'
63--- lib/lp/bugs/configure.zcml 2010-08-26 12:33:03 +0000
64+++ lib/lp/bugs/configure.zcml 2010-08-27 10:03:47 +0000
65@@ -882,7 +882,9 @@
66 attributes="
67 addComment
68 updateImportance
69- updateStatus"
70+ updateStatus
71+ reset
72+ "
73 set_attributes="
74 last_error_type
75 lastchanged
76@@ -890,7 +892,8 @@
77 needscheck
78 next_check
79 remote_importance
80- remotestatus"/>
81+ remotestatus
82+ "/>
83 </class>
84
85 <!-- https://launchpad.net/bugs/98639 -->
86
87=== modified file 'lib/lp/bugs/interfaces/bugwatch.py'
88--- lib/lp/bugs/interfaces/bugwatch.py 2010-08-20 20:31:18 +0000
89+++ lib/lp/bugs/interfaces/bugwatch.py 2010-08-27 10:03:47 +0000
90@@ -275,6 +275,17 @@
91 :raises: `BugWatchCannotBeRescheduled` if
92 `IBugWatch.can_be_rescheduled` is False.
93 """
94+ def reset():
95+ """Completely reset the watch.
96+
97+ When called, the following attributes are reset:
98+ * last_error_type -> None
99+ * lastchanged -> None
100+ * lastchecked -> None
101+ * nextcheck -> now
102+ * remoteimportance -> None
103+ * remotestatus -> None
104+ """
105
106
107 # Defined here because of circular imports.
108
109=== modified file 'lib/lp/bugs/model/bugwatch.py'
110--- lib/lp/bugs/model/bugwatch.py 2010-08-20 20:31:18 +0000
111+++ lib/lp/bugs/model/bugwatch.py 2010-08-27 10:03:47 +0000
112@@ -364,6 +364,15 @@
113
114 self.next_check = next_check
115
116+ def reset(self):
117+ """See `IBugWatch`."""
118+ self.last_error_type = None
119+ self.lastchanged = None
120+ self.lastchecked = None
121+ self.next_check = UTC_NOW
122+ self.remote_importance = None
123+ self.remotestatus = None
124+
125
126 class BugWatchSet(BugSetBase):
127 """A set for BugWatch"""
128
129=== modified file 'lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt'
130--- lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt 2010-05-17 15:10:01 +0000
131+++ lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt 2010-08-27 10:03:47 +0000
132@@ -261,3 +261,70 @@
133 Traceback (most recent call last):
134 ...
135 LookupError: label 'Update Now'
136+
137+
138+Resetting a watch
139+-----------------
140+
141+It's possible to reset a watch at any time by clicking the "Reset this
142+watch" button on the watch's page.
143+
144+ >>> from lp.testing.sampledata import ADMIN_EMAIL
145+ >>> login(ADMIN_EMAIL)
146+ >>> bug_watch = factory.makeBugWatch()
147+ >>> bug_watch.lastchecked = datetime.now(utc)
148+ >>> watch_url = (
149+ ... 'http://bugs.launchpad.dev/bugs/%s/+watch/%s' %
150+ ... (bug_watch.bug.id, bug_watch.id))
151+ >>> logout()
152+
153+The "Reset this watch" button will appear for administrators.
154+
155+ >>> admin_browser.open(watch_url)
156+ >>> admin_browser.getControl('Reset this watch')
157+ <SubmitControl...>
158+
159+It also appears for Launchpad Developers.
160+
161+ >>> from zope.component import getUtility
162+ >>> from canonical.launchpad.interfaces.launchpad import (
163+ ... ILaunchpadCelebrities)
164+ >>> from lp.registry.interfaces.person import IPersonSet
165+
166+ >>> login(ADMIN_EMAIL)
167+ >>> admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
168+ >>> new_lp_developer = factory.makePerson(password='newpassword')
169+ >>> launchpad_developers = getUtility(
170+ ... ILaunchpadCelebrities).launchpad_developers
171+ >>> dev_added = launchpad_developers.addMember(
172+ ... new_lp_developer, admin_user)
173+
174+ >>> lp_dev_browser = setupBrowser(
175+ ... auth='Basic %s:newpassword' %
176+ ... new_lp_developer.preferredemail.email)
177+ >>> logout()
178+
179+ >>> lp_dev_browser.open(watch_url)
180+ >>> reset_button = lp_dev_browser.getControl('Reset this watch')
181+
182+Clicking the button will reset the watch completely.
183+
184+ >>> reset_button.click()
185+ >>> for message in find_tags_by_class(
186+ ... lp_dev_browser.contents, 'informational message'):
187+ ... print extract_text(message)
188+ The ... bug watch has been reset.
189+
190+ >>> data_tag = find_tag_by_id(
191+ ... user_browser.contents, 'bugwatch-lastchecked')
192+ >>> print extract_text(data_tag.renderContents())
193+ Checked:
194+
195+Should a non-admin, non-Launchpad-developer user visit the page, the
196+button will not appear.
197+
198+ >>> user_browser.open(watch_url)
199+ >>> user_browser.getControl('Reset this watch')
200+ Traceback (most recent call last):
201+ ...
202+ LookupError: label 'Reset this watch'
203
204=== modified file 'lib/lp/bugs/templates/bugwatch-portlet-details.pt'
205--- lib/lp/bugs/templates/bugwatch-portlet-details.pt 2010-04-15 15:14:21 +0000
206+++ lib/lp/bugs/templates/bugwatch-portlet-details.pt 2010-08-27 10:03:47 +0000
207@@ -38,7 +38,7 @@
208 </span>
209 </dd>
210 </dl>
211- <dl class="bugwatch-data">
212+ <dl class="bugwatch-data" id="bugwatch-lastchecked">
213 <dt>Checked:</dt>
214 <dd>
215 <span
216
217=== modified file 'lib/lp/bugs/tests/test_bugwatch.py'
218--- lib/lp/bugs/tests/test_bugwatch.py 2010-08-20 20:31:18 +0000
219+++ lib/lp/bugs/tests/test_bugwatch.py 2010-08-27 10:03:47 +0000
220@@ -6,17 +6,26 @@
221 __metaclass__ = type
222
223 import unittest
224+from datetime import (
225+ datetime,
226+ timedelta,
227+ )
228+from pytz import utc
229 from urlparse import urlunsplit
230
231 import transaction
232 from zope.component import getUtility
233+from zope.security.interfaces import Unauthorized
234 from zope.security.proxy import removeSecurityProxy
235
236+from lazr.lifecycle.snapshot import Snapshot
237+
238 from canonical.database.constants import UTC_NOW
239 from canonical.launchpad.ftests import (
240 ANONYMOUS,
241 login,
242 )
243+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
244 from canonical.launchpad.scripts.garbo import BugWatchActivityPruner
245 from canonical.launchpad.scripts.logger import QuietFakeLogger
246 from canonical.launchpad.webapp import urlsplit
247@@ -46,6 +55,7 @@
248 login_person,
249 TestCaseWithFactory,
250 )
251+from lp.testing.sampledata import ADMIN_EMAIL
252
253
254 class ExtractBugTrackerAndBugTestBase:
255@@ -672,3 +682,84 @@
256 messages = [activity.message for activity in self.bug_watch.activity]
257 for i in range(MAX_SAMPLE_SIZE):
258 self.failUnless("Activity %s" % i in messages)
259+
260+
261+class TestBugWatchResetting(TestCaseWithFactory):
262+
263+ layer = LaunchpadFunctionalLayer
264+
265+ def setUp(self):
266+ super(TestBugWatchResetting, self).setUp(user=ADMIN_EMAIL)
267+ self.bug_watch = self.factory.makeBugWatch()
268+ self.bug_watch.last_error_type = BugWatchActivityStatus.BUG_NOT_FOUND
269+ self.bug_watch.lastchanged = datetime.now(utc) - timedelta(days=1)
270+ self.bug_watch.lastchecked = datetime.now(utc) - timedelta(days=1)
271+ self.bug_watch.next_check = datetime.now(utc) + timedelta(days=7)
272+ self.bug_watch.remote_importance = 'IMPORTANT'
273+ self.bug_watch.remotestatus = 'FIXED'
274+ self.default_bug_watch_fields = [
275+ 'last_error_type',
276+ 'lastchanged',
277+ 'lastchecked',
278+ 'next_check',
279+ 'remote_importance',
280+ 'remotestatus',
281+ ]
282+ self.original_bug_watch = Snapshot(
283+ self.bug_watch, self.default_bug_watch_fields)
284+
285+ def _assertBugWatchHasBeenChanged(self, expected_changes=None):
286+ """Assert that a bug watch has been changed.
287+
288+ :param expected_changes: A list of the attribute names that are
289+ expected to have changed. If supplied, an assertion error
290+ will be raised if one of the expected_changes members has
291+ not changed *or* an attribute not in expected_changes has
292+ changed. If not supplied, *all* attributes are expected to
293+ have changed.
294+ """
295+ if expected_changes is None:
296+ expected_changes = self.default_bug_watch_fields
297+
298+ actual_changes = []
299+ has_changed = True
300+ for expected_change in expected_changes:
301+ original_value = getattr(self.original_bug_watch, expected_change)
302+ current_value = getattr(self.bug_watch, expected_change)
303+ if original_value != current_value:
304+ has_changed = has_changed and True
305+ actual_changes.append(expected_change)
306+ else:
307+ has_changed = False
308+
309+ self.assertTrue(
310+ has_changed,
311+ "Bug watch did not change as expected.\n"
312+ "Expected changes: %s\n"
313+ "Actual changes: %s" % (expected_changes, actual_changes))
314+
315+ def test_reset_resets(self):
316+ # Calling reset() on a watch resets all of the attributes of the
317+ # bug watch.
318+ admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
319+ login_person(admin_user)
320+ self.bug_watch.reset()
321+ self._assertBugWatchHasBeenChanged()
322+
323+ def test_unprivileged_user_cant_reset_watches(self):
324+ # An unprivileged user can't call the reset() method on a bug
325+ # watch.
326+ unprivileged_user = self.factory.makePerson()
327+ login_person(unprivileged_user)
328+ self.assertRaises(Unauthorized, getattr, self.bug_watch, 'reset')
329+
330+ def test_lp_developer_can_reset_watches(self):
331+ # A Launchpad developer can call the reset() method on a bug
332+ # watch.
333+ admin_user = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
334+ lp_developers = getUtility(ILaunchpadCelebrities).launchpad_developers
335+ lp_dev = self.factory.makePerson()
336+ lp_developers.addMember(lp_dev, admin_user)
337+ login_person(lp_dev)
338+ self.bug_watch.reset()
339+ self._assertBugWatchHasBeenChanged()