launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06635
[Merge] lp:~wallyworld/launchpad/update-sharing-button-948911 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/update-sharing-button-948911 into lp:launchpad with lp:~wallyworld/launchpad/fix-sharing-terminology-948083 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #948911 in Launchpad itself: "Add the green ‘add’ additional information type button to the sharing observer table"
https://bugs.launchpad.net/launchpad/+bug/948911
Bug #949005 in Launchpad itself: "sharing policy tags wrap"
https://bugs.launchpad.net/launchpad/+bug/949005
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/update-sharing-button-948911/+merge/96574
Add functionality to allow existing sharing permissions for a user to be edited.
Plus:
- modified css for the sharing tags so they do not wrap to fix bug 949005
- moved +Share link from side portlet to above table
- modified css for "sprite" to add in a few pixels of bottom padding
The last change above was to address the existing issue whereby the sprite images would have their last few pixels sliced off the bottom. The change was done to modifiers.css and fixes a lot of existing places where the sprites are rendered.
A major structural change was introduced to the widgets, implementing a more correct MVC approach provising better separation of concerns and being done with future scenarios in mind. When the results of performing the sharing permissions editing come in, the success handler simply updates the view model in the json cache. Then syncUI is called (the YUI lifecycle method) to cause the widgets' UIs to be updated. Previously add_sharee and delete_shared were called directly on the sharee table widget. However, this did not suit what is needed for this current work and the new implementation also much better handles scenarios where multiple model changes come in the one transaction.
The changes above also fix the problem whereby the +Share link is used to select someone already in the table, causing duplicate table entries. Now, if this happens, the table row is correctly updated with the new sharing data.
The sharee picker was extended to allow a specific screen to be requested when it is displayed. This allowed the add sharee and update sharee functionality to reuse the same UI elements. Also, the sharee picker has the Submit link disabled if no information types are selected, thereby disallowing an accidental deletion of a sharee if they are submitted to the server with nothing selected.
When this branch lands, the sharing view should be ready for more widespread exposure.
== Demo and QA ==
The sharing view now looks like:
http://people.canonical.com/~ianb/sharing-view.png
== Tests ==
YUI tests were updated to test the new functionality.
bin/test -vvct test_pillarsharingview -t test_shareetable -t test_shareepicker
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical/launchpad/icing/css/modifiers.css
lib/canonical/launchpad/icing/css/components/sharing.css
lib/lp/registry/javascript/sharing/pillarsharingview.js
lib/lp/registry/javascript/sharing/shareepicker.js
lib/lp/registry/javascript/sharing/shareetable.js
lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js
lib/lp/registry/javascript/sharing/tests/test_shareepicker.js
lib/lp/registry/javascript/sharing/tests/test_shareetable.js
lib/lp/registry/templates/pillar-sharing.pt
--
https://code.launchpad.net/~wallyworld/launchpad/update-sharing-button-948911/+merge/96574
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/update-sharing-button-948911 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/css/components/sharing.css'
--- lib/canonical/launchpad/icing/css/components/sharing.css 2012-03-08 03:12:23 +0000
+++ lib/canonical/launchpad/icing/css/components/sharing.css 2012-03-09 05:07:23 +0000
@@ -1,5 +1,5 @@
table.sharing td {
- padding: 0;
+ padding: 2px 0 2px 0;
margin: 0;
}
table.sharing ul {
=== modified file 'lib/canonical/launchpad/icing/css/modifiers.css'
--- lib/canonical/launchpad/icing/css/modifiers.css 2012-03-02 16:18:36 +0000
+++ lib/canonical/launchpad/icing/css/modifiers.css 2012-03-09 05:07:23 +0000
@@ -196,7 +196,7 @@
Sprites
*/
.sprite {
- padding: 0px 0 0px 18px;
+ padding: 0 0 4px 18px;
line-height: 18px;
}
=== modified file 'lib/lp/registry/javascript/sharing/pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-03-08 04:26:43 +0000
+++ lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-03-09 05:07:23 +0000
@@ -25,12 +25,22 @@
sharee_table: {
value: null
+ },
+
+ information_types_by_value: {
+ value: null
}
};
Y.extend(PillarSharingView, Y.Widget, {
initializer: function(config) {
+ var information_types_by_value = {};
+ Y.Array.each(LP.cache.information_types, function(info_type) {
+ information_types_by_value[info_type.value] = info_type.title;
+ });
+ this.set('information_types_by_value', information_types_by_value);
+
var vocab = 'ValidPillarOwner';
var header = 'Grant access to project artifacts.';
if (Y.Lang.isValue(config)) {
@@ -65,20 +75,14 @@
this.set('sharee_picker', picker);
},
- destructor: function() { },
-
renderUI: function() {
- var information_types = {};
- Y.Array.each(LP.cache.information_types, function(info_type) {
- information_types[info_type.value] = info_type.title;
- });
var sharing_permissions = LP.cache.sharing_permissions;
var sharee_data = LP.cache.sharee_data;
var otns = Y.lp.registry.sharing.shareetable;
var sharee_table = new otns.ShareeTableWidget({
sharees: sharee_data,
sharing_permissions: sharing_permissions,
- information_types: information_types
+ information_types: this.get('information_types_by_value')
});
this.set('sharee_table', sharee_table);
sharee_table.render();
@@ -98,12 +102,18 @@
self.confirm_sharee_removal(
e.details[0], e.details[1], e.details[2]);
});
+ sharee_table.subscribe(
+ otns.ShareeTableWidget.UPDATE_SHAREE, function(e) {
+ self.update_sharee_interaction(
+ e.details[0], e.details[1], e.details[2]);
+ });
},
syncUI: function() {
+ var sharee_table = this.get('sharee_table');
+ sharee_table.syncUI();
},
-
/**
* Show a spinner next to the delete icon.
*
@@ -162,12 +172,12 @@
* @param person_uri
*/
remove_sharee_success: function(person_uri) {
- var sharee_table = this.get('sharee_table');
var sharee_data = LP.cache.sharee_data;
+ var self = this;
Y.Array.some(sharee_data, function(sharee, index) {
if (sharee.self_link === person_uri) {
sharee_data.splice(index, 1);
- sharee_table.delete_sharee(sharee);
+ self.syncUI();
return true;
}
});
@@ -230,13 +240,23 @@
* The server call to add the specified sharee has succeeded.
* Update the model and view.
* @method save_sharing_selection_success
- * @param sharee
+ * @param updated_sharee
*/
- save_sharing_selection_success: function(sharee) {
- var sharee_table = this.get('sharee_table');
+ save_sharing_selection_success: function(updated_sharee) {
var sharee_data = LP.cache.sharee_data;
- sharee_data.splice(0, 0, sharee);
- sharee_table.add_sharee(sharee);
+ var sharee_replaced = false;
+ Y.Array.some(sharee_data, function(sharee, index) {
+ if (updated_sharee.name === sharee.name) {
+ sharee_replaced = true;
+ sharee_data.splice(index, 1, updated_sharee);
+ return true;
+ }
+ return false;
+ });
+ if (!sharee_replaced) {
+ sharee_data.splice(0, 0, updated_sharee);
+ }
+ this.syncUI();
},
/**
@@ -268,6 +288,43 @@
};
this.get('lp_client').named_post(
'/+services/sharing', 'sharePillarInformation', y_config);
+ },
+
+ /**
+ * The user has clicked the (+) icon for a sharee. We display the sharing
+ * picker to allow the sharing permissions to be updated.
+ * @param update_link
+ * @param person_uri
+ * @param person_name
+ */
+ update_sharee_interaction: function(update_link, person_uri, person_name) {
+ var information_types_by_value =
+ this.get('information_types_by_value');
+ var sharee_data = LP.cache.sharee_data;
+ var selected_permissions = [];
+ Y.Array.some(sharee_data, function(sharee) {
+ var full_person_uri = Y.lp.client.normalize_uri(person_uri);
+ full_person_uri = Y.lp.client.get_absolute_uri(full_person_uri);
+ if (sharee.self_link !== full_person_uri) {
+ return false;
+ }
+ Y.each(sharee.permissions, function(permission, info_type_value) {
+ //we only support ALL for now (need a tri-state checkbox)
+ if ('ALL' === permission) {
+ selected_permissions.push(
+ information_types_by_value[info_type_value]);
+ }
+ });
+ return true;
+ });
+ this.get('sharee_picker').show({
+ first_step: 2,
+ sharee: {
+ person_uri: person_uri,
+ person_name: person_name
+ },
+ selected_permissions: selected_permissions
+ });
}
});
=== modified file 'lib/lp/registry/javascript/sharing/shareepicker.js'
--- lib/lp/registry/javascript/sharing/shareepicker.js 2012-03-08 03:12:23 +0000
+++ lib/lp/registry/javascript/sharing/shareepicker.js 2012-03-09 05:07:23 +0000
@@ -116,21 +116,80 @@
].join(' ');
step_two_content = Y.Node.create(step_two_html);
var self = this;
- step_two_content.one('a.prev').on('click', function(e) {
- e.preventDefault();
- self._display_step_one();
- });
+ // Remove the back link if required.
+ if (Y.Lang.isBoolean(data.back_enabled)
+ && !data.back_enabled ) {
+ step_two_content.one('a.prev').remove(true);
+ } else {
+ step_two_content.one('a.prev').on('click', function(e) {
+ e.preventDefault();
+ self._display_step_one();
+ });
+ }
+ // Wire up the next (ie submit) links.
step_two_content.all('.next').on('click', function(e) {
e.preventDefault();
- self.fire('save', data, 2);
+ // Only submit if at least one info type is selected.
+ if (!self._all_info_choices_unticked(step_two_content)) {
+ self.fire('save', data, 2);
+ }
});
step_two_content.one('div.step-links')
.insert(self._make_policy_selector(), 'before');
step_one_content.insert(step_two_content, 'after');
- }
+ step_two_content.all('input[name=field.visibility]')
+ .on('click', function(e) {
+ self._disable_select_if_all_info_choices_unticked(
+ step_two_content);
+ });
+ }
+ // If we have been given values for the information_type data, ensure
+ // the relevant checkboxes are ticked.
+ if (Y.Lang.isArray(data.information_types)) {
+ Y.Array.each(data.information_types, function(info_type) {
+ var cb = step_two_content.one(
+ 'input[name=field.visibility]' +
+ '[value="'+info_type+'"]');
+ if (Y.Lang.isValue(cb)) {
+ cb.set('checked', true);
+ }
+ });
+ }
+ this._disable_select_if_all_info_choices_unticked(step_two_content);
this._fade_in(step_two_content, step_one_content);
},
+ /**
+ * Are all the info type checkboxes unticked?
+ * @param content
+ * @return {Boolean}
+ * @private
+ */
+ _all_info_choices_unticked: function(content) {
+ var all_unticked = true;
+ content.all('input[name=field.visibility]')
+ .each(function(info_node) {
+ all_unticked &= !info_node.get('checked');
+ });
+ return all_unticked;
+ },
+
+ /**
+ * Disable the select links if no info type checkboxes are ticked.
+ * @param content
+ * @private
+ */
+ _disable_select_if_all_info_choices_unticked: function(content) {
+ var disable_links = this._all_info_choices_unticked(content);
+ content.all('.next').each(function(node) {
+ if (disable_links) {
+ node.addClass('invalid-link');
+ } else {
+ node.removeClass('invalid-link');
+ }
+ });
+ },
+
_publish_result: function(data) {
// Determine the chosen information type. data already contains the
// selected person due to the base picker behaviour.
@@ -184,15 +243,6 @@
// bar as the user steps through the picker screens.
},
- _clear: function() {
- var contentBox = this.get('contentBox');
- var first_button = contentBox.one('input[id=field.visibility.0]');
- if (first_button !== null) {
- first_button.set('checked', 'checked');
- }
- this.constructor.superclass._clear.call(this);
- },
-
hide: function() {
this.get('boundingBox').setStyle('display', 'none');
var contentBox = this.get('contentBox');
@@ -203,8 +253,33 @@
this.constructor.superclass.hide.call(this);
},
- show: function() {
- this._display_step_one();
+ /**
+ * Show the picker. We can pass in config which allows us to tell the
+ * picker to show a screen other than the first, and whether to disable
+ * the back link.
+ * @param state_config
+ */
+ show: function(state_config) {
+ var config = {
+ first_step: 1
+ };
+ if (Y.Lang.isValue(state_config)) {
+ config = Y.merge(config, state_config);
+ }
+ switch (config.first_step) {
+ case 2:
+ var data = {
+ title: config.sharee.person_name,
+ api_uri: config.sharee.person_uri,
+ information_types: config.selected_permissions,
+ back_enabled: false
+ };
+ this._display_step_two(data);
+ break;
+ default:
+ this._display_step_one();
+ break;
+ }
this.get('boundingBox').setStyle('display', 'block');
this.constructor.superclass.show.call(this);
}
=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
--- lib/lp/registry/javascript/sharing/shareetable.js 2012-03-08 03:19:25 +0000
+++ lib/lp/registry/javascript/sharing/shareetable.js 2012-03-09 05:07:23 +0000
@@ -13,6 +13,7 @@
var
NAME = "shareeTableWidget",
// Events
+ UPDATE_SHAREE = 'updateSharee',
REMOVE_SHAREE = 'removeSharee';
@@ -31,7 +32,15 @@
},
// The list of sharees to display.
sharees: {
- value: []
+ value: [],
+ // We clone the data passed in so external modifications do not
+ // interfere.
+ setter: function(value) {
+ if (!Y.Lang.isArray(value)) {
+ return value;
+ }
+ return Y.JSON.parse(Y.JSON.stringify(value));
+ }
},
// The information types: public, embargoedsecurity, userdata etc.
information_types: {
@@ -70,11 +79,10 @@
'sharee_row_template', this._sharee_row_template());
this.set(
'sharee_policy_template', this._sharee_policy_template());
+ this.publish(UPDATE_SHAREE);
this.publish(REMOVE_SHAREE);
},
- destructor: function() { },
-
_sharee_table_template: function() {
return [
'<table class="sharing listing" id="sharee-table">',
@@ -101,17 +109,26 @@
_sharee_row_template: function() {
return [
- '<tr id="permission-{{name}}"><td>',
+ '<tr id="permission-{{name}}" data-name="{{name}}"><td>',
' <a href="{{web_link}}" class="sprite person">' +
' {{display_name}}',
' <span class="formHelp">{{role}}</span></a>',
'</td>',
- '<td id="remove-{{name}}">',
+ '<td align="right">',
+ '<span id="update-{{name}}">',
+ ' <a title="Update sharing for {{display_name}}"',
+ ' href="#" class="sprite add"',
+ ' data-self_link="{{self_link}}"',
+ ' data-person_name="{{display_name}}">',
+ ' </a>',
+ '</span>',
+ '<span id="remove-{{name}}">',
' <a title="Stop sharing with {{display_name}}"',
- ' href="#" class="sprite remove"' +
- ' data-self_link="{{self_link}}"' +
+ ' href="#" class="sprite remove"',
+ ' data-self_link="{{self_link}}"',
' data-person_name="{{display_name}}">',
- ' </a>',
+ ' </a>',
+ '</span>',
'</td>',
'<td id="td-permission-{{name}}">',
' <span class="sortkey">1</span>',
@@ -128,7 +145,8 @@
_sharee_policy_template: function() {
return [
'{{#information_types}}',
- '<li><span id="{{policy}}-permission-{{sharee_name}}">',
+ '<li class="nowrap">',
+ '<span id="{{policy}}-permission-{{sharee_name}}">',
' <span class="value"></span>',
' <a class="editicon sprite edit" href="#"> </a>',
'</span></li>',
@@ -218,12 +236,104 @@
var sharee_link = delete_link.getAttribute('data-self_link');
var person_name = delete_link.getAttribute('data-person_name');
self.fire(REMOVE_SHAREE, delete_link, sharee_link, person_name);
- }, 'td[id^=remove-] a');
- },
-
- syncUI: function() { },
-
- // Transform the sharee access policy data from the model into a form
+ }, 'span[id^=remove-] a');
+ this.get('sharee_table').delegate('click', function(e) {
+ e.preventDefault();
+ var update_link = e.currentTarget;
+ var sharee_link = update_link.getAttribute('data-self_link');
+ var person_name = update_link.getAttribute('data-person_name');
+ self.fire(UPDATE_SHAREE, update_link, sharee_link, person_name);
+ }, 'span[id^=update-] a');
+ },
+
+ syncUI: function() {
+ // Examine the widget's data model and add any new sharees and delete
+ // any which have been removed.
+ var existing_sharees = this.get('sharees');
+ var new_sharees = LP.cache.sharee_data;
+ this._prepareShareeDisplayData(new_sharees);
+ var new_or_updated_sharees = [];
+ var deleted_sharees = [];
+ var self = this;
+ Y.Array.each(new_sharees, function(sharee) {
+ var existing_sharee =
+ self._get_sharee_from_model(sharee.name, existing_sharees);
+ if (!Y.Lang.isValue(existing_sharee)) {
+ new_or_updated_sharees.push(sharee);
+ } else {
+ if (!self._permissions_equal(
+ sharee.permissions, existing_sharee.permissions)) {
+ new_or_updated_sharees.push(sharee);
+ }
+ }
+ });
+ Y.Array.each(existing_sharees, function(sharee) {
+ var new_sharee =
+ self._get_sharee_from_model(sharee.name, new_sharees);
+ if (!Y.Lang.isValue(new_sharee)) {
+ deleted_sharees.push(sharee);
+ }
+ });
+ if (new_or_updated_sharees.length > 0) {
+ this.update_sharees(new_or_updated_sharees);
+ }
+ if (deleted_sharees.length > 0) {
+ this.delete_sharees(deleted_sharees);
+ }
+ this.set('sharees', new_sharees);
+ },
+
+ /**
+ * Return true if the permission values in left do not match those in right.
+ * @param left
+ * @param right
+ * @return {Boolean}
+ * @private
+ */
+ _permissions_equal: function(left, right) {
+ var result = true;
+ Y.some(left, function(sharing_value, info_type) {
+ var right_value = right[info_type];
+ if (sharing_value !== right_value) {
+ result = false;
+ return true;
+ }
+ return false;
+ });
+ if (!result) {
+ return false;
+ }
+ Y.some(right, function(sharing_value, info_type) {
+ var _value = left[info_type];
+ if (!Y.Lang.isValue(left[info_type])) {
+ result = false;
+ return true;
+ }
+ return false;
+ });
+ return result;
+ },
+
+ /**
+ * The the named sharee exists in the model, return it.
+ * @param sharee_name
+ * @param model
+ * @return {*}
+ * @private
+ */
+ _get_sharee_from_model: function(sharee_name, model) {
+ var sharee_data = null;
+ Y.Array.some(model, function(sharee) {
+ if (sharee.name === sharee_name) {
+ sharee_data = sharee;
+ return true;
+ }
+ return false;
+ });
+ return sharee_data;
+ },
+
+ // Transform the sharee information type data from the model into a form
// that can be used with the handlebars template.
_prepareShareeDisplayData: function(sharees) {
Y.Array.forEach(sharees, function(sharee) {
@@ -237,58 +347,81 @@
});
},
- // Add a new sharee to the table.
- add_sharee: function(sharee) {
- this._prepareShareeDisplayData([sharee]);
+ // Add or update new sharees in the table.
+ update_sharees: function(sharees) {
+ this._prepareShareeDisplayData(sharees);
+ var update_node_selectors = [];
var partials = {
sharee_access_policies:
this.get('sharee_policy_template')
};
- var row_html = Y.lp.mustache.to_html(
- this.get('sharee_row_template'), sharee, partials);
- var new_table_row = Y.Node.create(row_html);
- var first_row = this.get('sharee_table')
- .one('tbody>:first-child');
- var row_node;
- if (Y.Lang.isValue(first_row)) {
- row_node = first_row.insertBefore(new_table_row, first_row);
- } else {
- row_node = this.get('sharee_table').one('tbody')
- .appendChild(new_table_row);
- }
- this.render_sharee_sharing_info(sharee);
+ var sharee_table = this.get('sharee_table');
+ var self = this;
+ Y.Array.each(sharees, function(sharee) {
+ var row_html = Y.lp.mustache.to_html(
+ self.get('sharee_row_template'), sharee, partials);
+ var new_table_row = Y.Node.create(row_html);
+ var row_node = sharee_table
+ .one('tr[id=permission-' + sharee.name + ']');
+ if (Y.Lang.isValue(row_node)) {
+ row_node.replace(new_table_row);
+ } else {
+ var first_row = sharee_table.one('tbody>:first-child');
+ if (Y.Lang.isValue(first_row)) {
+ first_row.insertBefore(new_table_row, first_row);
+ } else {
+ sharee_table.one('tbody').appendChild(new_table_row);
+ }
+ }
+ update_node_selectors.push(
+ 'tr[id=permission-' + sharee.name + ']');
+ self.render_sharee_sharing_info(sharee);
+ });
var anim_duration = this.get('anim_duration');
var anim = Y.lp.anim.green_flash(
- {node: row_node, duration:anim_duration});
+ {node: sharee_table.all(
+ update_node_selectors.join(',')), duration:anim_duration});
anim.run();
},
- // Delete the specified sharee from the table.
- delete_sharee: function(sharee) {
- var table_row = this.get('sharee_table')
- .one('tr[id=permission-' + sharee.name + ']');
- if (!Y.Lang.isValue(table_row)) {
+ // Delete the specified sharees from the table.
+ delete_sharees: function(sharees) {
+ var deleted_row_selectors = [];
+ var sharee_table = this.get('sharee_table');
+ Y.Array.each(sharees, function(sharee) {
+ var selector = 'tr[id=permission-' + sharee.name + ']';
+ var table_row = sharee_table.one(selector);
+ if (Y.Lang.isValue(table_row)) {
+ deleted_row_selectors.push(selector);
+ }
+ });
+ if (deleted_row_selectors.length === 0) {
return;
}
+ var rows_to_delete = sharee_table.all(deleted_row_selectors.join(','));
+ var delete_rows = function() {
+ rows_to_delete.remove(true);
+ };
var anim_duration = this.get('anim_duration');
if (anim_duration === 0 ) {
- table_row.remove(true);
+ delete_rows();
return;
}
var anim = Y.lp.anim.green_flash(
- {node: table_row, duration:anim_duration});
+ {node: rows_to_delete, duration:anim_duration});
anim.on('end', function() {
- table_row.remove(true);
+ delete_rows();
});
anim.run();
}
});
ShareeTableWidget.NAME = NAME;
+ShareeTableWidget.UPDATE_SHAREE = UPDATE_SHAREE;
ShareeTableWidget.REMOVE_SHAREE = REMOVE_SHAREE;
namespace.ShareeTableWidget = ShareeTableWidget;
}, "0.1", { "requires": [
- 'node', 'event', 'collection', 'lazr.choiceedit',
+ 'node', 'event', 'collection', 'json', 'lazr.choiceedit',
'lp.mustache', 'lp.registry.sharing.shareepicker'] });
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-03-08 04:26:43 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-03-09 05:07:23 +0000
@@ -20,13 +20,14 @@
{'name': 'fred', 'display_name': 'Fred Bloggs',
'role': '(Maintainer)', web_link: '~fred',
'self_link': '~fred',
- 'permissions': {'P1': 's1', 'P2': 's2'}},
+ 'permissions': {'P1': 'ALL', 'P2': 's2'}},
{'name': 'john', 'display_name': 'John Smith',
- 'role': '', web_link: '~smith', 'self_link': '~smith',
- 'permissions': {'P1': 's1', 'P3': 's3'}}
+ 'role': '', web_link: '~john',
+ 'self_link': 'file:///api/devel/~john',
+ 'permissions': {'P1': 'ALL', 'P3': 's3'}}
],
sharing_permissions: [
- {'value': 's1', 'title': 'S1',
+ {'value': 'ALL', 'title': 'All',
'description': 'Sharing 1'},
{'value': 's2', 'title': 'S2',
'description': 'Sharing 2'}
@@ -78,6 +79,47 @@
Y.Assert.isNotNull(Y.one('.yui3-sharee_picker'));
},
+ // Clicking a update sharee sharee link calls
+ // the update_sharee_interaction method with the correct parameters.
+ test_update_sharee_click: function() {
+ this.view = this._create_Widget();
+ this.view.render();
+ var update_sharee_called = false;
+ this.view.update_sharee_interaction = function(
+ update_link, person_uri, person_name) {
+ Y.Assert.areEqual('~fred', person_uri);
+ Y.Assert.areEqual('Fred Bloggs', person_name);
+ Y.Assert.areEqual(update_link_to_click, update_link);
+ update_sharee_called = true;
+
+ };
+ var update_link_to_click =
+ Y.one('#sharee-table span[id=update-fred] a');
+ update_link_to_click.simulate('click');
+ Y.Assert.isTrue(update_sharee_called);
+ },
+
+ // The update_sharee_interaction method shows the correctly
+ // configured sharing picker.
+ test_update_sharee_interaction: function() {
+ this.view = this._create_Widget();
+ this.view.render();
+ var show_picker_called = false;
+ var sharee_picker = this.view.get('sharee_picker');
+ sharee_picker.show = function(config) {
+ Y.Assert.areEqual(2, config.first_step);
+ Y.Assert.areEqual('~john', config.sharee.person_uri);
+ Y.Assert.areEqual('John', config.sharee.person_name);
+ Y.ArrayAssert.itemsAreEqual(
+ ['Policy 1'], config.selected_permissions);
+ show_picker_called = true;
+ };
+ var update_link =
+ Y.one('#sharee-table span[id=update-smith] a');
+ this.view.update_sharee_interaction(update_link, '~john', 'John');
+ Y.Assert.isTrue(show_picker_called);
+ },
+
// Clicking the sharing link opens the sharing picker
test_sharing_link_click: function() {
this.view = this._create_Widget();
@@ -103,7 +145,7 @@
};
var delete_link_to_click =
- Y.one('#sharee-table td[id=remove-fred] a');
+ Y.one('#sharee-table span[id=remove-fred] a');
delete_link_to_click.simulate('click');
Y.Assert.isTrue(confirmRemove_called);
},
@@ -166,7 +208,7 @@
remove_sharee_success_called = true;
};
var delete_link =
- Y.one('#sharee-table td[id=remove-fred] a');
+ Y.one('#sharee-table span[id=remove-fred] a');
this.view.perform_remove_sharee(delete_link, '~fred');
Y.Assert.areEqual(
'/api/devel/+services/sharing',
@@ -179,19 +221,16 @@
Y.Assert.isTrue(remove_sharee_success_called);
},
- // The removeSharee callback updates the model and table.
+ // The removeSharee callback updates the model and syncs the UI.
test_remove_sharee_success: function() {
this.view = this._create_Widget({anim_duration: 0.001});
this.view.render();
- var delete_sharee_called = false;
- var expected_sharee = window.LP.cache.sharee_data[0];
- var sharee_table = this.view.get('sharee_table');
- sharee_table.delete_sharee = function(sharee) {
- Y.Assert.areEqual(expected_sharee, sharee);
- delete_sharee_called = true;
+ var syncUI_called = false;
+ this.view.syncUI = function() {
+ syncUI_called = true;
};
this.view.remove_sharee_success('~fred');
- Y.Assert.isTrue(delete_sharee_called);
+ Y.Assert.isTrue(syncUI_called);
Y.Array.each(window.LP.cache.sharee_data,
function(sharee) {
Y.Assert.areNotEqual('fred', sharee.name);
@@ -210,7 +249,6 @@
});
this.view.render();
var save_sharing_selection_success_called = false;
- var self = this;
this.view.save_sharing_selection_success = function(sharee) {
Y.Assert.areEqual('joe', sharee.name);
save_sharing_selection_success_called = true;
@@ -254,21 +292,19 @@
},
// The save_sharing_selection_success callback updates the model and
- // table.
+ // syncs the UI.
test_save_sharing_selection_success: function() {
this.view = this._create_Widget({anim_duration: 0.001});
this.view.render();
- var add_sharee_called = false;
var new_sharee = {
'name': 'joe'
};
- var sharee_table = this.view.get('sharee_table');
- sharee_table.add_sharee = function(sharee) {
- Y.Assert.areEqual(new_sharee, sharee);
- add_sharee_called = true;
+ var syncUI_called = false;
+ this.view.syncUI = function() {
+ syncUI_called = true;
};
this.view.save_sharing_selection_success(new_sharee);
- Y.Assert.isTrue(add_sharee_called);
+ Y.Assert.isTrue(syncUI_called);
var model_updated = false;
Y.Array.some(window.LP.cache.sharee_data,
function(sharee) {
@@ -276,6 +312,19 @@
return model_updated;
});
Y.Assert.isTrue(model_updated);
+ },
+
+ // Test that syncUI works as expected.
+ test_syncUI: function() {
+ this.view = this._create_Widget();
+ this.view.render();
+ var sharee_table = this.view.get('sharee_table');
+ var table_syncUI_called = false;
+ sharee_table.syncUI = function() {
+ table_syncUI_called = true;
+ };
+ this.view.syncUI();
+ Y.Assert.isTrue(table_syncUI_called);
}
}));
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareepicker.js'
--- lib/lp/registry/javascript/sharing/tests/test_shareepicker.js 2012-03-08 03:12:23 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareepicker.js 2012-03-09 05:07:23 +0000
@@ -122,6 +122,65 @@
Y.Assert.isNotNull(step_two_content.one('button.next'));
},
+ // Test that by default, show() opens the first picker screen.
+ test_default_show: function() {
+ this.picker = this._create_picker();
+ // Select a person to trigger transition to next step.
+ this.picker.set('results', this.vocabulary);
+ this.picker.render();
+ this.picker.show();
+ var cb = this.picker.get('contentBox');
+ var steptitle = cb.one('.contains-steptitle h2').getContent();
+ Y.Assert.areEqual(
+ 'Search for user or exclusive team with whom to share',
+ steptitle);
+ },
+
+ // Test that show() can be used to open the second picker screen with
+ // specified information displayed.
+ test_show_selected_screen: function() {
+ var selected_result;
+ this.picker = this._create_picker(
+ {
+ save: function(result) {
+ selected_result = result;
+ }
+ }
+ );
+
+ // Select a person to trigger transition to next step.
+ this.picker.set('results', this.vocabulary);
+ this.picker.render();
+ this.picker.show({
+ first_step: 2,
+ sharee: {
+ person_uri: '~/fred',
+ person_name: 'Fred'
+ },
+ selected_permissions: ['Policy 1']
+ });
+ var cb = this.picker.get('contentBox');
+ var steptitle = cb.one('.contains-steptitle h2').getContent();
+ Y.Assert.areEqual(
+ 'Select sharing policy for Fred', steptitle);
+ // Selected permission checkboxes should be ticked.
+ cb.all('input[name=field.visibility]')
+ .each(function(node) {
+ if (node.get('checked')) {
+ Y.Assert.areEqual('Policy 1', node.get('value'));
+ } else {
+ Y.Assert.areNotEqual('Policy 1', node.get('value'));
+ }
+ });
+ // Back link should not he shown
+ var back_link = cb.one('a.prev');
+ Y.Assert.isNull(back_link);
+ // When submit is clicked, the correct person uri is used.
+ var select_link = cb.one('a.next');
+ select_link.simulate('click');
+ Y.Assert.areEqual('~/fred', selected_result.api_uri);
+ },
+
// Test that the back link goes back to step one when step two is
// active.
test_second_step_back_link: function() {
@@ -173,6 +232,51 @@
Y.ArrayAssert.itemsAreEqual(
['Policy 2'], selected_result.information_types);
Y.Assert.areEqual('~/fred', selected_result.api_uri);
+ },
+
+ // When no info types are selected, the Submit links are disabled.
+ test_no_selected_info_types: function() {
+ var save_called = false;
+ this.picker = this._create_picker(
+ {
+ save: function(result) {
+ save_called = true;
+ }
+ }
+ );
+ this.picker.render();
+ this.picker.show({
+ first_step: 2,
+ sharee: {
+ person_uri: '~/fred',
+ person_name: 'Fred'
+ }
+ });
+ var cb = this.picker.get('contentBox');
+ // Check when first showing the picker.
+ cb.all('a.next', function(link) {
+ Y.Assert.isTrue(link.hasClass('invalid-link'));
+ });
+ // Check that save is not called even if link is clicked.
+ var select_link = cb.one('a.next');
+ select_link.simulate('click');
+ Y.Assert.isFalse(save_called);
+ // Select one info type and check submit links are enabled.
+ cb.one('input[name=field.visibility]').simulate('click');
+ cb.all('a.next', function(link) {
+ Y.Assert.isFalse(link.hasClass('invalid-link'));
+ });
+ // De-select info type and submit links are disabled again.
+ cb.one('input[name=field.visibility]').simulate('click');
+ cb.all('a.next', function(link) {
+ Y.Assert.isTrue(link.hasClass('invalid-link'));
+ });
+ select_link.simulate('click');
+ Y.Assert.isFalse(save_called);
+ // Once at least one info type is selected, save is called.
+ cb.one('input[name=field.visibility]').simulate('click');
+ select_link.simulate('click');
+ Y.Assert.isTrue(save_called);
}
}));
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareetable.js'
--- lib/lp/registry/javascript/sharing/tests/test_shareetable.js 2012-03-08 03:12:23 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareetable.js 2012-03-09 05:07:23 +0000
@@ -10,39 +10,49 @@
name: 'lp.registry.sharing.shareetable_tests',
setUp: function () {
- this.sharee_data = [
- {'name': 'fred', 'display_name': 'Fred Bloggs',
- 'role': '(Maintainer)', web_link: '~fred',
- 'self_link': '~fred',
- 'permissions': {'P1': 's1', 'P2': 's2'}},
- {'name': 'john', 'display_name': 'John Smith',
- 'role': '', web_link: '~smith', 'self_link': '~smith',
- 'permissions': {'P1': 's1', 'P3': 's3'}}
- ];
- this.sharing_permissions = [
- {'value': 's1', 'title': 'S1',
- 'description': 'Sharing 1'},
- {'value': 's2', 'title': 'S2',
- 'description': 'Sharing 2'}
- ];
- this.information_types = {
- 'P1': 'Policy 1',
- 'P2': 'Policy 2',
- 'P3': 'Policy 3'
+ window.LP = {
+ links: {},
+ cache: {
+ context: {self_link: "~pillar" },
+ sharee_data: [
+ {'name': 'fred', 'display_name': 'Fred Bloggs',
+ 'role': '(Maintainer)', web_link: '~fred',
+ 'self_link': '~fred',
+ 'permissions': {'P1': 's1', 'P2': 's2'}},
+ {'name': 'john', 'display_name': 'John Smith',
+ 'role': '', web_link: '~smith', 'self_link': '~smith',
+ 'permissions': {'P1': 's1', 'P3': 's3'}}
+ ],
+ sharing_permissions: [
+ {'value': 's1', 'title': 'S1',
+ 'description': 'Sharing 1'},
+ {'value': 's2', 'title': 'S2',
+ 'description': 'Sharing 2'}
+ ],
+ information_types: [
+ {index: '0', value: 'P1', title: 'Policy 1',
+ description: 'Policy 1 description'},
+ {index: '1', value: 'P2', title: 'Policy 2',
+ description: 'Policy 2 description'},
+ {index: '2', value: 'P3', title: 'Policy 3',
+ description: 'Policy 3 description'}
+ ]
+ }
};
},
tearDown: function () {
Y.one('#sharee-table').empty(true);
+ delete window.LP;
},
_create_Widget: function() {
var ns = Y.lp.registry.sharing.shareetable;
return new ns.ShareeTableWidget({
anim_duration: 0.001,
- sharees: this.sharee_data,
- sharing_permissions: this.sharing_permissions,
- information_types: this.information_types
+ sharees: window.LP.cache.sharee_data,
+ sharing_permissions: window.LP.cache.sharing_permissions,
+ information_types: window.LP.cache.information_types
});
},
@@ -66,9 +76,13 @@
Y.Assert.isNotNull(
Y.one('#sharee-table tr[id=permission-'
+ sharee.name + ']'));
+ // The update link
+ Y.Assert.isNotNull(
+ Y.one('#sharee-table span[id=update-'
+ + sharee.name + '] a'));
// The delete link
Y.Assert.isNotNull(
- Y.one('#sharee-table td[id=remove-'
+ Y.one('#sharee-table span[id=remove-'
+ sharee.name + '] a'));
// The sharing permissions
var permission;
@@ -93,7 +107,30 @@
});
},
- // The add_sharee call adds the sharee to the table.
+ // When the update link is clicked, the correct event is published.
+ test_sharee_update_click: function() {
+ this.sharee_table = this._create_Widget();
+ this.sharee_table.render();
+ var event_fired = false;
+ var ns = Y.lp.registry.sharing.shareetable;
+ this.sharee_table.subscribe(
+ ns.ShareeTableWidget.UPDATE_SHAREE, function(e) {
+ var update_link = e.details[0];
+ var sharee_uri = e.details[1];
+ var person_name = e.details[2];
+ Y.Assert.areEqual('~fred', sharee_uri);
+ Y.Assert.areEqual('Fred Bloggs', person_name);
+ Y.Assert.areEqual(update_link_to_click, update_link);
+ event_fired = true;
+ }
+ );
+ var update_link_to_click =
+ Y.one('#sharee-table span[id=update-fred] a');
+ update_link_to_click.simulate('click');
+ Y.Assert.isTrue(event_fired);
+ },
+
+ // The update_sharees call adds new sharees to the table.
test_sharee_add: function() {
this.sharee_table = this._create_Widget();
this.sharee_table.render();
@@ -102,7 +139,7 @@
'role': '(Maintainer)', web_link: '~joe',
'self_link': '~joe',
'permissions': {'P1': 's2'}};
- this.sharee_table.add_sharee(new_sharee);
+ this.sharee_table.update_sharees([new_sharee]);
var self = this;
this.wait(function() {
self._test_sharee_rendered(new_sharee);
@@ -110,6 +147,23 @@
);
},
+ // The update_sharees call updates existing sharees in the table.
+ test_sharee_update: function() {
+ this.sharee_table = this._create_Widget();
+ this.sharee_table.render();
+ var updated_sharee = {
+ 'name': 'fred', 'display_name': 'Fred Bloggs',
+ 'role': '(Maintainer)', web_link: '~fred',
+ 'self_link': '~fred',
+ 'permissions': {'P1': 's2', 'P2': 's1'}};
+ this.sharee_table.update_sharees([updated_sharee]);
+ var self = this;
+ this.wait(function() {
+ self._test_sharee_rendered(updated_sharee);
+ }, 60
+ );
+ },
+
// When the delete link is clicked, the correct event is published.
test_sharee_delete_click: function() {
this.sharee_table = this._create_Widget();
@@ -128,22 +182,53 @@
}
);
var delete_link_to_click =
- Y.one('#sharee-table td[id=remove-fred] a');
+ Y.one('#sharee-table span[id=remove-fred] a');
delete_link_to_click.simulate('click');
Y.Assert.isTrue(event_fired);
},
- // The delete_sharee call removes the sharee from the table.
+ // The delete_sharees call removes the sharees from the table.
test_sharee_delete: function() {
this.sharee_table = this._create_Widget();
this.sharee_table.render();
var row_selector = '#sharee-table tr[id=permission-fred]';
Y.Assert.isNotNull(Y.one(row_selector));
- this.sharee_table.delete_sharee(this.sharee_data[0]);
+ this.sharee_table.delete_sharees(
+ [window.LP.cache.sharee_data[0]]);
this.wait(function() {
Y.Assert.isNull(Y.one(row_selector));
}, 60
);
+ },
+
+ // Model changes are rendered correctly when syncUI() is called.
+ test_syncUI: function() {
+ this.sharee_table = this._create_Widget();
+ this.sharee_table.render();
+ // We manipulate the cached model data - delete, add and update
+ var sharee_data = window.LP.cache.sharee_data;
+ // Delete the first record.
+ sharee_data.splice(0, 1);
+ // Insert a new record.
+ var new_sharee = {
+ 'name': 'joe', 'display_name': 'Joe Smith',
+ 'role': '(Maintainer)', web_link: '~joe',
+ 'self_link': '~joe',
+ 'permissions': {'P1': 's2'}};
+ sharee_data.splice(0, 0, new_sharee);
+ // Update a record.
+ sharee_data[1].permissions = {'P1': 's2', 'P2': 's1'};
+ this.sharee_table.syncUI();
+ // Check the results.
+ var self = this;
+ this.wait(function() {
+ Y.Array.each(sharee_data, function(sharee) {
+ self._test_sharee_rendered(sharee);
+ });
+ var deleted_row = '#sharee-table tr[id=permission-fred]';
+ Y.Assert.isNull(Y.one(deleted_row));
+ }, 60
+ );
}
}));
=== modified file 'lib/lp/registry/templates/pillar-sharing.pt'
--- lib/lp/registry/templates/pillar-sharing.pt 2012-03-08 03:12:23 +0000
+++ lib/lp/registry/templates/pillar-sharing.pt 2012-03-09 05:07:23 +0000
@@ -28,7 +28,9 @@
Proprietary, embargoed security, or user-data information is
shared with these users and teams.
</p>
-
+ <div style="padding-bottom: 0.5em">
+ <a id='add-sharee-link' class='sprite add js-action' href="#">Share with someone</a>
+ </div>
<table id="sharee-table" class="disclosure listing">
</table>
</div>
@@ -40,9 +42,6 @@
flux. Once they settle down, the view needs to populate some methods to
set the numbers in this portlet.
</tal:comment>
- <div id="portlet-disclosure-links" class="first portlet">
- <a id='add-sharee-link' class='sprite add js-action' href="#">Share</a>
- </div>
<div id="portlet-disclosure-summary" class="portlet">
<p>
Permavirgin is shared with <a href="#">0</a> users:
