Merge lp:~allenap/launchpad/deriveDistroSeries-json-weirdness-bug-753249 into lp:launchpad

[Gavin Panella]

[r=gary][bug=753249] Enable append_qs() to deal with array argument values.

append_qs() in client.js could not deal with array argument values
properly. This is something that any API call that accepts a list
needs, so I've implemented it.

I've manually verified that this fixes the linked bug. I started work
on a Windmill test, but got stupid errors from Windmill regarding
profiles in Firefox. Purging Firefox and re-installing makes no
difference. I've dumped the work I've started into another branch and
I'll file a bug for that. Meanwhile I want to get this landed.

[Gary Poster (gary)]

Very nice, Gavin. Thank you.

If you haven't run "make lint," please do--I have been led to believe that we now have Crockford's jslint hooked up.

As a matter of sharing a discussion we've been having, we've been talking about putting "var" outside of for statements, to accurately represent scope. Crockford apparently recommends this. Do what you will, if make lint does not complain. :-)

review: Approve

=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js	2011-04-19 18:14:20 +0000
+++ lib/lp/app/javascript/client.js	2011-04-26 20:56:03 +0000
@@ -86,14 +86,18 @@
 // Generally useful functions.
 module.append_qs = function(qs, key, value) {
     /* Append a key-value pair to a query string. */
-    if (qs === undefined) {
-        qs = "";
-    }
-    if (qs.length > 0) {
-        qs += '&';
-    }
-    qs += encodeURIComponent(key) + "=" + encodeURIComponent(value);
-    return qs;
+    var elems = (qs && qs.length > 0) ? [qs] : [];
+    var enc = encodeURIComponent;
+    if (Y.Lang.isArray(value)) {
+        var index;
+        for (index = 0; index < value.length; index++) {
+            elems.push(enc(key) + "=" + enc(value[index]));
+        }
+    }
+    else {
+        elems.push(enc(key) + "=" + enc(value));
+    }
+    return elems.join("&");
 };
 
 module.normalize_uri = function(uri) {

=== modified file 'lib/lp/app/javascript/tests/test_lp_client.js'
--- lib/lp/app/javascript/tests/test_lp_client.js	2011-04-11 12:52:27 +0000
+++ lib/lp/app/javascript/tests/test_lp_client.js	2011-04-26 20:56:03 +0000
@@ -19,12 +19,19 @@
 
     test_normalize_uri: function() {
         var normalize = Y.lp.client.normalize_uri;
-        Assert.areEqual(normalize("http://www.example.com/api/devel/foo"), "/api/devel/foo");
-        Assert.areEqual(normalize("http://www.example.com/foo/bar"), "/foo/bar");
-        Assert.areEqual(normalize("/foo/bar"), "/api/devel/foo/bar");
-        Assert.areEqual(normalize("/api/devel/foo/bar"), "/api/devel/foo/bar");
-        Assert.areEqual(normalize("foo/bar"), "/api/devel/foo/bar");
-        Assert.areEqual(normalize("api/devel/foo/bar"), "/api/devel/foo/bar");
+        Assert.areEqual(
+            normalize("http://www.example.com/api/devel/foo"),
+            "/api/devel/foo");
+        Assert.areEqual(
+            normalize("http://www.example.com/foo/bar"), "/foo/bar");
+        Assert.areEqual(
+            normalize("/foo/bar"), "/api/devel/foo/bar");
+        Assert.areEqual(
+            normalize("/api/devel/foo/bar"), "/api/devel/foo/bar");
+        Assert.areEqual(
+            normalize("foo/bar"), "/api/devel/foo/bar");
+        Assert.areEqual(
+            normalize("api/devel/foo/bar"), "/api/devel/foo/bar");
     },
 
     test_append_qs: function() {
@@ -33,14 +40,28 @@
         Assert.areEqual("P%C3%83%C2%B6ll%C3%83%C2%A4=Perell%C3%83%C2%B3", qs);
     },
 
+    test_append_qs_with_array: function() {
+        // append_qs() appends multiple arguments to the query string
+        // when a parameter value is an array.
+        var qs = "";
+        qs = Y.lp.client.append_qs(qs, "foo", ["bar", "baz"]);
+        Assert.areEqual("foo=bar&foo=baz", qs);
+        // All values in the array are encoded correctly too.
+        qs = Y.lp.client.append_qs(qs, "a&b", ["a+b"]);
+        Assert.areEqual("foo=bar&foo=baz&a%26b=a%2Bb", qs);
+    },
+
     test_field_uri: function() {
       var get_field_uri = Y.lp.client.get_field_uri;
-      Assert.areEqual(get_field_uri("http://www.example.com/api/devel/foo", "field"),
-                      "/api/devel/foo/field");
-      Assert.areEqual(get_field_uri("/no/slash", "field"),
-                      "/api/devel/no/slash/field");
-      Assert.areEqual(get_field_uri("/has/slash/", "field"),
-                      "/api/devel/has/slash/field");
+      Assert.areEqual(
+          get_field_uri("http://www.example.com/api/devel/foo", "field"),
+          "/api/devel/foo/field");
+      Assert.areEqual(
+          get_field_uri("/no/slash", "field"),
+          "/api/devel/no/slash/field");
+      Assert.areEqual(
+          get_field_uri("/has/slash/", "field"),
+          "/api/devel/has/slash/field");
     }
 }));
 
@@ -90,7 +111,9 @@
           'lp_html': {'first': "<p>Hello</p><p>World</p>"}
         };
         var entry = new Y.lp.client.Entry(null, entry_repr, "a_self_link");
-        Assert.areEqual("<p>Hello</p><p>World</p>", entry.getHTML('first').get('innerHTML'));
+        Assert.areEqual(
+            "<p>Hello</p><p>World</p>",
+            entry.getHTML('first').get('innerHTML'));
         // If there is no html representation, null is returned.
         Assert.areEqual(null, entry.getHTML('second'));
       },
@@ -152,7 +175,9 @@
         Assert.areEqual('first', first_event.name);
         Assert.areEqual('Hello', first_event.old_value);
         Assert.areEqual('World<boo/>', first_event.new_value);
-        Assert.areEqual('<p>World html<boo></boo></p>', first_event.new_value_html.get('innerHTML'));
+        Assert.areEqual(
+            '<p>World html<boo></boo></p>',
+            first_event.new_value_html.get('innerHTML'));
         Assert.areEqual(entry, first_event.entry);
 
         Assert.areEqual('second', second_event.name);