Add latency tracking for porting comments
I2c774cfe6 adds support for porting unresolved comments across
patchsets.
Add latency tracking for the API to determine if it is feasible
to render porting comments along with normal comments, not displaying
any comments until the API returns.
Change-Id: Icb88045e262dc4aecda1efd819a1ef9fcd891a1e
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 8504df4..1119a20 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
@@ -151,6 +151,7 @@
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';
const CHANGE_ID_ERROR = {
MISMATCH: 'mismatch',
@@ -2127,9 +2128,20 @@
this._robotCommentThreads = undefined;
if (!this._changeNum)
throw new Error('missing required changeNum property');
- return this.$.commentAPI
+
+ const portedCommentsPromise = this.$.commentAPI.getPortedComments(
+ this._changeNum
+ );
+ const commentsPromise = this.$.commentAPI
.loadAll(this._changeNum)
- .then(comments => this._recomputeComments(comments));
+ .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/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 0514553..8dce63f 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
@@ -35,6 +35,7 @@
RobotCommentInfo,
UrlEncodedCommentId,
NumericChangeId,
+ RevisionId,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {CommentSide} from '../../../constants/constants';
@@ -611,6 +612,19 @@
);
}
+ getPortedComments(changeNum: NumericChangeId, revision?: RevisionId) {
+ if (!revision) revision = 'current';
+ return Promise.all([
+ this.$.restAPI.getPortedComments(changeNum, revision),
+ this.$.restAPI.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
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 4bde136..f075d00 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
@@ -88,6 +88,7 @@
PreferencesInfo,
RepoName,
RevisionInfo,
+ PortedCommentsAndDrafts,
} from '../../../types/common';
import {ChangeViewState, CommitRange, FileRange} from '../../../types/types';
import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select';
@@ -101,6 +102,7 @@
import {CommentMap} 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';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
@@ -1060,6 +1062,16 @@
return;
}
+ let portedCommentsPromise: Promise<PortedCommentsAndDrafts>;
+ let portedCommentsPatchNum: PatchSetNum;
+ if (value.changeNum && value.patchNum) {
+ portedCommentsPatchNum = value.patchNum;
+ portedCommentsPromise = this.$.commentAPI.getPortedComments(
+ value.changeNum,
+ value.patchNum
+ );
+ }
+
const promises: Promise<unknown>[] = [];
promises.push(this._getDiffPreferences());
@@ -1080,10 +1092,30 @@
this._loading = true;
return Promise.all(promises)
.then(r => {
+ this.reporting.time(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
this._loading = false;
this._initPatchRange();
this._initCommitRange();
this.$.diffHost.comments = this._commentsForDiff;
+ if (!portedCommentsPromise) {
+ // _initPatchRange() ensures _patchRange is set
+ // AppElementDiffViewParam ensures _changeNum is set
+ portedCommentsPatchNum = this._patchRange!.patchNum;
+ value.changeNum = this._changeNum!;
+ portedCommentsPromise = this.$.commentAPI.getPortedComments(
+ this._changeNum!,
+ this._patchRange!.patchNum
+ );
+ }
+ portedCommentsPromise.then(() => {
+ // do not report latency if user has changed patchsets during request
+ if (
+ !patchNumEquals(portedCommentsPatchNum, this._patchRange?.patchNum)
+ ) {
+ return;
+ }
+ this.reporting.timeEnd(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
+ });
const edit = r[4] as EditInfo | undefined;
if (edit) {
this.set(`_change.revisions.${edit.commit.commit}`, {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index 7d08560..8e90c38 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -2795,6 +2795,28 @@
return this._changeBaseURL(changeNum, patchNum).then(url => url + endpoint);
}
+ getPortedComments(
+ changeNum: NumericChangeId,
+ revision: RevisionId
+ ): Promise<PathToCommentsInfoMap | undefined> {
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/ported_comments/',
+ revision,
+ });
+ }
+
+ getPortedDrafts(
+ changeNum: NumericChangeId,
+ revision: RevisionId
+ ): Promise<PathToCommentsInfoMap | undefined> {
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/ported_drafts/',
+ revision,
+ });
+ }
+
saveDiffDraft(
changeNum: NumericChangeId,
patchNum: PatchSetNum,
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
index 743e0f4..e6139e5 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
@@ -20,6 +20,10 @@
// TODO(dmfilippov): TS-fix-any use more specific type instead if possible
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/services/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
index 950619b..08dbb16 100644
--- a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
@@ -457,6 +457,16 @@
cancelCondition?: CancelConditionCallback
): Promise<ChangeInfo | undefined | null>;
+ getPortedComments(
+ changeNum: NumericChangeId,
+ revision: RevisionId
+ ): Promise<PathToCommentsInfoMap | undefined>;
+
+ getPortedDrafts(
+ changeNum: NumericChangeId,
+ revision: RevisionId
+ ): Promise<PathToCommentsInfoMap | undefined>;
+
getDiffComments(
changeNum: NumericChangeId
): Promise<PathToCommentsInfoMap | undefined>;
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 1d1d32e..6b184b9 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1184,6 +1184,11 @@
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
