Merge lp:~rharding/launchpad/fix_banner_cleanup into lp:launchpad

[Richard Harding]

Slight clean and decouple of the banner and information type JS.

= Summary =

Originally I introduced a bug in the privacy banner and information type
updates. I had moved a function off the JS module namespace that was used
elsewhere. In the process of updating that, some other code was added that
isn't required. This tries to decouple back up the banner from the information
type as much as possible.

== Implementation Notes ==

The test for the text_node is purely because of the testing. It's a lot of extra html to bootstrap that we don't need to test against.

We force the event driven code to pass the banner text so that the information_type method get_banner_text is kept within it's module and privacy.js isn't calling into it.

The test_privacy then no longer needs to bootstrap all the information_type data in the LP.cache and the tests are updated to make sure the text data is passed into the event.

The event drive code isn't currently used, but will be tied into place in a future branch.

== Q/A ==

The banner should fire and work when a bug is changed to private, to public,
and security related information types.

== Tests ==

test_information_type.html
test_privacy.html

[j.c.sackett (jcsackett)]

Looks fine.

review: Approve

=== modified file 'lib/lp/app/javascript/banners/banner.js'
--- lib/lp/app/javascript/banners/banner.js	2012-09-28 02:39:42 +0000
+++ lib/lp/app/javascript/banners/banner.js	2012-10-08 11:39:21 +0000
@@ -125,7 +125,12 @@
         if (!Y.Lang.isValue(new_text)) {
             new_text = this.get('notification_text');
         }
-        text_node.set('text', new_text);
+
+        if (text_node) {
+            text_node.set('text', new_text);
+        } else {
+            Y.log('No text node to update banner text.', 'error');
+        }
     }
 
 }, {
@@ -147,5 +152,5 @@
 });
 
 }, '0.1', {
-    requires: ['base', 'node', 'anim', 'widget', 'lp.mustache']
+    requires: ['base', 'node', 'anim', 'widget', 'lp.mustache', 'yui-log']
 });

=== modified file 'lib/lp/app/javascript/banners/privacy.js'
--- lib/lp/app/javascript/banners/privacy.js	2012-09-27 03:17:02 +0000
+++ lib/lp/app/javascript/banners/privacy.js	2012-10-08 11:39:21 +0000
@@ -75,26 +75,26 @@
 
     _make_private: function (ev) {
         // Update the text in the banner before we show it.
-        var banner_text;
         var body = Y.one('body');
         body.replaceClass('public', 'private');
 
-        if (ev.text) {
-            banner_text = ev.text;
-        } else {
-           banner_text = Y.lp.app.information_type.get_banner_text(ev.value);
+        if (!ev.text) {
+            throw('Showing a privacy banner must supply text for the banner');
         }
-
-        this.updateText(banner_text);
+        this.updateText(ev.text);
         this.show();
     },
 
     bindUI: function () {
         var that = this;
+        var info_type = Y.lp.app.information_type;
         Banner.prototype.bindUI.apply(this, arguments);
-        var info_type = Y.lp.app.information_type;
+
+        // We care about changes to Information Type in the UI.
         Y.on(info_type.EV_ISPUBLIC, that._make_public, that);
         Y.on(info_type.EV_ISPRIVATE, that._make_private, that);
+
+        // And provide our own manual events for the Security banner usage.
         Y.on(ns.EV_SHOW, that._custom_message, that);
         Y.on(ns.EV_HIDE, function (ev) {
             that.hide();

=== modified file 'lib/lp/app/javascript/banners/tests/test_privacy.js'
--- lib/lp/app/javascript/banners/tests/test_privacy.js	2012-09-27 03:17:02 +0000
+++ lib/lp/app/javascript/banners/tests/test_privacy.js	2012-10-08 11:39:21 +0000
@@ -11,42 +11,6 @@
         name: 'privacy_tests',
 
         setUp: function () {
-            window.LP = {
-                cache: {
-                    context: {
-                        web_link: ''
-                    },
-                    notifications_text: {
-                        muted: ''
-                    },
-                    bug: {
-                        information_type: 'Public',
-                        self_link: '/bug/1'
-                    },
-                    information_type_data: {
-                        PUBLIC: {
-                            value: 'PUBLIC', name: 'Public',
-                            is_private: false, order: 1,
-                            description: 'Public Description'
-                        },
-                        PUBLICSECURITY: {
-                            value: 'PUBLICSECURITY', name: 'Public Security',
-                            is_private: false, order: 2,
-                            description: 'Public Security Description'
-                        },
-                        PROPRIETARY: {
-                            value: 'PROPRIETARY', name: 'Proprietary',
-                            is_private: true, order: 3,
-                            description: 'Private Description'
-                        },
-                        USERDATA: {
-                            value: 'USERDATA', name: 'Private',
-                            is_private: true, order: 4,
-                            description: 'Private Description'
-                        }
-                    }
-                }
-            };
             var main = Y.Node.create('<div id="maincontent"></div>');
             var login_logout = Y.Node.create('<div></div>')
                 .addClass('login-logout');
@@ -120,11 +84,15 @@
 
         test_info_type_private_event: function () {
             var banner = Y.lp.app.banner.privacy.getPrivacyBanner();
+            var body = Y.one('body');
+            var msg = 'Some private message';
+            Y.fire('information_type:is_private', {
+                text: msg,
+                value: 'PROPRIETARY'
+            });
             Y.Assert.areEqual(
-                'The information on this page is private.',
+                msg,
                 Y.one('.global-notification').get('text'));
-            Y.fire('information_type:is_private');
-            var body = Y.one('body');
             Y.Assert.isTrue(body.hasClass('private'),
                             'Body should be private');
             Y.Assert.isTrue(banner.get('visible'),

=== modified file 'lib/lp/app/javascript/information_type.js'
--- lib/lp/app/javascript/information_type.js	2012-09-28 16:47:42 +0000
+++ lib/lp/app/javascript/information_type.js	2012-10-08 11:39:21 +0000
@@ -38,6 +38,7 @@
  *
  * @event information_type:is_private
  * @param value The new information type value.
+ * @param text A message about the new information type value.
  */
 Y.publish(ns.EV_ISPRIVATE, {
     emitFacade: true
@@ -61,6 +62,7 @@
                                   'value',
                                   'is_private')) {
         Y.fire(ns.EV_ISPRIVATE, {
+            text: ns.get_banner_text(ev.value),
             value: ev.value
         });
     } else {

=== modified file 'lib/lp/app/javascript/tests/test_information_type.js'
--- lib/lp/app/javascript/tests/test_information_type.js	2012-09-28 01:25:36 +0000
+++ lib/lp/app/javascript/tests/test_information_type.js	2012-10-08 11:39:21 +0000
@@ -42,7 +42,7 @@
                         PROPRIETARY: {
                             value: 'PROPRIETARY', name: 'Proprietary',
                             is_private: true, order: 3,
-                            description: 'Private Description'
+                            description: 'Proprietary Description'
                         },
                         USERDATA: {
                             value: 'USERDATA', name: 'Private',
@@ -402,6 +402,9 @@
             });
             var private_ev = Y.on('information_type:is_private', function (ev) {
                 Y.Assert.areEqual('PROPRIETARY', ev.value);
+                Y.Assert.areEqual(
+                    'This page contains Proprietary information.',
+                    ev.text);
                 called = true;
             });