launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/security_banner_107842 into lp:launchpad.

Commit message:
Update information type javascript to not count public security as a private event.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1078423 in Launchpad itself: "the security banner stays on the page when changed to public security"
  https://bugs.launchpad.net/launchpad/+bug/1078423

For more details, see:
https://code.launchpad.net/~rharding/launchpad/security_banner_107842/+merge/134187

Small fix to make sure we don't count public security as worthy of having the privacy banner showing with the warning.

To fix this we just update the event filters that signify what's private vs public. Adds tests for the information type values.
-- 
https://code.launchpad.net/~rharding/launchpad/security_banner_107842/+merge/134187
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/security_banner_107842 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/information_type.js'
--- lib/lp/app/javascript/information_type.js	2012-11-12 13:45:58 +0000
+++ lib/lp/app/javascript/information_type.js	2012-11-13 20:11:19 +0000
@@ -59,7 +59,7 @@
         is_private = true;
     }
 
-    if (value.indexOf('SECURITY') !== -1) {
+    if (value.indexOf('PRIVATESECURITY') !== -1) {
         is_private = true;
     }
 
@@ -178,7 +178,7 @@
 ns.get_banner_text = function(value) {
     // Construct a different message for security related banner content.
     var text;
-    if (value.indexOf('SECURITY') !== -1) {
+    if (value.indexOf('PRIVATESECURITY') !== -1) {
         var security_text = "This report will be private " +
                             "because it is a security " +
                             "vulnerability. You can " +

=== modified file 'lib/lp/app/javascript/tests/test_information_type.js'
--- lib/lp/app/javascript/tests/test_information_type.js	2012-11-12 13:45:58 +0000
+++ lib/lp/app/javascript/tests/test_information_type.js	2012-11-13 20:11:19 +0000
@@ -356,6 +356,54 @@
             // Clean up our event since it's global to our Y instance.
             public_ev.detach();
             private_ev.detach();
+        },
+
+        test_private_security_is_private: function () {
+            // A value of PRIVATESECURITY counts as a private event.
+            var called = false;
+
+            // However is should not fire an is_private event.
+            var public_ev = Y.on('information_type:is_public', function (ev) {
+                called = false;
+            });
+            var private_ev = Y.on('information_type:is_private', function (ev) {
+                Y.Assert.areEqual('PRIVATESECURITY', ev.value);
+                called = true;
+            });
+
+            Y.fire('information_type:change', {
+                value: 'PRIVATESECURITY'
+            });
+
+            Y.Assert.isTrue(called, 'Did get a called event');
+
+            // Clean up our event since it's global to our Y instance.
+            public_ev.detach();
+            private_ev.detach();
+        },
+
+        test_public_security_is_public: function () {
+            // A value of PUBLICSECURITY counts as a public event.
+            var called = false;
+
+            // However is should not fire an is_private event.
+            var public_ev = Y.on('information_type:is_public', function (ev) {
+                Y.Assert.areEqual('PUBLICSECURITY', ev.value);
+                called = true;
+            });
+            var private_ev = Y.on('information_type:is_private', function (ev) {
+                called = false;
+            });
+
+            Y.fire('information_type:change', {
+                value: 'PUBLICSECURITY'
+            });
+
+            Y.Assert.isTrue(called, 'Did get a called event');
+
+            // Clean up our event since it's global to our Y instance.
+            public_ev.detach();
+            private_ev.detach();
         }
     }));