Merge lp:~gary/launchpad/bug770287 into lp:launchpad

Proposed by Gary Poster
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 12919
Proposed branch: lp:~gary/launchpad/bug770287
Merge into: lp:launchpad
Diff against target: 126 lines (+33/-17)
2 files modified
lib/lp/bugs/browser/structuralsubscription.py (+9/-10)
lib/lp/registry/browser/tests/test_subscription_links.py (+24/-7)
To merge this branch: bzr merge lp:~gary/launchpad/bug770287
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+58979@code.launchpad.net

Commit message

[r=benji][bug=770287] If there is no bugtracker on a product, do not render links, even if the parent project has a bugtracker

Description of the change

This branch fixes an edge case, as described in https://bugs.edge.launchpad.net/launchpad/+bug/770287 and the first comment.

Using the helper seemed the most elegant approach: the helper's "pillar" was exactly the value we wanted.

lint is happy, after rearranging some comments.

Thank you.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
2--- lib/lp/bugs/browser/structuralsubscription.py 2011-04-21 21:51:33 +0000
3+++ lib/lp/bugs/browser/structuralsubscription.py 2011-04-25 18:15:54 +0000
4@@ -59,6 +59,7 @@
5 IStructuralSubscription,
6 IStructuralSubscriptionForm,
7 IStructuralSubscriptionTarget,
8+ IStructuralSubscriptionTargetHelper,
9 )
10 from lp.registry.interfaces.distribution import (
11 IDistribution,
12@@ -380,10 +381,8 @@
13 bug subscriptions.
14 """
15 sst = self._getSST()
16- target = sst
17- if sst.parent_subscription_target is not None:
18- target = sst.parent_subscription_target
19- return (target.bug_tracking_usage == ServiceUsage.LAUNCHPAD and
20+ pillar = IStructuralSubscriptionTargetHelper(sst).pillar
21+ return (pillar.bug_tracking_usage == ServiceUsage.LAUNCHPAD and
22 sst.userCanAlterBugSubscription(self.user, self.user))
23
24 @enabled_with_permission('launchpad.AnyPerson')
25@@ -433,15 +432,15 @@
26 # Get this only if we need to.
27 membership = list(user.teams_participated_in)
28 for team in administrated_teams:
29- # If the user is not a member of the team itself, then skip it,
30- # because structural subscriptions and their filters can only be
31- # edited by the subscriber.
32+ # If the user is not a member of the team itself, then
33+ # skip it, because structural subscriptions and their
34+ # filters can only be edited by the subscriber.
35 # This can happen if the user is an owner but not a member.
36 if not team in membership:
37 continue
38- # If the context is a distro AND a bug supervisor is set AND
39- # the admininistered team is not a member of the bug supervisor
40- # team THEN skip it.
41+ # If the context is a distro AND a bug supervisor is set
42+ # AND the admininistered team is not a member of the bug
43+ # supervisor team THEN skip it.
44 if (is_distro and context.bug_supervisor is not None and
45 not team.inTeam(context.bug_supervisor)):
46 continue
47
48=== modified file 'lib/lp/registry/browser/tests/test_subscription_links.py'
49--- lib/lp/registry/browser/tests/test_subscription_links.py 2011-04-18 15:19:26 +0000
50+++ lib/lp/registry/browser/tests/test_subscription_links.py 2011-04-25 18:15:54 +0000
51@@ -576,11 +576,11 @@
52 # Tests for when the IStructuralSubscriptionTarget does not use Launchpad for
53 # bug tracking. In those cases the links should not be shown.
54
55-class ProductDoesNotUseLPView(ProductView):
56+class _DoesNotUseLP(ProductView):
57 """Test structural subscriptions on the product view."""
58
59 def setUp(self):
60- super(ProductDoesNotUseLPView, self).setUp()
61+ super(_DoesNotUseLP, self).setUp()
62 self.target = self.factory.makeProduct(official_malone=False)
63
64 def test_subscribe_link_feature_flag_on_owner(self):
65@@ -601,6 +601,21 @@
66 self.assertNewLinksMissing()
67
68
69+class ProductDoesNotUseLPView(_DoesNotUseLP):
70+
71+ def test_subscribe_link_no_bugtracker_parent_bugtracker(self):
72+ # If there is no bugtracker, do not render links, even if the
73+ # parent has a bugtracker (see bug 770287).
74+ project = self.factory.makeProject()
75+ with person_logged_in(self.target.owner):
76+ self.target.project = project
77+ another_product = self.factory.makeProduct(
78+ project=project, official_malone=True)
79+ self._create_scenario(self.regular_user, 'on')
80+ self.assertOldLinkMissing()
81+ self.assertNewLinksMissing()
82+
83+
84 class ProductDoesNotUseLPBugs(ProductDoesNotUseLPView):
85 """Test structural subscriptions on the product bugs view."""
86
87@@ -625,7 +640,7 @@
88 self.assertNewLinksMissing()
89
90
91-class ProjectGroupDoesNotUseLPView(ProductDoesNotUseLPView):
92+class ProjectGroupDoesNotUseLPView(_DoesNotUseLP):
93 """Test structural subscriptions on the project group view."""
94
95 rootsite = None
96@@ -650,8 +665,10 @@
97 self.factory.makeProduct(
98 project=self.target, official_malone=False)
99
100-
101-class ProductSeriesDoesNotUseLPView(ProductDoesNotUseLPView):
102+ test_subscribe_link_no_bugtracker_parent_bugtracker = None
103+
104+
105+class ProductSeriesDoesNotUseLPView(_DoesNotUseLP):
106
107 def setUp(self):
108 super(ProductSeriesDoesNotUseLPView, self).setUp()
109@@ -659,7 +676,7 @@
110 self.target = self.factory.makeProductSeries(product=product)
111
112
113-class ProductSeriesDoesNotUseLPBugs(ProductDoesNotUseLPView):
114+class ProductSeriesDoesNotUseLPBugs(_DoesNotUseLP):
115
116 def setUp(self):
117 super(ProductSeriesDoesNotUseLPBugs, self).setUp()
118@@ -667,7 +684,7 @@
119 self.target = self.factory.makeProductSeries(product=product)
120
121
122-class DistributionSourcePackageDoesNotUseLPView(ProductDoesNotUseLPView):
123+class DistributionSourcePackageDoesNotUseLPView(_DoesNotUseLP):
124 """Test structural subscriptions on the distro src pkg view."""
125
126 def setUp(self):