launchpad-reviewers team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Gary Poster has proposed merging lp:~gary/launchpad/bug770287 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #770287 in Launchpad itself: "bug mail links should not appear if project does not use LP for bug tracking"
  https://bugs.launchpad.net/launchpad/+bug/770287

For more details, see:
https://code.launchpad.net/~gary/launchpad/bug770287/+merge/58979

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.
-- 
https://code.launchpad.net/~gary/launchpad/bug770287/+merge/58979
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug770287 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py	2011-04-21 21:51:33 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py	2011-04-25 18:15:54 +0000
@@ -59,6 +59,7 @@
     IStructuralSubscription,
     IStructuralSubscriptionForm,
     IStructuralSubscriptionTarget,
+    IStructuralSubscriptionTargetHelper,
     )
 from lp.registry.interfaces.distribution import (
     IDistribution,
@@ -380,10 +381,8 @@
         bug subscriptions.
         """
         sst = self._getSST()
-        target = sst
-        if sst.parent_subscription_target is not None:
-            target = sst.parent_subscription_target
-        return (target.bug_tracking_usage == ServiceUsage.LAUNCHPAD and
+        pillar = IStructuralSubscriptionTargetHelper(sst).pillar
+        return (pillar.bug_tracking_usage == ServiceUsage.LAUNCHPAD and
                 sst.userCanAlterBugSubscription(self.user, self.user))
 
     @enabled_with_permission('launchpad.AnyPerson')
@@ -433,15 +432,15 @@
             # Get this only if we need to.
             membership = list(user.teams_participated_in)
             for team in administrated_teams:
-                # If the user is not a member of the team itself, then skip it,
-                # because structural subscriptions and their filters can only be
-                # edited by the subscriber.
+                # If the user is not a member of the team itself, then
+                # skip it, because structural subscriptions and their
+                # filters can only be edited by the subscriber.
                 # This can happen if the user is an owner but not a member.
                 if not team in membership:
                     continue
-                # If the context is a distro AND a bug supervisor is set AND
-                # the admininistered team is not a member of the bug supervisor
-                # team THEN skip it.
+                # If the context is a distro AND a bug supervisor is set
+                # AND the admininistered team is not a member of the bug
+                # supervisor team THEN skip it.
                 if (is_distro and context.bug_supervisor is not None and
                     not team.inTeam(context.bug_supervisor)):
                     continue

=== modified file 'lib/lp/registry/browser/tests/test_subscription_links.py'
--- lib/lp/registry/browser/tests/test_subscription_links.py	2011-04-18 15:19:26 +0000
+++ lib/lp/registry/browser/tests/test_subscription_links.py	2011-04-25 18:15:54 +0000
@@ -576,11 +576,11 @@
 # Tests for when the IStructuralSubscriptionTarget does not use Launchpad for
 # bug tracking.  In those cases the links should not be shown.
 
-class ProductDoesNotUseLPView(ProductView):
+class _DoesNotUseLP(ProductView):
     """Test structural subscriptions on the product view."""
 
     def setUp(self):
-        super(ProductDoesNotUseLPView, self).setUp()
+        super(_DoesNotUseLP, self).setUp()
         self.target = self.factory.makeProduct(official_malone=False)
 
     def test_subscribe_link_feature_flag_on_owner(self):
@@ -601,6 +601,21 @@
         self.assertNewLinksMissing()
 
 
+class ProductDoesNotUseLPView(_DoesNotUseLP):
+
+    def test_subscribe_link_no_bugtracker_parent_bugtracker(self):
+        # If there is no bugtracker, do not render links, even if the
+        # parent has a bugtracker (see bug 770287).
+        project = self.factory.makeProject()
+        with person_logged_in(self.target.owner):
+            self.target.project = project
+        another_product = self.factory.makeProduct(
+            project=project, official_malone=True)
+        self._create_scenario(self.regular_user, 'on')
+        self.assertOldLinkMissing()
+        self.assertNewLinksMissing()
+
+
 class ProductDoesNotUseLPBugs(ProductDoesNotUseLPView):
     """Test structural subscriptions on the product bugs view."""
 
@@ -625,7 +640,7 @@
         self.assertNewLinksMissing()
 
 
-class ProjectGroupDoesNotUseLPView(ProductDoesNotUseLPView):
+class ProjectGroupDoesNotUseLPView(_DoesNotUseLP):
     """Test structural subscriptions on the project group view."""
 
     rootsite = None
@@ -650,8 +665,10 @@
         self.factory.makeProduct(
             project=self.target, official_malone=False)
 
-
-class ProductSeriesDoesNotUseLPView(ProductDoesNotUseLPView):
+    test_subscribe_link_no_bugtracker_parent_bugtracker = None
+
+
+class ProductSeriesDoesNotUseLPView(_DoesNotUseLP):
 
     def setUp(self):
         super(ProductSeriesDoesNotUseLPView, self).setUp()
@@ -659,7 +676,7 @@
         self.target = self.factory.makeProductSeries(product=product)
 
 
-class ProductSeriesDoesNotUseLPBugs(ProductDoesNotUseLPView):
+class ProductSeriesDoesNotUseLPBugs(_DoesNotUseLP):
 
     def setUp(self):
         super(ProductSeriesDoesNotUseLPBugs, self).setUp()
@@ -667,7 +684,7 @@
         self.target = self.factory.makeProductSeries(product=product)
 
 
-class DistributionSourcePackageDoesNotUseLPView(ProductDoesNotUseLPView):
+class DistributionSourcePackageDoesNotUseLPView(_DoesNotUseLP):
     """Test structural subscriptions on the distro src pkg view."""
 
     def setUp(self):