From 2d9610cd2bcff00089caee8f61200bad772854ac Mon Sep 17 00:00:00 2001 From: Kasper Nilsson Date: Thu, 23 Aug 2018 13:18:11 -0700 Subject: [PATCH] Utilize gr-error-dialog Use an error dialog for change action failures and server errors. Bug: Issue 8457 Change-Id: I021065ebfd286ed9a8ae8402799a6e4173065a28 --- .../gr-change-actions/gr-change-actions.js | 8 ++++++- .../gr-change-actions_test.html | 10 ++++++--- .../gr-error-manager/gr-error-manager.html | 9 ++++++++ .../core/gr-error-manager/gr-error-manager.js | 17 +++++++++++++- .../gr-error-manager_test.html | 22 ++++++++++++++++--- 5 files changed, 58 insertions(+), 8 deletions(-) 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 b12b9062e0..98f970755f 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 @@ -212,6 +212,12 @@ * @event show-alert */ + /** + * Fires when a change action fails. + * + * @event show-error + */ + properties: { /** * @type {{ @@ -1154,7 +1160,7 @@ _handleResponseError(response) { return response.text().then(errText => { - this.fire('show-alert', + this.fire('show-error', {message: `Could not perform action: ${errText}`}); if (!errText.startsWith('Change is already up to date')) { throw Error(errText); 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 cfd2c454ba..91b39deede 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 @@ -1370,6 +1370,7 @@ limitations under the License. suite('_send', () => { let cleanup; let payload; + let onShowError; let onShowAlert; setup(() => { @@ -1378,6 +1379,8 @@ limitations under the License. element.latestPatchNum = 12; payload = {foo: 'bar'}; + onShowError = sinon.stub(); + element.addEventListener('show-error', onShowError); onShowAlert = sinon.stub(); element.addEventListener('show-alert', onShowAlert); }); @@ -1395,7 +1398,7 @@ limitations under the License. test('change action', () => { return element._send('DELETE', payload, '/endpoint', false, cleanup) .then(() => { - assert.isFalse(onShowAlert.called); + assert.isFalse(onShowError.called); assert.isTrue(cleanup.calledOnce); assert.isTrue(sendStub.calledWith(42, 'DELETE', '/endpoint', null, payload)); @@ -1405,7 +1408,7 @@ limitations under the License. test('revision action', () => { return element._send('DELETE', payload, '/endpoint', true, cleanup) .then(() => { - assert.isFalse(onShowAlert.called); + assert.isFalse(onShowError.called); assert.isTrue(cleanup.calledOnce); assert.isTrue(sendStub.calledWith(42, 'DELETE', '/endpoint', 12, payload)); @@ -1423,6 +1426,7 @@ limitations under the License. return element._send('DELETE', payload, '/endpoint', true, cleanup) .then(() => { assert.isTrue(onShowAlert.calledOnce); + assert.isFalse(onShowError.called); assert.isTrue(cleanup.calledOnce); assert.isFalse(sendStub.called); }); @@ -1441,7 +1445,7 @@ limitations under the License. return element._send('DELETE', payload, '/endpoint', true, cleanup) .then(() => { - assert.isFalse(onShowAlert.called); + assert.isFalse(onShowError.called); assert.isTrue(cleanup.called); assert.isTrue(sendStub.calledOnce); assert.isTrue(handleErrorStub.called); diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.html b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.html index 95c540341e..3ec4bb5818 100644 --- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.html +++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.html @@ -17,11 +17,20 @@ limitations under the License. + + diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js index 758148e606..e3596e7e53 100644 --- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js +++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js @@ -62,6 +62,7 @@ this.listen(document, 'network-error', '_handleNetworkError'); this.listen(document, 'auth-error', '_handleAuthError'); this.listen(document, 'show-alert', '_handleShowAlert'); + this.listen(document, 'show-error', '_handleShowErrorDialog'); this.listen(document, 'visibilitychange', '_handleVisibilityChange'); this.listen(document, 'show-auth-required', '_handleAuthRequired'); }, @@ -73,6 +74,7 @@ this.unlisten(document, 'auth-error', '_handleAuthError'); this.unlisten(document, 'show-auth-required', '_handleAuthRequired'); this.unlisten(document, 'visibilitychange', '_handleVisibilityChange'); + this.unlisten(document, 'show-error', '_handleShowErrorDialog'); }, _shouldSuppressError(msg) { @@ -101,7 +103,7 @@ // This indicates the auth token is no longer valid. this._handleAuthError(); } else if (!this._shouldSuppressError(text)) { - this._showAlert('Server error: ' + text); + this._showErrorDialog('Server error: ' + text); } console.error(text); }); @@ -257,5 +259,18 @@ _handleWindowFocus() { this.flushDebouncer('checkLoggedIn'); }, + + _handleShowErrorDialog(e) { + this._showErrorDialog(e.detail.message); + }, + + _handleDismissErrorDialog() { + this.$.errorOverlay.close(); + }, + + _showErrorDialog(message) { + this.$.errorDialog.text = message; + this.$.errorOverlay.open(); + }, }); })(); diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html index c4ba8d280b..e28b979a60 100644 --- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html +++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html @@ -87,7 +87,7 @@ limitations under the License. }); test('show normal server error', done => { - const showAlertStub = sandbox.stub(element, '_showAlert'); + const showErrorStub = sandbox.stub(element, '_showErrorDialog'); const textSpy = sandbox.spy(() => { return Promise.resolve('ZOMG'); }); element.fire('server-error', {response: {status: 500, text: textSpy}}); @@ -96,8 +96,8 @@ limitations under the License. element.$.restAPI.getLoggedIn.lastCall.returnValue, textSpy.lastCall.returnValue, ]).then(() => { - assert.isTrue(showAlertStub.calledOnce); - assert.isTrue(showAlertStub.lastCall.calledWithExactly( + assert.isTrue(showErrorStub.calledOnce); + assert.isTrue(showErrorStub.lastCall.calledWithExactly( 'Server error: ZOMG')); done(); }); @@ -279,5 +279,21 @@ limitations under the License. element._showAlert(); assert.isTrue(hideStub.calledOnce); }); + + test('show-error', () => { + const openStub = sandbox.stub(element.$.errorOverlay, 'open'); + const closeStub = sandbox.stub(element.$.errorOverlay, 'close'); + const message = 'test message'; + element.fire('show-error', {message}); + flushAsynchronousOperations(); + + assert.isTrue(openStub.called); + assert.equal(element.$.errorDialog.text, message); + + element.$.errorDialog.fire('dismiss'); + flushAsynchronousOperations(); + + assert.isTrue(closeStub.called); + }); });