launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/js-client-link into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #911973 in Launchpad itself: "Objects returned via JS client have wrong lp_original_uri"
  https://bugs.launchpad.net/launchpad/+bug/911973

For more details, see:
https://code.launchpad.net/~bac/launchpad/js-client-link/+merge/87532

If the object returned by wrap_resource_on_success has a self_link, it
should be used as the URI (eventually set as lp_original_uri).

You'll note I added a test to test_lp_client_integration even though
it remains disabled.
-- 
https://code.launchpad.net/~bac/launchpad/js-client-link/+merge/87532
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/js-client-link into lp:launchpad.

=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js	2011-12-19 09:53:25 +0000
+++ lib/lp/app/javascript/client.js	2012-01-04 20:27:34 +0000
@@ -343,12 +343,17 @@
             representation = Y.JSON.parse(response.responseText);
             // If the response contains a notification header, display the
             // notifications.
-            notificaxns = response.getResponseHeader('X-Lazr-Notifications');
-            if (notificaxns !== null) {
-                module.display_notifications(notificaxns);
+            var notifications = response.getResponseHeader(
+                'X-Lazr-Notifications');
+            if (notifications !== null) {
+                module.display_notifications(notifications);
+            }
+            if (Y.Lang.isValue(representation) &&
+                Y.Lang.isValue(representation.self_link)) {
+                uri = representation.self_link;
             }
             wrapped = client.wrap_resource(uri, representation);
-            result = old_on_success(wrapped);
+            var result = old_on_success(wrapped);
             if (update_cache) {
               module.update_cache(wrapped);
             }

=== modified file 'lib/lp/app/javascript/tests/test_lp_client_integration.js'
--- lib/lp/app/javascript/tests/test_lp_client_integration.js	2011-11-18 17:09:28 +0000
+++ lib/lp/app/javascript/tests/test_lp_client_integration.js	2012-01-04 20:27:34 +0000
@@ -127,13 +127,27 @@
     Y.Assert.areSame(1, config.result.total_size);
   },
 
+  test_named_get_uri: function() {
+    var data = serverfixture.setup(
+      this, 'create_product_with_milestone_and_login');
+    var client = new Y.lp.client.Launchpad({sync: true});
+    var config = makeTestConfig({parameters: {name: data.milestone.name}});
+    var product = new Y.lp.client.Entry(
+      client, data.product, data.product.self_link);
+    product.named_get('getMilestone', config);
+    Y.Assert.isTrue(config.successful, 'Getting milestone failed');
+    var milestone = config.result;
+    Y.Assert.isInstanceOf(Y.lp.client.Entry, milestone);
+    Y.Assert.areSame(data.milestone_self_link, milestone.lp_original_uri);
+  },
+
   test_named_post_integration: function() {
     var data = serverfixture.setup(this, 'create_bug_and_login');
     var client = new Y.lp.client.Launchpad({sync: true});
     var config = makeTestConfig();
     client.named_post(
       data.bug.self_link, 'mute', config);
-    Y.Assert.isTrue(config.successful);
+    Y.Assert.isTrue(config.successful, "named_post failed: " + config.result);
   },
 
   test_follow_link: function() {

=== modified file 'lib/lp/app/javascript/tests/test_lp_client_integration.py.disabled'
--- lib/lp/app/javascript/tests/test_lp_client_integration.py.disabled	2011-11-23 07:14:36 +0000
+++ lib/lp/app/javascript/tests/test_lp_client_integration.py.disabled	2012-01-04 20:27:34 +0000
@@ -7,6 +7,9 @@
 __metaclass__ = type
 __all__ = []
 
+from zope.traversing.browser import absoluteURL
+from lazr.restful.interfaces import IWebServiceClientRequest
+
 from lp.testing import person_logged_in
 from lp.testing.yuixhr import (
     login_as_person,
@@ -38,6 +41,12 @@
 def create_product_and_login(request, data):
     login_as_person(data['user'])
 
+@create_product_and_login.extend
+def create_product_with_milestone_and_login(request, data):
+    data['milestone'] = factory.makeMilestone(
+        product=data['product'])
+    api_request = IWebServiceClientRequest(request)
+    data['milestone_self_link'] = absoluteURL(data['milestone'], api_request)
 
 @create_product_and_login.extend
 def create_bug_and_login(request, data):