launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-760121 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #760121 in Launchpad itself: "New feature-flagged JS (+) Subscribe link does not work"
  https://bugs.launchpad.net/launchpad/+bug/760121

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-760121/+merge/57726

= Summary =

The mute link is present on the bugs page conditionally.  If it does not
appear, the JS for the 'subscribe' action hits a JS error and fails.

== Proposed fix ==

Make the get_mute_subscriptions method return null and fix all call
sites to take it into account.

Also fixed some lint, misspellings in help file, bad HTML markup, and
poor presentation of choices in the subscription overlay.

== Pre-implementation notes ==

Chat with Gary

== Implementation details ==

As above.

== Tests ==

:(

== Demo and Q/A ==

Go to a bug page and repeated subscribe, mute, and unmute/subscribe.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/tests/picker.html
  lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt
  lib/lp/app/templates/inline-picker.pt
  lib/lp/bugs/javascript/tests/test_subscription.js
  lib/lp/bugs/templates/bug-portlet-watch.pt
  lib/lp/bugs/help/subscription-mute.html
  lib/lp/bugs/javascript/bugtask_index_portlets.js
  lib/lp/bugs/browser/bugsubscription.py

./lib/lp/app/templates/inline-picker.pt
       1: unbound prefix


I have no idea what 'unbound prefix' means and I don't see anything
wrong with that template.
-- 
https://code.launchpad.net/~bac/launchpad/bug-760121/+merge/57726
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-760121 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/tests/picker.html'
--- lib/lp/app/javascript/tests/picker.html	2011-04-11 00:18:45 +0000
+++ lib/lp/app/javascript/tests/picker.html	2011-04-14 16:45:39 +0000
@@ -37,9 +37,9 @@
               Edit
             </button>
           </span>
-           <span class="yui3-activator-data-box">
-           </span>
-          <div class="yui3-activator-message-box yui3-activator-hidden"/>
+          <span class="yui3-activator-data-box">
+          </span>
+          <span class="yui3-activator-message-box yui3-activator-hidden"></span>
         </span>
       </span>
   </div>

=== modified file 'lib/lp/app/templates/inline-picker.pt'
--- lib/lp/app/templates/inline-picker.pt	2011-03-10 01:25:13 +0000
+++ lib/lp/app/templates/inline-picker.pt	2011-04-14 16:45:39 +0000
@@ -1,6 +1,6 @@
 <span tal:attributes="id view/content_box_id">
     <span class="yui3-activator-data-box">
-    <tal:attribute replace="structure view/default_html"/>
+      <tal:attribute replace="structure view/default_html"/>
     </span>
     <button class="lazr-btn yui3-activator-act yui3-activator-hidden">
       Edit
@@ -10,7 +10,7 @@
                          title view/edit_title"
          class="sprite edit"></a>
     </noscript>
-    <div class="yui3-activator-message-box yui3-activator-hidden"/>
+    <div class="yui3-activator-message-box yui3-activator-hidden"></div>
 </span>
 <script tal:condition="view/can_write"
         tal:content="structure string:

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2011-04-07 21:31:29 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2011-04-14 16:45:39 +0000
@@ -166,13 +166,13 @@
     # A mapping of BugNotificationLevel values to descriptions to be
     # shown on the +subscribe page.
     _bug_notification_level_descriptions = {
+        BugNotificationLevel.COMMENTS: (
+            "a change is made to this bug or a new comment is added, "),
+        BugNotificationLevel.METADATA: (
+            "any change is made to this bug, other than a new comment "
+            "being added, or"),
         BugNotificationLevel.LIFECYCLE: (
-            "The bug is fixed or re-opened."),
-        BugNotificationLevel.METADATA: (
-            "Any change is made to this bug, other than a new comment "
-            "being added."),
-        BugNotificationLevel.COMMENTS: (
-            "A change is made to this bug or a new comment is added."),
+            "the bug is fixed or re-opened."),
         }
 
     @property
@@ -595,7 +595,6 @@
         cache = IJSONRequestCache(self.request).objects
         cache.update(references)
         cache['bug_subscription_info'] = subdata
-        
 
     @property
     def label(self):

=== modified file 'lib/lp/bugs/help/subscription-mute.html'
--- lib/lp/bugs/help/subscription-mute.html	2011-04-13 18:38:54 +0000
+++ lib/lp/bugs/help/subscription-mute.html	2011-04-14 16:45:39 +0000
@@ -12,19 +12,20 @@
     <h1>What happens when I "mute" a bug?</h1>
 
     <p>
-      You may have a subscription to a bug target that sends email
-      notifications about bug activity.  However, you may not be interested in
-      a particular bug.  In that case you can "mute" the bug and you will not
-      recieve notifications about it.
+      You may have a subscription to a bug target (e.g. a project or
+      distribution) that sends email notifications about bug activity.
+      However, you may not be interested in a particular bug.  In that case
+      you can "mute" the bug and you will not receive notifications about it.
     </p>
     <p>
       When "unmuting" a bug you are asked what kinds of messages you would
-      like to recieve about the bug.
+      like to receive about the bug.
     </p>
     <p>
       If you are subscribed via a team, but the team delivers its bug messages
       via a mailing list or some other preferred email, the mute will not be
       honored because doing so would prevent the other team members from
-      recieving the notifications.
+      receiving the notifications.
+    </p>
   </body>
 </html>

=== modified file 'lib/lp/bugs/javascript/bugtask_index_portlets.js'
--- lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-04-13 18:03:03 +0000
+++ lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-04-14 16:45:39 +0000
@@ -252,6 +252,9 @@
 function get_mute_subscription() {
     setup_client_and_bug();
     var mute_link = Y.one('.menu-link-mute_subscription');
+    if (Y.Lang.isNull(mute_link)) {
+        return null;
+    }
     var mute_subscription = new Y.lp.bugs.subscriber.Subscription({
         link: mute_link,
         spinner: Y.one('#mute-unmute-spinner'),
@@ -279,6 +282,9 @@
     }
 
     var mute_subscription = get_mute_subscription();
+    if (Y.Lang.isNull(mute_subscription)) {
+        return;
+    }
     var mute_link = mute_subscription.get('link');
     var parent_node = mute_link.get('parentNode');
     mute_link.addClass('js-action');
@@ -665,7 +671,8 @@
         // If the user has a mute on the bug we add some UI polish to
         // the subscription overlay.
         var mute_subscription = get_mute_subscription();
-        if(mute_subscription.get('is_muted')) {
+        if (!Y.Lang.isNull(mute_subscription) &&
+            mute_subscription.get('is_muted')) {
             Y.lp.bugs.bug_subscription.set_up_bug_notification_level_field();
         }
         subscription_overlay.show();
@@ -1208,9 +1215,11 @@
     var link = subscription.get('link');
     var link_parent = link.get('parentNode');
     var mute_subscription = get_mute_subscription();
+    var is_muted = (!Y.Lang.isNull(mute_subscription) &&
+                    mute_subscription.get('is_muted'));
     if (link_parent.hasClass('subscribed-false') &&
         link_parent.hasClass('dup-subscribed-false') &&
-        !mute_subscription.get('is_muted')) {
+        !is_muted) {
         // The user isn't subscribed or muted, so subscribe them.
         subscription.set(
             'bug_notification_level',
@@ -1229,7 +1238,7 @@
             on: {
                 success: function(lp_subscription) {
                     subscription.enable_spinner('Updating subscription...');
-                    if (mute_subscription.get('is_muted')) {
+                    if (is_muted) {
                         mute_subscription.enable_spinner('Unmuting...');
                     }
                     lp_subscription.set(
@@ -1246,7 +1255,7 @@
                                     node: link_parent
                                     });
                                 anim.run();
-                                if (mute_subscription.get('is_muted')) {
+                                if (is_muted) {
                                     mute_subscription.set('is_muted', false);
                                     mute_subscription.disable_spinner(
                                         "Mute bug mail");
@@ -1272,7 +1281,7 @@
         lp_client.get(subscription_url, config);
     } else {
         // The user is already subscribed and wants to unsubscribe.
-        if (mute_subscription.get('is_muted')) {
+        if (is_muted) {
             unmute_current_user(mute_subscription);
         } else {
             unsubscribe_current_user(subscription);

=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.js'
--- lib/lp/bugs/javascript/tests/test_subscription.js	2011-04-14 15:58:00 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.js	2011-04-14 16:45:39 +0000
@@ -951,9 +951,9 @@
         var sub = {
             bug: {
                 private: false,
-                security_related: false,
+                security_related: false
             },
-            principal_is_reporter: false,
+            principal_is_reporter: false
         };
         var info = {
             direct: _constructCategory([sub]),
@@ -1433,4 +1433,3 @@
     Y.Test.Runner.run();
 });
 });
-

=== modified file 'lib/lp/bugs/templates/bug-portlet-watch.pt'
--- lib/lp/bugs/templates/bug-portlet-watch.pt	2009-09-10 20:18:28 +0000
+++ lib/lp/bugs/templates/bug-portlet-watch.pt	2011-04-14 16:45:39 +0000
@@ -7,7 +7,7 @@
   <h2>Remote bug watches</h2>
   <ul>
     <li tal:repeat="watch context/watches">
-      <span class="sprite bug-remote" />
+      <span class="sprite bug-remote"></span>
       <a tal:replace="structure watch/fmt:external-link" />
       <tal:block tal:condition="watch/remotestatus">
         <br />[<span tal:content="watch/remotestatus" />]

=== modified file 'lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt'
--- lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt	2010-12-21 22:31:04 +0000
+++ lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt	2011-04-14 16:45:39 +0000
@@ -49,7 +49,7 @@
         <button class="lazr-btn yui3-activator-act yui3-activator-hidden">
           Edit
         </button>
-        <div class="yui3-activator-message-box yui3-activator-hidden" />
+        <div class="yui3-activator-message-box yui3-activator-hidden"></div>
       </span>
     </td>
 
@@ -143,7 +143,7 @@
           <button class="lazr-btn yui3-activator-act yui3-activator-hidden">
             Edit
           </button>
-          <div class="yui3-activator-message-box yui3-activator-hidden" />
+          <div class="yui3-activator-message-box yui3-activator-hidden"></div>
         </span>
       </tal:has_no_watch>
     </td>