launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-761124 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #761124 in Launchpad itself: "subscribing to milestone bugs does not work with new interface"
  https://bugs.launchpad.net/launchpad/+bug/761124

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-761124/+merge/58140

= 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
-- 
https://code.launchpad.net/~bac/launchpad/bug-761124/+merge/58140
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-761124 into lp:launchpad.

=== 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-18 15:21:40 +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_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-18 15:21:40 +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-18 15:21:40 +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"