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
=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-09-09 11:28:03 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-10-14 13:59:13 +0000
@@ -100,6 +100,8 @@
100IBranchMergeProposal['createComment'].queryTaggedValue(100IBranchMergeProposal['createComment'].queryTaggedValue(
101 LAZR_WEBSERVICE_EXPORTED)['params']['parent'].schema = \101 LAZR_WEBSERVICE_EXPORTED)['params']['parent'].schema = \
102 ICodeReviewComment102 ICodeReviewComment
103patch_entry_return_type(
104 IBranchMergeProposal, 'createComment', ICodeReviewComment)
103IBranchMergeProposal['all_comments'].value_type.schema = ICodeReviewComment105IBranchMergeProposal['all_comments'].value_type.schema = ICodeReviewComment
104IBranchMergeProposal['nominateReviewer'].queryTaggedValue(106IBranchMergeProposal['nominateReviewer'].queryTaggedValue(
105 LAZR_WEBSERVICE_EXPORTED)['return_type'].schema = ICodeReviewVoteReference107 LAZR_WEBSERVICE_EXPORTED)['return_type'].schema = ICodeReviewVoteReference
106108
=== modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-10-06 21:58:10 +0000
+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-10-14 13:59:13 +0000
@@ -83,7 +83,7 @@
83 }83 }
84 };84 };
85 error_handler.showError = function(error_msg) {85 error_handler.showError = function(error_msg) {
86 display_error(Y.get('.menu-link-addsubscriber'), error_msg);86 Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg);
87 };87 };
8888
89 var config = {89 var config = {
@@ -210,12 +210,11 @@
210 });210 });
211 privacy_link.addClass('js-action');211 privacy_link.addClass('js-action');
212 }212 }
213 create_error_overlay();
214 setup_inline_commenting();
215 setup_add_attachment();213 setup_add_attachment();
216 }, window);214 }, window);
217};215};
218216
217
219/*218/*
220 * Clear the subscribe someone else picker.219 * Clear the subscribe someone else picker.
221 *220 *
@@ -313,7 +312,6 @@
313312
314 setup_unsubscribe_icon_handlers(subscription);313 setup_unsubscribe_icon_handlers(subscription);
315 setup_subscribe_someone_else_handler(subscription);314 setup_subscribe_someone_else_handler(subscription);
316 create_error_overlay();
317}315}
318316
319/*317/*
@@ -568,64 +566,6 @@
568 lp_bug_entry.lp_save(config);566 lp_bug_entry.lp_save(config);
569};567};
570568
571/*
572 * Create the form overlay to use when encountering errors.
573 *
574 * @method create_error_overlay
575*/
576function create_error_overlay() {
577 if (error_overlay === undefined) {
578 error_overlay = new Y.lazr.FormOverlay({
579 headerContent: '<h2>Error</h2>',
580 form_header: '',
581 form_content: '',
582 form_submit_button: Y.Node.create(
583 '<button style="display:none"></button>'),
584 form_cancel_button: cancel_form_button(),
585 centered: true,
586 visible: false
587 });
588 error_overlay.render();
589 }
590}
591
592/*
593 * Create a form button for canceling an error form
594 * that won't reload the page on submit.
595 *
596 * @method cancel_form_button
597 * @return button {Node} The form's cancel button.
598*/
599function cancel_form_button() {
600 var button = Y.Node.create('<button>OK</button>');
601 button.on('click', function(e) {
602 e.preventDefault();
603 error_overlay.hide();
604 });
605 return button;
606}
607
608/*
609 * Take an error message and display in an overlay (creating it if necessary).
610 *
611 * @method display_error
612 * @param flash_node {Node} The node to red flash.
613 * @param msg {String} The message to display.
614*/
615function display_error(flash_node, msg) {
616 create_error_overlay();
617 if (flash_node) {
618 var anim = Y.lazr.anim.red_flash({ node: flash_node });
619 anim.on('end', function(e) {
620 error_overlay.showError(msg);
621 error_overlay.show();
622 });
623 anim.run();
624 } else {
625 error_overlay.showError(msg);
626 error_overlay.show();
627 }
628}
629569
630/*570/*
631 * Traverse the DOM of a given remove icon to find571 * Traverse the DOM of a given remove icon to find
@@ -652,6 +592,7 @@
652 return user_uri;592 return user_uri;
653}593}
654594
595
655/*596/*
656 * Build the HTML for a user link for the subscribers list.597 * Build the HTML for a user link for the subscribers list.
657 *598 *
@@ -1014,7 +955,7 @@
1014 };955 };
1015 error_handler.showError = function (error_msg) {956 error_handler.showError = function (error_msg) {
1016 var flash_node = Y.get('.' + person.get('css_name'));957 var flash_node = Y.get('.' + person.get('css_name'));
1017 display_error(flash_node, error_msg);958 Y.lp.display_error(flash_node, error_msg);
1018959
1019 };960 };
1020961
@@ -1089,7 +1030,7 @@
1089 subscription.disable_spinner();1030 subscription.disable_spinner();
1090 };1031 };
1091 error_handler.showError = function (error_msg) {1032 error_handler.showError = function (error_msg) {
1092 display_error(subscription_link, error_msg);1033 Y.lp.display_error(subscription_link, error_msg);
1093 };1034 };
10941035
1095 var config = {1036 var config = {
@@ -1145,7 +1086,7 @@
1145 subscription.disable_spinner();1086 subscription.disable_spinner();
1146 };1087 };
1147 error_handler.showError = function (error_msg) {1088 error_handler.showError = function (error_msg) {
1148 display_error(subscription_link, error_msg);1089 Y.lp.display_error(subscription_link, error_msg);
1149 };1090 };
11501091
1151 var config = {1092 var config = {
@@ -1257,7 +1198,7 @@
1257 backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF'1198 backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF'
1258 });1199 });
1259 status_choice_edit.showError = function(err) {1200 status_choice_edit.showError = function(err) {
1260 display_error(null, err);1201 Y.lp.display_error(null, err);
1261 };1202 };
1262 status_choice_edit.on('save', function(e) {1203 status_choice_edit.on('save', function(e) {
1263 var cb = status_choice_edit.get('contentBox');1204 var cb = status_choice_edit.get('contentBox');
@@ -1289,7 +1230,7 @@
1289 backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF'1230 backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF'
1290 });1231 });
1291 importance_choice_edit.showError = function(err) {1232 importance_choice_edit.showError = function(err) {
1292 display_error(null, err);1233 Y.lp.display_error(null, err);
1293 };1234 };
1294 importance_choice_edit.on('save', function(e) {1235 importance_choice_edit.on('save', function(e) {
1295 var cb = importance_choice_edit.get('contentBox');1236 var cb = importance_choice_edit.get('contentBox');
@@ -1324,7 +1265,7 @@
1324 clickable_content: false1265 clickable_content: false
1325 });1266 });
1326 milestone_choice_edit.showError = function(err) {1267 milestone_choice_edit.showError = function(err) {
1327 display_error(null, err);1268 Y.lp.display_error(null, err);
1328 };1269 };
1329 milestone_choice_edit.plug({1270 milestone_choice_edit.plug({
1330 fn: Y.lp.client.plugins.PATCHPlugin, cfg: {1271 fn: Y.lp.client.plugins.PATCHPlugin, cfg: {
@@ -1378,7 +1319,7 @@
1378 "header": "Change assignee",1319 "header": "Change assignee",
1379 "remove_button_text": "Remove assignee",1320 "remove_button_text": "Remove assignee",
1380 "null_display_value": "Unassigned"});1321 "null_display_value": "Unassigned"});
1381 assignee_picker.render()1322 assignee_picker.render();
1382 }1323 }
1383};1324};
13841325
@@ -1486,7 +1427,7 @@
1486 },1427 },
14871428
1488 showError: function(err) {1429 showError: function(err) {
1489 display_error(null, err);1430 Y.lp.display_error(null, err);
1490 },1431 },
14911432
1492 syncUI: function() {1433 syncUI: function() {
@@ -1549,7 +1490,7 @@
1549function check_can_be_unsubscribed(subscription) {1490function check_can_be_unsubscribed(subscription) {
1550 var error_handler = new LP.client.ErrorHandler();1491 var error_handler = new LP.client.ErrorHandler();
1551 error_handler.showError = function (error_msg) {1492 error_handler.showError = function (error_msg) {
1552 display_error(Y.get('.menu-link-addsubscriber'), error_msg);1493 Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg);
1553 };1494 };
15541495
1555 var config = {1496 var config = {
@@ -1618,7 +1559,7 @@
16181559
1619 var error_handler = new LP.client.ErrorHandler();1560 var error_handler = new LP.client.ErrorHandler();
1620 error_handler.showError = function(error_msg) {1561 error_handler.showError = function(error_msg) {
1621 display_error(Y.get('.menu-link-addsubscriber'), error_msg);1562 Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg);
1622 };1563 };
16231564
1624 if (subscription.is_already_subscribed()) {1565 if (subscription.is_already_subscribed()) {
@@ -1631,88 +1572,6 @@
1631}1572}
16321573
1633/*1574/*
1634 * Set up and handle submitting a comment inline.
1635 *
1636 * @method setup_inline_commenting
1637 */
1638function setup_inline_commenting() {
1639 var submit_button = Y.get('[id="field.actions.save"]');
1640 var progress_message = Y.Node.create(
1641 '<span class="update-in-progress-message">Saving...</span>');
1642 var comment_input = Y.get('[id="field.comment"]');
1643
1644 var error_handler = new LP.client.ErrorHandler();
1645 error_handler.clearProgressUI = function () {
1646 clearProgressUI();
1647 };
1648 error_handler.showError = function (error_msg) {
1649 display_error(submit_button, error_msg);
1650 };
1651
1652 function clearProgressUI() {
1653 progress_message.get('parentNode').replaceChild(
1654 submit_button, progress_message);
1655 comment_input.removeAttribute('disabled');
1656 }
1657
1658 function hide_or_show_submit_button(e) {
1659 if (comment_input.get('value') === '') {
1660 submit_button.set('disabled', true);
1661 }
1662 else {
1663 submit_button.set('disabled', false);
1664 }
1665 }
1666 /* Hook up hide_or_show_submit_button on both keyup and mouseup to
1667 handle both entering text with the keyboard, and pasting with the
1668 mouse. */
1669 comment_input.on('keyup', hide_or_show_submit_button);
1670 comment_input.on('mouseup', hide_or_show_submit_button);
1671 hide_or_show_submit_button(null);
1672 submit_button.addClass('js-action');
1673 submit_button.setStyle('display', 'inline');
1674 submit_button.on('click', function(e) {
1675 e.halt();
1676 var comment_text = comment_input.get('value');
1677 /* Don't try to add an empty comment. */
1678 if (trim(comment_text) === '') {
1679 return;
1680 }
1681 var config = {
1682 on: {
1683 success: function(message_entry) {
1684 var config = {
1685 on: {
1686 success: function(message_html) {
1687 var fieldset = Y.get('#add-comment-form');
1688 var legend = Y.get('#add-comment-form legend');
1689 var comment = Y.Node.create(message_html);
1690 fieldset.get('parentNode').insertBefore(
1691 comment, fieldset);
1692 clearProgressUI();
1693 comment_input.set('value', '');
1694 Y.lazr.anim.green_flash({node: comment}).run();
1695 }
1696 },
1697 accept: LP.client.XHTML
1698 };
1699 lp_client.get(message_entry.get('self_link'), config);
1700 },
1701 failure: error_handler.getFailureHandler()
1702 },
1703 parameters: {
1704 content: comment_input.get('value')
1705 }
1706 };
1707 comment_input.set('disabled', 'true');
1708 submit_button.get('parentNode').replaceChild(
1709 progress_message, submit_button);
1710 lp_client.named_post(
1711 bug_repr.self_link, 'newMessage', config);
1712 });
1713}
1714
1715/*
1716 * Click handling to pass comment text to the attachment1575 * Click handling to pass comment text to the attachment
1717 * page if there is a comment.1576 * page if there is a comment.
1718 *1577 *
@@ -1734,5 +1593,6 @@
1734}, '0.1', {requires: ['base', 'oop', 'node', 'event', 'io-base', 'substitute',1593}, '0.1', {requires: ['base', 'oop', 'node', 'event', 'io-base', 'substitute',
1735 'widget-position-ext', 'lazr.formoverlay', 'lazr.anim',1594 'widget-position-ext', 'lazr.formoverlay', 'lazr.anim',
1736 'lazr.base', 'lazr.overlay', 'lazr.choiceedit',1595 'lazr.base', 'lazr.overlay', 'lazr.choiceedit',
1737 'lp.picker', 'lp.client.plugins', 'lp.subscriber']});1596 'lp.picker', 'lp.client.plugins', 'lp.subscriber',
1597 'lp.errors']});
17381598
17391599
=== added file 'lib/canonical/launchpad/javascript/lp/comment.js'
--- lib/canonical/launchpad/javascript/lp/comment.js 1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/javascript/lp/comment.js 2009-10-14 13:59:13 +0000
@@ -0,0 +1,349 @@
1YUI.add('lp.comment', function(Y) {
2
3Y.lp = Y.namespace('lp');
4
5var Comment = function () {
6 Comment.superclass.constructor.apply(this, arguments);
7};
8
9
10Comment.NAME = 'comment';
11
12Comment.ATTRS = {
13};
14Y.extend(Comment, Y.Widget, {
15
16 /**
17 * Initialize the Comment
18 *
19 * @method initializer
20 */
21 initializer: function() {
22 this.submit_button = this.get_submit();
23 this.comment_input = Y.get('[id="field.comment"]');
24 this.lp_client = new LP.client.Launchpad();
25 this.error_handler = new LP.client.ErrorHandler();
26 this.error_handler.clearProgressUI = bind(this.clearProgressUI, this);
27 this.error_handler.showError = bind(function (error_msg) {
28 Y.lp.display_error(this.submit_button, error_msg);
29 }, this);
30 this.progress_message = Y.Node.create(
31 '<span class="update-in-progress-message">Saving...</span>');
32 },
33
34 /**
35 * Return the Submit button.
36 *
37 * This is provided so that it can be overridden in subclasses.
38 *
39 * @method get_submit
40 */
41 get_submit: function(){
42 return Y.get('[id="field.actions.save"]');
43 },
44 /**
45 * Implementation of Widget.renderUI.
46 *
47 * This redisplays the submit button, in case it has been hidden by
48 * the web page.
49 *
50 * @method renderUI
51 */
52 renderUI: function() {
53 this.submit_button.addClass('js-action');
54 this.submit_button.setStyle('display', 'inline');
55 },
56 /**
57 * Ensure that the widget's values are suitable for submission.
58 *
59 * The contents of the comment field must contain at least one
60 * non-whitespace character.
61 *
62 * @method validate
63 */
64 validate: function() {
65 return trim(this.comment_input.get('value')) !== '';
66 },
67 /**
68 * Make the widget enabled or disabled.
69 *
70 * @method set_disabled
71 * @param disabled A boolean, true if the widget is disabled.
72 */
73 set_disabled: function(disabled){
74 this.comment_input.set('disabled', disabled);
75 },
76 /**
77 * Add the widget's comment as a new comment, updating the display.
78 *
79 * @method add_comment
80 * @param e An event
81 */
82 add_comment: function(e){
83 e.halt();
84 /* Don't try to add an empty comment. */
85 if (!this.validate()) {
86 return;
87 }
88 this.set_disabled(true);
89 this.submit_button.get('parentNode').replaceChild(
90 this.progress_message, this.submit_button);
91 this.post_comment(bind(function(message_entry) {
92 this.get_comment_HTML(
93 message_entry, bind(this.insert_comment_HTML, this));
94 }, this));
95 },
96 /**
97 * Post the comment to the Launchpad API
98 *
99 * @method post_comment
100 * @param callback A callable to call if the post is successful.
101 */
102 post_comment: function(callback) {
103 var config = {
104 on: {
105 success: callback,
106 failure: this.error_handler.getFailureHandler()
107 },
108 parameters: {content: this.comment_input.get('value')}
109 };
110 this.lp_client.named_post(
111 LP.client.cache.bug.self_link, 'newMessage', config);
112 },
113 /**
114 * Retrieve the HTML of the specified message entry.
115 *
116 * @method get_comment_HTML
117 * @param message_entry The comment to get the HTML for.
118 * @param callback On success, call this with the HTML of the comment.
119 */
120 get_comment_HTML: function(message_entry, callback){
121 var config = {
122 on: {
123 success: callback
124 },
125 accept: LP.client.XHTML
126 };
127 this.lp_client.get(message_entry.get('self_link'), config);
128 },
129 /**
130 * Insert the specified HTML into the page.
131 *
132 * @method insert_comment_HTML
133 * @param message_html The HTML of the comment to insert.
134 */
135 insert_comment_HTML: function(message_html) {
136 var fieldset = Y.get('#add-comment-form');
137 var comment = Y.Node.create(message_html);
138 fieldset.get('parentNode').insertBefore(comment, fieldset);
139 this.reset_contents();
140 Y.lazr.anim.green_flash({node: comment}).run();
141 },
142 /**
143 * Reset the widget to a blank state.
144 *
145 * @method reset_contents
146 */
147 reset_contents: function() {
148 this.clearProgressUI();
149 this.comment_input.set('value', '');
150 this.syncUI();
151 },
152 /**
153 * Stop indicating that a submission is in progress.
154 *
155 * @method clearProgressUI
156 */
157 clearProgressUI: function(){
158 this.progress_message.get('parentNode').replaceChild(
159 this.submit_button, this.progress_message);
160 this.set_disabled(false);
161 },
162 /**
163 * Implementation of Widget.bindUI: Bind events to methods.
164 *
165 * Key and mouse presses (e.g. mouse paste) call syncUI, in case the submit
166 * button needs to be updated. Clicking on the submit button invokes
167 * add_comment.
168 *
169 * @method bindUI
170 */
171 bindUI: function(){
172 this.comment_input.on('keyup', bind(this.syncUI, this));
173 this.comment_input.on('mouseup', bind(this.syncUI, this));
174 this.submit_button.on('click', bind(this.add_comment, this));
175 },
176 /**
177 * Implementation of Widget.syncUI: Update appearance according to state.
178 *
179 * This just updates the submit button.
180 *
181 * @method syncUI
182 */
183 syncUI: function(){
184 this.submit_button.set('disabled', !this.validate());
185 }
186});
187
188Y.lp.Comment = Comment;
189
190var CodeReviewComment = function(){
191 CodeReviewComment.superclass.constructor.apply(this, arguments);
192};
193CodeReviewComment.NAME = 'codereviewcomment';
194
195
196Y.extend(CodeReviewComment, Comment, {
197 /**
198 * Initialize the CodeReviewComment
199 *
200 * @method initializer
201 */
202 initializer: function() {
203 this.vote_input = Y.get('[id="field.vote"]');
204 this.review_type = Y.get('[id="field.review_type"]');
205 },
206 /**
207 * Return the Submit button.
208 *
209 * @method get_submit
210 */
211 get_submit: function(){
212 return Y.get('[id="field.actions.add"]');
213 },
214 /**
215 * Return the vote value selected, or null if none is selected.
216 *
217 * @method get_vote
218 */
219 get_vote: function() {
220 var selected_idx = this.vote_input.get('selectedIndex');
221 var selected = this.vote_input.get('options').item(selected_idx);
222 if (selected.get('value') === ''){
223 return null;
224 }
225 return selected.get('innerHTML');
226 },
227 /**
228 * Ensure that the widget's values are suitable for submission.
229 *
230 * This allows the vote to be submitted, even when no text is specified
231 * for the comment.
232 *
233 * @method validate
234 */
235 validate: function(){
236 if (this.get_vote() !== null) {
237 return true;
238 }
239 return CodeReviewComment.superclass.validate.apply(this);
240 },
241 /**
242 * Make the widget enabled or disabled.
243 *
244 * @method set_disabled
245 * @param disabled A boolean, true if the widget is disabled.
246 */
247 set_disabled: function(disabled){
248 CodeReviewComment.superclass.set_disabled.call(this, disabled);
249 this.vote_input.set('disabled', disabled);
250 this.review_type.set('disabled', disabled);
251 },
252 /**
253 * Post the comment to the Launchpad API
254 *
255 * @method post_comment
256 * @param callback A callable to call if the post is successful.
257 */
258 post_comment: function(callback) {
259 var config = {
260 on: {
261 success: callback,
262 failure: this.error_handler.getFailureHandler()
263 },
264 parameters: {
265 content: this.comment_input.get('value'),
266 subject: '',
267 review_type: this.review_type.get('value'),
268 vote: this.get_vote()
269 }
270 };
271 this.lp_client.named_post(
272 LP.client.cache.context.self_link, 'createComment', config);
273 },
274 /**
275 * Retrieve the HTML of the specified message entry.
276 *
277 * @method get_comment_HTML
278 * @param message_entry The comment to get the HTML for.
279 * @param callback On success, call this with the HTML of the comment.
280 */
281 get_comment_HTML: function(comment_entry, callback) {
282 fragment_url = 'comments/' + comment_entry.get('id') + '/+fragment';
283 Y.io(fragment_url, {
284 on: {
285 success: function(id, response){
286 callback(response.responseText);
287 },
288 failure: this.error_handler.getFailureHandler()
289 }
290 });
291 },
292 /**
293 * Reset the widget to a blank state.
294 *
295 * @method reset_contents
296 */
297 reset_contents: function() {
298 this.review_type.set('value', '');
299 this.vote_input.set('selectedIndex', 0);
300 CodeReviewComment.superclass.reset_contents.apply(this);
301 },
302 /**
303 * Insert the specified HTML into the page.
304 *
305 * @method insert_comment_HTML
306 * @param message_html The HTML of the comment to insert.
307 */
308 insert_comment_HTML: function(message_html){
309 var conversation = Y.get('[id=conversation]');
310 var comment = Y.Node.create(message_html);
311 conversation.appendChild(comment);
312 this.reset_contents();
313 Y.lazr.anim.green_flash({node: comment}).run();
314 },
315 renderUI: function() {
316 CodeReviewComment.superclass.renderUI.apply(this);
317 Y.get('#inline-add-comment').setStyle('display', 'block');
318 },
319 /**
320 * Implementation of Widget.bindUI: Bind events to methods.
321 *
322 * In addition to Comment behaviour, mouseups and keyups on the vote and
323 * review type cause a sync.
324 *
325 * @method bindUI
326 */
327 bindUI: function() {
328 CodeReviewComment.superclass.bindUI.apply(this);
329 this.vote_input.on('mouseup', bind(this.syncUI, this));
330 this.review_type.on('keyup', bind(this.syncUI, this));
331 this.review_type.on('mouseup', bind(this.syncUI, this));
332 },
333 /**
334 * Implementation of Widget.syncUI: Update appearance according to state.
335 *
336 * This enables and disables the review type, in addition to Comment
337 * behaviour.
338 *
339 * @method syncUI
340 */
341 syncUI: function() {
342 CodeReviewComment.superclass.syncUI.apply(this);
343 var review_type_disabled = (this.get_vote() === null);
344 this.review_type.set('disabled', review_type_disabled);
345 }
346});
347Y.lp.CodeReviewComment = CodeReviewComment;
348
349}, '0.1' ,{requires:['oop', 'io', 'widget', 'node', 'lp.client.plugins', 'lp.errors']});
0350
=== added file 'lib/canonical/launchpad/javascript/lp/errors.js'
--- lib/canonical/launchpad/javascript/lp/errors.js 1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/javascript/lp/errors.js 2009-10-14 13:59:13 +0000
@@ -0,0 +1,81 @@
1YUI.add('lp.errors', function(Y) {
2
3Y.lp = Y.namespace('lp');
4
5/*
6 * Create a form button for canceling an error form
7 * that won't reload the page on submit.
8 *
9 * @method cancel_form_button
10 * @return button {Node} The form's cancel button.
11*/
12var cancel_form_button = function() {
13 var button = Y.Node.create('<button>OK</button>');
14 button.on('click', function(e) {
15 e.preventDefault();
16 error_overlay.hide();
17 });
18 return button;
19};
20
21
22var error_overlay;
23/*
24 * Create the form overlay to use when encountering errors.
25 *
26 * @method create_error_overlay
27*/
28var create_error_overlay = function() {
29 if (error_overlay === undefined) {
30 error_overlay = new Y.lazr.FormOverlay({
31 headerContent: '<h2>Error</h2>',
32 form_header: '',
33 form_content: '',
34 form_submit_button: Y.Node.create(
35 '<button style="display:none"></button>'),
36 form_cancel_button: cancel_form_button(),
37 centered: true,
38 visible: false
39 });
40 error_overlay.render();
41 }
42};
43
44/**
45 * Run a callback, optionally flashing a specified node red beforehand.
46 *
47 * If the supplied node evaluates false, the callback is invoked immediately.
48 *
49 * @method maybe_red_flash
50 * @param flash_node The node to flash red, or null for no flash.
51 * @param callback The callback to invoke.
52 */
53var maybe_red_flash = function(flash_node, callback)
54{
55 if (flash_node) {
56 var anim = Y.lazr.anim.red_flash({ node: flash_node });
57 anim.on('end', callback);
58 anim.run();
59 } else {
60 callback();
61 }
62};
63
64
65/*
66 * Take an error message and display in an overlay (creating it if necessary).
67 *
68 * @method display_error
69 * @param flash_node {Node} The node to red flash.
70 * @param msg {String} The message to display.
71*/
72var display_error = function(flash_node, msg) {
73 create_error_overlay();
74 maybe_red_flash(flash_node, function(){
75 error_overlay.showError(msg);
76 error_overlay.show();
77 });
78};
79
80Y.lp.display_error = display_error;
81}, '0.1', {requires:['lazr.formoverlay']});
082
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2009-09-22 18:05:41 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2009-10-14 13:59:13 +0000
@@ -25,6 +25,14 @@
25 tal:define="lp_js string:${icingroot}/build"25 tal:define="lp_js string:${icingroot}/build"
26 tal:attributes="src string:${lp_js}/bugs/bug_tags_entry.js">26 tal:attributes="src string:${lp_js}/bugs/bug_tags_entry.js">
27 </script>27 </script>
28 <script type="text/javascript"
29 tal:define="lp_js string:${icingroot}/build"
30 tal:attributes="src string:${lp_js}/lp/comment.js">
31 </script>
32 <script type="text/javascript"
33 tal:define="lp_js string:${icingroot}/build"
34 tal:attributes="src string:${lp_js}/lp/errors.js">
35 </script>
28 </tal:devmode>36 </tal:devmode>
29 <script type="text/javascript">37 <script type="text/javascript">
30 YUI().use('base', 'node', 'oop', 'event', 'bugs.bugtask_index', function(Y) {38 YUI().use('base', 'node', 'oop', 'event', 'bugs.bugtask_index', function(Y) {
@@ -282,6 +290,12 @@
282 var button = document.getElementById('field.actions.save');290 var button = document.getElementById('field.actions.save');
283 button.style.display = 'none';291 button.style.display = 'none';
284 </script>292 </script>
293 <script type="text/javascript">
294 YUI().use('lp.comment', function(Y) {
295 var comment = new Y.lp.Comment();
296 comment.render();
297 })
298 </script>
285 </div>299 </div>
286 <tal:attachment-link300 <tal:attachment-link
287 define="add_attachment_link context_menu/addcomment"301 define="add_attachment_link context_menu/addcomment"
288302
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2009-09-28 21:06:28 +0000
+++ lib/lp/code/browser/configure.zcml 2009-10-14 13:59:13 +0000
@@ -612,6 +612,9 @@
612 <browser:page612 <browser:page
613 name="+comment-footer"613 name="+comment-footer"
614 template="../templates/codereviewcomment-footer.pt"/>614 template="../templates/codereviewcomment-footer.pt"/>
615 <browser:page
616 name="+fragment"
617 template="../templates/codereviewcomment-fragment.pt"/>
615 </browser:pages>618 </browser:pages>
616 <browser:page619 <browser:page
617 name="+reply"620 name="+reply"
618621
=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py 2009-10-01 13:25:12 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py 2009-10-14 13:59:13 +0000
@@ -41,9 +41,10 @@
41from canonical.launchpad.webapp.interfaces import ITableBatchNavigator41from canonical.launchpad.webapp.interfaces import ITableBatchNavigator
42from lazr.restful.fields import CollectionField, Reference42from lazr.restful.fields import CollectionField, Reference
43from lazr.restful.declarations import (43from lazr.restful.declarations import (
44 call_with, export_as_webservice_entry, export_read_operation,44 call_with, export_as_webservice_entry, export_factory_operation,
45 export_write_operation, exported, operation_parameters,45 export_read_operation, export_write_operation, exported,
46 operation_returns_entry, rename_parameters_as, REQUEST_USER)46 operation_parameters, operation_returns_entry, rename_parameters_as,
47 REQUEST_USER)
4748
4849
49class InvalidBranchMergeProposal(Exception):50class InvalidBranchMergeProposal(Exception):
@@ -454,7 +455,8 @@
454 vote=Choice(vocabulary=CodeReviewVote), review_type=Text(),455 vote=Choice(vocabulary=CodeReviewVote), review_type=Text(),
455 parent=Reference(schema=Interface))456 parent=Reference(schema=Interface))
456 @call_with(owner=REQUEST_USER)457 @call_with(owner=REQUEST_USER)
457 @export_write_operation()458 # ICodeReviewComment supplied as Interface to avoid circular imports.
459 @export_factory_operation(Interface, [])
458 def createComment(owner, subject, content=None, vote=None,460 def createComment(owner, subject, content=None, vote=None,
459 review_type=None, parent=None):461 review_type=None, parent=None):
460 """Create an ICodeReviewComment associated with this merge proposal.462 """Create an ICodeReviewComment associated with this merge proposal.
461463
=== modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt'
--- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2009-10-12 09:40:17 +0000
+++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2009-10-14 13:59:13 +0000
@@ -146,10 +146,12 @@
146146
147Now the code review should be made.147Now the code review should be made.
148148
149 >>> comment = reviewer_webservice.named_post(149 >>> comment_result = reviewer_webservice.named_post(
150 ... merge_proposal['self_link'], 'createComment',150 ... merge_proposal['self_link'], 'createComment',
151 ... subject='Great work', content='This is great work',151 ... subject='Great work', content='This is great work',
152 ... vote=CodeReviewVote.APPROVE.title, review_type='code').jsonBody()152 ... vote=CodeReviewVote.APPROVE.title, review_type='code')
153 >>> comment_link = comment_result.getHeader('Location')
154 >>> comment = reviewer_webservice.get(comment_link).jsonBody()
153 >>> pprint_entry(comment)155 >>> pprint_entry(comment)
154 branch_merge_proposal_link: u'http://.../~source/fooix/fix-it/+merge/1'156 branch_merge_proposal_link: u'http://.../~source/fooix/fix-it/+merge/1'
155 id: 3157 id: 3
156158
=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt 2009-09-19 05:01:40 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt 2009-10-14 13:59:13 +0000
@@ -20,6 +20,14 @@
20 tal:define="lp_js string:${icingroot}/build"20 tal:define="lp_js string:${icingroot}/build"
21 tal:attributes="src string:${lp_js}/code/codereview.js">21 tal:attributes="src string:${lp_js}/code/codereview.js">
22 </script>22 </script>
23 <script type="text/javascript"
24 tal:define="lp_js string:${icingroot}/build"
25 tal:attributes="src string:${lp_js}/lp/comment.js">
26 </script>
27 <script type="text/javascript"
28 tal:define="lp_js string:${icingroot}/build"
29 tal:attributes="src string:${lp_js}/lp/errors.js">
30 </script>
23</metal:block>31</metal:block>
2432
2533
@@ -65,10 +73,23 @@
65 tal:content="structure link/render">73 tal:content="structure link/render">
66 Add comment74 Add comment
67 </div>75 </div>
68 <tal:comments condition="view/conversation"76 <div id="conversation"
69 replace="structure view/conversation/@@+render"/>77 tal:content="structure view/conversation/@@+render"/>
78 </div>
79 <!-- Hide inline commenting if YUI isn't used. -->
80 <div id="inline-add-comment" style="display: none">
81 <tal:comment replace="structure context/@@+comment/++form++" />
82 <div class="actions" id="launchpad-form-actions">
83 <input type="submit" id="field.actions.add" name="field.actions.add" value="Save Comment" class="button" />
84 </div>
70 </div>85 </div>
7186
87 <script type="text/javascript">
88 YUI().use('lp.comment', function(Y) {
89 var comment = new Y.lp.CodeReviewComment();
90 comment.render();
91 })
92 </script>
72 <div class="yui-g">93 <div class="yui-g">
73 <div class="yui-u first">94 <div class="yui-u first">
74 <div id="source-revisions"95 <div id="source-revisions"
7596
=== added file 'lib/lp/code/templates/codereviewcomment-fragment.pt'
--- lib/lp/code/templates/codereviewcomment-fragment.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/codereviewcomment-fragment.pt 2009-10-14 13:59:13 +0000
@@ -0,0 +1,2 @@
1<tal:fragment replace="structure view/comment/@@+render"
2 xmlns:tal="http://xml.zope.org/namespaces/tal" />
03
=== modified file 'lib/lp/code/windmill/testing.py'
--- lib/lp/code/windmill/testing.py 2009-09-15 08:58:30 +0000
+++ lib/lp/code/windmill/testing.py 2009-10-14 13:59:13 +0000
@@ -9,6 +9,9 @@
9 ]9 ]
1010
1111
12import windmill
13
14from canonical.launchpad.webapp import canonical_url as real_canonical_url
12from canonical.testing.layers import BaseWindmillLayer15from canonical.testing.layers import BaseWindmillLayer
1316
1417
@@ -16,3 +19,16 @@
16 """Layer for Code Windmill tests."""19 """Layer for Code Windmill tests."""
1720
18 base_url = 'http://code.launchpad.dev:8085/'21 base_url = 'http://code.launchpad.dev:8085/'
22
23
24def canonical_url(*args, **kwargs):
25 """Wrapper for canonical_url that ensures test URLs are returned.
26
27 Real canonical URLs require SSL support, but our test clients don't
28 trust the certificate.
29 """
30 url = real_canonical_url(*args, **kwargs)
31 url = url.replace(
32 'http://code.launchpad.dev', windmill.settings['TEST_URL'])
33 assert url.startswith(windmill.settings['TEST_URL'])
34 return url
1935
=== added file 'lib/lp/code/windmill/tests/test_merge_proposal_commenting.py'
--- lib/lp/code/windmill/tests/test_merge_proposal_commenting.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/windmill/tests/test_merge_proposal_commenting.py 2009-10-14 13:59:13 +0000
@@ -0,0 +1,55 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Test for the inline commenting UI."""
5
6__metaclass__ = type
7__all__ = []
8
9import unittest
10
11import transaction
12from windmill.authoring import WindmillTestClient
13
14from canonical.launchpad.windmill.testing import lpuser
15from canonical.uuid import generate_uuid
16from lp.code.windmill.testing import canonical_url, CodeWindmillLayer
17from lp.testing import TestCaseWithFactory
18
19WAIT_PAGELOAD = u'30000'
20WAIT_ELEMENT_COMPLETE = u'30000'
21WAIT_CHECK_CHANGE = u'1000'
22ADD_COMMENT_BUTTON = (
23 u'//input[@id="field.actions.add" and @class="button js-action"]')
24
25
26class TestMergeProposalCommenting(TestCaseWithFactory):
27
28 layer = CodeWindmillLayer
29
30
31 def test_merge_proposal_commenting(self):
32 """Test commenting on bugs."""
33 client = WindmillTestClient('Bug commenting')
34 lpuser.NO_PRIV.ensure_login(client)
35
36 proposal = self.factory.makeBranchMergeProposal()
37 transaction.commit()
38 client.open(url=canonical_url(proposal))
39 client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
40 client.waits.forElement(xpath=ADD_COMMENT_BUTTON)
41
42 # Generate a unique piece of text, so we can run the test multiple
43 # times, without resetting the db.
44 new_comment_text = generate_uuid()
45 client.type(text=new_comment_text, id="field.comment")
46 client.click(xpath=ADD_COMMENT_BUTTON)
47 # A PRE inside a boardCommentBody, itself somewhere in the
48 # #conversation
49 client.waits.forElement(
50 xpath='//div[@id="conversation"]//div[@class="boardCommentBody"]'
51 '/pre[contains(., "%s")]' % new_comment_text)
52
53
54def test_suite():
55 return unittest.TestLoader().loadTestsFromName(__name__)