launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05778
[Merge] lp:~wallyworld/launchpad/view-private-branch-artifacts-605130 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/view-private-branch-artifacts-605130 into lp:launchpad with lp:~wallyworld/launchpad/private-owned-entity-traversal as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #605130 in Launchpad itself: "subscribed team cannot view branch owned by (a different) private team"
https://bugs.launchpad.net/launchpad/+bug/605130
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/view-private-branch-artifacts-605130/+merge/84060
--
https://code.launchpad.net/~wallyworld/launchpad/view-private-branch-artifacts-605130/+merge/84060
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/view-private-branch-artifacts-605130 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugbranch.py'
--- lib/lp/bugs/browser/bugbranch.py 2011-09-13 05:23:16 +0000
+++ lib/lp/bugs/browser/bugbranch.py 2011-12-01 03:03:10 +0000
@@ -2,6 +2,7 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser view classes for BugBranch-related objects."""
+from canonical.launchpad.webapp.authorization import precache_permission_for_objects
__metaclass__ = type
__all__ = [
@@ -110,7 +111,12 @@
def merge_proposals(self):
"""Return a list of active proposals for the branch."""
branch = self.context.branch
- return latest_proposals_for_each_branch(branch.landing_targets)
+ proposals = latest_proposals_for_each_branch(branch.landing_targets)
+ if self.user is not None:
+ reviewers = [proposal.reviewer for proposal in proposals]
+ precache_permission_for_objects(
+ self.request, "launchpad.LimitedView", reviewers)
+ return proposals
@property
def show_branch_status(self):
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2011-09-28 11:30:52 +0000
+++ lib/lp/code/browser/branch.py 2011-12-01 03:03:10 +0000
@@ -2,6 +2,7 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Branch views."""
+from zope.security.proxy import removeSecurityProxy
__metaclass__ = type
@@ -96,7 +97,7 @@
stepthrough,
stepto,
)
-from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.authorization import check_permission, precache_permission_for_objects
from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
from canonical.launchpad.webapp.menu import structured
from lp.app.browser.launchpad import Hierarchy
@@ -442,6 +443,11 @@
def initialize(self):
self.branch = self.context
self.notices = []
+ authorised_people = [self.branch.owner]
+ if self.user is not None:
+ precache_permission_for_objects(
+ self.request, "launchpad.LimitedView", authorised_people)
+
# Replace our context with a decorated branch, if it is not already
# decorated.
if not isinstance(self.context, DecoratedBranch):
@@ -532,7 +538,15 @@
@cachedproperty
def landing_targets(self):
"""Return a filtered list of landing targets."""
- return latest_proposals_for_each_branch(self.context.landing_targets)
+ proposals = latest_proposals_for_each_branch(
+ self.context.landing_targets)
+ if self.user is not None:
+ reviewers = [
+ proposal.reviewer for proposal in proposals
+ if proposal.reviewer]
+ precache_permission_for_objects(
+ self.request, "launchpad.LimitedView", reviewers)
+ return proposals
@property
def latest_landing_candidates(self):
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2011-11-20 05:56:28 +0000
+++ lib/lp/code/browser/branchmergeproposal.py 2011-12-01 03:03:10 +0000
@@ -76,7 +76,7 @@
stepthrough,
stepto,
)
-from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.authorization import check_permission, precache_permission_for_objects
from canonical.launchpad.webapp.breadcrumb import Breadcrumb
from canonical.launchpad.webapp.interfaces import IPrimaryContext
from canonical.launchpad.webapp.menu import (
@@ -840,6 +840,12 @@
def reviews(self):
"""Return the decorated votes for the proposal."""
users_vote = self.context.getUsersVoteReference(self.user)
+ if self.user is not None:
+ reviewers = [
+ vote.reviewer for vote in self.context.votes
+ if vote.reviewer]
+ precache_permission_for_objects(
+ self.request, "launchpad.LimitedView", reviewers)
return [DecoratedCodeReviewVoteReference(vote, self.user, users_vote)
for vote in self.context.votes]
=== modified file 'lib/lp/code/browser/branchsubscription.py'
--- lib/lp/code/browser/branchsubscription.py 2011-09-19 06:57:55 +0000
+++ lib/lp/code/browser/branchsubscription.py 2011-12-01 03:03:10 +0000
@@ -19,7 +19,7 @@
canonical_url,
LaunchpadView,
)
-from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.authorization import check_permission, precache_permission_for_objects
from canonical.launchpad.webapp.interfaces import IPrimaryContext
from canonical.launchpad.webapp.menu import structured
from lp.app.browser.launchpadform import (
@@ -48,9 +48,16 @@
def subscriptions(self):
"""Return a decorated list of branch subscriptions."""
+ if self.user is not None:
+ subscribers = [
+ subscription.person
+ for subscription in self.context.subscriptions]
+ precache_permission_for_objects(
+ self.request, "launchpad.LimitedView", subscribers)
+
visible_subscriptions = [
subscription for subscription in self.context.subscriptions
- if check_permission('launchpad.View', subscription.person)]
+ if check_permission('launchpad.LimitedView', subscription.person)]
return sorted(
visible_subscriptions,
key=lambda subscription: subscription.person.displayname)
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2011-10-21 02:47:28 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2011-12-01 03:03:10 +0000
@@ -5,12 +5,14 @@
__metaclass__ = type
+from BeautifulSoup import BeautifulSoup
from datetime import (
datetime,
)
from textwrap import dedent
import pytz
+from zope.publisher.interfaces import NotFound
from zope.security.proxy import removeSecurityProxy
from canonical.config import config
@@ -26,7 +28,7 @@
extract_text,
find_tag_by_id,
setupBrowser,
- )
+ setupBrowserForUser)
from lp.app.interfaces.headings import IRootContext
from lp.bugs.interfaces.bugtask import (
BugTaskStatus,
@@ -50,6 +52,7 @@
BranchVisibilityRule,
)
from lp.code.interfaces.branchtarget import IBranchTarget
+from lp.registry.interfaces.person import PersonVisibility
from lp.testing import (
BrowserTestCase,
login,
@@ -543,6 +546,138 @@
self.assertEqual(linked_bug_urls[1], links[4]['href'])
+class TestBranchViewPrivateArtifacts(BrowserTestCase):
+ """ Tests that branches with private team artifacts can be viewed.
+
+ A Branch may be associated with a private team as follows:
+ - the owner
+ - a subscriber
+ - committed a revision
+ - be a code reviewer
+
+ A logged in user who is not authorised to see the private team(s) still
+ needs to be able to view the branch. The private team will be rendered in
+ the normal way, displaying the team name and Launchpad URL.
+ """
+
+ layer = DatabaseFunctionalLayer
+
+ def _getBrowser(self, user=None):
+ if user is None:
+ browser = setupBrowser()
+ logout()
+ return browser
+ else:
+ login_person(user)
+ return setupBrowserForUser(user=user)
+
+ def test_aaaa(self):
+ # A branch with a private owner is not rendered for anon users.
+ private_owner = self.factory.makeTeam(
+ visibility=PersonVisibility.PRIVATE)
+ # Viewing the branch results in an error.
+ url = canonical_url(removeSecurityProxy(private_owner))
+ user = self.factory.makePerson()
+ browser = self._getBrowser(user)
+ self.assertRaises(NotFound, browser.open, url)
+
+ def test_bbbb(self):
+ # A branch with a private owner is not rendered for anon users.
+ private_owner = self.factory.makeTeam(
+ visibility=PersonVisibility.PRIVATE)
+ branch = self.factory.makeAnyBranch(owner=private_owner)
+ # Viewing the branch results in an error.
+ url = canonical_url(branch, rootsite='code')
+ browser = self._getBrowser()
+ self.assertRaises(NotFound, browser.open, url)
+
+ def test_view_branch_with_private_owner(self):
+ # A branch with a private owner is rendered.
+ private_owner = self.factory.makeTeam(
+ displayname="PrivateTeam", visibility=PersonVisibility.PRIVATE)
+ branch = self.factory.makeAnyBranch(owner=private_owner)
+ # Ensure the branch owner is rendered.
+ url = canonical_url(branch, rootsite='code')
+ user = self.factory.makePerson()
+ browser = self._getBrowser(user)
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ self.assertIsNotNone(soup.find('a', text="PrivateTeam"))
+
+ def test_anonymous_view_branch_with_private_owner(self):
+ # A branch with a private owner is not rendered for anon users.
+ private_owner = self.factory.makeTeam(
+ visibility=PersonVisibility.PRIVATE)
+ branch = self.factory.makeAnyBranch(owner=private_owner)
+ # Viewing the branch results in an error.
+ url = canonical_url(branch, rootsite='code')
+ browser = self._getBrowser()
+ self.assertRaises(NotFound, browser.open, url)
+
+ def test_view_branch_with_private_subscriber(self):
+ # A branch with a private subscriber is rendered.
+ private_subscriber = self.factory.makeTeam(
+ name="privateteam", visibility=PersonVisibility.PRIVATE)
+ branch = self.factory.makeAnyBranch()
+ with person_logged_in(branch.owner):
+ self.factory.makeBranchSubscription(
+ branch, private_subscriber, branch.owner)
+ # Ensure the branch subscriber is rendered.
+ url = canonical_url(branch, rootsite='code')
+ user = self.factory.makePerson()
+ browser = self._getBrowser(user)
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ self.assertIsNotNone(
+ soup.find('div', attrs={'id': 'subscriber-privateteam'}))
+
+ def test_anonymous_view_branch_with_private_subscriber(self):
+ # A branch with a private subscriber is not rendered for anon users.
+ private_subscriber = self.factory.makeTeam(
+ name="privateteam", visibility=PersonVisibility.PRIVATE)
+ branch = self.factory.makeAnyBranch()
+ with person_logged_in(branch.owner):
+ self.factory.makeBranchSubscription(
+ branch, private_subscriber, branch.owner)
+ # Viewing the branch results in an error.
+ url = canonical_url(branch, rootsite='code')
+ browser = self._getBrowser()
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ self.assertIsNone(
+ soup.find('div', attrs={'id': 'subscriber-privateteam'}))
+
+ def test_view_branch_with_private_reviewer(self):
+ # A branch with a private reviewer is rendered.
+ private_reviewer = self.factory.makeTeam(
+ displayname="PrivateTeam", visibility=PersonVisibility.PRIVATE)
+ branch = self.factory.makeAnyBranch()
+ with person_logged_in(branch.owner):
+ self.factory.makeBranchMergeProposal(
+ source_branch=branch, reviewer=private_reviewer)
+ # Ensure the branch reviewer is rendered.
+ url = canonical_url(branch, rootsite='code')
+ user = self.factory.makePerson()
+ browser = self._getBrowser(user)
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ self.assertIsNotNone(
+ soup.find('a', text="PrivateTeam"))
+
+ def test_anonymous_view_branch_with_private_reviewer(self):
+ # A branch with a private reviewer is not rendered for anon users.
+ private_reviewer = self.factory.makeTeam(
+ visibility=PersonVisibility.PRIVATE)
+ branch = self.factory.makeAnyBranch()
+ with person_logged_in(branch.owner):
+ self.factory.makeBranchMergeProposal(
+ source_branch=branch, reviewer=private_reviewer)
+ # Viewing the branch results in an error.
+ url = canonical_url(branch, rootsite='code')
+ browser = self._getBrowser()
+ self.assertRaises(NotFound, browser.open, url)
+
+
class TestBranchAddView(TestCaseWithFactory):
"""Test the BranchAddView view."""
Follow ups
