launchpad-reviewers team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Gary Poster has proposed merging lp:~gary/launchpad/bug754958 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #754958 in Launchpad itself: "Convert "Don't send email about comments" (default unchecked) to "Send email about comments" (default checked)"
  https://bugs.launchpad.net/launchpad/+bug/754958

For more details, see:
https://code.launchpad.net/~gary/launchpad/bug754958/+merge/57010

Ths simple branch changes the UI per a request from jml: the comment checkbox should have positive language, not negative.
-- 
https://code.launchpad.net/~gary/launchpad/bug754958/+merge/57010
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug754958 into lp:launchpad.

=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js	2011-04-07 20:05:55 +0000
+++ lib/lp/registry/javascript/structural-subscription.js	2011-04-08 20:27:33 +0000
@@ -11,7 +11,7 @@
 
 var namespace = Y.namespace('lp.registry.structural_subscription');
 
-var FILTER_COMMENTS = 'filter-comments',
+var INCLUDE_COMMENTS = 'include-comments',
     FILTER_WRAPPER = 'filter-wrapper',
     ACCORDION_WRAPPER = 'accordion-wrapper',
     ADDED_OR_CLOSED = 'added-or-closed',
@@ -81,12 +81,12 @@
 
     // Set the notification level.
     var added_or_closed = list_contains(form_data.events, ADDED_OR_CLOSED);
-    var filter_comments = list_contains(form_data.filters, FILTER_COMMENTS);
+    var include_comments = list_contains(form_data.filters, INCLUDE_COMMENTS);
 
     // Chattiness: Lifecycle < Details < Discussion.
     if (added_or_closed) {
         patch_data.bug_notification_level = 'Lifecycle';
-    } else if (!filter_comments) {
+    } else if (include_comments) {
         patch_data.bug_notification_level = 'Discussion';
     } else {
         patch_data.bug_notification_level = 'Details';
@@ -350,7 +350,9 @@
     set_radio_buttons(
         content_node, [ADDED_OR_CLOSED, ADDED_OR_CHANGED], ADDED_OR_CLOSED);
     set_checkboxes(
-        content_node, [FILTER_COMMENTS, ADVANCED_FILTER], []);
+        content_node,
+        [INCLUDE_COMMENTS, ADVANCED_FILTER],
+        [INCLUDE_COMMENTS]);
     collapse_node(Y.one('#' + ACCORDION_WRAPPER), {duration: 0});
     collapse_node(Y.one('#' + FILTER_WRAPPER), {duration: 0});
 }
@@ -701,9 +703,9 @@
         '      <dt></dt>' +
         '      <dd>' +
         '        <input type="checkbox" name="filters"' +
-        '            value="filter-comments"' +
-        '            id="filter-comments">' +
-        '        <label for="filter-comments">Don\'t send mail about' +
+        '            value="include-comments"' +
+        '            id="include-comments">' +
+        '        <label for="include-comments">Send mail about' +
         '          comments</label><br>' +
         '        <input type="checkbox" name="filters"' +
         '            value="advanced-filter"' +
@@ -972,16 +974,16 @@
             event = ADDED_OR_CLOSED;
             filters = [];
             break;
-        case 'Details':
-            filters.push(FILTER_COMMENTS);
+        case 'Discussion':
+            filters.push(INCLUDE_COMMENTS);
             break;
     }
-    // 'Discussion' case is the default and handled by the declared
+    // 'Details' case is the default and handled by the declared
     // values in the code.
     set_radio_buttons(
         content_node, [ADDED_OR_CLOSED, ADDED_OR_CHANGED], event);
     set_checkboxes(
-        content_node, [FILTER_COMMENTS, ADVANCED_FILTER], filters);
+        content_node, [INCLUDE_COMMENTS, ADVANCED_FILTER], filters);
     handle_change(ADDED_OR_CHANGED, FILTER_WRAPPER, {duration: 0});
     handle_change(ADVANCED_FILTER, ACCORDION_WRAPPER, {duration: 0});
 }
@@ -1313,13 +1315,13 @@
     // Format event details.
     var events; // When will email be sent?
     if (filter.bug_notification_level === 'Discussion') {
-        events = 'You will recieve an email when any change '+
+        events = 'You will receive an email when any change '+
             'is made or a comment is added.';
     } else if (filter.bug_notification_level === 'Details') {
-        events = 'You will recieve an email when any changes '+
+        events = 'You will receive an email when any changes '+
             'are made to the bug.  Bug comments will not be sent.';
     } else if (filter.bug_notification_level === 'Lifecycle') {
-        events = 'You will recieve an email when bugs are '+
+        events = 'You will receive an email when bugs are '+
             'opened or closed.';
     } else {
         throw new Error('Unrecognized events.');

=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js	2011-04-08 15:23:33 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js	2011-04-08 20:27:33 +0000
@@ -782,7 +782,7 @@
             var form_data = {
                 name: [],
                 events: [],
-                filters: ['filter-comments']
+                filters: []
             };
             var patch_data = module._extract_form_data(form_data);
             Assert.areEqual(
@@ -793,7 +793,7 @@
             var form_data = {
                 name: [],
                 events: [],
-                filters: []
+                filters: ['include-comments']
             };
             var patch_data = module._extract_form_data(form_data);
             Assert.areEqual(