Merge lp:~bac/launchpad/bug-761124 into lp:launchpad

[Brad Crittenden]

[r=benji][bug=761124] Retarget an overly restrictive conditional in the milestone index template.

= Summary =

The JavaScript on the milestone page was not being called to transform
the subscription link.

== Proposed fix ==

The <script> is inside <tal:head-epilogue> which had a conditional
preventing it from being rendered and executed. Move the condition to
surround only those parts it applies to.

Once the JS was called it became obvious that the required data was not
being exposed in LP.cache.

Tests have been added to ensure the setup call is rendered in the HTML
and that the cache is populated.

== Pre-implementation notes ==

None.

== Implementation details ==

As above.

== Tests ==

bin/test -vvt test_subscription_links

== Demo and Q/A ==

https://launchpad.dev/firefox/+milestone/1.0 should show a green
'Subscribe to bug mail' link for any logged in user.

= Launchpad lint =

Will fix.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/templates/milestone-index.pt
  lib/lp/registry/browser/milestone.py
  lib/lp/registry/browser/tests/test_subscription_links.py

./lib/lp/registry/browser/tests/test_subscription_links.py
      84: E302 expected 2 blank lines, found 1

[Benji York (benji)]

Looks good.

review: Approve (code)

=== modified file 'lib/lp/registry/browser/milestone.py'
--- lib/lp/registry/browser/milestone.py	2011-03-29 22:34:04 +0000
+++ lib/lp/registry/browser/milestone.py	2011-04-19 01:04:17 +0000
@@ -56,6 +56,7 @@
 from lp.app.widgets.date import DateWidget
 from lp.bugs.browser.bugtask import BugTaskListingItem
 from lp.bugs.browser.structuralsubscription import (
+    expose_structural_subscription_data_to_js,
     StructuralSubscriptionMenuMixin,
     StructuralSubscriptionTargetTraversalMixin,
     )
@@ -212,6 +213,8 @@
         """See `LaunchpadView`."""
         self.form = self.request.form
         self.processDeleteFiles()
+        expose_structural_subscription_data_to_js(
+            self.context, self.request, self.user)
 
     @property
     def expire_cache_minutes(self):

=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py	2011-03-03 22:57:03 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py	2011-04-19 01:04:17 +0000
@@ -239,7 +239,7 @@
     def test_milestone_eager_loading(self):
         # Verify that the number of queries does not increase with more
         # bugs with different assignees.
-        browses_under_limit = BrowsesWithQueryLimit(35, self.owner)
+        browses_under_limit = BrowsesWithQueryLimit(36, self.owner)
         self.add_bug(3)
         self.assertThat(self.milestone, browses_under_limit)
         self.add_bug(10)
@@ -253,7 +253,7 @@
         # is very large already, if the test fails due to such a change,
         # please cut some more of the existing fat out of it rather than
         # increasing the cap.
-        page_query_limit = 36
+        page_query_limit = 37
         product = self.factory.makeProduct()
         login_person(product.owner)
         milestone = self.factory.makeMilestone(
@@ -423,7 +423,7 @@
     def test_milestone_eager_loading(self):
         # Verify that the number of queries does not increase with more
         # bugs with different assignees.
-        browses_under_limit = BrowsesWithQueryLimit(34, self.owner)
+        browses_under_limit = BrowsesWithQueryLimit(35, self.owner)
         self.add_bug(4)
         self.assertThat(self.milestone, browses_under_limit)
         self.add_bug(10)

=== modified file 'lib/lp/registry/browser/tests/test_subscription_links.py'
--- lib/lp/registry/browser/tests/test_subscription_links.py	2011-04-13 17:58:31 +0000
+++ lib/lp/registry/browser/tests/test_subscription_links.py	2011-04-19 01:04:17 +0000
@@ -72,6 +72,14 @@
         self.assertNotEqual(
             None, self.new_edit_link,
             "Expected edit_bug_mail link missing")
+        # Ensure the LP.cache has been populated.
+        self.assertTrue("LP.cache['administratedTeams']" in self.contents)
+        # And that the call to setup the subscription is in the HTML.  A
+        # windmill test is required to ensure that the call actually
+        # succeeded, by checking the link class for 'js-action'.
+        setup = ("""module.setup({content_box: """
+                 """"#structural-subscription-content-box"});""")
+        self.assertTrue(setup in self.contents)
 
 
 class _TestStructSubs(TestCaseWithFactory, _TestResultsMixin):

=== modified file 'lib/lp/registry/templates/milestone-index.pt'
--- lib/lp/registry/templates/milestone-index.pt	2011-04-13 01:19:03 +0000
+++ lib/lp/registry/templates/milestone-index.pt	2011-04-19 01:04:17 +0000
@@ -6,13 +6,14 @@
   metal:use-macro="view/macro:page/main_side"
   i18n:domain="launchpad">
 
-  <tal:head-epilogue metal:fill-slot="head_epilogue"
-    condition="view/is_project_milestone">
-    <style id="hide-side-portlets" type="text/css">
+  <tal:head-epilogue metal:fill-slot="head_epilogue">
+    <tal:is_project condition="view/is_project_milestone">
+      <style id="hide-side-portlets" type="text/css">
         .side {
-            background: #fff;
-            }
-    </style>
+          background: #fff;
+        }
+      </style>
+    </tal:is_project>
     <tal:uses_launchpad_bugtracker
        condition="context/target/bug_tracking_usage/enumvalue:LAUNCHPAD">
       <script type="text/javascript"