Merge lp://staging/~intellectronica/launchpad/bugtask-ajax-target into lp://staging/launchpad
- bugtask-ajax-target
- Merge into devel
Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp://staging/~intellectronica/launchpad/bugtask-ajax-target |
Merge into: | lp://staging/launchpad |
Diff against target: | None lines |
To merge this branch: | bzr merge lp://staging/~intellectronica/launchpad/bugtask-ajax-target |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Paul Hummer (community) | ui | Approve | |
Gavin Panella (community) | code | Approve | |
Review via email:
|
Commit message
Description of the change
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Eleanor Berger (intellectronica) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Gavin Panella (allenap) wrote : | # |
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/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -1208,6 +1208,7 @@
> }
>
> var tr = Y.get('#' + conf.row_id);
> + var bugtarget_content = Y.get('
> var status_content = tr.query(
> var importance_content = tr.query(
> var milestone_content = tr.query(
> @@ -1223,6 +1224,20 @@
> }
>
> if ((LP.client.
> + if (Y.Lang.
> + if (conf.target_
> + var bugtarget_picker = Y.lp.picker.
> + 'Product',
> + conf.bugtask_path,
> + "target_link",
> + bugtarget_
> + true,
> + false,
> + {"step_title": "Search products",
> + "header": "Change product"});
> + }
> + }
> +
> if (conf.user_
> var status_choice_edit = new Y.ChoiceSource({
> contentBox: status_content,
>
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -78,8 +78,23 @@
> var save = function (picker_result) {
> activator.
> var success_handler = function (entry) {
> - var node = Y.Node.create(
> - '<span>' + entry.get(
> + var xhtml = Y.DOM.create(
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(
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.childNode
> + var element = xhtml.childNode
> + if (element.tagName == 'DT') {
> + current_field = element.innerHTML;
We control this field, so I gu...
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Gavin Panella (allenap) wrote : | # |
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://
to a webservice test for the addition of the mutator_for() decorator
to transitionToTar
Thanks!
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Paul Hummer (rockstar) wrote : | # |
After filing bug 419489 and the removal of the "remove" link, I'm happy with this branch.
Preview Diff
1 | === modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js' |
2 | --- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-08-25 10:55:16 +0000 |
3 | +++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-08-26 10:38:56 +0000 |
4 | @@ -1208,6 +1208,7 @@ |
5 | } |
6 | |
7 | var tr = Y.get('#' + conf.row_id); |
8 | + var bugtarget_content = Y.get('#bugtarget-picker-' + conf.row_id); |
9 | var status_content = tr.query('.status-content'); |
10 | var importance_content = tr.query('.importance-content'); |
11 | var milestone_content = tr.query('.milestone-content'); |
12 | @@ -1223,6 +1224,20 @@ |
13 | } |
14 | |
15 | if ((LP.client.links.me !== undefined) && (LP.client.links.me !== null)) { |
16 | + if (Y.Lang.isValue(bugtarget_content)) { |
17 | + if (conf.target_is_product) { |
18 | + var bugtarget_picker = Y.lp.picker.addPickerPatcher( |
19 | + 'Product', |
20 | + conf.bugtask_path, |
21 | + "target_link", |
22 | + bugtarget_content.get('id'), |
23 | + true, |
24 | + false, |
25 | + {"step_title": "Search products", |
26 | + "header": "Change product"}); |
27 | + } |
28 | + } |
29 | + |
30 | if (conf.user_can_edit_status) { |
31 | var status_choice_edit = new Y.ChoiceSource({ |
32 | contentBox: status_content, |
33 | |
34 | === modified file 'lib/canonical/launchpad/javascript/lp/picker.js' |
35 | --- lib/canonical/launchpad/javascript/lp/picker.js 2009-08-05 14:02:16 +0000 |
36 | +++ lib/canonical/launchpad/javascript/lp/picker.js 2009-08-26 10:04:41 +0000 |
37 | @@ -78,8 +78,23 @@ |
38 | var save = function (picker_result) { |
39 | activator.renderProcessing(); |
40 | var success_handler = function (entry) { |
41 | - var node = Y.Node.create( |
42 | - '<span>' + entry.get('assignee_link') + '</span>'); |
43 | + var xhtml = Y.DOM.create(entry)[1]; |
44 | + var current_field = null; |
45 | + for (var i = 0; i < xhtml.childNodes.length; i++) { |
46 | + var element = xhtml.childNodes[i]; |
47 | + if (element.tagName == 'DT') { |
48 | + current_field = element.innerHTML; |
49 | + continue; |
50 | + } |
51 | + if (element.tagName == 'DD') { |
52 | + if (current_field == attribute_name) { |
53 | + // The field value is found |
54 | + node = Y.get(element).query('span'); |
55 | + } else if (current_field == 'self_link') { |
56 | + picker._resource_uri = element.innerHTML; |
57 | + } |
58 | + } |
59 | + } |
60 | activator.renderSuccess(node); |
61 | show_hide_buttons(); |
62 | }; |
63 | @@ -89,7 +104,7 @@ |
64 | picker_result.api_uri); |
65 | |
66 | var client = new LP.client.Launchpad(); |
67 | - client.patch(resource_uri, patch_payload, { |
68 | + client.patch(picker._resource_uri, patch_payload, { |
69 | accept: 'application/xhtml+xml', |
70 | on: { |
71 | success: success_handler, |
72 | @@ -119,7 +134,7 @@ |
73 | patch_payload[attribute_name] = null; |
74 | |
75 | var client = new LP.client.Launchpad(); |
76 | - client.patch(resource_uri, patch_payload, { |
77 | + client.patch(picker._resource_uri, patch_payload, { |
78 | on: { |
79 | success: success_handler, |
80 | failure: failure_handler |
81 | @@ -128,6 +143,7 @@ |
82 | }; |
83 | |
84 | var picker = Y.lp.picker.create(vocabulary, save, config); |
85 | + picker._resource_uri = resource_uri; |
86 | var extra_buttons = Y.Node.create( |
87 | '<div style="text-align: center; height: 3em; ' + |
88 | 'white-space: nowrap"/>'); |
89 | @@ -158,6 +174,8 @@ |
90 | activator.render(); |
91 | |
92 | show_hide_buttons(); |
93 | + |
94 | + return picker; |
95 | }; |
96 | |
97 | /** |
98 | @@ -297,5 +315,5 @@ |
99 | }, '0.1', { |
100 | requires: [ |
101 | 'io', 'dom', 'dump', 'lazr.picker', 'lazr.activator', 'json-parse', |
102 | - 'lp.client.helpers' |
103 | + 'datatype-xml', 'lp.client.helpers' |
104 | ]}); |
105 | |
106 | === modified file 'lib/lp/bugs/browser/bugtask.py' |
107 | --- lib/lp/bugs/browser/bugtask.py 2009-08-25 12:35:23 +0000 |
108 | +++ lib/lp/bugs/browser/bugtask.py 2009-08-26 10:38:56 +0000 |
109 | @@ -10,6 +10,7 @@ |
110 | 'BugListingBatchNavigator', |
111 | 'BugListingPortletView', |
112 | 'BugNominationsView', |
113 | + 'bugtarget_renderer', |
114 | 'BugTargetTraversalMixin', |
115 | 'BugTargetView', |
116 | 'BugTaskContextMenu', |
117 | @@ -76,7 +77,7 @@ |
118 | from lazr.lifecycle.snapshot import Snapshot |
119 | from lazr.restful.interface import copy_field |
120 | from lazr.restful.interfaces import ( |
121 | - IFieldHTMLRenderer, IReferenceChoice, IWebServiceClientRequest) |
122 | + IFieldHTMLRenderer, IReference, IReferenceChoice, IWebServiceClientRequest) |
123 | |
124 | from canonical.config import config |
125 | from canonical.database.sqlbase import cursor |
126 | @@ -137,7 +138,8 @@ |
127 | from canonical.launchpad.webapp.authorization import check_permission |
128 | from canonical.launchpad.webapp.batching import TableBatchNavigator |
129 | from canonical.launchpad.webapp.menu import structured |
130 | -from canonical.launchpad.webapp.tales import PersonFormatterAPI, FormattersAPI |
131 | +from canonical.launchpad.webapp.tales import ( |
132 | + FormattersAPI, ObjectImageDisplayAPI, PersonFormatterAPI) |
133 | |
134 | from canonical.lazr.interfaces import IObjectPrivacy |
135 | from lazr.restful.interfaces import IJSONRequestCache |
136 | @@ -161,7 +163,24 @@ |
137 | def assignee_renderer(context, field, request): |
138 | """Render a bugtask assignee as a link.""" |
139 | def render(value): |
140 | - return PersonFormatterAPI(context.assignee).link('+assignedbugs') |
141 | + if context.assignee is None: |
142 | + return '' |
143 | + else: |
144 | + return PersonFormatterAPI(context.assignee).link(None) |
145 | + return render |
146 | + |
147 | +@component.adapter(IBugTask, IReference, IWebServiceClientRequest) |
148 | +@implementer(IFieldHTMLRenderer) |
149 | +def bugtarget_renderer(context, field, request): |
150 | + """Render a bugtarget as a link.""" |
151 | + def render(value): |
152 | + html = """<span> |
153 | + <a href="%(href)s" class="%(class)s">%(displayname)s</a> |
154 | + </span>""" % { |
155 | + 'href': canonical_url(context.target), |
156 | + 'class': ObjectImageDisplayAPI(context.target).sprite_css(), |
157 | + 'displayname': context.bugtargetdisplayname} |
158 | + return html |
159 | return render |
160 | |
161 | def unique_title(title): |
162 | @@ -3151,6 +3170,7 @@ |
163 | 'bugtask_path': '/'.join( |
164 | [''] + canonical_url(self.context).split('/')[3:]), |
165 | 'prefix': get_prefix(self.context), |
166 | + 'target_is_product': IProduct.providedBy(self.context.target), |
167 | 'status_widget_items': self.status_widget_items, |
168 | 'status_value': self.context.status.title, |
169 | 'importance_widget_items': self.importance_widget_items, |
170 | |
171 | === modified file 'lib/lp/bugs/configure.zcml' |
172 | --- lib/lp/bugs/configure.zcml 2009-08-24 20:28:33 +0000 |
173 | +++ lib/lp/bugs/configure.zcml 2009-08-25 13:51:39 +0000 |
174 | @@ -270,11 +270,13 @@ |
175 | factory="lp.bugs.browser.bugtask.assignee_renderer" |
176 | name="assignee"/> |
177 | <adapter |
178 | + factory="lp.bugs.browser.bugtask.bugtarget_renderer" |
179 | + name="target"/> |
180 | + <adapter |
181 | provides="canonical.launchpad.webapp.interfaces.IBreadcrumb" |
182 | for="lp.bugs.interfaces.bugtask.IBugTask" |
183 | factory="lp.bugs.browser.bugtask.BugTaskBreadcrumb" |
184 | permission="zope.Public"/> |
185 | - |
186 | |
187 | <!-- NullBugTask --> |
188 | |
189 | |
190 | === modified file 'lib/lp/bugs/interfaces/bugtask.py' |
191 | --- lib/lp/bugs/interfaces/bugtask.py 2009-08-20 01:11:52 +0000 |
192 | +++ lib/lp/bugs/interfaces/bugtask.py 2009-08-25 13:51:39 +0000 |
193 | @@ -634,6 +634,7 @@ |
194 | value is set to None, date_assigned is also set to None. |
195 | """ |
196 | |
197 | + @mutator_for(target) |
198 | @operation_parameters( |
199 | target=copy_field(target)) |
200 | @export_write_operation() |
201 | |
202 | === modified file 'lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt' |
203 | --- lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt 2009-08-21 16:23:58 +0000 |
204 | +++ lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt 2009-08-26 10:38:56 +0000 |
205 | @@ -36,12 +36,20 @@ |
206 | </tal:not-conjoined-task> |
207 | </td> |
208 | <td tal:condition="not:indent_task"> |
209 | - <a tal:attributes=" |
210 | - href context/target/fmt:url; |
211 | - title view/target_link_title; |
212 | - class context/target/image:sprite_css" |
213 | - tal:content="context/bugtargetdisplayname" |
214 | - /> |
215 | + <span tal:attributes="id string:bugtarget-picker-${row_id}"> |
216 | + <span class="yui-activator-data-box"> |
217 | + <span title="This project’s license has not been specified."> |
218 | + <a tal:attributes="href context/target/fmt:url; |
219 | + title view/target_link_title; |
220 | + class context/target/image:sprite_css" |
221 | + tal:content="context/bugtargetdisplayname" /> |
222 | + </span> |
223 | + </span> |
224 | + <button class="lazr-btn yui-activator-act yui-activator-hidden"> |
225 | + Edit |
226 | + </button> |
227 | + <div class="yui-activator-message-box yui-activator-hidden" /> |
228 | + </span> |
229 | </td> |
230 | |
231 | <tal:conjoined-task condition="is_conjoined_slave"> |
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.