Merge lp://staging/~intellectronica/launchpad/bugtask-ajax-target into lp://staging/launchpad

This branch implements an ajax product picker, which allows the user to change the target of a bug inline. It uses the picker infrastructure added by Edwin (for assignee, originally) though I had to make some modifications to make it work.

 * The picker now works with the new entry format returned from the server. The results is now formatted using XHTML so extracting the rendered value is a bit more complicated. This code can probably be factored out later for general use.

 * Because the canonical_url of a bugtask depends on its target, after the target changes the resource to patch changes to. To make that happen I had to resort to hack in which I keep the URI in a field of the picker, instead of using a closure, so that it can be changed later. Ideally, this should be done with proper YUI3 attributes and exposed using events, but that's beyond the scope of this branch and I don't want to block on it.

 * I have also had to add a formatter adapter for rendering the xhtml representation of a bug target, and fix a problem with the assignee formatter where it would bomb if assignee is None (all formatters are now being used for all attributes when the xhtml representation is required, so the attribute might be None if the task is unassigned).

To test, go to a bug with a product task and change the target.

Hi Tom,

I have a lot of comments about this branch, but I think it's a nice
feature and it works well.

Gavin.

> === modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
> --- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-08-25 10:55:16 +0000
> +++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-08-26 11:02:06 +0000
> @@ -1208,6 +1208,7 @@
> }
>
> var tr = Y.get('#' + conf.row_id);
> + var bugtarget_content = Y.get('#bugtarget-picker-' + conf.row_id);
> var status_content = tr.query('.status-content');
> var importance_content = tr.query('.importance-content');
> var milestone_content = tr.query('.milestone-content');
> @@ -1223,6 +1224,20 @@
> }
>
> if ((LP.client.links.me !== undefined) && (LP.client.links.me !== null)) {
> + if (Y.Lang.isValue(bugtarget_content)) {
> + if (conf.target_is_product) {
> + var bugtarget_picker = Y.lp.picker.addPickerPatcher(
> + 'Product',
> + conf.bugtask_path,
> + "target_link",
> + bugtarget_content.get('id'),
> + true,
> + false,
> + {"step_title": "Search products",
> + "header": "Change product"});
> + }
> + }
> +
> if (conf.user_can_edit_status) {
> var status_choice_edit = new Y.ChoiceSource({
> contentBox: status_content,
>
> === modified file 'lib/canonical/launchpad/javascript/lp/picker.js'
> --- lib/canonical/launchpad/javascript/lp/picker.js 2009-08-05 14:02:16 +0000
> +++ lib/canonical/launchpad/javascript/lp/picker.js 2009-08-26 11:02:06 +0000
> @@ -78,8 +78,23 @@
> var save = function (picker_result) {
> activator.renderProcessing();
> var success_handler = function (entry) {
> - var node = Y.Node.create(
> - '<span>' + entry.get('assignee_link') + '</span>');
> + var xhtml = Y.DOM.create(entry)[1];

This is fragile; the reason is that the first element returned from
Y.DOM.create() is a text node containing "\n\n", which is the text
after the XML PI and the start of the dl. If the rendering of this
ever changes it'll break.

The following strips the XML PI to demonstrate the problem.

    entry = entry.replace(/<[?].+[?]>/, "");
    var xhtml = Y.DOM.create(entry); // No [1]

It seems that Y.DOM.create considers whitespace after the PI to be
significant, while unenclosed whitespace is not. I don't know if this
is correct behaviour, but it's annoying.

Perhaps it's possible to change the XML renderer to remove this
whitespace? Or add a test to make sure it's always there? Mmm, but
even then, I just get a gut feeling that this is not behaviour we
should rely upon in YUI; it might change. Gah! Software is sh*t :)

> + var current_field = null;
> + for (var i = 0; i < xhtml.childNodes.length; i++) {
> + var element = xhtml.childNodes[i];
> + if (element.tagName == 'DT') {
> + current_field = element.innerHTML;

We control this field, so I gu...

Tom said on IRC:

  "i appreciate that the code for extracting the xhtml is ugly (i
   even wrote so in my mp), but since it works, and since we
   control both the renderer and the parser, i think it's fine to
   defer improving it until we write a nice general purpose
   extractor later on."

  "i also don't think it's worth adding code for handling
   in-the-air collisions. we never do"

Which is all fine with me.

So, with the changes at http://pastebin.ubuntu.com/259896/ in addition
to a webservice test for the addition of the mutator_for() decorator
to transitionToTarget(), merge approved :)

Thanks!

After filing bug 419489 and the removal of the "remove" link, I'm happy with this branch.