Merge lp:~gmb/launchpad/make-+subscribe-play-nice-bug-735397 into lp:launchpad

Proposed by Graham Binns
Status: Merged
Merged at revision: 12619
Proposed branch: lp:~gmb/launchpad/make-+subscribe-play-nice-bug-735397
Merge into: lp:launchpad
Diff against target: 708 lines (+383/-58)
9 files modified
lib/lp/bugs/browser/bugsubscription.py (+33/-10)
lib/lp/bugs/browser/tests/test_bugsubscription_views.py (+99/-0)
lib/lp/bugs/configure.zcml (+2/-0)
lib/lp/bugs/interfaces/bug.py (+17/-0)
lib/lp/bugs/javascript/bug_subscription.js (+59/-0)
lib/lp/bugs/javascript/bugtask_index_portlets.js (+103/-48)
lib/lp/bugs/model/bug.py (+21/-0)
lib/lp/bugs/templates/bug-subscription.pt (+19/-0)
lib/lp/bugs/tests/test_bug.py (+30/-0)
To merge this branch: bzr merge lp:~gmb/launchpad/make-+subscribe-play-nice-bug-735397
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+53839@code.launchpad.net

Commit message

[r=jcsackett][bug=735397] The BugTask:+subscribe page has been updated so that it plays nicely with muted subscriptions. The controls on the BugTask:+index page should now behave correctly, too.

Description of the change

This branch makes the +subscribe view for a bug, and its associated
overlay on the bug page, play nice with muted subscriptions.

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

 - I've updated BugSubscriptionSubscribeSelfView so that it handles
   muted subscriptions properly and presents options related to unmuting
   to the user.

== lib/lp/bugs/browser/tests/test_bugsubscription_views.py ==

 - I've added tests to cover the changes I've made to
   BugSubscriptionSubscribeSelfView.

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

 - I've updated the IBug declaration to include the mute() and unmute()
   methods.

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

 - I've added two methods, mute() and unmute() to IBug. These are needed
   for two reasons:
   1. I want to not have to fool around with subscriptions in the JS
      unless I need to (see below).
   2. We may want to replace the mute/unmute via subscriptions
      functionality with something else (like a BugMute table). Having
      these two methods means we can do it reasonably simply without
      mucking about with View and JS too much.

== lib/lp/bugs/javascript/bug_subscription.js ==

 - I've added this file with some JavaScript to add UI polish to the
   +subscribe view. This should help make the view a little less
   confusing when people hit it whilst they have a mute on a bug.

== lib/lp/bugs/javascript/bugtask_index_portlets.js ==

 - I've made quite a lot of changes here, including several
   refactorings. The main points are:
   - Clicking Mute will now remove you from the subscribers list if
     you're there.
   - If you're muted and you click Subscribe, you'll see the updated
     version of the +subscribe form, with all the JS goodness I've added
     in bug_subscription.js.
   - Using the Subscribe link to unmute and resubscribe really just
     updates your subscription, but it also makes all the UI elements on
     the BugTask:+index page play nice.

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

 - I've added implementations of IBug.mute() and unmute().

== lib/lp/bugs/templates/bug-subscription.pt ==

 - I've updated the page to include the JS calls for
   bug_subscription.js.

== lib/lp/bugs/tests/test_bug.py ==

 - I've added tests for Bug.mute() and unmute().

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

Graham--

Code wise, this all looks good to me.

I do have a couple of questions, but they're not critiques of the code, just me being confused and seeing this as a good opportunity to figure something out.

* What's the exact difference between a muted subscription and not being subscribed? Is it a difference between getting all comments and just say, getting notified when something is Fix released?

* The answer to the first may answer this, but: why is unmuting the same as unsubscribing--wouldn't unmuting leave a subscription but signal I want to get notified of more events?

* In bugtask_index_portlets, you pull out a bunch of code from some callbacks and create a function, `update_mute_after_subscription_change`, that runs the same code. This is pretty clearly to keep code DRY, but you then manually call it rather than pass it in as the callback. I'm the first to say I don't know a ton about how callbacks work vs manually calling a function, so I was wondering what the reason is for not passing it an as a callback?

Again, this is all about helping me figure things out, not a problem with your branch.

Thanks for any answers, and nice work.

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

> * What's the exact difference between a muted subscription and not being subscribed? Is it a difference between getting all comments and just say, getting notified when something is Fix released?

If you have a muted subscription you will get no mail about the bug
*at all*. None. Nada. Zilch. Zip. Nothing.

> * The answer to the first may answer this, but: why is unmuting the same as unsubscribing--wouldn't unmuting leave a subscription but signal I want to get notified of more events?

This is going to change in a subsequent branch. Instead of just
removing the (muted) subscription we'll display the new improved
+subscribe form when you click unmute, so your options will be:

 (*) Unmute bug mail [which removes the mute]
 ( ) Unmute bug mail and subscribe me to this bug

> * In bugtask_index_portlets, you pull out a bunch of code from some callbacks and create a function, `update_mute_after_subscription_change`, that runs the same code. This is pretty clearly to keep code DRY, but you then manually call it rather than pass it in as the callback. I'm the first to say I don't know a ton about how callbacks work vs manually calling a function, so I was wondering what the reason is for not passing it an as a callback?

In this case there's no difference between the two at all. Passing the
callback is the equivalent of having a local-scope function or a
Lambda in Python. The only time callbacks matter is when you're doing
things asynchronously, and in this case we weren't; the callback just
meant you had to go and look for the definition of the callback
function, so it seemed simpler to make it part of a named function and
call that directly.

Hope that helps.

Revision history for this message
j.c.sackett (jcsackett) wrote :

Thanks for those answers, Graham.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
2--- lib/lp/bugs/browser/bugsubscription.py 2011-03-10 12:42:35 +0000
3+++ lib/lp/bugs/browser/bugsubscription.py 2011-03-17 15:02:39 +0000
4@@ -222,9 +222,20 @@
5
6 @cachedproperty
7 def _update_subscription_term(self):
8+ if self.user_is_muted:
9+ label = "Unmute bug mail from this bug and subscribe me to it"
10+ else:
11+ label = "Update my current subscription"
12 return SimpleTerm(
13- 'update-subscription', 'update-subscription',
14- 'Update my current subscription')
15+ 'update-subscription', 'update-subscription', label)
16+
17+ @cachedproperty
18+ def _unsubscribe_current_user_term(self):
19+ if self._use_advanced_features and self.user_is_muted:
20+ label = "Unmute bug mail from this bug"
21+ else:
22+ label = 'Unsubscribe me from this bug'
23+ return SimpleTerm(self.user, self.user.name, label)
24
25 @cachedproperty
26 def _subscription_field(self):
27@@ -233,12 +244,12 @@
28 for person in self._subscribers_for_current_user:
29 if person.id == self.user.id:
30 if (self._use_advanced_features and
31- self.user_is_subscribed_directly):
32- subscription_terms.append(self._update_subscription_term)
33+ (self.user_is_subscribed_directly or
34+ self.user_is_muted)):
35+ subscription_terms.append(
36+ self._update_subscription_term)
37 subscription_terms.insert(
38- 0, SimpleTerm(
39- person, person.name,
40- 'Unsubscribe me from this bug'))
41+ 0, self._unsubscribe_current_user_term)
42 self_subscribed = True
43 else:
44 subscription_terms.append(
45@@ -279,6 +290,8 @@
46 """See `LaunchpadFormView`."""
47 super(BugSubscriptionSubscribeSelfView, self).setUpWidgets()
48 if self._use_advanced_features:
49+ self.widgets['bug_notification_level'].widget_class = (
50+ 'bug-notification-level-field')
51 if self._subscriber_count_for_current_user == 0:
52 # We hide the subscription widget if the user isn't
53 # subscribed, since we know who the subscriber is and we
54@@ -298,14 +311,22 @@
55 self.widgets['bug_notification_level'].visible = False
56
57 @cachedproperty
58+ def user_is_muted(self):
59+ return self.context.bug.isMuted(self.user)
60+
61+ @cachedproperty
62 def user_is_subscribed_directly(self):
63 """Is the user subscribed directly to this bug?"""
64- return self.context.bug.isSubscribed(self.user)
65+ return (
66+ self.context.bug.isSubscribed(self.user) and not
67+ self.user_is_muted)
68
69 @cachedproperty
70 def user_is_subscribed_to_dupes(self):
71 """Is the user subscribed to dupes of this bug?"""
72- return self.context.bug.isSubscribedToDupes(self.user)
73+ return (
74+ self.context.bug.isSubscribedToDupes(self.user) and not
75+ self.user_is_muted)
76
77 @property
78 def user_is_subscribed(self):
79@@ -347,8 +368,10 @@
80 bug_notification_level = None
81
82 if (subscription_person == self._update_subscription_term.value and
83- self.user_is_subscribed):
84+ (self.user_is_subscribed or self.user_is_muted)):
85 self._handleUpdateSubscription(level=bug_notification_level)
86+ elif self.user_is_muted and subscription_person == self.user:
87+ self._handleUnsubscribeCurrentUser()
88 elif (not self.user_is_subscribed and
89 (subscription_person == self.user)):
90 self._handleSubscribe(bug_notification_level)
91
92=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
93--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-03-10 12:42:35 +0000
94+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-03-17 15:02:39 +0000
95@@ -29,6 +29,9 @@
96
97 def setUp(self):
98 super(BugSubscriptionAdvancedFeaturesTestCase, self).setUp()
99+ self.bug = self.factory.makeBug()
100+ self.person = self.factory.makePerson()
101+ self.team = self.factory.makeTeam()
102 with feature_flags():
103 set_feature_flag(u'malone.advanced-subscriptions.enabled', u'on')
104
105@@ -276,6 +279,102 @@
106 BugNotificationLevel.COMMENTS,
107 default_notification_level_value)
108
109+ def test_muted_subs_have_unmute_option(self):
110+ # If a user has a muted subscription, the
111+ # BugSubscriptionSubscribeSelfView's subscription field will
112+ # show an "Unmute" option.
113+ with person_logged_in(self.person):
114+ self.bug.mute(self.person, self.person)
115+
116+ with feature_flags():
117+ with person_logged_in(self.person):
118+ subscribe_view = create_initialized_view(
119+ self.bug.default_bugtask, name='+subscribe')
120+ subscription_widget = (
121+ subscribe_view.widgets['subscription'])
122+ # The Unmute option is actually treated the same way as
123+ # the unsubscribe option.
124+ self.assertEqual(
125+ "Unmute bug mail from this bug",
126+ subscription_widget.vocabulary.getTerm(self.person).title)
127+
128+ def test_muted_subs_have_unmute_and_update_option(self):
129+ # If a user has a muted subscription, the
130+ # BugSubscriptionSubscribeSelfView's subscription field will
131+ # show an option to unmute the subscription and update it to a
132+ # new BugNotificationLevel.
133+ with person_logged_in(self.person):
134+ self.bug.mute(self.person, self.person)
135+
136+ with feature_flags():
137+ with person_logged_in(self.person):
138+ subscribe_view = create_initialized_view(
139+ self.bug.default_bugtask, name='+subscribe')
140+ subscription_widget = (
141+ subscribe_view.widgets['subscription'])
142+ update_term = subscription_widget.vocabulary.getTermByToken(
143+ 'update-subscription')
144+ self.assertEqual(
145+ "Unmute bug mail from this bug and subscribe me to it",
146+ update_term.title)
147+
148+ def test_unmute_unmutes(self):
149+ # Using the "Unmute bug mail" option when the user has a muted
150+ # subscription will remove the muted subscription.
151+ with person_logged_in(self.person):
152+ self.bug.mute(self.person, self.person)
153+
154+ with feature_flags():
155+ with person_logged_in(self.person):
156+ level = BugNotificationLevel.METADATA
157+ form_data = {
158+ 'field.subscription': self.person.name,
159+ # Although this isn't used we must pass it for the
160+ # sake of form validation.
161+ 'field.bug_notification_level': level.title,
162+ 'field.actions.continue': 'Continue',
163+ }
164+ subscribe_view = create_initialized_view(
165+ self.bug.default_bugtask, form=form_data,
166+ name='+subscribe')
167+ self.assertFalse(self.bug.isMuted(self.person))
168+ self.assertFalse(self.bug.isSubscribed(self.person))
169+
170+ def test_update_when_muted_updates(self):
171+ # Using the "Unmute and subscribe me" option when the user has a
172+ # muted subscription will update the existing subscription to a
173+ # new BugNotificationLevel.
174+ with person_logged_in(self.person):
175+ muted_subscription = self.bug.mute(self.person, self.person)
176+
177+ with feature_flags():
178+ with person_logged_in(self.person):
179+ level = BugNotificationLevel.COMMENTS
180+ form_data = {
181+ 'field.subscription': 'update-subscription',
182+ 'field.bug_notification_level': level.title,
183+ 'field.actions.continue': 'Continue',
184+ }
185+ subscribe_view = create_initialized_view(
186+ self.bug.default_bugtask, form=form_data,
187+ name='+subscribe')
188+ self.assertFalse(self.bug.isMuted(self.person))
189+ self.assertTrue(self.bug.isSubscribed(self.person))
190+ self.assertEqual(
191+ level, muted_subscription.bug_notification_level)
192+
193+ def test_bug_notification_level_field_has_widget_class(self):
194+ # The bug_notification_level widget has a widget_class property
195+ # that can be used to manipulate it with JavaScript.
196+ with person_logged_in(self.person):
197+ with feature_flags():
198+ subscribe_view = create_initialized_view(
199+ self.bug.default_bugtask, name='+subscribe')
200+ widget_class = (
201+ subscribe_view.widgets['bug_notification_level'].widget_class)
202+ self.assertEqual(
203+ 'bug-notification-level-field', widget_class)
204+
205
206 class BugPortletSubcribersIdsTests(TestCaseWithFactory):
207
208
209=== modified file 'lib/lp/bugs/configure.zcml'
210--- lib/lp/bugs/configure.zcml 2011-03-15 04:15:45 +0000
211+++ lib/lp/bugs/configure.zcml 2011-03-17 15:02:39 +0000
212@@ -782,6 +782,7 @@
213 linkMessage
214 markAsDuplicate
215 markUserAffected
216+ mute
217 newMessage
218 removeWatch
219 setPrivate
220@@ -791,6 +792,7 @@
221 unlinkBranch
222 unlinkCVE
223 unlinkHWSubmission
224+ unmute
225 unsubscribe
226 unsubscribeFromDupes
227 updateHeat"
228
229=== modified file 'lib/lp/bugs/interfaces/bug.py'
230--- lib/lp/bugs/interfaces/bug.py 2011-03-15 04:15:45 +0000
231+++ lib/lp/bugs/interfaces/bug.py 2011-03-17 15:02:39 +0000
232@@ -32,6 +32,7 @@
233 export_write_operation,
234 exported,
235 mutator_for,
236+ operation_for_version,
237 operation_parameters,
238 operation_returns_collection_of,
239 operation_returns_entry,
240@@ -489,6 +490,22 @@
241 with a BugNotificationLevel of NOTHING.
242 """
243
244+ @operation_parameters(
245+ person=Reference(IPerson, title=_('Person'), required=False))
246+ @call_with(muted_by=REQUEST_USER)
247+ @export_write_operation()
248+ @operation_for_version('devel')
249+ def mute(person, muted_by):
250+ """Add a muted subscription for `person`."""
251+
252+ @operation_parameters(
253+ person=Reference(IPerson, title=_('Person'), required=False))
254+ @call_with(unmuted_by=REQUEST_USER)
255+ @export_write_operation()
256+ @operation_for_version('devel')
257+ def unmute(person, unmuted_by):
258+ """Remove a muted subscription for `person`."""
259+
260 def getDirectSubscriptions():
261 """A sequence of IBugSubscriptions directly linked to this bug."""
262
263
264=== added file 'lib/lp/bugs/javascript/bug_subscription.js'
265--- lib/lp/bugs/javascript/bug_subscription.js 1970-01-01 00:00:00 +0000
266+++ lib/lp/bugs/javascript/bug_subscription.js 2011-03-17 15:02:39 +0000
267@@ -0,0 +1,59 @@
268+/* Copyright 2011 Canonical Ltd. This software is licensed under the
269+ * GNU Affero General Public License version 3 (see the file LICENSE).
270+ *
271+ * A bugtracker form overlay that can create a bugtracker within any page.
272+ *
273+ * @namespace Y.lp.bugs.bugtracker_overlay
274+ * @requires dom, node, io-base, lazr.anim, lazr.formoverlay
275+ */
276+YUI.add('lp.bugs.bug_subscription', function(Y) {
277+var namespace = Y.namespace('lp.bugs.bug_subscription');
278+var level_div;
279+
280+// XXX: gmb 2011-03-17 bug=728457
281+// This fix for resizing needs to be incorporated into
282+// lazr.effects. When that is done it should be removed from here.
283+/*
284+ * Make sure that the bug_notification_level div is hidden properly .
285+ * @method clean_up_level_div
286+ */
287+namespace.clean_up_level_div = function() {
288+ if (Y.Lang.isValue(level_div)) {
289+ level_div.setStyles({
290+ height: 0,
291+ visibility: 'hidden'
292+ });
293+ }
294+};
295+
296+namespace.set_up_bug_notification_level_field = function() {
297+ level_div = Y.one('.bug-notification-level-field');
298+ var subscription_radio_buttons = Y.all(
299+ 'input[name=field.subscription]');
300+
301+ // Only collapse the bug_notification_level field if the buttons are
302+ // available to display it again.
303+ if (Y.Lang.isValue(level_div) && subscription_radio_buttons.size() > 1) {
304+ var slide_in_anim = Y.lazr.effects.slide_in(level_div);
305+ slide_in_anim.on('end', namespace.clean_up_level_div);
306+ var slide_out_anim = Y.lazr.effects.slide_out(level_div);
307+ slide_out_anim.on('end', function() {
308+ level_div.setStyles({
309+ visibility: 'visible'
310+ });
311+ });
312+ slide_in_anim.run();
313+ Y.each(subscription_radio_buttons, function(subscription_button) {
314+ subscription_button.on('click', function(e) {
315+ if(e.target.get('value') == 'update-subscription') {
316+ slide_out_anim.run();
317+ } else {
318+ slide_in_anim.run();
319+ }
320+ });
321+ });
322+ }
323+};
324+
325+}, "0.1", {"requires": ["dom", "node", "io-base", "lazr.anim", "lazr.effects",
326+ ]});
327
328=== modified file 'lib/lp/bugs/javascript/bugtask_index_portlets.js'
329--- lib/lp/bugs/javascript/bugtask_index_portlets.js 2011-03-14 14:39:33 +0000
330+++ lib/lp/bugs/javascript/bugtask_index_portlets.js 2011-03-17 15:02:39 +0000
331@@ -247,13 +247,10 @@
332 }
333
334 /*
335- * Set up the handlers for the mute / unmute link.
336+ * Set up and return a Subscription object for the mute link.
337+ * @method get_mute_subscription
338 */
339-function setup_mute_link_handlers() {
340- if (LP.links.me === undefined) {
341- return;
342- }
343-
344+function get_mute_subscription() {
345 setup_client_and_bug();
346 var mute_link = Y.one('.menu-link-mute_subscription');
347 var mute_subscription = new Y.lp.bugs.subscriber.Subscription({
348@@ -264,41 +261,56 @@
349 subscriber_ids: subscriber_ids
350 })
351 });
352+ var parent_node = mute_link.get('parentNode');
353+ mute_subscription.set(
354+ 'is_subscribed', !parent_node.hasClass('subscribed-false'));
355+ mute_subscription.set(
356+ 'is_muted', parent_node.hasClass('muted-true'));
357 mute_subscription.set(
358 'person', mute_subscription.get('subscriber'));
359+ return mute_subscription;
360+}
361+
362+/*
363+ * Set up the handlers for the mute / unmute link.
364+ */
365+function setup_mute_link_handlers() {
366+ if (LP.links.me === undefined) {
367+ return;
368+ }
369+
370+ var mute_subscription = get_mute_subscription();
371+ var mute_link = mute_subscription.get('link');
372+ var parent_node = mute_link.get('parentNode');
373 mute_link.addClass('js-action');
374 mute_link.on('click', function(e) {
375 e.halt();
376- var parent_node = mute_link.get('parentNode');
377- var is_subscribed = !parent_node.hasClass('subscribed-false');
378 var is_muted = parent_node.hasClass('muted-true');
379 mute_subscription.enable_spinner('Muting...');
380 if (! is_muted) {
381- mute_subscription.set(
382- 'bug_notification_level', "Nothing");
383- success_callback = function() {
384- parent_node.removeClass('muted-false');
385- parent_node.addClass('muted-true');
386- mute_subscription.set('is_muted', true);
387- update_subscription_after_mute_or_unmute(
388- mute_subscription);
389- }
390- mute_current_user(mute_subscription, success_callback);
391+ mute_current_user(mute_subscription);
392 } else {
393- success_callback = function() {
394- parent_node.removeClass('muted-true');
395- parent_node.addClass('muted-false');
396- mute_subscription.set('is_muted', false);
397- update_subscription_after_mute_or_unmute(
398- mute_subscription);
399- }
400- unmute_current_user(mute_subscription, success_callback);
401+ unmute_current_user(mute_subscription);
402 }
403- mute_subscription.disable_spinner();
404 });
405 }
406
407 /*
408+ * Update the Mute link after the user's subscriptions or mutes have
409+ * changed.
410+ */
411+function update_mute_after_subscription_change(mute_subscription) {
412+ var parent_node = mute_subscription.get('link').get('parentNode');
413+ if (mute_subscription.get('is_muted')) {
414+ parent_node.removeClass('muted-false');
415+ parent_node.addClass('muted-true');
416+ } else {
417+ parent_node.removeClass('muted-true');
418+ parent_node.addClass('muted-false');
419+ }
420+}
421+
422+/*
423 * Update the subscription links after the mute button has been clicked.
424 *
425 * @param mute_subscription {Object} A Y.lp.bugs.subscriber.Subscription
426@@ -593,6 +605,13 @@
427 centered: true,
428 visible: false
429 });
430+ // Register a couple of handlers to clean up the overlay when it's
431+ // hidden.
432+ subscription_overlay.get('form_cancel_button').on(
433+ 'click',
434+ Y.lp.bugs.bug_subscription.clean_up_level_div);
435+ subscription_overlay.on(
436+ 'cancel', Y.lp.bugs.bug_subscription.clean_up_level_div);
437 subscription_overlay.render('#privacy-form-container');
438 return subscription_overlay;
439 }
440@@ -615,6 +634,7 @@
441 subscription_overlay.set(
442 'form_submit_callback', function(form_data) {
443 handle_advanced_subscription_overlay(subscription, form_data);
444+ Y.lp.bugs.bug_subscription.clean_up_level_div();
445 subscription_overlay.hide();
446 subscription_overlay.loadFormContentAndRender(
447 subscription_link_url);
448@@ -629,6 +649,13 @@
449 subscription_overlay.renderUI();
450 subscription_overlay.bindUI();
451 subscription.disable_spinner();
452+
453+ // If the user has a mute on the bug we add some UI polish to
454+ // the subscription overlay.
455+ var mute_subscription = get_mute_subscription();
456+ if(mute_subscription.get('is_muted')) {
457+ Y.lp.bugs.bug_subscription.set_up_bug_notification_level_field();
458+ }
459 subscription_overlay.show();
460 }
461 function on_failure(id, response, subscription_overlay) {
462@@ -784,9 +811,8 @@
463 *
464 * @method mute_current_user
465 * @param subscription {Object} A Y.lp.bugs.subscribe.Subscription object.
466- * @param success_callback {Object} A function to be called on success.
467 */
468-function mute_current_user(subscription, success_callback) {
469+function mute_current_user(subscription) {
470 subscription.enable_spinner('Muting...');
471 var subscription_link = subscription.get('link');
472 var subscriber = subscription.get('subscriber');
473@@ -805,11 +831,25 @@
474 success: function(client) {
475 subscription.disable_spinner('Unmute bug mail');
476 var flash_node = subscription_link.get('parentNode');
477- var anim = Y.lazr.anim.green_flash({ node: flash_node });
478- anim.run();
479- if (Y.Lang.isValue(success_callback)) {
480- success_callback();
481+ var mute_anim = Y.lazr.anim.green_flash({ node: flash_node });
482+ mute_anim.run();
483+
484+ // Remove the subscriber's name from the subscriber
485+ // list, if it's there.
486+ var subscriber_node = Y.one('.' + subscriber.get('css_name'));
487+ if (Y.Lang.isValue(subscriber_node)) {
488+ var subscriber_anim = Y.lazr.anim.green_flash({
489+ node: subscriber_node
490+ });
491+ subscriber_anim.on('end', function(e) {
492+ remove_user_name_link(subscriber_node);
493+ set_none_for_empty_subscribers();
494+ });
495+ subscriber_anim.run();
496 }
497+ subscription.set('is_muted', true);
498+ update_mute_after_subscription_change(subscription);
499+ update_subscription_after_mute_or_unmute(subscription);
500 },
501
502 failure: error_handler.getFailureHandler()
503@@ -817,12 +857,10 @@
504
505 parameters: {
506 person: Y.lp.client.get_absolute_uri(
507- subscriber.get('escaped_uri')),
508- suppress_notify: false,
509- level: bug_notification_level
510+ subscriber.get('escaped_uri'))
511 }
512 };
513- lp_client.named_post(bug_repr.self_link, 'subscribe', config);
514+ lp_client.named_post(bug_repr.self_link, 'mute', config);
515 }
516
517 /*
518@@ -830,9 +868,8 @@
519 *
520 * @method unmute_current_user
521 * @param subscription {Object} A Y.lp.bugs.subscriber.Subscription object.
522- * @param success_callback {Object} A function to be called on success.
523 */
524-function unmute_current_user(subscription, success_callback) {
525+function unmute_current_user(subscription) {
526 subscription.enable_spinner('Unmuting...');
527 var subscription_link = subscription.get('link');
528 var subscriber = subscription.get('subscriber');
529@@ -852,15 +889,20 @@
530 var flash_node = subscription_link.get('parentNode');
531 var anim = Y.lazr.anim.green_flash({ node: flash_node });
532 anim.run();
533- if (Y.Lang.isValue(success_callback)) {
534- success_callback();
535- }
536+ subscription.set('is_muted', false);
537+ update_mute_after_subscription_change(subscription);
538+ update_subscription_after_mute_or_unmute(subscription);
539 },
540
541 failure: error_handler.getFailureHandler()
542+ },
543+
544+ parameters: {
545+ person: Y.lp.client.get_absolute_uri(
546+ subscriber.get('escaped_uri'))
547 }
548 };
549- lp_client.named_post(bug_repr.self_link, 'unsubscribe', config);
550+ lp_client.named_post(bug_repr.self_link, 'unmute', config);
551 }
552
553 /*
554@@ -1153,17 +1195,19 @@
555 function handle_advanced_subscription_overlay(subscription, form_data) {
556 var link = subscription.get('link');
557 var link_parent = link.get('parentNode');
558+ var mute_subscription = get_mute_subscription();
559 if (link_parent.hasClass('subscribed-false') &&
560- link_parent.hasClass('dup-subscribed-false')) {
561- // The user isn't subscribed, so subscribe them.
562+ link_parent.hasClass('dup-subscribed-false') &&
563+ !mute_subscription.get('is_muted')) {
564+ // The user isn't subscribed or muted, so subscribe them.
565 subscription.set(
566 'bug_notification_level',
567 form_data['field.bug_notification_level']);
568 subscribe_current_user(subscription);
569 } else if (
570 form_data['field.subscription'] == 'update-subscription') {
571- // The user is already subscribed and wants to update their
572- // subscription.
573+ // The user is already subscribed or is muted and wants to
574+ // update their subscription.
575 setup_client_and_bug();
576 var person_name = subscription.get('person').get('name');
577 var subscription_url =
578@@ -1173,6 +1217,9 @@
579 on: {
580 success: function(lp_subscription) {
581 subscription.enable_spinner('Updating subscription...');
582+ if (mute_subscription.get('is_muted')) {
583+ mute_subscription.enable_spinner('Unmuting...');
584+ }
585 lp_subscription.set(
586 'bug_notification_level',
587 form_data['field.bug_notification_level'][0])
588@@ -1185,6 +1232,14 @@
589 node: link_parent
590 });
591 anim.run();
592+ if (mute_subscription.get('is_muted')) {
593+ mute_subscription.set('is_muted', false);
594+ mute_subscription.disable_spinner(
595+ "Mute bug mail");
596+ update_mute_after_subscription_change(
597+ mute_subscription);
598+ add_user_name_link(subscription);
599+ }
600 },
601 failure: function(e) {
602 subscription.disable_spinner(
603@@ -1366,4 +1421,4 @@
604 "lazr.overlay", "lazr.choiceedit", "lp.app.picker",
605 "lp.client",
606 "lp.client.plugins", "lp.bugs.subscriber",
607- "lp.app.errors"]});
608+ "lp.bugs.bug_subscription", "lp.app.errors"]});
609
610=== modified file 'lib/lp/bugs/model/bug.py'
611--- lib/lp/bugs/model/bug.py 2011-03-15 04:15:45 +0000
612+++ lib/lp/bugs/model/bug.py 2011-03-17 15:02:39 +0000
613@@ -836,6 +836,27 @@
614 BugNotificationLevel.NOTHING)
615 return not subscriptions.is_empty()
616
617+ def mute(self, person, muted_by):
618+ """See `IBug`."""
619+ # If there's an existing subscription, update it.
620+ store = Store.of(self)
621+ subscriptions = store.find(
622+ BugSubscription,
623+ BugSubscription.bug == self,
624+ BugSubscription.person == person)
625+ if subscriptions.is_empty():
626+ return self.subscribe(
627+ person, muted_by, level=BugNotificationLevel.NOTHING)
628+ else:
629+ subscription = subscriptions.one()
630+ subscription.bug_notification_level = (
631+ BugNotificationLevel.NOTHING)
632+ return subscription
633+
634+ def unmute(self, person, unmuted_by):
635+ """See `IBug`."""
636+ self.unsubscribe(person, unmuted_by)
637+
638 @property
639 def subscriptions(self):
640 """The set of `BugSubscriptions` for this bug."""
641
642=== modified file 'lib/lp/bugs/templates/bug-subscription.pt'
643--- lib/lp/bugs/templates/bug-subscription.pt 2010-10-18 10:14:37 +0000
644+++ lib/lp/bugs/templates/bug-subscription.pt 2011-03-17 15:02:39 +0000
645@@ -10,6 +10,25 @@
646 i18n:domain="malone"
647 >
648
649+ <metal:block fill-slot="head_epilogue">
650+ <script type="text/javascript"
651+ tal:condition="devmode"
652+ tal:content="string:var yui_base='${yui}';"></script>
653+ <script type="text/javascript"
654+ tal:condition="devmode"
655+ tal:define="lp_js string:${icingroot}/build"
656+ tal:attributes="src string:${lp_js}/bugs/filebug-dupefinder.js"></script>
657+
658+ <script type="text/javascript" tal:condition="view/user_is_muted">
659+ LPS.use('base', 'node', 'oop', 'event', 'lp.bugs.bug_subscription',
660+ function(Y) {
661+ Y.on(
662+ 'domready',
663+ Y.lp.bugs.bug_subscription.set_up_bug_notification_level_field);
664+ });
665+ </script>
666+ </metal:block>
667+
668 <body>
669 <div metal:fill-slot="main">
670
671
672=== modified file 'lib/lp/bugs/tests/test_bug.py'
673--- lib/lp/bugs/tests/test_bug.py 2011-03-04 16:17:08 +0000
674+++ lib/lp/bugs/tests/test_bug.py 2011-03-17 15:02:39 +0000
675@@ -44,3 +44,33 @@
676 # subscription.
677 with person_logged_in(self.person):
678 self.assertEqual(False, self.bug.isMuted(self.person))
679+
680+ def test_mute_mutes_user(self):
681+ # Bug.mute() adds a muted subscription for the user passed to
682+ # it.
683+ with person_logged_in(self.person):
684+ muted_subscription = self.bug.mute(
685+ self.person, self.person)
686+ self.assertEqual(
687+ BugNotificationLevel.NOTHING,
688+ muted_subscription.bug_notification_level)
689+
690+ def test_mute_mutes_user_with_existing_subscription(self):
691+ # Bug.mute() will update an existing subscription so that it
692+ # becomes muted.
693+ with person_logged_in(self.person):
694+ subscription = self.bug.subscribe(self.person, self.person)
695+ muted_subscription = self.bug.mute(self.person, self.person)
696+ self.assertEqual(subscription, muted_subscription)
697+ self.assertEqual(
698+ BugNotificationLevel.NOTHING,
699+ subscription.bug_notification_level)
700+
701+ def test_unmute_unmutes_user(self):
702+ # Bug.unmute() will remove a muted subscription for the user
703+ # passed to it.
704+ with person_logged_in(self.person):
705+ self.bug.mute(self.person, self.person)
706+ self.assertTrue(self.bug.isMuted(self.person))
707+ self.bug.unmute(self.person, self.person)
708+ self.assertFalse(self.bug.isMuted(self.person))