launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03275
[Merge] lp:~wallyworld/launchpad/assign-non-contributor into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/assign-non-contributor into lp:launchpad with lp:~wallyworld/launchpad/windmill-setup-improvement as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #698138 in Launchpad itself: "Assigning a non-contributor to a bugtask should show a warning"
https://bugs.launchpad.net/launchpad/+bug/698138
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/assign-non-contributor/+merge/55869
When a bug task assignee is changed inline and the new assignee is a non-contributer, prompt the user to confirm their choice of assignee. Currently the warning is only display if Javascript is disabled and the assignee changed using a form submission.
== Implementation ==
This new functionality is delivered on top of new validation infrastructure built into Launchpad's picker widget (which itself extends Y.lazr.Picker).
When a picker is created, one can optionally pass in a validation callback. This will be invoked when the user selects a picker item. Without the validation callback, the picker save function is invoked immediately. However, with the validation callback, the callback is invoked and returns either true/false. If true, save is invoked, if false, the picker stays open at the current selection.
For this bug fix, a yesno validation callback is implemented in the picker. This is the first example of using the validation callback infrastructure. The picker widget provides the boiler plate to wire it all together. The bug task Javascript provides the actual callback business logic. When the user makes a selection, an xhr call is made to see if that person is a contributor. If they aren't, a confirmation prompt appears allowing the user to answer "no" and choose another assignee. The conformation prompt replaces the picker content and animation is used to make it look good.
== Demo ==
Here's a screencast: http://people.canonical.com/~ianb/assignee_validation.ogv
== Tests ==
Tests were added for the new infrastructure and a Windmill test. The picker implementation was modified slightly to allow a list of vocabulary terms to be passed in as well as a vocabulary name. This was required to allow a test case to create a picker.
There were no existing Javascript tests for Launchpad's picker so a new test suite was added: lp/app/javascript/tests/picker.js
This test suite specifically tests the new yes/no validation callback added to the picker:
test_picker_can_be_instantiated()
test_no_confirmation_required()
test_confirmation_yes()
test_confirmation_no()
A unit test was added for the BugContributorView:
TestBugContributorView
test_non_contributor()
test_contributor()
A windmill test was added for the specific use case of in-line bug assignment:
lp/bugs/windmill/tests/test_bug_inline_assignment.py
= Launchpad lint =
I fixed some existing Javascript lint issues but was not comfortable about changing the suggested !== and === items.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/javascript/picker.js
lib/lp/app/javascript/tests/picker.html
lib/lp/app/javascript/tests/picker.js
lib/lp/bugs/browser/bugtask.py
lib/lp/bugs/browser/configure.zcml
lib/lp/bugs/browser/tests/test_bugtask.py
lib/lp/bugs/javascript/bugtask_index.js
lib/lp/bugs/windmill/tests/test_bug_inline_assignment.py
./lib/lp/app/javascript/picker.js
92: Expected '!==' and instead saw '!='.
222: Expected '!==' and instead saw '!='.
286: Expected '!==' and instead saw '!='.
302: Expected '!==' and instead saw '!='.
307: Expected '!==' and instead saw '!='.
355: Expected '!==' and instead saw '!='.
355: Expected '!==' and instead saw '!='.
422: Move 'var' declarations to the top of the function.
422: Stopping. (86% scanned).
0: JSLINT had a fatal error.
193: Line exceeds 78 characters.
./lib/lp/bugs/javascript/bugtask_index.js
269: Expected '===' and instead saw '=='.
726: Expected '===' and instead saw '=='.
759: Expected '===' and instead saw '=='.
815: Expected '===' and instead saw '=='.
817: Expected '===' and instead saw '=='.
886: Expected '===' and instead saw '=='.
1008: Unexpected 'in'. Compare with undefined, or use the hasOwnProperty method instead.
1022: Expected '===' and instead saw '=='.
1035: Expected '===' and instead saw '=='.
387: Line exceeds 78 characters.
388: Line exceeds 78 characters.
389: Line exceeds 78 characters.
443: Line exceeds 78 characters.
--
https://code.launchpad.net/~wallyworld/launchpad/assign-non-contributor/+merge/55869
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/assign-non-contributor into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css 2011-03-30 10:47:41 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css 2011-04-11 04:28:30 +0000
@@ -1054,6 +1054,11 @@
.sml-informational::before {
content: url('/@@/info');
}
+.important-notice-popup {
+ padding: 1em 1em 0 1em;
+ width: auto;
+ overflow: hidden;
+ }
.important-notice-container {
text-align: center;
width: 100%;
=== modified file 'lib/lp/app/javascript/picker.js'
--- lib/lp/app/javascript/picker.js 2011-03-31 04:52:03 +0000
+++ lib/lp/app/javascript/picker.js 2011-04-11 04:28:30 +0000
@@ -39,7 +39,7 @@
var remove_button_text = 'Remove';
var null_display_value = 'None';
var show_search_box = true;
- resource_uri = Y.lp.client.normalize_uri(resource_uri)
+ resource_uri = Y.lp.client.normalize_uri(resource_uri);
if (config !== undefined) {
if (config.remove_button_text !== undefined) {
@@ -101,7 +101,6 @@
var success_handler = function (entry) {
activator.renderSuccess(entry.getHTML(attribute_name));
show_hide_buttons();
- return;
};
var patch_payload = {};
@@ -151,7 +150,7 @@
};
config.save = save;
- var picker = namespace.create(vocabulary, config);
+ var picker = namespace.create(vocabulary, config, activator);
picker._resource_uri = resource_uri;
var extra_buttons = Y.Node.create(
'<div style="text-align: center; height: 3em; ' +
@@ -219,8 +218,110 @@
* Remove the Loading.... spinner (if it exists).
*/
function hide_temporary_spinner(temp_spinner) {
- if( temp_spinner != null )
+ if( temp_spinner != null ) {
temp_spinner.addClass('unseen');
+ }
+}
+
+/*
+ * After the user selects an item using the picker, this function can be used
+ * to present the user with a yes/no prompt to ensure they really want to use
+ * the selection. If they answer yes, the selection is processed as normal. If
+ * they answer no, they can make another selection.
+ *
+ * @param {Picker} picker The picker displaying the yes/no content.
+ * @param {String} content The html content to display.
+ * @param {String} yes_label The label for the "yes" button.
+ * @param {String} no_label The label for the "no" button.
+ * @param {Object} yes_fn The function to call if the user answers yes.
+ * @param {Object} no_fn The function to call if the user answers no.
+ */
+namespace.yesno_save_confirmation = function(
+ picker, content, yes_label, no_label, yes_fn, no_fn) {
+ var validation_content_container = Y.Node.create(
+ "<div class='validation-node'/>");
+ validation_content_container.appendChild(Y.Node.create(
+ '<div class="step-on" style="width: 100%;"/>'));
+
+ var ask_user_node = Y.Node.create("<div class='transparent'/>");
+ var ask_content = Y.Node.create(
+ "<div class='important-notice-popup'/>");
+ ask_content.setContent(content);
+ var buttons = Y.Node.create(
+ '<div class="yesyno_buttons" style="text-align: center; ' +
+ 'height: 3em; white-space: nowrap"/>');
+ var yes_button = Y.Node.create(
+ '<button type="button">'+yes_label+'</button>');
+ var no_button = Y.Node.create(
+ '<button type="button">'+no_label+'</button>');
+ yes_button.on('click', function(e) {
+ e.halt();
+ if (Y.Lang.isFunction(yes_fn) ) {
+ yes_fn();
+ }
+ form_reset(picker);
+ });
+ no_button.on('click', function(e) {
+ e.halt();
+ if (Y.Lang.isFunction(no_fn) ) {
+ no_fn();
+ }
+ form_reset(picker);
+ });
+ buttons.appendChild(yes_button);
+ buttons.appendChild(no_button);
+ ask_user_node.appendChild(ask_content);
+ ask_user_node.appendChild(buttons);
+ validation_content_container.appendChild(ask_user_node);
+ var content_node = picker.get('contentBox').one('.yui3-widget-bd');
+ content_node.insert(validation_content_container, 'before');
+ animate_validation_content(picker, ask_user_node);
+};
+
+/*
+ * Insert the validation content into the form and animate its appearance.
+ */
+function animate_validation_content(picker, validation_content) {
+ var content_node = picker.get('contentBox').one('.yui3-widget-bd');
+ content_node.hide();
+ var steps_node = picker.get('contentBox').one('.steps');
+ if (steps_node != null) {
+ steps_node.hide();
+ }
+ var validation_fade_in = new Y.Anim({
+ node: validation_content,
+ to: {opacity: 1},
+ duration: 0.9
+ });
+ validation_fade_in.run();
+}
+
+/*
+ * Restore a picker to it's functional state after a validation operation.
+ */
+function form_reset(picker) {
+ var steps_node = picker.get('contentBox').one('.steps');
+ if (steps_node != null) {
+ steps_node.show();
+ }
+ var validation_node = picker.get('contentBox').one('.validation-node');
+ var content_node = picker.get('contentBox').one('.yui3-widget-bd');
+ if (validation_node != null) {
+ validation_node.get('parentNode').removeChild(validation_node);
+ content_node.addClass('transparent');
+ content_node.setStyle('opacity', 0);
+ content_node.show();
+ var content_fade_in = new Y.Anim({
+ node: content_node,
+ to: {opacity: 1},
+ duration: 0.6
+ });
+ content_fade_in.run();
+ } else {
+ content_node.removeClass('transparent');
+ content_node.setStyle('opacity', 1);
+ content_node.show();
+ }
}
/**
@@ -236,27 +337,27 @@
* config.save is a Function (optional) which takes
* a single string argument.
*/
-namespace.create = function (vocabulary, config) {
+namespace.create = function (vocabulary, config, activator) {
if (Y.UA.ie) {
return;
}
+ var header = 'Choose an item.';
+ var step_title = "Enter search terms";
if (config !== undefined) {
- var header = 'Choose an item.';
if (config.header !== undefined) {
header = config.header;
}
- var step_title = "Enter search terms";
if (config.step_title !== undefined) {
step_title = config.step_title;
}
}
- if (typeof vocabulary != 'string') {
+ if (typeof vocabulary != 'string' && typeof vocabulary != 'object') {
throw new TypeError(
"vocabulary argument for Y.lp.picker.create() must be a " +
- "string: " + vocabulary);
+ "string or associative array: " + vocabulary);
}
var new_config = Y.merge(config, {
@@ -275,13 +376,26 @@
picker.subscribe('save', function (e) {
Y.log('Got save event.');
- if (Y.Lang.isFunction(config.save)) {
- config.save(e.details[Y.lazr.Picker.SAVE_RESULT]);
+ e.preventDefault();
+ var picker_result = e.details[Y.lazr.Picker.SAVE_RESULT];
+ var do_save = function() {
+ if (Y.Lang.isFunction(config.save)) {
+ config.save(picker_result);
+ }
+ picker._defaultSave(e);
+ };
+ var validate_callback = config.validate_callback;
+ if (Y.Lang.isFunction(validate_callback)) {
+ validate_callback(
+ picker, picker_result, do_save, undefined);
+ } else {
+ do_save();
}
});
picker.subscribe('cancel', function (e) {
Y.log('Got cancel event.');
+ form_reset(picker);
});
// Search for results, create batches and update the display.
@@ -291,15 +405,7 @@
var search_text = e.details[0];
var selected_batch = e.details[1] || 0;
var start = BATCH_SIZE * selected_batch;
- var client = new Y.lp.client.Launchpad();
-
- var success_handler = function (ignore, response, args) {
- var entry = Y.JSON.parse(response.responseText);
- var total_size = entry.total_size;
- var start = entry.start;
- var results = entry.entries;
-
- hide_temporary_spinner(config.temp_spinner);
+ var display_vocabulary = function(results, total_size, start) {
if (total_size > (MAX_BATCHES * BATCH_SIZE)) {
picker.set('error',
'Too many matches. Please try to narrow your search.');
@@ -322,36 +428,50 @@
});
}
}
-
picker.set('batches', batches);
}
}
};
- var qs = '';
- qs = Y.lp.client.append_qs(qs, 'name', vocabulary);
- qs = Y.lp.client.append_qs(qs, 'search_text', search_text);
- qs = Y.lp.client.append_qs(qs, 'batch', BATCH_SIZE);
- qs = Y.lp.client.append_qs(qs, 'start', start);
-
- // The uri needs to be relative, so that the vocabulary
- // has the same context as the form. Some vocabularies
- // use the context to limit the results to the same project.
- var uri = '@@+huge-vocabulary?' + qs;
-
- Y.io(uri, {
- headers: {'Accept': 'application/json'},
- timeout: 20000,
- on: {
- success: success_handler,
- failure: function (arg) {
- hide_temporary_spinner(config.temp_spinner);
- picker.set('error', 'Loading results failed.');
- picker.set('search_mode', false);
- Y.log("Loading " + uri + " failed.");
+ // We can pass in a vocabulary name
+ if (typeof vocabulary == 'string') {
+ var success_handler = function (ignore, response, args) {
+ var entry = Y.JSON.parse(response.responseText);
+ var total_size = entry.total_size;
+ var start = entry.start;
+ var results = entry.entries;
+ hide_temporary_spinner(config.temp_spinner);
+ display_vocabulary(results, total_size, start);
+ };
+
+ var qs = '';
+ qs = Y.lp.client.append_qs(qs, 'name', vocabulary);
+ qs = Y.lp.client.append_qs(qs, 'search_text', search_text);
+ qs = Y.lp.client.append_qs(qs, 'batch', BATCH_SIZE);
+ qs = Y.lp.client.append_qs(qs, 'start', start);
+
+ // The uri needs to be relative, so that the vocabulary
+ // has the same context as the form. Some vocabularies
+ // use the context to limit the results to the same project.
+ var uri = '@@+huge-vocabulary?' + qs;
+
+ Y.io(uri, {
+ headers: {'Accept': 'application/json'},
+ timeout: 20000,
+ on: {
+ success: success_handler,
+ failure: function (arg) {
+ hide_temporary_spinner(config.temp_spinner);
+ picker.set('error', 'Loading results failed.');
+ picker.set('search_mode', false);
+ Y.log("Loading " + uri + " failed.");
+ }
}
- }
- });
+ });
+ // Or we can pass in a vocabulary directly.
+ } else {
+ display_vocabulary(vocabulary, Y.Object.size(vocabulary), 1);
+ }
};
picker.after('search', search_handler);
=== added file 'lib/lp/app/javascript/tests/picker.html'
--- lib/lp/app/javascript/tests/picker.html 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/picker.html 2011-04-11 04:28:30 +0000
@@ -0,0 +1,48 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
+ "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd";>
+<html>
+ <head>
+ <title>Launchpad Picker</title>
+
+ <!-- YUI 3.0 Setup -->
+ <script type="text/javascript" src="../../../../canonical/launchpad/icing/yui/yui/yui.js"></script>
+ <link rel="stylesheet" href="../../../../canonical/launchpad/icing/yui/cssreset/reset.css"/>
+ <link rel="stylesheet" href="../../../../canonical/launchpad/icing/yui/cssfonts/fonts.css"/>
+ <link rel="stylesheet" href="../../../../canonical/launchpad/icing/yui/cssbase/base.css"/>
+ <link rel="stylesheet" href="../../../../canonical/launchpad/javascript/test.css" />
+
+ <!-- Some required dependencies -->
+ <script type="text/javascript" src="../../../../canonical/launchpad/icing/lazr/build/testing/config.js"></script>
+ <script type="text/javascript" src="../../../../canonical/launchpad/icing/lazr/build/activator/activator.js"></script>
+ <script type="text/javascript" src="../../../../canonical/launchpad/icing/lazr/build/overlay/overlay.js"></script>
+ <script type="text/javascript" src="../../../../canonical/launchpad/icing/lazr/build/picker/picker.js"></script>
+ <script type="text/javascript" src="../../../../canonical/launchpad/icing/lazr/build/anim/anim.js"></script>
+ <script type="text/javascript" src="../../../../canonical/launchpad/icing/lazr/build/lazr/lazr.js"></script>
+ <script type="text/javascript" src="../client.js"></script>
+
+ <!-- The module under test -->
+ <script type="text/javascript" src="../picker.js"></script>
+
+ <!-- The test suite -->
+ <script type="text/javascript" src="picker.js"></script>
+</head>
+<body class="yui3-skin-sam">
+ <div class="yui3-widget yui3-activator yui3-activator-focused">
+ <span id="picker_id" class="yui3-activator-content yui3-activator-success">
+ <span id="pickertest">
+ <span>
+ A picker widget test
+ <button id="edit-pickertest-btn"
+ class="lazr-btn yui3-activator-act yui3-activator-hidden">
+ Edit
+ </button>
+ </span>
+ <span class="yui3-activator-data-box">
+ </span>
+ <div class="yui3-activator-message-box yui3-activator-hidden"/>
+ </span>
+ </span>
+ </div>
+ <div id="log"></div>
+</body>
+</html>
=== added file 'lib/lp/app/javascript/tests/picker.js'
--- lib/lp/app/javascript/tests/picker.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/picker.js 2011-04-11 04:28:30 +0000
@@ -0,0 +1,180 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI({
+ base: '../../../../canonical/launchpad/icing/yui/',
+ filter: 'raw',
+ combine: false,
+ fetchCSS: false
+ }).use('test', 'console', 'node', 'lp', 'lp.client', 'escape', 'event',
+ 'event-focus', 'event-simulate', 'lazr.picker','lp.app.picker',
+ function(Y) {
+
+var Assert = Y.Assert;
+
+/*
+ * A wrapper for the Y.Event.simulate() function. The wrapper accepts
+ * CSS selectors and Node instances instead of raw nodes.
+ */
+function simulate(widget, selector, evtype, options) {
+ var rawnode = Y.Node.getDOMNode(widget.one(selector));
+ Y.Event.simulate(rawnode, evtype, options);
+}
+
+/* Helper function to clean up a dynamically added widget instance. */
+function cleanup_widget(widget) {
+ // Nuke the boundingBox, but only if we've touched the DOM.
+ if (widget.get('rendered')) {
+ var bb = widget.get('boundingBox');
+ bb.get('parentNode').removeChild(bb);
+ }
+ // Kill the widget itself.
+ widget.destroy();
+}
+
+var suite = new Y.Test.Suite("Launchpad Picker Tests");
+
+/*
+ * Test cases for a picker with a yesno validation callback.
+ */
+suite.add(new Y.Test.Case({
+
+ name: 'picker_yesyno_validation',
+
+ setUp: function() {
+ this.vocabulary = [
+ {"value": "fred", "title": "Fred", "css": "sprite-person",
+ "description": "fred@xxxxxxxxxxx", "api_uri": "~/fred"},
+ {"value": "frieda", "title": "Frieda", "css": "sprite-person",
+ "description": "frieda@xxxxxxxxxxx", "api_uri": "~/frieda"}
+ ];
+ },
+
+ tearDown: function() {
+ cleanup_widget(this.picker);
+ },
+
+ create_picker: function(validate_callback) {
+ this.picker = Y.lp.app.picker.addPickerPatcher(
+ this.vocabulary,
+ "foo/bar",
+ "test_link",
+ "picker_id",
+ {"step_title": "Choose someone",
+ "header": "Pick Someone",
+ "remove_button_text": "Remove someone",
+ "null_display_value": "Noone",
+ "show_remove_button": true,
+ "show_assign_me_button": true,
+ "validate_callback": validate_callback});
+ },
+
+ test_picker_can_be_instantiated: function() {
+ // Ensure the picker can be instantiated.
+ this.create_picker();
+ Assert.isInstanceOf(
+ Y.lazr.Picker, this.picker, "Picker failed to be instantiated");
+ },
+
+ // Called when the picker saves it's data. Sets a flag for checking.
+ picker_save_callback: function(save_flag) {
+ return function(e) {
+ save_flag.event_has_fired = true;
+ };
+ },
+
+ // A validation callback stub. Instead of going to the server to see if
+ // a picker value requires confirmation, we compare it to a known value.
+ yesno_validate_callback: function(save_flag, expected_value) {
+ var save_fn = this.picker_save_callback(save_flag);
+ return function(picker, value, ignore, cancel_fn) {
+ Assert.areEqual(
+ expected_value, value.api_uri, "unexpected picker value");
+ if (value === null) {
+ return true;
+ }
+ var requires_confirmation = value.api_uri !== "~/fred";
+ if (requires_confirmation) {
+ var yesno_content = "<p>Confirm selection</p>";
+ return Y.lp.app.picker.yesno_save_confirmation(
+ picker, yesno_content, "Yes", "No",
+ save_fn, cancel_fn);
+ } else {
+ save_fn();
+ }
+ return True;
+ };
+ },
+
+ test_no_confirmation_required: function() {
+ // Test that the picker saves the selected value if no confirmation
+ // is required.
+ var save_flag = {};
+ this.create_picker(this.yesno_validate_callback(save_flag, "~/fred"));
+ this.picker.set('results', this.vocabulary);
+ this.picker.render();
+
+ simulate(
+ this.picker.get('boundingBox').one('.yui3-picker-results'),
+ 'li:nth-child(1)', 'click');
+ Assert.isTrue(save_flag.event_has_fired, "save event wasn't fired.");
+ },
+
+ test_confirmation_yes: function() {
+ // Test that the picker saves the selected value if the user answers
+ // "Yes" to a confirmation request.
+ var save_flag = {};
+ this.create_picker(
+ this.yesno_validate_callback(save_flag, "~/frieda"));
+ this.picker.set('results', this.vocabulary);
+ this.picker.render();
+
+ simulate(
+ this.picker.get('boundingBox').one('.yui3-picker-results'),
+ 'li:nth-child(2)', 'click');
+ var yesno = this.picker.get('contentBox').one('.yesyno_buttons');
+ simulate(
+ yesno, 'button:nth-child(1)', 'click');
+ Assert.isTrue(
+ save_flag.event_has_fired, "save event wasn't fired.");
+ },
+
+ test_confirmation_no: function() {
+ // Test that the picker doesn't save the selected value if the answers
+ // "No" to a confirmation request.
+ var save_flag = {};
+ this.create_picker(
+ this.yesno_validate_callback(save_flag, "~/frieda"));
+ this.picker.set('results', this.vocabulary);
+ this.picker.render();
+
+ simulate(
+ this.picker.get('boundingBox').one('.yui3-picker-results'),
+ 'li:nth-child(2)', 'click');
+ var yesno = this.picker.get('contentBox').one('.yesyno_buttons');
+ simulate(
+ yesno, 'button:nth-child(2)', 'click');
+ Assert.areEqual(
+ undefined, save_flag.event_has_fired,
+ "save event wasn't fired.");
+ }
+}));
+
+// Lock, stock, and two smoking barrels.
+var handle_complete = function(data) {
+ status_node = Y.Node.create(
+ '<p id="complete">Test status: complete</p>');
+ Y.one('body').appendChild(status_node);
+ };
+Y.Test.Runner.on('complete', handle_complete);
+Y.Test.Runner.add(suite);
+
+var yui_console = new Y.Console({
+ newestOnTop: false
+});
+yui_console.render('#log');
+
+Y.on('domready', function() {
+ Y.Test.Runner.run();
+});
+
+});
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2011-03-30 11:36:37 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-04-11 04:28:30 +0000
@@ -6,6 +6,7 @@
__metaclass__ = type
__all__ = [
+ 'BugContributorView',
'BugListingBatchNavigator',
'BugListingPortletInfoView',
'BugListingPortletStatsView',
@@ -43,6 +44,7 @@
]
import cgi
+import simplejson
from collections import defaultdict
from datetime import (
datetime,
@@ -73,6 +75,7 @@
IReference,
IWebServiceClientRequest,
)
+from lazr.restful.marshallers import URLDereferencingMixin
from lazr.uri import URI
from pytz import utc
from simplejson import dumps
@@ -163,6 +166,7 @@
from canonical.launchpad.webapp.breadcrumb import Breadcrumb
from canonical.launchpad.webapp.interfaces import ILaunchBag
from canonical.launchpad.webapp.menu import structured
+from canonical.launchpad.webapp.publisher import UserAttributeCache
from canonical.lazr.interfaces import IObjectPrivacy
from canonical.lazr.utils import smartquote
from lp.answers.interfaces.questiontarget import IQuestionTarget
@@ -1648,6 +1652,31 @@
setUpWidgets(self, IBugTask, IDisplayWidget, names=field_names)
+class BugContributorView(URLDereferencingMixin, UserAttributeCache):
+ """A view used to see if a person is a contributor to a bug target.
+
+ This view is called by Javascript an returns a data structure with the
+ result of the contributor check as well as other details so that a message
+ can be displayed to the user.
+ """
+
+ def __init__(self, context, request):
+ self.context = context
+ self.request = request
+
+ def __call__(self):
+ result = {}
+ person_uri = self.request.form['person']
+ person_resource = self.dereference_url(person_uri)
+ person = removeSecurityProxy(person_resource).context
+ self.request.response.setHeader('Content-type', 'application/json')
+ result['is_contributor'] = person.isBugContributorInTarget(
+ self.user, self.context.pillar)
+ result['person_name'] = person.displayname
+ result['pillar_name'] = self.context.pillar.displayname
+ return simplejson.dumps(result)
+
+
class BugTaskListingView(LaunchpadView):
"""A view designed for displaying bug tasks in lists."""
# Note that this right now is only used in tests and to render
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2011-03-30 11:04:40 +0000
+++ lib/lp/bugs/browser/configure.zcml 2011-04-11 04:28:30 +0000
@@ -537,6 +537,12 @@
permission="launchpad.View"
name="+activity"
template="../templates/bug-activity.pt"/>
+ <browser:page
+ for="lp.bugs.interfaces.bugtask.IBugTask"
+ class="lp.bugs.browser.bugtask.BugContributorView"
+ permission="launchpad.View"
+ name="+check-contributor"
+ attribute="__call__"/>
<browser:url
for="lp.bugs.interfaces.bugactivity.IBugActivity"
path_expression="string:activity"
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2011-04-05 06:13:39 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2011-04-11 04:28:30 +0000
@@ -3,6 +3,7 @@
__metaclass__ = type
+import simplejson
from datetime import datetime
from pytz import UTC
@@ -25,6 +26,7 @@
LaunchpadFunctionalLayer,
)
from lp.bugs.browser.bugtask import (
+ BugContributorView,
BugTaskEditView,
BugTasksAndNominationsView,
)
@@ -764,3 +766,43 @@
contents = view.render()
help_link = find_tag_by_id(contents, 'getting-started-help')
self.assertIs(None, help_link)
+
+
+class TestBugContributorView(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestBugContributorView, self).setUp()
+ login(ADMIN_EMAIL)
+
+ def test_non_contributor(self):
+ bug = self.factory.makeBug()
+ # Create a person who has not contributed
+ person = self.factory.makePerson()
+ request = LaunchpadTestRequest()
+ request.form['person'] = "/api/devel/~%s" % person.name
+ view = BugContributorView(bug.default_bugtask, request)
+ json_result = view()
+ result = simplejson.loads(json_result)
+ self.assertFalse(result['is_contributor'])
+ self.assertEqual(person.displayname, result['person_name'])
+ self.assertEqual(
+ bug.default_bugtask.pillar.displayname, result['pillar_name'])
+
+ def test_contributor(self):
+ product = self.factory.makeProduct()
+ bug = self.factory.makeBug(product=product)
+ bug1 = self.factory.makeBug(product=product)
+ # Create a person who has contributed
+ person = self.factory.makePerson()
+ bug1.default_bugtask.transitionToAssignee(person)
+ request = LaunchpadTestRequest()
+ request.form['person'] = "/api/devel/~%s" % person.name
+ view = BugContributorView(bug.default_bugtask, request)
+ json_result = view()
+ result = simplejson.loads(json_result)
+ self.assertTrue(result['is_contributor'])
+ self.assertEqual(person.displayname, result['person_name'])
+ self.assertEqual(
+ bug.default_bugtask.pillar.displayname, result['pillar_name'])
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2011-03-22 05:56:34 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2011-04-11 04:28:30 +0000
@@ -50,7 +50,7 @@
display_privacy_notification();
});
}
-
+
/*
* Check the page for links related to overlay forms and request the HTML
* for these forms.
@@ -443,7 +443,7 @@
Y.one('body').addClass('feature-flag-bugs-private-notification-enabled');
/* Set the visible flag so that the content moves down. */
Y.one('body').addClass('global-notification-visible');
-
+
if (Y.one('.global-notification').hasClass('hidden')) {
Y.one('.global-notification').addClass('transparent');
Y.one('.global-notification').removeClass('hidden');
@@ -464,12 +464,12 @@
duration: 0.2,
easing: Y.Easing.easeOut
});
-
+
fade_in.run();
body_space.run();
login_space.run();
}
-
+
Y.on('click', function(e) {
hide_privacy_notification(true);
e.halt();
@@ -509,11 +509,11 @@
body_space.on('end', function() {
Y.one('body').removeClass('global-notification-visible');
});
-
+
fade_out.run();
body_space.run();
login_space.run();
-
+
if (highlight_portlet) {
var portlet_colour = new Y.Anim({
node: '.portlet.private',
@@ -521,7 +521,7 @@
color: '#fff',
backgroundColor:'#8d1f1f'
},
- duration: 0.4,
+ duration: 0.4
});
portlet_colour.run();
}
@@ -830,6 +830,58 @@
milestone_choice_edit.render();
}
if (Y.Lang.isValue(assignee_content)) {
+ // A validation callback called by the picker when the user selects
+ // an assignee. We check to see if an assignee is a contributor and if
+ // they are not, the user is asked to confirm their selection.
+ var validate_assignee = function(picker, value, save_fn, cancel_fn) {
+ if (value === null) {
+ return true;
+ }
+ 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();
+ error_handler.showError = function(error_msg) {
+ Y.lp.app.errors.display_error(null, error_msg);
+ };
+ var uri = '+check-contributor';
+ var qs = Y.lp.client.append_qs('', 'person', assignee_uri);
+ var y_config = {
+ method: "POST",
+ headers: {'Accept': 'application/json'},
+ on: {
+ failure: error_handler.getFailureHandler()
+ },
+ sync: true,
+ data: qs
+ };
+ var result = Y.io(uri, y_config);
+
+ var contributor_info = Y.JSON.parse(result.responseText);
+ var is_contributor = contributor_info.is_contributor;
+ var person = contributor_info.person_name;
+ var pillar = contributor_info.pillar_name;
+ var requires_confirmation =
+ result.status === 200 && !is_contributor;
+ if (requires_confirmation) {
+ // Handle assignment to non contributor
+ var yesno_content_template =
+ "<p>{person_name} did not previously have any assigned " +
+ "bugs in {pillar}.</p>" +
+ "<p>Do you really want to assign them to this bug?</p>";
+ var yesno_content = Y.Lang.substitute(
+ yesno_content_template,
+ {person_name: person, pillar: pillar});
+ return Y.lp.app.picker.yesno_save_confirmation(
+ picker, yesno_content, "Assign", "Choose Again",
+ save_fn, cancel_fn);
+ } else {
+ if (Y.Lang.isFunction(save_fn)) {
+ save_fn();
+ }
+ }
+ return result.status === 200;
+ };
+
var step_title =
(conf.assignee_vocabulary == 'ValidAssignee') ?
"Search for people or teams" :
@@ -844,7 +896,8 @@
"remove_button_text": "Remove assignee",
"null_display_value": "Unassigned",
"show_remove_button": conf.user_can_unassign,
- "show_assign_me_button": true});
+ "show_assign_me_button": true,
+ "validate_callback": validate_assignee});
// Ordinary users can select only themselves and their teams.
// Do not show the team selection, if a user is not a member
// of any team,
=== added file 'lib/lp/bugs/windmill/tests/test_bug_inline_assignment.py'
--- lib/lp/bugs/windmill/tests/test_bug_inline_assignment.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/windmill/tests/test_bug_inline_assignment.py 2011-04-11 04:28:30 +0000
@@ -0,0 +1,57 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from lp.bugs.windmill.testing import BugsWindmillLayer
+from lp.testing import WindmillTestCase
+from lp.testing.windmill import lpuser
+from lp.testing.windmill.constants import (
+ FOR_ELEMENT,
+ )
+
+
+class TestInlineAssignment(WindmillTestCase):
+
+ layer = BugsWindmillLayer
+
+ def test_inline_assignment_non_contributer(self):
+ """Test assigning bug to a non contributor displays a confirmation."""
+
+ import transaction
+ # Create a person who has not contributed
+ fred = self.factory.makePerson(name="fred")
+ transaction.commit()
+
+ client, start_url = self.getClientFor(
+ "/firefox/+bug/1", lpuser.SAMPLE_PERSON)
+
+ ASSIGN_BUTTON = (u'//*[@id="affected-software"]//tr//td[5]' +
+ '//button[contains(@class, "yui3-activator-act")]')
+ client.waits.forElement(xpath=ASSIGN_BUTTON, timeout=FOR_ELEMENT)
+ client.click(xpath=ASSIGN_BUTTON)
+
+ VISIBLE_PICKER_OVERLAY = (
+ u'//div[contains(@class, "yui3-picker ") and '
+ 'not(contains(@class, "yui3-picker-hidden"))]')
+
+ client.waits.forElement(
+ xpath=VISIBLE_PICKER_OVERLAY, timeout=FOR_ELEMENT)
+
+ def full_picker_element_xpath(element_path):
+ return VISIBLE_PICKER_OVERLAY + element_path
+
+ client.type(xpath=full_picker_element_xpath(
+ "//input[@class='yui3-picker-search']"), text='fred')
+ client.click(xpath=full_picker_element_xpath(
+ "//div[@class='yui3-picker-search-box']/button"))
+
+ PICKER_RESULT = full_picker_element_xpath(
+ "//ul[@class='yui3-picker-results']/li[1]/span")
+
+ client.waits.forElement(xpath=PICKER_RESULT, timeout=FOR_ELEMENT)
+ client.click(xpath=PICKER_RESULT)
+
+ CONFIRMATION = ("//div[@class='important-notice-balloon']")
+ client.waits.forElement(xpath=CONFIRMATION, timeout=FOR_ELEMENT)
+ self.client.asserts.assertTextIn(
+ xpath=CONFIRMATION,
+ validator="Fred did not previously have any assigned bugs")
