Merge lp:~leonardr/launchpad/use-web-link into lp:launchpad

[Tim Penhey]

[r=jtv] [ui=none][bug=316694][incr] Take advantage of web_link when changing the URL in the browser bar, rather than hacking self_link into web_link.

This branch removes some tech debt. Previously, when the picker edit widget made a PATCH request that changed the URL of the context object (such as changing the target of a bug), the new URL was determined by search-and-replacing the new self_link.

Now that bug 316694 has been fixed, the document returned by the PATCH request includes a web_link, which is the value we were hacking out of self_link. This branch makes the code that retrieves self_link (which we need, to see whether the URL has changed) also look for web_link, and use it directly rather than deriving it from self_link.

There's at least one other place where we hack self_link into web_link, but it's not as easy to fix, so I'm submitting this branch in the meantime.

To test:

1. Visit https://bugs.launchpad.dev/firefox/+bug/1

Edit the project by clicking the edit button beneath "Affects", doing a search for "launchpad", and selecting Launchpad as the project.

The URL in the browser address bar should change to https://bugs.launchpad.dev/launchpad/+bug/1.

2. Create a recipe for a branch.

Change its owner, and the browser address bar should also change.

Change its daily build archive, and the browser address bar should stay as is.

[Jeroen T. Vermeulen (jtv)]

I can't say I like the "&&" at the beginning of a line, but I won't force my tastes on you. Otherwise, looks fine.

As per IRC, we're not foreseeing any legitimate scenario under which web_link might not be present, so the one obvious path of breakage is not to be expected.

Jeroen

review: Approve (code)

=== modified file 'lib/lp/app/javascript/picker.js'
--- lib/lp/app/javascript/picker.js	2011-01-27 02:09:37 +0000
+++ lib/lp/app/javascript/picker.js	2011-02-09 16:20:30 +0000
@@ -112,6 +112,7 @@
             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');
@@ -121,6 +122,8 @@
                         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 = (
@@ -130,15 +133,10 @@
             });
             activator.renderSuccess(success_message_node);
             show_hide_buttons();
-            if (editing_main_context && content_uri_has_changed) {
-              // XXX Tim Penhey 2011-01-18 Bug #316694:
-              // This is a slightly nasty hack that saves us from the need
-              // to have a more established way of getting the web URL of
-              // an API object. Once such a solution is available we should
-              // fix this.
-              var new_url = picker._resource_uri.replace('/api/devel', '');
-              window.location = new_url;
-          }
+            if (editing_main_context && content_uri_has_changed
+                && page_uri !== null) {
+                window.location = page_uri;
+            }
         };
 
         var patch_payload = {};