launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/public-private-bugsy-madness into lp:launchpad with lp:~jcsackett/launchpad/public-private-callback-madness as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #898702 in Launchpad itself: "Warn users when they are putting their private team in a public position"
  https://bugs.launchpad.net/launchpad/+bug/898702

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/public-private-bugsy-madness/+merge/85755

Summary
=======
In lp:~jcsackett/launchpad/public-private-callback-madness, the inline-picker was set up with a yes-no confirmation to warn people when they were assigning a private team to a public role. Assignment to bugs was deferred b/c of pending design decisions.

This branch adds the same behavior to bug assignment.

Preimplementation
=================
Spoke with Curtis Hovey

Implementation
==============
There is already an existing yes-no confirmation on private bugs, for when you assign a person who have never previously made a contribution to the project to confirm you intended to assign a new person. This adds the public-private check in the same area.

This has the regrettable side effect of creating the (rare) situation where when a private-team is assigned to a bug on a project for the first time, the user will receive two notifications. I will file a bug for this, but resolving a multiple callback notification issue is outside the scope of this work, and the multiple notifications issue is (regrettably) common in LP.

Tests
=====
bin/test -vvc --layer=YUI

QA
==
Assign a private team to a bugtask; you will recieve a warning that doing so will disclose the existence of the private team.

Lint
====
My JS linter isn't working, but I know that there is lint in bugtask_index.js to fix. I will do so before landing this branch.
-- 
https://code.launchpad.net/~jcsackett/launchpad/public-private-bugsy-madness/+merge/85755
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/public-private-bugsy-madness into lp:launchpad.

=== modified file 'lib/lp/app/templates/inline-picker.pt'
--- lib/lp/app/templates/inline-picker.pt	2011-12-14 21:59:37 +0000
+++ lib/lp/app/templates/inline-picker.pt	2011-12-14 21:59:37 +0000
@@ -43,7 +43,6 @@
         config = {
         on: {
                 success: function (person) {
-                    console.dir(LP.cache.context);
                     private_person = person.get('private'); 
                     public_context = true;
                     if (LP.cache.context.private !== undefined) {

=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2011-12-14 03:24:50 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2011-12-14 21:59:37 +0000
@@ -1038,6 +1038,35 @@
                     return;
                 }
             }
+
+            var do_private_check_save = function () {
+                var client = new Y.lp.client.Launchpad();
+                config = {
+                on: {
+                        success: function (person) {
+                            var private_person = person.get('private'); 
+                            var public_context = true;
+                            if (LP.cache.context.private !== undefined) {
+                                public_context = !(LP.cache.context.private); 
+                            } 
+
+                            if (private_person && public_context) {
+                                var yesno_content =
+                                    '<p>The team you are assigning to this role is private; ' +
+                                    ' continuing will expose the existence of this team.</p>' +
+                                    '<p>Do you wish to continue?</p>'
+                                Y.lp.app.picker.yesno_save_confirmation(
+                                        picker, yesno_content, 'Yes', 'Choose Again',
+                                        save_fn, cancel_fn);
+                            } else {
+                                do_save(); 
+                            }
+                        }
+                    }
+                }
+                client.get(value.api_uri, config);
+            };
+
             var assignee_uri = Y.lp.client.normalize_uri(value.api_uri);
             assignee_uri = Y.lp.client.get_absolute_uri(assignee_uri);
             var error_handler = new Y.lp.client.ErrorHandler();
@@ -1061,10 +1090,10 @@
                             {person_name: person, pillar: pillar});
                     Y.lp.app.picker.yesno_save_confirmation(
                             picker, yesno_content, "Assign", "Choose Again",
-                            save_fn, cancel_fn);
+                            do_private_check_save, cancel_fn);
                 } else {
                     if (Y.Lang.isFunction(save_fn)) {
-                        save_fn();
+                        do_private_check_save();
                     }
                 }
             };