Merge branch 'ph/fixDiffsLineNumbers' into 'master'

Fixed unified diff components showing the incorrect numbers

See merge request gitlab-org/gitlab!48491

app/assets/javascripts/diffs/components/diff_row.vue

@@ -157,10 +157,10 @@ export default {
             "
           />
         </div>
-        <div :class="classNameMapCellLeft" class="diff-td diff-line-num old_line">
+        <div v-if="inline" :class="classNameMapCellLeft" class="diff-td diff-line-num old_line">
           <a
-            v-if="line.left.old_line"
-            :data-linenumber="line.left.old_line"
+            v-if="line.left.new_line"
+            :data-linenumber="line.left.new_line"
             :href="line.lineHrefOld"
             @click="setHighlightedRow(line.lineCode)"
           >
@@ -179,21 +179,14 @@ export default {
       </template>
       <template v-else>
         <div data-testid="leftEmptyCell" class="diff-td diff-line-num old_line empty-cell"></div>
-        <div class="diff-td diff-line-num old_line empty-cell"></div>
+        <div v-if="inline" class="diff-td diff-line-num old_line empty-cell"></div>
         <div class="diff-td line-coverage left-side empty-cell"></div>
         <div class="diff-td line_content with-coverage parallel left-side empty-cell"></div>
       </template>
     </div>
-    <div
-      v-if="!inline || (line.right && Boolean(line.right.type))"
-      class="diff-grid-right right-side"
-    >
+    <div v-if="!inline" class="diff-grid-right right-side">
       <template v-if="line.right">
-        <div
-          :class="classNameMapCellRight"
-          data-testid="rightLineNumber"
-          class="diff-td diff-line-num new_line"
-        >
+        <div :class="classNameMapCellRight" class="diff-td diff-line-num new_line">
           <span
             v-if="shouldRenderCommentButton"
             v-gl-tooltip
@@ -231,15 +224,6 @@ export default {
             "
           />
         </div>
-        <div :class="classNameMapCellRight" class="diff-td diff-line-num new_line">
-          <a
-            v-if="line.right.new_line"
-            :data-linenumber="line.right.new_line"
-            :href="line.lineHrefNew"
-            @click="setHighlightedRow(line.lineCode)"
-          >
-          </a>
-        </div>
         <div
           v-gl-tooltip.hover
           :title="coverageState.text"

app/assets/javascripts/diffs/store/utils.js

@@ -47,7 +47,7 @@ export const parallelizeDiffLines = (diffLines, inline) => {
   for (let i = 0, diffLinesLength = diffLines.length, index = 0; i < diffLinesLength; i += 1) {
     const line = diffLines[i];
 
-    if (isRemoved(line)) {
+    if (isRemoved(line) || inline) {
       lines.push({
         [LINE_POSITION_LEFT]: line,
         [LINE_POSITION_RIGHT]: null,
@@ -59,7 +59,7 @@ export const parallelizeDiffLines = (diffLines, inline) => {
       }
       index += 1;
     } else if (isAdded(line)) {
-      if (freeRightIndex !== null && !inline) {
+      if (freeRightIndex !== null) {
         // If an old line came before this without a line on the right, this
         // line can be put to the right of it.
         lines[freeRightIndex].right = line;

app/assets/stylesheets/framework/diffs.scss

@@ -597,10 +597,6 @@ table.code {
   .diff-grid-right {
     display: grid;
     grid-template-columns: 50px 8px 1fr;
-
-    .diff-td:nth-child(2) {
-      display: none;
-    }
   }
 
   .diff-grid-comments {
@@ -631,20 +627,6 @@ table.code {
     .diff-grid-left,
     .diff-grid-right {
       grid-template-columns: 50px 50px 8px 1fr;
-
-      .diff-td:nth-child(2) {
-        display: block;
-      }
-    }
-
-    .diff-grid-left .old:nth-child(1) [data-linenumber],
-    .diff-grid-right .new:nth-child(2) [data-linenumber] {
-      display: inline;
-    }
-
-    .diff-grid-left .old:nth-child(2) [data-linenumber],
-    .diff-grid-right .new:nth-child(1) [data-linenumber] {
-      display: none;
     }
   }
 }

spec/frontend/diffs/components/diff_row_spec.js

@@ -97,18 +97,18 @@ describe('DiffRow', () => {
     ${'right'}
   `('$side side', ({ side }) => {
     it(`renders empty cells if ${side} is unavailable`, () => {
-      const wrapper = createWrapper({ props: { line: testLines[2] } });
+      const wrapper = createWrapper({ props: { line: testLines[2], inline: false } });
       expect(wrapper.find(`[data-testid="${side}LineNumber"]`).exists()).toBe(false);
       expect(wrapper.find(`[data-testid="${side}EmptyCell"]`).exists()).toBe(true);
     });
 
     it('renders comment button', () => {
-      const wrapper = createWrapper({ props: { line: testLines[3] } });
+      const wrapper = createWrapper({ props: { line: testLines[3], inline: false } });
       expect(wrapper.find(`[data-testid="${side}CommentButton"]`).exists()).toBe(true);
     });
 
     it('renders avatars', () => {
-      const wrapper = createWrapper({ props: { line: testLines[0] } });
+      const wrapper = createWrapper({ props: { line: testLines[0], inline: false } });
       expect(wrapper.find(`[data-testid="${side}Discussions"]`).exists()).toBe(true);
     });
   });

spec/frontend/diffs/store/utils_spec.js

@@ -1119,25 +1119,14 @@ describe('DiffsStoreUtils', () => {
       );
     });
 
-    /**
-     * What's going on here?
-     *
-     * The inline version of parallelizeDiffLines simply keeps the difflines
-     * in the same order they are received as opposed to shuffling them
-     * to be "side by side".
-     *
-     * This keeps the underlying data structure the same which simplifies
-     * the components, but keeps the changes grouped together as users
-     * expect when viewing changes inline.
-     */
-    it('converts inline diff lines to inline diff lines with a parallel structure', () => {
+    it('converts inline diff lines', () => {
       const file = getDiffFileMock();
       const files = utils.parallelizeDiffLines(file.highlighted_diff_lines, true);
 
       expect(files[5].left).toEqual(file.parallel_diff_lines[5].left);
       expect(files[5].right).toBeNull();
-      expect(files[6].left).toBeNull();
-      expect(files[6].right).toEqual(file.parallel_diff_lines[5].right);
+      expect(files[6].left).toEqual(file.parallel_diff_lines[5].right);
+      expect(files[6].right).toBeNull();
     });
   });
 });