Merge "Fix handling of ported comments (broken after big comment refactor)"
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 50399be..a08bf39 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
@@ -33,8 +33,6 @@
   createCommentThreads,
   isInPatchRange,
   isDraftThread,
-  isInBaseOfPatchRange,
-  isInRevisionOfPatchRange,
   isPatchsetLevel,
   addPath,
 } from '../../../utils/comment-util';
@@ -338,20 +336,8 @@
         comment => comment.id === portedComment.id
       )!;
 
-      if (
-        (originalComment.line && !portedComment.line) ||
-        (originalComment.range && !portedComment.range)
-      ) {
-        thread.rangeInfoLost = true;
-      }
-
-      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;
-      }
+      // Original comment shown anyway? No need to port.
+      if (isInPatchRange(originalComment, patchRange)) return false;
 
       if (thread.commentSide === CommentSide.PARENT) {
         // TODO(dhruvsri): Add handling for merge parents
@@ -364,6 +350,17 @@
 
       if (!isUnresolved(thread) && !isDraftThread(thread)) return false;
 
+      if (
+        (originalComment.line && !portedComment.line) ||
+        (originalComment.range && !portedComment.range)
+      ) {
+        thread.rangeInfoLost = true;
+      }
+      // TODO: It probably makes more sense to set the patch_set in
+      // portedComment either in the backend or in the RestApi layer. Then we
+      // could check `!isInPatchRange(portedComment, patchRange)` and then set
+      // thread.patchNum = portedComment.patch_set;
+      thread.patchNum = patchRange.patchNum;
       thread.range = portedComment.range;
       thread.line = portedComment.line;
       thread.ported = true;
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 9770261..2738eb8 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
@@ -131,7 +131,8 @@
             {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
+        // check that the location of the thread matches the ported comment
+        assert.equal(portedThreads[0].patchNum, 4);
         assert.deepEqual(portedThreads[0].range, {
           start_line: 136,
           start_character: 16,
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 1411c88..bd05edb 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -528,7 +528,7 @@
     return html`
       <a href="${this.getUrlForComment()}">
         <span class="portedMessage" @click="${this.handlePortedMessageClick}">
-          From patchset ${this.comment?.patch_set}]]
+          From patchset ${this.comment?.patch_set}
         </span>
       </a>
     `;
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 49e9674..ee26915 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -188,6 +188,10 @@
    * thread only contains an unsaved draft.
    */
   rootId?: UrlEncodedCommentId;
+  /**
+   * Note that all location information is typically identical to that of the
+   * first comment, but not for ported comments!
+   */
   path: string;
   commentSide: CommentSide;
   /* mergeParentNum is the merge parent number only valid for merge commits
@@ -201,8 +205,17 @@
      FILE comments. */
   line?: LineNumber;
   range?: CommentRange;
-  ported?: boolean; // is the comment ported over from a previous patchset
-  rangeInfoLost?: boolean; // if BE was unable to determine a range for this
+  /**
+   * Was the thread ported over from its original location to a newer patchset?
+   * If yes, then the location information above contains the ported location,
+   * but the comments still have the original location set.
+   */
+  ported?: boolean;
+  /**
+   * Only relevant when ported:true. Means that no ported range could be
+   * computed. `line` and `range` can be undefined then.
+   */
+  rangeInfoLost?: boolean;
 }
 
 export function equalLocation(t1?: CommentThread, t2?: CommentThread) {