launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/oops_912178 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #912178 in Launchpad itself: "With custom bug listings turned on, bugs search produces an OOPS"
  https://bugs.launchpad.net/launchpad/+bug/912178

For more details, see:
https://code.launchpad.net/~rharding/launchpad/oops_912178/+merge/87675

= Summary =
The main bug search page threw an oops due to a missing method the template expected.

== Proposed Fix ==
Add the search_macro_title method to the View object so that it can be provided to the template. 

== Implementation Details ==
During fixing this, it was also found that the javascript for the bug columns is built with the knowledge that there is always a content in LP.cache for it. This View does not provide a context. I added a special case in order to provide the bare minimum of a lp.client.Entry object for the rest of the javascript to proceed.

== Tests ==
./bin/test -cvvt "test_bugs\.

== Demo and Q/A ==
Make sure you can load and operate the bug listing at bugs.launchpad.net/bugs/+bugs

== Lint ==

Linting changed files:
  lib/lp/app/javascript/client.js
  lib/lp/app/javascript/listing_navigator.js
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/browser/tests/test_bugs.py

-- 
https://code.launchpad.net/~rharding/launchpad/oops_912178/+merge/87675
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/oops_912178 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/listing_navigator.js'
--- lib/lp/app/javascript/listing_navigator.js	2011-12-15 17:09:48 +0000
+++ lib/lp/app/javascript/listing_navigator.js	2012-01-05 22:02:31 +0000
@@ -16,6 +16,19 @@
     return new Y.NodeList([]);
 }
 
+/**
+ * If there is no context (say /bugs/+bugs) we need to generate a weblink for
+ * the lp.client to use for things. This is a helper to take the current url
+ * and try to generate a likely web_link value to build a url off of.
+ */
+function likely_web_link() {
+    var url = location.href;
+    var cut_at = url.indexOf('+');
+
+    // make sure we trim the trailing / off the web link we generate
+    return url.substr(0, cut_at - 1);
+}
+
 
 /**
  * Constructor.
@@ -357,6 +370,18 @@
             }
         };
         var context = this.get_current_batch().context;
+
+        if (!context) {
+            // try to see if there is no context we can fake the object in
+            // order to pass through
+            context = new Y.lp.client.Entry(
+                new Y.lp.client.Launchpad(),
+                {
+                    'web_link': likely_web_link()
+                }
+            );
+        }
+
         var view_name = this.get_current_batch().view_name;
 
         if (Y.Lang.isValue(this.get('io_provider'))) {

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-12-30 08:03:42 +0000
+++ lib/lp/bugs/browser/bugtask.py	2012-01-05 22:02:31 +0000
@@ -4233,6 +4233,9 @@
         """Return the heading to search all Bugs."""
         return "Search all bug reports"
 
+    def search_macro_title(self):
+        return u'Search all bugs'
+
     @property
     def label(self):
         return self.getSearchPageHeading()

=== modified file 'lib/lp/bugs/browser/tests/test_bugs.py'
--- lib/lp/bugs/browser/tests/test_bugs.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/browser/tests/test_bugs.py	2012-01-05 22:02:31 +0000
@@ -5,10 +5,15 @@
 
 __metaclass__ = type
 
+from contextlib import contextmanager
 from zope.component import getUtility
 
 from lp.bugs.interfaces.malone import IMaloneApplication
 from lp.bugs.publisher import BugsLayer
+from lp.testing import (
+    set_feature_flag,
+    feature_flags,
+    )
 from lp.services.webapp.publisher import canonical_url
 from lp.testing import (
     celebrity_logged_in,
@@ -19,6 +24,14 @@
 from lp.testing.views import create_initialized_view
 
 
+@contextmanager
+def dynamic_listings():
+    """Context manager to enable new bug listings."""
+    with feature_flags():
+        set_feature_flag(u'bugs.dynamic_bug_listings.enabled', u'on')
+        yield
+
+
 class TestMaloneView(TestCaseWithFactory):
     """Test the MaloneView for the Bugs application."""
     layer = DatabaseFunctionalLayer
@@ -90,3 +103,14 @@
         self.assertIn(picker_vocab, text)
         focus_script = "setFocusByName('field.searchtext')"
         self.assertIn(focus_script, text)
+
+    def test_search_all_bugs_rendering(self):
+        with dynamic_listings():
+            view = create_initialized_view(
+                self.application,
+                '+bugs',
+                rootsite='bugs')
+            content = view.render()
+
+        # we should get some valid content out of this
+        self.assertIn('Search all bugs', content)