launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/privacy-banner-with-better-info into lp:launchpad with lp:~jcsackett/launchpad/simplify-everything as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/privacy-banner-with-better-info/+merge/107244

Summary
=======
This branch updates the use of the privacy banner on the bugtask index page to
show the appropriate information type in the banner.

Preimp
======
Spoke with Curtis Hovey, Steve Kowalik.

Implementation
==============
* updateText is updated to set 'text', not 'innerText'; 'text' works better in
  dynamic updates in some browsers.

* lp.bugs.browser.bugtask gets an information_type property, which returns the
  information type title for the associated bug.

* the banner-macro privacy banner is updated to grab the view.information_type
  attribute, if it exists. If it exist, the text is rendered as "This page
  contains $info_type data." If not, the default message of "This page
  contains private data." is used.

* information_type_choice.js uses Banner.updateText to update the privacy
  banner text based on the information_type the bug is changed to via the
  portlet. The corresponding test is also updated.

Tests
=====
bin/test -vvc -t beta -t privacy -t banner -t type_choice --layer=YUI

QA
==
Go play with the privacy portlet information type choice picker on a bug
change. The privacy banner text should update with the information_type.
Reload the page a few times with different information_types; it should load
showing the appropriate message for the info type.

LoC
===
This is part of the disclosure project.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/banners/beta-notification.js
  lib/lp/app/javascript/banners/tests/test_banner.js
  lib/lp/app/javascript/banners/banner.js
  lib/lp/bugs/templates/bugtarget-macros-filebug.pt
  lib/lp/app/templates/banner-macros.pt
  lib/lp/app/javascript/banners/privacy.js
  lib/lp/app/javascript/banners/tests/test_privacy.js
  lib/lp/bugs/javascript/tests/test_information_type_choice.js
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/javascript/information_type_choice.js
-- 
https://code.launchpad.net/~jcsackett/launchpad/privacy-banner-with-better-info/+merge/107244
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/privacy-banner-with-better-info into lp:launchpad.

=== modified file 'lib/lp/app/javascript/banners/banner.js'
--- lib/lp/app/javascript/banners/banner.js	2012-05-24 16:43:40 +0000
+++ lib/lp/app/javascript/banners/banner.js	2012-05-24 16:43:40 +0000
@@ -119,7 +119,7 @@
             new_text = this.get('notification_text'); 
         }
         text_node = this.get('contentBox').one('.banner-text');
-        text_node.set('innerText', new_text);
+        text_node.set('text', new_text);
     },
 
 }, {

=== modified file 'lib/lp/app/templates/banner-macros.pt'
--- lib/lp/app/templates/banner-macros.pt	2012-05-24 16:43:40 +0000
+++ lib/lp/app/templates/banner-macros.pt	2012-05-24 16:43:40 +0000
@@ -13,7 +13,16 @@
       <div class="yui3-privacybanner-content">
         <div class="global-notification">
           <span class="sprite notification-private"></span>
-          <span class="banner-text">The information on this page is private.</span>
+          <tal:info_type
+             define="info_type python: getattr(view, 'information_type', None)">
+            <span tal:condition="not: info_type" class="banner-text">
+              The information on this page is private.
+            </span>
+            <span tal:condition="info_type" class="banner-text">
+              <tal:text
+                content="string: This page contains ${info_type} information."/>
+            </span>
+          </tal:info_type>
         </div>
       </div>
     </div>

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2012-05-18 05:57:41 +0000
+++ lib/lp/bugs/browser/bugtask.py	2012-05-24 16:43:40 +0000
@@ -702,6 +702,10 @@
     def recommended_canonical_url(self):
         return canonical_url(self.context.bug, rootsite='bugs')
 
+    @property
+    def information_type(self):
+        return self.context.bug.information_type.title
+
     def initialize(self):
         """Set up the needed widgets."""
         bug = self.context.bug

=== modified file 'lib/lp/bugs/javascript/information_type_choice.js'
--- lib/lp/bugs/javascript/information_type_choice.js	2012-05-17 09:45:29 +0000
+++ lib/lp/bugs/javascript/information_type_choice.js	2012-05-24 16:43:40 +0000
@@ -42,6 +42,9 @@
     subscription_ns.update_subscription_status();
     update_information_type_description(value);
     if (private_type) {
+        privacy_banner.updateText(Y.Lang.substitute(
+            "This page contains {info_type} information.",
+            { info_type: value }));
         body.replaceClass('public', 'private');
         privacy_banner.show();
     } else {

=== modified file 'lib/lp/bugs/javascript/tests/test_information_type_choice.js'
--- lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-05-17 09:45:29 +0000
+++ lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-05-24 16:43:40 +0000
@@ -54,7 +54,8 @@
             Y.lp.app.banner.privacy.getPrivacyBanner = function () {
                 return {
                     show: function () { Y.fire('test:banner:show'); },
-                    hide: function () { Y.fire('test:banner:hide'); }
+                    hide: function () { Y.fire('test:banner:hide'); },
+                    updateText: function () { Y.fire('test:banner:update'); }
                 };
             };
             return old_func;
@@ -88,15 +89,20 @@
                 'Everyone can see this information.',
                 description_node.get('text'));
             var old_func = this._shim_privacy_banner();
-            var flag = false;
+            var hide_flag = false;
+            var update_flag = false;
             Y.on('test:banner:show', function() {
-                flag = true;
+                hide_flag = true;
+            });
+            Y.on('test:banner:update', function() {
+                update_flag = true;
             });
 
             ns.information_type_save_success('Private');
             var body = Y.one('body');
             Y.Assert.isTrue(body.hasClass('private'));
-            Y.Assert.isTrue(flag);
+            Y.Assert.isTrue(hide_flag);
+            Y.Assert.isTrue(update_flag);
             Y.Assert.areEqual('Private', description_node.get('text'));
             this._unshim_privacy_banner(old_func);
         },