← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~thumper/launchpad/client-cache-sync into lp:launchpad

 

Tim Penhey has proposed merging lp:~thumper/launchpad/client-cache-sync into lp:launchpad with lp:~thumper/launchpad/lp-client-yui-module as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #724004 client cache gets out of date
  https://bugs.launchpad.net/bugs/724004

For more details, see:
https://code.launchpad.net/~thumper/launchpad/client-cache-sync/+merge/51049
-- 
https://code.launchpad.net/~thumper/launchpad/client-cache-sync/+merge/51049
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/client-cache-sync into lp:launchpad.
=== modified file 'lib/canonical/launchpad/javascript/client/client.js'
--- lib/canonical/launchpad/javascript/client/client.js	2011-02-24 00:26:17 +0000
+++ lib/canonical/launchpad/javascript/client/client.js	2011-02-24 00:26:17 +0000
@@ -2,7 +2,6 @@
 // An AJAX client that runs against Launchpad's web service.
 var LP = {
   cache: {},
-  client: {},
   links: {}
 };
 
@@ -180,17 +179,64 @@
     return data;
 };
 
+function update_cached_object(cache_name, cache, entry)
+{
+  var fields_changed = new Array();
+  for (var name in cache) {
+    var old_value = cache[name];
+    var new_value = entry.get(name);
+    if (name != 'lp_html') {
+      if (old_value != new_value) {
+        fields_changed.push(name);
+        cache[name] = new_value;
+      }
+    }
+    else {
+      // Since we don't care here about the content, we aren't using the
+      // values here to determine if the field has changed, so we can just
+      // update the cache.
+      for (var html_name in old_value) {
+        old_value[html_name] = new_value[html_name];
+      }
+    }
+  }
+
+  if (fields_changed.length > 0) {
+    var event_name = 'lp:' + cache_name + ':changed';
+    Y.fire(event_name, fields_changed, entry);
+  }
+}
+
+
+module.update_cache = function(entry) {
+  if (!entry) return;
+  var original_uri = entry.lp_original_uri;
+  var full_uri = module.get_absolute_uri(original_uri);
+  for (var name in LP.cache) {
+    var cached_object = LP.cache[name];
+    if (cached_object['self_link'] == full_uri) {
+      Y.log(name + ' cached object has been updated.');
+      update_cached_object(name, cached_object, entry);
+    }
+  }
+}
+
 module.wrap_resource_on_success = function(ignore, response, args) {
     var client = args[0];
     var uri = args[1];
     var old_on_success = args[2];
+    var update_cache = args[3];
     var representation, wrapped;
     if (old_on_success) {
         var media_type = response.getResponseHeader('Content-Type');
         if (media_type.substring(0,16) == 'application/json') {
             representation = Y.JSON.parse(response.responseText);
             wrapped = client.wrap_resource(uri, representation);
-            return old_on_success(wrapped);
+            result = old_on_success(wrapped);
+            if (update_cache) {
+              module.update_cache(wrapped);
+            }
+            return result;
         } else {
             return old_on_success(response.responseText);
         }
@@ -391,6 +437,23 @@
     return this.get(key);
 };
 
+Entry.prototype.getHTML = function(key) {
+  var lp_html = this.get('lp_html');
+  if (lp_html) {
+    // First look for the key.
+    var value = lp_html[key];
+    if (value !== undefined) {
+      return value;
+    }
+    // now look for key_link
+    value = lp_html[key + '_link'];
+    if (value !== undefined) {
+      return value;
+    }
+  }
+  return null;
+};
+
 module.Entry = Entry;
 
 // The Launchpad client itself.
@@ -416,10 +479,11 @@
         }
 
         var old_on_success = on.success;
+        var update_cache = false;
         on.success = module.wrap_resource_on_success;
         var client = this;
         var y_config = { on: on,
-                         'arguments': [client, uri, old_on_success],
+                         'arguments': [client, uri, old_on_success, update_cache],
                          'headers': headers,
                          data: data};
         Y.io(uri, y_config);
@@ -464,9 +528,10 @@
             return module.wrap_resource_on_success(undefined, response, args);
         };
         var client = this;
+        var update_cache = false;
         var y_config = { method: "POST",
                          on: on,
-                         'arguments': [client, uri, old_on_success],
+                         'arguments': [client, uri, old_on_success, update_cache],
                          data: data};
         Y.io(uri, y_config);
     },
@@ -477,8 +542,9 @@
         uri = module.normalize_uri(uri);
 
         var old_on_success = on.success;
+        var update_cache = true;
         on.success = module.wrap_resource_on_success;
-        args = [this, uri, old_on_success];
+        args = [this, uri, old_on_success, update_cache];
 
         var extra_headers = {
                 "X-HTTP-Method-Override": "PATCH",
@@ -675,13 +741,13 @@
         resource: {},
 
         /**
-         * Is this a patch for only a field,
-         * not the entire resource object?
+         * Should the resulting field get the value from the lp_html
+         * attribute?
          *
-         * @attribute patch_field
+         * @attribute use_html
          * @type Boolean
          */
-        patch_field: false,
+        use_html: false,
 
         /**
          * The function to use to format the returned result into a form that
@@ -729,6 +795,7 @@
         // Save the config object that the user passed in so that we can pass
         // any extra parameters through to the lp.client constructor.
         this.extra_config = config || {};
+        this.extra_config['accept'] = 'application/json;include=lp_html';
 
         // Save a reference to the original _saveData()
         //method before wrapping it.
@@ -774,12 +841,8 @@
 
         var patch_payload;
         var val = owner.getInput();
-        if (this.get('patch_field')) {
-            patch_payload = val;
-        } else {
-            patch_payload = {};
-            patch_payload[attribute] = val;
-        }
+        patch_payload = {};
+        patch_payload[attribute] = val;
 
         var callbacks = {
             on: {
@@ -819,7 +882,11 @@
         if (Y.Lang.isString(result)) {
             return result;
         } else {
+          if (this.get('use_html')) {
+            return result.getHTML(attribute);
+          } else {
             return result.get(attribute);
+          }
         }
     }
 });

=== added file 'lib/lp/app/javascript/lp.ui.js'
--- lib/lp/app/javascript/lp.ui.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/lp.ui.js	2011-02-24 00:26:17 +0000
@@ -0,0 +1,20 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Launchpad helper methods for generic UI stuff.
+ *
+ * @module Y.lp.ui
+ */
+YUI.add('lp.ui', function(Y) {
+
+    var module = Y.namespace('lp.ui');
+
+    module.update_field = function(selector, content)
+    {
+      var element = Y.one(selector);
+      element.set('innerHTML', content);
+      Y.lazr.anim.green_flash({node:element}).run();
+    }
+
+  }, "0.1", {"requires": ["node", "lazr.anim"]}
+);

=== modified file 'lib/lp/app/javascript/picker.js'
--- lib/lp/app/javascript/picker.js	2011-02-24 00:26:17 +0000
+++ lib/lp/app/javascript/picker.js	2011-02-24 00:26:17 +0000
@@ -40,9 +40,6 @@
     var null_display_value = 'None';
     var show_search_box = true;
     resource_uri = Y.lp.client.normalize_uri(resource_uri)
-    var full_resource_uri = Y.lp.client.get_absolute_uri(resource_uri);
-    var current_context_uri = LP.cache['context']['self_link'];
-    var editing_main_context = (full_resource_uri == current_context_uri);
 
     if (config !== undefined) {
         if (config.remove_button_text !== undefined) {
@@ -102,41 +99,13 @@
     var save = function (picker_result) {
         activator.renderProcessing();
         var success_handler = function (entry) {
-            // XXX mars 2009-12-1
-            // The approach we use here is deprecated.  Instead of requesting
-            // the entire entity we should only request the fields we need.
-            // Then most of this code can go away. See bug #490826.
-            var success_message_node = null;
-            var xhtml = Y.Node.create(entry);
-            var current_field = null;
-            var content_uri_has_changed = false;
-            // The entry is an XHTML document with a <dl> node at the root.  We
-            // want to process each <dt><dd> tag pair under that root.
-            var page_uri = null;
-            xhtml.all('dl *').each(function(element) {
-                if (element.get('tagName') == 'DT') {
-                    current_field = element.get('innerHTML');
-                } else if (element.get('tagName') == 'DD') {
-                    if (current_field == attribute_name) {
-                        // The field value is found
-                        success_message_node = Y.Node.create('<span></span>');
-                        rendered_content = element.get('innerHTML');
-                        success_message_node.set('innerHTML', rendered_content);
-                    } else if (current_field == 'web_link') {
-                        page_uri = element.get('innerHTML');
-                    } else if (current_field == 'self_link') {
-                        picker._resource_uri = element.get('innerHTML');
-                        content_uri_has_changed = (
-                            full_resource_uri != picker._resource_uri);
-                    }
-                }
-            });
-            activator.renderSuccess(success_message_node);
-            show_hide_buttons();
-            if (editing_main_context && content_uri_has_changed
-                && page_uri !== null) {
-                window.location = page_uri;
-            }
+
+          var xhtml = entry.getHTML(attribute_name);
+          var success_message_node = Y.Node.create('<span></span>');
+          success_message_node.set('innerHTML', xhtml);
+          activator.renderSuccess(success_message_node);
+          show_hide_buttons();
+          return;
         };
 
         var patch_payload = {};
@@ -145,7 +114,7 @@
 
         var client = new Y.lp.client.Launchpad();
         client.patch(picker._resource_uri, patch_payload, {
-            accept: 'application/xhtml+xml',
+            accept: 'application/json;include=lp_html',
             on: {
                 success: success_handler,
                 failure: failure_handler
@@ -177,6 +146,7 @@
         // Use picker._resource_uri, since it might have been changed
         // from the outside after the widget has already been initialized.
         client.patch(picker._resource_uri, patch_payload, {
+            accept: 'application/json;include=lp_html',
             on: {
                 success: success_handler,
                 failure: failure_handler

=== modified file 'lib/lp/app/templates/base-layout.pt'
--- lib/lp/app/templates/base-layout.pt	2011-02-07 07:08:25 +0000
+++ lib/lp/app/templates/base-layout.pt	2011-02-24 00:26:17 +0000
@@ -194,7 +194,7 @@
     define="render_time modules/canonical.launchpad.webapp.adapter/summarize_requests;"
     replace='structure string:&lt;script type="text/javascript"&gt;
   var render_time = "${render_time} &amp;bull;";
-  LPS.use("node", function(Y) {
+  LPS.use("node", "event-custom", function(Y) {
     Y.on("domready", function() {
       var node = Y.one("#rendertime");
       node.set("innerHTML", render_time);

=== modified file 'lib/lp/app/templates/text-area-editor.pt'
--- lib/lp/app/templates/text-area-editor.pt	2011-01-27 20:25:05 +0000
+++ lib/lp/app/templates/text-area-editor.pt	2011-02-24 00:26:17 +0000
@@ -23,9 +23,8 @@
             widget.editor.plug({
                 fn: Y.lp.client.plugins.PATCHPlugin, cfg: {
                   patch: ${view/json_attribute},
-                  resource: ${view/json_attribute_uri},
-                  patch_field: true,
-                  accept: 'application/xhtml+xml'
+                  resource: ${view/json_resource_uri},
+                  use_html: true
             }});
             if (!Y.UA.opera) {
                 widget.render();

=== modified file 'lib/lp/app/templates/text-line-editor.pt'
--- lib/lp/app/templates/text-line-editor.pt	2011-01-27 20:25:05 +0000
+++ lib/lp/app/templates/text-line-editor.pt	2011-02-24 00:26:17 +0000
@@ -17,8 +17,7 @@
             widget.editor.plug({
                 fn: Y.lp.client.plugins.PATCHPlugin, cfg: {
                   patch: ${view/json_attribute},
-                  resource: ${view/json_attribute_uri},
-                  patch_field: true
+                  resource: ${view/json_resource_uri}
                   }});
             widget.render();
         });

=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
--- lib/lp/code/interfaces/sourcepackagerecipe.py	2011-02-22 23:07:17 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py	2011-02-24 00:26:17 +0000
@@ -45,7 +45,6 @@
     Choice,
     Datetime,
     Int,
-    Object,
     Text,
     TextLine,
     )
@@ -76,14 +75,16 @@
 class ISourcePackageRecipeData(Interface):
     """A recipe as database data, not text."""
 
-    base_branch = Object(
-        schema=IBranch, title=_("Base branch"), description=_(
-            "The base branch to use when building the recipe."))
+    base_branch = exported(
+        Reference(
+            IBranch, title=_("The base branch used by this recipe."),
+            required=True, readonly=True))
 
-    deb_version_template = TextLine(
-        title=_('deb-version template'),
-        description = _(
-            'The template that will be used to generate a deb version.'),)
+    deb_version_template = exported(
+        TextLine(
+            title=_('deb-version template'), readonly=True,
+            description = _(
+                'The template that will be used to generate a deb version.')))
 
     def getReferencedBranches():
         """An iterator of the branches referenced by this recipe."""
@@ -213,9 +214,6 @@
     debianized source tree.
     """
     export_as_webservice_entry()
-    base_branch = Reference(
-        IBranch, title=_("The base branch used by this recipe."),
-        required=True, readonly=True)
 
 
 class ISourcePackageRecipeSource(Interface):

=== added file 'lib/lp/code/javascript/sourcepackagerecipe.index.js'
--- lib/lp/code/javascript/sourcepackagerecipe.index.js	1970-01-01 00:00:00 +0000
+++ lib/lp/code/javascript/sourcepackagerecipe.index.js	2011-02-24 00:26:17 +0000
@@ -0,0 +1,28 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Update the recipe page on context updates.
+ *
+ * @module Y.lp.code.sourcepackagerecipe.index
+ */
+YUI.add('lp.code.sourcepackagerecipe.index', function(Y) {
+
+    var module = Y.namespace('lp.code.sourcepackagerecipe.index');
+
+    function recipe_changed(fields_changed, entry) {
+      if (Y.Array.indexOf(fields_changed, "deb_version_template") >= 0) {
+        Y.lp.ui.update_field(
+          '#debian-version dd', entry.get('deb_version_template'));
+      }
+      if (Y.Array.indexOf(fields_changed, "base_branch_link") >= 0) {
+        Y.lp.ui.update_field(
+          '#base-branch dd', entry.getHTML('base_branch_link'));
+      }
+    }
+
+    module.setup = function() {
+      Y.on('lp:context:changed', recipe_changed);
+    };
+
+  }, "0.1", {"requires": ["lp.ui"]}
+);

=== modified file 'lib/lp/code/templates/sourcepackagerecipe-index.pt'
--- lib/lp/code/templates/sourcepackagerecipe-index.pt	2011-02-24 00:26:17 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-index.pt	2011-02-24 00:26:17 +0000
@@ -136,11 +136,16 @@
   </div>
   <div class='portlet'>
     <h2>Recipe contents</h2>
-        <tal:widget replace="structure view/recipe_text_widget"/>
+    <tal:widget replace="structure view/recipe_text_widget"/>
   </div>
 
-  <tal:script>
-    <script id='requestbuild-script' type="text/javascript" tal:content="string:
+  <script type="text/javascript">
+    LPS.use('lp.code.sourcepackagerecipe.index', function(Y) {
+      Y.on('domready', Y.lp.code.sourcepackagerecipe.index.setup);
+    });
+  </script>
+
+  <script id='requestbuild-script' type="text/javascript" tal:content="string:
       LPS.use('io-base', 'lp.code.requestbuild_overlay', function(Y) {
         if(Y.UA.ie) {
             return;
@@ -160,7 +165,6 @@
       });"
     >
     </script>
-  </tal:script>
 </div>
 </body>
 </html>


Follow ups