launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06952
[Merge] lp:~wallyworld/launchpad/sharing-details-link-971198 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/sharing-details-link-971198 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #971198 in Launchpad itself: "Navigate to sharing details page from sharee row in sharing information table"
https://bugs.launchpad.net/launchpad/+bug/971198
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/sharing-details-link-971198/+merge/100353
== Implementation ==
Add a new boolean attribute 'shared_items_exist' to the json info for each sharee.
Tweak the mustache template used to render each row to show the link to the details page if the attribute is true.
== Tests ==
Enhance the test_shareetable yui test
Add new tests to TestSharingService
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/javascript/sharing/shareetable.js
lib/lp/registry/javascript/sharing/tests/test_shareetable.js
lib/lp/registry/services/sharingservice.py
lib/lp/registry/services/tests/test_sharingservice.py
--
https://code.launchpad.net/~wallyworld/launchpad/sharing-details-link-971198/+merge/100353
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/sharing-details-link-971198 into lp:launchpad.
=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
--- lib/lp/registry/javascript/sharing/shareetable.js 2012-03-31 11:32:15 +0000
+++ lib/lp/registry/javascript/sharing/shareetable.js 2012-04-02 03:21:19 +0000
@@ -175,7 +175,13 @@
' </ul>',
'</td>',
'<td></td>',
- '<td><span class="formHelp">No items shared</span>',
+ '<td>',
+ '{{#shared_items_exist}}',
+ '<a href="+sharingdetails/{{name}}">View shared items.</a>',
+ '{{/shared_items_exist}}',
+ '{{^shared_items_exist}}',
+ '<span class="formHelp">No items shared.</span>',
+ '{{/shared_items_exist}}',
'</td>',
'</tr>'].join(' ');
},
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareetable.js'
--- lib/lp/registry/javascript/sharing/tests/test_shareetable.js 2012-03-31 11:32:15 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareetable.js 2012-04-02 03:21:19 +0000
@@ -21,6 +21,7 @@
'permissions': {'P1': 's1', 'P2': 's2'}},
{'name': 'john.smith', 'display_name': 'John Smith',
'role': '', web_link: '~smith', 'self_link': '~smith',
+ 'shared_items_exist': true,
'permissions': {'P1': 's1', 'P3': 's3'}}
]
}
@@ -124,9 +125,9 @@
// The given sharee is correctly rendered.
_test_sharee_rendered: function(sharee) {
// The sharee row
- Y.Assert.isNotNull(
- Y.one('#sharee-table tr[id=permission-'
- + sharee.name + ']'));
+ var sharee_row = Y.one('#sharee-table tr[id=permission-'
+ + sharee.name + ']');
+ Y.Assert.isNotNull(sharee_row);
// The update link
Y.Assert.isNotNull(
Y.one('#sharee-table span[id=update-'
@@ -150,6 +151,17 @@
Y.Assert.areEqual(
expected_content, permission_node.get('text'));
});
+ // The shared items link.
+ var shared_items_cell = sharee_row.one('td:nth-child(5)');
+ if (sharee.shared_items_exist) {
+ Y.Assert.isNotNull(
+ shared_items_cell.one(
+ 'a[href=+sharingdetails/' + sharee.name + ']'));
+ } else {
+ Y.Assert.areEqual(
+ 'No items shared.',
+ Y.Lang.trim(shared_items_cell.get('text')));
+ }
},
// The sharee table is correctly rendered.
=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py 2012-03-31 11:32:15 +0000
+++ lib/lp/registry/services/sharingservice.py 2012-04-02 03:21:19 +0000
@@ -124,16 +124,23 @@
result = []
request = get_current_web_service_request()
browser_request = IWebBrowserOriginatingRequest(request)
+ details_enabled = bool((getFeatureFlag(
+ 'disclosure.enhanced_sharing_details.enabled')))
for (grantee, permissions) in grant_permissions:
+ some_things_sharred = False
+ sharee_permissions = {}
+ for (policy, permission) in permissions.iteritems():
+ sharee_permissions[policy.type.name] = permission.name
+ if details_enabled and permission == SharingPermission.SOME:
+ some_things_sharred = True
result.append({
'name': grantee.name,
'meta': 'team' if grantee.is_team else 'person',
'display_name': grantee.displayname,
'self_link': absoluteURL(grantee, request),
'web_link': absoluteURL(grantee, browser_request),
- 'permissions': dict(
- (policy.type.name, permission.name)
- for (policy, permission) in permissions.iteritems())})
+ 'permissions': sharee_permissions,
+ 'shared_items_exist': some_things_sharred})
return result
@available_with_permission('launchpad.Edit', 'pillar')
=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py 2012-03-31 11:32:15 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2012-04-02 03:21:19 +0000
@@ -43,6 +43,7 @@
WRITE_FLAG = {'disclosure.enhanced_sharing.writable': 'true'}
+DETAILS_FLAG = {'disclosure.enhanced_sharing_details.enabled': 'true'}
class TestSharingService(TestCaseWithFactory):
@@ -65,9 +66,13 @@
'permissions': {}}
browser_request = IWebBrowserOriginatingRequest(request)
sharee_data['web_link'] = absoluteURL(sharee, browser_request)
+ shared_items_exist = False
permissions = {}
for (policy, permission) in policy_permissions:
permissions[policy.name] = unicode(permission.name)
+ if permission == SharingPermission.SOME:
+ shared_items_exist = True
+ sharee_data['shared_items_exist'] = shared_items_exist
sharee_data['permissions'] = permissions
return sharee_data
@@ -123,8 +128,27 @@
distro,
[InformationType.EMBARGOEDSECURITY, InformationType.USERDATA])
- def test_jsonShareeData(self):
- # jsonShareeData returns the expected data.
+ def test_jsonShareeData_with_Some(self):
+ # jsonShareeData returns the expected data for a grantee with
+ # permissions which include SOME.
+ product = self.factory.makeProduct()
+ [policy1, policy2] = getUtility(IAccessPolicySource).findByPillar(
+ [product])
+ grantee = self.factory.makePerson()
+ with FeatureFixture(DETAILS_FLAG):
+ sharees = self.service.jsonShareeData(
+ [(grantee, {
+ policy1: SharingPermission.ALL,
+ policy2: SharingPermission.SOME})])
+ expected_data = self._makeShareeData(
+ grantee,
+ [(policy1.type, SharingPermission.ALL),
+ (policy2.type, SharingPermission.SOME)])
+ self.assertContentEqual([expected_data], sharees)
+
+ def test_jsonShareeData_with_Some_without_flag(self):
+ # jsonShareeData returns the expected data for a grantee with
+ # permissions which include SOME and the feature flag not set.
product = self.factory.makeProduct()
[policy1, policy2] = getUtility(IAccessPolicySource).findByPillar(
[product])
@@ -137,6 +161,23 @@
grantee,
[(policy1.type, SharingPermission.ALL),
(policy2.type, SharingPermission.SOME)])
+ expected_data['shared_items_exist'] = False
+ self.assertContentEqual([expected_data], sharees)
+
+ def test_jsonShareeData_without_Some(self):
+ # jsonShareeData returns the expected data for a grantee with only ALL
+ # permissions.
+ product = self.factory.makeProduct()
+ [policy1, policy2] = getUtility(IAccessPolicySource).findByPillar(
+ [product])
+ grantee = self.factory.makePerson()
+ with FeatureFixture(DETAILS_FLAG):
+ sharees = self.service.jsonShareeData(
+ [(grantee, {
+ policy1: SharingPermission.ALL})])
+ expected_data = self._makeShareeData(
+ grantee,
+ [(policy1.type, SharingPermission.ALL)])
self.assertContentEqual([expected_data], sharees)
def _assert_getPillarShareeData(self, pillar):
