launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06614
[Merge] lp:~wallyworld/launchpad/delete-pillar-observer-947947 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/delete-pillar-observer-947947 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #947947 in Launchpad itself: "connect delete icon in pillar observer table to back end"
https://bugs.launchpad.net/launchpad/+bug/947947
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/delete-pillar-observer-947947/+merge/96327
== Implementation ==
Implement the back end observer deletion code for removing access grants from observers.
Includes merging in Steve's branch to rename the AccessPolicyType enum and all the associated variable renames.
To help perform the deletion in the AccessPolicyService, add a new method to IAccessPolicyGrantFlatSource:
def findArtifactsByGrantee(grantee, policies):
"""Find the `IAccessArtifact`s for grantee and policies.
This method allows the artifacts for a grantee to be found so they can be deleted.
The javascript pillar sharing widget was updated to provide a confirmation dialog so the user can confirm the removal.
== Tests ==
Add test for the new methods introduced.
Plus update tests to account for the variable renaming.
bin/test -vvct test_accesspolicy
-t test_accesspolicyservice
-t test_observerpicker
-t test_observertable
-t test_pillarsharingview
-t test_pillar_sharing
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/pillar.py
lib/lp/registry/browser/tests/test_pillar_sharing.py
lib/lp/registry/interfaces/accesspolicy.py
lib/lp/registry/interfaces/accesspolicyservice.py
lib/lp/registry/javascript/disclosure/observerpicker.js
lib/lp/registry/javascript/disclosure/observertable.js
lib/lp/registry/javascript/disclosure/pillarsharingview.js
lib/lp/registry/javascript/disclosure/tests/test_observerpicker.js
lib/lp/registry/javascript/disclosure/tests/test_observertable.js
lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.html
lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.js
lib/lp/registry/model/accesspolicy.py
lib/lp/registry/services/accesspolicyservice.py
lib/lp/registry/services/tests/test_accesspolicyservice.py
lib/lp/registry/tests/test_accesspolicy.py
--
https://code.launchpad.net/~wallyworld/launchpad/delete-pillar-observer-947947/+merge/96327
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/delete-pillar-observer-947947 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py 2012-03-05 01:20:59 +0000
+++ lib/lp/registry/browser/pillar.py 2012-03-07 12:04:22 +0000
@@ -240,8 +240,8 @@
return getUtility(IService, 'accesspolicy')
@property
- def access_policies(self):
- return self._getAccessPolicyService().getAccessPolicies(self.context)
+ def information_types(self):
+ return self._getAccessPolicyService().getInformationTypes(self.context)
@property
def sharing_permissions(self):
@@ -279,6 +279,6 @@
if not getFeatureFlag('disclosure.enhanced_sharing.enabled'):
raise Unauthorized("This feature is not yet available.")
cache = IJSONRequestCache(self.request)
- cache.objects['access_policies'] = self.access_policies
+ cache.objects['information_types'] = self.information_types
cache.objects['sharing_permissions'] = self.sharing_permissions
cache.objects['observer_data'] = self.observer_data
=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-03-03 04:28:09 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-03-07 12:04:22 +0000
@@ -81,7 +81,7 @@
with FeatureFixture(FLAG):
view = create_initialized_view(self.pillar, name='+sharing')
cache = IJSONRequestCache(view.request)
- self.assertIsNotNone(cache.objects.get('access_policies'))
+ self.assertIsNotNone(cache.objects.get('information_types'))
self.assertIsNotNone(cache.objects.get('sharing_permissions'))
aps = getUtility(IService, 'accesspolicy')
observers = aps.getPillarObservers(self.pillar)
=== modified file 'lib/lp/registry/interfaces/accesspolicy.py'
--- lib/lp/registry/interfaces/accesspolicy.py 2012-03-07 01:24:33 +0000
+++ lib/lp/registry/interfaces/accesspolicy.py 2012-03-07 12:04:22 +0000
@@ -218,3 +218,10 @@
:param policies: a collection of `IAccesPolicy`s.
"""
+
+ def findArtifactsByGrantee(grantee, policies):
+ """Find the `IAccessArtifact`s for grantee and policies.
+
+ :param grantee: the access artifact grantee.
+ :param policies: a collection of `IAccesPolicy`s.
+ """
=== modified file 'lib/lp/registry/interfaces/accesspolicyservice.py'
--- lib/lp/registry/interfaces/accesspolicyservice.py 2012-03-07 02:04:23 +0000
+++ lib/lp/registry/interfaces/accesspolicyservice.py 2012-03-07 12:04:22 +0000
@@ -39,11 +39,11 @@
# version 'devel'
export_as_webservice_entry(publish_web_link=False, as_of='beta')
- def getAccessPolicies(pillar):
- """Return the allowed access policy types for the given pillar."""
+ def getInformationTypes(pillar):
+ """Return the allowed information types for the given pillar."""
def getSharingPermissions():
- """Return the access policy sharing permissions."""
+ """Return the information sharing permissions."""
@export_read_operation()
@operation_parameters(
@@ -57,22 +57,23 @@
@operation_parameters(
pillar=Reference(IPillar, title=_('Pillar'), required=True),
observer=Reference(IPerson, title=_('Observer'), required=True),
- access_policy_types=List(Choice(vocabulary=InformationType)))
+ information_types=List(Choice(vocabulary=InformationType)))
@operation_for_version('devel')
- def updatePillarObserver(pillar, observer, access_policy_types, user):
- """Ensure observer has the grants for access policies on a pillar."""
+ def updatePillarObserver(pillar, observer, information_types, user):
+ """Ensure observer has the grants for information types on a pillar."""
@export_write_operation()
@operation_parameters(
pillar=Reference(IPillar, title=_('Pillar'), required=True),
observer=Reference(IPerson, title=_('Observer'), required=True),
- access_policy_type=Choice(vocabulary=InformationType, required=False))
+ information_types=List(
+ Choice(vocabulary=InformationType), required=False))
@operation_for_version('devel')
- def deletePillarObserver(pillar, observer, access_policy_type):
+ def deletePillarObserver(pillar, observer, information_types):
"""Remove an observer from a pillar.
:param pillar: the pillar from which to remove access
:param observer: the person or team to remove
- :param access_policy_type: if None, remove all access, otherwise just
- remove the specified access_policy
+ :param information_types: if None, remove all access, otherwise just
+ remove the specified access_policies
"""
=== modified file 'lib/lp/registry/javascript/disclosure/observerpicker.js'
--- lib/lp/registry/javascript/disclosure/observerpicker.js 2012-03-06 20:58:51 +0000
+++ lib/lp/registry/javascript/disclosure/observerpicker.js 2012-03-07 12:04:22 +0000
@@ -18,13 +18,13 @@
ObserverPicker.ATTRS = {
/**
- * The value, in percentage, of the progress bar.
+ * The available information types.
*
- * @attribute access_policies
+ * @attribute information_types
* @type Object
* @default []
*/
- access_policies: {
+ information_types: {
value: []
},
// Override for testing
@@ -37,13 +37,13 @@
Y.extend(ObserverPicker, Y.lazr.picker.Picker, {
initializer: function(config) {
ObserverPicker.superclass.initializer.apply(this, arguments);
- var access_policies = [];
+ var information_types = [];
if (config !== undefined) {
- if (config.access_policies !== undefined) {
- access_policies = config.access_policies;
+ if (config.information_types !== undefined) {
+ information_types = config.information_types;
}
}
- this.set('access_policies', access_policies);
+ this.set('information_types', information_types);
var self = this;
this.subscribe('save', function (e) {
e.preventDefault();
@@ -98,7 +98,7 @@
},
_display_step_two: function(data) {
- var title = Y.Lang.substitute('Select access policy for {name}',
+ var title = Y.Lang.substitute('Select visibility policy for {name}',
{name: data.title});
this.set('steptitle', title);
this.set('progress', 75);
@@ -132,17 +132,17 @@
},
_publish_result: function(data) {
- // Determine the chosen access policy type. data already contains the
+ // Determine the chosen information type. data already contains the
// selected person due to the base picker behaviour.
var contentBox = this.get('contentBox');
- var selected_access_policies = [];
+ var selected_info_types = [];
contentBox.all('input[name=field.visibility]')
.each(function(node) {
if (node.get('checked')) {
- selected_access_policies.push(node.get('value'));
+ selected_info_types.push(node.get('value'));
}
});
- data.access_policies = selected_access_policies;
+ data.information_types = selected_info_types;
// Publish the result with step_nr 0 to indicate we have finished.
this.fire('save', data, 0);
},
@@ -173,7 +173,7 @@
'{{/policies}}',
'</tbody></table></div>'
].join(''), {
- policies: this.get('access_policies')
+ policies: this.get('information_types')
});
return Y.Node.create(html);
},
=== modified file 'lib/lp/registry/javascript/disclosure/observertable.js'
--- lib/lp/registry/javascript/disclosure/observertable.js 2012-03-06 05:22:53 +0000
+++ lib/lp/registry/javascript/disclosure/observertable.js 2012-03-07 12:04:22 +0000
@@ -33,8 +33,8 @@
observers: {
value: []
},
- // The access policy types: public, publicsecurity, userdata etc.
- access_policy_types: {
+ // The information types: public, embargoedsecurity, userdata etc.
+ information_types: {
value: {}
},
// The sharing permission choices: all, some, nothing etc.
@@ -109,7 +109,8 @@
'<td id="remove-{{name}}">',
' <a title="Share nothing with this user"',
' href="#" class="sprite remove"' +
- ' data-self_link="{{self_link}}">',
+ ' data-self_link="{{self_link}}"' +
+ ' data-person_name="{{display_name}}">',
' </a>',
'</td>',
'<td id="td-permission-{{name}}">',
@@ -126,19 +127,19 @@
_observer_policy_template: function() {
return [
- '{{#access_policies}}',
+ '{{#information_types}}',
'<li><span id="{{policy}}-permission-{{observer_name}}">',
' <span class="value"></span>',
' <a class="editicon sprite edit" href="#"> </a>',
'</span></li>',
- '{{/access_policies}}'].join(' ');
+ '{{/information_types}}'].join(' ');
},
// Render the popup widget to pick the sharing permission for an
// access policy.
render_observer_policy: function(
observer, policy, current_value) {
- var access_policy_types = this.get('access_policy_types');
+ var information_types = this.get('information_types');
var sharing_permissions = this.get('sharing_permissions');
var choice_items = [];
Y.Array.forEach(sharing_permissions, function(permission) {
@@ -148,7 +149,7 @@
value: permission.value,
name: permission.title,
source_name: Y.Lang.substitute(source_name,
- {policy_name: access_policy_types[policy],
+ {policy_name: information_types[policy],
permission_name: permission.title})
});
});
@@ -165,7 +166,7 @@
value_location: value_location,
editicon: editicon,
value: current_value,
- title: "Share " + access_policy_types[policy] + " with "
+ title: "Share " + information_types[policy] + " with "
+ observer.display_name,
items: choice_items,
elementToFlash: contentBox,
@@ -215,7 +216,8 @@
e.preventDefault();
var delete_link = e.currentTarget;
var observer_link = delete_link.getAttribute('data-self_link');
- self.fire(REMOVE_OBSERVER, delete_link, observer_link);
+ var person_name = delete_link.getAttribute('data-person_name');
+ self.fire(REMOVE_OBSERVER, delete_link, observer_link, person_name);
}, 'td[id^=remove-] a');
},
@@ -226,12 +228,12 @@
_prepareObserverDisplayData: function(observers) {
Y.Array.forEach(observers, function(observer) {
var observer_policies = observer.permissions;
- var policy_values = [];
+ var info_types = [];
Y.each(observer_policies, function(policy_value, policy) {
- policy_values.push({policy: policy,
+ info_types.push({policy: policy,
observer_name: observer.name});
});
- observer.access_policies = policy_values;
+ observer.information_types = info_types;
});
},
=== modified file 'lib/lp/registry/javascript/disclosure/pillarsharingview.js'
--- lib/lp/registry/javascript/disclosure/pillarsharingview.js 2012-03-06 12:03:09 +0000
+++ lib/lp/registry/javascript/disclosure/pillarsharingview.js 2012-03-07 12:04:22 +0000
@@ -54,7 +54,7 @@
headerContent: Y.Node.create("<h2></h2>").set('text', header),
zIndex: 1000,
visible: false,
- access_policies: LP.cache.access_policies,
+ information_types: LP.cache.information_types,
save: function(result) {
self.save_sharing_selection(result);
}
@@ -68,9 +68,9 @@
destructor: function() { },
renderUI: function() {
- var access_policy_types = {};
- Y.Array.each(LP.cache.access_policies, function(policy) {
- access_policy_types[policy.value] = policy.title;
+ 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 observer_data = LP.cache.observer_data;
@@ -78,7 +78,7 @@
var observer_table = new otns.ObserverTableWidget({
observers: observer_data,
sharing_permissions: sharing_permissions,
- access_policy_types: access_policy_types
+ information_types: information_types
});
this.set('observer_table', observer_table);
observer_table.render();
@@ -95,7 +95,8 @@
var otns = Y.lp.registry.disclosure.observertable;
observer_table.subscribe(
otns.ObserverTableWidget.REMOVE_OBSERVER, function(e) {
- self.perform_remove_observer(e.details[0], e.details[1]);
+ self.confirm_observer_removal(
+ e.details[0], e.details[1], e.details[2]);
});
},
@@ -129,6 +130,33 @@
},
/**
+ * Prompt the user to confirm the removal of the selected observer.
+ *
+ * @method confirm_observer_removal
+ */
+ confirm_observer_removal: function(delete_link, person_uri, person_name) {
+ var confirm_text_template = [
+ '<p class="large-warning" style="padding:2px 2px 0 36px;">',
+ ' You are about to remove sharing of<br>',
+ ' "{pillar}" with {person_name}.<br><br>',
+ ' <strong>Please confirm you really want to do this.</strong>',
+ '</p>'
+ ].join('');
+ var confirm_text = Y.Lang.sub(confirm_text_template,
+ {pillar: LP.cache.context.display_name,
+ person_name: person_name});
+ var self = this;
+ var co = new Y.lp.app.confirmationoverlay.ConfirmationOverlay({
+ submit_fn: function() {
+ self.perform_remove_observer(delete_link, person_uri);
+ },
+ form_content: confirm_text,
+ headerContent: '<h2>Confirm sharing removal</h2>'
+ });
+ co.show();
+ },
+
+ /**
* The server call to remove the specified observer has succeeded.
* Update the model and view.
* @method remove_observer_success
@@ -236,7 +264,7 @@
parameters: {
pillar: pillar_uri,
observer: person_uri,
- access_policy_types: selection_result.access_policies
+ information_types: selection_result.information_types
}
};
this.get('lp_client').named_post(
@@ -250,5 +278,6 @@
}, "0.1", { "requires": [
'node', 'lp.client', 'lp.mustache', 'lazr.picker', 'lp.app.picker',
'lp.mustache', 'lp.registry.disclosure.observerpicker',
- 'lp.registry.disclosure.observertable'] });
+ 'lp.registry.disclosure.observertable', 'lp.app.confirmationoverlay'
+ ]});
=== modified file 'lib/lp/registry/javascript/disclosure/tests/test_observerpicker.js'
--- lib/lp/registry/javascript/disclosure/tests/test_observerpicker.js 2012-03-06 12:03:09 +0000
+++ lib/lp/registry/javascript/disclosure/tests/test_observerpicker.js 2012-03-07 12:04:22 +0000
@@ -18,7 +18,7 @@
"description": "frieda@xxxxxxxxxxx", "api_uri": "~/frieda",
"metadata": "team"}
];
- this.access_policies = [
+ this.information_types = [
{index: '0', value: 'P1', title: 'Policy 1',
description: 'Policy 1 description'},
{index: '1', value: 'P2', title: 'Policy 2',
@@ -46,7 +46,7 @@
_create_picker: function(overrides) {
var config = {
- anim_duratrion: 0,
+ anim_duration: 0,
progressbar: true,
progress: 50,
headerContent: "<h2>Grant access</h2>",
@@ -54,7 +54,7 @@
"with whom to share",
zIndex: 1000,
visible: false,
- access_policies: this.access_policies
+ information_types: this.information_types
};
if (overrides !== undefined) {
config = Y.merge(config, overrides);
@@ -103,15 +103,16 @@
// The step title should be updated according to the selected
// person.
steptitle = cb.one('.contains-steptitle h2').getContent();
- Y.Assert.areEqual('Select access policy for Fred', steptitle);
+ Y.Assert.areEqual(
+ 'Select visibility policy for Fred', steptitle);
// The second step ui should be visible.
var step_two_content = cb.one('.picker-content-two');
Y.Assert.isFalse(step_two_content.hasClass('unseen'));
// The second step ui should contain input buttons for each access
// policy type.
- Y.Array.each(this.access_policies, function(policy) {
+ Y.Array.each(this.information_types, function(info_type) {
var rb = step_two_content.one(
- 'input[value="' + policy.title + '"]');
+ 'input[value="' + info_type.title + '"]');
Y.Assert.isNotNull(rb);
});
// There should be a link back to previous step.
@@ -170,7 +171,7 @@
var select_link = step_two_content.one('a.next');
select_link.simulate('click');
Y.ArrayAssert.itemsAreEqual(
- ['Policy 2'], selected_result.access_policies);
+ ['Policy 2'], selected_result.information_types);
Y.Assert.areEqual('~/fred', selected_result.api_uri);
}
}));
=== modified file 'lib/lp/registry/javascript/disclosure/tests/test_observertable.js'
--- lib/lp/registry/javascript/disclosure/tests/test_observertable.js 2012-03-06 05:22:53 +0000
+++ lib/lp/registry/javascript/disclosure/tests/test_observertable.js 2012-03-07 12:04:22 +0000
@@ -25,7 +25,7 @@
{'value': 's2', 'title': 'S2',
'description': 'Sharing 2'}
];
- this.access_policies = {
+ this.information_types = {
'P1': 'Policy 1',
'P2': 'Policy 2',
'P3': 'Policy 3'
@@ -42,7 +42,7 @@
anim_duration: 0.001,
observers: this.observer_data,
sharing_permissions: this.sharing_permissions,
- access_policy_types: this.access_policies
+ information_types: this.information_types
});
},
@@ -70,7 +70,7 @@
Y.Assert.isNotNull(
Y.one('#observer-table td[id=remove-'
+ observer.name + '] a'));
- // The access policy sharing permissions
+ // The sharing permissions
var permission;
for (permission in observer.permissions) {
if (observer.permissions.hasOwnProperty(permission)) {
@@ -120,7 +120,9 @@
ns.ObserverTableWidget.REMOVE_OBSERVER, function(e) {
var delete_link = e.details[0];
var observer_uri = e.details[1];
+ var person_name = e.details[2];
Y.Assert.areEqual('~fred', observer_uri);
+ Y.Assert.areEqual('Fred Bloggs', person_name);
Y.Assert.areEqual(delete_link_to_click, delete_link);
event_fired = true;
}
=== modified file 'lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.html'
--- lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.html 2012-03-01 05:46:50 +0000
+++ lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.html 2012-03-07 12:04:22 +0000
@@ -33,12 +33,16 @@
<script type="text/javascript"
src="../../../../../../build/js/lp/app/lp.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>
<script type="text/javascript"
src="../../../../../../build/js/lp/app/activator/activator.js"></script>
<script type="text/javascript"
src="../../../../../../build/js/lp/app/choiceedit/choiceedit.js"></script>
<script type="text/javascript"
+ src="../../../../../../build/js/lp/app/formoverlay/formoverlay.js"></script>
+ <script type="text/javascript"
src="../../../../../../build/js/lp/app/overlay/overlay.js"></script>
<script type="text/javascript"
src="../../../../../../build/js/lp/app/picker/picker.js"></script>
=== modified file 'lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.js'
--- lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.js 2012-03-06 12:03:09 +0000
+++ lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.js 2012-03-07 12:04:22 +0000
@@ -31,7 +31,7 @@
{'value': 's2', 'title': 'S2',
'description': 'Sharing 2'}
],
- access_policies: [
+ information_types: [
{index: '0', value: 'P1', title: 'Policy 1',
description: 'Policy 1 description'},
{index: '1', value: 'P2', title: 'Policy 2',
@@ -88,24 +88,65 @@
.hasClass('yui3-observer_picker-hidden'));
},
- // Clicking a delete observer link calls the perform_remove_observer
+ // Clicking a delete observer link calls the confirm_observer_removal
// method with the correct parameters.
test_delete_observer_click: function() {
this.view = this._create_Widget();
this.view.render();
+ var confirmRemove_called = false;
+ this.view.confirm_observer_removal = function(
+ delete_link, person_uri, person_name) {
+ Y.Assert.areEqual('~fred', person_uri);
+ Y.Assert.areEqual('Fred Bloggs', person_name);
+ Y.Assert.areEqual(delete_link_to_click, delete_link);
+ confirmRemove_called = true;
+
+ };
+ var delete_link_to_click =
+ Y.one('#observer-table td[id=remove-fred] a');
+ delete_link_to_click.simulate('click');
+ Y.Assert.isTrue(confirmRemove_called);
+ },
+
+ //Test the behaviour of the removal confirmation dialog.
+ _test_confirm_observer_removal: function(click_ok) {
+ this.view = this._create_Widget();
+ this.view.render();
var performRemove_called = false;
- var self = this;
this.view.perform_remove_observer = function(
delete_link, person_uri) {
Y.Assert.areEqual('~fred', person_uri);
- Y.Assert.areEqual(delete_link_to_click, delete_link);
+ Y.Assert.areEqual(delete_link, delete_link);
performRemove_called = true;
};
- var delete_link_to_click =
+ var delete_link =
Y.one('#observer-table td[id=remove-fred] a');
- delete_link_to_click.simulate('click');
- Y.Assert.isTrue(performRemove_called);
+ this.view.confirm_observer_removal(
+ delete_link, '~fred', 'Fred Bloggs');
+ var co = Y.one('.yui3-overlay.yui3-lp-app-confirmationoverlay');
+ var actions = co.one('.yui3-lazr-formoverlay-actions');
+ var btn_style;
+ if (click_ok) {
+ btn_style = '.ok-btn';
+ } else {
+ btn_style = '.cancel-btn';
+ }
+ var button = actions.one(btn_style);
+ button.simulate('click');
+ Y.Assert.areEqual(click_ok, performRemove_called);
+ Y.Assert.isTrue(
+ co.hasClass('yui3-lp-app-confirmationoverlay-hidden'));
+ },
+
+ //Test the remove confirmation dialog when the user clicks Ok.
+ test_confirm_observer_removal_ok: function() {
+ this._test_confirm_observer_removal(true);
+ },
+
+ //Test the remove confirmation dialog when the user clicks Cancel.
+ test_confirm_observer_removal_cancel: function() {
+ this._test_confirm_observer_removal(false);
},
// The perform_remove_observer method makes the expected XHR calls.
@@ -174,7 +215,7 @@
Y.Assert.areEqual('joe', observer.name);
save_sharing_selection_success_called = true;
};
- // Use the picker to select a new observer and access policy.
+ // Use the picker to select a new observer and information type.
var observer_picker = this.view.get('observer_picker');
var picker_results = [
{"value": "joe", "title": "Joe", "css": "sprite-person",
@@ -203,7 +244,7 @@
expected_url = Y.lp.client.append_qs(
expected_url, 'observer', person_uri);
expected_url = Y.lp.client.append_qs(
- expected_url, 'access_policy_types', ['Policy 2']);
+ expected_url, 'information_types', ['Policy 2']);
Y.Assert.areEqual(expected_url, mockio.last_request.config.data);
mockio.last_request.successJSON({
'resource_type_link': 'entity',
=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py 2012-03-07 01:24:33 +0000
+++ lib/lp/registry/model/accesspolicy.py 2012-03-07 12:04:22 +0000
@@ -330,3 +330,13 @@
ids = [policy.id for policy in policies]
return IStore(cls).find(
Person, Person.id == cls.grantee_id, cls.policy_id.is_in(ids))
+
+ @classmethod
+ def findArtifactsByGrantee(cls, grantee, policies):
+ """See `IAccessPolicyGrantFlatSource`."""
+ ids = [policy.id for policy in policies]
+ return IStore(cls).find(
+ AccessArtifact,
+ AccessArtifact.id == cls.abstract_artifact_id,
+ cls.grantee_id == grantee.id,
+ cls.policy_id.is_in(ids))
=== modified file 'lib/lp/registry/services/accesspolicyservice.py'
--- lib/lp/registry/services/accesspolicyservice.py 2012-03-07 01:24:33 +0000
+++ lib/lp/registry/services/accesspolicyservice.py 2012-03-07 12:04:22 +0000
@@ -19,7 +19,9 @@
SharingPermission,
)
from lp.registry.interfaces.accesspolicy import (
+ IAccessArtifactGrantSource,
IAccessPolicySource,
+ IAccessPolicyGrantFlatSource,
IAccessPolicyGrantSource,
)
from lp.registry.interfaces.accesspolicyservice import IAccessPolicyService
@@ -29,7 +31,7 @@
class AccessPolicyService:
- """Service providing operations for access policies.
+ """Service providing operations for adding and removing pillar observers.
Service is accessed via a url of the form
'/services/accesspolicy?ws.op=...
@@ -42,27 +44,27 @@
"""See `IService`."""
return 'accesspolicy'
- def getAccessPolicies(self, pillar):
+ def getInformationTypes(self, pillar):
"""See `IAccessPolicyService`."""
- allowed_policy_types = [
+ allowed_types = [
InformationType.EMBARGOEDSECURITY,
InformationType.USERDATA]
# Products with current commercial subscriptions are also allowed to
- # have a PROPRIETARY access policy.
+ # have a PROPRIETARY information type.
if (IProduct.providedBy(pillar) and
pillar.has_current_commercial_subscription):
- allowed_policy_types.append(InformationType.PROPRIETARY)
+ allowed_types.append(InformationType.PROPRIETARY)
- policies_data = []
- for x, policy in enumerate(allowed_policy_types):
+ result_data = []
+ for x, policy in enumerate(allowed_types):
item = dict(
index=x,
value=policy.name,
title=policy.title,
description=policy.description
)
- policies_data.append(item)
- return policies_data
+ result_data.append(item)
+ return result_data
def getSharingPermissions(self):
"""See `IAccessPolicyService`."""
@@ -96,31 +98,31 @@
person_data = resource.toDataForJSON()
person_data['permissions'] = {}
person_by_id[policy_grant.grantee.id] = person_data
+ result.append(person_data)
person_data = person_by_id[policy_grant.grantee.id]
person_data['permissions'][policy_grant.policy.type.name] = (
SharingPermission.ALL.name)
- result.append(person_data)
return result
@available_with_permission('launchpad.Edit', 'pillar')
- def updatePillarObserver(self, pillar, observer, access_policy_types,
+ def updatePillarObserver(self, pillar, observer, information_types,
user):
"""See `IAccessPolicyService`."""
# We do not support adding observers to project groups.
assert not IProjectGroup.providedBy(pillar)
- pillar_policy_types = [
- (pillar, access_policy_type)
- for access_policy_type in access_policy_types]
+ pillar_info_types = [
+ (pillar, information_type)
+ for information_type in information_types]
# Create any missing pillar access policies.
policy_source = getUtility(IAccessPolicySource)
- pillar_policies = list(policy_source.find(pillar_policy_types))
+ pillar_policies = list(policy_source.find(pillar_info_types))
existing_policy_types = [
(pillar, pillar_policy.type) for pillar_policy in pillar_policies]
required_policies = (
- set(pillar_policy_types).difference(existing_policy_types))
+ set(pillar_info_types).difference(existing_policy_types))
if len(required_policies) > 0:
pillar_policies.extend(policy_source.create(required_policies))
@@ -154,13 +156,38 @@
resource = EntryResource(observer, request)
person_data = resource.toDataForJSON()
permissions = {}
- for access_policy_type in access_policy_types:
- permissions[access_policy_type.name] = SharingPermission.ALL.name
+ for information_type in information_types:
+ permissions[information_type.name] = SharingPermission.ALL.name
person_data['permissions'] = permissions
return person_data
@available_with_permission('launchpad.Edit', 'pillar')
- def deletePillarObserver(self, pillar, observer, access_policy_type):
+ def deletePillarObserver(self, pillar, observer,
+ information_types=None):
"""See `IAccessPolicyService`."""
- # TODO - implement this
- pass
+
+ policy_source = getUtility(IAccessPolicySource)
+ if information_types is None:
+ # We delete all policy grants for the pillar.
+ pillar_policies = policy_source.findByPillar([pillar])
+ else:
+ # We delete selected policy grants for the pillar.
+ pillar_policy_types = [
+ (pillar, information_type)
+ for information_type in information_types]
+ pillar_policies = list(policy_source.find(pillar_policy_types))
+
+ # First delete any access policy grants.
+ policy_grant_source = getUtility(IAccessPolicyGrantSource)
+ policy_grants = [(policy, observer) for policy in pillar_policies]
+ grants = [
+ (grant.policy, grant.grantee)
+ for grant in policy_grant_source.find(policy_grants)]
+ policy_grant_source.revoke(grants)
+
+ # Second delete any access artifact grants.
+ ap_grant_flat = getUtility(IAccessPolicyGrantFlatSource)
+ to_delete = ap_grant_flat.findArtifactsByGrantee(
+ observer, pillar_policies)
+ accessartifact_grant_source = getUtility(IAccessArtifactGrantSource)
+ accessartifact_grant_source.revokeByArtifact(to_delete)
=== modified file 'lib/lp/registry/services/tests/test_accesspolicyservice.py'
--- lib/lp/registry/services/tests/test_accesspolicyservice.py 2012-03-07 01:24:33 +0000
+++ lib/lp/registry/services/tests/test_accesspolicyservice.py 2012-03-07 12:04:22 +0000
@@ -57,8 +57,8 @@
observer_data['permissions'] = permissions
return observer_data
- def _test_getAccessPolicies(self, pillar, expected_policies):
- policy_data = self.service.getAccessPolicies(pillar)
+ def _test_getInformationTypes(self, pillar, expected_policies):
+ policy_data = self.service.getInformationTypes(pillar)
expected_data = []
for x, policy in enumerate(expected_policies):
item = dict(
@@ -70,31 +70,31 @@
expected_data.append(item)
self.assertContentEqual(expected_data, policy_data)
- def test_getAccessPolicies_product(self):
+ def test_getInformationTypes_product(self):
product = self.factory.makeProduct()
- self._test_getAccessPolicies(
+ self._test_getInformationTypes(
product,
[InformationType.EMBARGOEDSECURITY, InformationType.USERDATA])
- def test_getAccessPolicies_expired_commercial_product(self):
+ def test_getInformationTypes_expired_commercial_product(self):
product = self.factory.makeProduct()
self.factory.makeCommercialSubscription(product, expired=True)
- self._test_getAccessPolicies(
+ self._test_getInformationTypes(
product,
[InformationType.EMBARGOEDSECURITY, InformationType.USERDATA])
- def test_getAccessPolicies_commercial_product(self):
+ def test_getInformationTypes_commercial_product(self):
product = self.factory.makeProduct()
self.factory.makeCommercialSubscription(product)
- self._test_getAccessPolicies(
+ self._test_getInformationTypes(
product,
[InformationType.EMBARGOEDSECURITY,
InformationType.USERDATA,
InformationType.PROPRIETARY])
- def test_getAccessPolicies_distro(self):
+ def test_getInformationTypes_distro(self):
distro = self.factory.makeDistribution()
- self._test_getAccessPolicies(
+ self._test_getInformationTypes(
distro,
[InformationType.EMBARGOEDSECURITY, InformationType.USERDATA])
@@ -164,21 +164,21 @@
policy, grantee=observer, grantor=grantor)
# Now call updatePillarObserver will the grants we want.
- access_policy_types = [
+ information_types = [
InformationType.EMBARGOEDSECURITY,
InformationType.USERDATA]
observer_data = self.service.updatePillarObserver(
- pillar, observer, access_policy_types, grantor)
+ pillar, observer, information_types, grantor)
policies = getUtility(IAccessPolicySource).findByPillar([pillar])
policy_grant_source = getUtility(IAccessPolicyGrantSource)
grants = policy_grant_source.findByPolicy(policies)
- self.assertEqual(grants.count(), len(access_policy_types))
+ self.assertEqual(grants.count(), len(information_types))
for grant in grants:
self.assertEqual(grantor, grant.grantor)
self.assertEqual(observer, grant.grantee)
- self.assertIn(grant.policy.type, access_policy_types)
+ self.assertIn(grant.policy.type, information_types)
expected_observer_data = self._makeObserverData(
- observer, access_policy_types)
+ observer, information_types)
self.assertEqual(expected_observer_data, observer_data)
def test_updateProjectGroupObserver_not_allowed(self):
@@ -209,11 +209,10 @@
# updatePillarObserver raises an Unauthorized exception if the user is
# not permitted to do so.
observer = self.factory.makePerson()
- access_policy_type = InformationType.USERDATA
user = self.factory.makePerson()
self.assertRaises(
Unauthorized, self.service.updatePillarObserver,
- pillar, observer, [access_policy_type], user)
+ pillar, observer, [InformationType.USERDATA], user)
def test_updatePillarObserverAnonymous(self):
# Anonymous users are not allowed.
@@ -227,6 +226,77 @@
login_person(self.factory.makePerson())
self._test_updatePillarObserverUnauthorized(product)
+ def _test_deletePillarObserver(self, pillar, types_to_delete=None):
+ # Make grants for some information types.
+ information_types = [
+ InformationType.EMBARGOEDSECURITY,
+ InformationType.USERDATA]
+ access_policies = []
+ for info_type in information_types:
+ access_policy = self.factory.makeAccessPolicy(
+ pillar=pillar, type=info_type)
+ access_policies.append(access_policy)
+ grantee = self.factory.makePerson()
+ # Make some access policy grants for our observer.
+ for access_policy in access_policies:
+ self.factory.makeAccessPolicyGrant(access_policy, grantee)
+ # Make some artifact grants for our observer.
+ artifact = self.factory.makeAccessArtifact()
+ self.factory.makeAccessArtifactGrant(artifact, grantee)
+ for access_policy in access_policies:
+ self.factory.makeAccessPolicyArtifact(
+ artifact=artifact, policy=access_policy)
+ # Make some access policy grants for another observer.
+ another = self.factory.makePerson()
+ self.factory.makeAccessPolicyGrant(access_policies[0], another)
+ # Delete data for a specific information type.
+ self.service.deletePillarObserver(pillar, grantee, types_to_delete)
+ # Assemble the expected data for the remaining access grants for
+ # grantee.
+ expected_data = []
+ if types_to_delete is not None:
+ expected_information_types = (
+ set(information_types).difference(types_to_delete))
+ remaining_grantee_person_data = self._makeObserverData(
+ grantee, expected_information_types)
+ expected_data.append(remaining_grantee_person_data)
+ # Add the data for the other observer.
+ another_person_data = self._makeObserverData(
+ another, information_types[:1])
+ expected_data.append(another_person_data)
+ self.assertContentEqual(
+ expected_data, self.service.getPillarObservers(pillar))
+
+ def test_deleteProductObserverAll(self):
+ # Users with launchpad.Edit can delete all access for an observer.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ login_person(owner)
+ self._test_deletePillarObserver(product)
+
+ def test_deleteProductObserverPolicies(self):
+ # Users with launchpad.Edit can delete selected policy access for an
+ # observer.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ login_person(owner)
+ self._test_deletePillarObserver(product, [InformationType.USERDATA])
+
+ def test_deleteDistroObserverAll(self):
+ # Users with launchpad.Edit can delete all access for an observer.
+ owner = self.factory.makePerson()
+ distro = self.factory.makeDistribution(owner=owner)
+ login_person(owner)
+ self._test_deletePillarObserver(distro)
+
+ def test_deleteDistroObserverPolicies(self):
+ # Users with launchpad.Edit can delete selected policy access for an
+ # observer.
+ owner = self.factory.makePerson()
+ distro = self.factory.makeDistribution(owner=owner)
+ login_person(owner)
+ self._test_deletePillarObserver(distro, [InformationType.USERDATA])
+
class ApiTestMixin:
"""Common tests for launchpadlib and webservice."""
=== modified file 'lib/lp/registry/tests/test_accesspolicy.py'
--- lib/lp/registry/tests/test_accesspolicy.py 2012-03-07 01:24:33 +0000
+++ lib/lp/registry/tests/test_accesspolicy.py 2012-03-07 12:04:22 +0000
@@ -414,3 +414,19 @@
self.assertContentEqual(
[policy_grant.grantee, artifact_grant.grantee],
apgfs.findGranteesByPolicy([policy]))
+
+ def test_findArtifactsByGrantee(self):
+ # findArtifactsByGrantee() returns the artifacts for grantee for any of
+ # the policies.
+ apgfs = getUtility(IAccessPolicyGrantFlatSource)
+ policy = self.factory.makeAccessPolicy()
+ grantee = self.factory.makePerson()
+ # Artifacts not linked to the policy do not show up.
+ artifact = self.factory.makeAccessArtifact()
+ self.factory.makeAccessArtifactGrant(artifact, grantee)
+ self.assertContentEqual(
+ [], apgfs.findArtifactsByGrantee(grantee, [policy]))
+ # Artifacts linked to the policy do show up.
+ self.factory.makeAccessPolicyArtifact(artifact=artifact, policy=policy)
+ self.assertContentEqual(
+ [artifact], apgfs.findArtifactsByGrantee(grantee, [policy]))
