launchpad-reviewers team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/launchpad/finish-it-bug-737648 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #737648 in Launchpad itself: "Clicking unmute when you're muted should display the subscribe overlay."
  https://bugs.launchpad.net/launchpad/+bug/737648

For more details, see:
https://code.launchpad.net/~gmb/launchpad/finish-it-bug-737648/+merge/54189

This branch does the final work to make the mute button polished and
shiny and all-working.

Changes made:

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

 - I've updated the mute_subscription link in the BugContextMenu so that
   links to the mute page when the user is not muted and the +subscribe
   page when they are. This is because it is the subscribe page that
   handles our unmute-and-resubscribe story.

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

 - I've updated the mute link onclick handler so that it loads and shows
   the advanced subscription overlay instead of simply unmuting the
   current user.
 - I've updated the update_mute_after_subscription_change function so
   that it handles updating the text of the link after someone has muted
   or unmuted.
 - I've updated the get_subscribe_self_subscription() function so that
   it sets various properties on the subscription object that it
   returns. This is so that we can use get_subscribe_self_subscription()
   in handle_advanced_subscription_overlay().
 - I've updated handle_advanced_subscription_overlay() so that it now
   fetches the subscribe_self and mute_ subscriptions itself rather than
   having them passed in. This means that it will always have the latest
   state rather than having to rely on other functions having set things
   properly.
-- 
https://code.launchpad.net/~gmb/launchpad/finish-it-bug-737648/+merge/54189
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/finish-it-bug-737648 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2011-03-18 19:07:40 +0000
+++ lib/lp/bugs/browser/bug.py	2011-03-21 11:47:16 +0000
@@ -278,11 +278,13 @@
         user = getUtility(ILaunchBag).user
         if self.context.bug.isMuted(user):
             text = "Unmute bug mail"
+            link = "+subscribe"
         else:
             text = "Mute bug mail"
+            link = "+mute"
 
         return Link(
-            '+mute', text, icon='remove', summary=(
+            link, text, icon='remove', summary=(
                 "Mute this bug so that you will never receive emails "
                 "about it."))
 

=== modified file 'lib/lp/bugs/javascript/bugtask_index_portlets.js'
--- lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-03-18 14:58:13 +0000
+++ lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-03-21 11:47:16 +0000
@@ -286,11 +286,14 @@
     mute_link.on('click', function(e) {
         e.halt();
         var is_muted = parent_node.hasClass('muted-true');
-        mute_subscription.enable_spinner('Muting...');
         if (! is_muted) {
+            mute_subscription.enable_spinner('Muting...');
             mute_current_user(mute_subscription);
         } else {
-            unmute_current_user(mute_subscription);
+            var unmute_overlay = setup_advanced_subscription_overlay(
+               mute_subscription);
+            load_and_show_advanced_subscription_overlay(
+               mute_subscription, unmute_overlay);
         }
     });
 }
@@ -300,13 +303,22 @@
  * changed.
  */
 function update_mute_after_subscription_change(mute_subscription) {
-    var parent_node = mute_subscription.get('link').get('parentNode');
+    var mute_link = mute_subscription.get('link');
+    var parent_node = mute_link.get('parentNode');
     if (mute_subscription.get('is_muted')) {
         parent_node.removeClass('muted-false');
         parent_node.addClass('muted-true');
+        mute_link.setAttribute(
+            'href', mute_link.getAttribute('href').replace(
+                '+mute', '+subscribe'));
+        mute_subscription.disable_spinner("Unmute bug mail");
     } else {
         parent_node.removeClass('muted-true');
         parent_node.addClass('muted-false');
+        mute_link.setAttribute(
+            'href', mute_link.getAttribute('href').replace(
+                '+subscribe', '+mute'));
+        mute_subscription.disable_spinner("Mute bug mail");
     }
 }
 
@@ -356,6 +368,9 @@
         'link').get('parentNode').hasClass('dup-subscribed-true');
     subscription.set('is_direct', is_direct);
     subscription.set('has_dupes', has_dupes);
+    subscription.set('can_be_unsubscribed', true);
+    subscription.set('person', subscription.get('subscriber'));
+    subscription.set('is_team', false);
     return subscription;
 }
 
@@ -633,11 +648,9 @@
         'link').get('href') + '/++form++';
     subscription_overlay.set(
         'form_submit_callback', function(form_data) {
-        handle_advanced_subscription_overlay(subscription, form_data);
+        handle_advanced_subscription_overlay(form_data);
         Y.lp.bugs.bug_subscription.clean_up_level_div();
         subscription_overlay.hide();
-        subscription_overlay.loadFormContentAndRender(
-            subscription_link_url);
     });
 
     // Normally we'd just call loadFormContentAndRender() here, but we
@@ -1189,10 +1202,10 @@
  * Handle the advanced_subscription_overlay's form submissions.
  *
  * @method handle_advanced_subscription_overlay
- * @param subscription {Object} A Y.lp.bugs.subscriber.Subscription object.
  * @param form_data {Object} The data from the submitted form.
  */
-function handle_advanced_subscription_overlay(subscription, form_data) {
+function handle_advanced_subscription_overlay(form_data) {
+    var subscription = get_subscribe_self_subscription();
     var link = subscription.get('link');
     var link_parent = link.get('parentNode');
     var mute_subscription = get_mute_subscription();