launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06876
[Merge] lp:~wallyworld/launchpad/sharing-add-remove-vertical-960245 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/sharing-add-remove-vertical-960245 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #960245 in Launchpad itself: "remove and add icon are vertical"
https://bugs.launchpad.net/launchpad/+bug/960245
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/sharing-add-remove-vertical-960245/+merge/99242
== Implementation ==
This started out as a quick fix to add a nowrap class to the sharee actions span. But in testing some issues were discovered:
1. XHR tests didn't properly check that permission info was rendered
2. Adding a new sharee to a previously empty table didn't update the table hearer (for example) and so the sharing permission help link was missing. Reloading the page fixed it.
So the above issues were fixed as part of this branch, plus a few other minor test issues were fixed.
== Tests ==
Update existing tests, improve _test_sharee_rendered() helper to check for permission content.
bin/test -vvct test_pillarsharingview -t test_shareetable
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/javascript/sharing/pillarsharingview.js
lib/lp/registry/javascript/sharing/shareetable.js
lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js
lib/lp/registry/javascript/sharing/tests/test_shareetable.js
--
https://code.launchpad.net/~wallyworld/launchpad/sharing-add-remove-vertical-960245/+merge/99242
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/sharing-add-remove-vertical-960245 into lp:launchpad.
=== modified file 'lib/lp/registry/javascript/sharing/pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-03-21 02:24:01 +0000
+++ lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-03-26 01:18:19 +0000
@@ -99,12 +99,12 @@
},
renderUI: function() {
- 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,
+ sharing_permissions:
+ this.get('sharing_permissions_by_value'),
information_types: this.get('information_types_by_value'),
write_enabled: this.get('write_enabled')
});
=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
--- lib/lp/registry/javascript/sharing/shareetable.js 2012-03-21 11:48:23 +0000
+++ lib/lp/registry/javascript/sharing/shareetable.js 2012-03-26 01:18:19 +0000
@@ -49,7 +49,7 @@
},
// The sharing permission choices: all, some, nothing etc.
sharing_permissions: {
- value: []
+ value: {}
},
// The node holding the sharee table.
sharee_table: {
@@ -83,6 +83,8 @@
this.set(
'sharee_row_template', this._sharee_row_template());
this.set(
+ 'sharee_table_empty_row', this._sharee_table_empty_row());
+ this.set(
'sharee_policy_template', this._sharee_policy_template());
this.navigator = this.make_navigator();
var self = this;
@@ -152,7 +154,7 @@
' {{display_name}}',
' <span class="formHelp">{{role}}</span></a>',
'</td>',
- '<td class="action-icons">',
+ '<td class="action-icons nowrap">',
'<span id="remove-{{name}}">',
' <a title="Stop sharing with {{display_name}}"',
' href="#" class="sprite remove"',
@@ -178,6 +180,14 @@
'</tr>'].join(' ');
},
+ _sharee_table_empty_row: function() {
+ return [
+ '<tr><td colspan="5" style="padding-left: 0.25em">',
+ 'This project\'s private information is not shared with ',
+ 'anyone.',
+ '</td></tr>'].join('');
+ },
+
_sharee_policy_template: function() {
return [
'{{#information_types}}',
@@ -196,15 +206,15 @@
var information_types = this.get('information_types');
var sharing_permissions = this.get('sharing_permissions');
var choice_items = [];
- Y.Array.forEach(sharing_permissions, function(permission) {
+ Y.each(sharing_permissions, function(title, value) {
var source_name =
'<strong>{policy_name}:</strong> {permission_name}';
choice_items.push({
- value: permission.value,
- name: permission.title,
+ value: value,
+ name: title,
source_name: Y.Lang.substitute(source_name,
{policy_name: information_types[policy],
- permission_name: permission.title})
+ permission_name: title})
});
});
@@ -267,12 +277,6 @@
_render_sharees: function(sharees) {
var sharee_table = this.get('sharee_table');
- if (sharees.length === 0) {
- sharee_table.one('tr#sharee-table-loading td')
- .setContent("This project's private information is " +
- "not shared with anyone.");
- return;
- }
var partials = {
sharee_access_policies:
this.get('sharee_policy_template'),
@@ -283,6 +287,10 @@
this.get('sharee_table_template'),
{sharees: sharees}, partials);
var table_node = Y.Node.create(html);
+ if (sharees.length === 0) {
+ table_node.one('tbody').appendChild(
+ Y.Node.create(this.get('sharee_table_empty_row')));
+ }
sharee_table.replace(table_node);
this.render_sharing_info(sharees);
this._update_editable_status();
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-03-21 02:24:01 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-03-26 01:18:19 +0000
@@ -15,7 +15,8 @@
window.LP = {
links: {},
cache: {
- context: {self_link: "~pillar" },
+ context: {
+ self_link: "~pillar", display_name: 'Pillar'},
sharee_data: [
{'name': 'fred', 'display_name': 'Fred Bloggs',
'role': '(Maintainer)', web_link: '~fred',
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareetable.js'
--- lib/lp/registry/javascript/sharing/tests/test_shareetable.js 2012-03-20 21:00:26 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareetable.js 2012-03-26 01:18:19 +0000
@@ -22,23 +22,18 @@
{'name': 'john.smith', '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'}
]
}
};
+ this.sharing_permissions = {
+ s1: 'S1',
+ s2: 'S2'
+ };
+ this.information_types = {
+ P1: 'Policy 1',
+ P2: 'Policy 2',
+ P3: 'Policy 3'
+ };
this.fixture = Y.one('#fixture');
var sharee_table = Y.Node.create(
Y.one('#sharee-table-template').getContent());
@@ -48,7 +43,7 @@
tearDown: function () {
if (this.fixture !== null) {
- this.fixture.empty();
+ this.fixture.empty(true);
}
delete this.fixture;
delete window.LP;
@@ -62,10 +57,11 @@
sharee_table: Y.one('#sharee-table'),
anim_duration: 0,
sharees: window.LP.cache.sharee_data,
- sharing_permissions: window.LP.cache.sharing_permissions,
- information_types: window.LP.cache.information_types,
+ sharing_permissions: this.sharing_permissions,
+ information_types: this.information_types,
write_enabled: true
}, overrides);
+ window.LP.cache.sharee_data = config.sharees;
var ns = Y.lp.registry.sharing.shareetable;
return new ns.ShareeTableWidget(config);
},
@@ -106,8 +102,7 @@
Y.Assert.areEqual(
"This project's private information is not shared " +
"with anyone.",
- Y.one('#sharee-table tr#sharee-table-loading td')
- .getContent());
+ Y.one('#sharee-table tr td').getContent());
},
// The given sharee is correctly rendered.
@@ -125,16 +120,20 @@
Y.one('#sharee-table span[id=remove-'
+ sharee.name + '] a'));
// The sharing permissions
- var permission;
- for (permission in sharee.permissions) {
- if (sharee.permissions.hasOwnProperty(permission)) {
- Y.Assert.isNotNull(
- Y.one('#sharee-table td[id=td-permission-'
- + sharee.name + '] ul li '
- + 'span[id='+permission+'-permission-'
- + sharee.name + '] span.value'));
- }
- }
+ var self = this;
+ Y.each(sharee.permissions, function(permission, info_type) {
+ var permission_node =
+ Y.one('#sharee-table td[id=td-permission-'
+ + sharee.name + '] ul li '
+ + 'span[id=' + info_type + '-permission-'
+ + sharee.name + '] span.value');
+ Y.Assert.isNotNull(permission_node);
+ var expected_content =
+ self.information_types[info_type] + ': ' +
+ self.sharing_permissions[permission];
+ Y.Assert.areEqual(
+ expected_content, permission_node.get('text'));
+ });
},
// The sharee table is correctly rendered.
