Merge lp:~benji/launchpad/bug-778689 into lp:launchpad

[Benji York]

[r=gmb][bug=778689] make project group milestone pages not OOPS

This branch fixes bug 778689. There was no test coverage for a menu
mixin when viewing project group milestones and the code couldn't handle
it.

The fix was simple: since subscribing to project group milestones isn't
allowed, just disable the links.

The related tests can be run with:

    bin/test -c -m lp.registry.browser.tests.test_subscription_links

The make lint report is clean:

    = Launchpad lint =

    Checking for conflicts and issues in changed files.

    Linting changed files:
    lib/lp/bugs/browser/structuralsubscription.py
    lib/lp/registry/browser/tests/test_subscription_links.py

[Graham Binns (gmb)]

We agreed on IRC that you need to add a comment to your test so that readers don't need to luck up the bug report. Otherwise, r=me.

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py	2011-05-03 13:38:30 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py	2011-05-10 15:46:25 +0000
@@ -381,6 +381,11 @@
         bug subscriptions.
         """
         sst = self._getSST()
+        # ProjectGroup milestones aren't really structural subscription
+        # targets as they're not real milestones, so you can't subscribe to
+        # them.
+        if IProjectGroupMilestone.providedBy(sst):
+            return False
         pillar = IStructuralSubscriptionTargetHelper(sst).pillar
         return (pillar.bug_tracking_usage == ServiceUsage.LAUNCHPAD and
                 sst.userCanAlterBugSubscription(self.user, self.user))

=== modified file 'lib/lp/registry/browser/tests/test_subscription_links.py'
--- lib/lp/registry/browser/tests/test_subscription_links.py	2011-04-25 18:13:31 +0000
+++ lib/lp/registry/browser/tests/test_subscription_links.py	2011-05-10 15:46:25 +0000
@@ -15,7 +15,11 @@
 from canonical.launchpad.testing.pages import first_tag_by_class
 from canonical.testing.layers import DatabaseFunctionalLayer
 
+from lp.bugs.browser.structuralsubscription import (
+    StructuralSubscriptionMenuMixin,
+    )
 from lp.registry.interfaces.person import IPersonSet
+from lp.registry.model.milestone import ProjectMilestone
 from lp.services.features.testing import FeatureFixture
 from lp.testing import (
     celebrity_logged_in,
@@ -175,6 +179,27 @@
             project=self.target, official_malone=True)
 
 
+class ProjectGroupMilestone(TestCaseWithFactory):
+    """Make sure that projects' "virtual" milestones don't break things."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_for_bug_778689(self):
+        with person_logged_in(self.factory.makePerson()):
+            # Project groups have "virtual" milestones that aren't stored in
+            # the database directly (they're inherited from the contained
+            # products).  Viewing one of those group milestones would generate
+            # an OOPS because adapting the milestone to
+            # IStructuralSubscriptionTargetHelper would attempt to look them
+            # up in the database, raising an exception.
+            project = self.factory.makeProject()
+            product = self.factory.makeProduct(project=project)
+            mixin = StructuralSubscriptionMenuMixin()
+            mixin.context = ProjectMilestone(project, '11.04', None, True)
+            # Before bug 778689 was fixed, this would raise an exception.
+            mixin._enabled
+
+
 class ProjectGroupBugs(ProjectGroupView):
     """Test structural subscriptions on the project group bugs view."""