Merge lp:~rockstar/launchpad/fix-launchpad-dev into lp:launchpad

[Paul Hummer]

[Paul Hummer (rockstar)]

This branch fixes bug #403839 - There was a launchpad.dev link being generated
by the javascript.

I could've just fixed the HTML being generated, but I thought I'd rather
experiment with dealing with entire page fragments. The UI is exactly the same
as before (sans the bugfix), but this way, I'm not duplicating HTML.

The way that I do this is subject to change, as I have two more branches to
play around with this idea. I've chatted with a few people about this, and
each fragment implementation has different pros/cons, and I'd like to find the
easiest to follow, and metal macros are not the easiest.

To test, run the windmill tests in lp/code/windmill/test_branch_links.py

 reviewer deryck

Cheers,
Paul

[Deryck Hodge (deryck)]

Hi, Paul.

As we discussed on IRC, I think you need better error handling
here that just an alert since you have two-levels of XHR success
callbacks. I think you should make use of LP.client.ErrorHandler.
Our style guide also calls for JSDoc comments, so the comments need
updating throughout.

I'm marking this "Approve" based on these things being done, and since
otherwise, this code looks really nice. I like the approach you've
taken here! I do like the way you can build HTML in TAL. It makes
for more readable JS code, certainly.

Cheers,
deryck

review: Approve

=== modified file 'lib/canonical/launchpad/javascript/code/branchlinks.js'
--- lib/canonical/launchpad/javascript/code/branchlinks.js	2009-07-14 06:10:05 +0000
+++ lib/canonical/launchpad/javascript/code/branchlinks.js	2009-07-28 18:25:15 +0000
@@ -14,6 +14,9 @@
 
 var link_bug_overlay;
 
+/*
+ * Connect the links to the javascript events.
+ */
 Y.branchlinks.connect_branchlinks = function() {
 
     link_bug_overlay = new Y.lazr.FormOverlay({
@@ -53,6 +56,10 @@
 
 };
 
+/*
+ * Connect the remove links of each bug link to the javascript functions to
+ * remove the links.
+ */
 function connect_remove_links() {
     var linked_bugs = Y.all('.delete-buglink');
     if (Y.Lang.isValue(linked_bugs)) {
@@ -64,6 +71,9 @@
     }
 }
 
+/*
+ * Link a specified bug to the branch.
+ */
 function link_bug_to_branch(data) {
     link_bug_overlay.hide();
 
@@ -88,7 +98,7 @@
                     var config = {
                         on: {
                             success: function(bugtasks) {
-                                add_html_bug(bug, bugtasks);
+                                update_bug_links(bug);
                             }
                         }
                     };
@@ -109,6 +119,39 @@
     });
 }
 
+/*
+ * Update the list of bug links.
+ */
+function update_bug_links(bug) {
+
+    BUG_LINK_SNIPPET = '++bug-links';
+    Y.io(BUG_LINK_SNIPPET, {
+        on: {
+            success: function(id, response) {
+                destroy_temporary_spinner();
+                Y.get('#linkbug').set(
+                    'innerHTML', 'Link to another bug report');
+                Y.get('#buglink-list').set('innerHTML', response.responseText);
+                var new_buglink = Y.get('#buglink-' + bug.get('id'));
+                var anim = Y.lazr.anim.green_flash({node: new_buglink});
+                anim.on('end', connect_remove_links);
+                anim.run();
+            },
+            failure: function(id, response) {
+                // At least remove the "Linking..." text
+                destroy_temporary_spinner();
+
+                alert('Unable to update bug links.');
+                Y.log(response);
+            }
+        }
+    });
+
+}
+
+/*
+ * Unlink a bug from the branch.
+ */
 function unlink_bug_from_branch(bugnumber) {
     link_bug_overlay.hide();
 
@@ -191,52 +234,6 @@
     }
 }
 
-
-function add_html_bug(bug, bugtasks) {
-
-    var bugnumber = parseInt(bug.get('id'), 10);
-    var bugtitle = bug.get('title').replace('<', '&lt;').replace('>', '&gt;');
-    var bugimportance = bugtasks.entries[0].importance;
-    var bugstatus = bugtasks.entries[0].status;
-
-    var text = [
-        '<div id="buglink-' + bugnumber + '">',
-        '<div class="bug-branch-summary">',
-        '<a href="https://bugs.launchpad.dev/bug/' + bugnumber + '"',
-        'class="sprite bug-' + bugimportance.toLowerCase() + '">',
-        'Bug #' + bugnumber + ': ' + bugtitle,
-        '</a> ',
-        '(<span class="importance' + bugimportance.toUpperCase() + '">',
-        bugimportance + '</span> &ndash;',
-        '<span class="status' + bugstatus.toUpperCase(),
-        '">' + bugstatus + '</span>)',
-        '<a title="Remove link" class="delete-buglink" href="+bug/',
-        bugnumber + '/+delete"',
-        'id="delete-buglink-' + bugnumber + '">',
-        '<img src="/@@/remove" />',
-        '</a>',
-        '</div>',
-        '</div>'].join('');
-    var new_buglink = Y.Node.create(text);
-
-    var buglinks = Y.get('#buglinks');
-    var last = Y.get('#linkbug').get('parentNode');
-    if (last) {
-        buglinks.insertBefore(new_buglink, last);
-    }
-
-    var temp_spinner = Y.get('#temp-spinner');
-    var spinner_parent = temp_spinner.get('parentNode');
-    spinner_parent.removeChild(temp_spinner);
-
-    anim = Y.lazr.anim.green_flash({node: new_buglink});
-    anim.run();
-    connect_remove_links();
-    Y.get("[id='field.bug']").set('value', '');
-    Y.get('#linkbug').set('innerHTML', 'Link to another bug');
-}
-
-
 /*
  * Show the temporary "Linking..." text
  */
@@ -252,5 +249,15 @@
     }
 }
 
+/*
+ * Destroy the temporary "Linking..." text
+ */
+function destroy_temporary_spinner() {
+
+    var temp_spinner = Y.get('#temp-spinner');
+    var spinner_parent = temp_spinner.get('parentNode');
+    spinner_parent.removeChild(temp_spinner);
+
+}
 
 }, '0.1', {requires: ['base', 'lazr.anim', 'lazr.formoverlay']});

=== modified file 'lib/canonical/launchpad/pagetitles.py'
--- lib/canonical/launchpad/pagetitles.py	2009-07-17 00:26:05 +0000
+++ lib/canonical/launchpad/pagetitles.py	2009-07-28 16:36:26 +0000
@@ -170,6 +170,8 @@
 branch_associations = ContextDisplayName(smartquote(
     '"%s" branch associations'))
 
+branch_bug_links = ContextDisplayName(smartquote('Bug links for %s'))
+
 branch_delete = ContextDisplayName(smartquote('Delete branch "%s"'))
 
 branch_edit = ContextDisplayName(smartquote('Change "%s" branch details'))

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2009-07-17 00:26:05 +0000
+++ lib/lp/code/browser/configure.zcml	2009-07-28 16:36:26 +0000
@@ -347,6 +347,9 @@
         <browser:page
             name="+merges"
             template="../templates/branch-merges.pt"/>
+        <browser:page
+            name="++bug-links"
+            template="../templates/branch-bug-links.pt"/>
     </browser:pages>
     <browser:pages
         for="lp.code.interfaces.branch.IBranch"

=== added file 'lib/lp/code/templates/branch-bug-links.pt'
--- lib/lp/code/templates/branch-bug-links.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/branch-bug-links.pt	2009-07-28 16:36:26 +0000
@@ -0,0 +1,11 @@
+<div
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n">
+
+  <tal:bugs tal:define="branch context;
+                        show_edit python:True;">
+    <metal:bug-branch-links use-macro="context/@@+macros/bug-branch-links"/>
+  </tal:bugs>
+
+</div>

=== modified file 'lib/lp/code/templates/branch-index.pt'
--- lib/lp/code/templates/branch-index.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/code/templates/branch-index.pt	2009-07-28 18:25:15 +0000
@@ -398,10 +398,12 @@
   <div id="related-bugs-and-blueprints">
     <h2>Linked bug reports and blueprints</h2>
     <div id="buglinks" class="actions">
-      <tal:bugs tal:define="branch context;
-                            show_edit python:True;">
-        <metal:bug-branch-links use-macro="context/@@+macros/bug-branch-links"/>
-      </tal:bugs>
+      <div id="buglink-list">
+        <tal:bugs tal:define="branch context;
+                              show_edit python:True;">
+          <metal:bug-branch-links use-macro="context/@@+macros/bug-branch-links"/>
+        </tal:bugs>
+      </div>
 
       <div
         tal:define="link context_menu/link_bug"