Code review comment for lp:~gmb/launchpad/prince-of-ajax-dupefinding-bug-358510

Revision history for this message
Graham Binns (gmb) wrote :

This branch lays the groundwork for making the +filebug dupefinder
asynchronous. The branch contains several JavaScript changes designed to
make async dupefinding work; once this branch lands a subsequent branch
will actually switch those changes on and hook them up as appropriate.

The contents of the branch have been reviewed before, but those branches
got far too out of date to be merged, so I'm resubmitting this for
review.

Here's a list of the changes I've made:

== lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js ==

 - I've added a couple of globals which will be used to hold references
   to the search field and search button on the +filebug form.
 - I've updated show_bug_reporting_form to cope with situations where
   there aren't any suggested dupes. I've also altered the function so
   that it's ready for the switch to async dupe finding, whereafter the
   filebug form will be loaded in a hidden container.
 - I've added a function, search_for_and_display_dupes(). This function
   takes care of the asynchronous dupefinding. It grabs the duplicate
   search URL from the +filebug page and uses it to make a GET request
   to Launchpad for the list of possible duplicates for a given bug
   title, then updates the page with that list. This function is
   currently dormant (see below).
 - I've added a function, set_up_dupe_finder, which will be called when
   the async dupe finder functionality is turned on and which sets up
   the dupe finder to work asynchronously.
 - I've moved the body of the old Y.bugs.setup_dupe_finder() into a
   function called Y.bugs.setup_dupes() (its previous name was something
   of a misnomer, really).
 - I've made Y.bugs.setup_dupe_finder() into a much smaller function,
   which will check for the ability to load duplicates asynchronously
   and then call set_up_dupe_finder() appropriately. Currently, this
   function isn't called anywhere, thus disabling the async dupe search.

== lib/canonical/launchpad/templates/launchpad-form.pt ==

 - I've added an 'id' attribute to the launchpad form macro, so that we
   can pass identifiers to forms to be able to better grab them with
   JavaScript.

== lib/lp/bugs/templates/bugtarget-filebug-submit-bug.pt ==

 - I've updated this template to call Y.bugs.setup_dupes() instead of
   .setup_dupe_finder()

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js
  lib/canonical/launchpad/templates/launchpad-form.pt
  lib/lp/bugs/templates/bugtarget-filebug-submit-bug.pt

== JSLint notices ==
jslint: No problem found in '/home/graham/canonical/lp-branches/prince-of-ajax-dupefinding-bug-358510/lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js'.

jslint: 1 file to lint.

« Back to merge proposal