launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10917
[Merge] lp:~wallyworld/launchpad/edit-sharing-policies2-1036437 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/edit-sharing-policies2-1036437 into lp:launchpad with lp:~wallyworld/launchpad/edit-sharing-policies-1036437 as a prerequisite.
Requested reviews:
Curtis Hovey (sinzui)
Related bugs:
Bug #1036437 in Launchpad itself: "UI for maintaining bug/branch sharing policies"
https://bugs.launchpad.net/launchpad/+bug/1036437
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/edit-sharing-policies2-1036437/+merge/119501
== Implementation ==
This branch follows on from the previous one to render bug and branch sharing policies. It adds the ability to save changes. Once a bug/branch sharing policy has been saved, the legacy policy choice is removed from the popup.
A small driveby change was made to the ChoiceSource widget so that it correctly updated the ui when the clickable_content attribute changes. Previously it was assumed this attribute would not change.
== Tests ==
Add new yui tests to test_pillarsharingview.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/javascript/choiceedit/choiceedit.js
lib/lp/registry/javascript/sharing/pillarsharingview.js
lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.html
lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js
--
https://code.launchpad.net/~wallyworld/launchpad/edit-sharing-policies2-1036437/+merge/119501
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/app/javascript/choiceedit/choiceedit.js'
--- lib/lp/app/javascript/choiceedit/choiceedit.js 2012-08-08 16:42:21 +0000
+++ lib/lp/app/javascript/choiceedit/choiceedit.js 2012-08-14 10:14:26 +0000
@@ -171,10 +171,30 @@
* @preventable _saveData
*/
this.publish(SAVE);
- if (!this.get('clickable_content')) {
- var content = this.get('contentBox');
+ },
+
+ /**
+ * Set up the click handler to activate the popup.
+ *
+ * @method _setupClickableContent
+ * @protected
+ */
+ _setupClickableContent: function() {
+ var content = this.get('contentBox');
+ content.detachAll();
+ var edit_icon = this.get('editicon');
+ if (Y.Lang.isValue(edit_icon)) {
+ edit_icon.detachAll();
+ }
+ var value = this.get('clickable_content');
+ var clickable_element;
+ if (value) {
+ clickable_element = content;
+ } else {
content.addClass('no-click');
- };
+ clickable_element = edit_icon;
+ }
+ clickable_element.on("click", this.onClick, this);
},
/**
@@ -188,19 +208,15 @@
* @protected
*/
_bindUIChoiceSource: function() {
+ this._setupClickableContent();
var that = this;
- var clickable_element;
- if (this.get('clickable_content')) {
- clickable_element = this.get('contentBox');
- } else {
- clickable_element = this.get('editicon');
- }
- clickable_element.on("click", this.onClick, this);
-
+ this.after("clickable_contentChange", function(e) {
+ that._setupClickableContent();
+ });
this.after("valueChange", function(e) {
- this.syncUI();
- if (this.get('flashEnabled')) {
- this._showSucceeded();
+ that.syncUI();
+ if (that.get('flashEnabled')) {
+ that._showSucceeded();
}
});
},
=== modified file 'lib/lp/registry/javascript/sharing/pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-08-14 10:14:26 +0000
+++ lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-08-14 10:14:26 +0000
@@ -37,6 +37,11 @@
sharing_permissions_by_value: {
value: null
+ },
+
+ legacy_sharing_policy_description: {
+ value: "Legacy project sharing policy will " +
+ "continue to be used until a new policy is configured."
}
};
@@ -141,25 +146,9 @@
return null;
}
sharing_policy_portlet.removeClass('hidden');
- var desc_node = Y.one(
- '#' + artifact_type + '-sharing-policy-description');
var current_value
= LP.cache.context[artifact_type + '_sharing_policy'];
- var legacy_description = "Legacy project sharing policy will " +
- "continue to be used until a new policy is configured.";
-
- var desc_text = legacy_description;
- if (current_value !== null) {
- Y.Array.some(LP.cache[artifact_type + '_sharing_policies'],
- function(policy_info) {
- if (policy_info.title === current_value) {
- desc_text = policy_info.description;
- return true;
- }
- return false;
- });
- }
- desc_node.set('text', desc_text);
+ this._update_policy_portlet(artifact_type, current_value);
var contentBox = sharing_policy_portlet.one(
'#' + artifact_type + '-sharing-policy');
var value_location = contentBox.one('.value');
@@ -169,20 +158,15 @@
choice_items.push({
value: 'LEGACY',
name: 'Legacy policy',
- description: legacy_description
+ description: this.get('legacy_sharing_policy_description')
});
}
- Y.each(LP.cache[artifact_type + '_sharing_policies'],
- function(policy) {
- choice_items.push({
- value: policy.title,
- name: policy.title,
- description: policy.description
- });
- });
+ choice_items.push.apply(
+ choice_items, this.getSharingPolicyInformation(artifact_type));
var editable = LP.cache.sharing_write_enabled
&& choice_items.length > 1;
var policy_edit = new Y.ChoiceSource({
+ flashEnabled: false,
clickable_content: editable,
contentBox: contentBox,
value_location: value_location,
@@ -232,13 +216,15 @@
if (this.bug_sharing_policy_widget !== null) {
this.bug_sharing_policy_widget.on('save', function(e) {
var policy = self.bug_sharing_policy_widget.get('value');
- Y.log(policy);
+ self.save_sharing_policy(
+ self.bug_sharing_policy_widget, 'bug', policy);
});
}
if (this.branch_sharing_policy_widget !== null) {
this.branch_sharing_policy_widget.on('save', function(e) {
var policy = self.branch_sharing_policy_widget.get('value');
- Y.log(policy);
+ self.save_sharing_policy(
+ self.branch_sharing_policy_widget, 'branch', policy);
});
}
},
@@ -260,6 +246,19 @@
}
},
+ getSharingPolicyInformation: function(artifact_type) {
+ var info = [];
+ Y.each(LP.cache[artifact_type + '_sharing_policies'],
+ function(policy) {
+ info.push({
+ value: policy.title,
+ name: policy.title,
+ description: policy.description
+ });
+ });
+ return info;
+ },
+
_make_invisible_artifacts_warning: function(information_types) {
return Y.lp.mustache.to_html([
"<div id='sharing-warning'",
@@ -279,6 +278,103 @@
});
},
+ /**
+ * Ensure the sharing policy portlet for the artifact type is up-to-date.
+ * @param artifact_type
+ * @param value
+ * @private
+ */
+ _update_policy_portlet: function(artifact_type, value) {
+ var desc_node = Y.one(
+ '#' + artifact_type + '-sharing-policy-description');
+ var desc_text = this.get('legacy_sharing_policy_description');
+ if (Y.Lang.isValue(value)) {
+ Y.Array.some(LP.cache[artifact_type + '_sharing_policies'],
+ function(policy_info) {
+ if (policy_info.title === value) {
+ desc_text = policy_info.description;
+ return true;
+ }
+ return false;
+ });
+ }
+ desc_node.set('text', desc_text);
+ },
+
+ /**
+ * A new sharing policy has been selected so save it.
+ * @param widget
+ * @param artifact_type
+ * @param value
+ */
+ save_sharing_policy: function(widget, artifact_type, value) {
+ var value_key = artifact_type + '_sharing_policy';
+ var error_handler = new Y.lp.client.ErrorHandler();
+ error_handler.showError = function(error_msg) {
+ var portlet_id = '#' + artifact_type + 'sharing-policy-portlet';
+ Y.lp.app.errors.display_error(
+ Y.one(portlet_id), error_msg);
+ };
+ var self = this;
+ error_handler.handleError = function(ioId, response) {
+ var orig_value = LP.cache.context[value_key];
+ if (!Y.Lang.isValue(orig_value)) {
+ orig_value = 'LEGACY';
+ }
+ widget.set('value', orig_value);
+ widget._showFailed();
+ self._update_policy_portlet(artifact_type, orig_value);
+ return false;
+ };
+ this._update_policy_portlet(artifact_type, value);
+ var pillar_uri = LP.cache.context.self_link;
+ var y_config = {
+ on: {
+ start: function() {
+ widget._uiSetWaiting();
+ },
+ end: function() {
+ widget._uiClearWaiting();
+ },
+ success: function() {
+ self.save_sharing_policy_success(
+ widget, artifact_type, value);
+ },
+ failure: error_handler.getFailureHandler()
+ },
+ parameters: {
+ branch_sharing_policy: value
+ }
+ };
+ var patch_data = {};
+ patch_data[value_key] = value;
+ this.get('lp_client').patch(pillar_uri, patch_data, y_config);
+ },
+
+ /**
+ * The sharing policy save operation has succeeded.
+ * @param widget
+ * @param artifact_type
+ * @param value
+ */
+ save_sharing_policy_success: function(widget, artifact_type, value) {
+ var value_key = artifact_type + '_sharing_policy';
+ LP.cache.context[value_key] = value;
+ widget._showSucceeded();
+ // Update the popup widget - we may need to hide the edit link.
+ var choice_items = this.getSharingPolicyInformation(artifact_type);
+ widget.set('items', choice_items);
+ if (choice_items.length <= 1) {
+ var sharing_policy_portlet = Y.one(
+ '#' + artifact_type + '-sharing-policy-portlet');
+ var contentBox = sharing_policy_portlet.one(
+ '#' + artifact_type + '-sharing-policy');
+ var editicon = contentBox.one('a.editicon');
+ editicon.addClass('hidden');
+ widget.set('clickable_content', false);
+ }
+ },
+
// A common error handler for XHR operations.
_error_handler: function(person_uri) {
var grantee_data = LP.cache.grantee_data;
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.html'
--- lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.html 2012-08-14 10:14:26 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.html 2012-08-14 10:14:26 +0000
@@ -32,6 +32,8 @@
<script type="text/javascript"
src="../../../../../../build/js/lp/app/lp.js"></script>
<script type="text/javascript"
+ src="../../../../../../build/js/lp/app/errors.js"></script>
+ <script type="text/javascript"
src="../../../../../../build/js/lp/app/confirmationoverlay/confirmationoverlay.js"></script>
<script type="text/javascript"
src="../../../../../../build/js/lp/app/mustache.js"></script>
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-08-14 10:14:26 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-08-14 10:14:26 +0000
@@ -15,7 +15,8 @@
cache: {
context: {
self_link: '~pillar', display_name: 'Pillar',
- bug_sharing_policy: 'Bug Policy 1'},
+ bug_sharing_policy: 'Bug Policy 1',
+ branch_sharing_policy: null},
grantee_data: [
{'name': 'fred', 'display_name': 'Fred Bloggs',
'role': '(Maintainer)', web_link: '~fred',
@@ -58,18 +59,26 @@
sharing_write_enabled: true
}
};
+ this.mockio = new Y.lp.testing.mockio.MockIo();
},
tearDown: function () {
Y.one('#fixture').empty(true);
delete window.LP;
+ delete this.mockio;
},
_create_Widget: function(cfg) {
+ var lp_client = new Y.lp.client.Launchpad({
+ io_provider: this.mockio
+ });
var config = Y.merge(cfg, {
+ lp_client: lp_client,
+ anim_duration: 0,
header: "Grant access",
steptitle: "Select user",
- vocabulary: "SharingVocab"
+ vocabulary: "SharingVocab",
+ legacy_sharing_policy_description: "Legacy"
});
var ns = Y.lp.registry.sharing.pillarsharingview;
return new ns.PillarSharingView(config);
@@ -240,13 +249,7 @@
// The perform_remove_grantee method makes the expected XHR calls.
test_perform_remove_grantee: function() {
- var mockio = new Y.lp.testing.mockio.MockIo();
- var lp_client = new Y.lp.client.Launchpad({
- io_provider: mockio
- });
- this.view = this._create_Widget({
- lp_client: lp_client
- });
+ this.view = this._create_Widget();
this.view.render();
var remove_grantee_success_called = false;
var self = this;
@@ -259,12 +262,12 @@
this.view.perform_remove_grantee(delete_link, '~fred');
Y.Assert.areEqual(
'/api/devel/+services/sharing',
- mockio.last_request.url);
+ this.mockio.last_request.url);
Y.Assert.areEqual(
'ws.op=deletePillarGrantee&pillar=~pillar' +
'&grantee=~fred',
- mockio.last_request.config.data);
- mockio.last_request.successJSON(['Invisible']);
+ this.mockio.last_request.config.data);
+ this.mockio.last_request.successJSON(['Invisible']);
Y.Assert.isTrue(remove_grantee_success_called);
Y.ArrayAssert.itemsAreEqual(
['Invisible'], LP.cache.invisible_information_types);
@@ -288,13 +291,7 @@
// XHR calls display errors correctly.
_assert_error_displayed_on_failure: function(invoke_operation) {
- var mockio = new Y.lp.testing.mockio.MockIo();
- var lp_client = new Y.lp.client.Launchpad({
- io_provider: mockio
- });
- this.view = this._create_Widget({
- lp_client: lp_client
- });
+ this.view = this._create_Widget();
this.view.render();
var display_error_called = false;
var grantee_table = this.view.get('grantee_table');
@@ -306,7 +303,7 @@
display_error_called = true;
};
invoke_operation(this.view);
- mockio.last_request.respond({
+ this.mockio.last_request.respond({
status: 500,
statusText: 'An error occurred'
});
@@ -325,14 +322,7 @@
// When a grantee is added, the expected XHR calls are made.
test_perform_add_grantee: function() {
- var mockio = new Y.lp.testing.mockio.MockIo();
- var lp_client = new Y.lp.client.Launchpad({
- io_provider: mockio
- });
- this.view = this._create_Widget({
- lp_client: lp_client,
- anim_duration: 0
- });
+ this.view = this._create_Widget();
this.view.render();
var save_sharing_selection_success_called = false;
this.view.save_sharing_selection_success = function(grantee) {
@@ -366,7 +356,7 @@
// Selection made using the picker, now check the results.
Y.Assert.areEqual(
'/api/devel/+services/sharing',
- mockio.last_request.url);
+ this.mockio.last_request.url);
var person_uri = Y.lp.client.normalize_uri('~/joe');
person_uri = Y.lp.client.get_absolute_uri(person_uri);
var expected_url;
@@ -382,8 +372,9 @@
expected_url, 'permissions', 'Policy 2,All');
expected_url = Y.lp.client.append_qs(
expected_url, 'permissions', 'Policy 3,Nothing');
- Y.Assert.areEqual(expected_url, mockio.last_request.config.data);
- mockio.last_request.successJSON({
+ Y.Assert.areEqual(
+ expected_url, this.mockio.last_request.config.data);
+ this.mockio.last_request.successJSON({
grantee_entry: {
'name': 'joe',
'self_link': '~joe'},
@@ -395,14 +386,7 @@
// When a permission is updated, the expected XHR calls are made.
test_perform_update_permission: function() {
- var mockio = new Y.lp.testing.mockio.MockIo();
- var lp_client = new Y.lp.client.Launchpad({
- io_provider: mockio
- });
- this.view = this._create_Widget({
- lp_client: lp_client,
- anim_duration: 0
- });
+ this.view = this._create_Widget();
this.view.render();
var save_sharing_selection_success_called = false;
this.view.save_sharing_selection_success = function(grantee) {
@@ -420,7 +404,7 @@
// Selection made, now check the results.
Y.Assert.areEqual(
'/api/devel/+services/sharing',
- mockio.last_request.url);
+ this.mockio.last_request.url);
var person_uri = Y.lp.client.normalize_uri('~fred');
person_uri = Y.lp.client.get_absolute_uri(person_uri);
var expected_url;
@@ -432,8 +416,9 @@
expected_url, 'grantee', person_uri);
expected_url = Y.lp.client.append_qs(
expected_url, 'permissions', 'Policy 1,Some');
- Y.Assert.areEqual(expected_url, mockio.last_request.config.data);
- mockio.last_request.successJSON({
+ Y.Assert.areEqual(
+ expected_url, this.mockio.last_request.config.data);
+ this.mockio.last_request.successJSON({
grantee_entry: {
'name': 'fred',
'self_link': '~fred'},
@@ -477,14 +462,7 @@
// If the XHR result of a sharePillarInformation call is null, the user
// is to be deleted.
test_save_sharing_selection_null_result: function() {
- var mockio = new Y.lp.testing.mockio.MockIo();
- var lp_client = new Y.lp.client.Launchpad({
- io_provider: mockio
- });
- this.view = this._create_Widget({
- lp_client: lp_client,
- anim_duration: 0
- });
+ this.view = this._create_Widget();
this.view.render();
var remove_grantee_success_called = false;
this.view.remove_grantee_success = function(grantee_uri) {
@@ -492,7 +470,7 @@
remove_grantee_success_called = true;
};
this.view.save_sharing_selection("~fred", ["P1,All"]);
- mockio.last_request.successJSON({
+ this.mockio.last_request.successJSON({
invisible_information_types: [],
grantee_entry: null});
Y.Assert.isTrue(remove_grantee_success_called);
@@ -549,13 +527,9 @@
var portlet = Y.one('#branch-sharing-policy-portlet');
Y.Assert.isFalse(portlet.hasClass('hidden'));
var desc_node = Y.one('#branch-sharing-policy-description');
- Y.Assert.areEqual(
- 'Legacy project sharing policy will continue to be ' +
- 'used until a new policy is configured.',
- desc_node.get('text').trim());
+ Y.Assert.areEqual('Legacy', desc_node.get('text'));
var value_node = Y.one('#branch-sharing-policy .value');
- Y.Assert.areEqual(
- 'Legacy policy', value_node.get('text').trim());
+ Y.Assert.areEqual('Legacy policy', value_node.get('text'));
},
// A pillar's sharing policy is correctly rendered.
@@ -591,10 +565,149 @@
var branch_edit_link = Y.one('#branch-sharing-policy .edit');
Y.Assert.isTrue(branch_edit_link.hasClass('hidden'));
+ },
+
+ // A save operation makes the expected XHR call.
+ _assert_sharing_policy_save: function(artifact_type, title) {
+ Y.one('#' + artifact_type + '-sharing-policy a.edit')
+ .simulate('click');
+ var permission_choice = Y.one(
+ 'a[href=#' + title + ' Policy 1]');
+ permission_choice.simulate('click');
+ Y.Assert.areEqual(
+ '/api/devel/~pillar',
+ this.mockio.last_request.url);
+ Y.Assert.areEqual(
+ '{"' + artifact_type + '_sharing_policy":"' +
+ title + ' Policy 1"}',
+ this.mockio.last_request.config.data);
+ },
+
+ // Bug sharing policy is saved.
+ test_bug_sharing_policy_save: function() {
+ window.LP.cache.context.bug_sharing_policy = null;
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save('bug', 'Bug');
+ },
+
+ // Bug sharing policy is saved and the client is updated.
+ _assert_bug_sharing_policy_save_success: function() {
+ window.LP.cache.context.bug_sharing_policy = null;
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save('bug', 'Bug');
+ var data = {
+ resource_type_link: 'product',
+ self_link: 'api/devel/~pillar',
+ bug_sharing_policy: 'Bug Policy 1'};
+ this.mockio.last_request.successJSON(data);
+ // Check the cached context.
+ Y.Assert.areEqual(
+ window.LP.cache.context.bug_sharing_policy,
+ 'Bug Policy 1');
+ // Check the portlet.
+ Y.Assert.areEqual(
+ 'Bug Policy 1',
+ Y.one('#bug-sharing-policy .value').get('text'));
+ Y.Assert.areEqual(
+ 'Bug Policy 1 description',
+ Y.one('#bug-sharing-policy-description').get('text'));
+ },
+
+ test_bug_sharing_policy_save_success: function() {
+ this._assert_bug_sharing_policy_save_success();
+ },
+
+ // There is more than one policy choice available for selection so
+ // the edit link is visible.
+ test_bug_policy_edit_link_visible_after_save: function() {
+ this._assert_bug_sharing_policy_save_success();
+ var permission_choice = Y.one('#bug-sharing-policy a.edit');
+ Y.Assert.isFalse(permission_choice.hasClass('hidden'));
+ },
+
+ // When a failure occurs, the client retains the existing data.
+ test_bug_sharing_policy_save_failure: function() {
+ window.LP.cache.context.bug_sharing_policy = null;
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save('bug', 'Bug');
+ this.mockio.failure();
+ // Check the cached context.
+ Y.Assert.isNull(window.LP.cache.context.bug_sharing_policy);
+ // Check the portlet.
+ Y.Assert.areEqual(
+ 'Legacy policy',
+ Y.one('#bug-sharing-policy .value').get('text'));
+ Y.Assert.areEqual(
+ 'Legacy',
+ Y.one('#bug-sharing-policy-description').get('text'));
+ },
+
+ // Branch sharing policy is saved.
+ test_branch_sharing_policy_save: function() {
+ window.LP.cache.context.bug_sharing_policy = null;
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save('branch', 'Branch');
+ },
+
+ // Branch sharing policy is saved and the client is updated.
+ _assert_branch_sharing_policy_save_success: function() {
+ window.LP.cache.context.bug_sharing_policy = null;
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save('branch', 'Branch');
+ var data = {
+ resource_type_link: 'product',
+ self_link: 'api/devel/~pillar',
+ branch_sharing_policy: 'Branch Policy 1'};
+ this.mockio.last_request.successJSON(data);
+ // Check the cached context.
+ Y.Assert.areEqual(
+ window.LP.cache.context.branch_sharing_policy,
+ 'Branch Policy 1');
+ // Check the portlet.
+ Y.Assert.areEqual(
+ 'Branch Policy 1',
+ Y.one('#branch-sharing-policy .value').get('text'));
+ Y.Assert.areEqual(
+ 'Branch Policy 1 description',
+ Y.one('#branch-sharing-policy-description').get('text'));
+ },
+
+ test_branch_sharing_policy_save_success: function() {
+ this._assert_branch_sharing_policy_save_success();
+ },
+
+ // There is more than one policy choice available for selection so
+ // the edit link is visible.
+ test_branch_policy_edit_link_visible_after_save: function() {
+ this._assert_branch_sharing_policy_save_success();
+ var permission_choice = Y.one('#branch-sharing-policy a.edit');
+ Y.Assert.isTrue(permission_choice.hasClass('hidden'));
+ },
+
+ // When a failure occurs, the client retains the existing data.
+ test_branch_sharing_policy_save_failure: function() {
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save('branch', 'Branch');
+ this.mockio.failure();
+ // Check the cached context.
+ Y.Assert.isNull(window.LP.cache.context.branch_sharing_policy);
+ // Check the portlet.
+ Y.Assert.areEqual(
+ 'Legacy policy',
+ Y.one('#branch-sharing-policy .value').get('text'));
+ Y.Assert.areEqual(
+ 'Legacy',
+ Y.one('#branch-sharing-policy-description').get('text'));
}
})));
}, '0.1', {'requires': ['test', 'console', 'event', 'node-event-simulate',
'lp.testing.mockio', 'lp.registry.sharing.granteepicker',
- 'lp.registry.sharing.granteetable',
+ 'lp.registry.sharing.granteetable', 'lp.app.errors',
'lp.registry.sharing.pillarsharingview']});
Follow ups
