Merge lp:~abentley/launchpad/reuse-refactor into lp:launchpad
- reuse-refactor
- Merge into devel
Proposed by
Aaron Bentley
Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp:~abentley/launchpad/reuse-refactor |
Merge into: | lp:launchpad |
Diff against target: |
668 lines 7 files modified
lib/canonical/launchpad/interfaces/_schema_circular_imports.py (+2/-0) lib/canonical/launchpad/javascript/bugs/bugtask-index.js (+15/-155) lib/canonical/launchpad/javascript/lp/comment.js (+191/-0) lib/canonical/launchpad/javascript/lp/errors.js (+81/-0) lib/lp/bugs/templates/bugtask-index.pt (+14/-0) lib/lp/code/interfaces/branchmergeproposal.py (+6/-4) lib/lp/code/stories/webservice/xx-branchmergeproposal.txt (+4/-2) |
To merge this branch: | bzr merge lp:~abentley/launchpad/reuse-refactor |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | code | Approve | |
Review via email: mp+13283@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote : | # |
Revision history for this message
Brad Crittenden (bac) wrote : | # |
Aaron this is a great change. Thanks for moving the comment and error functionality to a place they can be re-used.
The only tiny nit I have is at line 645 of the diff where you have an in-line comment, which our coding standards discourage.
review:
Approve
(code)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py' |
2 | --- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-09-09 11:28:03 +0000 |
3 | +++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-10-14 13:58:19 +0000 |
4 | @@ -100,6 +100,8 @@ |
5 | IBranchMergeProposal['createComment'].queryTaggedValue( |
6 | LAZR_WEBSERVICE_EXPORTED)['params']['parent'].schema = \ |
7 | ICodeReviewComment |
8 | +patch_entry_return_type( |
9 | + IBranchMergeProposal, 'createComment', ICodeReviewComment) |
10 | IBranchMergeProposal['all_comments'].value_type.schema = ICodeReviewComment |
11 | IBranchMergeProposal['nominateReviewer'].queryTaggedValue( |
12 | LAZR_WEBSERVICE_EXPORTED)['return_type'].schema = ICodeReviewVoteReference |
13 | |
14 | === modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js' |
15 | --- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-10-06 21:58:10 +0000 |
16 | +++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-10-14 13:58:19 +0000 |
17 | @@ -83,7 +83,7 @@ |
18 | } |
19 | }; |
20 | error_handler.showError = function(error_msg) { |
21 | - display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
22 | + Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
23 | }; |
24 | |
25 | var config = { |
26 | @@ -210,12 +210,11 @@ |
27 | }); |
28 | privacy_link.addClass('js-action'); |
29 | } |
30 | - create_error_overlay(); |
31 | - setup_inline_commenting(); |
32 | setup_add_attachment(); |
33 | }, window); |
34 | }; |
35 | |
36 | + |
37 | /* |
38 | * Clear the subscribe someone else picker. |
39 | * |
40 | @@ -313,7 +312,6 @@ |
41 | |
42 | setup_unsubscribe_icon_handlers(subscription); |
43 | setup_subscribe_someone_else_handler(subscription); |
44 | - create_error_overlay(); |
45 | } |
46 | |
47 | /* |
48 | @@ -568,64 +566,6 @@ |
49 | lp_bug_entry.lp_save(config); |
50 | }; |
51 | |
52 | -/* |
53 | - * Create the form overlay to use when encountering errors. |
54 | - * |
55 | - * @method create_error_overlay |
56 | -*/ |
57 | -function create_error_overlay() { |
58 | - if (error_overlay === undefined) { |
59 | - error_overlay = new Y.lazr.FormOverlay({ |
60 | - headerContent: '<h2>Error</h2>', |
61 | - form_header: '', |
62 | - form_content: '', |
63 | - form_submit_button: Y.Node.create( |
64 | - '<button style="display:none"></button>'), |
65 | - form_cancel_button: cancel_form_button(), |
66 | - centered: true, |
67 | - visible: false |
68 | - }); |
69 | - error_overlay.render(); |
70 | - } |
71 | -} |
72 | - |
73 | -/* |
74 | - * Create a form button for canceling an error form |
75 | - * that won't reload the page on submit. |
76 | - * |
77 | - * @method cancel_form_button |
78 | - * @return button {Node} The form's cancel button. |
79 | -*/ |
80 | -function cancel_form_button() { |
81 | - var button = Y.Node.create('<button>OK</button>'); |
82 | - button.on('click', function(e) { |
83 | - e.preventDefault(); |
84 | - error_overlay.hide(); |
85 | - }); |
86 | - return button; |
87 | -} |
88 | - |
89 | -/* |
90 | - * Take an error message and display in an overlay (creating it if necessary). |
91 | - * |
92 | - * @method display_error |
93 | - * @param flash_node {Node} The node to red flash. |
94 | - * @param msg {String} The message to display. |
95 | -*/ |
96 | -function display_error(flash_node, msg) { |
97 | - create_error_overlay(); |
98 | - if (flash_node) { |
99 | - var anim = Y.lazr.anim.red_flash({ node: flash_node }); |
100 | - anim.on('end', function(e) { |
101 | - error_overlay.showError(msg); |
102 | - error_overlay.show(); |
103 | - }); |
104 | - anim.run(); |
105 | - } else { |
106 | - error_overlay.showError(msg); |
107 | - error_overlay.show(); |
108 | - } |
109 | -} |
110 | |
111 | /* |
112 | * Traverse the DOM of a given remove icon to find |
113 | @@ -652,6 +592,7 @@ |
114 | return user_uri; |
115 | } |
116 | |
117 | + |
118 | /* |
119 | * Build the HTML for a user link for the subscribers list. |
120 | * |
121 | @@ -1014,7 +955,7 @@ |
122 | }; |
123 | error_handler.showError = function (error_msg) { |
124 | var flash_node = Y.get('.' + person.get('css_name')); |
125 | - display_error(flash_node, error_msg); |
126 | + Y.lp.display_error(flash_node, error_msg); |
127 | |
128 | }; |
129 | |
130 | @@ -1089,7 +1030,7 @@ |
131 | subscription.disable_spinner(); |
132 | }; |
133 | error_handler.showError = function (error_msg) { |
134 | - display_error(subscription_link, error_msg); |
135 | + Y.lp.display_error(subscription_link, error_msg); |
136 | }; |
137 | |
138 | var config = { |
139 | @@ -1145,7 +1086,7 @@ |
140 | subscription.disable_spinner(); |
141 | }; |
142 | error_handler.showError = function (error_msg) { |
143 | - display_error(subscription_link, error_msg); |
144 | + Y.lp.display_error(subscription_link, error_msg); |
145 | }; |
146 | |
147 | var config = { |
148 | @@ -1257,7 +1198,7 @@ |
149 | backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF' |
150 | }); |
151 | status_choice_edit.showError = function(err) { |
152 | - display_error(null, err); |
153 | + Y.lp.display_error(null, err); |
154 | }; |
155 | status_choice_edit.on('save', function(e) { |
156 | var cb = status_choice_edit.get('contentBox'); |
157 | @@ -1289,7 +1230,7 @@ |
158 | backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF' |
159 | }); |
160 | importance_choice_edit.showError = function(err) { |
161 | - display_error(null, err); |
162 | + Y.lp.display_error(null, err); |
163 | }; |
164 | importance_choice_edit.on('save', function(e) { |
165 | var cb = importance_choice_edit.get('contentBox'); |
166 | @@ -1324,7 +1265,7 @@ |
167 | clickable_content: false |
168 | }); |
169 | milestone_choice_edit.showError = function(err) { |
170 | - display_error(null, err); |
171 | + Y.lp.display_error(null, err); |
172 | }; |
173 | milestone_choice_edit.plug({ |
174 | fn: Y.lp.client.plugins.PATCHPlugin, cfg: { |
175 | @@ -1378,7 +1319,7 @@ |
176 | "header": "Change assignee", |
177 | "remove_button_text": "Remove assignee", |
178 | "null_display_value": "Unassigned"}); |
179 | - assignee_picker.render() |
180 | + assignee_picker.render(); |
181 | } |
182 | }; |
183 | |
184 | @@ -1486,7 +1427,7 @@ |
185 | }, |
186 | |
187 | showError: function(err) { |
188 | - display_error(null, err); |
189 | + Y.lp.display_error(null, err); |
190 | }, |
191 | |
192 | syncUI: function() { |
193 | @@ -1549,7 +1490,7 @@ |
194 | function check_can_be_unsubscribed(subscription) { |
195 | var error_handler = new LP.client.ErrorHandler(); |
196 | error_handler.showError = function (error_msg) { |
197 | - display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
198 | + Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
199 | }; |
200 | |
201 | var config = { |
202 | @@ -1618,7 +1559,7 @@ |
203 | |
204 | var error_handler = new LP.client.ErrorHandler(); |
205 | error_handler.showError = function(error_msg) { |
206 | - display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
207 | + Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
208 | }; |
209 | |
210 | if (subscription.is_already_subscribed()) { |
211 | @@ -1631,88 +1572,6 @@ |
212 | } |
213 | |
214 | /* |
215 | - * Set up and handle submitting a comment inline. |
216 | - * |
217 | - * @method setup_inline_commenting |
218 | - */ |
219 | -function setup_inline_commenting() { |
220 | - var submit_button = Y.get('[id="field.actions.save"]'); |
221 | - var progress_message = Y.Node.create( |
222 | - '<span class="update-in-progress-message">Saving...</span>'); |
223 | - var comment_input = Y.get('[id="field.comment"]'); |
224 | - |
225 | - var error_handler = new LP.client.ErrorHandler(); |
226 | - error_handler.clearProgressUI = function () { |
227 | - clearProgressUI(); |
228 | - }; |
229 | - error_handler.showError = function (error_msg) { |
230 | - display_error(submit_button, error_msg); |
231 | - }; |
232 | - |
233 | - function clearProgressUI() { |
234 | - progress_message.get('parentNode').replaceChild( |
235 | - submit_button, progress_message); |
236 | - comment_input.removeAttribute('disabled'); |
237 | - } |
238 | - |
239 | - function hide_or_show_submit_button(e) { |
240 | - if (comment_input.get('value') === '') { |
241 | - submit_button.set('disabled', true); |
242 | - } |
243 | - else { |
244 | - submit_button.set('disabled', false); |
245 | - } |
246 | - } |
247 | - /* Hook up hide_or_show_submit_button on both keyup and mouseup to |
248 | - handle both entering text with the keyboard, and pasting with the |
249 | - mouse. */ |
250 | - comment_input.on('keyup', hide_or_show_submit_button); |
251 | - comment_input.on('mouseup', hide_or_show_submit_button); |
252 | - hide_or_show_submit_button(null); |
253 | - submit_button.addClass('js-action'); |
254 | - submit_button.setStyle('display', 'inline'); |
255 | - submit_button.on('click', function(e) { |
256 | - e.halt(); |
257 | - var comment_text = comment_input.get('value'); |
258 | - /* Don't try to add an empty comment. */ |
259 | - if (trim(comment_text) === '') { |
260 | - return; |
261 | - } |
262 | - var config = { |
263 | - on: { |
264 | - success: function(message_entry) { |
265 | - var config = { |
266 | - on: { |
267 | - success: function(message_html) { |
268 | - var fieldset = Y.get('#add-comment-form'); |
269 | - var legend = Y.get('#add-comment-form legend'); |
270 | - var comment = Y.Node.create(message_html); |
271 | - fieldset.get('parentNode').insertBefore( |
272 | - comment, fieldset); |
273 | - clearProgressUI(); |
274 | - comment_input.set('value', ''); |
275 | - Y.lazr.anim.green_flash({node: comment}).run(); |
276 | - } |
277 | - }, |
278 | - accept: LP.client.XHTML |
279 | - }; |
280 | - lp_client.get(message_entry.get('self_link'), config); |
281 | - }, |
282 | - failure: error_handler.getFailureHandler() |
283 | - }, |
284 | - parameters: { |
285 | - content: comment_input.get('value') |
286 | - } |
287 | - }; |
288 | - comment_input.set('disabled', 'true'); |
289 | - submit_button.get('parentNode').replaceChild( |
290 | - progress_message, submit_button); |
291 | - lp_client.named_post( |
292 | - bug_repr.self_link, 'newMessage', config); |
293 | - }); |
294 | -} |
295 | - |
296 | -/* |
297 | * Click handling to pass comment text to the attachment |
298 | * page if there is a comment. |
299 | * |
300 | @@ -1734,5 +1593,6 @@ |
301 | }, '0.1', {requires: ['base', 'oop', 'node', 'event', 'io-base', 'substitute', |
302 | 'widget-position-ext', 'lazr.formoverlay', 'lazr.anim', |
303 | 'lazr.base', 'lazr.overlay', 'lazr.choiceedit', |
304 | - 'lp.picker', 'lp.client.plugins', 'lp.subscriber']}); |
305 | + 'lp.picker', 'lp.client.plugins', 'lp.subscriber', |
306 | + 'lp.errors']}); |
307 | |
308 | |
309 | === added file 'lib/canonical/launchpad/javascript/lp/comment.js' |
310 | --- lib/canonical/launchpad/javascript/lp/comment.js 1970-01-01 00:00:00 +0000 |
311 | +++ lib/canonical/launchpad/javascript/lp/comment.js 2009-10-14 13:58:19 +0000 |
312 | @@ -0,0 +1,191 @@ |
313 | +YUI.add('lp.comment', function(Y) { |
314 | + |
315 | +Y.lp = Y.namespace('lp'); |
316 | + |
317 | +var Comment = function () { |
318 | + Comment.superclass.constructor.apply(this, arguments); |
319 | +}; |
320 | + |
321 | + |
322 | +Comment.NAME = 'comment'; |
323 | + |
324 | +Comment.ATTRS = { |
325 | +}; |
326 | +Y.extend(Comment, Y.Widget, { |
327 | + |
328 | + /** |
329 | + * Initialize the Comment |
330 | + * |
331 | + * @method initializer |
332 | + */ |
333 | + initializer: function() { |
334 | + this.submit_button = this.get_submit(); |
335 | + this.comment_input = Y.get('[id="field.comment"]'); |
336 | + this.lp_client = new LP.client.Launchpad(); |
337 | + this.error_handler = new LP.client.ErrorHandler(); |
338 | + this.error_handler.clearProgressUI = bind(this.clearProgressUI, this); |
339 | + this.error_handler.showError = bind(function (error_msg) { |
340 | + Y.lp.display_error(this.submit_button, error_msg); |
341 | + }, this); |
342 | + this.progress_message = Y.Node.create( |
343 | + '<span class="update-in-progress-message">Saving...</span>'); |
344 | + }, |
345 | + |
346 | + /** |
347 | + * Return the Submit button. |
348 | + * |
349 | + * This is provided so that it can be overridden in subclasses. |
350 | + * |
351 | + * @method get_submit |
352 | + */ |
353 | + get_submit: function(){ |
354 | + return Y.get('[id="field.actions.save"]'); |
355 | + }, |
356 | + /** |
357 | + * Implementation of Widget.renderUI. |
358 | + * |
359 | + * This redisplays the submit button, in case it has been hidden by |
360 | + * the web page. |
361 | + * |
362 | + * @method renderUI |
363 | + */ |
364 | + renderUI: function() { |
365 | + this.submit_button.addClass('js-action'); |
366 | + this.submit_button.setStyle('display', 'inline'); |
367 | + }, |
368 | + /** |
369 | + * Ensure that the widget's values are suitable for submission. |
370 | + * |
371 | + * The contents of the comment field must contain at least one |
372 | + * non-whitespace character. |
373 | + * |
374 | + * @method validate |
375 | + */ |
376 | + validate: function() { |
377 | + return trim(this.comment_input.get('value')) !== ''; |
378 | + }, |
379 | + /** |
380 | + * Make the widget enabled or disabled. |
381 | + * |
382 | + * @method set_disabled |
383 | + * @param disabled A boolean, true if the widget is disabled. |
384 | + */ |
385 | + set_disabled: function(disabled){ |
386 | + this.comment_input.set('disabled', disabled); |
387 | + }, |
388 | + /** |
389 | + * Add the widget's comment as a new comment, updating the display. |
390 | + * |
391 | + * @method add_comment |
392 | + * @param e An event |
393 | + */ |
394 | + add_comment: function(e){ |
395 | + e.halt(); |
396 | + /* Don't try to add an empty comment. */ |
397 | + if (!this.validate()) { |
398 | + return; |
399 | + } |
400 | + this.set_disabled(true); |
401 | + this.submit_button.get('parentNode').replaceChild( |
402 | + this.progress_message, this.submit_button); |
403 | + this.post_comment(bind(function(message_entry) { |
404 | + this.get_comment_HTML( |
405 | + message_entry, bind(this.insert_comment_HTML, this)); |
406 | + }, this)); |
407 | + }, |
408 | + /** |
409 | + * Post the comment to the Launchpad API |
410 | + * |
411 | + * @method post_comment |
412 | + * @param callback A callable to call if the post is successful. |
413 | + */ |
414 | + post_comment: function(callback) { |
415 | + var config = { |
416 | + on: { |
417 | + success: callback, |
418 | + failure: this.error_handler.getFailureHandler() |
419 | + }, |
420 | + parameters: {content: this.comment_input.get('value')} |
421 | + }; |
422 | + this.lp_client.named_post( |
423 | + LP.client.cache.bug.self_link, 'newMessage', config); |
424 | + }, |
425 | + /** |
426 | + * Retrieve the HTML of the specified message entry. |
427 | + * |
428 | + * @method get_comment_HTML |
429 | + * @param message_entry The comment to get the HTML for. |
430 | + * @param callback On success, call this with the HTML of the comment. |
431 | + */ |
432 | + get_comment_HTML: function(message_entry, callback){ |
433 | + var config = { |
434 | + on: { |
435 | + success: callback |
436 | + }, |
437 | + accept: LP.client.XHTML |
438 | + }; |
439 | + this.lp_client.get(message_entry.get('self_link'), config); |
440 | + }, |
441 | + /** |
442 | + * Insert the specified HTML into the page. |
443 | + * |
444 | + * @method insert_comment_HTML |
445 | + * @param message_html The HTML of the comment to insert. |
446 | + */ |
447 | + insert_comment_HTML: function(message_html) { |
448 | + var fieldset = Y.get('#add-comment-form'); |
449 | + var comment = Y.Node.create(message_html); |
450 | + fieldset.get('parentNode').insertBefore(comment, fieldset); |
451 | + this.reset_contents(); |
452 | + Y.lazr.anim.green_flash({node: comment}).run(); |
453 | + }, |
454 | + /** |
455 | + * Reset the widget to a blank state. |
456 | + * |
457 | + * @method reset_contents |
458 | + */ |
459 | + reset_contents: function() { |
460 | + this.clearProgressUI(); |
461 | + this.comment_input.set('value', ''); |
462 | + this.syncUI(); |
463 | + }, |
464 | + /** |
465 | + * Stop indicating that a submission is in progress. |
466 | + * |
467 | + * @method clearProgressUI |
468 | + */ |
469 | + clearProgressUI: function(){ |
470 | + this.progress_message.get('parentNode').replaceChild( |
471 | + this.submit_button, this.progress_message); |
472 | + this.set_disabled(false); |
473 | + }, |
474 | + /** |
475 | + * Implementation of Widget.bindUI: Bind events to methods. |
476 | + * |
477 | + * Key and mouse presses (e.g. mouse paste) call syncUI, in case the submit |
478 | + * button needs to be updated. Clicking on the submit button invokes |
479 | + * add_comment. |
480 | + * |
481 | + * @method bindUI |
482 | + */ |
483 | + bindUI: function(){ |
484 | + this.comment_input.on('keyup', bind(this.syncUI, this)); |
485 | + this.comment_input.on('mouseup', bind(this.syncUI, this)); |
486 | + this.submit_button.on('click', bind(this.add_comment, this)); |
487 | + }, |
488 | + /** |
489 | + * Implementation of Widget.syncUI: Update appearance according to state. |
490 | + * |
491 | + * This just updates the submit button. |
492 | + * |
493 | + * @method syncUI |
494 | + */ |
495 | + syncUI: function(){ |
496 | + this.submit_button.set('disabled', !this.validate()); |
497 | + } |
498 | +}); |
499 | + |
500 | +Y.lp.Comment = Comment; |
501 | + |
502 | + |
503 | +}, '0.1' ,{requires:['oop', 'widget', 'lp.client.plugins', 'lp.errors']}); |
504 | |
505 | === added file 'lib/canonical/launchpad/javascript/lp/errors.js' |
506 | --- lib/canonical/launchpad/javascript/lp/errors.js 1970-01-01 00:00:00 +0000 |
507 | +++ lib/canonical/launchpad/javascript/lp/errors.js 2009-10-14 13:58:19 +0000 |
508 | @@ -0,0 +1,81 @@ |
509 | +YUI.add('lp.errors', function(Y) { |
510 | + |
511 | +Y.lp = Y.namespace('lp'); |
512 | + |
513 | +/* |
514 | + * Create a form button for canceling an error form |
515 | + * that won't reload the page on submit. |
516 | + * |
517 | + * @method cancel_form_button |
518 | + * @return button {Node} The form's cancel button. |
519 | +*/ |
520 | +var cancel_form_button = function() { |
521 | + var button = Y.Node.create('<button>OK</button>'); |
522 | + button.on('click', function(e) { |
523 | + e.preventDefault(); |
524 | + error_overlay.hide(); |
525 | + }); |
526 | + return button; |
527 | +}; |
528 | + |
529 | + |
530 | +var error_overlay; |
531 | +/* |
532 | + * Create the form overlay to use when encountering errors. |
533 | + * |
534 | + * @method create_error_overlay |
535 | +*/ |
536 | +var create_error_overlay = function() { |
537 | + if (error_overlay === undefined) { |
538 | + error_overlay = new Y.lazr.FormOverlay({ |
539 | + headerContent: '<h2>Error</h2>', |
540 | + form_header: '', |
541 | + form_content: '', |
542 | + form_submit_button: Y.Node.create( |
543 | + '<button style="display:none"></button>'), |
544 | + form_cancel_button: cancel_form_button(), |
545 | + centered: true, |
546 | + visible: false |
547 | + }); |
548 | + error_overlay.render(); |
549 | + } |
550 | +}; |
551 | + |
552 | +/** |
553 | + * Run a callback, optionally flashing a specified node red beforehand. |
554 | + * |
555 | + * If the supplied node evaluates false, the callback is invoked immediately. |
556 | + * |
557 | + * @method maybe_red_flash |
558 | + * @param flash_node The node to flash red, or null for no flash. |
559 | + * @param callback The callback to invoke. |
560 | + */ |
561 | +var maybe_red_flash = function(flash_node, callback) |
562 | +{ |
563 | + if (flash_node) { |
564 | + var anim = Y.lazr.anim.red_flash({ node: flash_node }); |
565 | + anim.on('end', callback); |
566 | + anim.run(); |
567 | + } else { |
568 | + callback(); |
569 | + } |
570 | +}; |
571 | + |
572 | + |
573 | +/* |
574 | + * Take an error message and display in an overlay (creating it if necessary). |
575 | + * |
576 | + * @method display_error |
577 | + * @param flash_node {Node} The node to red flash. |
578 | + * @param msg {String} The message to display. |
579 | +*/ |
580 | +var display_error = function(flash_node, msg) { |
581 | + create_error_overlay(); |
582 | + maybe_red_flash(flash_node, function(){ |
583 | + error_overlay.showError(msg); |
584 | + error_overlay.show(); |
585 | + }); |
586 | +}; |
587 | + |
588 | +Y.lp.display_error = display_error; |
589 | +}, '0.1', {requires:['lazr.formoverlay']}); |
590 | |
591 | === modified file 'lib/lp/bugs/templates/bugtask-index.pt' |
592 | --- lib/lp/bugs/templates/bugtask-index.pt 2009-09-22 18:05:41 +0000 |
593 | +++ lib/lp/bugs/templates/bugtask-index.pt 2009-10-14 13:58:19 +0000 |
594 | @@ -25,6 +25,14 @@ |
595 | tal:define="lp_js string:${icingroot}/build" |
596 | tal:attributes="src string:${lp_js}/bugs/bug_tags_entry.js"> |
597 | </script> |
598 | + <script type="text/javascript" |
599 | + tal:define="lp_js string:${icingroot}/build" |
600 | + tal:attributes="src string:${lp_js}/lp/comment.js"> |
601 | + </script> |
602 | + <script type="text/javascript" |
603 | + tal:define="lp_js string:${icingroot}/build" |
604 | + tal:attributes="src string:${lp_js}/lp/errors.js"> |
605 | + </script> |
606 | </tal:devmode> |
607 | <script type="text/javascript"> |
608 | YUI().use('base', 'node', 'oop', 'event', 'bugs.bugtask_index', function(Y) { |
609 | @@ -282,6 +290,12 @@ |
610 | var button = document.getElementById('field.actions.save'); |
611 | button.style.display = 'none'; |
612 | </script> |
613 | + <script type="text/javascript"> |
614 | + YUI().use('lp.comment', function(Y) { |
615 | + var comment = new Y.lp.Comment(); |
616 | + comment.render(); |
617 | + }) |
618 | + </script> |
619 | </div> |
620 | <tal:attachment-link |
621 | define="add_attachment_link context_menu/addcomment" |
622 | |
623 | === modified file 'lib/lp/code/interfaces/branchmergeproposal.py' |
624 | --- lib/lp/code/interfaces/branchmergeproposal.py 2009-10-01 13:25:12 +0000 |
625 | +++ lib/lp/code/interfaces/branchmergeproposal.py 2009-10-14 13:58:19 +0000 |
626 | @@ -41,9 +41,10 @@ |
627 | from canonical.launchpad.webapp.interfaces import ITableBatchNavigator |
628 | from lazr.restful.fields import CollectionField, Reference |
629 | from lazr.restful.declarations import ( |
630 | - call_with, export_as_webservice_entry, export_read_operation, |
631 | - export_write_operation, exported, operation_parameters, |
632 | - operation_returns_entry, rename_parameters_as, REQUEST_USER) |
633 | + call_with, export_as_webservice_entry, export_factory_operation, |
634 | + export_read_operation, export_write_operation, exported, |
635 | + operation_parameters, operation_returns_entry, rename_parameters_as, |
636 | + REQUEST_USER) |
637 | |
638 | |
639 | class InvalidBranchMergeProposal(Exception): |
640 | @@ -454,7 +455,8 @@ |
641 | vote=Choice(vocabulary=CodeReviewVote), review_type=Text(), |
642 | parent=Reference(schema=Interface)) |
643 | @call_with(owner=REQUEST_USER) |
644 | - @export_write_operation() |
645 | + # ICodeReviewComment supplied as Interface to avoid circular imports. |
646 | + @export_factory_operation(Interface, []) |
647 | def createComment(owner, subject, content=None, vote=None, |
648 | review_type=None, parent=None): |
649 | """Create an ICodeReviewComment associated with this merge proposal. |
650 | |
651 | === modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt' |
652 | --- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2009-10-12 09:40:17 +0000 |
653 | +++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2009-10-14 13:58:19 +0000 |
654 | @@ -146,10 +146,12 @@ |
655 | |
656 | Now the code review should be made. |
657 | |
658 | - >>> comment = reviewer_webservice.named_post( |
659 | + >>> comment_result = reviewer_webservice.named_post( |
660 | ... merge_proposal['self_link'], 'createComment', |
661 | ... subject='Great work', content='This is great work', |
662 | - ... vote=CodeReviewVote.APPROVE.title, review_type='code').jsonBody() |
663 | + ... vote=CodeReviewVote.APPROVE.title, review_type='code') |
664 | + >>> comment_link = comment_result.getHeader('Location') |
665 | + >>> comment = reviewer_webservice.get(comment_link).jsonBody() |
666 | >>> pprint_entry(comment) |
667 | branch_merge_proposal_link: u'http://.../~source/fooix/fix-it/+merge/1' |
668 | id: 3 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
= Summary =
Refactor the inline bug comment code for reuse, specifically by branch
merge proposals.
== Proposed fix ==
Extract the error handling code from bugtask-index.js into errors.js.
Extract the comment handling code into comment.js. Reorganize as a Widget.
== Pre-implementation notes ==
Preimplementation was with thumper.
== Implementation details == posal.createCom ment is factory_ operation. This ensures that its return value
As a drive-by, the export of IBranchMergePro
changed to export_
is usable.
== Tests == BugsWindmillLay er
bin/test -v --layer=
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files: code/interfaces /branchmergepro posal.py /launchpad/ javascript/ bugs/bugtask- index.js /launchpad/ javascript/ lp/errors. js /launchpad/ interfaces/ _schema_ circular_ imports. py bugs/templates/ bugtask- index.pt /launchpad/ javascript/ lp/comment. js
lib/lp/
lib/canonical
lib/canonical
lib/canonical
lib/lp/
lib/canonical
== JSLint notices == abentley/ launchpad/ inline- comment/ lib/canonical/ launchpad/ javascript/ lp/comment. js'.
jslint: No problem found in
'/home/
jslint: No problem found in abentley/ launchpad/ inline- comment/ lib/canonical/ launchpad/ javascript/ lp/errors. js'.
'/home/
jslint: No problem found in abentley/ launchpad/ inline- comment/ lib/canonical/ launchpad/ javascript/ bugs/bugtask- index.js' .
'/home/
jslint: 3 files to lint.
== Pylint notices ==
lib/lp/ code/interfaces /branchmergepro posal.py .event' (No module named fields' (No module named declarations' (No module
27: [F0401] Unable to import 'lazr.lifecycle
lifecycle)
42: [F0401] Unable to import 'lazr.restful.
restful)
43: [F0401] Unable to import 'lazr.restful.
named restful)
lib/canonical/ launchpad/ interfaces/ _schema_ circular_ imports. py declarations' (No module enigmail. mozdev. org
18: [F0401] Unable to import 'lazr.restful.
named restful)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkr UklMACgkQ0F+ nu1YWqI0TIgCfcs mvIb+qgNSDBGRnD yciBJvh FvxrnRxj4Miho4v OzpQF912z
h3gAnin+
=+Dh5
-----END PGP SIGNATURE-----