Merge "Highlight comment range"
diff --git a/java/com/google/gerrit/auth/BUILD b/java/com/google/gerrit/auth/BUILD
index a390e14..609ec8a 100644
--- a/java/com/google/gerrit/auth/BUILD
+++ b/java/com/google/gerrit/auth/BUILD
@@ -20,8 +20,6 @@
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
- "//java/com/google/gerrit/git",
- "//java/com/google/gerrit/jgit",
"//java/com/google/gerrit/proto",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/cache/serialize",
diff --git a/java/com/google/gerrit/common/data/testing/BUILD b/java/com/google/gerrit/common/data/testing/BUILD
index b9ec30b..d39d05c 100644
--- a/java/com/google/gerrit/common/data/testing/BUILD
+++ b/java/com/google/gerrit/common/data/testing/BUILD
@@ -6,7 +6,6 @@
srcs = glob(["**/*.java"]),
visibility = ["//visibility:public"],
deps = [
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//lib/truth",
],
diff --git a/java/com/google/gerrit/pgm/util/BUILD b/java/com/google/gerrit/pgm/util/BUILD
index f7c2b75..cfdd383 100644
--- a/java/com/google/gerrit/pgm/util/BUILD
+++ b/java/com/google/gerrit/pgm/util/BUILD
@@ -12,7 +12,6 @@
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/metrics/dropwizard",
"//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server/cache/h2",
"//java/com/google/gerrit/server/cache/mem",
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/schema",
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 69e398d..9fa7456 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -62,7 +62,6 @@
"//java/com/google/gerrit/server/util/git",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/util/cli",
- "//java/com/google/gerrit/util/ssl",
"//java/org/apache/commons/net",
"//lib:args4j",
"//lib:autolink",
@@ -144,7 +143,6 @@
visibility = ["//visibility:public"],
deps = [
":server",
- "//java/com/google/gerrit/auth",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/logging",
diff --git a/java/com/google/gerrit/server/cache/serialize/entities/BUILD b/java/com/google/gerrit/server/cache/serialize/entities/BUILD
index cb8c4ae..55080e8 100644
--- a/java/com/google/gerrit/server/cache/serialize/entities/BUILD
+++ b/java/com/google/gerrit/server/cache/serialize/entities/BUILD
@@ -5,12 +5,8 @@
srcs = glob(["*.java"]),
visibility = ["//visibility:public"],
deps = [
- "//java/com/google/gerrit/common:annotations",
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
- "//java/com/google/gerrit/git",
- "//java/com/google/gerrit/proto",
"//java/com/google/gerrit/server/cache/serialize",
"//lib:guava",
"//lib:jgit",
diff --git a/java/com/google/gerrit/server/data/BUILD b/java/com/google/gerrit/server/data/BUILD
index c3dc672..1aaab96 100644
--- a/java/com/google/gerrit/server/data/BUILD
+++ b/java/com/google/gerrit/server/data/BUILD
@@ -9,7 +9,6 @@
deps = [
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
- "//java/org/apache/commons/net",
"//lib:gson",
],
)
diff --git a/java/com/google/gerrit/server/git/TagSet.java b/java/com/google/gerrit/server/git/TagSet.java
index 43483bf..d6220a2 100644
--- a/java/com/google/gerrit/server/git/TagSet.java
+++ b/java/com/google/gerrit/server/git/TagSet.java
@@ -16,9 +16,9 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto.TagSetProto;
@@ -37,6 +37,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectIdOwnerMap;
import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevSort;
@@ -44,6 +45,12 @@
class TagSet {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private static final ImmutableSet<String> SKIPPABLE_REF_PREFIXES =
+ ImmutableSet.of(
+ RefNames.REFS_CHANGES,
+ RefNames.REFS_CACHE_AUTOMERGE,
+ RefNames.REFS_DRAFT_COMMENTS,
+ RefNames.REFS_STARRED_CHANGES);
private final Project.NameKey projectName;
private final Map<String, CachedRef> refs;
@@ -179,7 +186,9 @@
try (TagWalk rw = new TagWalk(git)) {
rw.setRetainBody(false);
- for (Ref ref : git.getRefDatabase().getRefs()) {
+ for (Ref ref :
+ git.getRefDatabase()
+ .getRefsByPrefixWithExclusions(RefDatabase.ALL, SKIPPABLE_REF_PREFIXES)) {
if (skip(ref)) {
continue;
@@ -365,9 +374,7 @@
static boolean skip(Ref ref) {
return ref.isSymbolic()
|| ref.getObjectId() == null
- || PatchSet.isChangeRef(ref.getName())
- || RefNames.isNoteDbMetaRef(ref.getName())
- || ref.getName().startsWith(RefNames.REFS_CACHE_AUTOMERGE);
+ || SKIPPABLE_REF_PREFIXES.stream().anyMatch(p -> ref.getName().startsWith(p));
}
private static boolean isTag(Ref ref) {
diff --git a/java/com/google/gerrit/server/group/testing/BUILD b/java/com/google/gerrit/server/group/testing/BUILD
index fd61dff..77bb777 100644
--- a/java/com/google/gerrit/server/group/testing/BUILD
+++ b/java/com/google/gerrit/server/group/testing/BUILD
@@ -7,7 +7,6 @@
testonly = True,
srcs = glob(["*.java"]),
deps = [
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/server",
"//lib:guava",
diff --git a/java/com/google/gerrit/server/project/testing/BUILD b/java/com/google/gerrit/server/project/testing/BUILD
index 3112b5a..ab75ec7 100644
--- a/java/com/google/gerrit/server/project/testing/BUILD
+++ b/java/com/google/gerrit/server/project/testing/BUILD
@@ -5,8 +5,5 @@
testonly = True,
srcs = glob(["*.java"]),
visibility = ["//visibility:public"],
- deps = [
- "//java/com/google/gerrit/common:server",
- "//java/com/google/gerrit/entities",
- ],
+ deps = ["//java/com/google/gerrit/entities"],
)
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index 3cb0796..6d3e222 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -8,7 +8,6 @@
name = "restapi",
srcs = glob(["**/*.java"]),
deps = [
- "//java/com/google/gerrit/auth",
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
@@ -19,13 +18,11 @@
"//java/com/google/gerrit/index:query_exception",
"//java/com/google/gerrit/index/project",
"//java/com/google/gerrit/json",
- "//java/com/google/gerrit/mail",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/util/time",
- "//java/com/google/gerrit/util/cli",
"//lib:args4j",
"//lib:blame-cache",
"//lib:gson",
diff --git a/java/gerrit/BUILD b/java/gerrit/BUILD
index 7dbf751..db831b7 100644
--- a/java/gerrit/BUILD
+++ b/java/gerrit/BUILD
@@ -5,7 +5,6 @@
srcs = glob(["**/*.java"]),
visibility = ["//visibility:public"],
deps = [
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
diff --git a/javatests/com/google/gerrit/acceptance/git/BUILD b/javatests/com/google/gerrit/acceptance/git/BUILD
index ef54c92..13311e3 100644
--- a/javatests/com/google/gerrit/acceptance/git/BUILD
+++ b/javatests/com/google/gerrit/acceptance/git/BUILD
@@ -21,7 +21,6 @@
deps = [
"//java/com/google/gerrit/acceptance:lib",
"//java/com/google/gerrit/git",
- "//java/com/google/gerrit/mail",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/BUILD b/javatests/com/google/gerrit/acceptance/rest/project/BUILD
index 5e1fc83..edcb1f9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/BUILD
+++ b/javatests/com/google/gerrit/acceptance/rest/project/BUILD
@@ -20,7 +20,6 @@
"LabelAssert.java",
],
deps = [
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
diff --git a/javatests/com/google/gerrit/acceptance/server/change/BUILD b/javatests/com/google/gerrit/acceptance/server/change/BUILD
index e41178f..19ca946 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/BUILD
+++ b/javatests/com/google/gerrit/acceptance/server/change/BUILD
@@ -18,8 +18,6 @@
"//java/com/google/gerrit/acceptance:lib",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
- "//java/com/google/gerrit/server/logging",
- "//java/com/google/gerrit/server/util/time",
"@guava//jar",
],
)
diff --git a/javatests/com/google/gerrit/auth/BUILD b/javatests/com/google/gerrit/auth/BUILD
index d4bb16d..6a41d01 100644
--- a/javatests/com/google/gerrit/auth/BUILD
+++ b/javatests/com/google/gerrit/auth/BUILD
@@ -17,16 +17,13 @@
"//prolog:gerrit-prolog-common",
],
deps = [
- "//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/auth",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/proto/testing",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/cache/serialize",
- "//java/com/google/gerrit/server/cache/testing",
"//java/com/google/gerrit/testing:gerrit-test-util",
- "//java/com/google/gerrit/truth",
"//lib:guava",
"//lib:guava-retrying",
"//lib:jgit",
diff --git a/javatests/com/google/gerrit/server/cache/serialize/BUILD b/javatests/com/google/gerrit/server/cache/serialize/BUILD
index 6976d19..fa6a717 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/BUILD
+++ b/javatests/com/google/gerrit/server/cache/serialize/BUILD
@@ -5,7 +5,6 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/entities",
- "//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/cache/serialize",
"//java/com/google/gerrit/server/cache/testing",
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD b/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
index b84febb..a8158fc 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
@@ -4,14 +4,10 @@
name = "tests",
srcs = glob(["*.java"]),
deps = [
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server/cache/serialize",
"//java/com/google/gerrit/server/cache/serialize/entities",
- "//java/com/google/gerrit/server/cache/testing",
- "//java/com/google/gerrit/testing:gerrit-test-util",
"//lib:guava",
"//lib:jgit",
"//lib:protobuf",
diff --git a/javatests/com/google/gerrit/server/group/db/BUILD b/javatests/com/google/gerrit/server/group/db/BUILD
index 3303338..9f9f459 100644
--- a/javatests/com/google/gerrit/server/group/db/BUILD
+++ b/javatests/com/google/gerrit/server/group/db/BUILD
@@ -6,7 +6,6 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/common:annotations",
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/common/data/testing:common-data-test-util",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/exceptions",
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index 0258e5d..43b9690 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -19,7 +19,6 @@
"//java/com/google/gerrit/acceptance/config",
"//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/common:annotations",
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/httpd",
diff --git a/javatests/com/google/gerrit/server/rules/BUILD b/javatests/com/google/gerrit/server/rules/BUILD
index 250b0ce..5c57ede 100644
--- a/javatests/com/google/gerrit/server/rules/BUILD
+++ b/javatests/com/google/gerrit/server/rules/BUILD
@@ -7,7 +7,6 @@
resources = ["//prologtests:gerrit_common_test"],
runtime_deps = ["//prolog:gerrit-prolog-common"],
deps = [
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/project/testing:project-test-util",
diff --git a/javatests/com/google/gerrit/server/update/BUILD b/javatests/com/google/gerrit/server/update/BUILD
index e175b95..4fe4ab04 100644
--- a/javatests/com/google/gerrit/server/update/BUILD
+++ b/javatests/com/google/gerrit/server/update/BUILD
@@ -10,7 +10,6 @@
],
deps = [
"//java/com/google/gerrit/common:annotations",
- "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
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 355be72..85bfed2 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
@@ -548,6 +548,7 @@
<gr-related-changes-list-experimental
change="[[_change]]"
id="relatedChangesExperimental"
+ patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"
></gr-related-changes-list-experimental>
</template>
<template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
index 5134b72..77bdf48 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
@@ -23,12 +23,16 @@
SubmittedTogetherInfo,
ChangeInfo,
RelatedChangeAndCommitInfo,
+ RelatedChangesInfo,
+ PatchSetNum,
+ CommitId,
} from '../../../types/common';
import {appContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {pluralize} from '../../../utils/string-util';
import {ChangeStatus} from '../../../constants/constants';
+import {getRevisionKey} from '../../../utils/change-util';
function isChangeInfo(
x: ChangeInfo | RelatedChangeAndCommitInfo | ParsedChangeInfo
@@ -44,12 +48,18 @@
@property()
change?: ParsedChangeInfo;
+ @property({type: String})
+ patchNum?: PatchSetNum;
+
@property()
_submittedTogether?: SubmittedTogetherInfo = {
changes: [],
non_visible_changes: 0,
};
+ @property()
+ _relatedResponse?: RelatedChangesInfo = {changes: []};
+
private readonly restApiService = appContext.restApiService;
static get styles() {
@@ -81,16 +91,57 @@
}
render() {
+ const relatedChanges = this._relatedResponse?.changes ?? [];
+ let showWhenCollapsedPredicate = this.showWhenCollapsedPredicateFactory(
+ relatedChanges.length,
+ relatedChanges.findIndex(relatedChange =>
+ this._changesEqual(relatedChange, this.change)
+ )
+ );
+ const connectedRevisions = this._computeConnectedRevisions(
+ this.change,
+ this.patchNum,
+ relatedChanges
+ );
+ const relatedChangeSection = html` <section
+ class="relatedChanges"
+ ?hidden=${!relatedChanges.length}
+ >
+ <h4 class="title">Relation chain</h4>
+ <gr-related-collapse .length=${relatedChanges.length}>
+ ${relatedChanges.map(
+ (change, index) =>
+ html`<gr-related-change
+ class="${classMap({
+ ['show-when-collapsed']: showWhenCollapsedPredicate(index),
+ })}"
+ .isCurrentChange="${this._changesEqual(change, this.change)}"
+ .change="${change}"
+ .connectedRevisions="${connectedRevisions}"
+ .href="${change?._change_number
+ ? GerritNav.getUrlForChangeById(
+ change._change_number,
+ change.project,
+ change._revision_number as PatchSetNum
+ )
+ : ''}"
+ .showChangeStatus=${true}
+ >${change.commit.subject}</gr-related-change
+ >`
+ )}
+ </gr-related-collapse>
+ </section>`;
+
const submittedTogetherChanges = this._submittedTogether?.changes ?? [];
const countNonVisibleChanges =
this._submittedTogether?.non_visible_changes ?? 0;
- const showWhenCollapsedPredicate = this.showWhenCollapsedPredicateFactory(
+ showWhenCollapsedPredicate = this.showWhenCollapsedPredicateFactory(
submittedTogetherChanges.length,
submittedTogetherChanges.findIndex(relatedChange =>
this._changesEqual(relatedChange, this.change)
)
);
- return html` <section
+ const submittedTogetherSection = html`<section
id="submittedTogether"
?hidden=${!submittedTogetherChanges?.length &&
!this._submittedTogether?.non_visible_changes}
@@ -98,20 +149,29 @@
<h4 class="title">Submitted together</h4>
<gr-related-collapse .length=${submittedTogetherChanges.length}>
${submittedTogetherChanges.map(
- (relatedChange, index) =>
+ (change, index) =>
html`<gr-related-change
class="${classMap({
['show-when-collapsed']: showWhenCollapsedPredicate(index),
})}"
- .currentChange="${this._changesEqual(relatedChange, this.change)}"
- .change="${relatedChange}"
- ></gr-related-change>`
+ .currentChange="${this._changesEqual(change, this.change)}"
+ .change="${change}"
+ .href="${GerritNav.getUrlForChangeById(
+ change._number,
+ change.project
+ )}"
+ .showSubmittableCheck=${true}
+ >${change.project}: ${change.branch}:
+ ${change.subject}</gr-related-change
+ >`
)}
</gr-related-collapse>
<div class="note" ?hidden=${!countNonVisibleChanges}>
(+ ${pluralize(countNonVisibleChanges, 'non-visible change')})
</div>
</section>`;
+
+ return html`${relatedChangeSection}${submittedTogetherSection}`;
}
showWhenCollapsedPredicateFactory(length: number, highlightIndex: number) {
@@ -128,11 +188,24 @@
reload() {
if (!this.change) return Promise.reject(new Error('change missing'));
- return this.restApiService
- .getChangesSubmittedTogether(this.change._number)
- .then(response => {
- this._submittedTogether = response;
- });
+ if (!this.patchNum) return Promise.reject(new Error('patchNum missing'));
+ const promises: Array<Promise<void>> = [
+ this.restApiService
+ .getRelatedChanges(this.change._number, this.patchNum)
+ .then(response => {
+ if (!response) {
+ throw new Error('getRelatedChanges returned undefined response');
+ }
+ this._relatedResponse = response;
+ }),
+ this.restApiService
+ .getChangesSubmittedTogether(this.change._number)
+ .then(response => {
+ this._submittedTogether = response;
+ }),
+ ];
+
+ return Promise.all(promises);
}
/**
@@ -165,6 +238,46 @@
}
return change._change_number;
}
+
+ /*
+ * A list of commit ids connected to change to understand if other change
+ * is direct or indirect ancestor / descendant.
+ */
+ _computeConnectedRevisions(
+ change?: ParsedChangeInfo,
+ patchNum?: PatchSetNum,
+ relatedChanges?: RelatedChangeAndCommitInfo[]
+ ) {
+ if (!patchNum || !relatedChanges || !change) {
+ return [];
+ }
+
+ const connected: CommitId[] = [];
+ const changeRevision = getRevisionKey(change, patchNum);
+ const commits = relatedChanges.map(c => c.commit);
+ let pos = commits.length - 1;
+
+ while (pos >= 0) {
+ const commit: CommitId = commits[pos].commit;
+ connected.push(commit);
+ // TODO(TS): Ensure that both (commit and changeRevision) are string and use === instead
+ // eslint-disable-next-line eqeqeq
+ if (commit == changeRevision) {
+ break;
+ }
+ pos--;
+ }
+ while (pos >= 0) {
+ for (let i = 0; i < commits[pos].parents.length; i++) {
+ if (connected.includes(commits[pos].parents[i].commit)) {
+ connected.push(commits[pos].commit);
+ break;
+ }
+ }
+ --pos;
+ }
+ return connected;
+ }
}
@customElement('gr-related-collapse')
@@ -232,10 +345,26 @@
@customElement('gr-related-change')
export class GrRelatedChange extends GrLitElement {
@property()
- change?: ChangeInfo;
+ change?: ChangeInfo | RelatedChangeAndCommitInfo;
@property()
- currentChange = false;
+ href?: string;
+
+ @property()
+ isCurrentChange = false;
+
+ @property()
+ showSubmittableCheck = false;
+
+ @property()
+ showChangeStatus = false;
+
+ /*
+ * Needed for calculation if change is direct or indirect ancestor/descendant
+ * to current change.
+ */
+ @property()
+ connectedRevisions?: CommitId[];
static get styles() {
return [
@@ -258,6 +387,29 @@
color: var(--deemphasized-text-color);
text-decoration: line-through;
}
+ .status {
+ color: var(--deemphasized-text-color);
+ font-weight: var(--font-weight-bold);
+ margin-left: var(--spacing-xs);
+ }
+ .notCurrent {
+ color: #e65100;
+ }
+ .indirectAncestor {
+ color: #33691e;
+ }
+ .submittableCheck {
+ padding-left: var(--spacing-s);
+ color: var(--positive-green-text-color);
+ display: none;
+ }
+ .submittableCheck.submittable {
+ display: inline;
+ }
+ .hidden,
+ .mobile {
+ display: none;
+ }
.submittableCheck {
padding-left: var(--spacing-s);
color: var(--positive-green-text-color);
@@ -281,30 +433,30 @@
role="img"
class="arrowToCurrentChange"
aria-label="Arrow marking current change"
- ?hidden=${!this.currentChange}
+ ?hidden=${!this.isCurrentChange}
>➔</span
>
<div class="changeContainer">
- <a
- href="${GerritNav.getUrlForChangeById(
- change._number,
- change.project
- )}"
- class="${linkClass}"
- >${change.project}: ${change.branch}: ${change.subject}</a
- >
- <span
- tabindex="-1"
- title="Submittable"
- class="submittableCheck ${linkClass}"
- role="img"
- aria-label="Submittable"
- >✓</span
- >
+ <a href="${this.href}" class="${linkClass}"><slot></slot></a>
+ ${this.showSubmittableCheck
+ ? html`<span
+ tabindex="-1"
+ title="Submittable"
+ class="submittableCheck ${linkClass}"
+ role="img"
+ aria-label="Submittable"
+ >✓</span
+ >`
+ : ''}
+ ${this.showChangeStatus && !isChangeInfo(change)
+ ? html`<span class="${this._computeChangeStatusClass(change)}">
+ (${this._computeChangeStatus(change)})
+ </span>`
+ : ''}
</div> `;
}
- _computeLinkClass(change: ChangeInfo) {
+ _computeLinkClass(change: ChangeInfo | RelatedChangeAndCommitInfo) {
const statuses = [];
if (change.status === ChangeStatus.ABANDONED) {
statuses.push('strikethrough');
@@ -314,6 +466,44 @@
}
return statuses.join(' ');
}
+
+ _computeChangeStatusClass(change: RelatedChangeAndCommitInfo) {
+ const classes = ['status'];
+ if (change._revision_number !== change._current_revision_number) {
+ classes.push('notCurrent');
+ } else if (this._isIndirectAncestor(change)) {
+ classes.push('indirectAncestor');
+ } else if (change.submittable) {
+ classes.push('submittable');
+ } else if (change.status === ChangeStatus.NEW) {
+ classes.push('hidden');
+ }
+ return classes.join(' ');
+ }
+
+ _computeChangeStatus(change: RelatedChangeAndCommitInfo) {
+ switch (change.status) {
+ case ChangeStatus.MERGED:
+ return 'Merged';
+ case ChangeStatus.ABANDONED:
+ return 'Abandoned';
+ }
+ if (change._revision_number !== change._current_revision_number) {
+ return 'Not current';
+ } else if (this._isIndirectAncestor(change)) {
+ return 'Indirect ancestor';
+ } else if (change.submittable) {
+ return 'Submittable';
+ }
+ return '';
+ }
+
+ _isIndirectAncestor(change: RelatedChangeAndCommitInfo) {
+ return (
+ this.connectedRevisions &&
+ !this.connectedRevisions.includes(change.commit.commit)
+ );
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 4ad99d5..ba0ca43 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -113,6 +113,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly reportingService = appContext.reportingService;
+
clear() {
this.loading = true;
this.hidden = true;
@@ -440,6 +442,25 @@
_computeNonVisibleChangesNote(n: number) {
return `(+ ${pluralize(n, 'non-visible change')})`;
}
+
+ // TODO(milutin): Temporary for data collection, remove when data collected
+ _reportClick(e: Event) {
+ const target = e.target as HTMLAnchorElement | undefined;
+ const section = target?.parentElement?.parentElement;
+ const sectionName = section?.getElementsByTagName('h4')[0]?.innerText;
+ const sectionLinks = [...(section?.getElementsByTagName('a') ?? [])];
+ const currentChange = section
+ ?.getElementsByClassName('arrowToCurrentChange')[0]
+ ?.nextElementSibling?.nextElementSibling?.getElementsByTagName('a')[0];
+
+ if (!target || !currentChange) return;
+ this.reportingService.reportInteraction('related-change-click', {
+ sectionName,
+ index: sectionLinks.indexOf(target) + 1,
+ countChanges: sectionLinks.length,
+ currentChangeIndex: sectionLinks.indexOf(currentChange) + 1,
+ });
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_html.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_html.ts
index 0f42028..8cb0638 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_html.ts
@@ -117,6 +117,7 @@
href$="[[_computeChangeURL(related._change_number, related.project, related._revision_number)]]"
class$="[[_computeLinkClass(related)]]"
title$="[[related.commit.subject]]"
+ on-click="_reportClick"
>
[[related.commit.subject]]
</a>
@@ -149,6 +150,7 @@
href$="[[_computeChangeURL(related._number, related.project)]]"
class$="[[_computeLinkClass(related)]]"
title$="[[related.project]]: [[related.branch]]: [[related.subject]]"
+ on-click="_reportClick"
>
[[related.project]]: [[related.branch]]: [[related.subject]]
</a>
@@ -176,6 +178,7 @@
href$="[[_computeChangeURL(change._number, change.project)]]"
class$="[[_computeLinkClass(change)]]"
title$="[[change.project]]: [[change.branch]]: [[change.subject]]"
+ on-click="_reportClick"
>
[[change.project]]: [[change.branch]]: [[change.subject]]
</a>
@@ -204,6 +207,7 @@
href$="[[_computeChangeURL(change._number, change.project)]]"
class$="[[_computeLinkClass(change)]]"
title$="[[change.branch]]: [[change.subject]]"
+ on-click="_reportClick"
>
[[change.branch]]: [[change.subject]]
</a>
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 68ff67b..e580c51 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -163,7 +163,6 @@
aria-label="${this.isExpanded
? 'Collapse result row'
: 'Expand result row'}"
- @click="${this.toggleExpanded}"
@keydown="${this.toggleExpanded}"
>
<iron-icon
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 468c8ca..19f43c2 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -594,6 +594,8 @@
export const _testOnly_findCommentById =
ChangeComments.prototype.findCommentById;
+export const _testOnly_getCommentsForPath =
+ ChangeComments.prototype.getCommentsForPath;
@customElement('gr-comment-api')
export class GrCommentApi extends GestureEventListeners(
LegacyElementMixin(PolymerElement)
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index 035c1a3..5ac37dc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -235,7 +235,11 @@
return result;
}
- moveToNextCommentThread(): CursorMoveResult {
+ moveToNextCommentThread(): CursorMoveResult | undefined {
+ if (this.isAtEnd()) {
+ fireEvent(this, 'navigate-to-next-file-with-comments');
+ return;
+ }
const result = this.$.cursorManager.next({
filter: (row: HTMLElement) => this._rowHasThread(row),
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 62b34e9..1550108 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -711,7 +711,12 @@
);
}
- _navToFile(path: string, fileList: string[], direction: -1 | 1) {
+ _navToFile(
+ path: string,
+ fileList: string[],
+ direction: -1 | 1,
+ navigateToFirstComment?: boolean
+ ) {
const newPath = this._getNavLinkPath(path, fileList, direction);
if (!newPath) return;
if (!this._change) return;
@@ -727,11 +732,18 @@
}
if (!newPath.path) return;
+ let lineNum;
+ if (navigateToFirstComment)
+ lineNum = this._changeComments?.getCommentsForPath(
+ newPath.path,
+ this._patchRange
+ )?.[0].line;
GerritNav.navigateToDiff(
this._change,
newPath.path,
this._patchRange.patchNum,
- this._patchRange.basePatchNum
+ this._patchRange.basePatchNum,
+ lineNum
);
}
@@ -1757,6 +1769,23 @@
this._navToFile(this._path, unreviewedFiles, 1);
}
+ _navigateToNextFileWithCommentThread() {
+ if (!this._path) return;
+ if (!this._fileList) return;
+ if (!this._patchRange) return;
+ if (!this._change) return;
+ const hasComment = (path: string) => {
+ return (
+ this._changeComments?.getCommentsForPath(path, this._patchRange!)
+ ?.length ?? 0 > 0
+ );
+ };
+ const filesWithComments = this._fileList.filter(
+ file => file === this._path || hasComment(file)
+ );
+ this._navToFile(this._path, filesWithComments, 1, true);
+ }
+
_handleReloadingDiffPreference() {
this._getDiffPreferences();
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
index a00927c..bfb0cd8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
@@ -428,6 +428,7 @@
<gr-diff-cursor
id="cursor"
on-navigate-to-next-unreviewed-file="_handleNextUnreviewedFile"
+ on-navigate-to-next-file-with-comments="_navigateToNextFileWithCommentThread"
></gr-diff-cursor>
<gr-comment-api id="commentAPI"></gr-comment-api>
`;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 34e74b6..46a670d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -21,7 +21,7 @@
import {ChangeStatus} from '../../../constants/constants.js';
import {TestKeyboardShortcutBinder} from '../../../test/test-utils.js';
import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
-import {_testOnly_findCommentById} from '../gr-comment-api/gr-comment-api.js';
+import {ChangeComments, _testOnly_findCommentById, _testOnly_getCommentsForPath} from '../gr-comment-api/gr-comment-api.js';
import {GerritView} from '../../../services/router/router-model.js';
import {
createChange,
@@ -137,6 +137,7 @@
computeUnresolvedNum: () => {},
getPaths: () => {},
getThreadsBySideForFile: () => [],
+ getCommentsForPath: _testOnly_getCommentsForPath,
findCommentById: _testOnly_findCommentById,
}));
@@ -465,6 +466,48 @@
assert.equal(element._setReviewed.lastCall.args[0], true);
});
+ test('moveToNextCommentThread navigates to next file', () => {
+ const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ const diffChangeStub = sinon.stub(element, '_navigateToChange');
+ sinon.stub(element.$.cursor, 'isAtEnd').returns(true);
+ element._changeNum = '42';
+ const comment = {
+ 'wheatley.md': [{
+ ...createComment(),
+ patch_set: 10,
+ line: 21,
+ }],
+ };
+ element._changeComments = new ChangeComments(comment);
+ element._patchRange = {
+ basePatchNum: PARENT,
+ patchNum: 10,
+ };
+ element._change = {
+ _number: 42,
+ revisions: {
+ a: {_number: 10, commit: {parents: []}},
+ },
+ };
+ element._files = getFilesFromFileList(
+ ['chell.go', 'glados.txt', 'wheatley.md']);
+ element._path = 'glados.txt';
+ element.changeViewState.selectedFileIndex = 1;
+ element._loggedIn = true;
+
+ MockInteractions.pressAndReleaseKeyOn(element, 78, 'shift', 'n');
+ flush();
+ assert.isTrue(diffNavStub.calledWithExactly(
+ element._change, 'wheatley.md', 10, PARENT, 21));
+
+ element._path = 'wheatley.md'; // navigated to next file
+
+ MockInteractions.pressAndReleaseKeyOn(element, 78, 'shift', 'n');
+ flush();
+
+ assert.isTrue(diffChangeStub.called);
+ });
+
test('shift+x shortcut expands all diff context', () => {
const expandStub = sinon.stub(element.$.diffHost, 'expandAllContext');
MockInteractions.pressAndReleaseKeyOn(element, 88, 'shift', 'x');
@@ -621,14 +664,14 @@
MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert.isTrue(element._loading);
assert(diffNavStub.lastCall.calledWithExactly(element._change,
- 'wheatley.md', 10, 5),
+ 'wheatley.md', 10, 5, undefined),
'Should navigate to /c/42/5..10/wheatley.md');
element._path = 'wheatley.md';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert.isTrue(element._loading);
assert(diffNavStub.lastCall.calledWithExactly(element._change,
- 'glados.txt', 10, 5),
+ 'glados.txt', 10, 5, undefined),
'Should navigate to /c/42/5..10/glados.txt');
element._path = 'glados.txt';
@@ -638,7 +681,8 @@
element._change,
'chell.go',
10,
- 5),
+ 5,
+ undefined),
'Should navigate to /c/42/5..10/chell.go');
element._path = 'chell.go';
@@ -692,13 +736,13 @@
MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert(diffNavStub.lastCall.calledWithExactly(element._change,
- 'wheatley.md', 1, PARENT),
+ 'wheatley.md', 1, PARENT, undefined),
'Should navigate to /c/42/1/wheatley.md');
element._path = 'wheatley.md';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(diffNavStub.lastCall.calledWithExactly(element._change,
- 'glados.txt', 1, PARENT),
+ 'glados.txt', 1, PARENT, undefined),
'Should navigate to /c/42/1/glados.txt');
element._path = 'glados.txt';
@@ -707,7 +751,8 @@
element._change,
'chell.go',
1,
- PARENT), 'Should navigate to /c/42/1/chell.go');
+ PARENT,
+ undefined), 'Should navigate to /c/42/1/chell.go');
element._path = 'chell.go';
changeNavStub.reset();
