Merge "Fix PluginLogFile to not open multiple appenders when run in parallel"
diff --git a/java/com/google/gerrit/server/comment/CommentContextKey.java b/java/com/google/gerrit/server/comment/CommentContextKey.java
index e4a927a..ccd50b7 100644
--- a/java/com/google/gerrit/server/comment/CommentContextKey.java
+++ b/java/com/google/gerrit/server/comment/CommentContextKey.java
@@ -6,11 +6,10 @@
import com.google.gerrit.proto.Protos;
import com.google.gerrit.server.cache.proto.Cache;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
-import java.util.Collection;
/**
* An identifier of a comment that should be used to load the comment context using {@link
- * CommentContextCache#get(CommentContextKey)}, or {@link CommentContextCache#getAll(Collection)}.
+ * CommentContextCache#get(CommentContextKey)}, or {@link CommentContextCache#getAll(Iterable)}.
*
* <p>The {@link CommentContextCacheImpl} implementation uses this class as the cache key, while
* replacing the {@link #path()} field with the hashed path.
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index b816264..e8f1fe1 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -138,8 +138,9 @@
}
/**
- * Create change notes based on a {@link Change.Id}. This requires using the Change index and
- * should only be used when {@link Project.NameKey} and the numeric change ID are not available.
+ * Create change notes based on a {@link com.google.gerrit.entities.Change.Id}. This requires
+ * using the Change index and should only be used when {@link
+ * com.google.gerrit.entities.Project.NameKey} and the numeric change ID are not available.
*/
public ChangeNotes createCheckedUsingIndexLookup(Change.Id changeId) {
InternalChangeQuery query = queryProvider.get().noFields();
@@ -155,9 +156,9 @@
}
/**
- * Create change notes based on a list of {@link Change.Id}s. This requires using the Change
- * index and should only be used when {@link Project.NameKey} and the numeric change ID are not
- * available.
+ * Create change notes based on a list of {@link com.google.gerrit.entities.Change.Id}s. This
+ * requires using the Change index and should only be used when {@link
+ * com.google.gerrit.entities.Project.NameKey} and the numeric change ID are not available.
*/
public List<ChangeNotes> createUsingIndexLookup(Collection<Change.Id> changeIds) {
List<ChangeNotes> notes = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java b/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java
index 9827a69..8f53751 100644
--- a/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java
+++ b/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java
@@ -56,7 +56,8 @@
/**
* Returns the old file path associated with the {@link FileHeader}, or empty if the file is
- * {@link Patch.ChangeType#ADDED} or {@link Patch.ChangeType#REWRITE}.
+ * {@link com.google.gerrit.entities.Patch.ChangeType#ADDED} or {@link
+ * com.google.gerrit.entities.Patch.ChangeType#REWRITE}.
*/
static Optional<String> getOldPath(FileHeader header) {
Patch.ChangeType changeType = getChangeType(header);
@@ -76,7 +77,7 @@
/**
* Returns the new file path associated with the {@link FileHeader}, or empty if the file is
- * {@link Patch.ChangeType#DELETED}.
+ * {@link com.google.gerrit.entities.Patch.ChangeType#DELETED}.
*/
static Optional<String> getNewPath(FileHeader header) {
Patch.ChangeType changeType = getChangeType(header);
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
index 41db9ee..66299a8 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
@@ -99,8 +99,8 @@
}
/**
- * Returns the {@link Account.Id} of the current user if a user is signed in. Catches exceptions
- * so that background jobs don't get impacted.
+ * Returns the {@link com.google.gerrit.entities.Account.Id} of the current user if a user is
+ * signed in. Catches exceptions so that background jobs don't get impacted.
*/
private Optional<Account.Id> getAccountIdOfIdentifiedUser() {
try {
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index cb34bdb..7f8add8 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -18,6 +18,13 @@
import static org.apache.http.HttpStatus.SC_CREATED;
import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
import static org.apache.http.HttpStatus.SC_OK;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.timeout;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
@@ -52,7 +59,6 @@
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
-import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -370,37 +376,31 @@
@Test
public void performanceLoggingForRestCall() throws Exception {
- TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
+ PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
try (Registration registration =
extensionRegistry.newRegistration().add(testPerformanceLogger)) {
RestResponse response = adminRestSession.put("/projects/new10");
assertThat(response.getStatusCode()).isEqualTo(SC_CREATED);
-
- // This assertion assumes that the server invokes the PerformanceLogger plugins before it
- // sends
- // the response to the client. If this assertion gets flaky it's likely that this got changed
- // on
- // server-side.
- assertThat(testPerformanceLogger.logEntries()).isNotEmpty();
+ verify(testPerformanceLogger, timeout(5000).atLeastOnce()).log(anyString(), anyLong(), any());
}
}
@Test
public void performanceLoggingForPush() throws Exception {
- TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
+ PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
try (Registration registration =
extensionRegistry.newRegistration().add(testPerformanceLogger)) {
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r = push.to("refs/heads/master");
r.assertOkStatus();
- assertThat(testPerformanceLogger.logEntries()).isNotEmpty();
+ verify(testPerformanceLogger, timeout(5000).atLeastOnce()).log(anyString(), anyLong(), any());
}
}
@Test
@GerritConfig(name = "tracing.performanceLogging", value = "false")
public void noPerformanceLoggingIfDisabled() throws Exception {
- TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
+ PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
try (Registration registration =
extensionRegistry.newRegistration().add(testPerformanceLogger)) {
RestResponse response = adminRestSession.put("/projects/new11");
@@ -410,7 +410,7 @@
PushOneCommit.Result r = push.to("refs/heads/master");
r.assertOkStatus();
- assertThat(testPerformanceLogger.logEntries()).isEmpty();
+ verifyZeroInteractions(testPerformanceLogger);
}
}
@@ -844,19 +844,6 @@
}
}
- private static class TestPerformanceLogger implements PerformanceLogger {
- private List<PerformanceLogEntry> logEntries = new ArrayList<>();
-
- @Override
- public void log(String operation, long durationMs, Metadata metadata) {
- logEntries.add(PerformanceLogEntry.create(operation, metadata));
- }
-
- ImmutableList<PerformanceLogEntry> logEntries() {
- return ImmutableList.copyOf(logEntries);
- }
- }
-
@AutoValue
abstract static class PerformanceLogEntry {
static PerformanceLogEntry create(String operation, Metadata metadata) {
diff --git a/plugins/gitiles b/plugins/gitiles
index a33c8b8..0cd08bb 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit a33c8b8d61b778f8ca84196b1a7cc1fd4fe24946
+Subproject commit 0cd08bbfac1a938079be8aaf6a6eef0e82ff9a41
diff --git a/plugins/replication b/plugins/replication
index 4efef1d..9c21a2f 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 4efef1d481eefaee287b27488be94f9acb044360
+Subproject commit 9c21a2f5cc539b5a13d2646edc15bd17d7977fb3
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 c683a32..df52323 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
@@ -149,7 +149,6 @@
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrMessagesList} from '../gr-messages-list/gr-messages-list';
import {GrThreadList} from '../gr-thread-list/gr-thread-list';
-import {PORTING_COMMENTS_CHANGE_LATENCY_LABEL} from '../../../services/gr-reporting/gr-reporting';
import {fireAlert, fireEvent, firePageError} from '../../../utils/event-util';
import {KnownExperimentId} from '../../../services/flags/flags';
import {fireTitleChange} from '../../../utils/event-util';
@@ -2080,19 +2079,11 @@
if (!this._changeNum)
throw new Error('missing required changeNum property');
- const portedCommentsPromise = this.$.commentAPI.getPortedComments(
- this._changeNum
- );
- const commentsPromise = this.$.commentAPI
- .loadAll(this._changeNum)
+ return this.$.commentAPI
+ .loadAll(this._changeNum, this._patchRange?.patchNum)
.then(comments => {
- this.reporting.time(PORTING_COMMENTS_CHANGE_LATENCY_LABEL);
this._recomputeComments(comments);
});
- Promise.all([portedCommentsPromise, commentsPromise]).then(() => {
- this.reporting.timeEnd(PORTING_COMMENTS_CHANGE_LATENCY_LABEL);
- });
- return commentsPromise;
}
/**
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 8fce39a..fb995b7 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -72,8 +72,9 @@
import {PolymerSpliceChange} from '@polymer/polymer/interfaces';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {ParsedChangeInfo} from '../../shared/gr-rest-api-interface/gr-reviewer-updates-parser';
-import {PatchSetFile} from '../../../types/types';
import {CustomKeyboardEvent} from '../../../types/events';
+import {PatchSetFile} from '../../../types/types';
+import {KnownExperimentId} from '../../../services/flags/flags';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -321,10 +322,14 @@
@property({type: Array})
_dynamicPrependedContentEndpoints?: string[];
+ private _isPortingCommentsExperimentEnabled = false;
+
private readonly reporting = appContext.reportingService;
private readonly restApiService = appContext.restApiService;
+ private readonly flagsService = appContext.flagsService;
+
get keyBindings() {
return {
esc: '_handleEscKey',
@@ -367,6 +372,9 @@
/** @override */
attached() {
super.attached();
+ this._isPortingCommentsExperimentEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.PORTING_COMMENTS
+ );
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
@@ -1544,9 +1552,11 @@
'changeComments, patchRange and diffPrefs must be set'
);
}
+
diffElem.threads = this.changeComments.getThreadsBySideForFile(
file,
- this.patchRange
+ this.patchRange,
+ this._isPortingCommentsExperimentEnabled
);
const promises: Array<Promise<unknown>> = [diffElem.reload()];
if (this._loggedIn && !this.diffPrefs.manual_review) {
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
index e62d481..2c21b4a 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
@@ -127,6 +127,7 @@
</template>
<gr-comment-thread
show-file-path=""
+ show-ported-comment="[[thread.ported]]"
change-num="[[changeNum]]"
comments="[[thread.comments]]"
diff-side="[[thread.diffSide]]"
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 1f84680..d5f1420 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
@@ -28,7 +28,7 @@
RobotCommentInfo,
UrlEncodedCommentId,
NumericChangeId,
- RevisionId,
+ PathToCommentsInfoMap,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {
@@ -43,9 +43,13 @@
UIRobot,
createCommentThreads,
isInPatchRange,
+ isDraftThread,
+ isInBaseOfPatchRange,
+ isInRevisionOfPatchRange,
} from '../../../utils/comment-util';
import {PatchSetFile, PatchNumOnly, isPatchSetFile} from '../../../types/types';
import {appContext} from '../../../services/app-context';
+import {CommentSide, Side} from '../../../constants/constants';
export type CommentIdToCommentThreadMap = {
[urlEncodedCommentId: string]: CommentThread;
@@ -58,6 +62,8 @@
private readonly _drafts: {[path: string]: UIDraft[]};
+ private readonly _portedComments: PathToCommentsInfoMap;
+
/**
* Construct a change comments object, which can be data-bound to child
* elements of that which uses the gr-comment-api.
@@ -65,11 +71,13 @@
constructor(
comments: {[path: string]: UIHuman[]} | undefined,
robotComments: {[path: string]: UIRobot[]} | undefined,
- drafts: {[path: string]: UIDraft[]} | undefined
+ drafts: {[path: string]: UIDraft[]} | undefined,
+ portedComments: PathToCommentsInfoMap | undefined
) {
this._comments = this._addPath(comments);
this._robotComments = this._addPath(robotComments);
this._drafts = this._addPath(drafts);
+ this._portedComments = portedComments || {};
}
/**
@@ -94,18 +102,10 @@
return updatedComments;
}
- get comments() {
- return this._comments;
- }
-
get drafts() {
return this._drafts;
}
- get robotComments() {
- return this._robotComments;
- }
-
findCommentById(commentId?: UrlEncodedCommentId): UIComment | undefined {
if (!commentId) return undefined;
const findComment = (comments: {[path: string]: UIComment[]}) => {
@@ -135,9 +135,9 @@
*/
getPaths(patchRange?: PatchRange): CommentMap {
const responses: {[path: string]: UIComment[]}[] = [
- this.comments,
+ this._comments,
this.drafts,
- this.robotComments,
+ this._robotComments,
];
const commentMap: CommentMap = {};
for (const response of responses) {
@@ -258,6 +258,15 @@
return allComments;
}
+ cloneWithUpdatedDrafts(drafts: {[path: string]: UIDraft[]} | undefined) {
+ return new ChangeComments(
+ this._comments,
+ this._robotComments,
+ drafts,
+ this._portedComments
+ );
+ }
+
/**
* Get the drafts for a path and optional patch num.
*
@@ -289,16 +298,6 @@
return allDrafts;
}
- getThreadsBySideForPath(
- path: string,
- patchRange: PatchRange
- ): CommentThread[] {
- return createCommentThreads(
- this.getCommentsForPath(path, patchRange),
- patchRange
- );
- }
-
/**
* Get the comments (with drafts and robot comments) for a path and
* patch-range. Returns an object with left and right properties mapping to
@@ -313,14 +312,14 @@
let comments: Comment[] = [];
let drafts: DraftInfo[] = [];
let robotComments: RobotCommentInfo[] = [];
- if (this.comments && this.comments[path]) {
- comments = this.comments[path];
+ if (this._comments && this._comments[path]) {
+ comments = this._comments[path];
}
if (this.drafts && this.drafts[path]) {
drafts = this.drafts[path];
}
- if (this.robotComments && this.robotComments[path]) {
- robotComments = this.robotComments[path];
+ if (this._robotComments && this._robotComments[path]) {
+ robotComments = this._robotComments[path];
}
drafts.forEach(d => {
@@ -336,14 +335,94 @@
});
}
- getThreadsBySideForFile(
+ /**
+ * Get the ported threads for given patch range.
+ * Ported threads are comment threads that were posted on an older patchset
+ * and are displayed on a later patchset.
+ * It is simply the original thread displayed on a newer patchset.
+ *
+ * Threads are ported over to all subsequent patchsets. So, a thread created
+ * on patchset 5 say will be ported over to patchsets 6,7,8 and beyond.
+ *
+ * Ported threads add a boolean property ported true to the thread object
+ * to indicate to the user that this is a ported thread.
+ *
+ * Any interactions with ported threads are reflected on the original threads.
+ * Replying to a ported thread ported from Patchset 6 shown on Patchset 10
+ * say creates a draft reply associated with Patchset 6, since the user is
+ * interacting with the original thread.
+ *
+ * Only threads with unresolved comments or drafts are ported over.
+ * If the thread is associated with either the left patchset or the right
+ * patchset, then we filter that ported thread from the return value
+ * as it will be rendered by default.
+ *
+ * If there is no appropriate range for the ported comments, then the backend
+ * does not return the range of the ported thread and it becomes a file level
+ * thread.
+ *
+ * @return only the ported threads for the specified file and patch range
+ */
+ _getPortedCommentThreads(
file: PatchSetFile,
patchRange: PatchRange
): CommentThread[] {
- return createCommentThreads(
+ const portedComments = this._portedComments[file.path];
+ if (file.basePath)
+ portedComments.push(...this._portedComments[file.basePath]);
+ if (!portedComments) return [];
+
+ // when forming threads in diff view, we filter for current patchrange but
+ // ported comments will involve comments that may not belong to the
+ // current patchrange, so we need to form threads for them using all
+ // comments
+ const allComments: UIComment[] = this.getAllCommentsForFile(file, true);
+
+ return createCommentThreads(allComments).filter(thread => {
+ // Robot comments and drafts are not ported over. A human reply to
+ // the robot comment will be ported over, thefore it's possible to
+ // have the root comment of the thread not be ported, hence loop over
+ // entire thread
+ const portedComment = portedComments.find(portedComment =>
+ thread.comments.some(c => portedComment.id === c.id)
+ );
+ if (!portedComment) return false;
+
+ if (
+ isInBaseOfPatchRange(thread.comments[0], patchRange) ||
+ isInRevisionOfPatchRange(thread.comments[0], patchRange)
+ ) {
+ // no need to port this thread as it will be rendered by default
+ return false;
+ }
+
+ // TODO(dhruvsri): Add handling for thread.commentSide = PARENT
+ if (thread.commentSide === CommentSide.PARENT) return false;
+
+ if (!isUnresolved(thread) && !isDraftThread(thread)) return false;
+
+ thread.range = portedComment.range;
+ thread.line = portedComment.line;
+ thread.ported = true;
+ thread.diffSide = Side.RIGHT;
+ return true;
+ });
+ }
+
+ getThreadsBySideForFile(
+ file: PatchSetFile,
+ patchRange: PatchRange,
+ includePortedComments?: boolean
+ ): CommentThread[] {
+ const threads = createCommentThreads(
this.getCommentsForFile(file, patchRange),
patchRange
);
+
+ if (includePortedComments) {
+ threads.push(...this._getPortedCommentThreads(file, patchRange));
+ }
+ return threads;
}
/**
@@ -451,41 +530,36 @@
super.created();
}
- getPortedComments(changeNum: NumericChangeId, revision?: RevisionId) {
- if (!revision) revision = CURRENT;
- return Promise.all([
- this.restApiService.getPortedComments(changeNum, revision),
- this.restApiService.getPortedDrafts(changeNum, revision),
- ]).then(result => {
- return {
- portedComments: result[0],
- portedDrafts: result[1],
- };
- });
- }
-
/**
* Load all comments (with drafts and robot comments) for the given change
* number. The returned promise resolves when the comments have loaded, but
* does not yield the comment data.
*/
- loadAll(changeNum: NumericChangeId) {
- const promises = [];
- promises.push(this.restApiService.getDiffComments(changeNum));
- promises.push(this.restApiService.getDiffRobotComments(changeNum));
- promises.push(this.restApiService.getDiffDrafts(changeNum));
+ loadAll(changeNum: NumericChangeId, patchNum?: PatchSetNum) {
+ const revision = patchNum || CURRENT;
+ const commentsPromise: [
+ Promise<PathToCommentsInfoMap | undefined>,
+ Promise<PathToRobotCommentsInfoMap | undefined>,
+ Promise<PathToCommentsInfoMap | undefined>,
+ Promise<PathToCommentsInfoMap | undefined>
+ ] = [
+ this.restApiService.getDiffComments(changeNum),
+ this.restApiService.getDiffRobotComments(changeNum),
+ this.restApiService.getDiffDrafts(changeNum),
+ this.restApiService.getPortedComments(changeNum, revision),
+ ];
- return Promise.all(promises).then(([comments, robotComments, drafts]) => {
- this._changeComments = new ChangeComments(
- comments,
- // TODO(TS): Promise.all somehow resolve all types to
- // PathToCommentsInfoMap given its PathToRobotCommentsInfoMap
- // returned from the second promise
- robotComments as PathToRobotCommentsInfoMap,
- drafts
- );
- return this._changeComments;
- });
+ return Promise.all(commentsPromise).then(
+ ([comments, robotComments, drafts, portedComments]) => {
+ this._changeComments = new ChangeComments(
+ comments,
+ robotComments,
+ drafts,
+ portedComments
+ );
+ return this._changeComments;
+ }
+ );
}
/**
@@ -497,11 +571,8 @@
if (!this._changeComments) {
return this.loadAll(changeNum);
}
- const oldChangeComments = this._changeComments;
return this.restApiService.getDiffDrafts(changeNum).then(drafts => {
- this._changeComments = new ChangeComments(
- oldChangeComments.comments,
- (oldChangeComments.robotComments as unknown) as PathToRobotCommentsInfoMap,
+ this._changeComments = this._changeComments!.cloneWithUpdatedDrafts(
drafts
);
return this._changeComments;
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
index 6b6756e..f724f41 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
@@ -19,7 +19,7 @@
import './gr-comment-api.js';
import {ChangeComments} from './gr-comment-api.js';
import {CommentSide} from '../../../constants/constants.js';
-import {isInRevisionOfPatchRange, isInBaseOfPatchRange} from '../../../utils/comment-util.js';
+import {isInRevisionOfPatchRange, isInBaseOfPatchRange, createCommentThreads} from '../../../utils/comment-util.js';
const basicFixture = fixtureFromElement('gr-comment-api');
@@ -147,7 +147,142 @@
});
});
- test('isInBaseOfPatchRange', () => {
+ suite('ported comments', () => {
+ let portedComments;
+ let changeComments;
+ setup(() => {
+ portedComments = {
+ 'karma.conf.js': [
+ {
+ unresolved: true,
+ patch_set: 4,
+ id: 'db977012_e1f13818',
+ line: 136,
+ range: {
+ start_line: 136,
+ start_character: 16,
+ end_line: 136,
+ end_character: 29,
+ },
+ updated: '2020-11-19 12:06:48.000000000',
+ },
+ ],
+ };
+
+ changeComments = new ChangeComments(
+ {
+ 'karma.conf.js': [
+ {
+ // resolved comment that will not be ported over
+ patch_set: 2,
+ id: 'ecf0b9fa_fe1a5f62',
+ line: 5,
+ updated: '2018-02-08 18:49:18.000000000',
+ unresolved: false,
+ },
+ {
+ unresolved: true,
+ // original comment that will be ported over to patchset 4
+ patch_set: 2,
+ id: 'db977012_e1f13818',
+ line: 136,
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 1,
+ end_character: 1,
+ },
+ updated: '2020-11-19 12:06:48.000000000',
+ },
+ ],
+ },
+ {},
+ {},
+ portedComments
+ );
+ });
+
+ test('threads containing ported comment are returned', () => {
+ assert.equal(changeComments.getAllThreadsForChange().length,
+ 2);
+
+ const portedThreads = changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'});
+
+ assert.equal(portedThreads.length, 1);
+ // check range of thread is from the ported comment and not the original
+ assert.deepEqual(portedThreads[0].range, {
+ start_line: 136,
+ start_character: 16,
+ end_line: 136,
+ end_character: 29,
+ });
+
+ // thread ported over if comparing patchset 1 vs patchset 4
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 1}
+ ).length, 1);
+
+ // verify ported thread is not returned if original thread will be
+ // shown
+ // original thread attached to right side
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 2, basePatchNum: 'PARENT'}
+ ).length, 0);
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 2, basePatchNum: 1}
+ ).length, 0);
+
+ // original thread attached to left side
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 3, basePatchNum: 2}
+ ).length, 0);
+ });
+
+ test('threads without any ported comment are filtered out', () => {
+ changeComments = new ChangeComments(
+ {
+ // comment that is not ported over
+ 'karma.conf.js': [
+ {
+ patch_set: 2,
+ id: 'ecf0b9fa_fe1a5f62',
+ line: 5,
+ updated: '2018-02-08 18:49:18.000000000',
+ unresolved: false,
+ },
+ ],
+ },
+ {},
+ {
+ 'karma.conf.js': [
+ // comment that is ported over but does not have any thread
+ // that has a comment that matches it
+ {
+ id: '503008e2_0ab203ee',
+ path: 'karma.conf.js',
+ line: 5,
+ in_reply_to: 'ecf0b9fa_fe1a5f62',
+ updated: '2018-02-13 22:48:48.018000000',
+ unresolved: true,
+ __draft: true,
+ __draftID: '0.m683trwff68',
+ patch_set: 2,
+ },
+ ],
+ },
+ portedComments
+ );
+
+ assert.equal(createCommentThreads(changeComments
+ .getAllCommentsForPath('karma.conf.js')).length, 1);
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'}
+ ).length, 0);
+ });
+ });
+
+ test('_isInBaseOfPatchRange', () => {
const comment = {patch_set: 1};
const patchRange = {basePatchNum: 1, patchNum: 2};
assert.isTrue(isInBaseOfPatchRange(comment,
@@ -296,7 +431,7 @@
],
};
element._changeComments =
- new ChangeComments(comments, robotComments, drafts, 1234);
+ new ChangeComments(comments, robotComments, drafts, {}, 1234);
});
test('getPaths', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 32c9964..d084457 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -779,6 +779,7 @@
threadEl.changeNum = this.changeNum;
threadEl.patchNum = thread.patchNum;
threadEl.showPatchset = false;
+ threadEl.showPortedComment = !!thread.ported;
// GrCommentThread does not understand 'FILE', but requires undefined.
threadEl.lineNum = thread.line !== 'FILE' ? thread.line : undefined;
threadEl.projectName = this.projectName;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index 9bde122..2c050f9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -24,6 +24,7 @@
import {Side, CommentSide} from '../../../constants/constants.js';
import {createChange} from '../../../test/test-data-generators.js';
import {FILE} from '../gr-diff/gr-diff-line.js';
+import {CoverageType} from '../../../types/types.js';
const basicFixture = fixtureFromElement('gr-diff-host');
@@ -1003,14 +1004,14 @@
assert.equal(actualThreads.length, 2);
- assert.equal(actualThreads[0].diffSide, 'left');
+ assert.equal(actualThreads[0].diffSide, Side.LEFT);
assert.equal(actualThreads[0].comments.length, 2);
assert.deepEqual(actualThreads[0].comments[0], comments[0]);
assert.deepEqual(actualThreads[0].comments[1], comments[1]);
assert.equal(actualThreads[0].patchNum, 1);
assert.equal(actualThreads[0].line, 1);
- assert.equal(actualThreads[1].diffSide, 'left');
+ assert.equal(actualThreads[1].diffSide, Side.LEFT);
assert.equal(actualThreads[1].comments.length, 1);
assert.deepEqual(actualThreads[1].comments[0], comments[2]);
assert.equal(actualThreads[1].patchNum, 1);
@@ -1035,7 +1036,7 @@
const expectedThreads = [
{
- diffSide: 'left',
+ diffSide: Side.LEFT,
commentSide: CommentSide.REVISION,
path: '/p',
rootId: 'betsys_confession',
@@ -1090,7 +1091,7 @@
});
test('_getOrCreateThread', () => {
- const diffSide = 'left';
+ const diffSide = Side.LEFT;
const commentSide = CommentSide.PARENT;
assert.isOk(element._getOrCreateThread('2', 3,
@@ -1126,7 +1127,7 @@
test('thread should use old file path if first created ' +
'on patch set (left) before renaming', () => {
- const diffSide = 'left';
+ const diffSide = Side.LEFT;
element.file = {basePath: 'file_renamed.txt', path: element.path};
assert.isOk(element._getOrCreateThread('2', 3,
@@ -1142,7 +1143,7 @@
test('thread should use new file path if first created' +
'on patch set (right) after renaming', () => {
- const diffSide = 'right';
+ const diffSide = Side.RIGHT;
element.file = {basePath: 'file_renamed.txt', path: element.path};
assert.isOk(element._getOrCreateThread('2', 3,
@@ -1158,7 +1159,7 @@
test('thread should use new file path if first created' +
'on patch set (left) but is base', () => {
- const diffSide = 'left';
+ const diffSide = Side.LEFT;
element.file = {basePath: 'file_renamed.txt', path: element.path};
assert.isOk(element._getOrCreateThread('2', 3,
@@ -1188,19 +1189,19 @@
const l3 = document.createElement('div');
l3.setAttribute('line-num', 3);
- l3.setAttribute('diff-side', 'left');
+ l3.setAttribute('diff-side', Side.LEFT);
const l5 = document.createElement('div');
l5.setAttribute('line-num', 5);
- l5.setAttribute('diff-side', 'left');
+ l5.setAttribute('diff-side', Side.LEFT);
const r3 = document.createElement('div');
r3.setAttribute('line-num', 3);
- r3.setAttribute('diff-side', 'right');
+ r3.setAttribute('diff-side', Side.RIGHT);
const r5 = document.createElement('div');
r5.setAttribute('line-num', 5);
- r5.setAttribute('diff-side', 'right');
+ r5.setAttribute('diff-side', Side.RIGHT);
const threadEls = [l3, l5, r3, r5];
assert.deepEqual(element._filterThreadElsForLocation(threadEls, line,
@@ -1213,11 +1214,11 @@
const line = {beforeNumber: 'FILE', afterNumber: 'FILE'};
const l = document.createElement('div');
- l.setAttribute('diff-side', 'left');
+ l.setAttribute('diff-side', Side.LEFT);
l.setAttribute('line-num', 'FILE');
const r = document.createElement('div');
- r.setAttribute('diff-side', 'right');
+ r.setAttribute('diff-side', Side.RIGHT);
r.setAttribute('line-num', 'FILE');
const threadEls = [l, r];
@@ -1320,31 +1321,37 @@
suite('coverage layer', () => {
let notifyStub;
+ let coverageProviderStub;
+ const exampleRanges = [
+ {
+ type: CoverageType.COVERED,
+ side: Side.RIGHT,
+ code_range: {
+ start_line: 1,
+ end_line: 2,
+ },
+ },
+ {
+ type: CoverageType.NOT_COVERED,
+ side: Side.RIGHT,
+ code_range: {
+ start_line: 3,
+ end_line: 4,
+ },
+ },
+ ];
+
setup(() => {
notifyStub = sinon.stub();
+ coverageProviderStub = sinon.stub().returns(
+ Promise.resolve(exampleRanges));
+
stub('gr-js-api-interface', {
getCoverageAnnotationApis() {
return Promise.resolve([{
notify: notifyStub,
getCoverageProvider() {
- return () => Promise.resolve([
- {
- type: 'COVERED',
- side: 'right',
- code_range: {
- start_line: 1,
- end_line: 2,
- },
- },
- {
- type: 'NOT_COVERED',
- side: 'right',
- code_range: {
- start_line: 3,
- end_line: 4,
- },
- },
- ]);
+ return coverageProviderStub;
},
}]);
},
@@ -1380,9 +1387,38 @@
element.reload();
flush(() => {
assert.equal(notifyStub.callCount, 2);
+ assert.isTrue(notifyStub.calledWithExactly(
+ 'some/path', 1, 2, Side.RIGHT));
+ assert.isTrue(notifyStub.calledWithExactly(
+ 'some/path', 3, 4, Side.RIGHT));
done();
});
});
+
+ test('provider is called with appropriate params', done => {
+ element.patchRange.basePatchNum = 1;
+ element.patchRange.patchNum = 3;
+
+ element.reload();
+ flush(() => {
+ assert.isTrue(coverageProviderStub.calledWithExactly(
+ 123, 'some/path', 1, 3, element.change));
+ done();
+ });
+ });
+
+ test('provider is called with appropriate params - special patchset values',
+ done => {
+ element.patchRange.basePatchNum = 'PARENT';
+ element.patchRange.patchNum = 'invalid';
+
+ element.reload();
+ flush(() => {
+ assert.isTrue(coverageProviderStub.calledWithExactly(
+ 123, 'some/path', undefined, undefined, element.change));
+ done();
+ });
+ });
});
suite('trailing newlines', () => {
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 a6f4717..853d3c6 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
@@ -47,7 +47,6 @@
computeLatestPatchNum,
patchNumEquals,
PatchSet,
- CURRENT,
} from '../../../utils/patch-set-util';
import {
addUnmodifiedFiles,
@@ -95,9 +94,9 @@
import {CommentMap, isInBaseOfPatchRange} from '../../../utils/comment-util';
import {AppElementParams} from '../../gr-app-types';
import {CustomKeyboardEvent, OpenFixPreviewEvent} from '../../../types/events';
-import {PORTING_COMMENTS_DIFF_LATENCY_LABEL} from '../../../services/gr-reporting/gr-reporting';
import {fireAlert, fireTitleChange} from '../../../utils/event-util';
+import {KnownExperimentId} from '../../../services/flags/flags';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
const MSG_LOADED_BLAME = 'Blame loaded';
@@ -264,6 +263,8 @@
@property({type: Number})
_focusLineNum?: number;
+ private _isPortingCommentsExperimentEnabled = false;
+
get keyBindings() {
return {
esc: '_handleEscKey',
@@ -343,6 +344,9 @@
this.$.cursor.reInitCursor();
};
this.$.diffHost.addEventListener('render', this._onRenderHandler);
+ this._isPortingCommentsExperimentEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.PORTING_COMMENTS
+ );
}
/** @override */
@@ -1047,11 +1051,6 @@
return;
}
- const portedCommentsPromise = this.$.commentAPI.getPortedComments(
- value.changeNum,
- value.patchNum || CURRENT
- );
-
const promises: Promise<unknown>[] = [];
promises.push(this._getDiffPreferences());
@@ -1063,7 +1062,7 @@
);
promises.push(this._getChangeDetail(this._changeNum));
- promises.push(this._loadComments());
+ promises.push(this._loadComments(value.patchNum));
promises.push(this._getChangeEdit());
@@ -1072,19 +1071,22 @@
this._loading = true;
return Promise.all(promises)
.then(r => {
- this.reporting.time(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
this._loading = false;
this._initPatchRange();
this._initCommitRange();
- if (this._changeComments && this._path && this._patchRange) {
- this.$.diffHost.threads = this._changeComments.getThreadsBySideForPath(
- this._path,
- this._patchRange
- );
- }
- portedCommentsPromise.then(() => {
- this.reporting.timeEnd(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
- });
+
+ if (!this._path) throw new Error('path must be defined');
+ if (!this._changeComments)
+ throw new Error('change comments must be defined');
+ if (!this._patchRange) throw new Error('patch range must be defined');
+
+ // TODO(dhruvsri): check if basePath should be set here
+ this.$.diffHost.threads = this._changeComments.getThreadsBySideForFile(
+ {path: this._path},
+ this._patchRange,
+ this._isPortingCommentsExperimentEnabled
+ );
+
const edit = r[4] as EditInfo | undefined;
if (edit) {
this.set(`_change.revisions.${edit.commit.commit}`, {
@@ -1542,11 +1544,13 @@
return url;
}
- _loadComments() {
+ _loadComments(patchSet?: PatchSetNum) {
if (!this._changeNum) throw new Error('Missing this._changeNum');
- return this.$.commentAPI.loadAll(this._changeNum).then(comments => {
- this._changeComments = comments;
- });
+ return this.$.commentAPI
+ .loadAll(this._changeNum, patchSet)
+ .then(comments => {
+ this._changeComments = comments;
+ });
}
@observe('_files.changeFilesByPath', '_path', '_patchRange', '_projectConfig')
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 a8d3abd..1cb22e8 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
@@ -23,7 +23,6 @@
import {SPECIAL_PATCH_SET_NUM} from '../../../utils/patch-set-util.js';
import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
import {_testOnly_findCommentById} from '../gr-comment-api/gr-comment-api.js';
-import {appContext} from '../../../services/app-context.js';
import {GerritView} from '../../core/gr-navigation/gr-navigation.js';
import {
createChange,
@@ -90,7 +89,6 @@
setup(async () => {
clock = sinon.useFakeTimers();
- sinon.stub(appContext.flagsService, 'isEnabled').returns(true);
stub('gr-rest-api-interface', {
getConfig() {
return Promise.resolve({change: {}});
@@ -119,6 +117,9 @@
getDiffDrafts() {
return Promise.resolve({});
},
+ getPortedComments() {
+ return Promise.resolve({});
+ },
getReviewedFiles() {
return Promise.resolve([]);
},
@@ -151,8 +152,7 @@
computeCommentThreadCount: () => {},
computeUnresolvedNum: () => {},
getPaths: () => {},
- getThreadsBySideForPath: () => {},
- getThreadsBySideForFile: () => {},
+ getThreadsBySideForFile: () => [],
findCommentById: _testOnly_findCommentById,
}));
@@ -178,6 +178,8 @@
basePatchNum: 1,
path: '/COMMIT_MSG',
};
+ element._path = '/COMMIT_MSG';
+ element._patchRange = {};
return element._paramsChanged.returnValues[0].then(() => {
assert.isTrue(element.reporting.diffViewDisplayed.calledOnce);
});
@@ -235,6 +237,8 @@
basePatchNum: 1,
path: '/COMMIT_MSG',
};
+ element._path = '/COMMIT_MSG';
+ element._patchRange = {};
return element._paramsChanged.returnValues[0].then(() => {
assert.isTrue(element._isBlameLoaded);
assert.isTrue(element._loadBlame.calledOnce);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
index eebcb15..038153a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
@@ -83,7 +83,7 @@
// For Gerrit these are instances of GrCommentThread, but other gr-diff users
// have different HTML elements in use for comment threads.
// TODO: Also document the required HTML attritbutes that thread elements must
-// have, e.g. 'comment-side', 'range', 'line-num', 'data-value'.
+// have, e.g. 'diff-side', 'range', 'line-num', 'data-value'.
export interface GrDiffThreadElement extends HTMLElement {
rootId: string;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index 2c8b704..557d1eb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -552,9 +552,9 @@
/** Make comments selectable when selected */
.selected-left.selected-comment
- ::slotted(gr-comment-thread[comment-side='left']),
+ ::slotted(gr-comment-thread[diff-side='left']),
.selected-right.selected-comment
- ::slotted(gr-comment-thread[comment-side='right']) {
+ ::slotted(gr-comment-thread[diff-side='right']) {
-webkit-user-select: text;
-moz-user-select: text;
-ms-user-select: text;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 0e0ee0c..1124cae 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -172,6 +172,9 @@
showFileName = true;
@property({type: Boolean})
+ showPortedComment = false;
+
+ @property({type: Boolean})
showPatchset = true;
get keyBindings() {
@@ -287,6 +290,11 @@
return path === SpecialFilePath.PATCHSET_LEVEL_COMMENTS;
}
+ _computeShowPortedComment(comment: UIComment) {
+ if (this._orderedComments.length === 0) return false;
+ return this.showPortedComment && comment.id === this._orderedComments[0].id;
+ }
+
_computeDisplayPath(path: string) {
const displayPath = computeDisplayPath(path);
if (displayPath === SpecialFilePath.PATCHSET_LEVEL_COMMENTS) {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
index 64be155..f5d794f 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
@@ -113,10 +113,12 @@
comments="{{comments}}"
robot-button-disabled="[[_shouldDisableAction(_showActions, _lastComment)]]"
change-num="[[changeNum]]"
+ project-name="[[projectName]]"
patch-num="[[patchNum]]"
draft="[[_isDraft(comment)]]"
show-actions="[[_showActions]]"
show-patchset="[[showPatchset]]"
+ show-ported-comment="[[_computeShowPortedComment(comment)]]"
side="[[comment.side]]"
project-config="[[_projectConfig]]"
on-create-fix-comment="_handleCommentFix"
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index b2dcb88..3dd851e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -38,6 +38,7 @@
import {getRootElement} from '../../../scripts/rootElement';
import {appContext} from '../../../services/app-context';
import {customElement, observe, property} from '@polymer/decorators';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {GrTextarea} from '../gr-textarea/gr-textarea';
import {GrStorage, StorageLocation} from '../gr-storage/gr-storage';
import {GrOverlay} from '../gr-overlay/gr-overlay';
@@ -46,6 +47,7 @@
NumericChangeId,
ConfigInfo,
PatchSetNum,
+ RepoName,
} from '../../../types/common';
import {GrButton} from '../gr-button/gr-button';
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
@@ -152,6 +154,9 @@
@property({type: Number})
changeNum?: NumericChangeId;
+ @property({type: String})
+ projectName?: RepoName;
+
@property({type: Object, notify: true, observer: '_commentChanged'})
comment?: UIComment;
@@ -254,6 +259,9 @@
@property({type: Object})
_selfAccount?: AccountDetailInfo;
+ @property({type: Boolean})
+ showPortedComment = false;
+
get keyBindings() {
return {
'ctrl+enter meta+enter ctrl+s meta+s': '_handleSaveKey',
@@ -294,6 +302,16 @@
return comment.author || this._selfAccount;
}
+ _getUrlForComment(comment: UIComment) {
+ if (!this.changeNum || !this.projectName) return '';
+ if (!comment.id) throw new Error('comment must have an id');
+ return GerritNav.getUrlForComment(
+ this.changeNum as NumericChangeId,
+ this.projectName,
+ comment.id
+ );
+ }
+
@observe('editing')
_onEditingChange(editing?: boolean) {
this.dispatchEvent(
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
index 64f0be1..6bf00a8 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
@@ -260,6 +260,11 @@
>
</gr-account-label>
</template>
+ <template is="dom-if" if="[[showPortedComment]]">
+ <a href="[[_getUrlForComment(comment)]]">
+ Ported from patchset [[comment.patch_set]]
+ </a>
+ </template>
<gr-tooltip-content
class="draftTooltip"
has-tooltip=""
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
index cd701fc..68a73933 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
@@ -20,7 +20,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-overlay_html';
import {IronOverlayMixin} from '../../../mixins/iron-overlay-mixin/iron-overlay-mixin';
-import {customElement, property} from '@polymer/decorators';
+import {customElement} from '@polymer/decorators';
import {IronOverlayBehavior} from '@polymer/iron-overlay-behavior/iron-overlay-behavior';
import {findActiveElement} from '../../../utils/dom-util';
import {fireEvent} from '../../../utils/event-util';
@@ -56,7 +56,6 @@
* @event fullscreen-overlay-opened
*/
- @property({type: Boolean})
private _fullScreenOpen = false;
private _boundHandleClose: () => void = () => super.close();
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index ba33954..36fe858 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -28,4 +28,5 @@
NEW_CONTEXT_CONTROLS = 'UiFeature__new_context_controls',
CI_REBOOT_CHECKS = 'UiFeature__ci_reboot_checks',
NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
+ PORTING_COMMENTS = 'UiFeature__porting_comments',
}
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
index 48c90f7..090c42f 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
@@ -21,10 +21,6 @@
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type EventDetails = any;
-export const PORTING_COMMENTS_DIFF_LATENCY_LABEL = 'PortingCommentsDiffLatency';
-export const PORTING_COMMENTS_CHANGE_LATENCY_LABEL =
- 'PortingCommentsChangeLatency';
-
export interface Timer {
reset(): this;
end(): this;
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index b454b6e..1628134 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1175,11 +1175,6 @@
export type PathToCommentsInfoMap = {[path: string]: CommentInfo[]};
-export type PortedCommentsAndDrafts = {
- portedComments?: PathToCommentsInfoMap;
- portedDrafts?: PathToCommentsInfoMap;
-};
-
/**
* The CommentRange entity describes the range of an inline comment.
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#comment-range
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 0726f67..8627591 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -158,6 +158,7 @@
rootId?: UrlEncodedCommentId;
diffSide?: Side;
range?: CommentRange;
+ ported?: boolean; // is the comment ported over from a previous patchset
}
export function getLastComment(thread?: CommentThread): UIComment | undefined {
