diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js index fa57770794..d4b0cb2aa5 100644 --- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js +++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js @@ -148,10 +148,13 @@ }); this.disabled = true; - return this._saveReview(obj).then(function(response) { - this.disabled = false; - if (!response.ok) { return response; } + var errFn = this._handle400Error.bind(this); + return this._saveReview(obj, errFn).then(function(response) { + if (!response.ok) { + return response; + } + this.disabled = false; this.draft = ''; this.fire('send', null, {bubbles: false}); }.bind(this)).catch(function(err) { @@ -170,6 +173,48 @@ } }, + _handle400Error: function(response) { + // A call to _saveReview could fail with a server error if erroneous + // reviewers were requested. This is signalled with a 400 Bad Request + // status. The default gr-rest-api-interface error handling would + // result in a large JSON response body being displayed to the user in + // the gr-error-manager toast. + // + // We can modify the error handling behavior by passing this function + // through to restAPI as a custom error handling function. Since we're + // short-circuiting restAPI we can do our own response parsing and fire + // the server-error ourselves. + // + this.disabled = false; + + if (response.status !== 400) { + // This is all restAPI does when there is no custom error handling. + this.fire('server-error', {response: response}); + return; + } + + // Process the response body, format a better error message, and fire + // an event for gr-event-manager to display. + var jsonPromise = this.$.restAPI.getResponseObject(response); + return jsonPromise.then(function(result) { + var errors = []; + ['reviewers', 'ccs'].forEach(function(state) { + for (var input in result[state]) { + var reviewer = result[state][input]; + if (!!reviewer.error) { + errors.push(reviewer.error); + } + } + }); + response = { + ok: false, + status: response.status, + text: function() { return Promise.resolve(errors.join(', ')); }, + }; + this.fire('server-error', {response: response}); + }.bind(this)); + }, + _computeShowLabels: function(patchNum, revisions) { var num = parseInt(patchNum, 10); for (var rev in revisions) { @@ -274,9 +319,9 @@ this.send(); }, - _saveReview: function(review) { + _saveReview: function(review, opt_errFn) { return this.$.restAPI.saveChangeReview(this.change._number, this.patchNum, - review); + review, opt_errFn); }, _reviewerPendingConfirmationUpdated: function(reviewer) { diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html index e65cc0453c..7dc601a3d1 100644 --- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html +++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html @@ -290,5 +290,29 @@ limitations under the License. assert.isTrue(eraseDraftCommentStub.calledWith(location)); }); + + test('400 converts to human-readable server-error', function(done) { + sandbox.stub(window, 'fetch', function() { + var text = '....{"reviewers":{"id1":{"error":"first error"}},' + + '"ccs":{"id2":{"error":"second error"}}}'; + return Promise.resolve({ + ok: false, + status: 400, + text: function() { return Promise.resolve(text); }, + }); + }); + + element.addEventListener('server-error', function(event) { + if (event.target !== element) { + return; + } + event.detail.response.text().then(function(body) { + assert.equal(body, 'first error, second error'); + done(); + }); + }); + + element.send(); + }); });