launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/do-not-hide-privacy into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/do-not-hide-privacy/+merge/71113

The privacy ribbon should not be hideable.

    Launchpad bug: https://bugs.launchpad.net/bugs/824000
    Pre-implementation: mrevell

Users can hide the privacy ribbon on private pages. This invites users to
forget they are working private data. Disclosure of private information can
cost a company money and contracts. The privacy ribbon is for the benefit of
the company or user who marked the data as private, not the user looking at
the page.

--------------------------------------------------------------------

RULES

    * Remove the 'Hide' onclick action and text from the ribbon
    * Keep the hide_privacy_notification() method because it is when an
      artefact is made public using ajax.


QA

    * Visit a private bug.
    * Verify there is no 'Hide' action.
    * Change the bug to public using the portlet ajax action
    * Verify the ribbon disappears.
    * Change the bug to private using the portlet ajax action
    * Verify the ribbon appears.


LINT

    lib/lp/app/javascript/privacy.js
    lib/lp/app/javascript/tests/test_privacy.js


TEST

    ./bin/test -vv -t test_privacy.html


IMPLEMENTATION

Removed the close node that contains the hide action.
    lib/lp/app/javascript/privacy.js
    lib/lp/app/javascript/tests/test_privacy.js
-- 
https://code.launchpad.net/~sinzui/launchpad/do-not-hide-privacy/+merge/71113
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/do-not-hide-privacy into lp:launchpad.

=== modified file 'lib/lp/app/javascript/privacy.js'
--- lib/lp/app/javascript/privacy.js	2011-08-03 18:12:27 +0000
+++ lib/lp/app/javascript/privacy.js	2011-08-10 21:12:36 +0000
@@ -32,21 +32,10 @@
     var notification_span = Y.Node.create('<span></span>')
         .addClass('sprite')
         .addClass('notification-private');
-    var close_link = Y.Node.create('<a></a>')
-        .addClass('global-notification-close')
-        .set('href', '#');
-    var close_span = Y.Node.create('<span></span>')
-        .addClass('sprite')
-        .addClass('notification-close');
-
     notification.set('text', notification_text);
-    close_link.set('text', "Hide");
 
     main.appendChild(notification);
     notification.appendChild(notification_span);
-    notification.appendChild(close_link);
-    close_link.appendChild(close_span);
-
 }
 namespace.setup_privacy_notification = setup_privacy_notification;
 
@@ -87,11 +76,6 @@
         body_space.run();
         login_space.run();
     }
-
-    Y.one('.global-notification-close').on('click', function(e) {
-        hide_privacy_notification();
-        e.halt();
-    });
 }
 namespace.display_privacy_notification = display_privacy_notification;
 

=== modified file 'lib/lp/app/javascript/tests/test_privacy.js'
--- lib/lp/app/javascript/tests/test_privacy.js	2011-08-01 15:02:45 +0000
+++ lib/lp/app/javascript/tests/test_privacy.js	2011-08-10 21:12:36 +0000
@@ -37,21 +37,12 @@
             var notification_span = Y.Node.create('<span></span>')
                 .addClass('sprite')
                 .addClass('notification-private');
-            var close_link = Y.Node.create('<a></a>')
-                .addClass('global-notification-close')
-                .set('href', '#');
-            var close_span = Y.Node.create('<span></span>')
-                .addClass('sprite')
-                .addClass('notification-close');
 
             notification.set('text', "This bug is private");
-            close_link.set('text', "Hide");
 
             container.appendChild(login_logout);
             container.appendChild(notification);
             notification.appendChild(notification_span);
-            notification.appendChild(close_link);
-            close_link.appendChild(close_span);
         },
 
         test_setup: function() {
@@ -66,9 +57,7 @@
             var ribbon = Y.one('.global-notification');
             Y.Assert.isTrue(ribbon.hasClass('hidden'));
 
-            // Text is both the passed in config and the "Hide" button text.
-            var expected_text = 'stuff is private' + 'Hide';
-            Y.Assert.areEqual(expected_text, ribbon.get('text'));
+            Y.Assert.areEqual(config.notification_text, ribbon.get('text'));
         },
 
         test_display_shows_ribbon: function () {