launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11976
[Merge] lp:~deryck/launchpad/sharing-page-for-specifications into lp:launchpad
Deryck Hodge has proposed merging lp:~deryck/launchpad/sharing-page-for-specifications into lp:launchpad with lp:~deryck/launchpad/use-specification-sharing-policy as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~deryck/launchpad/sharing-page-for-specifications/+merge/124314
This branch is the culmination of several branches of work to get sharing policy going for blueprints. This is the final branch, and it enables the +sharing page UI to be able to set project-wide policies for blueprints.
The primary work done here was to:
* Update +sharing page template to include specification policy
* Wire up sharing service JavaScript to update specification policy
* Update view code to provide specification_sharing_policies
* Update sharing service itself to provide getSpecificationSharingPolicies
(which is used to populate specification_sharing_policies)
The rest of the work is testing related. I added new test coverage for both the view changes and js changes. In the process, testing discovered that Distribution needed the IHasSharingPolicies interface updated to include specification_sharing_policy. This always returns PUBLIC for distros. Finally, the factory method makeProduct needed to be updated to set specification_sharing_policy via a keyword argument, to follow the pattern of branch and bug policies in other tests.
(I'll note here, too, that I noticed IHasSharingPolicies isn't consistently used. Only Distribution uses it, and Product just adds the XXX_sharing_policy methods directly to it's own interface. This should be refactored eventually, but is really outside the scope or time I have here to get this done.)
This work is lint free. The LOC increase is allowed due to this being part of the larger disclosure project.
To QA the work:
* Enable the feature flag "blueprints.information_type.enabled"
* Visit any project's +sharing page
* You should see the current specification policy listed
* You should be able to change it
(A final note: the +sharing page does a reload after an ajax post. This should be changed at some point to update the page inline; otherwise, there's no point wasting all that code and energy on API calls. We could just plain form post the changes if we're going to reload it.)
I will file bugs about these two side-note issues.
--
https://code.launchpad.net/~deryck/launchpad/sharing-page-for-specifications/+merge/124314
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/sharing-page-for-specifications into lp:launchpad.
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py 2012-09-03 22:08:40 +0000
+++ lib/lp/registry/browser/pillar.py 2012-09-14 01:07:22 +0000
@@ -56,6 +56,7 @@
from lp.registry.interfaces.projectgroup import IProjectGroup
from lp.registry.model.pillar import PillarPerson
from lp.services.config import config
+from lp.services.features import getFeatureFlag
from lp.services.propertycache import cachedproperty
from lp.services.webapp.batching import (
BatchNavigator,
@@ -291,6 +292,13 @@
return self._getSharingService().getBranchSharingPolicies(self.context)
@property
+ def specification_sharing_policies(self):
+ if getFeatureFlag('blueprints.information_type.enabled'):
+ return self._getSharingService().getSpecificationSharingPolicies(
+ self.context)
+ return None
+
+ @property
def sharing_permissions(self):
return self._getSharingService().getSharingPermissions()
@@ -344,6 +352,8 @@
cache.objects['bug_sharing_policies'] = self.bug_sharing_policies
cache.objects['branch_sharing_policies'] = (
self.branch_sharing_policies)
+ cache.objects['specification_sharing_policies'] = (
+ self.specification_sharing_policies)
view_names = set(reg.name for reg
in iter_view_registrations(self.__class__))
=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-09-05 22:02:39 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-09-14 01:07:22 +0000
@@ -29,6 +29,7 @@
from lp.registry.model.pillar import PillarPerson
from lp.services.config import config
from lp.services.database.lpstorm import IStore
+from lp.services.features.testing import FeatureFixture
from lp.services.webapp.interfaces import StormRangeFactoryError
from lp.services.webapp.publisher import canonical_url
from lp.testing import (
@@ -277,6 +278,18 @@
self.assertContentEqual(
grantee_data, cache.objects.get('grantee_data'))
+ def test_view_date_model_adds_specification(self):
+ # This test can move up to the above test when not feature flagged,
+ # but for now, ensure specification_sharing_policies is added to
+ # the cache if the flag is set.
+ feature_flag = {
+ 'blueprints.information_type.enabled': 'on'}
+ with FeatureFixture(feature_flag):
+ view = create_initialized_view(self.pillar, name='+sharing')
+ cache = IJSONRequestCache(view.request)
+ self.assertIsNotNone(
+ cache.objects.get('specification_sharing_policies'))
+
def test_view_batch_data(self):
# Test the expected batching data is in the json request cache.
view = create_initialized_view(self.pillar, name='+sharing')
=== modified file 'lib/lp/registry/interfaces/pillar.py'
--- lib/lp/registry/interfaces/pillar.py 2012-09-05 06:48:36 +0000
+++ lib/lp/registry/interfaces/pillar.py 2012-09-14 01:07:22 +0000
@@ -37,6 +37,7 @@
from lp.registry.enums import (
BranchSharingPolicy,
BugSharingPolicy,
+ SpecificationSharingPolicy,
)
@@ -100,6 +101,11 @@
description=_("Sharing policy for this pillar's bugs."),
required=False, readonly=True, vocabulary=BugSharingPolicy),
as_of='devel')
+ specification_sharing_policy = exported(Choice(
+ title=_('Specification sharing policy'),
+ description=_("Sharing policy for this project's specifications."),
+ required=False, readonly=True, vocabulary=SpecificationSharingPolicy),
+ as_of='devel')
class IPillarName(Interface):
=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py 2012-09-06 10:59:56 +0000
+++ lib/lp/registry/interfaces/sharingservice.py 2012-09-14 01:07:22 +0000
@@ -35,6 +35,7 @@
BugSharingPolicy,
InformationType,
SharingPermission,
+ SpecificationSharingPolicy,
)
from lp.registry.interfaces.person import IPerson
from lp.registry.interfaces.pillar import IPillar
@@ -148,6 +149,9 @@
def getBranchSharingPolicies(pillar):
"""Return the allowed branch sharing policies for the given pillar."""
+ def getSpecificationSharingPolicies(pillar):
+ """Return specification sharing policies for a given pillar."""
+
def getSharingPermissions():
"""Return the information sharing permissions."""
@@ -262,13 +266,17 @@
@operation_parameters(
pillar=Reference(IPillar, title=_('Pillar'), required=True),
branch_sharing_policy=Choice(vocabulary=BranchSharingPolicy),
- bug_sharing_policy=Choice(vocabulary=BugSharingPolicy))
+ bug_sharing_policy=Choice(vocabulary=BugSharingPolicy),
+ specification_sharing_policy=Choice(
+ vocabulary=SpecificationSharingPolicy))
@operation_for_version('devel')
def updatePillarSharingPolicies(pillar, branch_sharing_policy=None,
- bug_sharing_policy=None):
+ bug_sharing_policy=None,
+ specification_sharing_policy=None):
"""Update the sharing policies for a pillar.
:param pillar: the pillar to update
:param branch_sharing_policy: the new branch sharing policy
:param bug_sharing_policy: the new bug sharing policy
+ :param specification_sharing_policy: the new spec. sharing policy
"""
=== modified file 'lib/lp/registry/javascript/sharing/pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-09-05 06:48:36 +0000
+++ lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-09-14 01:07:22 +0000
@@ -125,6 +125,8 @@
= this._render_sharing_policy('bug', 'Bug');
this.branch_sharing_policy_widget
= this._render_sharing_policy('branch', 'Branch');
+ this.specification_sharing_policy_widget
+ = this._render_sharing_policy('specification', 'Specification');
},
// Render the sharing policy choice popup.
@@ -214,6 +216,15 @@
self.branch_sharing_policy_widget, 'branch', policy);
});
}
+ if (this.specification_sharing_policy_widget !== null) {
+ this.specification_sharing_policy_widget.on('save', function(e) {
+ var policy = self.specification_sharing_policy_widget.get(
+ 'value');
+ self.save_sharing_policy(
+ self.specification_sharing_policy_widget, 'specification',
+ policy);
+ });
+ }
},
syncUI: function() {
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.html'
--- lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.html 2012-08-22 20:28:52 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.html 2012-09-14 01:07:22 +0000
@@ -102,6 +102,15 @@
<div id="bug-sharing-policy-description" class="formHelp"></div>
</td>
</tr>
+ <tr id="specification-sharing-policy-row">
+ <td id="specification-sharing-policy">
+ Specification sharing policy:
+ <strong><span class="value"></span></strong>
+ <a class="editicon sprite edit action-icon"
+ style="padding-bottom: 0;">Edit</a>
+ <div id="specification-sharing-policy-description" class="formHelp"></div>
+ </td>
+ </tr>
</table>
</script>
</head>
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-09-10 21:02:05 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-09-14 01:07:22 +0000
@@ -16,7 +16,8 @@
context: {
self_link: '~pillar', display_name: 'Pillar',
bug_sharing_policy: 'Bug Policy 1',
- branch_sharing_policy: null},
+ branch_sharing_policy: null,
+ specification_sharing_policy: null},
grantee_data: [
{'name': 'fred', 'display_name': 'Fred Bloggs',
'role': '(Maintainer)', web_link: '~fred',
@@ -55,6 +56,11 @@
{index: '0', value: 'BRSP1',
title: 'Branch Policy 1',
description: 'Branch Policy 1 description'}
+ ],
+ specification_sharing_policies: [
+ {index: '0', value: 'SPSP1',
+ title: 'Specification Policy 1',
+ description: 'Specification Policy 1 description'}
]
}
};
@@ -568,6 +574,21 @@
this.mockio.last_request.config.data);
},
+ _assert_artifact_sharing_policy_save_success: function(
+ artifact_type, title) {
+ var artifact_key = artifact_type + '_sharing_policy';
+ window.LP.cache.context[artifact_key] = null;
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save(artifact_type, title);
+ var reload_called = false;
+ this.view._reload = function() {
+ reload_called = true;
+ };
+ this.mockio.last_request.successJSON({});
+ Y.Assert.isTrue(reload_called);
+ },
+
// Bug sharing policy is saved.
test_bug_sharing_policy_save: function() {
window.LP.cache.context.bug_sharing_policy = null;
@@ -576,22 +597,8 @@
this._assert_sharing_policy_save('bug', 'Bug');
},
- // Bug sharing policy is saved and the client is updated.
- _assert_bug_sharing_policy_save_success: function() {
- window.LP.cache.context.bug_sharing_policy = null;
- this.view = this._create_Widget();
- this.view.render();
- this._assert_sharing_policy_save('bug', 'Bug');
- var reload_called = false;
- this.view._reload = function() {
- reload_called = true;
- };
- this.mockio.last_request.successJSON({});
- Y.Assert.isTrue(reload_called);
- },
-
test_bug_sharing_policy_save_success: function() {
- this._assert_bug_sharing_policy_save_success();
+ this._assert_artifact_sharing_policy_save_success('bug', 'Bug');
},
// When a failure occurs, the client retains the existing data.
@@ -620,22 +627,9 @@
this._assert_sharing_policy_save('branch', 'Branch');
},
- // Branch sharing policy is saved and the client is updated.
- _assert_branch_sharing_policy_save_success: function() {
- window.LP.cache.context.bug_sharing_policy = null;
- this.view = this._create_Widget();
- this.view.render();
- this._assert_sharing_policy_save('branch', 'Branch');
- var reload_called = false;
- this.view._reload = function() {
- reload_called = true;
- };
- this.mockio.last_request.successJSON({});
- Y.Assert.isTrue(reload_called);
- },
-
test_branch_sharing_policy_save_success: function() {
- this._assert_branch_sharing_policy_save_success();
+ this._assert_artifact_sharing_policy_save_success(
+ 'branch', 'Branch');
},
// When a failure occurs, the client retains the existing data.
@@ -653,6 +647,40 @@
Y.Assert.areEqual(
'Legacy',
Y.one('#branch-sharing-policy-description').get('text'));
+ },
+
+ // Specification sharing policy is saved.
+ test_specification_sharing_policy_save: function() {
+ window.LP.cache.context.specification_sharing_policy = null;
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save(
+ 'specification', 'Specification');
+ },
+
+ test_specification_sharing_policy_save_success: function() {
+ this._assert_artifact_sharing_policy_save_success(
+ 'specification', 'Specification');
+ },
+
+ // When a failure occurs, the client retains the existing data.
+ test_specification_sharing_policy_save_failure: function() {
+ this.view = this._create_Widget();
+ this.view.render();
+ this._assert_sharing_policy_save(
+ 'specification', 'Specification');
+ this.mockio.failure();
+ // Check the cached context.
+ Y.Assert.isNull(
+ window.LP.cache.context.specification_sharing_policy);
+ // Check the portlet.
+ Y.Assert.areEqual(
+ 'Legacy policy',
+ Y.one('#specification-sharing-policy .value').get('text'));
+ Y.Assert.areEqual(
+ 'Legacy',
+ Y.one('#specification-sharing-policy-description').get(
+ 'text'));
}
})));
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2012-09-14 01:07:21 +0000
+++ lib/lp/registry/model/distribution.py 2012-09-14 01:07:22 +0000
@@ -97,6 +97,7 @@
InformationType,
PRIVATE_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
+ SpecificationSharingPolicy,
)
from lp.registry.errors import NoSuchDistroSeries
from lp.registry.interfaces.accesspolicy import IAccessPolicySource
@@ -294,6 +295,12 @@
return BugSharingPolicy.PUBLIC
@property
+ def specification_sharing_policy(self):
+ """See `IHasSharingPolicies."""
+ # Sharing policy for distributions is always PUBLIC.
+ return SpecificationSharingPolicy.PUBLIC
+
+ @property
def uploaders(self):
"""See `IDistribution`."""
# Get all the distribution archives and find out the uploaders
=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py 2012-09-06 10:59:56 +0000
+++ lib/lp/registry/services/sharingservice.py 2012-09-14 01:07:22 +0000
@@ -34,6 +34,7 @@
BugSharingPolicy,
PRIVATE_INFORMATION_TYPES,
SharingPermission,
+ SpecificationSharingPolicy,
)
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactGrantSource,
@@ -305,6 +306,24 @@
return self._makeEnumData(allowed_policies)
+ def getSpecificationSharingPolicies(self, pillar):
+ """See `ISharingService`."""
+ # Only Products have specification sharing policies. Distributions just
+ # default to Public.
+ allowed_policies = [SpecificationSharingPolicy.PUBLIC]
+ # Commercial projects also allow proprietary specifications.
+ if (IProduct.providedBy(pillar)
+ and pillar.has_current_commercial_subscription):
+ allowed_policies.extend([
+ SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY,
+ SpecificationSharingPolicy.PROPRIETARY_OR_PUBLIC,
+ SpecificationSharingPolicy.PROPRIETARY])
+ if (pillar.specification_sharing_policy and
+ not pillar.specification_sharing_policy in allowed_policies):
+ allowed_policies.append(pillar.specification_sharing_policy)
+
+ return self._makeEnumData(allowed_policies)
+
def getSharingPermissions(self):
"""See `ISharingService`."""
# We want the permissions displayed in the following order.
@@ -556,8 +575,10 @@
@available_with_permission('launchpad.Edit', 'pillar')
def updatePillarSharingPolicies(self, pillar, branch_sharing_policy=None,
- bug_sharing_policy=None):
- if not branch_sharing_policy and not bug_sharing_policy:
+ bug_sharing_policy=None,
+ specification_sharing_policy=None):
+ if (not branch_sharing_policy and not bug_sharing_policy and not
+ specification_sharing_policy):
return None
# Only Products have sharing policies.
if not IProduct.providedBy(pillar):
@@ -567,3 +588,5 @@
pillar.setBranchSharingPolicy(branch_sharing_policy)
if bug_sharing_policy:
pillar.setBugSharingPolicy(bug_sharing_policy)
+ if specification_sharing_policy:
+ pillar.setSpecificationSharingPolicy(specification_sharing_policy)
=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py 2012-09-10 10:20:39 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2012-09-14 01:07:22 +0000
@@ -26,6 +26,7 @@
BugSharingPolicy,
InformationType,
SharingPermission,
+ SpecificationSharingPolicy,
)
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactGrantSource,
@@ -207,6 +208,51 @@
self._assert_getBranchSharingPolicies(
distro, [BranchSharingPolicy.PUBLIC])
+ def _assert_getSpecificationSharingPolicies(
+ self, pillar, expected_policies):
+ policy_data = self.service.getSpecificationSharingPolicies(pillar)
+ self._assert_enumData(expected_policies, policy_data)
+
+ def test_getSpecificationSharingPolicies_product(self):
+ product = self.factory.makeProduct()
+ self._assert_getSpecificationSharingPolicies(
+ product, [SpecificationSharingPolicy.PUBLIC])
+
+ def test_getSpecificationSharingPolicies_expired_commercial_product(self):
+ product = self.factory.makeProduct()
+ self.factory.makeCommercialSubscription(product, expired=True)
+ self._assert_getSpecificationSharingPolicies(
+ product, [SpecificationSharingPolicy.PUBLIC])
+
+ def test_getSpecificationSharingPolicies_commercial_product(self):
+ product = self.factory.makeProduct()
+ self.factory.makeCommercialSubscription(product)
+ self._assert_getSpecificationSharingPolicies(
+ product,
+ [SpecificationSharingPolicy.PUBLIC,
+ SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY,
+ SpecificationSharingPolicy.PROPRIETARY_OR_PUBLIC,
+ SpecificationSharingPolicy.PROPRIETARY])
+
+ def test_getSpecificationSharingPolicies_product_with_embargoed(self):
+ # The sharing policies will contain the product's sharing policy even
+ # if it is not in the nominally allowed policy list.
+ product = self.factory.makeProduct(
+ specification_sharing_policy=(
+ SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY))
+ self._assert_getSpecificationSharingPolicies(
+ product,
+ [SpecificationSharingPolicy.PUBLIC,
+ SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY,
+ SpecificationSharingPolicy.PROPRIETARY_OR_PUBLIC,
+ SpecificationSharingPolicy.PROPRIETARY,
+ SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY])
+
+ def test_getSpecificationSharingPolicies_distro(self):
+ distro = self.factory.makeDistribution()
+ self._assert_getSpecificationSharingPolicies(
+ distro, [SpecificationSharingPolicy.PUBLIC])
+
def _assert_getBugSharingPolicies(self, pillar, expected_policies):
policy_data = self.service.getBugSharingPolicies(pillar)
self._assert_enumData(expected_policies, policy_data)
=== modified file 'lib/lp/registry/templates/pillar-sharing.pt'
--- lib/lp/registry/templates/pillar-sharing.pt 2012-09-05 13:09:18 +0000
+++ lib/lp/registry/templates/pillar-sharing.pt 2012-09-14 01:07:22 +0000
@@ -74,6 +74,25 @@
></div>
</td>
</tr>
+
+
+ <tr id="specification-sharing-policy-row"
+ tal:condition="view/specification_sharing_policies">
+ <td id="specification-sharing-policy">
+ Specification sharing policy:
+ <strong><span class="value"
+ tal:content="context/specification_sharing_policy/title|string:Legacy policy"
+ ></span></strong>
+ <a class="editicon sprite edit action-icon hidden" href="#"
+ style="padding-bottom: 0;">Edit</a>
+ <div id="specification-sharing-policy-description" class="formHelp"
+ tal:content="context/specification_sharing_policy/description|
+ string:Legacy project sharing policy will continue to be used until
+ a new policy is configured."
+ ></div>
+ </td>
+ </tr>
+
</table>
<h2>Who it's shared with</h2>
<div id="sharing-header">
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-09-12 12:17:27 +0000
+++ lib/lp/testing/factory.py 2012-09-14 01:07:22 +0000
@@ -142,6 +142,7 @@
DistroSeriesDifferenceStatus,
DistroSeriesDifferenceType,
InformationType,
+ SpecificationSharingPolicy,
TeamMembershipPolicy,
)
from lp.registry.interfaces.accesspolicy import (
@@ -957,7 +958,7 @@
title=None, summary=None, official_malone=None,
translations_usage=None, bug_supervisor=None, private_bugs=False,
driver=None, icon=None, bug_sharing_policy=None,
- branch_sharing_policy=None):
+ branch_sharing_policy=None, specification_sharing_policy=None):
"""Create and return a new, arbitrary Product."""
if owner is None:
owner = self.makePerson()
@@ -1001,12 +1002,18 @@
if ((branch_sharing_policy and
branch_sharing_policy != BranchSharingPolicy.PUBLIC) or
(bug_sharing_policy and
- bug_sharing_policy != BugSharingPolicy.PUBLIC)):
+ bug_sharing_policy != BugSharingPolicy.PUBLIC) or
+ (specification_sharing_policy and
+ specification_sharing_policy !=
+ SpecificationSharingPolicy.PUBLIC)):
self.makeCommercialSubscription(product)
if branch_sharing_policy:
naked_product.setBranchSharingPolicy(branch_sharing_policy)
if bug_sharing_policy:
naked_product.setBugSharingPolicy(bug_sharing_policy)
+ if specification_sharing_policy:
+ naked_product.setSpecificationSharingPolicy(
+ specification_sharing_policy)
return product
Follow ups
