Merge lp:~abentley/launchpad/reuse-refactor into lp:launchpad

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
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+13283@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

= 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 ==
As a drive-by, the export of IBranchMergeProposal.createComment is
changed to export_factory_operation. This ensures that its return value
is usable.

== Tests ==
bin/test -v --layer=BugsWindmillLayer

== 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:
  lib/lp/code/interfaces/branchmergeproposal.py
  lib/canonical/launchpad/javascript/bugs/bugtask-index.js
  lib/canonical/launchpad/javascript/lp/errors.js
  lib/canonical/launchpad/interfaces/_schema_circular_imports.py
  lib/lp/bugs/templates/bugtask-index.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.

== Pylint notices ==

lib/lp/code/interfaces/branchmergeproposal.py
    27: [F0401] Unable to import 'lazr.lifecycle.event' (No module named
lifecycle)
    42: [F0401] Unable to import 'lazr.restful.fields' (No module named
restful)
    43: [F0401] Unable to import 'lazr.restful.declarations' (No module
named restful)

lib/canonical/launchpad/interfaces/_schema_circular_imports.py
    18: [F0401] Unable to import 'lazr.restful.declarations' (No module
named restful)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrUklMACgkQ0F+nu1YWqI0TIgCfcsmvIb+qgNSDBGRnDyciBJvh
h3gAnin+FvxrnRxj4Miho4vOzpQF912z
=+Dh5
-----END PGP SIGNATURE-----

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
=== 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:58:19 +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:58:19 +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:58:19 +0000
@@ -0,0 +1,191 @@
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
190
191}, '0.1' ,{requires:['oop', 'widget', 'lp.client.plugins', 'lp.errors']});
0192
=== 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:58:19 +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:58:19 +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/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:58:19 +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:58:19 +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