Launchpad itself

Merge lp:~abentley/launchpad/bmp-inline into lp:launchpad

  1. bmp-inline
  2. Merge into devel
Proposed by Aaron Bentley
Status: Merged
Merged at revision: not available
Proposed branch: lp:~abentley/launchpad/bmp-inline
Merge into: lp:launchpad
Diff against target: 983 lines
12 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 (+349/-0)
lib/canonical/launchpad/javascript/lp/errors.js (+81/-0)
lib/lp/bugs/templates/bugtask-index.pt (+14/-0)
lib/lp/code/browser/configure.zcml (+3/-0)
lib/lp/code/interfaces/branchmergeproposal.py (+6/-4)
lib/lp/code/stories/webservice/xx-branchmergeproposal.txt (+4/-2)
lib/lp/code/templates/branchmergeproposal-index.pt (+23/-2)
lib/lp/code/templates/codereviewcomment-fragment.pt (+2/-0)
lib/lp/code/windmill/testing.py (+16/-0)
lib/lp/code/windmill/tests/test_merge_proposal_commenting.py (+55/-0)
To merge this branch: bzr merge lp:~abentley/launchpad/bmp-inline
Reviewer Review Type Date Requested Status
Paul Hummer (community) ui Approve
Brad Crittenden (community) code Approve
Review via email: mp+13299@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 reviewer rockstar ui
 reviewer launchpad-reviewers code

= Summary =
This branch provides bug-like inline commenting for merge proposals.

== Proposed fix ==
Implement a CodeReviewComment widget that extends Comment, use it in the
page. Only show it if YUI is enabled.

Replies are not yet supported as an inline operation.

== Pre-implementation notes ==
Preimplementation was with thumper. Further discussion was with rockstar

== Implementation details ==
To ensure consistent rendering, a new url, "+fragment" is provided, so
that the HTML fragment for a particular CodeReviewComment can be retrieved.

A new canonical_url function is provided for retrieving test URLs. (The
normal urls require an SSL certificate to be accepted.)

The conversation now has a top-level div with id "conversation" to make
it easier to append to the conversation.

== Tests ==
bin/test -v --layer=CodeWindmillLayer test_merge_proposal_commenting

== Demo and Q/A ==
Create a merge proposal and go to its page. The comment form will
appear at the bottom. Make a comment or vote.

= Launchpad lint =

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

Linting changed files:
  lib/lp/code/browser/configure.zcml
  lib/lp/code/templates/branchmergeproposal-index.pt
  lib/lp/code/windmill/tests/test_merge_proposal_commenting.py
  lib/lp/code/windmill/testing.py
  lib/lp/code/templates/codereviewcomment-fragment.pt
  lib/canonical/launchpad/javascript/lp/comment.js

== JSLint notices ==
jslint: No problem found in
'/home/abentley/launchpad/inline-comment/lib/canonical/launchpad/javascript/lp/comment.js'.

jslint: No problem found in
'/home/abentley/launchpad/inline-comment/lib/canonical/launchpad/javascript/lp/errors.js'.

jslint: No problem found in
'/home/abentley/launchpad/inline-comment/lib/canonical/launchpad/javascript/bugs/bugtask-index.js'.

jslint: 3 files to lint.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrUt+AACgkQ0F+nu1YWqI2gEgCcDExr+O2PM2T4hjymyJAjNlXx
R38AnjWMDltSr/2fFYU5ieWX9tJgqMfT
=HhFi
-----END PGP SIGNATURE-----

Revision history for this message
Aaron Bentley (abentley) wrote :

This branch includes changes from reuse-refactor, which was separately reviewed, so here's a diff with just the changes unique to this branch:

http://pastebin.ubuntu.com/292568/

Revision history for this message
Brad Crittenden (bac) :
review: Approve (code)
Revision history for this message
Paul Hummer (rockstar) wrote :

This looks good to me, but you'll need someone else to look at.

review: Approve (ui)

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:59:13 +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:59:13 +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:59:13 +0000
312@@ -0,0 +1,349 @@
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+var CodeReviewComment = function(){
503+ CodeReviewComment.superclass.constructor.apply(this, arguments);
504+};
505+CodeReviewComment.NAME = 'codereviewcomment';
506+
507+
508+Y.extend(CodeReviewComment, Comment, {
509+ /**
510+ * Initialize the CodeReviewComment
511+ *
512+ * @method initializer
513+ */
514+ initializer: function() {
515+ this.vote_input = Y.get('[id="field.vote"]');
516+ this.review_type = Y.get('[id="field.review_type"]');
517+ },
518+ /**
519+ * Return the Submit button.
520+ *
521+ * @method get_submit
522+ */
523+ get_submit: function(){
524+ return Y.get('[id="field.actions.add"]');
525+ },
526+ /**
527+ * Return the vote value selected, or null if none is selected.
528+ *
529+ * @method get_vote
530+ */
531+ get_vote: function() {
532+ var selected_idx = this.vote_input.get('selectedIndex');
533+ var selected = this.vote_input.get('options').item(selected_idx);
534+ if (selected.get('value') === ''){
535+ return null;
536+ }
537+ return selected.get('innerHTML');
538+ },
539+ /**
540+ * Ensure that the widget's values are suitable for submission.
541+ *
542+ * This allows the vote to be submitted, even when no text is specified
543+ * for the comment.
544+ *
545+ * @method validate
546+ */
547+ validate: function(){
548+ if (this.get_vote() !== null) {
549+ return true;
550+ }
551+ return CodeReviewComment.superclass.validate.apply(this);
552+ },
553+ /**
554+ * Make the widget enabled or disabled.
555+ *
556+ * @method set_disabled
557+ * @param disabled A boolean, true if the widget is disabled.
558+ */
559+ set_disabled: function(disabled){
560+ CodeReviewComment.superclass.set_disabled.call(this, disabled);
561+ this.vote_input.set('disabled', disabled);
562+ this.review_type.set('disabled', disabled);
563+ },
564+ /**
565+ * Post the comment to the Launchpad API
566+ *
567+ * @method post_comment
568+ * @param callback A callable to call if the post is successful.
569+ */
570+ post_comment: function(callback) {
571+ var config = {
572+ on: {
573+ success: callback,
574+ failure: this.error_handler.getFailureHandler()
575+ },
576+ parameters: {
577+ content: this.comment_input.get('value'),
578+ subject: '',
579+ review_type: this.review_type.get('value'),
580+ vote: this.get_vote()
581+ }
582+ };
583+ this.lp_client.named_post(
584+ LP.client.cache.context.self_link, 'createComment', config);
585+ },
586+ /**
587+ * Retrieve the HTML of the specified message entry.
588+ *
589+ * @method get_comment_HTML
590+ * @param message_entry The comment to get the HTML for.
591+ * @param callback On success, call this with the HTML of the comment.
592+ */
593+ get_comment_HTML: function(comment_entry, callback) {
594+ fragment_url = 'comments/' + comment_entry.get('id') + '/+fragment';
595+ Y.io(fragment_url, {
596+ on: {
597+ success: function(id, response){
598+ callback(response.responseText);
599+ },
600+ failure: this.error_handler.getFailureHandler()
601+ }
602+ });
603+ },
604+ /**
605+ * Reset the widget to a blank state.
606+ *
607+ * @method reset_contents
608+ */
609+ reset_contents: function() {
610+ this.review_type.set('value', '');
611+ this.vote_input.set('selectedIndex', 0);
612+ CodeReviewComment.superclass.reset_contents.apply(this);
613+ },
614+ /**
615+ * Insert the specified HTML into the page.
616+ *
617+ * @method insert_comment_HTML
618+ * @param message_html The HTML of the comment to insert.
619+ */
620+ insert_comment_HTML: function(message_html){
621+ var conversation = Y.get('[id=conversation]');
622+ var comment = Y.Node.create(message_html);
623+ conversation.appendChild(comment);
624+ this.reset_contents();
625+ Y.lazr.anim.green_flash({node: comment}).run();
626+ },
627+ renderUI: function() {
628+ CodeReviewComment.superclass.renderUI.apply(this);
629+ Y.get('#inline-add-comment').setStyle('display', 'block');
630+ },
631+ /**
632+ * Implementation of Widget.bindUI: Bind events to methods.
633+ *
634+ * In addition to Comment behaviour, mouseups and keyups on the vote and
635+ * review type cause a sync.
636+ *
637+ * @method bindUI
638+ */
639+ bindUI: function() {
640+ CodeReviewComment.superclass.bindUI.apply(this);
641+ this.vote_input.on('mouseup', bind(this.syncUI, this));
642+ this.review_type.on('keyup', bind(this.syncUI, this));
643+ this.review_type.on('mouseup', bind(this.syncUI, this));
644+ },
645+ /**
646+ * Implementation of Widget.syncUI: Update appearance according to state.
647+ *
648+ * This enables and disables the review type, in addition to Comment
649+ * behaviour.
650+ *
651+ * @method syncUI
652+ */
653+ syncUI: function() {
654+ CodeReviewComment.superclass.syncUI.apply(this);
655+ var review_type_disabled = (this.get_vote() === null);
656+ this.review_type.set('disabled', review_type_disabled);
657+ }
658+});
659+Y.lp.CodeReviewComment = CodeReviewComment;
660+
661+}, '0.1' ,{requires:['oop', 'io', 'widget', 'node', 'lp.client.plugins', 'lp.errors']});
662
663=== added file 'lib/canonical/launchpad/javascript/lp/errors.js'
664--- lib/canonical/launchpad/javascript/lp/errors.js 1970-01-01 00:00:00 +0000
665+++ lib/canonical/launchpad/javascript/lp/errors.js 2009-10-14 13:59:13 +0000
666@@ -0,0 +1,81 @@
667+YUI.add('lp.errors', function(Y) {
668+
669+Y.lp = Y.namespace('lp');
670+
671+/*
672+ * Create a form button for canceling an error form
673+ * that won't reload the page on submit.
674+ *
675+ * @method cancel_form_button
676+ * @return button {Node} The form's cancel button.
677+*/
678+var cancel_form_button = function() {
679+ var button = Y.Node.create('<button>OK</button>');
680+ button.on('click', function(e) {
681+ e.preventDefault();
682+ error_overlay.hide();
683+ });
684+ return button;
685+};
686+
687+
688+var error_overlay;
689+/*
690+ * Create the form overlay to use when encountering errors.
691+ *
692+ * @method create_error_overlay
693+*/
694+var create_error_overlay = function() {
695+ if (error_overlay === undefined) {
696+ error_overlay = new Y.lazr.FormOverlay({
697+ headerContent: '<h2>Error</h2>',
698+ form_header: '',
699+ form_content: '',
700+ form_submit_button: Y.Node.create(
701+ '<button style="display:none"></button>'),
702+ form_cancel_button: cancel_form_button(),
703+ centered: true,
704+ visible: false
705+ });
706+ error_overlay.render();
707+ }
708+};
709+
710+/**
711+ * Run a callback, optionally flashing a specified node red beforehand.
712+ *
713+ * If the supplied node evaluates false, the callback is invoked immediately.
714+ *
715+ * @method maybe_red_flash
716+ * @param flash_node The node to flash red, or null for no flash.
717+ * @param callback The callback to invoke.
718+ */
719+var maybe_red_flash = function(flash_node, callback)
720+{
721+ if (flash_node) {
722+ var anim = Y.lazr.anim.red_flash({ node: flash_node });
723+ anim.on('end', callback);
724+ anim.run();
725+ } else {
726+ callback();
727+ }
728+};
729+
730+
731+/*
732+ * Take an error message and display in an overlay (creating it if necessary).
733+ *
734+ * @method display_error
735+ * @param flash_node {Node} The node to red flash.
736+ * @param msg {String} The message to display.
737+*/
738+var display_error = function(flash_node, msg) {
739+ create_error_overlay();
740+ maybe_red_flash(flash_node, function(){
741+ error_overlay.showError(msg);
742+ error_overlay.show();
743+ });
744+};
745+
746+Y.lp.display_error = display_error;
747+}, '0.1', {requires:['lazr.formoverlay']});
748
749=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
750--- lib/lp/bugs/templates/bugtask-index.pt 2009-09-22 18:05:41 +0000
751+++ lib/lp/bugs/templates/bugtask-index.pt 2009-10-14 13:59:13 +0000
752@@ -25,6 +25,14 @@
753 tal:define="lp_js string:${icingroot}/build"
754 tal:attributes="src string:${lp_js}/bugs/bug_tags_entry.js">
755 </script>
756+ <script type="text/javascript"
757+ tal:define="lp_js string:${icingroot}/build"
758+ tal:attributes="src string:${lp_js}/lp/comment.js">
759+ </script>
760+ <script type="text/javascript"
761+ tal:define="lp_js string:${icingroot}/build"
762+ tal:attributes="src string:${lp_js}/lp/errors.js">
763+ </script>
764 </tal:devmode>
765 <script type="text/javascript">
766 YUI().use('base', 'node', 'oop', 'event', 'bugs.bugtask_index', function(Y) {
767@@ -282,6 +290,12 @@
768 var button = document.getElementById('field.actions.save');
769 button.style.display = 'none';
770 </script>
771+ <script type="text/javascript">
772+ YUI().use('lp.comment', function(Y) {
773+ var comment = new Y.lp.Comment();
774+ comment.render();
775+ })
776+ </script>
777 </div>
778 <tal:attachment-link
779 define="add_attachment_link context_menu/addcomment"
780
781=== modified file 'lib/lp/code/browser/configure.zcml'
782--- lib/lp/code/browser/configure.zcml 2009-09-28 21:06:28 +0000
783+++ lib/lp/code/browser/configure.zcml 2009-10-14 13:59:13 +0000
784@@ -612,6 +612,9 @@
785 <browser:page
786 name="+comment-footer"
787 template="../templates/codereviewcomment-footer.pt"/>
788+ <browser:page
789+ name="+fragment"
790+ template="../templates/codereviewcomment-fragment.pt"/>
791 </browser:pages>
792 <browser:page
793 name="+reply"
794
795=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
796--- lib/lp/code/interfaces/branchmergeproposal.py 2009-10-01 13:25:12 +0000
797+++ lib/lp/code/interfaces/branchmergeproposal.py 2009-10-14 13:59:13 +0000
798@@ -41,9 +41,10 @@
799 from canonical.launchpad.webapp.interfaces import ITableBatchNavigator
800 from lazr.restful.fields import CollectionField, Reference
801 from lazr.restful.declarations import (
802- call_with, export_as_webservice_entry, export_read_operation,
803- export_write_operation, exported, operation_parameters,
804- operation_returns_entry, rename_parameters_as, REQUEST_USER)
805+ call_with, export_as_webservice_entry, export_factory_operation,
806+ export_read_operation, export_write_operation, exported,
807+ operation_parameters, operation_returns_entry, rename_parameters_as,
808+ REQUEST_USER)
809
810
811 class InvalidBranchMergeProposal(Exception):
812@@ -454,7 +455,8 @@
813 vote=Choice(vocabulary=CodeReviewVote), review_type=Text(),
814 parent=Reference(schema=Interface))
815 @call_with(owner=REQUEST_USER)
816- @export_write_operation()
817+ # ICodeReviewComment supplied as Interface to avoid circular imports.
818+ @export_factory_operation(Interface, [])
819 def createComment(owner, subject, content=None, vote=None,
820 review_type=None, parent=None):
821 """Create an ICodeReviewComment associated with this merge proposal.
822
823=== modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt'
824--- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2009-10-12 09:40:17 +0000
825+++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2009-10-14 13:59:13 +0000
826@@ -146,10 +146,12 @@
827
828 Now the code review should be made.
829
830- >>> comment = reviewer_webservice.named_post(
831+ >>> comment_result = reviewer_webservice.named_post(
832 ... merge_proposal['self_link'], 'createComment',
833 ... subject='Great work', content='This is great work',
834- ... vote=CodeReviewVote.APPROVE.title, review_type='code').jsonBody()
835+ ... vote=CodeReviewVote.APPROVE.title, review_type='code')
836+ >>> comment_link = comment_result.getHeader('Location')
837+ >>> comment = reviewer_webservice.get(comment_link).jsonBody()
838 >>> pprint_entry(comment)
839 branch_merge_proposal_link: u'http://.../~source/fooix/fix-it/+merge/1'
840 id: 3
841
842=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
843--- lib/lp/code/templates/branchmergeproposal-index.pt 2009-09-19 05:01:40 +0000
844+++ lib/lp/code/templates/branchmergeproposal-index.pt 2009-10-14 13:59:13 +0000
845@@ -20,6 +20,14 @@
846 tal:define="lp_js string:${icingroot}/build"
847 tal:attributes="src string:${lp_js}/code/codereview.js">
848 </script>
849+ <script type="text/javascript"
850+ tal:define="lp_js string:${icingroot}/build"
851+ tal:attributes="src string:${lp_js}/lp/comment.js">
852+ </script>
853+ <script type="text/javascript"
854+ tal:define="lp_js string:${icingroot}/build"
855+ tal:attributes="src string:${lp_js}/lp/errors.js">
856+ </script>
857 </metal:block>
858
859
860@@ -65,10 +73,23 @@
861 tal:content="structure link/render">
862 Add comment
863 </div>
864- <tal:comments condition="view/conversation"
865- replace="structure view/conversation/@@+render"/>
866+ <div id="conversation"
867+ tal:content="structure view/conversation/@@+render"/>
868+ </div>
869+ <!-- Hide inline commenting if YUI isn't used. -->
870+ <div id="inline-add-comment" style="display: none">
871+ <tal:comment replace="structure context/@@+comment/++form++" />
872+ <div class="actions" id="launchpad-form-actions">
873+ <input type="submit" id="field.actions.add" name="field.actions.add" value="Save Comment" class="button" />
874+ </div>
875 </div>
876
877+ <script type="text/javascript">
878+ YUI().use('lp.comment', function(Y) {
879+ var comment = new Y.lp.CodeReviewComment();
880+ comment.render();
881+ })
882+ </script>
883 <div class="yui-g">
884 <div class="yui-u first">
885 <div id="source-revisions"
886
887=== added file 'lib/lp/code/templates/codereviewcomment-fragment.pt'
888--- lib/lp/code/templates/codereviewcomment-fragment.pt 1970-01-01 00:00:00 +0000
889+++ lib/lp/code/templates/codereviewcomment-fragment.pt 2009-10-14 13:59:13 +0000
890@@ -0,0 +1,2 @@
891+<tal:fragment replace="structure view/comment/@@+render"
892+ xmlns:tal="http://xml.zope.org/namespaces/tal" />
893
894=== modified file 'lib/lp/code/windmill/testing.py'
895--- lib/lp/code/windmill/testing.py 2009-09-15 08:58:30 +0000
896+++ lib/lp/code/windmill/testing.py 2009-10-14 13:59:13 +0000
897@@ -9,6 +9,9 @@
898 ]
899
900
901+import windmill
902+
903+from canonical.launchpad.webapp import canonical_url as real_canonical_url
904 from canonical.testing.layers import BaseWindmillLayer
905
906
907@@ -16,3 +19,16 @@
908 """Layer for Code Windmill tests."""
909
910 base_url = 'http://code.launchpad.dev:8085/'
911+
912+
913+def canonical_url(*args, **kwargs):
914+ """Wrapper for canonical_url that ensures test URLs are returned.
915+
916+ Real canonical URLs require SSL support, but our test clients don't
917+ trust the certificate.
918+ """
919+ url = real_canonical_url(*args, **kwargs)
920+ url = url.replace(
921+ 'http://code.launchpad.dev', windmill.settings['TEST_URL'])
922+ assert url.startswith(windmill.settings['TEST_URL'])
923+ return url
924
925=== added file 'lib/lp/code/windmill/tests/test_merge_proposal_commenting.py'
926--- lib/lp/code/windmill/tests/test_merge_proposal_commenting.py 1970-01-01 00:00:00 +0000
927+++ lib/lp/code/windmill/tests/test_merge_proposal_commenting.py 2009-10-14 13:59:13 +0000
928@@ -0,0 +1,55 @@
929+# Copyright 2009 Canonical Ltd. This software is licensed under the
930+# GNU Affero General Public License version 3 (see the file LICENSE).
931+
932+"""Test for the inline commenting UI."""
933+
934+__metaclass__ = type
935+__all__ = []
936+
937+import unittest
938+
939+import transaction
940+from windmill.authoring import WindmillTestClient
941+
942+from canonical.launchpad.windmill.testing import lpuser
943+from canonical.uuid import generate_uuid
944+from lp.code.windmill.testing import canonical_url, CodeWindmillLayer
945+from lp.testing import TestCaseWithFactory
946+
947+WAIT_PAGELOAD = u'30000'
948+WAIT_ELEMENT_COMPLETE = u'30000'
949+WAIT_CHECK_CHANGE = u'1000'
950+ADD_COMMENT_BUTTON = (
951+ u'//input[@id="field.actions.add" and @class="button js-action"]')
952+
953+
954+class TestMergeProposalCommenting(TestCaseWithFactory):
955+
956+ layer = CodeWindmillLayer
957+
958+
959+ def test_merge_proposal_commenting(self):
960+ """Test commenting on bugs."""
961+ client = WindmillTestClient('Bug commenting')
962+ lpuser.NO_PRIV.ensure_login(client)
963+
964+ proposal = self.factory.makeBranchMergeProposal()
965+ transaction.commit()
966+ client.open(url=canonical_url(proposal))
967+ client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
968+ client.waits.forElement(xpath=ADD_COMMENT_BUTTON)
969+
970+ # Generate a unique piece of text, so we can run the test multiple
971+ # times, without resetting the db.
972+ new_comment_text = generate_uuid()
973+ client.type(text=new_comment_text, id="field.comment")
974+ client.click(xpath=ADD_COMMENT_BUTTON)
975+ # A PRE inside a boardCommentBody, itself somewhere in the
976+ # #conversation
977+ client.waits.forElement(
978+ xpath='//div[@id="conversation"]//div[@class="boardCommentBody"]'
979+ '/pre[contains(., "%s")]' % new_comment_text)
980+
981+
982+def test_suite():
983+ return unittest.TestLoader().loadTestsFromName(__name__)