launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01587
[Merge] lp:~bac/launchpad/bug-652149 into lp:launchpad/devel
Brad Crittenden has proposed merging lp:~bac/launchpad/bug-652149 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#652149 Branch visibility status and link missing on project group front page
https://bugs.launchpad.net/bugs/652149
= Summary =
The code view for a project group did not show the default branch rules.
It did have a link for 'Define branch visibility' but the permission on
it was wrong, so commercial admins did not see it.
== Proposed fix ==
Convert the template to main_side, add a portlet for the privacy setting
display and a portlet for the link to define branch visibility.
== Pre-implementation notes ==
Brief chat with Curtis.
== Implementation details ==
I included some drive-by fixes. Removed redundancy from
lp.testing.sampledata and cleaned up the page template for
product-branches to not use conditional paragraphs.
== Tests ==
bin/test -vvm lp.code -t TestProjectGroupBranchesPage
== Demo and Q/A ==
Visit a project group such as https://launchpad.dev/mozilla and
determine everything is in order.
= Launchpad lint =
Bah, I'll check to ensure the real lint issues are fixed.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/code/templates/project-branches.pt
lib/lp/code/templates/product-branches.pt
lib/lp/code/browser/tests/test_branchlisting.py
lib/lp/testing/sampledata.py
lib/lp/code/stories/branches/xx-branch-visibility-policy.txt
lib/lp/registry/browser/project.py
lib/lp/code/browser/branchlisting.py
./lib/lp/code/stories/branches/xx-branch-visibility-policy.txt
1: narrative uses a moin header.
67: source exceeds 78 characters.
94: narrative uses a moin header.
104: source exceeds 78 characters.
107: source exceeds 78 characters.
125: source exceeds 78 characters.
126: want exceeds 78 characters.
134: source exceeds 78 characters.
138: source exceeds 78 characters.
146: narrative uses a moin header.
198: narrative uses a moin header.
231: source exceeds 78 characters.
253: source exceeds 78 characters.
./lib/lp/code/browser/branchlisting.py
1388: E302 expected 2 blank lines, found 1
--
https://code.launchpad.net/~bac/launchpad/bug-652149/+merge/38705
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-652149 into lp:launchpad/devel.
=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py 2010-10-08 20:16:11 +0000
+++ lib/lp/code/browser/branchlisting.py 2010-10-18 10:51:05 +0000
@@ -94,6 +94,7 @@
PersonActiveReviewsView,
PersonProductActiveReviewsView,
)
+from lp.code.browser.branchvisibilitypolicy import BranchVisibilityPolicyView
from lp.code.browser.summary import BranchCountSummaryView
from lp.code.enums import (
BranchLifecycleStatus,
@@ -532,7 +533,8 @@
return "listing sortable"
-class BranchListingView(LaunchpadFormView, FeedsMixin):
+class BranchListingView(LaunchpadFormView, FeedsMixin,
+ BranchVisibilityPolicyView):
"""A base class for views of branch listings."""
schema = IBranchListingFilter
field_names = ['lifecycle', 'sort_by']
@@ -1383,7 +1385,6 @@
'sort_by': BranchListingSort.LIFECYCLE,
}
-
class ProjectBranchesView(BranchListingView):
"""View for branch listing for a project."""
=== modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
--- lib/lp/code/browser/tests/test_branchlisting.py 2010-08-24 02:21:50 +0000
+++ lib/lp/code/browser/tests/test_branchlisting.py 2010-10-18 10:51:05 +0000
@@ -33,11 +33,15 @@
GroupedDistributionSourcePackageBranchesView,
SourcePackageBranchesView,
)
+from lp.code.enums import BranchVisibilityRule
from lp.code.interfaces.seriessourcepackagebranch import (
IMakeOfficialBranchLinks,
)
from lp.code.model.branch import Branch
-from lp.registry.interfaces.person import PersonVisibility
+from lp.registry.interfaces.person import (
+ IPersonSet,
+ PersonVisibility,
+ )
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.model.person import Owner
from lp.registry.model.product import Product
@@ -50,6 +54,10 @@
time_counter,
)
from lp.testing.factory import remove_security_proxy_and_shout_at_engineer
+from lp.testing.sampledata import (
+ ADMIN_EMAIL,
+ COMMERCIAL_ADMIN_EMAIL,
+ )
from lp.testing.views import create_initialized_view
@@ -421,5 +429,71 @@
self.assertIs(None, branches)
+class TestProjectGroupBranchesPage(BrowserTestCase):
+ """Test for the project group branches page.
+
+ This is the default page shown for a person on the code subdomain.
+ """
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ BrowserTestCase.setUp(self)
+ self.project = self.factory.makeProject(owner=self.user)
+
+ def test_project_with_no_branch_visibility_rule(self):
+ browser = self.getUserBrowser(
+ canonical_url(self.project, rootsite='code'))
+ privacy_portlet = find_tag_by_id(browser.contents, 'privacy-portlet')
+ text = extract_text(privacy_portlet)
+ expected = ("By default, new branches you create for projects in .* "
+ "are Public initially. Individual "
+ "projects may override this setting.")
+ self.assertTextMatchesExpressionIgnoreWhitespace(
+ expected, text)
+
+ def test_project_with_private_branch_visibility_rule(self):
+ self.project.setBranchVisibilityTeamPolicy(
+ None, BranchVisibilityRule.FORBIDDEN)
+ browser = self.getUserBrowser(
+ canonical_url(self.project, rootsite='code'))
+ privacy_portlet = find_tag_by_id(browser.contents, 'privacy-portlet')
+ text = extract_text(privacy_portlet)
+ expected = ("By default, new branches you create for projects in .* "
+ "are Forbidden initially. Individual "
+ "projects may override this setting.")
+ self.assertTextMatchesExpressionIgnoreWhitespace(
+ expected, text)
+
+ def _testBranchVisibilityLink(self, user):
+ browser = self.getUserBrowser(
+ url=canonical_url(self.project, rootsite='code'),
+ user=user)
+ action_portlet = find_tag_by_id(browser.contents, 'action-portlet')
+ text = extract_text(action_portlet)
+ expected = '.*Define branch visibility.*'
+ self.assertTextMatchesExpressionIgnoreWhitespace(
+ expected, text)
+
+ def test_branch_visibility_link_admin(self):
+ # An admin will be displayed a link to define branch visibility in the
+ # action portlet.
+ admin = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+ self._testBranchVisibilityLink(admin)
+
+ def test_branch_visibility_link_commercial_admin(self):
+ # An commercial admin will be displayed a link to define branch
+ # visibility in the action portlet.
+ admin = getUtility(IPersonSet).getByEmail(COMMERCIAL_ADMIN_EMAIL)
+ self._testBranchVisibilityLink(admin)
+
+ def test_branch_visibility_link_non_admin(self):
+ # A non-admin will not see the action portlet.
+ browser = self.getUserBrowser(
+ url=canonical_url(self.project, rootsite='code'))
+ action_portlet = find_tag_by_id(browser.contents, 'action-portlet')
+ self.assertIs(None, action_portlet)
+
+
def test_suite():
return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/code/stories/branches/xx-branch-visibility-policy.txt'
--- lib/lp/code/stories/branches/xx-branch-visibility-policy.txt 2009-07-20 18:22:54 +0000
+++ lib/lp/code/stories/branches/xx-branch-visibility-policy.txt 2010-10-18 10:51:05 +0000
@@ -1,7 +1,7 @@
= Branch Visibility Policy Pages =
Controlling the branch visibility policies for products and projects is only
-available to launchpad admins and launchpad commercial admins.
+available to Launchpad admins and Launchpad commercial admins.
Not to anonymous people.
@@ -68,6 +68,28 @@
>>> print commercial_browser.url
http://launchpad.dev/firefox/+addbranchvisibilitypolicy
+Admins can define branch visibility on projects, too.
+
+ >>> admin_browser.open('http://code.launchpad.dev/mozilla')
+ >>> admin_browser.getLink('Define branch visibility').click()
+ >>> print admin_browser.url
+ http://launchpad.dev/mozilla/+branchvisibility
+
+ >>> admin_browser.getLink('Set policy for a team').click()
+ >>> print admin_browser.url
+ http://launchpad.dev/mozilla/+addbranchvisibilitypolicy
+
+As can commercial admins.
+
+ >>> commercial_browser.open('http://code.launchpad.dev/mozilla')
+ >>> commercial_browser.getLink('Define branch visibility').click()
+ >>> print commercial_browser.url
+ http://launchpad.dev/mozilla/+branchvisibility
+
+ >>> commercial_browser.getLink('Set policy for a team').click()
+ >>> print commercial_browser.url
+ http://launchpad.dev/mozilla/+addbranchvisibilitypolicy
+
== Default policies ==
=== modified file 'lib/lp/code/templates/product-branches.pt'
--- lib/lp/code/templates/product-branches.pt 2010-10-08 15:57:37 +0000
+++ lib/lp/code/templates/product-branches.pt 2010-10-18 10:51:05 +0000
@@ -22,16 +22,10 @@
tal:define="are_private view/new_branches_are_private"
tal:attributes="class python: are_private and 'first portlet private' or 'first portlet public'">
- <p tal:condition="not:view/new_branches_are_private" id="privacy-text">
- New branches you create for <tal:name replace="context/displayname"/>
- are <strong>public</strong> initially.
- </p>
-
- <p tal:condition="view/new_branches_are_private" id="privacy-text">
- New branches you create for <tal:name replace="context/displayname"/>
- are <strong>private</strong> initially.
- </p>
-
+ <p id="privacy-text">
+ New branches you create for <tal:name replace="context/displayname"/>
+ are <strong tal:content="view/base_visibility_rule/title">Public</strong> initially.
+ </p>
</div>
<div id="involvement" class="portlet"
=== modified file 'lib/lp/code/templates/project-branches.pt'
--- lib/lp/code/templates/project-branches.pt 2009-09-17 00:27:40 +0000
+++ lib/lp/code/templates/project-branches.pt 2010-10-18 10:51:05 +0000
@@ -3,21 +3,41 @@
xmlns:tal="http://xml.zope.org/namespaces/tal";
xmlns:metal="http://xml.zope.org/namespaces/metal";
xmlns:i18n="http://xml.zope.org/namespaces/i18n";
- metal:use-macro="view/macro:page/main_only"
+ metal:use-macro="view/macro:page/main_side"
i18n:domain="launchpad">
<body>
+ <metal:side fill-slot="side"
+ tal:define="context_menu context/menu:context">
+ <div id="branch-portlet"
+ tal:condition="not: context/codehosting_usage/enumvalue:UNKNOWN">
+ <div id="privacy-portlet"
+ tal:define="base_vis_rule view/base_visibility_rule;
+ are_private not:base_vis_rule/enumvalue:PUBLIC"
+ tal:attributes="class python: are_private and 'first portlet private' or 'first portlet public'">
+
+ <p id="privacy-text">
+ By default, new branches you create for projects in <tal:name replace="context/displayname"/>
+ are <strong tal:content="view/base_visibility_rule/title">Public</strong>
+ initially. Individual projects may override this setting.
+ </p>
+
+ </div>
+ </div>
+
+ <div id="action-portlet"
+ class="portlet"
+ tal:define="menu context/menu:overview;
+ link menu/branch_visibility"
+ tal:condition="link/enabled">
+ <div tal:content="structure link/render" />
+ </div>
+ </metal:side>
+
<div metal:fill-slot="main"
tal:define="branches view/branches">
- <div style="float:right" id="floating-links"
- tal:define="menu context/menu:overview">
- <div tal:define="link menu/branch_visibility"
- tal:condition="link/enabled"
- tal:content="structure link/render" />
- </div>
-
<tal:branchlisting content="structure branches/@@+branch-listing" />
</div>
=== modified file 'lib/lp/registry/browser/project.py'
--- lib/lp/registry/browser/project.py 2010-09-23 03:17:10 +0000
+++ lib/lp/registry/browser/project.py 2010-10-18 10:51:05 +0000
@@ -254,7 +254,7 @@
'RDF</abbr> metadata')
return Link('+rdf', text, icon='download-icon')
- @enabled_with_permission('launchpad.Admin')
+ @enabled_with_permission('launchpad.Commercial')
def branch_visibility(self):
text = 'Define branch visibility'
return Link('+branchvisibility', text, icon='edit', site='mainsite')
=== modified file 'lib/lp/testing/sampledata.py'
--- lib/lp/testing/sampledata.py 2010-08-30 17:05:49 +0000
+++ lib/lp/testing/sampledata.py 2010-10-18 10:51:05 +0000
@@ -58,7 +58,6 @@
USER_EMAIL = 'test@xxxxxxxxxxxxx'
VCS_IMPORTS_MEMBER_EMAIL = 'david.allouche@xxxxxxxxxxxxx'
COMMERCIAL_ADMIN_EMAIL = 'commercial-member@xxxxxxxxxxxxx'
-ADMIN_EMAIL = 'foo.bar@xxxxxxxxxxxxx'
SAMPLE_PERSON_EMAIL = USER_EMAIL
# A user that is an admin of ubuntu-team, which has upload rights
# to Ubuntu.
