diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js index 1420949967..0dd9e39177 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js @@ -42,6 +42,12 @@ * @event page-error */ + /** + * Fired if being logged in is required. + * + * @event show-auth-required + */ + properties: { /** * URL params passed from the router. @@ -718,11 +724,18 @@ _handleAKey: function(e) { if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e) || - !this._loggedIn) { return; } + this.modifierPressed(e)) { + return; + } + this._getLoggedIn().then(function(isLoggedIn) { + if (!isLoggedIn) { + this.fire('show-auth-required'); + return; + } - e.preventDefault(); - this._openReplyDialog(); + e.preventDefault(); + this._openReplyDialog(); + }.bind(this)); }, _handleDKey: function(e) { diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html index 907417cd5d..fc20ef85af 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html @@ -70,19 +70,36 @@ limitations under the License. assert(showStub.lastCall.calledWithExactly('/dashboard/self')); }); - test('A should toggle overlay', function() { + test('A fires an error event when not logged in', function(done) { + sandbox.stub(element, '_getLoggedIn').returns(Promise.resolve(false)); + var loggedInErrorSpy = sandbox.spy(); + element.addEventListener('show-auth-required', loggedInErrorSpy); MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); - var overlayEl = element.$.replyOverlay; - assert.isFalse(overlayEl.opened); - element._loggedIn = true; + flush(function() { + assert.isFalse(element.$.replyOverlay.opened); + assert.isTrue(loggedInErrorSpy.called); + done(); + }); + }); + test('shift A does not open reply overlay', function(done) { + sandbox.stub(element, '_getLoggedIn').returns(Promise.resolve(true)); MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift', 'a'); - assert.isFalse(overlayEl.opened); + flush(function() { + assert.isFalse(element.$.replyOverlay.opened); + done(); + }); + }); + test('A toggles overlay when logged in', function(done) { + sandbox.stub(element, '_getLoggedIn').returns(Promise.resolve(true)); MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); - assert.isTrue(overlayEl.opened); - overlayEl.close(); - assert.isFalse(overlayEl.opened); + flush(function() { + assert.isTrue(element.$.replyOverlay.opened); + element.$.replyOverlay.close(); + assert.isFalse(element.$.replyOverlay.opened); + done(); + }); }); test('X should expand all messages', function() { 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 80f293d7b9..2d7d2a9476 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 @@ -24,4 +24,3 @@ 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 c5d18d1c1b..bd890b8b27 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 @@ -52,12 +52,14 @@ this.listen(document, 'network-error', '_handleNetworkError'); this.listen(document, 'show-alert', '_handleShowAlert'); this.listen(document, 'visibilitychange', '_handleVisibilityChange'); + this.listen(document, 'show-auth-required', '_handleAuthRequired'); }, detached: function() { this._clearHideAlertHandle(); this.unlisten(document, 'server-error', '_handleServerError'); this.unlisten(document, 'network-error', '_handleNetworkError'); + this.unlisten(document, 'show-auth-required', '_handleAuthRequired'); this.unlisten(document, 'visibilitychange', '_handleVisibilityChange'); }, @@ -65,13 +67,18 @@ return msg.indexOf(TOO_MANY_FILES) > -1; }, + _handleAuthRequired: function() { + this._showAuthErrorAlert( + 'Log in is required to perform that action.', 'Log in.'); + }, + _handleServerError: function(e) { if (e.detail.response.status === 403) { this._getLoggedIn().then(function(loggedIn) { if (loggedIn) { // The app was logged at one point and is now getting auth errors. // This indicates the auth token is no longer valid. - this._showAuthErrorAlert(); + this._showAuthErrorAlert('Auth error', 'Refresh credentials.'); } }.bind(this)); } else { @@ -121,12 +128,12 @@ } }, - _showAuthErrorAlert: function() { + _showAuthErrorAlert: function(errorText, actionText) { // TODO(viktard): close alert if it's not for auth error. if (this._alertElement) { return; } this._alertElement = this._createToastAlert(); - this._alertElement.show('Auth error', 'Refresh credentials.'); + this._alertElement.show(errorText, actionText); this.listen(this._alertElement, 'action', '_createLoginPopup'); this._refreshingCredentials = true; 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 781220a7bb..da15c08109 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 @@ -56,6 +56,13 @@ limitations under the License. }); }); + test('show logged in error', function() { + sandbox.stub(element, '_showAuthErrorAlert'); + element.fire('show-auth-required'); + assert.isTrue(element._showAuthErrorAlert.calledWithExactly( + 'Log in is required to perform that action.', 'Log in.')); + }); + test('show normal server error', function(done) { var showAlertStub = sandbox.stub(element, '_showAlert'); var textSpy = sandbox.spy(function() { return Promise.resolve('ZOMG'); }); diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js index 05a7f72b45..52e869fd17 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js @@ -32,6 +32,12 @@ * @event line-selected */ + /** + * Fired if being logged in is required. + * + * @event show-auth-required + */ + properties: { changeNum: String, noAutoRender: { @@ -146,7 +152,10 @@ addDraftAtLine: function(el) { this._selectLine(el); this._getLoggedIn().then(function(loggedIn) { - if (!loggedIn) { return; } + if (!loggedIn) { + this.fire('show-auth-required'); + return; + } var value = el.getAttribute('data-value'); if (value === GrDiffLine.FILE) { diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html index 9288e92599..60cf798af3 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html @@ -60,6 +60,17 @@ limitations under the License. assert.isFalse(element.classList.contains('no-left')); }); + test('addDraftAtLine', function(done) { + sandbox.stub(element, '_selectLine'); + var loggedInErrorSpy = sandbox.spy(); + element.addEventListener('show-auth-required', loggedInErrorSpy); + element.addDraftAtLine(); + flush(function() { + assert.isTrue(loggedInErrorSpy.called); + done(); + }); + }); + test('view does not start with displayLine classList', function() { assert.isFalse( element.$$('.diffContainer').classList.contains('displayLine')); @@ -578,6 +589,20 @@ limitations under the License. }); }); + test('addDraftAtLine', function(done) { + var fakeLineEl = {getAttribute: sandbox.stub().returns(42)}; + sandbox.stub(element, '_selectLine'); + sandbox.stub(element, '_addDraft'); + var loggedInErrorSpy = sandbox.spy(); + element.addEventListener('show-auth-required', loggedInErrorSpy); + element.addDraftAtLine(fakeLineEl); + flush(function() { + assert.isFalse(loggedInErrorSpy.called); + assert.isTrue(element._addDraft.calledWithExactly(fakeLineEl, 42)); + done(); + }); + }); + suite('handle comment-update', function() { setup(function() {