Launchpad itself

Merge lp:~gmb/launchpad/reschedule-button-qafix-bug-558415 into lp:launchpad/db-devel

  1. reschedule-button-qafix-bug-558415
  2. Merge into db-devel
Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~gmb/launchpad/reschedule-button-qafix-bug-558415
Merge into: lp:launchpad/db-devel
Diff against target: 475 lines (+173/-112)
8 files modified
lib/lp/bugs/browser/bugwatch.py (+2/-41)
lib/lp/bugs/browser/tests/bugwatch-views.txt (+0/-55)
lib/lp/bugs/configure.zcml (+4/-1)
lib/lp/bugs/doc/bugwatch.txt (+84/-0)
lib/lp/bugs/interfaces/bugwatch.py (+15/-0)
lib/lp/bugs/model/bugwatch.py (+55/-2)
lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt (+11/-11)
lib/lp/bugs/templates/bugwatch-portlet-activity.pt (+2/-2)
To merge this branch: bzr merge lp:~gmb/launchpad/reschedule-button-qafix-bug-558415
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+23698@code.launchpad.net

Commit message

Bug watches can now be rescheduled by any user rather than just administrators.

Description of the change

This branch fixes a bug with the orignal fix for bug 558415 whereby the
reschedule button would be shown on a bug watch page but nobody but an
admin could click it without it OOPSing.

To fix this I've added a setNextCheck() method to IBugWatch. This can be
called by any user. The upshot of this is that users have a way of
rescheduling a watch, which we can control, without changing next_check
directly.

== lib/lp/bugs/browser/bugwatch.py ==

 - I've modified the BugWatchActivityPortletView to use
   BugWatch.setNextCheck() as described below.

== lib/lp/bugs/browser/tests/bugwatch-views.txt ==

 - I've moved the tests from this file into doc/bugwatch.txt since the
   logic for determining whether a watch can be updated is now part of
   the watch, not the view.

== lib/lp/bugs/configure.zcml ==

 - I've updated the ZCML as appropriate.

== lib/lp/bugs/doc/bugwatch.txt ==

 - I've moved the test from bugwatch-views.txt here and updated them as
   appropriate.
 - I've added tests for BugWatch.setNextCheck()

== lib/lp/bugs/interfaces/bugwatch.py ==

 - I've added a setNextCheck() method and can_be_rescheduled and
   failed_activity properties to IBugWatch.

== lib/lp/bugs/model/bugwatch.py ==

 - I've moved the implementation of userCanReschedule() here from the
   BugWatchActivityPortletView can called it can_be_rescheduled.
 - I've added implementations of IBugWatch.failed_activity and
   IBugWatch.setNextCheck().

== lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt ==

 - I've updated the test to use the user_browser rather than the admin
   browser. This demonstrates that any user can use the reschedule
   button.

== lib/lp/bugs/templates/bugwatch-portlet-activity.pt ==

 - I've updated the template to use the failed_activity and activity
   properties of the BugWatch rather than using view properties.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Graham this branch looks good. You have one typo of 'wathc'.

Also I know you just copied it from the existing code, but I'd expect to see

return failure_ratio < WATCH_RESCHEDULE_THRESHOLD

So that 6/10 failures would still return true.

review: Approve (code)
Revision history for this message
Brad Crittenden (bac) wrote :

Graham I forgot to mention there were some valid lint issues with this branch, like unneeded imports.

Revision history for this message
Graham Binns (gmb) wrote :

On 20 April 2010 18:25, Brad Crittenden <email address hidden> wrote:
> Graham I forgot to mention there were some valid lint issues with this branch, like unneeded imports.

Thanks Brad, I'll take care of those now.

--
Graham Binns | PGP Key: EC66FA7D

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/bugwatch.py'
2--- lib/lp/bugs/browser/bugwatch.py 2010-04-15 15:14:21 +0000
3+++ lib/lp/bugs/browser/bugwatch.py 2010-04-20 20:31:31 +0000
4@@ -10,13 +10,10 @@
5 'BugWatchEditView',
6 'BugWatchView']
7
8-from datetime import datetime
9-from pytz import utc
10
11 from zope.component import getUtility
12 from zope.interface import Interface
13
14-from canonical.cachedproperty import cachedproperty
15 from canonical.database.constants import UTC_NOW
16 from canonical.widgets.textwidgets import URIWidget
17
18@@ -35,9 +32,6 @@
19 from canonical.launchpad.webapp.menu import structured
20
21
22-WATCH_RESCHEDULE_THRESHOLD = 0.6
23-
24-
25 class BugWatchSetNavigation(GetitemNavigation):
26
27 usedfor = IBugWatchSet
28@@ -158,48 +152,15 @@
29
30 schema = BugWatchEditForm
31
32- @cachedproperty
33- def total_watch_activity_count(self):
34- return self.context.activity.count()
35-
36- @cachedproperty
37- def failed_watch_activity_count(self):
38- failed_activity_count = len([
39- activity for activity in self.context.activity if
40- activity.result not in BUG_WATCH_ACTIVITY_SUCCESS_STATUSES])
41- return failed_activity_count
42-
43 def userCanReschedule(self, action=None):
44 """Return True if the current user can reschedule the bug watch."""
45- if (self.context.next_check is not None and
46- self.context.next_check <= datetime.now(utc)):
47- # If the watch is already scheduled for a time in the past
48- # (or for right now) it can't be rescheduled, since it
49- # should be be checked by the next checkwatches run anyway.
50- return False
51-
52- if self.total_watch_activity_count == 0:
53- # Don't show the reschedule button if the watch has never
54- # been checked.
55- return False
56-
57- if self.failed_watch_activity_count == 0:
58- # Don't show the reschedule button if the watch has never
59- # failed.
60- return False
61-
62- # If the ratio is lower than the reschedule threshold, we
63- # can show the button.
64- failure_ratio = (
65- float(self.failed_watch_activity_count) /
66- self.total_watch_activity_count)
67- return failure_ratio <= WATCH_RESCHEDULE_THRESHOLD
68+ return self.context.can_be_rescheduled
69
70 @action('Update Now', name='reschedule', condition=userCanReschedule)
71 def reschedule_action(self, action, data):
72 """Schedule the current bug watch for immediate checking."""
73 bugwatch = self.context
74- bugwatch.next_check = UTC_NOW
75+ bugwatch.setNextCheck(UTC_NOW)
76 self.request.response.addInfoNotification(
77 structured(
78 'The <a href="%(url)s">%(bugtracker)s #%(remote_bug)s</a> '
79
80=== modified file 'lib/lp/bugs/browser/tests/bugwatch-views.txt'
81--- lib/lp/bugs/browser/tests/bugwatch-views.txt 2010-04-15 15:14:21 +0000
82+++ lib/lp/bugs/browser/tests/bugwatch-views.txt 2010-04-20 20:31:31 +0000
83@@ -110,58 +110,3 @@
84 'result_text': 'Synchronisation succeeded'}
85
86
87-Rescheduling a watch
88---------------------
89-
90-The BugWatch activity portlet provides a method, userCanReschedule(),
91-which indicates whether or not a user can reschedule a bug watch for
92-checking. For a new bug watch this will be False.
93-
94- >>> from pytz import utc
95- >>> from datetime import datetime
96- >>> schedulable_watch = factory.makeBugWatch()
97- >>> schedulable_watch.next_check = None
98- >>> scheduling_view = create_initialized_view(
99- ... schedulable_watch, '+portlet-activity')
100-
101- >>> scheduling_view.userCanReschedule()
102- False
103-
104-If there's been activity on the watch but it's always been successful,
105-userCanReschedule() will return False.
106-
107- >>> schedulable_watch.addActivity()
108- >>> scheduling_view.userCanReschedule()
109- False
110-
111-If the watch's updates have failed less than 60% of the time,
112-userCanReschedule() will return True
113-
114- >>> scheduling_view = create_initialized_view(
115- ... schedulable_watch, '+portlet-activity')
116- >>> schedulable_watch.addActivity(
117- ... result=BugWatchActivityStatus.BUG_NOT_FOUND)
118- >>> scheduling_view.userCanReschedule()
119- True
120-
121-If the watch is rescheduled, the button will disappear, since the
122-next_check time for the watch will be in the past (or in this case is
123-now) and therefore it will be checked with the next checkwatches run.
124-
125- >>> schedulable_watch.next_check = datetime.now(utc)
126- >>> scheduling_view = create_initialized_view(
127- ... schedulable_watch, '+portlet-activity')
128- >>> scheduling_view.userCanReschedule()
129- False
130-
131-However, if the watch has failed more than 60% of the time the button
132-will disappear again, since it's assumed that the watch needs attention
133-in order for it to be able to work again.
134-
135- >>> schedulable_watch.next_check = None
136- >>> schedulable_watch.addActivity(
137- ... result=BugWatchActivityStatus.BUG_NOT_FOUND)
138- >>> scheduling_view = create_initialized_view(
139- ... schedulable_watch, '+portlet-activity')
140- >>> scheduling_view.userCanReschedule()
141- False
142
143=== modified file 'lib/lp/bugs/configure.zcml'
144--- lib/lp/bugs/configure.zcml 2010-04-14 12:55:44 +0000
145+++ lib/lp/bugs/configure.zcml 2010-04-20 20:31:31 +0000
146@@ -849,7 +849,9 @@
147 bug
148 bugtasks
149 bugtracker
150+ can_be_rescheduled
151 datecreated
152+ failed_activity
153 getLastErrorMessage
154 hasComment
155 unpushed_comments
156@@ -870,7 +872,8 @@
157 permission="launchpad.AnyPerson"
158 attributes="
159 destroySelf
160- addActivity"
161+ addActivity
162+ setNextCheck"
163 set_attributes="bugtracker remotebug"/>
164 <require
165 permission="launchpad.Admin"
166
167=== modified file 'lib/lp/bugs/doc/bugwatch.txt'
168--- lib/lp/bugs/doc/bugwatch.txt 2010-03-26 10:39:53 +0000
169+++ lib/lp/bugs/doc/bugwatch.txt 2010-04-20 20:31:31 +0000
170@@ -513,3 +513,87 @@
171 >>> bug.removeWatch(bug_watch, factory.makePerson())
172 >>> [bug_watch.remotebug for bug_watch in bug.watches]
173 []
174+
175+
176+Checking if a watch can be rescheduled
177+--------------------------------------
178+
179+IBugWatch provides an attribute, can_be_rescheduled, which indicates
180+whether or not the watch can be rescheduled. For a new bug watch this
181+will be False.
182+
183+ >>> schedulable_watch = factory.makeBugWatch()
184+ >>> schedulable_watch.next_check = None
185+ >>> schedulable_watch.can_be_rescheduled
186+ False
187+
188+If there's been activity on the watch but it's always been successful,
189+can_be_rescheduled will be False.
190+
191+ >>> schedulable_watch.addActivity()
192+ >>> schedulable_watch.can_be_rescheduled
193+ False
194+
195+If the watch's updates have failed less than 60% of the time,
196+can_be_rescheduled will be True
197+
198+ >>> from lp.bugs.interfaces.bugwatch import BugWatchActivityStatus
199+ >>> schedulable_watch.addActivity(
200+ ... result=BugWatchActivityStatus.BUG_NOT_FOUND)
201+ >>> schedulable_watch.can_be_rescheduled
202+ True
203+
204+If the watch is rescheduled, can_be_rescheduled will be False, since the
205+next_check time for the watch will be in the past (or in this case is
206+now) and therefore it will be checked with the next checkwatches run.
207+
208+ >>> from pytz import utc
209+ >>> from datetime import datetime
210+ >>> schedulable_watch.next_check = datetime.now(utc)
211+ >>> schedulable_watch.can_be_rescheduled
212+ False
213+
214+However, if the watch has failed more than 60% of the time
215+can_be_rescheduled will be False, since it's assumed that the watch
216+needs attention in order for it to be able to work again.
217+
218+ >>> schedulable_watch.next_check = None
219+ >>> schedulable_watch.addActivity(
220+ ... result=BugWatchActivityStatus.BUG_NOT_FOUND)
221+ >>> schedulable_watch.can_be_rescheduled
222+ False
223+
224+
225+Rescheduling a watch
226+--------------------
227+
228+The rescheduling of a watch is done via IBugWatch.setNextCheck(). This
229+is to ensure that watches are only rescheduled when can_be_rescheduled
230+is True (note that the BugWatch Scheduler bypasses setNextCheck() and
231+sets next_check directly because it has admin privileges).
232+
233+The schedulable_watch that we used in the previous test cannot currently
234+be rescheduled.
235+
236+ >>> schedulable_watch.can_be_rescheduled
237+ False
238+
239+Calling setNextCheck() on this watch will cause an Exception,
240+BugWatchCannotBeRescheduled, to be raised.
241+
242+ >>> schedulable_watch.setNextCheck(datetime.now(utc))
243+ Traceback (most recent call last):
244+ ...
245+ BugWatchCannotBeRescheduled...
246+
247+If we add some activity to the watch, to make its can_be_rescheduled
248+property become True, setNextCheck() will succeed.
249+
250+ >>> schedulable_watch.addActivity()
251+ >>> schedulable_watch.can_be_rescheduled
252+ True
253+
254+ >>> next_check = datetime.now(utc)
255+ >>> schedulable_watch.setNextCheck(next_check)
256+ >>> schedulable_watch.next_check == next_check
257+ True
258
259=== modified file 'lib/lp/bugs/interfaces/bugwatch.py'
260--- lib/lp/bugs/interfaces/bugwatch.py 2010-04-16 14:50:46 +0000
261+++ lib/lp/bugs/interfaces/bugwatch.py 2010-04-20 20:31:31 +0000
262@@ -10,6 +10,7 @@
263 __all__ = [
264 'BUG_WATCH_ACTIVITY_SUCCESS_STATUSES',
265 'BugWatchActivityStatus',
266+ 'BugWatchCannotBeRescheduled',
267 'IBugWatch',
268 'IBugWatchActivity',
269 'IBugWatchSet',
270@@ -210,6 +211,10 @@
271 Text(title=_('The URL at which to view the remote bug.'),
272 readonly=True))
273
274+ can_be_rescheduled = Attribute(
275+ "A True or False indicator of whether or not this watch can be "
276+ "rescheduled.")
277+
278 def updateImportance(remote_importance, malone_importance):
279 """Update the importance of the bug watch and any linked bug task.
280
281@@ -250,6 +255,13 @@
282 def addActivity(result=None, message=None, oops_id=None):
283 """Add an `IBugWatchActivity` record for this BugWatch."""
284
285+ def setNextCheck(next_check):
286+ """Set the next_check time of the watch.
287+
288+ :raises: `BugWatchCannotBeRescheduled` if
289+ `IBugWatch.can_be_rescheduled` is False.
290+ """
291+
292
293 # Defined here because of circular imports.
294 IBugTracker['watches'].value_type.schema = IBugWatch
295@@ -363,3 +375,6 @@
296 title=_('OOPS ID'), readonly=True,
297 description=_("The OOPS ID associated with this activity."))
298
299+
300+class BugWatchCannotBeRescheduled(Exception):
301+ """The current `IBugWatch` can't be rescheduled."""
302
303=== modified file 'lib/lp/bugs/model/bugwatch.py'
304--- lib/lp/bugs/model/bugwatch.py 2010-04-16 14:50:46 +0000
305+++ lib/lp/bugs/model/bugwatch.py 2010-04-20 20:31:31 +0000
306@@ -12,6 +12,9 @@
307
308 import re
309 import urllib
310+
311+from datetime import datetime
312+from pytz import utc
313 from urlparse import urlunsplit
314
315 from zope.event import notify
316@@ -44,8 +47,9 @@
317
318 from lp.bugs.interfaces.bugtracker import BugTrackerType, IBugTrackerSet
319 from lp.bugs.interfaces.bugwatch import (
320- BugWatchActivityStatus, IBugWatch, IBugWatchActivity, IBugWatchSet,
321- NoBugTrackerFound, UnrecognizedBugTrackerURL)
322+ BUG_WATCH_ACTIVITY_SUCCESS_STATUSES, BugWatchActivityStatus,
323+ BugWatchCannotBeRescheduled, IBugWatch, IBugWatchActivity,
324+ IBugWatchSet, NoBugTrackerFound, UnrecognizedBugTrackerURL)
325 from lp.bugs.model.bugmessage import BugMessage
326 from lp.bugs.model.bugset import BugSetBase
327 from lp.bugs.model.bugtask import BugTask
328@@ -66,6 +70,9 @@
329 }
330
331
332+WATCH_RESCHEDULE_THRESHOLD = 0.6
333+
334+
335 class BugWatch(SQLBase):
336 """See `IBugWatch`."""
337 implements(IBugWatch)
338@@ -307,6 +314,52 @@
339 BugWatchActivity.bug_watch == self).order_by(
340 Desc('activity_date'))
341
342+ @property
343+ def can_be_rescheduled(self):
344+ """See `IBugWatch`."""
345+ if (self.next_check is not None and
346+ self.next_check <= datetime.now(utc)):
347+ # If the watch is already scheduled for a time in the past
348+ # (or for right now) it can't be rescheduled, since it
349+ # should be be checked by the next checkwatches run anyway.
350+ return False
351+
352+ if self.activity.is_empty():
353+ # Don't show the reschedule button if the watch has never
354+ # been checked.
355+ return False
356+
357+ if self.failed_activity.is_empty():
358+ # Don't show the reschedule button if the watch has never
359+ # failed.
360+ return False
361+
362+ # If the ratio is lower than the reschedule threshold, we
363+ # can show the button.
364+ failure_ratio = (
365+ float(self.failed_activity.count()) /
366+ self.activity.count())
367+ return failure_ratio <= WATCH_RESCHEDULE_THRESHOLD
368+
369+ @property
370+ def failed_activity(self):
371+ store = Store.of(self)
372+ success_status_ids = [
373+ status.value for status in BUG_WATCH_ACTIVITY_SUCCESS_STATUSES]
374+
375+ return store.find(
376+ BugWatchActivity,
377+ BugWatchActivity.bug_watch == self,
378+ Not(In(BugWatchActivity.result, success_status_ids))).order_by(
379+ Desc('activity_date'))
380+
381+ def setNextCheck(self, next_check):
382+ """See `IBugWatch`."""
383+ if not self.can_be_rescheduled:
384+ raise BugWatchCannotBeRescheduled()
385+
386+ self.next_check = next_check
387+
388
389 class BugWatchSet(BugSetBase):
390 """A set for BugWatch"""
391
392=== modified file 'lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt'
393--- lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt 2010-04-15 15:14:21 +0000
394+++ lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt 2010-04-20 20:31:31 +0000
395@@ -140,8 +140,8 @@
396 ... (bug_watch.bug.id, bug_watch.id))
397 >>> logout()
398
399- >>> admin_browser.open(watch_url)
400- >>> admin_browser.getControl('Update Now')
401+ >>> user_browser.open(watch_url)
402+ >>> user_browser.getControl('Update Now')
403 Traceback (most recent call last):
404 ...
405 LookupError: label 'Update Now'
406@@ -153,8 +153,8 @@
407 >>> bug_watch.addActivity()
408 >>> logout()
409
410- >>> admin_browser.open(watch_url)
411- >>> admin_browser.getControl('Update Now')
412+ >>> user_browser.open(watch_url)
413+ >>> user_browser.getControl('Update Now')
414 Traceback (most recent call last):
415 ...
416 LookupError: label 'Update Now'
417@@ -166,11 +166,11 @@
418 >>> bug_watch.addActivity(result=BugWatchActivityStatus.BUG_NOT_FOUND)
419 >>> logout()
420
421- >>> admin_browser.open(watch_url)
422- >>> reschedule_button = admin_browser.getControl('Update Now')
423+ >>> user_browser.open(watch_url)
424+ >>> reschedule_button = user_browser.getControl('Update Now')
425
426 >>> data_tag = find_tag_by_id(
427- ... admin_browser.contents, 'bugwatch-next_check')
428+ ... user_browser.contents, 'bugwatch-next_check')
429 >>> print extract_text(data_tag.renderContents())
430 Next check: Not yet scheduled
431
432@@ -180,22 +180,22 @@
433 >>> reschedule_button.click()
434
435 >>> for message in find_tags_by_class(
436- ... admin_browser.contents, 'informational message'):
437+ ... user_browser.contents, 'informational message'):
438 ... print extract_text(message)
439 The ... bug watch has been scheduled for immediate checking.
440
441 Looking at the watch +edit page again, we can see that the watch has
442 been scheduled.
443
444- >>> admin_browser.open(watch_url)
445+ >>> user_browser.open(watch_url)
446 >>> data_tag = find_tag_by_id(
447- ... admin_browser.contents, 'bugwatch-next_check')
448+ ... user_browser.contents, 'bugwatch-next_check')
449 >>> print extract_text(data_tag.renderContents())
450 Next check: 2...
451
452 The button will no longer be shown on the page.
453
454- >>> reschedule_button = admin_browser.getControl('Update Now')
455+ >>> reschedule_button = user_browser.getControl('Update Now')
456 Traceback (most recent call last):
457 ...
458 LookupError: label 'Update Now'
459
460=== modified file 'lib/lp/bugs/templates/bugwatch-portlet-activity.pt'
461--- lib/lp/bugs/templates/bugwatch-portlet-activity.pt 2010-04-15 15:14:21 +0000
462+++ lib/lp/bugs/templates/bugwatch-portlet-activity.pt 2010-04-20 20:31:31 +0000
463@@ -17,10 +17,10 @@
464 <div>
465 This watch has failed to update at
466 <tal:fail-count
467- replace="view/failed_watch_activity_count" />
468+ replace="context/failed_activity/count" />
469 out of the last
470 <tal:fail-count
471- replace="view/total_watch_activity_count" />
472+ replace="context/activity/count" />
473 attempts.
474 </div>
475 <div>

Subscribers

People subscribed via source and target branches

to status/vote changes: