launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/public-private-callback-madness into lp:launchpad with lp:~jcsackett/launchpad/patcher-all-the-things 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-callback-madness/+merge/85552

Summary
=======
When a user assigns a private entity (i.e. a private team) to a public role, we want to warn them they are disclosing the existence of the team publicly, and ask if they're sure they want to proceed.

This branch uses a (somewhat elaborate) callback setup to use the existing yesno_confirmation function as a validate_callback for the picker patcher, which is used in all places where this possibility occurs (this was implemented in the dependent branch).

There is another case that needs to be address--assignment to public bugs. Right now bugs already use a yesno validator, and they don't play nicely. This branch attempted to deal with that, but it was causing enough problems that the bug assignment is part of another branch that will follow up this one.

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

Implementation
==============
In the inline-picker template, the setup now includes the creation of a new validator_callback, which checks the private status of the selected entity and the private status of the context being assigned too. If a private entity is being assigned into a public entity, then it sets up the yesno confirmation.

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

QA
==
Attempt to assign a private team as the maintainer of a product or project. It will prompt you to be sure you want to do this, explaining the implicit disclosure.
-- 
https://code.launchpad.net/~jcsackett/launchpad/public-private-callback-madness/+merge/85552
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/public-private-callback-madness into lp:launchpad.

=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py	2011-12-13 21:04:29 +0000
+++ lib/lp/app/browser/lazrjs.py	2011-12-13 21:04:29 +0000
@@ -33,6 +33,7 @@
     )
 from zope.schema.vocabulary import getVocabularyRegistry
 
+from canonical.launchpad.interfaces.launchpad import IPrivacy
 from canonical.launchpad.webapp.interfaces import ILaunchBag
 from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.launchpad.webapp.vocabulary import IHugeVocabulary
@@ -374,6 +375,8 @@
         :param edit_url: The URL to use for editing when the user isn't logged
             in and when JS is off.  Defaults to the edit_view on the context.
         :param edit_title: Used to set the title attribute of the anchor.
+        :param help_link: Used to set a link for help for the widget.
+        :param target_context: The target the person is being set for.
         """
         super(InlinePersonEditPickerWidget, self).__init__(
             context, exported_field, default_html, content_box_id, header,

=== modified file 'lib/lp/app/templates/inline-picker.pt'
--- lib/lp/app/templates/inline-picker.pt	2011-12-13 21:04:29 +0000
+++ lib/lp/app/templates/inline-picker.pt	2011-12-13 21:04:29 +0000
@@ -26,20 +26,56 @@
     </noscript>
     <div class="yui3-activator-message-box yui3-activator-hidden"></div>
 </span>
+
 <script tal:condition="view/can_write"
         tal:content="structure string:
-LPS.use('lp.app.picker', function(Y) {
+LPS.use('lp.app.picker', 'lp.client', function(Y) {
     if (Y.UA.ie) {
         return;
     }
 
+    var picker_config = ${view/json_config}
+
+    var check_disclosure_callback = function(
+        picker, picker_result, do_save, do_cancel) {
+        var client = new Y.lp.client.Launchpad();
+        var private = null;
+        config = {
+        on: {
+                success: function (person) {
+                    console.dir(LP.cache.context);
+                    private_person = person.get('private'); 
+                    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',
+                                do_save, do_cancel);
+                    } else {
+                        do_save(); 
+                    }
+                }
+            }
+        }
+        client.get(picker_result.api_uri, config);
+
+    };
+    picker_config.validate_callback = check_disclosure_callback;
+
     Y.on('load', function(e) {
       Y.lp.app.picker.addPickerPatcher(
         ${view/json_vocabulary_name},
         ${view/json_resource_uri},
         ${view/json_attribute},
         ${view/json_content_box_id},
-        ${view/json_config});
+        picker_config);
       }, window);
 });
 "/>