Merge branch 'ph/236143/addedUtilMethodToCreateParallelLines' into 'master'

Add util method to frontend to convert inline diff lines to parallel

See merge request gitlab-org/gitlab!39943

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

@@ -717,3 +717,74 @@ export const getDefaultWhitespace = (queryString, cookie) => {
   if (cookie === NO_SHOW_WHITESPACE) return false;
   return true;
 };
+
+export const isAdded = line => ['new', 'new-nonewline'].includes(line.type);
+export const isRemoved = line => ['old', 'old-nonewline'].includes(line.type);
+export const isUnchanged = line => !line.type;
+export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includes(line.type);
+
+/**
+ * Pass in the inline diff lines array which gets converted
+ * to the parallel diff lines.
+ * This allows for us to convert inline diff lines to parallel
+ * on the frontend without needing to send any requests
+ * to the API.
+ *
+ * This method has been taken from the already existing backend
+ * implementation at lib/gitlab/diff/parallel_diff.rb
+ *
+ * @param {Object[]} diffLines - inline diff lines
+ *
+ * @returns {Object[]} parallel lines
+ */
+export const parallelizeDiffLines = (diffLines = []) => {
+  let freeRightIndex = null;
+  const lines = [];
+
+  for (let i = 0, diffLinesLength = diffLines.length, index = 0; i < diffLinesLength; i += 1) {
+    const line = diffLines[i];
+
+    if (isRemoved(line)) {
+      lines.push({
+        [LINE_POSITION_LEFT]: line,
+        [LINE_POSITION_RIGHT]: null,
+      });
+
+      if (freeRightIndex === null) {
+        // Once we come upon a new line it can be put on the right of this old line
+        freeRightIndex = index;
+      }
+      index += 1;
+    } else if (isAdded(line)) {
+      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;
+
+        // If there are any other old lines on the left that don't yet have
+        // a new counterpart on the right, update the free_right_index
+        const nextFreeRightIndex = freeRightIndex + 1;
+        freeRightIndex = nextFreeRightIndex < index ? nextFreeRightIndex : null;
+      } else {
+        lines.push({
+          [LINE_POSITION_LEFT]: null,
+          [LINE_POSITION_RIGHT]: line,
+        });
+
+        freeRightIndex = null;
+        index += 1;
+      }
+    } else if (isMeta(line) || isUnchanged(line)) {
+      // line in the right panel is the same as in the left one
+      lines.push({
+        [LINE_POSITION_LEFT]: line,
+        [LINE_POSITION_RIGHT]: line,
+      });
+
+      freeRightIndex = null;
+      index += 1;
+    }
+  }
+
+  return lines;
+};

spec/frontend/diffs/components/diff_expansion_cell_spec.js

@@ -10,7 +10,6 @@ import diffFileMockData from '../mock_data/diff_file';
 
 const EXPAND_UP_CLASS = '.js-unfold';
 const EXPAND_DOWN_CLASS = '.js-unfold-down';
-const LINE_TO_USE = 5;
 const lineSources = {
   [INLINE_DIFF_VIEW_TYPE]: 'highlighted_diff_lines',
   [PARALLEL_DIFF_VIEW_TYPE]: 'parallel_diff_lines',
@@ -66,7 +65,7 @@ describe('DiffExpansionCell', () => {
 
   beforeEach(() => {
     mockFile = cloneDeep(diffFileMockData);
-    mockLine = getLine(mockFile, INLINE_DIFF_VIEW_TYPE, LINE_TO_USE);
+    mockLine = getLine(mockFile, INLINE_DIFF_VIEW_TYPE, 8);
     store = createStore();
     store.state.diffs.diffFiles = [mockFile];
     jest.spyOn(store, 'dispatch').mockReturnValue(Promise.resolve());
@@ -126,12 +125,12 @@ describe('DiffExpansionCell', () => {
 
   describe('any row', () => {
     [
-      { diffViewType: INLINE_DIFF_VIEW_TYPE, file: { parallel_diff_lines: [] } },
-      { diffViewType: PARALLEL_DIFF_VIEW_TYPE, file: { highlighted_diff_lines: [] } },
-    ].forEach(({ diffViewType, file }) => {
+      { diffViewType: INLINE_DIFF_VIEW_TYPE, lineIndex: 8, file: { parallel_diff_lines: [] } },
+      { diffViewType: PARALLEL_DIFF_VIEW_TYPE, lineIndex: 7, file: { highlighted_diff_lines: [] } },
+    ].forEach(({ diffViewType, file, lineIndex }) => {
       describe(`with diffViewType (${diffViewType})`, () => {
         beforeEach(() => {
-          mockLine = getLine(mockFile, diffViewType, LINE_TO_USE);
+          mockLine = getLine(mockFile, diffViewType, lineIndex);
           store.state.diffs.diffFiles = [{ ...mockFile, ...file }];
           store.state.diffs.diffViewType = diffViewType;
         });
@@ -189,10 +188,10 @@ describe('DiffExpansionCell', () => {
         });
 
         it('on expand down clicked, dispatch loadMoreLines', () => {
-          mockFile[lineSources[diffViewType]][LINE_TO_USE + 1] = cloneDeep(
-            mockFile[lineSources[diffViewType]][LINE_TO_USE],
+          mockFile[lineSources[diffViewType]][lineIndex + 1] = cloneDeep(
+            mockFile[lineSources[diffViewType]][lineIndex],
           );
-          const nextLine = getLine(mockFile, diffViewType, LINE_TO_USE + 1);
+          const nextLine = getLine(mockFile, diffViewType, lineIndex + 1);
 
           nextLine.meta_data.old_pos = 300;
           nextLine.meta_data.new_pos = 300;

spec/frontend/diffs/components/diff_file_spec.js

@@ -45,7 +45,7 @@ describe('DiffFile', () => {
 
       vm.$nextTick()
         .then(() => {
-          expect(el.querySelectorAll('.line_content').length).toBe(5);
+          expect(el.querySelectorAll('.line_content').length).toBe(8);
           expect(el.querySelectorAll('.js-line-expansion-content').length).toBe(1);
           triggerEvent('.btn-clipboard');
         })

spec/frontend/diffs/components/inline_diff_expansion_row_spec.js

@@ -5,12 +5,13 @@ import InlineDiffExpansionRow from '~/diffs/components/inline_diff_expansion_row
 import diffFileMockData from '../mock_data/diff_file';
 
 describe('InlineDiffExpansionRow', () => {
-  const matchLine = diffFileMockData.highlighted_diff_lines[5];
+  const mockData = { ...diffFileMockData };
+  const matchLine = mockData.highlighted_diff_lines.pop();
 
   const createComponent = (options = {}) => {
     const cmp = Vue.extend(InlineDiffExpansionRow);
     const defaults = {
-      fileHash: diffFileMockData.file_hash,
+      fileHash: mockData.file_hash,
       contextLinesPath: 'contextLinesPath',
       line: matchLine,
       isTop: false,

spec/frontend/diffs/components/inline_diff_view_spec.js

@@ -30,8 +30,8 @@ describe('InlineDiffView', () => {
     it('should have rendered diff lines', () => {
       const el = component.$el;
 
-      expect(el.querySelectorAll('tr.line_holder').length).toEqual(5);
-      expect(el.querySelectorAll('tr.line_holder.new').length).toEqual(2);
+      expect(el.querySelectorAll('tr.line_holder').length).toEqual(8);
+      expect(el.querySelectorAll('tr.line_holder.new').length).toEqual(4);
       expect(el.querySelectorAll('tr.line_expansion.match').length).toEqual(1);
       expect(el.textContent.indexOf('Bad dates')).toBeGreaterThan(-1);
     });

spec/frontend/diffs/components/parallel_diff_view_spec.js

-import Vue from 'vue';
-import { createComponentWithStore } from 'helpers/vue_mount_component_helper';
+import Vuex from 'vuex';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
 import { createStore } from '~/mr_notes/stores';
 import ParallelDiffView from '~/diffs/components/parallel_diff_view.vue';
-import * as constants from '~/diffs/constants';
+import parallelDiffTableRow from '~/diffs/components/parallel_diff_table_row.vue';
 import diffFileMockData from '../mock_data/diff_file';
 
-describe('ParallelDiffView', () => {
-  let component;
-  const getDiffFileMock = () => ({ ...diffFileMockData });
+let wrapper;
+const localVue = createLocalVue();
+
+localVue.use(Vuex);
 
-  beforeEach(() => {
-    const diffFile = getDiffFileMock();
+function factory() {
+  const diffFile = { ...diffFileMockData };
+  const store = createStore();
 
-    component = createComponentWithStore(Vue.extend(ParallelDiffView), createStore(), {
+  wrapper = shallowMount(ParallelDiffView, {
+    localVue,
+    store,
+    propsData: {
       diffFile,
       diffLines: diffFile.parallel_diff_lines,
-    }).$mount();
+    },
   });
+}
 
+describe('ParallelDiffView', () => {
   afterEach(() => {
-    component.$destroy();
+    wrapper.destroy();
   });
 
-  describe('assigned', () => {
-    describe('diffLines', () => {
-      it('should normalize lines for empty cells', () => {
-        expect(component.diffLines[0].left.type).toEqual(constants.EMPTY_CELL_TYPE);
-        expect(component.diffLines[1].left.type).toEqual(constants.EMPTY_CELL_TYPE);
-      });
-    });
+  it('renders diff lines', () => {
+    factory();
+
+    expect(wrapper.findAll(parallelDiffTableRow).length).toBe(8);
   });
 });

spec/frontend/diffs/mock_data/diff_file.js

@@ -56,8 +56,8 @@ export default {
       old_line: null,
       new_line: 1,
       discussions: [],
-      text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
-      rich_text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
+      text: '<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
+      rich_text: '<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
       meta_data: null,
     },
     {
@@ -66,8 +66,8 @@ export default {
       old_line: null,
       new_line: 2,
       discussions: [],
-      text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
-      rich_text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
+      text: '<span id="LC2" class="line" lang="plaintext"></span>\n',
+      rich_text: '<span id="LC2" class="line" lang="plaintext"></span>\n',
       meta_data: null,
     },
     {
@@ -76,8 +76,8 @@ export default {
       old_line: 1,
       new_line: 3,
       discussions: [],
-      text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
-      rich_text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
+      text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
+      rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
       meta_data: null,
     },
     {
@@ -86,8 +86,8 @@ export default {
       old_line: 2,
       new_line: 4,
       discussions: [],
-      text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
-      rich_text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
+      text: '<span id="LC4" class="line" lang="plaintext"></span>\n',
+      rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n',
       meta_data: null,
     },
     {
@@ -96,8 +96,38 @@ export default {
       old_line: 3,
       new_line: 5,
       discussions: [],
-      text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
-      rich_text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
+      text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
+      rich_text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
+      meta_data: null,
+    },
+    {
+      line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_6',
+      type: 'old',
+      old_line: 4,
+      new_line: null,
+      discussions: [],
+      text: '<span id="LC6" class="line" lang="plaintext"></span>\n',
+      rich_text: '<span id="LC6" class="line" lang="plaintext"></span>\n',
+      meta_data: null,
+    },
+    {
+      line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_7',
+      type: 'new',
+      old_line: null,
+      new_line: 5,
+      discussions: [],
+      text: '<span id="LC7" class="line" lang="plaintext"></span>\n',
+      rich_text: '<span id="LC7" class="line" lang="plaintext"></span>\n',
+      meta_data: null,
+    },
+    {
+      line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_9',
+      type: 'new',
+      old_line: null,
+      new_line: 6,
+      discussions: [],
+      text: '<span id="LC7" class="line" lang="plaintext"></span>\n',
+      rich_text: '<span id="LC7" class="line" lang="plaintext"></span>\n',
       meta_data: null,
     },
     {
@@ -116,43 +146,39 @@ export default {
   ],
   parallel_diff_lines: [
     {
-      left: {
-        type: 'empty-cell',
-      },
+      left: null,
       right: {
         line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1',
         type: 'new',
         old_line: null,
         new_line: 1,
         discussions: [],
-        text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
+        text: '<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
         rich_text: '<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
         meta_data: null,
       },
     },
     {
-      left: {
-        type: 'empty-cell',
-      },
+      left: null,
       right: {
         line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2',
         type: 'new',
         old_line: null,
         new_line: 2,
         discussions: [],
-        text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
+        text: '<span id="LC2" class="line" lang="plaintext"></span>\n',
         rich_text: '<span id="LC2" class="line" lang="plaintext"></span>\n',
         meta_data: null,
       },
     },
     {
       left: {
-        line_Code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3',
+        line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3',
         type: null,
         old_line: 1,
         new_line: 3,
         discussions: [],
-        text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
+        text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         meta_data: null,
       },
@@ -162,7 +188,7 @@ export default {
         old_line: 1,
         new_line: 3,
         discussions: [],
-        text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
+        text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         meta_data: null,
       },
@@ -174,7 +200,7 @@ export default {
         old_line: 2,
         new_line: 4,
         discussions: [],
-        text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
+        text: '<span id="LC4" class="line" lang="plaintext"></span>\n',
         rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n',
         meta_data: null,
       },
@@ -184,7 +210,7 @@ export default {
         old_line: 2,
         new_line: 4,
         discussions: [],
-        text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
+        text: '<span id="LC4" class="line" lang="plaintext"></span>\n',
         rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n',
         meta_data: null,
       },
@@ -196,7 +222,7 @@ export default {
         old_line: 3,
         new_line: 5,
         discussions: [],
-        text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
+        text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         rich_text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         meta_data: null,
       },
@@ -206,11 +232,46 @@ export default {
         old_line: 3,
         new_line: 5,
         discussions: [],
-        text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
+        text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         rich_text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         meta_data: null,
       },
     },
+    {
+      left: {
+        line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_6',
+        type: 'old',
+        old_line: 4,
+        new_line: null,
+        discussions: [],
+        text: '<span id="LC6" class="line" lang="plaintext"></span>\n',
+        rich_text: '<span id="LC6" class="line" lang="plaintext"></span>\n',
+        meta_data: null,
+      },
+      right: {
+        line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_7',
+        type: 'new',
+        old_line: null,
+        new_line: 5,
+        discussions: [],
+        text: '<span id="LC7" class="line" lang="plaintext"></span>\n',
+        rich_text: '<span id="LC7" class="line" lang="plaintext"></span>\n',
+        meta_data: null,
+      },
+    },
+    {
+      left: null,
+      right: {
+        line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_9',
+        type: 'new',
+        old_line: null,
+        new_line: 6,
+        discussions: [],
+        text: '<span id="LC7" class="line" lang="plaintext"></span>\n',
+        rich_text: '<span id="LC7" class="line" lang="plaintext"></span>\n',
+        meta_data: null,
+      },
+    },
     {
       left: {
         line_code: null,

spec/frontend/diffs/store/utils_spec.js

@@ -1167,4 +1167,59 @@ describe('DiffsStoreUtils', () => {
       expect(utils.getDefaultWhitespace(undefined, '0')).toBe(true);
     });
   });
+
+  describe('isAdded', () => {
+    it.each`
+      type               | expected
+      ${'new'}           | ${true}
+      ${'new-nonewline'} | ${true}
+      ${'old'}           | ${false}
+    `('returns $expected when type is $type', ({ type, expected }) => {
+      expect(utils.isAdded({ type })).toBe(expected);
+    });
+  });
+
+  describe('isRemoved', () => {
+    it.each`
+      type               | expected
+      ${'old'}           | ${true}
+      ${'old-nonewline'} | ${true}
+      ${'new'}           | ${false}
+    `('returns $expected when type is $type', ({ type, expected }) => {
+      expect(utils.isRemoved({ type })).toBe(expected);
+    });
+  });
+
+  describe('isUnchanged', () => {
+    it.each`
+      type     | expected
+      ${null}  | ${true}
+      ${'new'} | ${false}
+      ${'old'} | ${false}
+    `('returns $expected when type is $type', ({ type, expected }) => {
+      expect(utils.isUnchanged({ type })).toBe(expected);
+    });
+  });
+
+  describe('isMeta', () => {
+    it.each`
+      type               | expected
+      ${'match'}         | ${true}
+      ${'new-nonewline'} | ${true}
+      ${'old-nonewline'} | ${true}
+      ${'new'}           | ${false}
+    `('returns $expected when type is $type', ({ type, expected }) => {
+      expect(utils.isMeta({ type })).toBe(expected);
+    });
+  });
+
+  describe('parallelizeDiffLines', () => {
+    it('converts inline diff lines to parallel diff lines', () => {
+      const file = getDiffFileMock();
+
+      expect(utils.parallelizeDiffLines(file.highlighted_diff_lines)).toEqual(
+        file.parallel_diff_lines,
+      );
+    });
+  });
 });