Merge lp:~stevenk/launchpad/redirect-unsub-private-branch into lp:launchpad

Proposed by Steve Kowalik on 2012-08-09
Status: Merged
Approved by: Steve Kowalik on 2012-08-09
Approved revision: no longer in the source branch.
Merged at revision: 15778
Proposed branch: lp:~stevenk/launchpad/redirect-unsub-private-branch
Merge into: lp:launchpad
Diff against target: 99 lines (+57/-1)
2 files modified
lib/lp/code/browser/branchsubscription.py (+10/-1)
lib/lp/code/browser/tests/test_branch.py (+47/-0)
To merge this branch: bzr merge lp:~stevenk/launchpad/redirect-unsub-private-branch
Reviewer Review Type Date Requested Status
Ian Booth (community) 2012-08-09 Approve on 2012-08-09
Review via email: mp+118859@code.launchpad.net

Commit Message

If the person unsubscribing themselves from a branch no longer has access to it, redirect them to branch's target canonical_url.

Description of the Change

If the person unsubscribing themselves from a branch no longer has access to it, redirect them to branch's target canonical_url. Add two tests for this case -- one where the subscriber has an APG, so doesn't lose access, and one where the subscriber does not, and so does lose access.

To post a comment you must log in.
Ian Booth (wallyworld) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/browser/branchsubscription.py'
2--- lib/lp/code/browser/branchsubscription.py 2012-06-21 23:00:20 +0000
3+++ lib/lp/code/browser/branchsubscription.py 2012-08-09 02:14:18 +0000
4@@ -13,6 +13,7 @@
5 ]
6
7 from lazr.restful.utils import smartquote
8+from zope.component import getUtility
9 from zope.interface import implements
10
11 from lp.app.browser.launchpadform import (
12@@ -20,6 +21,7 @@
13 LaunchpadEditFormView,
14 LaunchpadFormView,
15 )
16+from lp.app.interfaces.services import IService
17 from lp.code.enums import BranchSubscriptionNotificationLevel
18 from lp.code.interfaces.branchsubscription import IBranchSubscription
19 from lp.services.webapp import (
20@@ -289,6 +291,13 @@
21
22 @property
23 def next_url(self):
24- return canonical_url(self.branch)
25+ url = canonical_url(self.branch)
26+ # If the subscriber can no longer see the branch, redirect them away.
27+ service = getUtility(IService, 'sharing')
28+ ignored, branches = service.getVisibleArtifacts(
29+ self.person, branches=[self.branch])
30+ if not branches:
31+ url = canonical_url(self.branch.target)
32+ return url
33
34 cancel_url = next_url
35
36=== modified file 'lib/lp/code/browser/tests/test_branch.py'
37--- lib/lp/code/browser/tests/test_branch.py 2012-07-27 03:51:46 +0000
38+++ lib/lp/code/browser/tests/test_branch.py 2012-08-09 02:14:18 +0000
39@@ -36,6 +36,7 @@
40 BranchVisibilityRule,
41 )
42 from lp.registry.enums import InformationType
43+from lp.registry.interfaces.accesspolicy import IAccessPolicySource
44 from lp.registry.interfaces.person import PersonVisibility
45 from lp.services.config import config
46 from lp.services.database.constants import UTC_NOW
47@@ -653,6 +654,52 @@
48 reviews_list = soup.find('dl', attrs={'class': 'reviews'})
49 self.assertIsNone(reviews_list.find('a', text='Privateteam'))
50
51+ def test_unsubscribe_private_branch(self):
52+ # Unsubscribing from a branch with a policy grant still allows the
53+ # branch to be seen.
54+ product = self.factory.makeProduct()
55+ owner = self.factory.makePerson()
56+ subscriber = self.factory.makePerson()
57+ [ap] = getUtility(IAccessPolicySource).find(
58+ [(product, InformationType.USERDATA)])
59+ self.factory.makeAccessPolicyGrant(
60+ policy=ap, grantee=subscriber, grantor=product.owner)
61+ branch = self.factory.makeBranch(
62+ product=product, owner=owner,
63+ information_type=InformationType.USERDATA)
64+ with person_logged_in(owner):
65+ self.factory.makeBranchSubscription(branch, subscriber, owner)
66+ base_url = canonical_url(branch, rootsite='code')
67+ expected_title = '%s : Code : %s' % (
68+ branch.name, product.displayname)
69+ url = '%s/+subscription/%s' % (base_url, subscriber.name)
70+ browser = self._getBrowser(user=subscriber)
71+ browser.open(url)
72+ browser.getControl('Unsubscribe').click()
73+ self.assertEqual(base_url, browser.url)
74+ self.assertEqual(expected_title, browser.title)
75+
76+ def test_unsubscribe_private_branch_no_access(self):
77+ # Unsubscribing from a branch with no access will redirect to the
78+ # context of the branch.
79+ product = self.factory.makeProduct()
80+ owner = self.factory.makePerson()
81+ subscriber = self.factory.makePerson()
82+ branch = self.factory.makeBranch(
83+ product=product, owner=owner,
84+ information_type=InformationType.USERDATA)
85+ with person_logged_in(owner):
86+ self.factory.makeBranchSubscription(branch, subscriber, owner)
87+ base_url = canonical_url(branch, rootsite='code')
88+ product_url = canonical_url(product, rootsite='code')
89+ url = '%s/+subscription/%s' % (base_url, subscriber.name)
90+ browser = self._getBrowser(user=subscriber)
91+ browser.open(url)
92+ browser.getControl('Unsubscribe').click()
93+ self.assertEqual(product_url, browser.url)
94+ expected_title = "Code : %s" % product.displayname
95+ self.assertEqual(expected_title, browser.title)
96+
97
98 class TestBranchReviewerEditView(TestCaseWithFactory):
99 """Test the BranchReviewerEditView view."""