diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js index 23cf5cfbfc..e8717b566a 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js @@ -116,7 +116,7 @@ observer: '_toggleResolved', }, - _numPendingDiffRequests: { + _numPendingDraftRequests: { type: Object, value: {number: 0}, // Intentional to share the object across instances. }, @@ -180,12 +180,11 @@ this.disabled = true; - this._eraseDraftComment(); - this._xhrPromise = this._saveDraft(this.comment).then(response => { this.disabled = false; if (!response.ok) { return response; } + this._eraseDraftComment(); return this.$.restAPI.getResponseObject(response).then(obj => { const comment = obj; comment.__draft = true; @@ -204,6 +203,8 @@ this.disabled = false; throw err; }); + + return this._xhrPromise; }, _eraseDraftComment() { @@ -467,15 +468,23 @@ }, _showStartRequest() { - const numPending = ++this._numPendingDiffRequests.number; + const numPending = ++this._numPendingDraftRequests.number; this._updateRequestToast(numPending); }, _showEndRequest() { - const numPending = --this._numPendingDiffRequests.number; + const numPending = --this._numPendingDraftRequests.number; this._updateRequestToast(numPending); }, + _handleFailedDraftRequest() { + this._numPendingDraftRequests.number--; + + // Cancel the debouncer so that error toasts from the error-manager will + // not be overridden. + this.cancelDebouncer('draft-toast'); + }, + _updateRequestToast(numPending) { const message = this._getSavingMessage(numPending); this.debounce('draft-toast', () => { @@ -491,7 +500,11 @@ this._showStartRequest(); return this.$.restAPI.saveDiffDraft(this.changeNum, this.patchNum, draft) .then(result => { - this._showEndRequest(); + if (result.ok) { + this._showEndRequest(); + } else { + this._handleFailedDraftRequest(); + } return result; }); }, @@ -500,7 +513,11 @@ this._showStartRequest(); return this.$.restAPI.deleteDiffDraft(this.changeNum, this.patchNum, draft).then(result => { - this._showEndRequest(); + if (result.ok) { + this._showEndRequest(); + } else { + this._handleFailedDraftRequest(); + } return result; }); }, diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html index 824a1752ff..8b6ec5ed04 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html @@ -493,6 +493,27 @@ limitations under the License. element._handleConfirmDiscard({preventDefault: sinon.stub()}); }); + test('storage is cleared only after save success', () => { + const eraseStub = sandbox.stub(element, '_eraseDraftComment'); + sandbox.stub(element.$.restAPI, 'getResponseObject') + .returns(Promise.resolve({})); + + sandbox.stub(element, '_saveDraft').returns(Promise.resolve({ok: false})); + + const savePromise = element.save(); + assert.isFalse(eraseStub.called); + return savePromise.then(() => { + assert.isFalse(eraseStub.called); + + element._saveDraft.restore(); + sandbox.stub(element, '_saveDraft') + .returns(Promise.resolve({ok: true})); + return element.save().then(() => { + assert.isTrue(eraseStub.called); + }); + }); + }); + suite('confirm discard', () => { let saveDisabled; let discardStub; @@ -659,22 +680,22 @@ limitations under the License. test('_show{Start,End}Request', () => { const updateStub = sandbox.stub(element, '_updateRequestToast'); - element._numPendingDiffRequests.number = 1; + element._numPendingDraftRequests.number = 1; element._showStartRequest(); assert.isTrue(updateStub.calledOnce); assert.equal(updateStub.lastCall.args[0], 2); - assert.equal(element._numPendingDiffRequests.number, 2); + assert.equal(element._numPendingDraftRequests.number, 2); element._showEndRequest(); assert.isTrue(updateStub.calledTwice); assert.equal(updateStub.lastCall.args[0], 1); - assert.equal(element._numPendingDiffRequests.number, 1); + assert.equal(element._numPendingDraftRequests.number, 1); element._showEndRequest(); assert.isTrue(updateStub.calledThrice); assert.equal(updateStub.lastCall.args[0], 0); - assert.equal(element._numPendingDiffRequests.number, 0); + assert.equal(element._numPendingDraftRequests.number, 0); }); }); });