launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12546
[Merge] lp:~wallyworld/launchpad/privacy-javascript-errors-1057248 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/privacy-javascript-errors-1057248 into lp:launchpad.
Commit message:
Fix javascript errors in privacy.js and add tests
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1057248 in Launchpad itself: "javascript error on bugtask page"
https://bugs.launchpad.net/launchpad/+bug/1057248
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/privacy-javascript-errors-1057248/+merge/126594
== Implementation ==
privacy.js and informationtype.js were recently modified and contained errors and lint.
This branch addresses those issues and adds missing tests which would have caught the issues.
== Tests ==
Add some yui tests.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/javascript/information_type.js
lib/lp/app/javascript/banners/privacy.js
lib/lp/app/javascript/banners/tests/test_privacy.js
--
https://code.launchpad.net/~wallyworld/launchpad/privacy-javascript-errors-1057248/+merge/126594
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/privacy-javascript-errors-1057248 into lp:launchpad.
=== modified file 'lib/lp/app/javascript/banners/privacy.js'
--- lib/lp/app/javascript/banners/privacy.js 2012-09-26 13:31:17 +0000
+++ lib/lp/app/javascript/banners/privacy.js 2012-09-27 03:23:20 +0000
@@ -8,7 +8,6 @@
YUI.add('lp.app.banner.privacy', function(Y) {
var ns = Y.namespace('lp.app.banner.privacy');
var Banner = Y.lp.app.banner.Banner;
-var info_type = Y.lp.app.information_type;
ns.EV_SHOW = 'privacy_banner:show';
ns.EV_HIDE = 'privacy_banner:hide';
@@ -62,20 +61,20 @@
var body = Y.one('body');
body.replaceClass('public', 'private');
if (!ev.text) {
- thow('A custom privacy banner must have a text attribute');
+ throw('A custom privacy banner must have a text attribute');
}
this.updateText(ev.text);
this.show();
},
_make_public: function (ev) {
+ var body = Y.one('body');
body.replaceClass('private', 'public');
this.hide();
},
_make_private: function (ev) {
// Update the text in the banner before we show it.
- debugger;
var banner_text;
var body = Y.one('body');
body.replaceClass('public', 'private');
@@ -83,7 +82,7 @@
if (ev.text) {
banner_text = ev.text;
} else {
- banner_text = info_type.get_banner_text(ev.value);
+ banner_text = Y.lp.app.information_type.get_banner_text(ev.value);
}
this.updateText(banner_text);
@@ -91,15 +90,16 @@
},
bindUI: function () {
- that = this;
+ var that = this;
Banner.prototype.bindUI.apply(this, arguments);
+ var info_type = Y.lp.app.information_type;
Y.on(info_type.EV_ISPUBLIC, that._make_public, that);
Y.on(info_type.EV_ISPRIVATE, that._make_private, that);
Y.on(ns.EV_SHOW, that._custom_message, that);
Y.on(ns.EV_HIDE, function (ev) {
that.hide();
}, that);
- },
+ }
}, {
ATTRS: {
@@ -113,5 +113,7 @@
});
}, "0.1", {
- requires: ["base", "node", "anim", "lp.app.banner", "lp.app.information_type"]
+ requires: [
+ "base", "node", "anim", "lp.app.banner",
+ "lp.app.information_type"]
});
=== modified file 'lib/lp/app/javascript/banners/tests/test_privacy.js'
--- lib/lp/app/javascript/banners/tests/test_privacy.js 2012-09-26 13:02:39 +0000
+++ lib/lp/app/javascript/banners/tests/test_privacy.js 2012-09-27 03:23:20 +0000
@@ -11,6 +11,42 @@
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');
@@ -25,12 +61,13 @@
tearDown: function () {
Y.one('#maincontent').remove(true);
window._singleton_privacy_banner = null;
+ delete window.LP;
},
test_library_exists: function () {
Y.Assert.isObject(Y.lp.app.banner.privacy,
"Could not locate the lp.app.banner.privacy module");
- },
+ },
test_init: function () {
var banner = new Y.lp.app.banner.privacy.PrivacyBanner();
Y.Assert.areEqual(
@@ -79,6 +116,33 @@
Y.fire('privacy_banner:hide');
Y.Assert.isFalse(banner.get('visible'),
'Banner should not be visible.');
+ },
+
+ test_info_type_private_event: function () {
+ var banner = Y.lp.app.banner.privacy.getPrivacyBanner();
+ Y.Assert.areEqual(
+ 'The information on this page is private.',
+ 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'),
+ 'Banner should be visible.');
+ },
+
+ test_info_type_public_event: function () {
+ var banner = Y.lp.app.banner.privacy.getPrivacyBanner();
+ var new_text = 'New custom text';
+
+ Y.fire('privacy_banner:show', {
+ text: new_text
+ });
+ Y.fire('information_type:is_public');
+ var body = Y.one('body');
+ Y.Assert.isTrue(body.hasClass('public'), 'Body should be public');
+ Y.Assert.isFalse(banner.get('visible'),
+ 'Banner should not be visible.');
}
}));
=== modified file 'lib/lp/app/javascript/information_type.js'
--- lib/lp/app/javascript/information_type.js 2012-09-26 13:31:17 +0000
+++ lib/lp/app/javascript/information_type.js 2012-09-27 03:23:20 +0000
@@ -152,7 +152,7 @@
lp_client.io_provider.io(submit_url, config);
};
-var get_banner_text = function(value) {
+ns.get_banner_text = function(value) {
var text_template = "This page contains {info_type} information.";
var info_type = ns.get_cache_data_from_key(value, 'value', 'name');
return Y.Lang.sub(text_template, {'info_type': info_type});
@@ -278,9 +278,12 @@
cache_field,
data_field) {
var cache = LP.cache.information_type_data;
- for (var key in cache) {
- if (cache[key][cache_field] === cache_value) {
- return cache[key][data_field];
+ var key;
+ for (key in cache) {
+ if (cache.hasOwnProperty(key)) {
+ if (cache[key][cache_field] === cache_value) {
+ return cache[key][data_field];
+ }
}
}
};
@@ -296,8 +299,11 @@
*/
ns.cache_to_choicesource = function (cache) {
var data = [];
- for (var key in cache) {
- data.push(cache[key]);
+ var key;
+ for (key in cache) {
+ if (cache.hasOwnProperty(key)) {
+ data.push(cache[key]);
+ }
}
// We need to order data based on the order attribute.
data.sort(function(a, b) { return a.order - b.order; });
@@ -336,7 +342,7 @@
var private_type = LP.cache.information_type_data[value].is_private;
if (private_type) {
body.replaceClass('public', 'private');
- var banner_text = get_banner_text(value);
+ var banner_text = ns.get_banner_text(value);
privacy_banner.updateText(banner_text);
privacy_banner.show();
} else {
Follow ups
