launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06601
[Merge] lp:~wallyworld/launchpad/sharing-picker-943433 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/sharing-picker-943433 into lp:launchpad with lp:~wallyworld/launchpad/more-accesspolicy-service-4 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #943433 in Launchpad itself: "I cannot share all kinds of information in a single action"
https://bugs.launchpad.net/launchpad/+bug/943433
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/sharing-picker-943433/+merge/95851
== Implementation ==
The branch delivers 3 main changes:
1. Disclosure picker uses checkboxes instead of radio buttons to allow more than one policy type to be chosen when adding an observer.
2. Ensure the correct policy types are provided as choices - embargoed security and user data for products and distros; additionally proprietary for commercial products
3. addPillarObserver renamed to updatePillarObserver and new semantics - any existing grants not included in the updatePillarObserver parameters are deleted.
To support 3, I added a revoke() method to IAccessPolicyGrantSource
The reason for the semantic change in updatePillarObserver is so that when a link is added to the observer table to allow granted policies to be edited/changed, the user can tick the ones they want and untick the ones they don't want and simply hit save.
Next:
- backend support for delete icon on observer table rows
- add "edit" link for each observer to allow policies to be added/removed
== Tests ==
Add new test_revoke() to TestAccessPolicyGrantSource
Update existing yui tests for sharing view to account for new api
Add new tests to test_accesspolicyservice
bin/test -vvct test_accesspolicy -t test_accesspolicyservice -t test_pillarsharingview -t test_observertable
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/pillar.py
lib/lp/registry/interfaces/accesspolicy.py
lib/lp/registry/interfaces/accesspolicyservice.py
lib/lp/registry/interfaces/product.py
lib/lp/registry/javascript/disclosure/observerpicker.js
lib/lp/registry/javascript/disclosure/pillarsharingview.js
lib/lp/registry/javascript/disclosure/tests/test_observerpicker.js
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/sharing-picker-943433/+merge/95851
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/sharing-picker-943433 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py 2012-03-02 23:09:34 +0000
+++ lib/lp/registry/browser/pillar.py 2012-03-06 12:33:23 +0000
@@ -241,7 +241,7 @@
@property
def access_policies(self):
- return self._getAccessPolicyService().getAccessPolicies()
+ return self._getAccessPolicyService().getAccessPolicies(self.context)
@property
def sharing_permissions(self):
=== modified file 'lib/lp/registry/interfaces/accesspolicy.py'
--- lib/lp/registry/interfaces/accesspolicy.py 2012-02-28 12:14:41 +0000
+++ lib/lp/registry/interfaces/accesspolicy.py 2012-03-06 12:33:23 +0000
@@ -200,6 +200,13 @@
def findByPolicy(policies):
"""Return all `IAccessPolicyGrant` objects for the policies."""
+ def revoke(grants):
+ """Revoke the specified grants.
+
+ :param grants: a collection of (`IAccessPolicy`, grantee `IPerson`)
+ pairs.
+ """
+
class IAccessPolicyGrantFlatSource(Interface):
"""Experimental query utility to search through the flattened schema."""
=== modified file 'lib/lp/registry/interfaces/accesspolicyservice.py'
--- lib/lp/registry/interfaces/accesspolicyservice.py 2012-03-01 05:50:50 +0000
+++ lib/lp/registry/interfaces/accesspolicyservice.py 2012-03-06 12:33:23 +0000
@@ -10,7 +10,10 @@
'IAccessPolicyService',
]
-from zope.schema import Choice
+from zope.schema import (
+ Choice,
+ List,
+ )
from lazr.restful.declarations import (
call_with,
@@ -37,8 +40,8 @@
# version 'devel'
export_as_webservice_entry(publish_web_link=False, as_of='beta')
- def getAccessPolicies():
- """Return the access policy types."""
+ def getAccessPolicies(pillar):
+ """Return the allowed access policy types for the given pillar."""
def getSharingPermissions():
"""Return the access policy sharing permissions."""
@@ -55,10 +58,10 @@
@operation_parameters(
pillar=Reference(IPillar, title=_('Pillar'), required=True),
observer=Reference(IPerson, title=_('Observer'), required=True),
- access_policy_type=Choice(vocabulary=AccessPolicyType))
+ access_policy_types=List(Choice(vocabulary=AccessPolicyType)))
@operation_for_version('devel')
- def addPillarObserver(pillar, observer, access_policy_type, user):
- """Add an observer with the access policy to a pillar."""
+ def updatePillarObserver(pillar, observer, access_policy_types, user):
+ """Ensure observer has the grants for access policies on a pillar."""
@export_write_operation()
@operation_parameters(
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2012-02-14 07:06:37 +0000
+++ lib/lp/registry/interfaces/product.py 2012-03-06 12:33:23 +0000
@@ -729,6 +729,9 @@
"Whether the project's licensing requires a new "
"commercial subscription to use launchpad.")))
+ has_current_commercial_subscription = Attribute("""
+ Whether the project has a current commercial subscription.""")
+
license_status = Attribute("""
Whether the license is OPENSOURCE, UNREVIEWED, or PROPRIETARY.""")
=== modified file 'lib/lp/registry/javascript/disclosure/observerpicker.js'
--- lib/lp/registry/javascript/disclosure/observerpicker.js 2012-03-01 05:46:50 +0000
+++ lib/lp/registry/javascript/disclosure/observerpicker.js 2012-03-06 12:33:23 +0000
@@ -127,8 +127,6 @@
step_two_content.one('div.step-links')
.insert(self._make_policy_selector(), 'before');
step_one_content.insert(step_two_content, 'after');
- step_two_content.one('input[id=field.visibility.0]')
- .set('checked', 'checked');
}
this._fade_in(step_two_content, step_one_content);
},
@@ -137,14 +135,14 @@
// Determine the chosen access policy type. data already contains the
// selected person due to the base picker behaviour.
var contentBox = this.get('contentBox');
- var selected_access_policy;
+ var selected_access_policies = [];
contentBox.all('input[name=field.visibility]')
.each(function(node) {
if (node.get('checked')) {
- selected_access_policy = node.get('value');
+ selected_access_policies.push(node.get('value'));
}
});
- data.access_policy = selected_access_policy;
+ data.access_policies = selected_access_policies;
// Publish the result with step_nr 0 to indicate we have finished.
this.fire('save', data, 0);
},
@@ -152,15 +150,15 @@
_make_policy_selector: function() {
// The policy selector is a set of radio buttons.
var html = Y.lp.mustache.to_html([
- '<div style="margin-top: 0.75em">',
+ '<div style="margin: 0.75em 0 0 1em">',
'<table class="radio-button-widget"><tbody>',
'{{#policies}}',
' <tr>',
- ' <td rowspan="2"><input type="radio"',
+ ' <td rowspan="2"><input type="checkbox"',
' value="{{title}}"',
' name="field.visibility"',
' id="field.visibility.{{index}}"',
- ' class="radioType">',
+ ' class="checkboxType">',
' </td>',
' <td><label for="field.visibility.{{index}}">',
' <span class="accessPolicy{{value}}">{{title}}',
=== modified file 'lib/lp/registry/javascript/disclosure/pillarsharingview.js'
--- lib/lp/registry/javascript/disclosure/pillarsharingview.js 2012-03-01 05:46:50 +0000
+++ lib/lp/registry/javascript/disclosure/pillarsharingview.js 2012-03-06 12:33:23 +0000
@@ -59,8 +59,8 @@
self.save_sharing_selection(result);
}
});
- var picker =
- new Y.lp.registry.disclosure.observerpicker.ObserverPicker(new_config);
+ var ns = Y.lp.registry.disclosure.observerpicker;
+ var picker = new ns.ObserverPicker(new_config);
Y.lp.app.picker.setup_vocab_picker(picker, vocab, new_config);
this.set('observer_picker', picker);
},
@@ -158,7 +158,8 @@
var self = this;
var y_config = {
on: {
- start: Y.bind(self._show_delete_spinner, namespace, delete_link),
+ start: Y.bind(
+ self._show_delete_spinner, namespace, delete_link),
end: Y.bind(self._hide_delete_spinner, namespace, delete_link),
success: function() {
self.remove_observer_success(person_uri);
@@ -227,18 +228,19 @@
start: Y.bind(self._show_sharing_spinner, namespace),
end: Y.bind(self._hide_hiding_spinner, namespace),
success: function(observer_entry) {
- self.save_sharing_selection_success(observer_entry.getAttrs());
+ self.save_sharing_selection_success(
+ observer_entry.getAttrs());
},
failure: error_handler.getFailureHandler()
},
parameters: {
pillar: pillar_uri,
observer: person_uri,
- access_policy_type: selection_result.access_policy
+ access_policy_types: selection_result.access_policies
}
};
this.get('lp_client').named_post(
- '/+services/accesspolicy', 'addPillarObserver', y_config);
+ '/+services/accesspolicy', 'updatePillarObserver', y_config);
}
});
=== modified file 'lib/lp/registry/javascript/disclosure/tests/test_observerpicker.js'
--- lib/lp/registry/javascript/disclosure/tests/test_observerpicker.js 2012-03-01 05:46:50 +0000
+++ lib/lp/registry/javascript/disclosure/tests/test_observerpicker.js 2012-03-06 12:33:23 +0000
@@ -3,7 +3,8 @@
YUI.add('lp.registry.disclosure.observerpicker.test', function (Y) {
var tests = Y.namespace('lp.registry.disclosure.observerpicker.test');
- tests.suite = new Y.Test.Suite('lp.registry.disclosure.observerpicker Tests');
+ tests.suite = new Y.Test.Suite(
+ 'lp.registry.disclosure.observerpicker Tests');
tests.suite.add(new Y.Test.Case({
name: 'lp.registry.disclosure.observerpicker_tests',
@@ -58,8 +59,8 @@
if (overrides !== undefined) {
config = Y.merge(config, overrides);
}
- var picker =
- new Y.lp.registry.disclosure.observerpicker.ObserverPicker(config);
+ var ns = Y.lp.registry.disclosure.observerpicker;
+ var picker = new ns.ObserverPicker(config);
Y.lp.app.picker.setup_vocab_picker(picker, "TestVocab", config);
return picker;
},
@@ -113,9 +114,6 @@
'input[value="' + policy.title + '"]');
Y.Assert.isNotNull(rb);
});
- // The first policy button should be selected.
- Y.Assert.isTrue(step_two_content
- .one('input[id=field.visibility.0]').get('checked'));
// There should be a link back to previous step.
Y.Assert.isNotNull(step_two_content.one('a.prev'));
// There should be a button and link to finalise the selection.
@@ -171,7 +169,8 @@
step_two_content.one('input[value="Policy 2"]').simulate('click');
var select_link = step_two_content.one('a.next');
select_link.simulate('click');
- Y.Assert.areEqual('Policy 2', selected_result.access_policy);
+ Y.ArrayAssert.itemsAreEqual(
+ ['Policy 2'], selected_result.access_policies);
Y.Assert.areEqual('~/fred', selected_result.api_uri);
}
}));
=== modified file 'lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.js'
--- lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.js 2012-03-01 05:46:50 +0000
+++ lib/lp/registry/javascript/disclosure/tests/test_pillarsharingview.js 2012-03-06 12:33:23 +0000
@@ -197,13 +197,13 @@
person_uri = Y.lp.client.get_absolute_uri(person_uri);
var expected_url;
expected_url = Y.lp.client.append_qs(
- expected_url, 'ws.op', 'addPillarObserver');
+ expected_url, 'ws.op', 'updatePillarObserver');
expected_url = Y.lp.client.append_qs(
expected_url, 'pillar', '~pillar');
expected_url = Y.lp.client.append_qs(
expected_url, 'observer', person_uri);
expected_url = Y.lp.client.append_qs(
- expected_url, 'access_policy_type', 'Policy 2');
+ expected_url, 'access_policy_types', ['Policy 2']);
Y.Assert.areEqual(expected_url, mockio.last_request.config.data);
mockio.last_request.successJSON({
'resource_type_link': 'entity',
@@ -212,7 +212,8 @@
Y.Assert.isTrue(save_sharing_selection_success_called);
},
- // The save_sharing_selection_success callback updates the model and table.
+ // The save_sharing_selection_success callback updates the model and
+ // table.
test_save_sharing_selection_success: function() {
this.view = this._create_Widget({anim_duration: 0.001});
this.view.render();
=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py 2012-02-27 06:23:25 +0000
+++ lib/lp/registry/model/accesspolicy.py 2012-03-06 12:33:23 +0000
@@ -306,6 +306,11 @@
ids = [policy.id for policy in policies]
return IStore(cls).find(cls, cls.policy_id.is_in(ids))
+ @classmethod
+ def revoke(cls, grants):
+ """See `IAccessPolicyGrantSource`."""
+ cls.find(grants).remove()
+
class AccessPolicyGrantFlat(StormBase):
__storm_table__ = 'AccessPolicyGrantFlat'
=== modified file 'lib/lp/registry/services/accesspolicyservice.py'
--- lib/lp/registry/services/accesspolicyservice.py 2012-03-05 00:42:29 +0000
+++ lib/lp/registry/services/accesspolicyservice.py 2012-03-06 12:33:23 +0000
@@ -2,7 +2,6 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Classes for pillar and artifact access policy services."""
-from lp.registry.interfaces.projectgroup import IProjectGroup
__metaclass__ = type
__all__ = [
@@ -24,6 +23,8 @@
IAccessPolicyGrantSource,
)
from lp.registry.interfaces.accesspolicyservice import IAccessPolicyService
+from lp.registry.interfaces.product import IProduct
+from lp.registry.interfaces.projectgroup import IProjectGroup
from lp.services.webapp.authorization import available_with_permission
@@ -41,18 +42,28 @@
"""See `IService`."""
return 'accesspolicy'
- def getAccessPolicies(self):
+ def getAccessPolicies(self, pillar):
"""See `IAccessPolicyService`."""
- policies = []
- for x, policy in enumerate(AccessPolicyType):
+
+ allowed_policy_types = [
+ AccessPolicyType.EMBARGOEDSECURITY,
+ AccessPolicyType.USERDATA]
+ # Products with current commercial subscriptions are also allowed to
+ # have a PROPRIETARY access policy.
+ if (IProduct.providedBy(pillar) and
+ pillar.has_current_commercial_subscription):
+ allowed_policy_types.append(AccessPolicyType.PROPRIETARY)
+
+ policies_data = []
+ for x, policy in enumerate(allowed_policy_types):
item = dict(
index=x,
- value=policy.token,
+ value=policy.name,
title=policy.title,
- description=policy.value.description
+ description=policy.description
)
- policies.append(item)
- return policies
+ policies_data.append(item)
+ return policies_data
def getSharingPermissions(self):
"""See `IAccessPolicyService`."""
@@ -93,30 +104,56 @@
return result
@available_with_permission('launchpad.Edit', 'pillar')
- def addPillarObserver(self, pillar, observer, access_policy_type, user):
+ def updatePillarObserver(self,
+ pillar, observer, access_policy_types, user):
"""See `IAccessPolicyService`."""
# We do not support adding observers to project groups.
assert not IProjectGroup.providedBy(pillar)
- # Create a pillar access policy if one doesn't exist.
+ pillar_policy_types = [(pillar, access_policy_type)
+ for access_policy_type in access_policy_types]
+
+ # Create any missing pillar access policies.
policy_source = getUtility(IAccessPolicySource)
- pillar_access_policy = [(pillar, access_policy_type)]
- policy = policy_source.find(pillar_access_policy).one()
- if policy is None:
- [policy] = policy_source.create(pillar_access_policy)
- # We have a policy, create the grant if it doesn't exist.
+ pillar_policies = list(policy_source.find(pillar_policy_types))
+ existing_policy_types = [(pillar, pillar_policy.type)
+ for pillar_policy in pillar_policies]
+ required_policies = (
+ set(pillar_policy_types).difference(existing_policy_types))
+ if len(required_policies) > 0:
+ pillar_policies.extend(policy_source.create(required_policies))
+
+ # We have the policies, we need to figure out which grants we need to
+ # create. We also need to revoke any grants which are not required.
policy_grant_source = getUtility(IAccessPolicyGrantSource)
- if policy_grant_source.find([(policy, observer)]).count() == 0:
- policy_grant_source.grant([(policy, observer, user)])
+ policy_grants = [(policy, observer) for policy in pillar_policies]
+ existing_grants = [(grant.policy, grant.grantee)
+ for grant in policy_grant_source.find(policy_grants)]
+ required_grants = set(policy_grants).difference(existing_grants)
+
+ all_pillar_policies = policy_source.findByPillar([pillar])
+ possible_policy_grants = [(policy, observer)
+ for policy in all_pillar_policies]
+ possible_grants = [(grant.policy, grant.grantee)
+ for grant in policy_grant_source.find(possible_policy_grants)]
+
+ grants_to_revoke = set(possible_grants).difference(policy_grants)
+ # Create any newly required grants.
+ if len(required_grants) > 0:
+ policy_grant_source.grant([(policy, observer, user)
+ for policy, observer in required_grants])
+ # Now revoke any existing grants no longer required.
+ if len(grants_to_revoke) > 0:
+ policy_grant_source.revoke(grants_to_revoke)
# Return observer data to the caller.
request = get_current_web_service_request()
resource = EntryResource(observer, request)
person_data = resource.toDataForJSON()
- permissions = {
- access_policy_type.name: SharingPermission.ALL.name,
- }
+ permissions = {}
+ for access_policy_type in access_policy_types:
+ permissions[access_policy_type.name] = SharingPermission.ALL.name
person_data['permissions'] = permissions
return person_data
=== modified file 'lib/lp/registry/services/tests/test_accesspolicyservice.py'
--- lib/lp/registry/services/tests/test_accesspolicyservice.py 2012-03-05 00:42:29 +0000
+++ lib/lp/registry/services/tests/test_accesspolicyservice.py 2012-03-06 12:33:23 +0000
@@ -40,23 +40,69 @@
super(TestAccessPolicyService, self).setUp()
self.service = getUtility(IService, 'accesspolicy')
- def _makeObserverData(self, observer):
+ def _makeObserverData(self, observer, policy_types):
# Unpack an observer into its attributes and add in permissions.
request = get_current_web_service_request()
resource = EntryResource(observer, request)
observer_data = resource.toDataForJSON()
- observer_data['permissions'] = {
- AccessPolicyType.PROPRIETARY.name: SharingPermission.ALL.name}
+ permissions = {}
+ for policy in policy_types:
+ permissions[policy.name] = SharingPermission.ALL.name
+ observer_data['permissions'] = permissions
return observer_data
+ def _test_getAccessPolicies(self, pillar, expected_policies):
+ policy_data = self.service.getAccessPolicies(pillar)
+ expected_data = []
+ for x, policy in enumerate(expected_policies):
+ item = dict(
+ index=x,
+ value=policy.name,
+ title=policy.title,
+ description=policy.description
+ )
+ expected_data.append(item)
+ self.assertContentEqual(expected_data, policy_data)
+
+ def test_getAccessPolicies_product(self):
+ product = self.factory.makeProduct()
+ self._test_getAccessPolicies(
+ product,
+ [AccessPolicyType.EMBARGOEDSECURITY, AccessPolicyType.USERDATA])
+
+ def test_getAccessPolicies_expired_commercial_product(self):
+ product = self.factory.makeProduct()
+ self.factory.makeCommercialSubscription(product, expired=True)
+ self._test_getAccessPolicies(
+ product,
+ [AccessPolicyType.EMBARGOEDSECURITY, AccessPolicyType.USERDATA])
+
+ def test_getAccessPolicies_commercial_product(self):
+ product = self.factory.makeProduct()
+ self.factory.makeCommercialSubscription(product)
+ self._test_getAccessPolicies(
+ product,
+ [AccessPolicyType.EMBARGOEDSECURITY,
+ AccessPolicyType.USERDATA,
+ AccessPolicyType.PROPRIETARY])
+
+ def test_getAccessPolicies_distro(self):
+ distro = self.factory.makeDistribution()
+ self._test_getAccessPolicies(
+ distro,
+ [AccessPolicyType.EMBARGOEDSECURITY, AccessPolicyType.USERDATA])
+
def _test_getPillarObservers(self, pillar):
# getPillarObservers returns the expected data.
- access_policy = self.factory.makeAccessPolicy(pillar=pillar)
+ access_policy = self.factory.makeAccessPolicy(
+ pillar=pillar,
+ type=AccessPolicyType.PROPRIETARY)
grantee = self.factory.makePerson()
self.factory.makeAccessPolicyGrant(access_policy, grantee)
[observer] = self.service.getPillarObservers(pillar)
- person_data = self._makeObserverData(grantee)
- self.assertContentEqual(person_data, observer)
+ person_data = self._makeObserverData(
+ grantee, [AccessPolicyType.PROPRIETARY])
+ self.assertEqual(person_data, observer)
def test_getProductObservers(self):
# Users with launchpad.Driver can view observers.
@@ -93,65 +139,87 @@
login_person(self.factory.makePerson())
self._test_getPillarObserversUnauthorized(product)
- def _test_addPillarObserver(self, pillar):
- """addPillarObservers works and returns the expected data."""
+ def _test_updatePillarObserver(self, pillar):
+ """updatePillarObservers works and returns the expected data."""
observer = self.factory.makePerson()
- access_policy_type = AccessPolicyType.USERDATA
- user = self.factory.makePerson()
- observer_data = self.service.addPillarObserver(
- pillar, observer, access_policy_type, user)
- [policy] = getUtility(IAccessPolicySource).findByPillar([pillar])
+ grantor = self.factory.makePerson()
+
+ # Make existing grants to ensure updatePillarObserver handles those
+ # cases correctly.
+ # First, a grant that is in the add set - it wil be retained.
+ policy = self.factory.makeAccessPolicy(
+ pillar=pillar, type=AccessPolicyType.EMBARGOEDSECURITY)
+ self.factory.makeAccessPolicyGrant(
+ policy, grantee=observer, grantor=grantor)
+ # Second, a grant that is not in the add set - it will be deleted.
+ policy = self.factory.makeAccessPolicy(
+ pillar=pillar, type=AccessPolicyType.PROPRIETARY)
+ self.factory.makeAccessPolicyGrant(
+ policy, grantee=observer, grantor=grantor)
+
+ # Now call updatePillarObserver will the grants we want.
+ access_policy_types = [
+ AccessPolicyType.EMBARGOEDSECURITY,
+ AccessPolicyType.USERDATA]
+ observer_data = self.service.updatePillarObserver(
+ pillar, observer, access_policy_types, grantor)
+ policies = getUtility(IAccessPolicySource).findByPillar([pillar])
policy_grant_source = getUtility(IAccessPolicyGrantSource)
- [grant] = policy_grant_source.findByPolicy([policy])
- self.assertEqual(user, grant.grantor)
- self.assertEqual(observer, grant.grantee)
- self.assertEqual(policy, grant.policy)
- expected_observer_data = self._makeObserverData(observer)
- self.assertContentEqual(expected_observer_data, observer_data)
+ grants = policy_grant_source.findByPolicy(policies)
+ self.assertEqual(grants.count(), len(access_policy_types))
+ for grant in grants:
+ self.assertEqual(grantor, grant.grantor)
+ self.assertEqual(observer, grant.grantee)
+ self.assertIn(grant.policy.type, access_policy_types)
+ expected_observer_data = self._makeObserverData(
+ observer, access_policy_types)
+ self.assertEqual(expected_observer_data, observer_data)
- def test_addProjectGroupObserver_not_allowed(self):
+ def test_updateProjectGroupObserver_not_allowed(self):
# We cannot add observers to ProjectGroups.
owner = self.factory.makePerson()
project_group = self.factory.makeProject(owner=owner)
+ observer = self.factory.makePerson()
login_person(owner)
self.assertRaises(
- AssertionError, self._test_addPillarObserver, project_group)
+ AssertionError, self.service.updatePillarObserver,
+ project_group, observer, [AccessPolicyType.USERDATA], owner)
- def test_addProductObserver(self):
+ def test_updateProductObserver(self):
# Users with launchpad.Edit can add observers.
owner = self.factory.makePerson()
product = self.factory.makeProduct(owner=owner)
login_person(owner)
- self._test_addPillarObserver(product)
+ self._test_updatePillarObserver(product)
- def test_addDistroObserver(self):
+ def test_updateDistroObserver(self):
# Users with launchpad.Edit can add observers.
owner = self.factory.makePerson()
distro = self.factory.makeDistribution(owner=owner)
login_person(owner)
- self._test_addPillarObserver(distro)
+ self._test_updatePillarObserver(distro)
- def _test_addPillarObserverUnauthorized(self, pillar):
- # addPillarObserver raises an Unauthorized exception if the user is
+ def _test_updatePillarObserverUnauthorized(self, pillar):
+ # updatePillarObserver raises an Unauthorized exception if the user is
# not permitted to do so.
observer = self.factory.makePerson()
access_policy_type = AccessPolicyType.USERDATA
user = self.factory.makePerson()
self.assertRaises(
- Unauthorized, self.service.addPillarObserver,
- pillar, observer, access_policy_type, user)
+ Unauthorized, self.service.updatePillarObserver,
+ pillar, observer, [access_policy_type], user)
- def test_addPillarObserverAnonymous(self):
+ def test_updatePillarObserverAnonymous(self):
# Anonymous users are not allowed.
product = self.factory.makeProduct()
login(ANONYMOUS)
- self._test_addPillarObserverUnauthorized(product)
+ self._test_updatePillarObserverUnauthorized(product)
- def test_addPillarObserverAnyone(self):
+ def test_updatePillarObserverAnyone(self):
# Unauthorized users are not allowed.
product = self.factory.makeProduct()
login_person(self.factory.makePerson())
- self._test_addPillarObserverUnauthorized(product)
+ self._test_updatePillarObserverUnauthorized(product)
class ApiTestMixin:
=== modified file 'lib/lp/registry/tests/test_accesspolicy.py'
--- lib/lp/registry/tests/test_accesspolicy.py 2012-02-27 06:23:25 +0000
+++ lib/lp/registry/tests/test_accesspolicy.py 2012-03-06 12:33:23 +0000
@@ -362,6 +362,32 @@
grants,
getUtility(IAccessPolicyGrantSource).findByPolicy([policy]))
+ def test_revoke(self):
+ # revoke() removes the specified grants.
+ policy = self.factory.makeAccessPolicy()
+ grants = [
+ self.factory.makeAccessPolicyGrant(policy=policy)
+ for i in range(3)]
+
+ # Make some other grants to ensure they're unaffected.
+ other_grants = [
+ self.factory.makeAccessPolicyGrant(policy=policy)
+ for i in range(3)]
+ other_grants.extend([
+ self.factory.makeAccessPolicyGrant()
+ for i in range(3)])
+
+ to_delete = [(grant.policy, grant.grantee) for grant in grants]
+ getUtility(IAccessPolicyGrantSource).revoke(to_delete)
+ IStore(policy).invalidate()
+
+ for grant in grants:
+ self.assertRaises(LostObjectError, getattr, grant, 'grantor')
+ self.assertEqual(
+ 0, getUtility(IAccessPolicyGrantSource).find(to_delete).count())
+ for other_grant in other_grants:
+ self.assertIsNot(None, other_grant.grantor)
+
class TestAccessPolicyGrantFlatSource(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
