From 4b7c0885f186733aefda8f313b790085888b57a4 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Wed, 13 May 2020 14:01:56 -0600
Subject: [PATCH 1/2] Add an action to upgrade a `renamed` file to a `text`
 viewer

Return the promise and rethrow the error
so that other consumers can also respond
to potential errors
---
 app/assets/javascripts/diffs/store/actions.js | 37 +++++++++++++++++++
 app/assets/javascripts/diffs/store/utils.js   | 36 ++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index a6185746bb15d7..1975d6996a5ac6 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -16,6 +16,7 @@ import {
   idleCallback,
   allDiscussionWrappersExpanded,
   prepareDiffData,
+  prepareLineForRenamedFile,
 } from './utils';
 import * as types from './mutation_types';
 import {
@@ -627,6 +628,42 @@ export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => {
   }
 };
 
+export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { diffFile }) {
+  return axios
+    .get(diffFile.context_lines_path, {
+      params: {
+        full: true,
+        from_merge_request: true,
+      },
+    })
+    .then(({ data }) => {
+      const lines = data.map((line, index) =>
+        prepareLineForRenamedFile({
+          diffViewType: state.diffViewType,
+          line,
+          diffFile,
+          index,
+        }),
+      );
+
+      commit(types.SET_DIFF_FILE_VIEWER, {
+        filePath: diffFile.file_path,
+        viewer: {
+          ...diffFile.alternate_viewer,
+          collapsed: false,
+        },
+      });
+      commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: diffFile.file_path, lines });
+
+      dispatch('startRenderDiffsQueue');
+    })
+    .catch(error => {
+      dispatch('receiveFullDiffError', diffFile.file_path);
+
+      throw error;
+    });
+}
+
 export const setFileCollapsed = ({ commit }, { filePath, collapsed }) =>
   commit(types.SET_FILE_COLLAPSED, { filePath, collapsed });
 
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index eb4c6683035c6c..2be71c770877f0 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -303,6 +303,42 @@ function prepareLine(line) {
   }
 }
 
+export function prepareLineForRenamedFile({ line, diffViewType, diffFile, index = 0 }) {
+  /*
+    Renamed files are a little different than other diffs, which
+    is why this is distinct from `prepareDiffFileLines` below.
+
+    We don't get any of the diff file context when we get the diff
+    (so no "inline" vs. "parallel", no "line_code", etc.).
+
+    We can also assume that both the left and the right of each line
+    (for parallel diff view type) are identical, because the file
+    is renamed, not modified.
+
+    This should be cleaned up as part of the effort around flattening our data
+    ==> https://gitlab.com/groups/gitlab-org/-/epics/2852#note_304803402
+  */
+  const lineNumber = index + 1;
+  const cleanLine = {
+    ...line,
+    line_code: `${diffFile.file_hash}_${lineNumber}_${lineNumber}`,
+    new_line: lineNumber,
+    old_line: lineNumber,
+  };
+
+  prepareLine(cleanLine); // WARNING: In-Place Mutations!
+
+  if (diffViewType === PARALLEL_DIFF_VIEW_TYPE) {
+    return {
+      left: { ...cleanLine },
+      right: { ...cleanLine },
+      line_code: cleanLine.line_code,
+    };
+  }
+
+  return cleanLine;
+}
+
 function prepareDiffFileLines(file) {
   const inlineLines = file.highlighted_diff_lines;
   const parallelLines = file.parallel_diff_lines;
-- 
GitLab


From ddd9b64d28d01d1ee75d85057502c0ced41aea16 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 8 May 2020 01:14:22 -0600
Subject: [PATCH 2/2] Test action and additional util functions to upgrade
 renamed file

---
 spec/frontend/diffs/store/actions_spec.js | 82 +++++++++++++++++++++++
 spec/frontend/diffs/store/utils_spec.js   | 66 ++++++++++++++++++
 2 files changed, 148 insertions(+)

diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js
index 970bc99f8ff6ef..3fba661da442e0 100644
--- a/spec/frontend/diffs/store/actions_spec.js
+++ b/spec/frontend/diffs/store/actions_spec.js
@@ -40,6 +40,7 @@ import {
   receiveFullDiffError,
   fetchFullDiff,
   toggleFullDiff,
+  switchToFullDiffFromRenamedFile,
   setFileCollapsed,
   setExpandedDiffLines,
   setSuggestPopoverDismissed,
@@ -1252,6 +1253,87 @@ describe('DiffsStoreActions', () => {
     });
   });
 
+  describe('switchToFullDiffFromRenamedFile', () => {
+    const SUCCESS_URL = 'fakehost/context.success';
+    const ERROR_URL = 'fakehost/context.error';
+    const testFilePath = 'testpath';
+    const updatedViewerName = 'testviewer';
+    const preparedLine = { prepared: 'in-a-test' };
+    const testFile = {
+      file_path: testFilePath,
+      file_hash: 'testhash',
+      alternate_viewer: { name: updatedViewerName },
+    };
+    const updatedViewer = { name: updatedViewerName, collapsed: false };
+    const testData = [{ rich_text: 'test' }, { rich_text: 'file2' }];
+    let renamedFile;
+    let mock;
+
+    beforeEach(() => {
+      mock = new MockAdapter(axios);
+      jest.spyOn(utils, 'prepareLineForRenamedFile').mockImplementation(() => preparedLine);
+    });
+
+    afterEach(() => {
+      renamedFile = null;
+      mock.restore();
+    });
+
+    describe('success', () => {
+      beforeEach(() => {
+        renamedFile = { ...testFile, context_lines_path: SUCCESS_URL };
+        mock.onGet(SUCCESS_URL).replyOnce(200, testData);
+      });
+
+      it.each`
+        diffViewType
+        ${INLINE_DIFF_VIEW_TYPE}
+        ${PARALLEL_DIFF_VIEW_TYPE}
+      `(
+        'performs the correct mutations and starts a render queue for view type $diffViewType',
+        ({ diffViewType }) => {
+          return testAction(
+            switchToFullDiffFromRenamedFile,
+            { diffFile: renamedFile },
+            { diffViewType },
+            [
+              {
+                type: types.SET_DIFF_FILE_VIEWER,
+                payload: { filePath: testFilePath, viewer: updatedViewer },
+              },
+              {
+                type: types.SET_CURRENT_VIEW_DIFF_FILE_LINES,
+                payload: { filePath: testFilePath, lines: [preparedLine, preparedLine] },
+              },
+            ],
+            [{ type: 'startRenderDiffsQueue' }],
+          );
+        },
+      );
+    });
+
+    describe('error', () => {
+      beforeEach(() => {
+        renamedFile = { ...testFile, context_lines_path: ERROR_URL };
+        mock.onGet(ERROR_URL).reply(500);
+      });
+
+      it('dispatches the error handling action', () => {
+        const rejected = testAction(
+          switchToFullDiffFromRenamedFile,
+          { diffFile: renamedFile },
+          null,
+          [],
+          [{ type: 'receiveFullDiffError', payload: testFilePath }],
+        );
+
+        return rejected.catch(error =>
+          expect(error).toEqual(new Error('Request failed with status code 500')),
+        );
+      });
+    });
+  });
+
   describe('setFileCollapsed', () => {
     it('commits SET_FILE_COLLAPSED', done => {
       testAction(
diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js
index 31053a3055a92d..641373e666f629 100644
--- a/spec/frontend/diffs/store/utils_spec.js
+++ b/spec/frontend/diffs/store/utils_spec.js
@@ -361,6 +361,72 @@ describe('DiffsStoreUtils', () => {
     });
   });
 
+  describe('prepareLineForRenamedFile', () => {
+    const diffFile = {
+      file_hash: 'file-hash',
+    };
+    const lineIndex = 4;
+    const sourceLine = {
+      foo: 'test',
+      rich_text: ' <p>rich</p>', // Note the leading space
+    };
+    const correctLine = {
+      foo: 'test',
+      line_code: 'file-hash_5_5',
+      old_line: 5,
+      new_line: 5,
+      rich_text: '<p>rich</p>', // Note no leading space
+      discussionsExpanded: true,
+      discussions: [],
+      hasForm: false,
+      text: undefined,
+      alreadyPrepared: true,
+    };
+    let preppedLine;
+
+    beforeEach(() => {
+      preppedLine = utils.prepareLineForRenamedFile({
+        diffViewType: INLINE_DIFF_VIEW_TYPE,
+        line: sourceLine,
+        index: lineIndex,
+        diffFile,
+      });
+    });
+
+    it('copies over the original line object to the new prepared line', () => {
+      expect(preppedLine).toEqual(
+        expect.objectContaining({
+          foo: correctLine.foo,
+          rich_text: correctLine.rich_text,
+        }),
+      );
+    });
+
+    it('correctly sets the old and new lines, plus a line code', () => {
+      expect(preppedLine.old_line).toEqual(correctLine.old_line);
+      expect(preppedLine.new_line).toEqual(correctLine.new_line);
+      expect(preppedLine.line_code).toEqual(correctLine.line_code);
+    });
+
+    it('returns a single object with the correct structure for `inline` lines', () => {
+      expect(preppedLine).toEqual(correctLine);
+    });
+
+    it('returns a nested object with "left" and "right" lines + the line code for `parallel` lines', () => {
+      preppedLine = utils.prepareLineForRenamedFile({
+        diffViewType: PARALLEL_DIFF_VIEW_TYPE,
+        line: sourceLine,
+        index: lineIndex,
+        diffFile,
+      });
+
+      expect(Object.keys(preppedLine)).toEqual(['left', 'right', 'line_code']);
+      expect(preppedLine.left).toEqual(correctLine);
+      expect(preppedLine.right).toEqual(correctLine);
+      expect(preppedLine.line_code).toEqual(correctLine.line_code);
+    });
+  });
+
   describe('prepareDiffData', () => {
     let mock;
     let preparedDiff;
-- 
GitLab