Merge changes from topic 'notedb-batch-update-prep' * changes: Extract more helper methods into BatchUpdate ReviewDbBatchUpdate: Simplify some loop code BatchUpdate: Remove protected from static methods Expand javadoc for BatchUpdate contexts ChangeContext: Remove arg from bumpLastUpdatedOn
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt index fb884d3..60a56dd 100644 --- a/Documentation/dev-bazel.txt +++ b/Documentation/dev-bazel.txt
@@ -354,14 +354,9 @@ [[consume-jgit-from-development-tree]] -To consume the JGit dependency from the development tree, uncomment -`local_repository` rule for `jgit` in WORKSPACE file and uncomment -the lines in the `lib/jgit/**/BUILD` files for `jgit` rule aliases. -`jgit-dev=1` must be passed in to bazel: - ----- - $ bazel test --define jgit-dev=1 ... ----- +To consume the JGit dependency from the development tree, edit +`lib/jgit/jgit.bzl` setting LOCAL_JGIT_REPO to a directory holding a +JGit repository. [[clean-cache]] === Cleaning The download cache
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 6ea7ac5..7bea2b7 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt
@@ -5300,6 +5300,10 @@ Author of the message as an link:rest-api-accounts.html#account-info[AccountInfo] entity. + Unset if written by the Gerrit system. +|`real_author` |optional| +Real author of the message as an +link:rest-api-accounts.html#account-info[AccountInfo] entity. + +Set if the message was posted on behalf of another user. |`date` || The link:rest-api.html#timestamp[timestamp] this message was posted. |`message` ||The text left by the user.
diff --git a/WORKSPACE b/WORKSPACE index 0d725e8..e979466 100644 --- a/WORKSPACE +++ b/WORKSPACE
@@ -130,34 +130,9 @@ sha1 = "cdb2dcb4e22b83d6b32b93095f644c3462739e82", ) -load("//lib/jgit:jgit.bzl", "JGIT_VERS", "JGIT_REPO", "JGIT_SHA1", "JGIT_SRC_SHA1", "JGIT_SERVLET_SHA1", "JGIT_ARCHIVE_SHA1", "JGIT_JUNIT_SHA1") +load("//lib/jgit:jgit.bzl", "jgit_repos") -# -# Uncomment jgit repository to route JGit dependency to development tree. -# Pass jgit-dev=1 in to bazel: `bazel test --define jgit-dev=1 ...` -# Due to: https://github.com/bazelbuild/bazel/issues/2707 -# aliases to jgit should be uncommented in lib/jgit/**/BUILD files. -#local_repository( -# name = "jgit", -# path = "/home/<user>/projects/jgit", -#) - -maven_jar( - name = "jgit_lib", - artifact = "org.eclipse.jgit:org.eclipse.jgit:" + JGIT_VERS, - repository = JGIT_REPO, - sha1 = JGIT_SHA1, - src_sha1 = JGIT_SRC_SHA1, - unsign = True, -) - -maven_jar( - name = "jgit_servlet", - artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + JGIT_VERS, - repository = JGIT_REPO, - sha1 = JGIT_SERVLET_SHA1, - unsign = True, -) +jgit_repos() maven_jar( name = "javaewah", @@ -167,21 +142,6 @@ ) maven_jar( - name = "jgit_archive", - artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + JGIT_VERS, - repository = JGIT_REPO, - sha1 = JGIT_ARCHIVE_SHA1, -) - -maven_jar( - name = "jgit_junit", - artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + JGIT_VERS, - repository = JGIT_REPO, - sha1 = JGIT_JUNIT_SHA1, - unsign = True, -) - -maven_jar( name = "gwtjsonrpc", artifact = "com.google.gerrit:gwtjsonrpc:1.11", sha1 = "0990e7eec9eec3a15661edcf9232acbac4aeacec", @@ -297,8 +257,8 @@ maven_jar( name = "commons_codec", - artifact = "commons-codec:commons-codec:1.4", - sha1 = "4216af16d38465bbab0f3dff8efa14204f7a399a", + artifact = "commons-codec:commons-codec:1.10", + sha1 = "4b95f4897fa13f2cd904aee711aeafc0c5295cd8", ) maven_jar( @@ -713,6 +673,13 @@ sha1 = "42a25dc3219429f0e5d060061f71acb49bf010a0", ) +# Only needed when jgit is built from the development tree +maven_jar( + name = "hamcrest_library", + artifact = "org.hamcrest:hamcrest-library:1.3", + sha1 = "4785a3c21320980282f9f33d0d1264a69040538f", +) + TRUTH_VERS = "0.32" maven_jar(
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java index c69391c..7d9663a 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
@@ -39,8 +39,10 @@ import com.google.gerrit.extensions.api.changes.RevisionApi; import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.api.groups.GroupInput; +import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.client.Side; import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeMessageInfo; import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.common.GroupInfo; @@ -62,6 +64,7 @@ import com.google.gerrit.server.project.Util; import com.google.gerrit.server.query.change.ChangeData; import com.google.inject.Inject; +import java.util.EnumSet; import org.apache.http.Header; import org.apache.http.message.BasicHeader; import org.junit.After; @@ -529,6 +532,28 @@ assertThat(m.getRealAuthor()).isEqualTo(admin.id); // not user2 } + @Test + public void changeMessageCreatedOnBehalfOfHasRealUser() throws Exception { + allowCodeReviewOnBehalfOf(); + + PushOneCommit.Result r = createChange(); + ReviewInput in = new ReviewInput(); + in.onBehalfOf = user.id.toString(); + in.message = "Message on behalf of"; + in.label("Code-Review", 1); + + setApiUser(accounts.user2()); + gApi.changes().id(r.getChangeId()).revision(r.getPatchSetId().getId()).review(in); + + ChangeInfo info = + gApi.changes().id(r.getChangeId()).get(EnumSet.of(ListChangesOption.MESSAGES)); + assertThat(info.messages).hasSize(2); + + ChangeMessageInfo changeMessageInfo = Iterables.getLast(info.messages); + assertThat(changeMessageInfo.realAuthor).isNotNull(); + assertThat(changeMessageInfo.realAuthor._accountId).isEqualTo(accounts.user2().id.get()); + } + private void allowCodeReviewOnBehalfOf() throws Exception { ProjectConfig cfg = projectCache.checkedGet(project).getConfig(); LabelType codeReviewType = Util.codeReview();
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java index e79918f..735b84f 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java
@@ -20,6 +20,7 @@ public String id; public String tag; public AccountInfo author; + public AccountInfo realAuthor; public Timestamp date; public String message; public Integer _revisionNumber;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 219daf3..64aadde 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -1030,6 +1030,10 @@ cmi.message = message.getMessage(); cmi.tag = message.getTag(); cmi._revisionNumber = patchNum != null ? patchNum.get() : null; + Account.Id realAuthor = message.getRealAuthor(); + if (realAuthor != null) { + cmi.realAuthor = accountLoader.get(realAuthor); + } result.add(cmi); } }
diff --git a/lib/jgit/BUILD b/lib/jgit/BUILD index f0d25e6..e69de29 100644 --- a/lib/jgit/BUILD +++ b/lib/jgit/BUILD
@@ -1,7 +0,0 @@ -config_setting( - name = "dev", - values = { - "define": "jgit-dev=1", - }, - visibility = ["//visibility:public"], -)
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl index ac843d8..5378600 100644 --- a/lib/jgit/jgit.bzl +++ b/lib/jgit/jgit.bzl
@@ -1,19 +1,66 @@ -load("//tools/bzl:maven_jar.bzl", "GERRIT", "MAVEN_LOCAL", "MAVEN_CENTRAL") +load("//tools/bzl:maven_jar.bzl", "GERRIT", "MAVEN_LOCAL", "MAVEN_CENTRAL", "maven_jar") -JGIT_VERS = "4.6.0.201612231935-r.30-gd3148f300" +_JGIT_VERS = "4.6.0.201612231935-r.30-gd3148f300" -DOC_VERS = "4.6.0.201612231935-r" # Set to JGIT_VERS unless using a snapshot +_DOC_VERS = "4.6.0.201612231935-r" # Set to _JGIT_VERS unless using a snapshot -JGIT_DOC_URL = "http://download.eclipse.org/jgit/site/" + DOC_VERS + "/apidocs" +JGIT_DOC_URL = "http://download.eclipse.org/jgit/site/" + _DOC_VERS + "/apidocs" -JGIT_REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL. +_JGIT_REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL. -JGIT_SHA1 = "a2b5970b853f8fee64589fc1103c0ceb7677ba63" +# set this to use a local version. +# "/home/<user>/projects/jgit" +LOCAL_JGIT_REPO = "" -JGIT_SRC_SHA1 = "765f955774c36c226aa41fba7c20119451de2db7" +def jgit_repos(): + if LOCAL_JGIT_REPO: + native.local_repository( + name = "jgit", + path = LOCAL_JGIT_REPO, + ) + else: + jgit_maven_repos() -JGIT_SERVLET_SHA1 = "d3aa54bd610db9a5c246aa8fef13989982c98628" +def jgit_maven_repos(): + maven_jar( + name = "jgit_lib", + artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS, + repository = _JGIT_REPO, + sha1 = "a2b5970b853f8fee64589fc1103c0ceb7677ba63", + src_sha1 = "765f955774c36c226aa41fba7c20119451de2db7", + unsign = True, + ) + maven_jar( + name = "jgit_servlet", + artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS, + repository = _JGIT_REPO, + sha1 = "d3aa54bd610db9a5c246aa8fef13989982c98628", + unsign = True, + ) + maven_jar( + name = "jgit_archive", + artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS, + repository = _JGIT_REPO, + sha1 = "a728cf277396f1227c5a8dffcf5dee0188fc0821", + ) + maven_jar( + name = "jgit_junit", + artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS, + repository = _JGIT_REPO, + sha1 = "6c2b2f192c95d25a2e1576aee5d1169dd8bd2266", + unsign = True, + ) -JGIT_ARCHIVE_SHA1 = "a728cf277396f1227c5a8dffcf5dee0188fc0821" +def jgit_dep(name): + mapping = { + "@jgit_junit//jar": "@jgit//org.eclipse.jgit.junit:junit", + "@jgit_lib//jar:src": "@jgit//org.eclipse.jgit:libjgit-src.jar", + "@jgit_lib//jar": "@jgit//org.eclipse.jgit:jgit", + "@jgit_servlet//jar":"@jgit//org.eclipse.jgit.http.server:jgit-servlet", + "@jgit_archive//jar": "@jgit//org.eclipse.jgit.archive:jgit-archive", + } -JGIT_JUNIT_SHA1 = "6c2b2f192c95d25a2e1576aee5d1169dd8bd2266" + if LOCAL_JGIT_REPO: + return mapping[name] + else: + return name
diff --git a/lib/jgit/org.eclipse.jgit.archive/BUILD b/lib/jgit/org.eclipse.jgit.archive/BUILD index 9a5633c..198ff25 100644 --- a/lib/jgit/org.eclipse.jgit.archive/BUILD +++ b/lib/jgit/org.eclipse.jgit.archive/BUILD
@@ -1,10 +1,9 @@ +load("//lib/jgit:jgit.bzl", "jgit_dep") + java_library( name = "jgit-archive", data = ["//lib:LICENSE-jgit"], visibility = ["//visibility:public"], - exports = select({ - # "//lib/jgit:dev": ["@jgit//org.eclipse.jgit.archive:jgit-archive"], - "//conditions:default": ["@jgit_archive//jar"], - }), + exports = [jgit_dep("@jgit_archive//jar")], runtime_deps = ["//lib/jgit/org.eclipse.jgit:jgit"], )
diff --git a/lib/jgit/org.eclipse.jgit.http.server/BUILD b/lib/jgit/org.eclipse.jgit.http.server/BUILD index 24a58ab..6b5bf78 100644 --- a/lib/jgit/org.eclipse.jgit.http.server/BUILD +++ b/lib/jgit/org.eclipse.jgit.http.server/BUILD
@@ -1,10 +1,9 @@ +load("//lib/jgit:jgit.bzl", "jgit_dep") + java_library( name = "jgit-servlet", data = ["//lib:LICENSE-jgit"], visibility = ["//visibility:public"], - exports = select({ - # "//lib/jgit:dev": ["@jgit//org.eclipse.jgit.http.server:jgit-servlet"], - "//conditions:default": ["@jgit_servlet//jar"], - }), + exports = [jgit_dep("@jgit_servlet//jar")], runtime_deps = ["//lib/jgit/org.eclipse.jgit:jgit"], )
diff --git a/lib/jgit/org.eclipse.jgit.junit/BUILD b/lib/jgit/org.eclipse.jgit.junit/BUILD index ebfefec..ba6c42f 100644 --- a/lib/jgit/org.eclipse.jgit.junit/BUILD +++ b/lib/jgit/org.eclipse.jgit.junit/BUILD
@@ -1,11 +1,10 @@ +load("//lib/jgit:jgit.bzl", "jgit_dep") + java_library( name = "junit", testonly = 1, data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"], visibility = ["//visibility:public"], - exports = select({ - # "//lib/jgit:dev": ["@jgit//org.eclipse.jgit.junit:junit"], - "//conditions:default": ["@jgit_junit//jar"], - }), + exports = [jgit_dep("@jgit_junit//jar")], runtime_deps = ["//lib/jgit/org.eclipse.jgit:jgit"], )
diff --git a/lib/jgit/org.eclipse.jgit/BUILD b/lib/jgit/org.eclipse.jgit/BUILD index 38bfd4b..5586cb1 100644 --- a/lib/jgit/org.eclipse.jgit/BUILD +++ b/lib/jgit/org.eclipse.jgit/BUILD
@@ -1,20 +1,16 @@ +load("//lib/jgit:jgit.bzl", "jgit_dep") + java_library( name = "jgit", data = ["//lib:LICENSE-jgit"], visibility = ["//visibility:public"], - exports = select({ - # "//lib/jgit:dev": ["@jgit//org.eclipse.jgit:jgit"], - "//conditions:default": ["@jgit_lib//jar"], - }), + exports = [jgit_dep("@jgit_lib//jar")], runtime_deps = [":javaewah"], ) alias( name = "jgit-source", - actual = select({ - # "//lib/jgit:dev": "@jgit//org.eclipse.jgit:libjgit-src.jar", - "//conditions:default": "@jgit_lib//jar:src", - }), + actual = jgit_dep("@jgit_lib//jar:src"), visibility = ["//visibility:public"], )
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 1420949..0dd9e39 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 907417c..fc20ef8 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 @@ 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/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js index 3fc9440..a0189f2 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -118,7 +118,7 @@ observers: [ '_expandedPathsChanged(_expandedFilePaths.splices)', - '_setReviewedFiles(_shownFiles, _files, _reviewed.*)', + '_setReviewedFiles(_shownFiles, _files, _reviewed.*, _loggedIn)', ], keyBindings: { @@ -658,11 +658,12 @@ return files.base.slice(0, numFilesShown); }, - _setReviewedFiles: function(shownFiles, files, reviewedRecord) { + _setReviewedFiles: function(shownFiles, files, reviewedRecord, loggedIn) { + if (!loggedIn) { return; } var reviewed = reviewedRecord.base; var fileReviewed; for (var i = 0; i < files.length; i++) { - fileReviewed = this._computeReviewed(shownFiles[i], reviewed); + fileReviewed = this._computeReviewed(files[i], reviewed); this._files[i].isReviewed = fileReviewed; if (i < shownFiles.length) { this.set(['_shownFiles', i, 'isReviewed'], fileReviewed);
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html index 18796f6..66ad66c 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -535,6 +535,7 @@ {__path: 'myfile.txt'}, ]; element._reviewed = ['/COMMIT_MSG', 'myfile.txt']; + element._loggedIn = true; element.changeNum = '42'; element.patchRange = { basePatchNum: 'PARENT', @@ -704,7 +705,6 @@ test('show/hide diffs disabled for large amounts of files', function(done) { var computeSpy = sandbox.spy(element, '_fileListActionsVisible'); - sandbox.stub(element, '_setReviewedFiles'); element._files = []; element.changeNum = '42'; element.patchRange = { @@ -868,6 +868,7 @@ done(); }); }); + test('_renderInOrder logged in', function(done) { element._isLoggedIn = true; var reviewStub = sandbox.stub(element, '_reviewFile');
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 80f293d..2d7d2a9 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 @@ </template> <script src="gr-error-manager.js"></script> </dom-module> -
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 8209cde..bd890b8 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
@@ -15,8 +15,8 @@ 'use strict'; var HIDE_ALERT_TIMEOUT_MS = 5000; - var CHECK_SIGN_IN_INTERVAL_MS = 60*1000; - var STALE_CREDENTIAL_THRESHOLD_MS = 10*60*1000; + var CHECK_SIGN_IN_INTERVAL_MS = 60 * 1000; + var STALE_CREDENTIAL_THRESHOLD_MS = 10 * 60 * 1000; var SIGN_IN_WIDTH_PX = 690; var SIGN_IN_HEIGHT_PX = 500; var TOO_MANY_FILES = 'too many files to find conflicts'; @@ -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 781220a..da15c08 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 @@ }); }); + 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 05a7f72..52e869f 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 9288e92..60cf798 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 @@ 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 @@ }); }); + 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() {