launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #778323 in Launchpad itself: "On the subscription page, the help popup button opens up a 404 page."
  https://bugs.launchpad.net/launchpad/+bug/778323

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

= Summary =

A help file shown on the +subscriptions page is only available in the
bugs facet.

== Proposed fix ==

In situations where a help file is needed in another facet it is
possible to create a symbolic link from the other one to the real file.
 However if the +subscriptions page is navigated via a hand-crafted URL
it will be presented for all facets.  Thus, symlinks would be needed for
the help file from all application/help directories.

Instead, we make +subscriptions redirect to the bugs facet so that all
needed resources will be available.

== Pre-implementation notes ==

Talk with Gary

== Implementation details ==

As above.

== Tests ==

bin/test -vvm lp.bugs -t test_bugtarget_subscription

== Demo and Q/A ==

Go to https://code.launchpad.dev/ubuntu/+subscriptions.  Verify it
redirects to https://bugs.launchpad.dev/ubuntu/+subscriptions.  Click on
the help icon next to "Stop my emails..."  See that you don't get a 404.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/bugtarget.py
  lib/lp/bugs/browser/tests/test_bugtarget_subscription.py
  lib/lp/testing/views.py
-- 
https://code.launchpad.net/~bac/launchpad/bug-778323/+merge/61315
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-778323 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2011-05-04 16:46:43 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2011-05-17 21:05:43 +0000
@@ -26,6 +26,7 @@
 from datetime import datetime
 from operator import itemgetter
 import urllib
+from urlparse import urljoin
 
 from lazr.restful.interface import copy_field
 from pytz import timezone
@@ -96,6 +97,7 @@
     GhostWidget,
     ProductBugTrackerWidget,
     )
+from lp.bugs.publisher import BugsLayer
 from lp.bugs.browser.bugrole import BugRoleMixin
 from lp.bugs.browser.bugtask import BugTaskSearchListingView
 from lp.bugs.browser.structuralsubscription import (
@@ -1065,7 +1067,8 @@
                 config.malone.ubuntu_bug_filing_url)
 
     @safe_action
-    @action("Continue", name="projectgroupsearch", validator="validate_search")
+    @action("Continue", name="projectgroupsearch",
+            validator="validate_search")
     def projectgroup_search_action(self, action, data):
         """Search for similar bug reports."""
         # Don't give focus to any widget, to ensure that the browser
@@ -1568,6 +1571,14 @@
 
     def initialize(self):
         super(TargetSubscriptionView, self).initialize()
+        # Some resources such as help files are only provided on the bugs
+        # rootsite.  So if we got here via another, possibly hand-crafted, URL
+        # redirect to the equivalent URL on the bugs rootsite.
+        if not BugsLayer.providedBy(self.request):
+            new_url = urljoin(
+                self.request.getRootURL('bugs'), self.request['PATH_INFO'])
+            self.request.response.redirect(new_url)
+            return
         expose_structural_subscription_data_to_js(
             self.context, self.request, self.user, self.subscriptions)
 

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_subscription.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_subscription.py	2011-05-13 12:22:49 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_subscription.py	2011-05-17 21:05:43 +0000
@@ -12,6 +12,7 @@
     person_logged_in,
     TestCaseWithFactory,
     )
+from lp.testing.views import create_view
 
 
 class TargetSubscriptionViewTestCase(TestCaseWithFactory):
@@ -28,8 +29,24 @@
     def test_form_initializes(self):
         # It's a start.
         with person_logged_in(self.subscriber):
-            sub = self.product.addBugSubscription(
+            self.product.addBugSubscription(
                 self.subscriber, self.subscriber)
             harness = LaunchpadFormHarness(
                 self.product, TargetSubscriptionView)
             harness.view.initialize()
+
+    def test_does_not_redirect(self):
+        # +subscriptions on the bugs facet does not redirect.
+        with person_logged_in(self.subscriber):
+            view = create_view(
+                self.product, name='+subscriptions', rootsite='bugs')
+            view.initialize()
+            self.assertFalse(view._isRedirected())
+
+    def test_redirects(self):
+        # +subscriptions on anything but the bugs facet redirects.
+        with person_logged_in(self.subscriber):
+            view = create_view(
+                self.product, name='+subscriptions', rootsite='code')
+            view.initialize()
+            self.assertTrue(view._isRedirected())

=== modified file 'lib/lp/testing/views.py'
--- lib/lp/testing/views.py	2010-10-18 11:41:01 +0000
+++ lib/lp/testing/views.py	2011-05-17 21:05:43 +0000
@@ -47,7 +47,7 @@
     :param principal: The principal for the request, default to the
         unauthenticated principal.
     :param query_string: The query string for the request.
-    :patam cookie: The HTTP_COOKIE value for the request.
+    :param cookie: The HTTP_COOKIE value for the request.
     :param request: Use this request instead of creating a new one.
     :param path_info: The PATH_INFO value for the request.
     :param current_request: If True, the request will be set as the current