Merge "Merge branch 'stable-3.4'"
diff --git a/WORKSPACE b/WORKSPACE
index 6c01d03..0caf8c3 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -66,8 +66,8 @@
http_archive(
name = "build_bazel_rules_nodejs",
- sha256 = "1134ec9b7baee008f1d54f0483049a97e53a57cd3913ec9d6db625549c98395a",
- urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/3.4.0/rules_nodejs-3.4.0.tar.gz"],
+ sha256 = "10f534e1c80f795cffe1f2822becd4897754d18564612510c59b3c73544ae7c6",
+ urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/3.5.0/rules_nodejs-3.5.0.tar.gz"],
)
# Golang support for PolyGerrit local dev server.
diff --git a/java/com/google/gerrit/server/config/GerritServerConfigModule.java b/java/com/google/gerrit/server/config/GerritServerConfigModule.java
index 3777a55..27d1d58 100644
--- a/java/com/google/gerrit/server/config/GerritServerConfigModule.java
+++ b/java/com/google/gerrit/server/config/GerritServerConfigModule.java
@@ -20,8 +20,6 @@
import com.google.gerrit.server.securestore.SecureStore;
import com.google.gerrit.server.securestore.SecureStoreProvider;
import com.google.inject.AbstractModule;
-import com.google.inject.Guice;
-import com.google.inject.Injector;
import com.google.inject.ProvisionException;
import java.io.IOException;
import java.nio.file.Path;
@@ -42,22 +40,13 @@
}
private static String getSecureStoreFromGerritConfig(Path sitePath) {
- AbstractModule m =
- new AbstractModule() {
- @Override
- protected void configure() {
- bind(Path.class).annotatedWith(SitePath.class).toInstance(sitePath);
- bind(SitePaths.class);
- }
- };
- Injector injector = Guice.createInjector(m);
- SitePaths site = injector.getInstance(SitePaths.class);
- FileBasedConfig cfg = new FileBasedConfig(site.gerrit_config.toFile(), FS.DETECTED);
- if (!cfg.getFile().exists()) {
- return DefaultSecureStore.class.getName();
- }
-
try {
+ SitePaths site = new SitePaths(sitePath);
+ FileBasedConfig cfg = new FileBasedConfig(site.gerrit_config.toFile(), FS.DETECTED);
+ if (!cfg.getFile().exists()) {
+ return DefaultSecureStore.class.getName();
+ }
+
cfg.load();
String className = cfg.getString("gerrit", null, "secureStoreClass");
return nullToDefault(className);
diff --git a/package.json b/package.json
index f5eafee..a3329a1 100644
--- a/package.json
+++ b/package.json
@@ -3,9 +3,9 @@
"version": "3.1.0-SNAPSHOT",
"description": "Gerrit Code Review",
"dependencies": {
- "@bazel/rollup": "^3.4.0",
- "@bazel/terser": "^3.4.0",
- "@bazel/typescript": "^3.4.0"
+ "@bazel/rollup": "^3.5.0",
+ "@bazel/terser": "^3.5.0",
+ "@bazel/typescript": "^3.5.0"
},
"devDependencies": {
"@typescript-eslint/eslint-plugin": "^4.22.0",
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index c6d2ee0..f05d665 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -503,6 +503,9 @@
_activeTabs: string[] = [PrimaryTab.FILES, SecondaryTab.CHANGE_LOG];
@property({type: Boolean})
+ unresolvedOnly = false;
+
+ @property({type: Boolean})
_showAllRobotComments = false;
@property({type: Boolean})
@@ -819,6 +822,15 @@
if (tabName) break;
target = target?.parentElement as HTMLElement | null;
} while (target);
+
+ if (tabName === PrimaryTab.COMMENT_THREADS) {
+ // Show unresolved threads by default only if they are present
+ const hasUnresolvedThreads =
+ (this._commentThreads ?? []).filter(thread => isUnresolved(thread))
+ .length > 0;
+ if (hasUnresolvedThreads) this.unresolvedOnly = true;
+ }
+
this.reporting.reportInteraction('show-tab', {
tabName,
src: 'paper-tab-click',
@@ -900,13 +912,6 @@
return false;
}
- _computeShowUnresolved(threads?: CommentThread[]) {
- // If all threads are resolved and the Comments Tab is opened then show
- // all threads instead
- if (!threads?.length) return true;
- return threads.filter(thread => isUnresolved(thread)).length > 0;
- }
-
_robotCommentCountPerPatchSet(threads: CommentThread[]) {
return threads.reduce((robotCommentCountMap, thread) => {
const comments = thread.comments;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 6025268..069b720 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -570,7 +570,7 @@
logged-in="[[_loggedIn]]"
comment-tab-state="[[_tabState.commentTab]]"
only-show-robot-comments-with-human-reply=""
- unresolved-only="[[_computeShowUnresolved(_commentThreads)]]"
+ unresolved-only="[[unresolvedOnly]]"
show-comment-context
></gr-thread-list>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index e303dad..5504877 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -40,8 +40,9 @@
} from '../../../constants/constants';
import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {
- accountKey,
accountOrGroupKey,
+ isReviewerOrCC,
+ mapReviewer,
removeServiceUsers,
} from '../../../utils/account-util';
import {getDisplayName} from '../../../utils/display-name-util';
@@ -74,7 +75,6 @@
ParsedJSON,
PatchSetNum,
ProjectInfo,
- ReviewerInput,
Reviewers,
ReviewInput,
ReviewResult,
@@ -542,18 +542,6 @@
}
}
- _mapReviewer(addition: AccountAddition): ReviewerInput {
- if (addition.account) {
- return {reviewer: accountKey(addition.account)};
- }
- if (addition.group) {
- const reviewer = decodeURIComponent(addition.group.id) as GroupId;
- const confirmed = addition.group.confirmed;
- return {reviewer, confirmed};
- }
- throw new Error('Reviewer must be either an account or a group.');
- }
-
send(includeComments: boolean, startReview: boolean) {
this.reporting.time(Timing.SEND_REPLY);
const labels = this.$.labelScores.getLabelValues();
@@ -606,16 +594,24 @@
state?: ReviewerState
) => {
additions.forEach(addition => {
- const reviewer = this._mapReviewer(addition);
+ const reviewer = mapReviewer(addition);
if (state) reviewer.state = state;
reviewInput.reviewers?.push(reviewer);
});
};
reviewInput.reviewers = [];
+ assertIsDefined(this.change, 'change');
+ const change = this.change;
addToReviewInput(this.$.reviewers.additions(), ReviewerState.REVIEWER);
addToReviewInput(this.$.ccs.additions(), ReviewerState.CC);
- addToReviewInput(this.$.reviewers.removals(), ReviewerState.REMOVED);
- addToReviewInput(this.$.ccs.removals(), ReviewerState.REMOVED);
+ addToReviewInput(
+ this.$.reviewers.removals().filter(r => isReviewerOrCC(change, r)),
+ ReviewerState.REMOVED
+ );
+ addToReviewInput(
+ this.$.ccs.removals().filter(r => isReviewerOrCC(change, r)),
+ ReviewerState.REMOVED
+ );
this.disabled = true;
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
index b51eb5f..f5474d8 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
@@ -1075,6 +1075,11 @@
element._reviewers = [reviewer1, reviewer2];
element._ccs = [cc1, cc2, cc3];
+ element.change.reviewers = {
+ [ReviewerState.CC]: [],
+ [ReviewerState.REVIEWER]: [{_account_id: 33}],
+ };
+
const mutations = [];
stubSaveReview(review => mutations.push(...review.reviewers));
@@ -1128,7 +1133,7 @@
// Send and purge and verify moves, delete cc3.
await element.send();
- expect(mutations).to.have.lengthOf(7);
+ expect(mutations).to.have.lengthOf(5);
expect(mutations[0]).to.deep.equal(mapReviewer(cc1,
ReviewerState.REVIEWER));
expect(mutations[1]).to.deep.equal(mapReviewer(cc2,
@@ -1138,13 +1143,9 @@
expect(mutations[3]).to.deep.equal(mapReviewer(reviewer2,
ReviewerState.CC));
- // 3 remove events stored
+ // Only 1 account was initially part of the change
expect(mutations[4]).to.deep.equal({reviewer: 33, state:
ReviewerState.REMOVED});
- expect(mutations[5]).to.deep.equal({reviewer: 35, state:
- ReviewerState.REMOVED});
- expect(mutations[6]).to.deep.equal({reviewer: 37, state:
- ReviewerState.REMOVED});
});
test('emits cancel on esc key', () => {
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index ff37608..2a509d3 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -18,13 +18,17 @@
import {
AccountId,
AccountInfo,
+ ChangeInfo,
EmailAddress,
+ GroupId,
GroupInfo,
isAccount,
isGroup,
+ ReviewerInput,
} from '../types/common';
-import {AccountTag} from '../constants/constants';
+import {AccountTag, ReviewerState} from '../constants/constants';
import {assertNever} from './common-util';
+import {AccountAddition} from '../elements/shared/gr-account-list/gr-account-list';
export function accountKey(account: AccountInfo): AccountId | EmailAddress {
if (account._account_id) return account._account_id;
@@ -32,6 +36,30 @@
throw new Error('Account has neither _account_id nor email.');
}
+export function mapReviewer(addition: AccountAddition): ReviewerInput {
+ if (addition.account) {
+ return {reviewer: accountKey(addition.account)};
+ }
+ if (addition.group) {
+ const reviewer = decodeURIComponent(addition.group.id) as GroupId;
+ const confirmed = addition.group.confirmed;
+ return {reviewer, confirmed};
+ }
+ throw new Error('Reviewer must be either an account or a group.');
+}
+
+export function isReviewerOrCC(
+ change: ChangeInfo,
+ reviewerAddition: AccountAddition
+): boolean {
+ const reviewers = [
+ ...(change.reviewers[ReviewerState.CC] ?? []),
+ ...(change.reviewers[ReviewerState.REVIEWER] ?? []),
+ ];
+ const reviewer = mapReviewer(reviewerAddition);
+ return reviewers.some(r => accountOrGroupKey(r) === reviewer.reviewer);
+}
+
export function isServiceUser(account?: AccountInfo): boolean {
return !!account?.tags?.includes(AccountTag.SERVICE_USER);
}
diff --git a/tools/node_tools/package.json b/tools/node_tools/package.json
index 6e29299..a0427f2 100644
--- a/tools/node_tools/package.json
+++ b/tools/node_tools/package.json
@@ -3,8 +3,8 @@
"description": "Gerrit Build Tools",
"browser": false,
"dependencies": {
- "@bazel/rollup": "^3.4.0",
- "@bazel/typescript": "^3.4.0",
+ "@bazel/rollup": "^3.5.0",
+ "@bazel/typescript": "^3.5.0",
"@types/node": "^10.17.12",
"@types/parse5": "^4.0.0",
"@types/parse5-html-rewriting-stream": "^5.1.2",
diff --git a/tools/node_tools/yarn.lock b/tools/node_tools/yarn.lock
index b8ac5fb..2825e21 100644
--- a/tools/node_tools/yarn.lock
+++ b/tools/node_tools/yarn.lock
@@ -492,15 +492,15 @@
lodash "^4.17.13"
to-fast-properties "^2.0.0"
-"@bazel/rollup@^3.4.0":
- version "3.4.0"
- resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.4.0.tgz#cdecb2b90535ef51fb3d56cc8bc19996918bac1a"
- integrity sha512-QKnttbYyEQjRbWrOlkH2JuDnSww+9K7Ppil91zBTtr/qYTGW9XO0v7Ft3cs30s2NIWSGIuKj9/N5as+Uyratrw==
+"@bazel/rollup@^3.5.0":
+ version "3.5.0"
+ resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.5.0.tgz#3de2db08cbc62c3cffbbabaa4517ec250cf6419a"
+ integrity sha512-sFPqbzSbIn6h66uuZdXgK5oitSmEGtnDPfL3TwTS4ZWy75SpYvk9X1TFGlvkralEkVnFfdH15sq80/1t+YgQow==
-"@bazel/typescript@^3.4.0":
- version "3.4.0"
- resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.4.0.tgz#031d989682ff8605ed8745f31448c2f76a1b653a"
- integrity sha512-XlWrlQnsdQHTwsliUAf4mySHOgqRY2S57LKG2rKRjm+a015Lzlmxo6jRQaxjr68UmuhmlklRw0WfCFxdR81AvQ==
+"@bazel/typescript@^3.5.0":
+ version "3.5.0"
+ resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.5.0.tgz#605493f4f0a5297df8a7fcccb86a1a80ea2090bb"
+ integrity sha512-BtGFp4nYFkQTmnONCzomk7dkmOwaINBL3piq+lykBlcc6UxLe9iCAnZpOyPypB1ReN3k3SRNAa53x6oGScQxMg==
dependencies:
protobufjs "6.8.8"
semver "5.6.0"
diff --git a/yarn.lock b/yarn.lock
index ead8dbf..45b4f2c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -500,20 +500,20 @@
lodash "^4.17.19"
to-fast-properties "^2.0.0"
-"@bazel/rollup@^3.4.0":
- version "3.4.0"
- resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.4.0.tgz#cdecb2b90535ef51fb3d56cc8bc19996918bac1a"
- integrity sha512-QKnttbYyEQjRbWrOlkH2JuDnSww+9K7Ppil91zBTtr/qYTGW9XO0v7Ft3cs30s2NIWSGIuKj9/N5as+Uyratrw==
+"@bazel/rollup@^3.5.0":
+ version "3.5.0"
+ resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.5.0.tgz#3de2db08cbc62c3cffbbabaa4517ec250cf6419a"
+ integrity sha512-sFPqbzSbIn6h66uuZdXgK5oitSmEGtnDPfL3TwTS4ZWy75SpYvk9X1TFGlvkralEkVnFfdH15sq80/1t+YgQow==
-"@bazel/terser@^3.4.0":
- version "3.4.0"
- resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-3.4.0.tgz#9a25892977f00974e4195ff6cbe71ec0313a77d5"
- integrity sha512-E26ijh44aXIXcg3EQEZcL2nkGlWZtMka0gwmYo9bDRyGt6rCRhFuSBC0mz9YCifUhKuACKWXLHPz9wvh1CDkEA==
+"@bazel/terser@^3.5.0":
+ version "3.5.0"
+ resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-3.5.0.tgz#4b1c3a3b781e65547694aa05bc600c251e4d8c0b"
+ integrity sha512-dpWHn1Iu+w0uA/kvPb0pP+4Io0PrVuzCCbVg2Ow4uRt/gTFKQJJWp4EiTitEZlPA2dHlW7PHThAb93lGo2c8qA==
-"@bazel/typescript@^3.4.0":
- version "3.4.0"
- resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.4.0.tgz#031d989682ff8605ed8745f31448c2f76a1b653a"
- integrity sha512-XlWrlQnsdQHTwsliUAf4mySHOgqRY2S57LKG2rKRjm+a015Lzlmxo6jRQaxjr68UmuhmlklRw0WfCFxdR81AvQ==
+"@bazel/typescript@^3.5.0":
+ version "3.5.0"
+ resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.5.0.tgz#605493f4f0a5297df8a7fcccb86a1a80ea2090bb"
+ integrity sha512-BtGFp4nYFkQTmnONCzomk7dkmOwaINBL3piq+lykBlcc6UxLe9iCAnZpOyPypB1ReN3k3SRNAa53x6oGScQxMg==
dependencies:
protobufjs "6.8.8"
semver "5.6.0"
