launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/privacy-banner-better-text-ff-redo into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1004699 in Launchpad itself: "Privacy banner says "users data", but the portlet says "private""
  https://bugs.launchpad.net/launchpad/+bug/1004699

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/privacy-banner-better-text-ff-redo/+merge/108071

Summary
=======
A branch was created that shows the type of private information when
applicable (e.g. User Data, Embargoed Security). This branch did not however
factor in the requirement that this behavior only be in effect when the
appropriate feature flags are set.

That branch was rolled back; this branch undoes that rollback and adds the
necessary feature flag checks.

Implementation
==============
The information_type property of the BugtaskIndexView now uses the necessary
feature flags in determining what to return. 

The flag indicating use of the term "Privacy" vs "User Data" is now added to
the LP.cache in the same manner as the one that establishes whether to show
information_type in the UI.

The creation of the updated banner text in information_type_choice.js is in
its own small method; it now checks the feature flags to determine how to
update the banner text.

Tests
=====
bin/test -vvct test_bugtask
bin/test -vvct type_choice --layer=YUI

QA
==
Check the text in the privacy banner on a bugtask index page under all
combinations of the relevant feature flags.

LoC
===
This is part of disclosure work.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/templates/banner-macros.pt
  lib/lp/app/browser/informationtype.py
  lib/lp/bugs/javascript/tests/test_information_type_choice.js
  lib/lp/bugs/browser/tests/test_bugtask.py
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/javascript/information_type_choice.js
-- 
https://code.launchpad.net/~jcsackett/launchpad/privacy-banner-better-text-ff-redo/+merge/108071
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/privacy-banner-better-text-ff-redo into lp:launchpad.

=== modified file 'lib/lp/app/browser/informationtype.py'
--- lib/lp/app/browser/informationtype.py	2012-05-30 05:04:40 +0000
+++ lib/lp/app/browser/informationtype.py	2012-05-30 22:03:19 +0000
@@ -29,6 +29,8 @@
             type.title for type in PRIVATE_INFORMATION_TYPES]
         cache.objects['show_information_type_in_ui'] = (
             self.show_information_type_in_ui)
+        cache.objects['show_userdata_as_private'] = (
+            self.show_userdata_as_private)
 
     @property
     def show_information_type_in_ui(self):

=== modified file 'lib/lp/app/templates/banner-macros.pt'
--- lib/lp/app/templates/banner-macros.pt	2012-05-29 13:22:21 +0000
+++ lib/lp/app/templates/banner-macros.pt	2012-05-30 22:03:19 +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-29 13:22:21 +0000
+++ lib/lp/bugs/browser/bugtask.py	2012-05-30 22:03:19 +0000
@@ -702,6 +702,19 @@
     def recommended_canonical_url(self):
         return canonical_url(self.context.bug, rootsite='bugs')
 
+    @property
+    def information_type(self):
+        use_private_flag = getFeatureFlag(
+            'disclosure.display_userdata_as_private.enabled')
+        info_type_enabled_flag = getFeatureFlag(
+            'disclosure.show_information_type_in_ui.enabled')
+        value = None
+        if info_type_enabled_flag:
+            value = self.context.bug.information_type.title
+            if (use_private_flag and value == InformationType.USERDATA.title):
+                value = "Private"
+        return value
+
     def initialize(self):
         """Set up the needed widgets."""
         bug = self.context.bug

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2012-05-23 14:20:13 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2012-05-30 22:03:19 +0000
@@ -292,6 +292,21 @@
             'href="/foobar/+bugs?field.tag=depends-on%2B987"',
             browser.contents)
 
+    def test_information_type_with_flags(self):
+        owner = self.factory.makePerson()
+        bug = self.factory.makeBug(
+            owner=owner,
+            information_type=InformationType.USERDATA)
+        login_person(owner)
+        bugtask = self.factory.makeBugTask(bug=bug)
+        features = {'disclosure.show_information_type_in_ui.enabled': True}
+        with FeatureFixture(features):
+            view = create_initialized_view(bugtask, name="+index")
+            self.assertEqual('User Data', view.information_type)
+        features['disclosure.display_userdata_as_private.enabled'] = True
+        with FeatureFixture(features):
+            view = create_initialized_view(bugtask, name="+index")
+            self.assertEqual('Private', view.information_type)
 
 class TestBugTasksAndNominationsView(TestCaseWithFactory):
 

=== modified file 'lib/lp/bugs/javascript/information_type_choice.js'
--- lib/lp/bugs/javascript/information_type_choice.js	2012-05-29 13:22:21 +0000
+++ lib/lp/bugs/javascript/information_type_choice.js	2012-05-30 22:03:19 +0000
@@ -34,6 +34,21 @@
     }
 };
 
+namespace.get_information_type_banner_text = function(value) {
+    console.dir(LP.cache);
+    var fallback_text = "The information on this page is private.";
+    var text_template = "This page contains {info_type} information.";
+
+    if (value === "User Data" && LP.cache.show_userdata_as_private) {
+            value = "Private";
+    }
+    if (LP.cache.show_information_type_in_ui) {
+        return Y.Lang.substitute(text_template, {'info_type': value});
+    } else {
+        return fallback_text; 
+    }
+};
+
 namespace.information_type_save_success = function(value) {
     var body = Y.one('body');
     var private_type = (Y.Array.indexOf(LP.cache.private_types, value) >= 0);
@@ -42,6 +57,8 @@
     subscription_ns.update_subscription_status();
     update_information_type_description(value);
     if (private_type) {
+        var banner_text = namespace.get_information_type_banner_text(value);
+        privacy_banner.updateText(banner_text);
         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-29 13:22:21 +0000
+++ lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-05-30 22:03:19 +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);
         },