launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/cache-batches into lp:launchpad with lp:~abentley/launchpad/update-listings as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~abentley/launchpad/cache-batches/+merge/80000

= Summary =
Support caching batches of bug listings.

== Proposed fix ==
Store bug listings for the current page in lp.bug.buglistings.batches

== Pre-implementation notes ==
None

== Implementation details ==
Modified update_from_model to accept an ordering and cache results.
Modified update_listing to use cached results.


== Tests ==
bin/test -t buglisting.html

== Demo and Q/A ==
Not qa-able.

= Launchpad lint =

No lint.
-- 
https://code.launchpad.net/~abentley/launchpad/cache-batches/+merge/80000
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/cache-batches into lp:launchpad.

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2011-10-20 20:47:26 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2011-10-20 20:47:27 +0000
@@ -33,9 +33,13 @@
 };
 
 /**
- * A shim to use the data of an LP.cache to render the bug listings.
+ * A shim to use the data of an LP.cache to render the bug listings and cache
+ * their data.
+ *
+ * order_by is the ordering used by the model.
  */
-namespace.update_from_model = function(model){
+namespace.update_from_model = function(order_by, model){
+    namespace.batches[order_by] = model.mustache_model;
     namespace.rendertable(model.mustache_model);
 };
 
@@ -49,13 +53,18 @@
  * Config may contain an io_provider.
  */
 namespace.update_listing = function(order_by, config){
-    var lp_client = new Y.lp.client.Launchpad();
-    var cache = lp_client.wrap_resource(null, LP.cache);
-    var query = namespace.get_query(window.location);
+    var lp_client, cache, query;
+    if (Y.Lang.isValue(namespace.batches[order_by])){
+        namespace.update_from_model(order_by, namespace.batches[order_by]);
+        return;
+    }
+    lp_client = new Y.lp.client.Launchpad();
+    cache = lp_client.wrap_resource(null, LP.cache);
+    query = namespace.get_query(window.location);
     query.orderby = order_by;
     load_model_config = {
         on: {
-            success: namespace.update_from_model
+            success: Y.bind(namespace.update_from_model, window, order_by)
         }
     };
     if (Y.Lang.isValue(config)){
@@ -75,4 +84,6 @@
 };
 
 
+namespace.batches = {};
+
 }, "0.1", {"requires": ["node", 'lp.client']});

=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.js'
--- lib/lp/bugs/javascript/tests/test_buglisting.js	2011-10-20 20:47:26 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.js	2011-10-20 20:47:27 +0000
@@ -69,24 +69,72 @@
     },
     tearDown: function() {
         delete window.LP;
+        module.batches = {};
     },
-    test_update_listing: function() {
-        /* update_listing retrieves a listing for the new ordering and
-         * displays it */
+    get_intensity_listing: function(){
         mock_io = new Y.lp.testing.mockio.MockIo();
         module.update_listing('intensity', {io_provider: mock_io});
         Y.Assert.areEqual('',
             Y.one('#client-listing').get('innerHTML'));
-        Y.Assert.areEqual('/bar/+bugs/++model++?orderby=intensity',
-            mock_io.last_request.url);
         mock_io.last_request.successJSON({mustache_model:
             {item: [
                 {name: 'first'},
                 {name: 'second'}
             ]}
         });
+        return mock_io;
+    },
+    test_update_listing: function() {
+        /* update_listing retrieves a listing for the new ordering and
+         * displays it */
+        mock_io = this.get_intensity_listing();
         Y.Assert.areEqual('<ol><li>first</li><li>second</li></ol>',
             Y.one('#client-listing').get('innerHTML'));
+        Y.Assert.areEqual('/bar/+bugs/++model++?orderby=intensity',
+            mock_io.last_request.url);
+    },
+    test_update_listing_uses_cache: function() {
+        /* update_listing will use the cached value instead of making a second
+         * AJAX request. */
+        mock_io = this.get_intensity_listing();
+        Y.Assert.areEqual(1, mock_io.requests.length);
+        module.update_listing('intensity', {io_provider: mock_io});
+        Y.Assert.areEqual(1, mock_io.requests.length);
+    }
+}));
+suite.add(new Y.Test.Case({
+    name: 'Batch caching',
+
+    setUp: function () {
+        this.MY_NAME = "ME";
+        var lp_client = new Y.lp.client.Launchpad();
+        window.LP = { links: { me: "/~" + this.MY_NAME } };
+        set_fixture('<div id="client-listing"></div>');
+        window.LP.cache = {
+            context: {
+                resource_type_link: 'http://foo_type',
+                web_link: 'http://foo/bar'
+            },
+            mustache_model: {
+                foo: 'bar'
+            }
+        };
+        window.LP.mustache_listings = "<ol>" +
+            "{{#item}}<li>{{name}}</li>{{/item}}</ol>";
+    },
+    tearDown: function() {
+        delete window.LP;
+        module.batches = {};
+    },
+    test_update_from_model_caches: function() {
+        /* update_from_model caches the settings in the module.batches. */
+        module.update_from_model('intensity', {mustache_model: {item: [
+                {name: 'first'},
+                {name: 'second'}
+            ]}});
+        Y.lp.testing.assert.assert_equal_structure(
+            {item: [{name: 'first'}, {name: 'second'}]},
+            module.batches.intensity);
     }
 }));