launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11629
[Merge] lp:~wallyworld/launchpad/default-distro-policies-1041002 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/default-distro-policies-1041002 into lp:launchpad.
Requested reviews:
Curtis Hovey (sinzui)
Related bugs:
Bug #1041002 in Launchpad itself: "distributions are missing sharing polcies"
https://bugs.launchpad.net/launchpad/+bug/1041002
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/default-distro-policies-1041002/+merge/122802
== Implementation ==
Add bug and branch sharing policies to distributions, but hard code them to PUBLIC. A new interface is introduced - IHasSharingPolicies. Products get the attribute values from the database, distros from properties on the class. These attributes are used to feed the data model for the view using a common code base.
Aspects of the +sharing view were fixed so that they display the correct information for distros. eg usages of the text "This product..." were replaced with the pillar name, and product specific text about subscriptions was removed.
== Demo ==
Here's what the sharing page looks like for distros now:
http://people.canonical.com/~ianb/distro-sharing-page.png
== Tests ==
Update pillar sharing view and grantee table yui tests.
Add new tests to test_pillar_sharing to test the text on the sharing view.
Update tests which check the distro sharing policy values.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/tests/test_pillar_sharing.py
lib/lp/registry/interfaces/distribution.py
lib/lp/registry/interfaces/pillar.py
lib/lp/registry/javascript/sharing/granteetable.js
lib/lp/registry/javascript/sharing/pillarsharingview.js
lib/lp/registry/javascript/sharing/tests/test_granteetable.js
lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js
lib/lp/registry/model/distribution.py
lib/lp/registry/services/sharingservice.py
lib/lp/registry/services/tests/test_sharingservice.py
lib/lp/registry/templates/pillar-sharing.pt
lib/lp/registry/tests/test_distribution.py
--
https://code.launchpad.net/~wallyworld/launchpad/default-distro-policies-1041002/+merge/122802
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-09-03 22:08:40 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-09-05 07:18:19 +0000
@@ -333,6 +333,31 @@
# printed to stdout while browsing pages.
self.useFixture(FakeLogger())
+ def test_view_contents_non_commercial_project(self):
+ # Non commercial projects are rendered with the correct text.
+ url = canonical_url(self.pillar, view_name='+sharing')
+ browser = setupBrowserForUser(user=self.driver)
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ commercial_text = soup.find('p', {'id': 'commercial-project-text'})
+ non_commercial_text = soup.find(
+ 'p', {'id': 'non-commercial-project-text'})
+ self.assertIsNone(commercial_text)
+ self.assertIsNotNone(non_commercial_text)
+
+ def test_view_contents_commercial_project(self):
+ # Commercial projects are rendered with the correct text.
+ self.factory.makeCommercialSubscription(self.pillar)
+ url = canonical_url(self.pillar, view_name='+sharing')
+ browser = setupBrowserForUser(user=self.driver)
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ commercial_text = soup.find('p', {'id': 'commercial-project-text'})
+ non_commercial_text = soup.find(
+ 'p', {'id': 'non-commercial-project-text'})
+ self.assertIsNotNone(commercial_text)
+ self.assertIsNone(non_commercial_text)
+
class TestDistributionSharingView(PillarSharingViewTestMixin,
SharingBaseTestCase):
@@ -344,3 +369,15 @@
super(TestDistributionSharingView, self).setUp()
self.setupSharing(self.grantees)
login_person(self.driver)
+
+ def test_view_contents(self):
+ # Distributions are rendered with the correct text.
+ url = canonical_url(self.pillar, view_name='+sharing')
+ browser = setupBrowserForUser(user=self.driver)
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ commercial_text = soup.find('p', {'id': 'commercial-project-text'})
+ non_commercial_text = soup.find(
+ 'p', {'id': 'non-commercial-project-text'})
+ self.assertIsNone(commercial_text)
+ self.assertIsNone(non_commercial_text)
=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py 2012-09-02 23:01:19 +0000
+++ lib/lp/registry/interfaces/distribution.py 2012-09-05 07:18:19 +0000
@@ -81,7 +81,10 @@
IHasMilestones,
)
from lp.registry.interfaces.oopsreferences import IHasOOPSReferences
-from lp.registry.interfaces.pillar import IPillar
+from lp.registry.interfaces.pillar import (
+ IHasSharingPolicies,
+ IPillar,
+ )
from lp.registry.interfaces.role import (
IHasAppointedDriver,
IHasDrivers,
@@ -131,7 +134,7 @@
class IDistributionPublic(
IBugTarget, ICanGetMilestonesDirectly, IHasAppointedDriver,
- IHasBuildRecords, IHasDrivers, IHasMilestones,
+ IHasBuildRecords, IHasDrivers, IHasMilestones, IHasSharingPolicies,
IHasOOPSReferences, IHasOwner, IHasSprints, IHasTranslationImports,
ITranslationPolicy, IKarmaContext, ILaunchpadUsage, IMakesAnnouncements,
IOfficialBugTagTargetPublic, IPillar, IServiceUsage,
=== modified file 'lib/lp/registry/interfaces/pillar.py'
--- lib/lp/registry/interfaces/pillar.py 2012-03-09 19:45:15 +0000
+++ lib/lp/registry/interfaces/pillar.py 2012-09-05 07:18:19 +0000
@@ -27,16 +27,22 @@
)
from zope.schema import (
Bool,
+ Choice,
Int,
List,
TextLine,
)
from lp import _
+from lp.registry.enums import (
+ BranchSharingPolicy,
+ BugSharingPolicy,
+ )
__all__ = [
'IHasAliases',
+ 'IHasSharingPolicies',
'IPillar',
'IPillarName',
'IPillarNameSet',
@@ -82,6 +88,20 @@
"""
+class IHasSharingPolicies(Interface):
+ """Sharing policies used to define bug and branch visibility rules."""
+ branch_sharing_policy = exported(Choice(
+ title=_('Branch sharing policy'),
+ description=_("Sharing policy for this pillar's branches."),
+ required=False, readonly=True, vocabulary=BranchSharingPolicy),
+ as_of='devel')
+ bug_sharing_policy = exported(Choice(
+ title=_('Bug sharing policy'),
+ description=_("Sharing policy for this pillar's bugs."),
+ required=False, readonly=True, vocabulary=BugSharingPolicy),
+ as_of='devel')
+
+
class IPillarName(Interface):
"""A data structure for identifying a pillar.
=== modified file 'lib/lp/registry/javascript/sharing/granteetable.js'
--- lib/lp/registry/javascript/sharing/granteetable.js 2012-08-14 15:57:33 +0000
+++ lib/lp/registry/javascript/sharing/granteetable.js 2012-09-05 07:18:19 +0000
@@ -31,6 +31,10 @@
anim_duration: {
value: 1
},
+ // The display name of the pillar.
+ pillar_name: {
+ value: null
+ },
// The list of grantees to display.
grantees: {
value: [],
@@ -194,12 +198,14 @@
},
_grantee_table_empty_row: function() {
- return [
+ var row_template = [
'<tr id="grantee-table-not-shared">',
'<td colspan="5" style="padding-left: 0.25em">',
- 'This project\'s private information is not shared with ',
+ '{pillar_name}\'s private information is not shared with ',
'anyone.',
'</td></tr>'].join('');
+ return Y.Lang.sub(
+ row_template, {pillar_name: this.get('pillar_name')});
},
_grantee_policy_template: function() {
=== modified file 'lib/lp/registry/javascript/sharing/pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-09-04 00:10:23 +0000
+++ lib/lp/registry/javascript/sharing/pillarsharingview.js 2012-09-05 07:18:19 +0000
@@ -111,6 +111,7 @@
var grantee_data = LP.cache.grantee_data;
var otns = Y.lp.registry.sharing.granteetable;
var grantee_table = new otns.GranteeTableWidget({
+ pillar_name: LP.cache.context.display_name,
grantees: grantee_data,
sharing_permissions:
this.get('sharing_permissions_by_value'),
@@ -141,6 +142,7 @@
'#' + artifact_type + '-sharing-policy');
var value_location = contentBox.one('.value');
var editicon = contentBox.one('a.editicon');
+ editicon.removeClass('hidden');
var choice_items = [];
if (!Y.Lang.isValue(current_value)) {
choice_items.push({
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_granteetable.js'
--- lib/lp/registry/javascript/sharing/tests/test_granteetable.js 2012-08-14 15:57:33 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_granteetable.js 2012-09-05 07:18:19 +0000
@@ -56,6 +56,7 @@
overrides = {};
}
var config = Y.merge({
+ pillar_name: 'My Project',
grantee_table: Y.one('#grantee-table'),
anim_duration: 0,
grantees: window.LP.cache.grantee_data,
@@ -95,14 +96,15 @@
});
},
- // When there are no grantees, the table contains an informative message.
+ // When there are no grantees, the table contains an informative
+ // message.
test_no_grantee_message: function() {
this.grantee_table = this._create_Widget({
grantees: []
});
this.grantee_table.render();
Y.Assert.areEqual(
- "This project's private information is not shared " +
+ "My Project's private information is not shared " +
"with anyone.",
Y.one('#grantee-table tr td').getContent());
},
@@ -354,7 +356,7 @@
this.grantee_table.syncUI();
// Check the results.
Y.Assert.areEqual(
- "This project's private information is not shared " +
+ "My Project's private information is not shared " +
"with anyone.",
Y.one('#grantee-table tr#grantee-table-not-shared td')
.getContent());
=== modified file 'lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-09-03 23:40:59 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_pillarsharingview.js 2012-09-05 07:18:19 +0000
@@ -98,7 +98,7 @@
Y.Assert.isInstanceOf(
Y.lp.registry.sharing.pillarsharingview.PillarSharingView,
this.view,
- "Grantee table failed to be instantiated");
+ "Pillar sharing view failed to be instantiated");
// Check the picker config.
var grantee_picker = this.view.get('grantee_picker');
Y.Assert.areEqual(
@@ -111,6 +111,10 @@
test_render: function() {
this.view = this._create_Widget();
this.view.render();
+ // Check the grantee table config.
+ Y.Assert.areEqual(
+ this.view.get('grantee_table').get('pillar_name'),
+ 'Pillar');
// The grantee table - we'll just check one row
Y.Assert.isNotNull(
Y.one('#grantee-table tr[id=permission-fred]'));
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2012-09-03 01:35:58 +0000
+++ lib/lp/registry/model/distribution.py 2012-09-05 07:18:19 +0000
@@ -91,6 +91,8 @@
IFindOfficialBranchLinks,
)
from lp.registry.enums import (
+ BranchSharingPolicy,
+ BugSharingPolicy,
FREE_INFORMATION_TYPES,
InformationType,
PRIVATE_INFORMATION_TYPES,
@@ -280,6 +282,18 @@
return "Distribution"
@property
+ def branch_sharing_policy(self):
+ """See `IHasSharingPolicies."""
+ # Sharing policy for distributions is always PUBLIC.
+ return BranchSharingPolicy.PUBLIC
+
+ @property
+ def bug_sharing_policy(self):
+ """See `IHasSharingPolicies."""
+ # Sharing policy for distributions is always PUBLIC.
+ return BugSharingPolicy.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-04 22:25:36 +0000
+++ lib/lp/registry/services/sharingservice.py 2012-09-05 07:18:19 +0000
@@ -266,12 +266,12 @@
def getBranchSharingPolicies(self, pillar):
"""See `ISharingService`."""
- # Only Products have branch sharing policies.
- if not IProduct.providedBy(pillar):
- return []
+ # Only Products have branch sharing policies. Distributions just
+ # default to Public.
allowed_policies = [BranchSharingPolicy.PUBLIC]
# Commercial projects also allow proprietary branches.
- if pillar.has_current_commercial_subscription:
+ if (IProduct.providedBy(pillar)
+ and pillar.has_current_commercial_subscription):
allowed_policies.extend([
BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
@@ -284,12 +284,12 @@
def getBugSharingPolicies(self, pillar):
"""See `ISharingService`."""
- # Only Products have bug sharing policies.
- if not IProduct.providedBy(pillar):
- return []
+ # Only Products have bug sharing policies. Distributions just
+ # default to Public.
allowed_policies = [BugSharingPolicy.PUBLIC]
# Commercial projects also allow proprietary bugs.
- if pillar.has_current_commercial_subscription:
+ if (IProduct.providedBy(pillar)
+ and pillar.has_current_commercial_subscription):
allowed_policies.extend([
BugSharingPolicy.PUBLIC_OR_PROPRIETARY,
BugSharingPolicy.PROPRIETARY_OR_PUBLIC,
=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py 2012-09-04 00:02:07 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2012-09-05 07:18:19 +0000
@@ -201,7 +201,8 @@
def test_getBranchSharingPolicies_distro(self):
distro = self.factory.makeDistribution()
- self._assert_getBranchSharingPolicies(distro, [])
+ self._assert_getBranchSharingPolicies(
+ distro, [BranchSharingPolicy.PUBLIC])
def _assert_getBugSharingPolicies(self, pillar, expected_policies):
policy_data = self.service.getBugSharingPolicies(pillar)
@@ -228,7 +229,7 @@
def test_getBugSharingPolicies_distro(self):
distro = self.factory.makeDistribution()
- self._assert_getBugSharingPolicies(distro, [])
+ self._assert_getBugSharingPolicies(distro, [BugSharingPolicy.PUBLIC])
def test_jsonGranteeData_with_Some(self):
# jsonGranteeData returns the expected data for a grantee with
=== modified file 'lib/lp/registry/templates/pillar-sharing.pt'
--- lib/lp/registry/templates/pillar-sharing.pt 2012-09-03 23:40:59 +0000
+++ lib/lp/registry/templates/pillar-sharing.pt 2012-09-05 07:18:19 +0000
@@ -25,18 +25,22 @@
<body>
<div metal:fill-slot="main">
<h2>What's shared</h2>
- <p tal:condition="context/has_current_commercial_subscription|nothing">
+ <tal:what-shared condition="python:context.pillar_category == 'Project'">
+ <p id='commercial-project-text'
+ tal:condition="context/has_current_commercial_subscription|nothing">
You can choose whether new bugs and branches are public or
proprietary by default, and what you can change them to
afterwards. Changing the default here won't automatically change
existing bugs or branches.
</p>
- <p tal:condition="not: context/has_current_commercial_subscription|nothing">
+ <p id='non-commercial-project-text'
+ tal:condition="not: context/has_current_commercial_subscription|nothing">
Open source projects have limited access to sharing features. You can
<a href="https://help.launchpad.net/CommercialHosting";>purchase a
commercial subscription</a> to enable more privacy options, such as
setting bugs and branches to be proprietary by default.
</p>
+ </tal:what-shared>
<table>
<tr id="branch-sharing-policy-row"
tal:condition="view/branch_sharing_policies">
@@ -45,7 +49,7 @@
<strong><span class="value"
tal:content="context/branch_sharing_policy/title|string:Legacy policy"
></span></strong>
- <a class="editicon sprite edit action-icon" href="#"
+ <a class="editicon sprite edit action-icon hidden" href="#"
style="padding-bottom: 0;">Edit</a>
<div id="branch-sharing-policy-description" class="formHelp"
tal:content="context/branch_sharing_policy/description|
@@ -61,7 +65,7 @@
<strong><span class="value"
tal:content="context/bug_sharing_policy/title|string:Legacy policy"
></span></strong>
- <a class="editicon sprite edit action-icon" href="#"
+ <a class="editicon sprite edit action-icon hidden" href="#"
style="padding-bottom: 0;">Edit</a>
<div id="bug-sharing-policy-description" class="formHelp"
tal:content="context/bug_sharing_policy/description|
@@ -73,9 +77,9 @@
</table>
<h2>Who it's shared with</h2>
<div id="sharing-header">
- <p>
- Everyone can see your project's public information. You can
- choose who can see the private bugs and branches.
+ <p tal:content="string:
+ Everyone can see ${context/displayname}'s public information. You can
+ choose who can see the private bugs and branches.">
</p>
</div>
<ul class="horizontal">
=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py 2012-09-03 02:27:05 +0000
+++ lib/lp/registry/tests/test_distribution.py 2012-09-05 07:18:19 +0000
@@ -27,6 +27,8 @@
from lp.app.errors import NotFoundError
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.registry.enums import (
+ BranchSharingPolicy,
+ BugSharingPolicy,
EXCLUSIVE_TEAM_POLICY,
INCLUSIVE_TEAM_POLICY,
InformationType,
@@ -77,6 +79,14 @@
distro = self.factory.makeDistribution()
self.assertEqual("Distribution", distro.pillar_category)
+ def test_sharing_policies(self):
+ # The sharing policies are PUBLIC.
+ distro = self.factory.makeDistribution()
+ self.assertEqual(
+ BranchSharingPolicy.PUBLIC, distro.branch_sharing_policy)
+ self.assertEqual(
+ BugSharingPolicy.PUBLIC, distro.bug_sharing_policy)
+
def test_owner_cannot_be_open_team(self):
"""Distro owners cannot be open teams."""
for policy in INCLUSIVE_TEAM_POLICY:
Follow ups
