Merge "Do cleanup when change action requests fail"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 3bccdd8..ff77142 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -892,6 +892,11 @@
*/
_send(method, payload, actionEndpoint, revisionAction, cleanupFn,
opt_errorFn) {
+ const handleError = response => {
+ cleanupFn.call(this);
+ this._handleResponseError(response);
+ };
+
return this.fetchIsLatestKnown(this.change, this.$.restAPI)
.then(isLatest => {
if (!isLatest) {
@@ -904,13 +909,17 @@
Gerrit.Nav.navigateToChange(this.change);
},
});
+
+ // Because this is not a network error, call the cleanup function
+ // but not the error handler.
cleanupFn();
+
return Promise.resolve();
}
const patchNum = revisionAction ? this.patchNum : null;
return this.$.restAPI.getChangeURLAndSend(this.changeNum, method,
- patchNum, actionEndpoint, payload, this._handleResponseError,
- this).then(response => {
+ patchNum, actionEndpoint, payload, handleError, this)
+ .then(response => {
cleanupFn.call(this);
return response;
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 55cd982..11a2569 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -1084,5 +1084,88 @@
});
});
});
+
+ suite('_send', () => {
+ let cleanup;
+ let payload;
+ let onShowAlert;
+
+ setup(() => {
+ cleanup = sinon.stub();
+ element.changeNum = 42;
+ element.patchNum = 12;
+ payload = {foo: 'bar'};
+
+ onShowAlert = sinon.stub();
+ element.addEventListener('show-alert', onShowAlert);
+ });
+
+ suite('happy path', () => {
+ let sendStub;
+
+ setup(() => {
+ sandbox.stub(element, 'fetchIsLatestKnown')
+ .returns(Promise.resolve(true));
+ sendStub = sandbox.stub(element.$.restAPI, 'getChangeURLAndSend')
+ .returns(Promise.resolve({}));
+ });
+
+ test('change action', () => {
+ return element._send('DELETE', payload, '/endpoint', false, cleanup)
+ .then(() => {
+ assert.isFalse(onShowAlert.called);
+ assert.isTrue(cleanup.calledOnce);
+ assert.isTrue(sendStub.calledWith(42, 'DELETE', null,
+ '/endpoint', payload));
+ });
+ });
+
+ test('revision action', () => {
+ return element._send('DELETE', payload, '/endpoint', true, cleanup)
+ .then(() => {
+ assert.isFalse(onShowAlert.called);
+ assert.isTrue(cleanup.calledOnce);
+ assert.isTrue(sendStub.calledWith(42, 'DELETE', 12, '/endpoint',
+ payload));
+ });
+ });
+ });
+
+ suite('failure modes', () => {
+ test('non-latest', () => {
+ sandbox.stub(element, 'fetchIsLatestKnown')
+ .returns(Promise.resolve(false));
+ const sendStub = sandbox.stub(element.$.restAPI,
+ 'getChangeURLAndSend');
+
+ return element._send('DELETE', payload, '/endpoint', true, cleanup)
+ .then(() => {
+ assert.isTrue(onShowAlert.calledOnce);
+ assert.isTrue(cleanup.calledOnce);
+ assert.isFalse(sendStub.called);
+ });
+ });
+
+ test('send fails', () => {
+ sandbox.stub(element, 'fetchIsLatestKnown')
+ .returns(Promise.resolve(true));
+ const sendStub = sandbox.stub(element.$.restAPI,
+ 'getChangeURLAndSend',
+ (num, method, patchNum, endpoint, payload, onErr) => {
+ onErr();
+ return Promise.resolve(null);
+ });
+ const handleErrorStub = sandbox.stub(element, '_handleResponseError');
+
+ return element._send('DELETE', payload, '/endpoint', true, cleanup)
+ .then(() => {
+ assert.isFalse(onShowAlert.called);
+ assert.isTrue(cleanup.called);
+ assert.isTrue(sendStub.calledOnce);
+ assert.isTrue(handleErrorStub.called);
+ });
+ });
+ });
+ });
});
</script>
