← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rharding/launchpad/ajax_log_930141 into lp:launchpad

 

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #930141 in Launchpad itself: "ajax log for developers doesn't catch any events"
  https://bugs.launchpad.net/launchpad/+bug/930141

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

= Summary =
The Ajax Log is broken currently due to the changes to the JS work recently adding support for the combo loader.

== Proposed Fix ==
The events aren't in the same YUI sandbox space, so the ajax logging needs to be pulled in there. The best way to do that is to turn the current single "start_ajax_logging" code into a real YUI module that can be .use()'d and started from inside the LPJS YUI instance.

== Implementation Details ==
While making this a YUI module, I also added a stub test file for the module. I didn't go adding all the tests this would require at the moment, but it's a start.

== Tests ==
lib/lp/app/javascript/tests/test_ajax_log.html

== Demo and Q/A ==
Head to a page with ajax on it. /launchpad/+bugs for instance. Make sure that you can load the page, and change the sort order or other action that uses an ajax call. The "AJAX Log" should highlight and display the time for that request. This is for users with the feature flag: visible_render_time set.

== Lint ==

Linting changed files:
  lib/lp/app/javascript/ajax_log.js
  lib/lp/app/javascript/tests/test_ajax_log.html
  lib/lp/app/javascript/tests/test_ajax_log.js
  lib/lp/app/templates/base-layout.pt
-- 
https://code.launchpad.net/~rharding/launchpad/ajax_log_930141/+merge/92827
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/ajax_log_930141 into lp:launchpad.
=== modified file 'lib/lp/app/javascript/ajax_log.js'
--- lib/lp/app/javascript/ajax_log.js	2012-01-19 22:59:03 +0000
+++ lib/lp/app/javascript/ajax_log.js	2012-02-13 19:05:29 +0000
@@ -1,25 +1,48 @@
-/* Log time taken for AJAX requests.
-*/
-function start_ajax_logging() {
-    /* Requests slower than this are marked as slow. Value in seconds.
-    */
-    var AJAX_OK_TIME = 1;
-
-    YUI().use('node', 'lp.anim', function(Y) {
-        Y.on('contentready', function() {
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * A module to add support for watching and timing ajax events from the page.
+ * Driven by the feature flag: visible_render_time.
+ *
+ * @module lp.ajax_log
+ */
+YUI.add('lp.ajax_log', function (Y) {
+
+    var module = Y.namespace('lp');
+
+    /**
+     * Ajax log will watch ajax requests for timing information.
+     *
+     * @class ajax_log
+     * @extends Y.Base
+     * @namespace Y.lp
+     *
+     */
+    module.ajax_log = Y.Base.create('ajax_log', Y.Base, [], {
+        _bind_events: function () {
+            var that = this;
             var node = Y.one('#ajax-time-list');
             var ajax_request_times = {};
             var ajax_menu_animating = false;
             var flash_menu = Y.lp.anim.green_flash({node:'#ajax-time'});
+
+            // Open/close the log.
+            Y.on('click', function(e) {
+                e.halt();
+                Y.one('#ajax-time-list').toggleClass('hidden');
+            }, '#ajax-time a');
+
             flash_menu.on('end', function() {
                 ajax_menu_animating = false;
             });
+
             /* When an AJAX event starts, record the time.
             */
             Y.on('io:start', function(transactionid) {
                 var now = new Date();
                 ajax_request_times[transactionid] = now;
             });
+
             /* When an AJAX event finishes add it to the log.
             */
             Y.on('io:complete', function(transactionid, response) {
@@ -39,10 +62,10 @@
                     log_node.addClass('transaction-' + transactionid);
                     log_node.one('strong').set(
                         'text', time_taken.toFixed(2) + ' seconds');
-                    /* If the AJAX event takes longer than AJAX_OK_TIME
-                       then add a warning.
-                    */
-                    if (time_taken > AJAX_OK_TIME) {
+
+                    // if the AJAX event takes longer than ajax_ok_time
+                    //then add a warning.
+                    if (time_taken > that.get('ajax_ok_time')) {
                         log_node.one('strong').addClass('warning');
                     }
                     log_node.one('span').set(
@@ -60,26 +83,49 @@
                         log_node.one('span').append(oops_node);
                     }
                     node.prepend(log_node);
-                    /* Highlight the new entry in the log.
-                    */
+
+                    // Highlight the new entry in the log.
                     Y.lp.anim.green_flash({
                         node: '#ajax-time-list li.transaction-'+transactionid
-                        }).run();
-                    /* Signify a new entry has been added to the log.
-                    */
+                    }).run();
+
+                    // Signify a new entry has been added to the log.
                     if (ajax_menu_animating === false) {
                         flash_menu.run();
                         ajax_menu_animating = true;
                     }
                 }
             });
+        },
 
-            /* Open/close the log.
-            */
-            Y.on('click', function(e) {
-                e.halt();
-                Y.one('#ajax-time-list').toggleClass('hidden');
-            }, '#ajax-time a');
-        }, '#ajax-time');
+        /**
+         * Basic initializer to build the events for the instance.
+         *
+         * @method intializer
+         * @param {Object} cfg
+         *
+         */
+        initializer: function (cfg) {
+            var that = this;
+            Y.on('contentready', function() {
+                that._bind_events();
+            }, '#ajax-time');
+        }
+    }, {
+        ATTRS: {
+            /**
+             * Requests slower than this are marked as slow. Value in seconds.
+             *
+             * @attribute ajax_ok_time
+             * @default 1
+             * @type Integer
+             *
+             */
+            ajax_ok_time: {
+                value: 1
+            }
+        }
     });
-}
+}, '0.1', {
+    'requires': ['node', 'lp.anim']
+});

=== added file 'lib/lp/app/javascript/tests/test_ajax_log.html'
--- lib/lp/app/javascript/tests/test_ajax_log.html	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_ajax_log.html	2012-02-13 19:05:29 +0000
@@ -0,0 +1,51 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
+  "http://www.w3.org/TR/html4/strict.dtd";>
+<!--
+Copyright 2012 Canonical Ltd.  This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<html>
+  <head>
+      <title>Test ajax_log</title>
+
+      <!-- YUI and test setup -->
+      <script type="text/javascript"
+              src="../../../../../build/js/yui/yui/yui.js">
+      </script>
+      <link rel="stylesheet"
+      href="../../../../../build/js/yui/console/assets/console-core.css" />
+      <link rel="stylesheet"
+      href="../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+      <link rel="stylesheet"
+      href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+
+      <script type="text/javascript"
+              src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+
+      <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+
+      <!-- Dependencies -->
+      <script type="text/javascript"
+          src="../../../../../build/js/lp/app/anim/anim.js"></script>
+      <script type="text/javascript"
+          src="../../../../../build/js/lp/app/extras/extras.js"></script>
+
+
+      <!-- The module under test. -->
+      <script type="text/javascript" src="../ajax_log.js"></script>
+
+      <!-- Any css assert for this module. -->
+      <!-- <link rel="stylesheet" href="../assets/ajax_log-core.css" /> -->
+
+      <!-- The test suite. -->
+      <script type="text/javascript" src="test_ajax_log.js"></script>
+
+    </head>
+    <body class="yui3-skin-sam">
+        <ul id="suites">
+            <!-- <li>lp.large_indicator.test</li> -->
+            <li>lp.ajax_log.test</li>
+        </ul>
+    </body>
+</html>

=== added file 'lib/lp/app/javascript/tests/test_ajax_log.js'
--- lib/lp/app/javascript/tests/test_ajax_log.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_ajax_log.js	2012-02-13 19:05:29 +0000
@@ -0,0 +1,21 @@
+/* Copyright (c) 2012, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.ajax_log.test', function (Y) {
+
+    var tests = Y.namespace('lp.ajax_log.test');
+    tests.suite = new Y.Test.Suite('ajax_log Tests');
+
+    tests.suite.add(new Y.Test.Case({
+        name: 'ajax_log_tests',
+
+        setUp: function () {},
+        tearDown: function () {},
+
+        test_library_exists: function () {
+            Y.Assert.isObject(Y.lp.ajax_log,
+                "We should be able to locate the lp.ajax_log module");
+        }
+
+    }));
+
+}, '0.1', {'requires': ['test', 'console', 'lp.ajax_log']});

=== modified file 'lib/lp/app/templates/base-layout.pt'
--- lib/lp/app/templates/base-layout.pt	2012-02-01 15:31:32 +0000
+++ lib/lp/app/templates/base-layout.pt	2012-02-13 19:05:29 +0000
@@ -204,12 +204,12 @@
     tal:condition="request/features/visible_render_time"
     define="render_time modules/lp.services.webapp.adapter/summarize_requests;"
     replace='structure string:&lt;script type="text/javascript"&gt;
-  start_ajax_logging();
   var render_time = "${render_time}";
-  LPJS.use("node", function(Y) {
+  LPJS.use("node", "lp.ajax_log" , function(Y) {
     Y.on("domready", function() {
       var node = Y.one("#rendertime");
       node.set("innerHTML", render_time);
+      var ajax_log = new Y.lp.ajax_log();
     });
   });
 &lt;/script&gt;' />