launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02956
[Merge] lp:~gmb/launchpad/mute-button-cleanup-bug-204980 into lp:launchpad
Graham Binns has proposed merging lp:~gmb/launchpad/mute-button-cleanup-bug-204980 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #204980 in Launchpad itself: "bug contacts should be able to unsubscribe from implicit subscriptions"
https://bugs.launchpad.net/launchpad/+bug/204980
For more details, see:
https://code.launchpad.net/~gmb/launchpad/mute-button-cleanup-bug-204980/+merge/53401
This branch does a bunch of clean-up work for the bug mail mute button
story and activates the mute link for malone-alpha members.
The main aim of this branch was to make the mute button behave sensibly
and to not have other parts of the UI doing things which could confuse
the user. To this end, I've made the following tweaks:
- Muted users no longer appear in the subscriber list
- The subscribe link doesn't change when a user is muted; it will now
always display "Subscribe me" for muted subscriptions.
I've made the following changes:
== lib/lp/bugs/browser/bug.py ==
- I've added a current_user_mute_class property. This is used when
rendering the subscribers portlet so that the JavaScript can pick up
the user's mute status based on the CSS class of the Mute link's
parent (this might not be the most elegant way to do things but it's
consistent with how the Subscribe link works and I can't think of a
better way ATM).
== lib/lp/bugs/browser/bugsubscription.py ==
- I've updated the BugPortletSubscribersContents so that the
sorted_direct_subscriptions property doesn't return muted
subscriptions.
== lib/lp/bugs/browser/tests/test_bugsubscription_views.py ==
- I've added a test case to cover the change to
BugPortletSubscribersContents.sorted_direct_subscriptions.
== lib/lp/bugs/javascript/bugtask_index_portlets.js ==
- I've refactored the existing JS and added code to make the Mute link
behave as it should. It also now updates the Subscribe link properly.
== lib/lp/bugs/templates/bug-portlet-subscribers.pt ==
- I've enabled the mute link for malone-alpha users via the
malone.advanced-subscriptions.enabled flag.
--
https://code.launchpad.net/~gmb/launchpad/mute-button-cleanup-bug-204980/+merge/53401
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/mute-button-cleanup-bug-204980 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2011-03-09 11:53:54 +0000
+++ lib/lp/bugs/browser/bug.py 2011-03-15 11:01:24 +0000
@@ -511,6 +511,15 @@
else:
return 'subscribed-false %s' % dup_class
+ @property
+ def current_user_mute_class(self):
+ bug = self.context
+ subscription_class = self.current_user_subscription_class
+ if bug.isMuted(self.user):
+ return 'muted-true %s' % subscription_class
+ else:
+ return 'muted-false %s' % subscription_class
+
@cachedproperty
def _bug_attachments(self):
"""Get a dict of attachment type -> attachments list."""
=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py 2011-03-07 21:21:21 +0000
+++ lib/lp/bugs/browser/bugsubscription.py 2011-03-15 11:01:24 +0000
@@ -497,6 +497,9 @@
for subscription in direct_subscriptions:
if not check_permission('launchpad.View', subscription.person):
continue
+ if (subscription.bug_notification_level ==
+ BugNotificationLevel.NOTHING):
+ continue
if subscription.person == self.user:
can_unsubscribe = [subscription] + can_unsubscribe
elif subscription.canBeUnsubscribedByUser(self.user):
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-03-03 10:49:12 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-03-15 11:01:24 +0000
@@ -317,3 +317,31 @@
self.bug.default_bugtask, BugSubscriptionListView)
self.assertEqual(
list(harness.view.structural_subscriptions), [sub])
+
+
+class BugPortletSubscribersContentsTestCase(TestCaseWithFactory):
+ """Tests for the BugPortletSubscribersContents view."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(BugPortletSubscribersContentsTestCase, self).setUp()
+ self.bug = self.factory.makeBug()
+ self.subscriber = self.factory.makePerson()
+
+ def test_sorted_direct_subscriptions_doesnt_show_mutes(self):
+ # BugPortletSubscribersContents.sorted_direct_subscriptions does
+ # not return muted subscriptions.
+ with person_logged_in(self.subscriber):
+ subscription = self.bug.subscribe(
+ self.subscriber, self.subscriber,
+ level=BugNotificationLevel.NOTHING)
+ view = create_initialized_view(
+ self.bug, name="+bug-portlet-subscribers-content")
+ # Loop over the results of sorted_direct_subscriptions to
+ # extract the subscriptions from their
+ # SubscriptionAttrDecorator intances.
+ sorted_subscriptions = [
+ decorator.subscription for decorator in
+ view.sorted_direct_subscriptions]
+ self.assertFalse(subscription in sorted_subscriptions)
=== modified file 'lib/lp/bugs/javascript/bugtask_index_portlets.js'
--- lib/lp/bugs/javascript/bugtask_index_portlets.js 2011-03-09 15:42:42 +0000
+++ lib/lp/bugs/javascript/bugtask_index_portlets.js 2011-03-15 11:01:24 +0000
@@ -256,8 +256,6 @@
setup_client_and_bug();
var mute_link = Y.one('.menu-link-mute_subscription');
- var parent_node = mute_link.get('parentNode');
- var is_subscribed = !parent_node.hasClass('subscribed-false');
var mute_subscription = new Y.lp.bugs.subscriber.Subscription({
link: mute_link,
spinner: Y.one('#mute-unmute-spinner'),
@@ -271,29 +269,65 @@
mute_link.addClass('js-action');
mute_link.on('click', function(e) {
e.halt();
+ var parent_node = mute_link.get('parentNode');
+ var is_subscribed = !parent_node.hasClass('subscribed-false');
+ var is_muted = parent_node.hasClass('muted-true');
mute_subscription.enable_spinner('Muting...');
- if (! is_subscribed) {
+ if (! is_muted) {
mute_subscription.set(
'bug_notification_level', "Nothing");
- mute_current_user(mute_subscription);
+ success_callback = function() {
+ parent_node.removeClass('muted-false');
+ parent_node.addClass('muted-true');
+ mute_subscription.set('is_muted', true);
+ update_subscription_after_mute_or_unmute(
+ mute_subscription);
+ }
+ mute_current_user(mute_subscription, success_callback);
} else {
- unmute_current_user(mute_subscription);
+ success_callback = function() {
+ parent_node.removeClass('muted-true');
+ parent_node.addClass('muted-false');
+ mute_subscription.set('is_muted', false);
+ update_subscription_after_mute_or_unmute(
+ mute_subscription);
+ }
+ unmute_current_user(mute_subscription, success_callback);
}
mute_subscription.disable_spinner();
});
}
-
/*
- * Initialize callbacks for subscribe/unsubscribe links.
+ * Update the subscription links after the mute button has been clicked.
*
- * @method setup_subscription_link_handlers
+ * @param mute_subscription {Object} A Y.lp.bugs.subscriber.Subscription
+ * object.
*/
-function setup_subscription_link_handlers() {
- if (LP.links.me === undefined) {
- return;
+function update_subscription_after_mute_or_unmute(mute_subscription) {
+ var subscription = get_subscribe_self_subscription();
+ var subscription_link = subscription.get('link');
+
+ subscription.enable_spinner('Updating...');
+ if (mute_subscription.get('is_muted')) {
+ subscription.disable_spinner(subscription_labels.SUBSCRIBE);
+ if (subscription.has_duplicate_subscriptions()) {
+ set_subscription_link_parent_class(
+ subscription_link, false, true);
+ } else {
+ set_subscription_link_parent_class(
+ subscription_link, false, false);
+ }
+ } else {
+ subscription.disable_spinner(subscription_labels.SUBSCRIBE);
}
+}
+/*
+ * Set up and return a Subscription object for the direct subscription
+ * link.
+ */
+function get_subscribe_self_subscription() {
setup_client_and_bug();
var subscription = new Y.lp.bugs.subscriber.Subscription({
link: Y.one('.menu-link-subscription'),
@@ -310,7 +344,20 @@
'link').get('parentNode').hasClass('dup-subscribed-true');
subscription.set('is_direct', is_direct);
subscription.set('has_dupes', has_dupes);
-
+ return subscription;
+}
+
+/*
+ * Initialize callbacks for subscribe/unsubscribe links.
+ *
+ * @method setup_subscription_link_handlers
+ */
+function setup_subscription_link_handlers() {
+ if (LP.links.me === undefined) {
+ return;
+ }
+
+ var subscription = get_subscribe_self_subscription();
var subscription_overlay;
if (namespace.use_advanced_subscriptions) {
subscription_overlay = setup_advanced_subscription_overlay(
@@ -346,9 +393,7 @@
setup_subscribe_someone_else_handler(subscription);
if (namespace.use_advanced_subscriptions) {
- // This is currently disabled since the mute link is a work in
- // progress.
- // setup_mute_link_handlers();
+ setup_mute_link_handlers();
}
}
@@ -739,8 +784,9 @@
*
* @method mute_current_user
* @param subscription {Object} A Y.lp.bugs.subscribe.Subscription object.
+ * @param success_callback {Object} A function to be called on success.
*/
-function mute_current_user(subscription) {
+function mute_current_user(subscription, success_callback) {
subscription.enable_spinner('Muting...');
var subscription_link = subscription.get('link');
var subscriber = subscription.get('subscriber');
@@ -761,6 +807,9 @@
var flash_node = subscription_link.get('parentNode');
var anim = Y.lazr.anim.green_flash({ node: flash_node });
anim.run();
+ if (Y.Lang.isValue(success_callback)) {
+ success_callback();
+ }
},
failure: error_handler.getFailureHandler()
@@ -781,8 +830,9 @@
*
* @method unmute_current_user
* @param subscription {Object} A Y.lp.bugs.subscriber.Subscription object.
+ * @param success_callback {Object} A function to be called on success.
*/
-function unmute_current_user(subscription) {
+function unmute_current_user(subscription, success_callback) {
subscription.enable_spinner('Unmuting...');
var subscription_link = subscription.get('link');
var subscriber = subscription.get('subscriber');
@@ -802,6 +852,9 @@
var flash_node = subscription_link.get('parentNode');
var anim = Y.lazr.anim.green_flash({ node: flash_node });
anim.run();
+ if (Y.Lang.isValue(success_callback)) {
+ success_callback();
+ }
},
failure: error_handler.getFailureHandler()
=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers.pt 2011-03-09 15:42:42 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers.pt 2011-03-15 11:01:24 +0000
@@ -13,17 +13,13 @@
tal:content="structure context_menu/subscription/render" />
<div id="sub-unsub-spinner">Subscribing...</div>
<div tal:content="structure context_menu/addsubscriber/render" />
- <tal:comment replace="nothing">
- <!-- This section enables the mute link; it's disabled since
- that's a work-in-progress. -->
- <tal:show-mute
- condition="request/features/malone.advanced-subscriptions.enabled">
- <div
- tal:attributes="class view/current_user_subscription_class"
- tal:content="structure context_menu/mute_subscription/render" />
- <div id="mute-unmute-spinner">Unmuting...</div>
- </tal:show-mute>
- </tal:comment>
+ <tal:show-mute
+ condition="request/features/malone.advanced-subscriptions.enabled">
+ <div
+ tal:attributes="class view/current_user_mute_class"
+ tal:content="structure context_menu/mute_subscription/render" />
+ <div id="mute-unmute-spinner">Unmuting...</div>
+ </tal:show-mute>
</div>
<a id="subscribers-ids-link"
tal:define="bug context/bug|context"
