launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04410
[Merge] lp:~jcsackett/launchpad/decouple-privacy-notifications into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/decouple-privacy-notifications into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/decouple-privacy-notifications/+merge/69174
Summary
=======
We're trying to extend the privacy on private bugs to other contexts (e.g. bugcomments, branches). As part of that work, we need to decouple the privacy ribbon code from the bugtask code.
A previous branch started this by moving the main functions into privacy.js. This branch starts removing some of the tight coupling between those functions and bugtask_index.js
Preimplementation
=================
None; this is a continuation of previous work.
Implementation
==============
Two changes have been implemented in this branch: the placement of the feature flag guard for the privacy ribbon, and the means bugtasks determine whether to show the ribbon.
The feature flag guard check in the bugtask_template has been moved into the base layout, so code, bugs, teams, &c can have the flag set appropriately.
bugtask_index.pt no longer sets a javascript variable based on the bug's private status; instead, the associated javascript file now uses the body element's private css class status to determine whether to fire off display_privacy_notification; as this is consistent across private contexts, it can be re-used.
A new test file, test_privacy.js (and its accompanying html) were also added.
Tests
=====
bin/test -vvct bugtask
firefox lib/lp/app/javascript/tests/test_privacy.html
QA
==
Open a private bug with the privacy feature flag enabled; the ribbon should display as normal.
Confirm that it does not display on a public bug.
--
https://code.launchpad.net/~jcsackett/launchpad/decouple-privacy-notifications/+merge/69174
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/decouple-privacy-notifications into lp:launchpad.
=== modified file 'lib/lp/app/javascript/privacy.js'
--- lib/lp/app/javascript/privacy.js 2011-07-21 18:54:54 +0000
+++ lib/lp/app/javascript/privacy.js 2011-07-28 13:54:56 +0000
@@ -22,6 +22,7 @@
if (global_notification.hasClass('hidden')) {
global_notification.addClass('transparent');
global_notification.removeClass('hidden');
+
var fade_in = new Y.Anim({
node: global_notification,
to: {opacity: 1},
@@ -80,6 +81,9 @@
});
fade_out.on('end', function() {
fade_out.get('node').addClass('hidden');
+
+ //used to track the event for testing
+ this.fire('fadeout');
});
body_space.on('end', function() {
Y.one('body').removeClass('global-notification-visible');
@@ -106,4 +110,4 @@
namespace.hide_privacy_notification = hide_privacy_notification;
-}, "0.1", {"requires": ["base", "node", "lazr.anim", "lazr.base"]});
+}, "0.1", {"requires": ["base", "node", "anim"]});
=== added file 'lib/lp/app/javascript/tests/test_privacy.html'
--- lib/lp/app/javascript/tests/test_privacy.html 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_privacy.html 2011-07-28 13:54:56 +0000
@@ -0,0 +1,34 @@
+<html>
+ <head>
+ <title>Launchpad Comment Hiding</title>
+
+ <!-- YUI and test setup -->
+ <script type="text/javascript"
+ src="../../../../canonical/launchpad/icing/yui/yui/yui.js">
+ </script>
+ <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+ <script type="text/javascript"
+ src="../../../app/javascript/testing/testrunner.js"></script>
+
+ <script type="text/javascript" src="../../../app/javascript/client.js"></script>
+ <script type="text/javascript" src="../../../app/javascript/lp.js"></script>
+
+ <!-- The module under test -->
+ <script type="text/javascript" src="../privacy.js"></script>
+
+ <!-- The test suite -->
+ <script type="text/javascript" src="test_privacy.js"></script>
+ </head>
+ <body class="yui3-skin-sam">
+ <!-- The example markup required by the script to run -->
+ <div class="login-logout" />
+ <div class="global-notification hidden">
+ <span class="sprite notification-private"></span>
+ This bug is private
+ <a href="#" class="global-notification-close">
+ <span class="sprite notification-close"></span>
+ Hide
+ </a>
+ </div>
+ </body>
+</html>
=== added file 'lib/lp/app/javascript/tests/test_privacy.js'
--- lib/lp/app/javascript/tests/test_privacy.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_privacy.js 2011-07-28 13:54:56 +0000
@@ -0,0 +1,41 @@
+/* Copyright 2011 Canonical Ltd. This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ */
+
+// Set the "enabled" variable, normally set by base-layout-macros.
+// This must be a global variable for the code being tested to work.
+var privacy_notification_enabled = true;
+
+YUI().use('lp.testing.runner', 'test', 'console', 'node',
+ 'lp.app.privacy', 'node-event-simulate', function(Y) {
+
+ var suite = new Y.Test.Suite("lp.app.privacy Tests");
+
+ suite.add(new Y.Test.Case({
+ name: 'privacy',
+
+ test_display_shows_ribbon: function () {
+ ribbon = Y.one('.global-notification');
+ Y.Assert.isTrue(ribbon.hasClass('hidden'));
+ Y.lp.app.privacy.display_privacy_notification();
+ Y.Assert.isFalse(ribbon.hasClass('hidden'));
+ },
+
+ _check_ribbon_is_hidden: function () {
+ var ribbon = Y.one('.global-notification');
+ Y.Assert.isTrue(ribbon.hasClass('hidden'));
+ },
+
+ test_hide_removes_ribbon: function () {
+ Y.lp.app.privacy.display_privacy_notification();
+ Y.Assert.isFalse(ribbon.hasClass('hidden'));
+ Y.lp.app.privacy.hide_privacy_notification(false);
+
+ // We listen for a fadeout event to get around the timing issues, o
+ Y.on('fadeout', this._check_ribbon_is_hidden());
+ }
+ }));
+
+ Y.lp.testing.Runner.run(suite);
+});
+
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt 2011-07-08 11:46:41 +0000
+++ lib/lp/app/templates/base-layout-macros.pt 2011-07-28 13:54:56 +0000
@@ -106,6 +106,17 @@
<metal:load-lavascript use-macro="context/@@+base-layout-macros/load-javascript" />
+ <tal:if condition="request/features/bugs.private_notification.enabled">
+ <script type="text/javascript">
+ var privacy_notification_enabled = true;
+ </script>
+ </tal:if>
+ <tal:if condition="not:request/features/bugs.private_notification.enabled">
+ <script type="text/javascript">
+ var privacy_notification_enabled = false;
+ </script>
+ </tal:if>
+
<script id="base-layout-load-scripts" type="text/javascript">
LPS.use('node', 'event-delegate', 'lp', 'lp.app.links', 'lp.app.longpoll', function(Y) {
Y.on('load', function(e) {
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2011-07-27 00:01:47 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2011-07-28 13:54:56 +0000
@@ -43,11 +43,12 @@
/*
* Display the privacy notification if the bug is private
*/
- if (bug_private) {
- Y.on("domready", function () {
+
+ Y.on("domready", function () {
+ if (Y.one(document.body).hasClass('private')) {
Y.lp.app.privacy.display_privacy_notification();
- });
- }
+ }
+ });
/*
* Check the page for links related to overlay forms and request the HTML
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2011-07-25 04:32:49 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2011-07-28 13:54:56 +0000
@@ -10,26 +10,6 @@
<script type='text/javascript' tal:content="string:var yui_base='${yui}';" />
<script type='text/javascript' id='available-official-tags-js'
tal:content="view/available_official_tags_js" />
- <tal:if condition="context/bug/private">
- <script type="text/javascript">
- var bug_private = true;
- </script>
- </tal:if>
- <tal:if condition="not:context/bug/private">
- <script type="text/javascript">
- var bug_private = false;
- </script>
- </tal:if>
- <tal:if condition="request/features/bugs.private_notification.enabled">
- <script type="text/javascript">
- var privacy_notification_enabled = true;
- </script>
- </tal:if>
- <tal:if condition="not:request/features/bugs.private_notification.enabled">
- <script type="text/javascript">
- var privacy_notification_enabled = false;
- </script>
- </tal:if>
<script type="text/javascript">
LPS.use('base', 'node', 'oop', 'event', 'lp.bugs.bugtask_index',
'lp.bugs.subscribers',
