launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/privacy-banner-filebug into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1029524 in Launchpad itself: "Privacy banner is not shown when there is an error reporting a bug"
  https://bugs.launchpad.net/launchpad/+bug/1029524

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/privacy-banner-filebug/+merge/120474

Summary
=======
When a bug is marked as Private Security on filebug, the banner is shown;
however, if the form reloads b/c of an error, the banner vanishes even though
the form continues to say it is Private Security.

The code that determines the initial display of the banner on filebug isn't
aware of the value of the privacy selector.

Preimp
======
Spoke with Curtis Hovey.

Implementation
==============
There is already a method to determine if the banner needs to be shown on page
load, but it only knows about private by default bugs.

Code used to update the banner from the selected information type has been
moved into its own method, that is both used for the 'change' delegate on the
selector and for displaying the banner on initial load. The value is pulled
from the selector and passed into this function if bugs are not private by
default.

Tests
=====
bin/test -vvct filebug --layer=YUI

QA
==
File a private security bug without a description, causing a form error and
reload; the banner should still be displayed.

LoC
===
This is part of disclosure.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/javascript/filebug.js
-- 
https://code.launchpad.net/~jcsackett/launchpad/privacy-banner-filebug/+merge/120474
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/privacy-banner-filebug into lp:launchpad.

=== modified file 'lib/lp/bugs/javascript/filebug.js'
--- lib/lp/bugs/javascript/filebug.js	2012-07-17 03:51:38 +0000
+++ lib/lp/bugs/javascript/filebug.js	2012-08-20 21:42:25 +0000
@@ -28,18 +28,30 @@
         if (Y.Lang.isValue(search_button )) {
             search_button.set('value', 'Check again');
         }
+        set_default_privacy_banner();
         setup_information_type();
         setup_security_related();
         setupChoiceWidgets();
-        set_default_privacy_banner();
     }
 };
 
 var set_default_privacy_banner = function() {
-    var filebug_privacy_text = "This report will be private. " +
-        "You can disclose it later.";
-    update_privacy_banner(
-        LP.cache.bug_private_by_default, filebug_privacy_text);
+    var itypes_table = Y.one('.radio-button-widget');
+    var val = null;
+    if (itypes_table !== null) {
+        val = itypes_table.one(
+            "input[name='field.information_type']:checked").get('value');
+    } else {
+        val = 'PUBLIC'; 
+    }
+    
+    if (LP.cache.bug_private_by_default) {
+        var filebug_privacy_text = "This report will be private. " +
+            "You can disclose it later.";
+        update_privacy_banner(true, filebug_privacy_text);
+    } else {
+        update_banner_from_information_type(val);
+    }
 };
 
 var update_privacy_banner = function(show, banner_text) {
@@ -63,17 +75,22 @@
     return Y.Lang.substitute(text_template, {'info_type': value});
 };
 
+var update_banner_from_information_type = function(value) {
+    var banner_text = get_new_banner_text(value);
+    var private_type = (Y.Array.indexOf(
+        LP.cache.private_types, value) >= 0);
+    update_privacy_banner(private_type, banner_text);
+};
+
 var setup_information_type = function() {
     var itypes_table = Y.one('.radio-button-widget');
     if (!Y.Lang.isValue(itypes_table)) {
         return;
     }
+
+
     itypes_table.delegate('change', function() {
-        var banner_text = get_new_banner_text(this.get('value'));
-        var private_type = (Y.Array.indexOf(
-            LP.cache.private_types, this.get('value')) >= 0);
-
-        update_privacy_banner(private_type, banner_text);
+        update_banner_from_information_type(this.get('value'));
     }, "input[name='field.information_type']");
 };