Merge lp:~wallyworld/launchpad/private-dupe-bug-warning2-943497 into lp:launchpad

Proposed by Ian Booth
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 15719
Proposed branch: lp:~wallyworld/launchpad/private-dupe-bug-warning2-943497
Merge into: lp:launchpad
Diff against target: 809 lines (+434/-98)
7 files modified
lib/canonical/launchpad/icing/css/forms.css (+1/-2)
lib/canonical/launchpad/icing/css/modifiers.css (+8/-0)
lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css (+10/-0)
lib/lp/app/javascript/formoverlay/formoverlay.js (+14/-11)
lib/lp/bugs/javascript/duplicates.js (+278/-29)
lib/lp/bugs/javascript/tests/test_duplicates.html (+1/-0)
lib/lp/bugs/javascript/tests/test_duplicates.js (+122/-56)
To merge this branch: bzr merge lp:~wallyworld/launchpad/private-dupe-bug-warning2-943497
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+116793@code.launchpad.net

Commit message

Initial work to improve the bug duplicate form overlay to allow the selected bug to be previewed prior to submission.

Description of the change

== Implementation ==

This branch fixes a number of issues with the mark bug as duplicate form. See the linked bugs. The work is currently only for the bug selection widget used when marking bugs as duplicates. It would need to be generalised to be useful when selecting bugs to link to branches etc.

Not quite everything is implemented in this branch but it's big enough to be put up for review. The bit that is missing is marked with a TODO.

When a bug number is entered, a web service API call is made to get the json data for the bug. This data is presented to the user to allow then to confirm the bug is what they want. The TODO is because not all necessary data is returned and is currently hard coded. A new service API needs to be added to provided the required data.

Some css changes were necessary. Here some of the "non obvious" ones...

Needed to make spacing around form overlay form table correct due to css rule ordering :-

13 table.form, table.extra-options {
...
16 - margin: 1em 0;
17 + margin: 1em 0 inherit inherit;

Needed to un-squash form buttons when they are not little lazr icons.

48 +.yui3-lazr-formoverlay-form div.yui3-lazr-formoverlay-actions button {
49 + margin-right: 0.7em;
50 + }
51

Expand out the error div on the form overlay when there is an error so it is not all squashed up when the form is narrow.

58 +.yui3-lazr-formoverlay-form .yui3-lazr-formoverlay-errors:not(:empty) {
59 + min-width: 250px;
60 + text-align: center;
61 + }
62 +

== Demo and QA ==

Here's an early video of it in operation.

http://people.canonical.com/~ianb/bug-dup.ogv

This video has a number of minor flaws (eg uncentred form, text alignment). These are addressed in this branch.

== Tests ==

Add a number of new yui tests and update existing ones.

== Lint ==

Linting changed files:
  lib/canonical/launchpad/icing/css/forms.css
  lib/canonical/launchpad/icing/css/modifiers.css
  lib/lp/app/javascript/formoverlay/formoverlay.js
  lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css
  lib/lp/bugs/javascript/duplicates.js
  lib/lp/bugs/javascript/tests/test_duplicates.html
  lib/lp/bugs/javascript/tests/test_duplicates.js

Clean except for some css noise.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :
Download full text (4.0 KiB)

I think this is a good start. It is must better than what users are seeing now. I have a few remarks.

> === modified file 'lib/lp/app/javascript/formoverlay/formoverlay.js'
> --- lib/lp/app/javascript/formoverlay/formoverlay.js 2012-06-26 00:15:11 +0000
> +++ lib/lp/app/javascript/formoverlay/formoverlay.js 2012-07-26 11:02:24 +0000
> @@ -492,18 +492,19 @@
> * @method showError
> */
> showError: function(error_msgs){
> + var error_content;
> if (Y.Lang.isString(error_msgs)) {
> - error_msgs = [error_msgs];
> + error_content = Y.Node.create('<p></p>').set('text', error_msgs);
> + } else {
> + error_content = Y.Node.create(
> + '<p>The following errors were encountered:</p><ul></ul>');
> + var error_list = error_content.one('ul');
> + Y.each(error_msgs, function(error_msg){
> + error_list.appendChild(
> + Y.Node.create('<li></li>').set('text', error_msg));
> + });

This iteration is expensive. repeated appends also call repeated refreshes
You can build the nodes individually, or pass a string to update the UI once.

            var li_nodes = new Y.NodeList([]);
            Y.each(error_msgs, function(error_msg){
                li_nodes.push(
                    Y.Node.create('<li></li>').set('text', error_msg));
            });
            error_list.append(li_nodes);

> === modified file 'lib/lp/bugs/javascript/duplicates.js'
> --- lib/lp/bugs/javascript/duplicates.js 2012-07-25 00:31:04 +0000
> +++ lib/lp/bugs/javascript/duplicates.js 2012-07-26 11:02:24 +0000
> @@ -13,10 +13,10 @@
> // Overlay related vars.
> var submit_button_html =
> '<button type="submit" name="field.actions.change" ' +
> - 'value="Change" class="lazr-pos lazr-btn" >OK</button>';
> + '>OK</button>';
> var cancel_button_html =
> '<button type="button" name="field.actions.cancel" ' +
> - 'class="lazr-neg lazr-btn" >Cancel</button>';
> + '>Cancel</button>';

Thank you for removing the ambiguous and tiny icons. "Ok" is a poor
action term that Launchpad and other UI guidelines avoid. "Save" or
"Change" are better generic terms. If we have a separate action/link to
remove/clear duplicate, then this action would be named "Save
Duplicate".

> @@ -82,12 +85,250 @@
> update_dupe_link.on('click', function(e) {
> // Only go ahead if we have received the form content by the
> // time the user clicks:
> - if (that.duplicate_form_overlay) {
> + if (that.duplicate_form) {
> e.halt();
> - that.duplicate_form_overlay.show();
> + that.duplicate_form.show();
> Y.DOM.byId('field.duplicateof').focus();
> }
> });
> + // We the due form overlay is hidden, we need to reset the form.

Maybe s/We/When/;

> + // Template for rendering the bug details.
> + _bug_details_template: function() {
> + return [
> + '<div id="client-listing" style="max-width: 75em;">',

This looks too large. 60 is the max for English and that is what we
use else-where in Lp. Wh...

Read more...

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/icing/css/forms.css'
--- lib/canonical/launchpad/icing/css/forms.css 2012-06-23 13:44:13 +0000
+++ lib/canonical/launchpad/icing/css/forms.css 2012-07-27 01:21:21 +0000
@@ -90,7 +90,6 @@
90.extra-form-buttons {90.extra-form-buttons {
91 text-align: center;91 text-align: center;
92 padding-top: 1em;92 padding-top: 1em;
93 white-space: nowrap;
94 }93 }
95.extra-form-buttons button {94.extra-form-buttons button {
96 margin-right: 0.7em;95 margin-right: 0.7em;
@@ -140,7 +139,7 @@
140table.form, table.extra-options {139table.form, table.extra-options {
141 /* Many forms are laid out using tables, with appropriate spacing: */140 /* Many forms are laid out using tables, with appropriate spacing: */
142 /* http://launchpad.dev/firefox/+edit */141 /* http://launchpad.dev/firefox/+edit */
143 margin: 1em 0;142 margin: 1em 0 inherit inherit;
144 width: 100%;143 width: 100%;
145 }144 }
146table.form th {145table.form th {
147146
=== modified file 'lib/canonical/launchpad/icing/css/modifiers.css'
--- lib/canonical/launchpad/icing/css/modifiers.css 2012-07-23 03:40:33 +0000
+++ lib/canonical/launchpad/icing/css/modifiers.css 2012-07-27 01:21:21 +0000
@@ -112,6 +112,14 @@
112 height: 10px;112 height: 10px;
113 }113 }
114114
115.ellipsis {
116 white-space: nowrap;
117 overflow: hidden;
118 text-overflow: ellipsis;
119 -o-text-overflow: ellipsis;
120 -ms-text-overflow: ellipsis;
121 }
122
115.exception {123.exception {
116 color: #cc0000;124 color: #cc0000;
117 }125 }
118126
=== modified file 'lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css'
--- lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css 2012-05-12 01:56:24 +0000
+++ lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css 2012-07-27 01:21:21 +0000
@@ -21,12 +21,22 @@
21 text-align: right;21 text-align: right;
22 }22 }
2323
24.yui3-lazr-formoverlay-form div.yui3-lazr-formoverlay-actions button {
25 margin-right: 0.7em;
26 }
27
24.yui3-lazr-formoverlay-form .yui3-lazr-formoverlay-errors {28.yui3-lazr-formoverlay-form .yui3-lazr-formoverlay-errors {
25 padding-top: 0;29 padding-top: 0;
26 padding-bottom: 0;30 padding-bottom: 0;
27 color: red;31 color: red;
28 }32 }
2933
34.yui3-lazr-formoverlay-form .yui3-lazr-formoverlay-errors:not(:empty) {
35 min-width: 250px;
36 margin-left: auto;
37 margin-right: auto;
38 }
39
30.yui3-lazr-formoverlay-form table {40.yui3-lazr-formoverlay-form table {
31 /* This gets rid of the 12px margin-bottom that yui specifies41 /* This gets rid of the 12px margin-bottom that yui specifies
32 * in its base.css.42 * in its base.css.
3343
=== modified file 'lib/lp/app/javascript/formoverlay/formoverlay.js'
--- lib/lp/app/javascript/formoverlay/formoverlay.js 2012-06-26 00:15:11 +0000
+++ lib/lp/app/javascript/formoverlay/formoverlay.js 2012-07-27 01:21:21 +0000
@@ -492,18 +492,21 @@
492 * @method showError492 * @method showError
493 */493 */
494 showError: function(error_msgs){494 showError: function(error_msgs){
495 var error_content;
495 if (Y.Lang.isString(error_msgs)) {496 if (Y.Lang.isString(error_msgs)) {
496 error_msgs = [error_msgs];497 error_content = Y.Node.create('<p></p>').set('text', error_msgs);
498 } else {
499 error_content = Y.Node.create(
500 '<p>The following errors were encountered:</p><ul></ul>');
501 var error_list = error_content.one('ul');
502 var li_nodes = new Y.NodeList([]);
503 Y.each(error_msgs, function(error_msg){
504 li_nodes.push(
505 Y.Node.create('<li></li>').set('text', error_msg));
506 });
507 error_list.append(li_nodes);
497 }508 }
498 var error_html = "The following errors were encountered: <ul>";509 this.error_node.appendChild(error_content);
499 Y.each(error_msgs, function(error_msg){
500 // XXX noodles 2009-02-13 bug=342212. We need to decide on
501 // or provide our own escapeHTML() helper.
502 error_html += "<li>" + error_msg.replace(/<([^>]+)>/g,'') +
503 "</li>";
504 });
505 error_html += "</ul>";
506 this.error_node.set('innerHTML', error_html);
507 },510 },
508511
509 /**512 /**
@@ -512,7 +515,7 @@
512 * @method clearError515 * @method clearError
513 */516 */
514 clearError: function(){517 clearError: function(){
515 this.error_node.set('innerHTML', '');518 this.error_node.empty(true);
516 },519 },
517520
518 /**521 /**
519522
=== modified file 'lib/lp/bugs/javascript/duplicates.js'
--- lib/lp/bugs/javascript/duplicates.js 2012-07-25 00:31:04 +0000
+++ lib/lp/bugs/javascript/duplicates.js 2012-07-27 01:21:21 +0000
@@ -13,10 +13,10 @@
13// Overlay related vars.13// Overlay related vars.
14var submit_button_html =14var submit_button_html =
15 '<button type="submit" name="field.actions.change" ' +15 '<button type="submit" name="field.actions.change" ' +
16 'value="Change" class="lazr-pos lazr-btn" >OK</button>';16 '>Save Duplicate</button>';
17var cancel_button_html =17var cancel_button_html =
18 '<button type="button" name="field.actions.cancel" ' +18 '<button type="button" name="field.actions.cancel" ' +
19 'class="lazr-neg lazr-btn" >Cancel</button>';19 '>Cancel</button>';
2020
21/**21/**
22 * Manage the process of marking a bug as a duplicate.22 * Manage the process of marking a bug as a duplicate.
@@ -24,6 +24,8 @@
24 */24 */
25namespace.MarkBugDuplicate = Y.Base.create("markBugDupeWidget", Y.Widget, [], {25namespace.MarkBugDuplicate = Y.Base.create("markBugDupeWidget", Y.Widget, [], {
26 initializer: function(cfg) {26 initializer: function(cfg) {
27 this.lp_client = new Y.lp.client.Launchpad(cfg);
28 this.io_provider = Y.lp.client.get_configured_io_provider(cfg);
27 var update_dupe_link = cfg.update_dupe_link;29 var update_dupe_link = cfg.update_dupe_link;
28 if (update_dupe_link) {30 if (update_dupe_link) {
29 // Add a class denoting the link as js-action link.31 // Add a class denoting the link as js-action link.
@@ -55,19 +57,20 @@
55 '</p>';57 '</p>';
56 }58 }
5759
58 this.duplicate_form_overlay = new Y.lazr.FormOverlay({60 this.duplicate_form = new Y.lazr.FormOverlay({
59 headerContent: '<h2>Mark bug report as duplicate</h2>',61 headerContent: '<h2>Mark bug report as duplicate</h2>',
60 form_header: form_header,62 form_header: form_header,
61 form_submit_button: Y.Node.create(submit_button_html),63 form_submit_button: Y.Node.create(submit_button_html),
62 form_cancel_button: Y.Node.create(cancel_button_html),64 form_cancel_button: Y.Node.create(cancel_button_html),
63 centered: true,65 centered: true,
66 // The form submit first searches for the bug.
64 form_submit_callback:67 form_submit_callback:
65 Y.bind(this._update_bug_duplicate, this),68 Y.bind(this._find_duplicate_bug, this),
66 visible: false,69 visible: false,
67 io_provider: this.get('io_provider')70 io_provider: this.io_provider
68 });71 });
69 this.duplicate_form_overlay.render('#duplicate-form-container');72 this.duplicate_form.render('#duplicate-form-container');
70 this.duplicate_form_overlay.loadFormContentAndRender(73 this.duplicate_form.loadFormContentAndRender(
71 mark_dupe_form_url);74 mark_dupe_form_url);
72 },75 },
7376
@@ -82,12 +85,251 @@
82 update_dupe_link.on('click', function(e) {85 update_dupe_link.on('click', function(e) {
83 // Only go ahead if we have received the form content by the86 // Only go ahead if we have received the form content by the
84 // time the user clicks:87 // time the user clicks:
85 if (that.duplicate_form_overlay) {88 if (that.duplicate_form) {
86 e.halt();89 e.halt();
87 that.duplicate_form_overlay.show();90 that.duplicate_form.show();
88 Y.DOM.byId('field.duplicateof').focus();91 Y.DOM.byId('field.duplicateof').focus();
89 }92 }
90 });93 });
94 // When the dupe form overlay is hidden, we need to reset the form.
95 this.duplicate_form.after('visibleChange', function() {
96 if (!this.get('visible')) {
97 that._hide_confirm_node();
98 }
99 });
100 },
101
102 /**
103 * Show a spinner next to the specified node.
104 *
105 * @method _show_bug_spinner
106 * @private
107 */
108 _show_bug_spinner: function(node) {
109 if( node === null) {
110 return;
111 }
112 var spinner_node = Y.Node.create(
113 '<img class="spinner" src="/@@/spinner" alt="Searching..." />');
114 node.insert(spinner_node, 'after');
115 },
116
117 /**
118 * Hide the spinner next to the specified node.
119 *
120 * @method _hide_bug_spinner
121 * @private
122 */
123 _hide_bug_spinner: function(node) {
124 if( node === null) {
125 return;
126 }
127 var spinner = node.get('parentNode').one('.spinner');
128 if (spinner !== null) {
129 spinner.remove();
130 }
131 },
132
133 /**
134 * Look up the selected bug and get the user to confirm that it is the one
135 * they want.
136 *
137 * @param data
138 * @private
139 */
140 _find_duplicate_bug: function(data) {
141 var new_dup_id = Y.Lang.trim(data['field.duplicateof'][0]);
142 // If there's no bug data entered then we are deleting the duplicate
143 // link.
144 if (new_dup_id === '') {
145 this._update_bug_duplicate(new_dup_id);
146 return;
147 }
148 var that = this;
149 var qs_data
150 = Y.lp.client.append_qs("", "ws.accept", "application.json");
151
152 var bug_field = this.duplicate_form.form_node
153 .one('[id="field.duplicateof"]');
154 var config = {
155 on: {
156 start: function() {
157 that._show_bug_spinner(bug_field);
158 that.duplicate_form.clearError();
159 },
160 end: function() {
161 that._hide_bug_spinner(bug_field);
162 },
163 success: function(id, response) {
164 if (response.responseText === '') {
165 return;
166 }
167 var bug_data = Y.JSON.parse(response.responseText);
168 that._confirm_selected_bug(bug_data);
169 },
170 failure: function(id, response) {
171 var error_msg;
172 if (response.status === 404) {
173 error_msg = new_dup_id +
174 ' is not a valid bug number.';
175 } else {
176 error_msg = response.responseText;
177 }
178 that.duplicate_form.showError(error_msg);
179 }
180 },
181 data: qs_data
182 };
183 var uri
184 = Y.lp.client.get_absolute_uri("/api/devel/bugs/" + new_dup_id);
185 this.io_provider.io(uri, config);
186 },
187
188 // Template for rendering the bug details.
189 _bug_details_template: function() {
190 return [
191 '<div id="client-listing">',
192 ' <div class="buglisting-col1">',
193 ' <div class="importance {{importance_class}}">',
194 ' {{importance}}',
195 ' </div>',
196 ' <div class="status {{status_class}}">',
197 ' {{status}}',
198 ' </div>',
199 ' <div class="buginfo-extra">',
200 ' <div class="information_type">',
201 ' {{information_type}}',
202 ' </div>',
203 ' </div>',
204 ' </div>',
205 ' <div class="buglisting-col2" style="max-width: 60em;">',
206 ' <span class="bugnumber">#{{id}}</span>',
207 ' <a href="{{bug_url}}" class="bugtitle">{{bug_summary}}</a>',
208 ' <div class="buginfo-extra">',
209 ' <p class="ellipsis line-block" style="max-width: 70em;">',
210 ' {{description}}</p>',
211 ' </div>',
212 ' </div>',
213 '</div>'
214 ].join(' ');
215 },
216
217 // Template for the bug confirmation form.
218 _bug_confirmation_form_template: function() {
219 return [
220 '<div class="confirmation-node yui3-lazr-formoverlay-form" ',
221 'style="margin-top: 6px;">',
222 '{{> bug_details}}',
223 '<div class="yui3-lazr-formoverlay-errors"></div>',
224 '<div class="extra-form-buttons">',
225 ' <button class="yes_button" type="button">',
226 ' Save Duplicate</button>',
227 ' <button class="no_button" type="button">Search Again</button>',
228 ' <button class="cancel_button" type="button">Cancel</button>',
229 '</div>',
230 '</div>'].join('');
231 },
232
233 /**
234 * Ask the user to confirm the chosen bug is the one they want.
235 * @param bug_data
236 * @private
237 */
238 _confirm_selected_bug: function(bug_data) {
239 // TODO - get real data from the server
240 bug_data.importance = 'High';
241 bug_data.importance_class = 'importanceHIGH';
242 bug_data.status = 'Triaged';
243 bug_data.status_class = 'statusTRIAGED';
244 bug_data.bug_summary = bug_data.title;
245 bug_data.bug_url = bug_data.web_link;
246
247 var bug_id = bug_data.id;
248 var html = Y.lp.mustache.to_html(
249 this._bug_confirmation_form_template(), bug_data,
250 {bug_details: this._bug_details_template()});
251 var confirm_node = Y.Node.create(html);
252 this._show_confirm_node(confirm_node);
253 var that = this;
254 confirm_node.one(".yes_button")
255 .on('click', function(e) {
256 e.halt();
257 that._update_bug_duplicate(bug_id);
258 });
259
260 confirm_node.one(".no_button")
261 .on('click', function(e) {
262 e.halt();
263 that._hide_confirm_node(confirm_node);
264 });
265 confirm_node.one(".cancel_button")
266 .on('click', function(e) {
267 e.halt();
268 that.duplicate_form.hide();
269 });
270 },
271
272 // Centre the duplicate form along the x axis without changing y position.
273 _xaxis_centre: function() {
274 var viewport = Y.DOM.viewportRegion();
275 var new_x = (viewport.right + viewport.left)/2 -
276 this.duplicate_form.get('contentBox').get('offsetWidth')/2;
277 this.duplicate_form.move([new_x, this.duplicate_form._getY()]);
278
279 },
280
281 /** Show the bug selection confirmation node.
282 * @method _show_confirm_node
283 * @private
284 */
285 _show_confirm_node: function(confirmation_node) {
286 this.duplicate_form.form_header_node
287 .insert(confirmation_node, 'after');
288 this.confirmation_node = confirmation_node;
289 this._xaxis_centre();
290 this._fade_in(confirmation_node, this.duplicate_form.form_node);
291 },
292
293 /** Hide the bug selection confirmation node.
294 * @method _hide_confirm_node
295 * @private
296 */
297 _hide_confirm_node: function() {
298 this.duplicate_form.form_node.removeClass('hidden');
299 if (Y.Lang.isValue(this.confirmation_node)) {
300 this._fade_in(
301 this.duplicate_form.form_node, this.confirmation_node);
302 this._xaxis_centre();
303 this.confirmation_node.remove();
304 this.confirmation_node = null;
305 }
306 },
307
308 // Animate the display of content.
309 _fade_in: function(content_node, old_content, use_animation) {
310 content_node.removeClass('hidden');
311 if (old_content === null) {
312 content_node.removeClass('transparent');
313 content_node.setStyle('opacity', 1);
314 content_node.show();
315 return;
316 }
317 old_content.addClass('hidden');
318 if (!Y.Lang.isValue(use_animation)) {
319 use_animation = this.get('use_animation');
320 }
321 if (!use_animation) {
322 old_content.setStyle('opacity', 1);
323 return;
324 }
325 content_node.addClass('transparent');
326 content_node.setStyle('opacity', 0);
327 var fade_in = new Y.Anim({
328 node: content_node,
329 to: {opacity: 1},
330 duration: 0.8
331 });
332 fade_in.run();
91 },333 },
92334
93 /**335 /**
@@ -101,6 +343,7 @@
101 */343 */
102 _update_bug_duplicate_success: function(updated_entry, new_dup_url,344 _update_bug_duplicate_success: function(updated_entry, new_dup_url,
103 new_dup_id) {345 new_dup_id) {
346 this.duplicate_form.hide();
104 updated_entry.set('duplicate_of_link', new_dup_url);347 updated_entry.set('duplicate_of_link', new_dup_url);
105 this.set('lp_bug_entry', updated_entry);348 this.set('lp_bug_entry', updated_entry);
106349
@@ -129,16 +372,20 @@
129 dupe_span.one('a').set('href', update_dup_url);372 dupe_span.one('a').set('href', update_dup_url);
130 this._hide_comment_on_duplicate_warning();373 this._hide_comment_on_duplicate_warning();
131 }374 }
375 var anim_duration = 1;
376 if (!this.get('anim_duration')) {
377 anim_duration = 0;
378 }
132 Y.lp.anim.green_flash({379 Y.lp.anim.green_flash({
133 node: dupe_span,380 node: dupe_span,
134 duration: this.get('anim_duration')381 duration: anim_duration
135 }).run();382 }).run();
136 // ensure the new link is hooked up correctly:383 // ensure the new link is hooked up correctly:
137 var that = this;384 var that = this;
138 dupe_span.one('a').on(385 dupe_span.one('a').on(
139 'click', function(e){386 'click', function(e){
140 e.preventDefault();387 e.preventDefault();
141 that.duplicate_form_overlay.show();388 that.duplicate_form.show();
142 Y.DOM.byId('field.duplicateof').focus();389 Y.DOM.byId('field.duplicateof').focus();
143 });390 });
144 },391 },
@@ -156,23 +403,26 @@
156 // Reset the lp_bug_entry.duplicate_of_link as it wasn't403 // Reset the lp_bug_entry.duplicate_of_link as it wasn't
157 // updated.404 // updated.
158 this.get('lp_bug_entry').set('duplicate_of_link', old_dup_url);405 this.get('lp_bug_entry').set('duplicate_of_link', old_dup_url);
406 var error_msg = response.responseText;
159 if (response.status === 400) {407 if (response.status === 400) {
160 this.duplicate_form_overlay.showError(408 var error_info = response.responseText.split('\n');
161 new_dup_id + ' is not a valid bug number or nickname.');409 error_msg = error_info.slice(1).join(' ');
162 } else {
163 this.duplicate_form_overlay.showError(response.responseText);
164 }410 }
165 this.duplicate_form_overlay.show();411 this.confirmation_node.one('.yui3-lazr-formoverlay-errors')
412 .setContent(error_msg);
413 this.confirmation_node.one(".yes_button").addClass('hidden');
414 this._xaxis_centre();
415
166 },416 },
167417
168 /**418 /**
169 * Update the bug duplicate via the LP API419 * Update the bug duplicate via the LP API
170 *420 *
171 * @method _update_bug_duplicate421 * @method _update_bug_duplicate
172 * @param data422 * @param new_dup_id
173 * @private423 * @private
174 */424 */
175 _update_bug_duplicate: function(data) {425 _update_bug_duplicate: function(new_dup_id) {
176 // XXX noodles 2009-03-17 bug=336866 It seems the etag426 // XXX noodles 2009-03-17 bug=336866 It seems the etag
177 // returned by lp_save() is incorrect. Remove it for now427 // returned by lp_save() is incorrect. Remove it for now
178 // so that the second save does not result in a '412428 // so that the second save does not result in a '412
@@ -186,11 +436,7 @@
186 var lp_bug_entry = this.get('lp_bug_entry');436 var lp_bug_entry = this.get('lp_bug_entry');
187 lp_bug_entry.removeAttr('http_etag');437 lp_bug_entry.removeAttr('http_etag');
188438
189 // Hide the formoverlay:
190 this.duplicate_form_overlay.hide();
191
192 var new_dup_url = null;439 var new_dup_url = null;
193 var new_dup_id = Y.Lang.trim(data['field.duplicateof'][0]);
194 if (new_dup_id !== '') {440 if (new_dup_id !== '') {
195 var self_link = lp_bug_entry.get('self_link');441 var self_link = lp_bug_entry.get('self_link');
196 var last_slash_index = self_link.lastIndexOf('/');442 var last_slash_index = self_link.lastIndexOf('/');
@@ -201,14 +447,20 @@
201447
202 var dupe_span = this.get('dupe_span');448 var dupe_span = this.get('dupe_span');
203 var that = this;449 var that = this;
450 var submit_btn = null;
451 if (Y.Lang.isValue(this.confirmation_node)) {
452 submit_btn = this.confirmation_node.one(".yes_button");
453 }
204 var config = {454 var config = {
205 on: {455 on: {
206 start: function() {456 start: function() {
207 dupe_span.removeClass('sprite bug-dupe');457 dupe_span.removeClass('sprite bug-dupe');
208 dupe_span.addClass('update-in-progress-message');458 dupe_span.addClass('update-in-progress-message');
459 that._show_bug_spinner(submit_btn);
209 },460 },
210 end: function() {461 end: function() {
211 dupe_span.removeClass('update-in-progress-message');462 dupe_span.removeClass('update-in-progress-message');
463 that._hide_bug_spinner(submit_btn);
212 },464 },
213 success: function(updated_entry) {465 success: function(updated_entry) {
214 that._update_bug_duplicate_success(466 that._update_bug_duplicate_success(
@@ -233,7 +485,7 @@
233 */485 */
234 _show_comment_on_duplicate_warning: function() {486 _show_comment_on_duplicate_warning: function() {
235 var duplicate_warning = Y.one('#warning-comment-on-duplicate');487 var duplicate_warning = Y.one('#warning-comment-on-duplicate');
236 if (duplicate_warning === null) {488 if (!Y.Lang.isValue(duplicate_warning)) {
237 var container = Y.one('#add-comment-form');489 var container = Y.one('#add-comment-form');
238 var first_node = container.get('firstChild');490 var first_node = container.get('firstChild');
239 duplicate_warning = Y.Node.create(491 duplicate_warning = Y.Node.create(
@@ -286,16 +538,13 @@
286 }538 }
287 },539 },
288 // Override for testing.540 // Override for testing.
289 io_provider: {541 use_animation: {
290 },542 value: true
291 // Override for testing.
292 anim_duration: {
293 value: 1
294 }543 }
295 }544 }
296});545});
297546
298}, "0.1", {"requires": [547}, "0.1", {"requires": [
299 "base", "io", "oop", "node", "event", "json", "lazr.formoverlay",548 "base", "io", "oop", "node", "event", "json", "lazr.formoverlay",
300 "lazr.effects", "lp.app.widgets.expander",549 "lazr.effects", "lp.app.widgets.expander", "lp.mustache",
301 "lp.app.formwidgets.resizing_textarea", "plugin"]});550 "lp.app.formwidgets.resizing_textarea", "plugin"]});
302551
=== modified file 'lib/lp/bugs/javascript/tests/test_duplicates.html'
--- lib/lp/bugs/javascript/tests/test_duplicates.html 2012-07-25 00:31:04 +0000
+++ lib/lp/bugs/javascript/tests/test_duplicates.html 2012-07-27 01:21:21 +0000
@@ -27,6 +27,7 @@
27 <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />27 <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
2828
29 <!-- Dependencies -->29 <!-- Dependencies -->
30 <script type="text/javascript" src="../../../../../build/js/lp/app/mustache.js"></script>
30 <script type="text/javascript" src="../../../../../build/js/lp/app/anim/anim.js"></script>31 <script type="text/javascript" src="../../../../../build/js/lp/app/anim/anim.js"></script>
31 <script type="text/javascript" src="../../../../../build/js/lp/app/client.js"></script>32 <script type="text/javascript" src="../../../../../build/js/lp/app/client.js"></script>
32 <script type="text/javascript" src="../../../../../build/js/lp/app/errors.js"></script>33 <script type="text/javascript" src="../../../../../build/js/lp/app/errors.js"></script>
3334
=== modified file 'lib/lp/bugs/javascript/tests/test_duplicates.js'
--- lib/lp/bugs/javascript/tests/test_duplicates.js 2012-07-25 12:13:38 +0000
+++ lib/lp/bugs/javascript/tests/test_duplicates.js 2012-07-27 01:21:21 +0000
@@ -51,7 +51,7 @@
51 var widget = new Y.lp.bugs.duplicates.MarkBugDuplicate({51 var widget = new Y.lp.bugs.duplicates.MarkBugDuplicate({
52 srcNode: '#duplicate-actions',52 srcNode: '#duplicate-actions',
53 lp_bug_entry: this.lp_bug_entry,53 lp_bug_entry: this.lp_bug_entry,
54 anim_duration: 0,54 use_animation: false,
55 io_provider: this.mockio55 io_provider: this.mockio
56 });56 });
57 widget.render();57 widget.render();
@@ -97,8 +97,8 @@
97 Y.Assert.areEqual('http://foo/+duplicate', url);97 Y.Assert.areEqual('http://foo/+duplicate', url);
98 },98 },
9999
100 // The duplicate entry form renders and submits the expected data.100 // The search form renders and submits the expected data.
101 _assert_duplicate_form_submission: function(bug_id) {101 _assert_search_form_submission: function(bug_id) {
102 var form = Y.one('#duplicate-form-container');102 var form = Y.one('#duplicate-form-container');
103 Y.Assert.isTrue(103 Y.Assert.isTrue(
104 form.one('div.pretty-overlay-window')104 form.one('div.pretty-overlay-window')
@@ -108,15 +108,123 @@
108 Y.one('#duplicate-form-container div.pretty-overlay-window')108 Y.one('#duplicate-form-container div.pretty-overlay-window')
109 .hasClass('yui3-lazr-formoverlay-hidden'));109 .hasClass('yui3-lazr-formoverlay-hidden'));
110 Y.DOM.byId('field.duplicateof').value = bug_id;110 Y.DOM.byId('field.duplicateof').value = bug_id;
111 form.one('.lazr-pos').simulate('click');111 form.one('[name="field.actions.change"]').simulate('click');
112 Y.Assert.areEqual(112 var expected_url = '/api/devel/bugs/1';
113 '/api/devel/bugs/1',
114 this.mockio.last_request.url);
115 var expected_link = '{}';
116 if (bug_id !== '') {113 if (bug_id !== '') {
117 expected_link =114 expected_url = 'file:///api/devel/bugs/' + bug_id;
115 }
116 Y.Assert.areEqual(expected_url, this.mockio.last_request.url);
117 },
118
119 // The bug entry form is visible and the confirmation form is invisible
120 // or visa versa.
121 _assert_form_state: function(confirm_form_visible) {
122 Y.Assert.areEqual(
123 confirm_form_visible,
124 Y.one('#duplicate-form-container form')
125 .hasClass('hidden'));
126 var bug_info = Y.one('#duplicate-form-container ' +
127 '.confirmation-node #client-listing');
128 if (confirm_form_visible) {
129 Y.Assert.isNotNull(bug_info);
130 } else {
131 Y.Assert.isNull(bug_info);
132 }
133 },
134
135 // Invoke a successful search operation and check the form state.
136 _assert_search_form_success: function(bug_id) {
137 var expected_updated_entry = {
138 id: bug_id,
139 uri: 'api/devel/bugs/' + bug_id,
140 duplicate_of_link: 'api/devel/bugs/' + bug_id,
141 self_link: 'api/devel/bugs/' + bug_id};
142 this.mockio.last_request.successJSON(expected_updated_entry);
143 this._assert_form_state(true);
144 },
145
146 // A successful search for a bug displays the confirmation form.
147 test_initial_bug_search_success: function() {
148 this.widget = this._createWidget(false);
149 this._assert_search_form_submission(3);
150 this._assert_search_form_success(3);
151 },
152
153 // After a successful search, hitting the Search Again button takes us
154 // back to the bug details entry form.
155 test_initial_bug_search_try_again: function() {
156 this.widget = this._createWidget(false);
157 this._assert_search_form_submission(3);
158 this._assert_search_form_success(3);
159 Y.one('#duplicate-form-container .no_button')
160 .simulate('click');
161 this._assert_form_state(false);
162 },
163
164 // After a successful search, hitting the Select bug button initiates
165 // the mark as duplicate operation.
166 test_bug_search_select_bug: function() {
167 this.widget = this._createWidget(false);
168 this._assert_search_form_submission(3);
169 this._assert_search_form_success(3);
170 var update_bug_duplicate_called = false;
171 this.widget._update_bug_duplicate = function(bug_id) {
172 update_bug_duplicate_called = true;
173 Y.Assert.areEqual(3, bug_id);
174 };
175 Y.one('#duplicate-form-container .yes_button')
176 .simulate('click');
177 this._assert_form_state(true);
178 Y.Assert.isTrue(update_bug_duplicate_called);
179 },
180
181 // The specified error message is displayed.
182 _assert_error_display: function(message) {
183 var selector
184 = '#duplicate-form-container div.pretty-overlay-window';
185 Y.Assert.isFalse(
186 Y.one(selector).hasClass('yui3-lazr-formoverlay-hidden'));
187 var error_msg = Y.one('.yui3-lazr-formoverlay-errors p');
188 Y.Assert.areEqual(message, error_msg.get('text'));
189 },
190
191 // The error is displayed as expected when the initial bug search
192 // fails with a generic error.
193 test_initial_bug_search_generic_failure: function() {
194 this.widget = this._createWidget(false);
195 this._assert_search_form_submission(3);
196 var response = {
197 status: 500,
198 responseText: 'An error occurred'
199 };
200 this.mockio.respond(response);
201 this._assert_error_display('An error occurred');
202 },
203
204 // The error is displayed as expected when the initial bug search
205 // fails with a 404 (invalid/not found bug id).
206 test_initial_bug_search_invalid_bug_failure: function() {
207 this.widget = this._createWidget(false);
208 this._assert_search_form_submission(3);
209 var response = {
210 status: 404,
211 responseText: 'An error occurred'
212 };
213 this.mockio.respond(response);
214 this._assert_error_display('3 is not a valid bug number.');
215 },
216
217 // The duplicate entry form renders and submits the expected data.
218 _assert_confirmation_form_submission: function(bug_id) {
219 this._assert_search_form_submission(bug_id);
220 this._assert_search_form_success(bug_id);
221 Y.one('#duplicate-form-container .yes_button')
222 .simulate('click');
223 this._assert_form_state(true);
224 Y.Assert.areEqual(
225 '/api/devel/bugs/1', this.mockio.last_request.url);
226 var expected_link =
118 '{"duplicate_of_link":"api/devel/bugs/' + bug_id + '"}';227 '{"duplicate_of_link":"api/devel/bugs/' + bug_id + '"}';
119 }
120 Y.Assert.areEqual(228 Y.Assert.areEqual(
121 expected_link, this.mockio.last_request.config.data);229 expected_link, this.mockio.last_request.config.data);
122 },230 },
@@ -124,6 +232,7 @@
124 // Submitting a bug dupe works as expected.232 // Submitting a bug dupe works as expected.
125 test_duplicate_form_submission_success: function() {233 test_duplicate_form_submission_success: function() {
126 this.widget = this._createWidget(false);234 this.widget = this._createWidget(false);
235 this._assert_confirmation_form_submission(3);
127 var success_called = false;236 var success_called = false;
128 this.widget._update_bug_duplicate_success =237 this.widget._update_bug_duplicate_success =
129 function(updated_entry, new_dup_url, new_dup_id) {238 function(updated_entry, new_dup_url, new_dup_id) {
@@ -134,7 +243,6 @@
134 Y.Assert.areEqual(3, new_dup_id);243 Y.Assert.areEqual(3, new_dup_id);
135 success_called = true;244 success_called = true;
136 };245 };
137 this._assert_duplicate_form_submission(3);
138 var expected_updated_entry = {246 var expected_updated_entry = {
139 uri: 'api/devel/bugs/1',247 uri: 'api/devel/bugs/1',
140 duplicate_of_link: 'api/devel/bugs/3',248 duplicate_of_link: 'api/devel/bugs/3',
@@ -146,6 +254,7 @@
146 // A submission failure is handled as expected.254 // A submission failure is handled as expected.
147 test_duplicate_form_submission_failure: function() {255 test_duplicate_form_submission_failure: function() {
148 this.widget = this._createWidget(false);256 this.widget = this._createWidget(false);
257 this._assert_confirmation_form_submission(3);
149 var failure_called = false;258 var failure_called = false;
150 this.widget._update_bug_duplicate_failure =259 this.widget._update_bug_duplicate_failure =
151 function(response, old_dup_url, new_dup_id) {260 function(response, old_dup_url, new_dup_id) {
@@ -155,7 +264,6 @@
155 Y.Assert.areEqual(3, new_dup_id);264 Y.Assert.areEqual(3, new_dup_id);
156 failure_called = true;265 failure_called = true;
157 };266 };
158 this._assert_duplicate_form_submission(3);
159 this.mockio.respond({267 this.mockio.respond({
160 status: 400,268 status: 400,
161 responseText: 'There was an error',269 responseText: 'There was an error',
@@ -165,7 +273,8 @@
165273
166 // Submitting a dupe removal request works as expected.274 // Submitting a dupe removal request works as expected.
167 test_duplicate_form_submission_remove_dupe: function() {275 test_duplicate_form_submission_remove_dupe: function() {
168 this.widget = this._createWidget(true);276 this.widget = this._createWidget(false);
277 this._assert_search_form_submission('');
169 var success_called = false;278 var success_called = false;
170 this.widget._update_bug_duplicate_success =279 this.widget._update_bug_duplicate_success =
171 function(updated_entry, new_dup_url, new_dup_id) {280 function(updated_entry, new_dup_url, new_dup_id) {
@@ -174,7 +283,6 @@
174 Y.Assert.areEqual('', new_dup_id);283 Y.Assert.areEqual('', new_dup_id);
175 success_called = true;284 success_called = true;
176 };285 };
177 this._assert_duplicate_form_submission('');
178 var expected_updated_entry =286 var expected_updated_entry =
179 '{"duplicate_of_link":""}';287 '{"duplicate_of_link":""}';
180 this.mockio.success({288 this.mockio.success({
@@ -221,48 +329,6 @@
221 Y.one('#mark-duplicate-text .menu-link-mark-dupe'));329 Y.one('#mark-duplicate-text .menu-link-mark-dupe'));
222 // Test the duplicate warning message is gone.330 // Test the duplicate warning message is gone.
223 Y.Assert.isNull(Y.one('#warning-comment-on-duplicate'));331 Y.Assert.isNull(Y.one('#warning-comment-on-duplicate'));
224 },
225
226 // The remove bug duplicate error function works as expected for
227 // generic errors.
228 test_update_bug_duplicate_generic_failure: function() {
229 this.widget = this._createWidget(false);
230 var data = {
231 self_link: 'api/devel/bugs/1'};
232 var new_bug_entry = new Y.lp.client.Entry(
233 this.lp_client, data, data.self_link);
234 var response = {
235 status: 500,
236 responseText: 'An error occurred'
237 };
238 this.widget._update_bug_duplicate_failure(response, null, 3);
239 Y.Assert.isFalse(
240 Y.one('#duplicate-form-container div.pretty-overlay-window')
241 .hasClass('yui3-lazr-formoverlay-hidden'));
242 var error_msg = Y.one('.yui3-lazr-formoverlay-errors ul li');
243 Y.Assert.areEqual('An error occurred', error_msg.get('text'));
244 },
245
246 // The remove bug duplicate error function works as expected for
247 // invalid bug errors.
248 test_update_bug_duplicate_invalid_bug_failure: function() {
249 this.widget = this._createWidget(false);
250 var data = {
251 self_link: 'api/devel/bugs/1'};
252 var new_bug_entry = new Y.lp.client.Entry(
253 this.lp_client, data, data.self_link);
254 var response = {
255 status: 400,
256 responseText: 'An error occurred'
257 };
258 this.widget._update_bug_duplicate_failure(response, null, 3);
259 Y.Assert.isFalse(
260 Y.one('#duplicate-form-container div.pretty-overlay-window')
261 .hasClass('yui3-lazr-formoverlay-hidden'));
262 var error_msg = Y.one('.yui3-lazr-formoverlay-errors ul li');
263 Y.Assert.areEqual(
264 '3 is not a valid bug number or nickname.',
265 error_msg.get('text'));
266 }332 }
267 }));333 }));
268334