Use LineNumber type for CommentThread interface
LineNumber type specifies when it's a FILE level thread instead
by setting value "FILE".
Change-Id: I37d4cf37f2596b693b8f76af7f4173920f21eff4
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 367bbd6..bbe99b9 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
@@ -520,12 +520,16 @@
comments: [comment],
patchNum: comment.patch_set,
path: comment.__path || comment.path!,
- line: comment.line,
rootId: comment.id,
+ line: comment.line,
+ range: comment.range,
};
if (comment.side) {
newThread.commentSide = comment.side;
}
+ if (!comment.line && !comment.range) {
+ newThread.line = 'FILE';
+ }
threads.push(newThread);
idThreadMap[comment.id] = newThread;
}
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 be6f646..d9b6b53 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
@@ -543,6 +543,12 @@
path: 'file/one',
line: 1,
rootId: '01',
+ range: {
+ start_line: 1,
+ start_character: 2,
+ end_line: 2,
+ end_character: 2,
+ },
}, {
comments: [
{
@@ -559,6 +565,7 @@
patchNum: 2,
path: 'file/one',
line: 2,
+ range: undefined,
rootId: '03',
}, {
comments: [
@@ -595,6 +602,7 @@
path: 'file/one',
line: 1,
rootId: '04',
+ range: undefined,
}, {
comments: [
{
@@ -610,6 +618,7 @@
path: 'file/two',
line: 2,
rootId: '05',
+ range: undefined,
}, {
comments: [
{
@@ -625,6 +634,7 @@
path: 'file/two',
line: 2,
rootId: '06',
+ range: undefined,
}, {
comments: [
{
@@ -654,6 +664,7 @@
path: 'file/three',
line: 1,
rootId: '07',
+ range: undefined,
}, {
comments: [
{
@@ -669,6 +680,7 @@
path: 'file/three',
line: 1,
rootId: '09',
+ range: undefined,
}, {
comments: [
{
@@ -686,6 +698,7 @@
path: 'file/four',
line: 1,
rootId: '10',
+ range: undefined,
}, {
comments: [
{
@@ -701,6 +714,7 @@
patchNum: 5,
path: 'file/four',
line: 1,
+ range: undefined,
}, {
comments: [
{
@@ -717,6 +731,7 @@
patchNum: 3,
path: 'file/two',
line: 1,
+ range: undefined,
}, {
comments: [
{
@@ -735,6 +750,7 @@
patchNum: 2,
path: 'file/one',
line: 1,
+ range: undefined,
},
];
const threads = element._changeComments.getAllThreadsForChange();
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 b2dde49..f0bcfa7 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
@@ -68,7 +68,7 @@
} from '../../../constants/constants';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select';
-import {LineNumber} from '../gr-diff/gr-diff-line';
+import {LineNumber, FILE} from '../gr-diff/gr-diff-line';
import {GrCommentThread} from '../../shared/gr-comment-thread/gr-comment-thread';
import {PatchSetFile} from '../../../types/types';
import {KnownExperimentId} from '../../../services/flags/flags';
@@ -913,9 +913,7 @@
function matchesFileComment(threadEl: GrCommentThread) {
return (
threadEl.getAttribute('comment-side') === side &&
- // line/range comments have 1-based line set, if line is falsy it's
- // a file comment
- !threadEl.getAttribute('line-num')
+ threadEl.getAttribute('line-num') === FILE
);
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index a61a310..22ccffe 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -419,7 +419,7 @@
for (const threadEl of threadEls) {
const side = getSide(threadEl);
- const lineNum = Number(threadEl.getAttribute('line-num')) || FILE;
+ const lineNum = threadEl.getAttribute('line-num') || FILE;
const commentRange = threadEl.range || {};
keyLocations[side][lineNum] = true;
// Add start_line as well if exists,
@@ -568,12 +568,7 @@
return;
}
- // TODO(TS): existing logic always pass undefined lineNum
- // for file level comment, the drafts API will reject the
- // request if file level draft contains the `line: 'FILE'` field
- // probably should do this inside of the _createComment, this
- // is just to keep existing behavior.
- this._createComment(el, lineNum === FILE ? undefined : lineNum);
+ this._createComment(el, lineNum);
}
createRangeComment() {
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 878c01c..4bad587 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
@@ -50,6 +50,7 @@
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {GrStorage, StorageLocation} from '../gr-storage/gr-storage';
import {CustomKeyboardEvent} from '../../../types/events';
+import {LineNumber, FILE} from '../../diff/gr-diff/gr-diff-line';
const UNRESOLVED_EXPAND_COUNT = 5;
const NEWLINE_PATTERN = /\n/g;
@@ -88,7 +89,7 @@
* diff widget like gr-diff to show the thread in the right location:
*
* line-num:
- * 1-based line number or undefined if it refers to the entire file.
+ * 1-based line number or 'FILE' if it refers to the entire file.
*
* comment-side:
* "left" or "right". These indicate which of the two diffed versions
@@ -146,8 +147,8 @@
@property({type: Boolean})
showFilePath = false;
- @property({type: Number, reflectToAttribute: true})
- lineNum?: number;
+ @property({type: Object, reflectToAttribute: true})
+ lineNum?: LineNumber;
@property({type: Boolean, notify: true, reflectToAttribute: true})
unresolved?: boolean;
@@ -200,7 +201,7 @@
this._setInitialExpandedState();
}
- addOrEditDraft(lineNum?: number, rangeParam?: CommentRange) {
+ addOrEditDraft(lineNum?: LineNumber, rangeParam?: CommentRange) {
const lastComment = this.comments[this.comments.length - 1] || {};
if (isDraft(lastComment)) {
const commentEl = this._commentElWithDraftID(
@@ -223,7 +224,7 @@
}
}
- addDraft(lineNum?: number, range?: CommentRange, unresolved?: boolean) {
+ addDraft(lineNum?: LineNumber, range?: CommentRange, unresolved?: boolean) {
const draft = this._newDraft(lineNum, range);
draft.__editing = true;
draft.unresolved = unresolved === false ? unresolved : true;
@@ -272,7 +273,7 @@
path,
patchNum,
undefined,
- this.lineNum
+ this.lineNum === FILE ? undefined : this.lineNum
);
}
const id = this.comments[0].id;
@@ -293,14 +294,14 @@
}
_computeDisplayLine() {
- if (this.lineNum) return `#${this.lineNum}`;
- // If range is set, then lineNum equals the end line of the range.
- if (!this.lineNum && !this.range) {
+ if (this.lineNum === FILE) {
if (this.path === SpecialFilePath.PATCHSET_LEVEL_COMMENTS) {
return '';
}
- return 'FILE';
+ return FILE;
}
+ if (this.lineNum) return `#${this.lineNum}`;
+ // If range is set, then lineNum equals the end line of the range.
if (this.range) return `#${this.range.end_line}`;
return '';
}
@@ -490,7 +491,7 @@
return d;
}
- _newDraft(lineNum?: number, range?: CommentRange) {
+ _newDraft(lineNum?: LineNumber, range?: CommentRange) {
const d: UIDraft = {
__draft: true,
__draftID: Math.random().toString(36),
@@ -516,7 +517,7 @@
d.side = this._getSide(this.isOnParent);
d.__commentSide = this.commentSide;
- if (lineNum) {
+ if (lineNum && lineNum !== FILE) {
d.line = lineNum;
}
if (range) {
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 5f8aa82..e1e002a 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -21,9 +21,11 @@
RobotCommentInfo,
Timestamp,
UrlEncodedCommentId,
+ CommentRange,
} from '../types/common';
import {CommentSide, Side} from '../constants/constants';
import {parseDate} from './date-util';
+import {LineNumber} from '../elements/diff/gr-diff/gr-diff-line';
export interface DraftCommentProps {
__draft?: boolean;
@@ -101,13 +103,10 @@
comments: UIComment[];
patchNum?: PatchSetNum;
path: string;
- // TODO(TS): It would be nice to use LineNumber here, but the comment thread
- // element actually relies on line to be undefined for file comments. Be
- // aware of element attribute getters and setters, if you try to refactor
- // this. :-) Still worthwhile to do ...
- line?: number;
+ line?: LineNumber;
rootId: UrlEncodedCommentId;
commentSide?: CommentSide;
+ range?: CommentRange;
}
export function getLastComment(thread?: CommentThread): UIComment | undefined {
