Merge branch 'tor/defect/mr-versions-wrong-file-diff' into 'master'

Include MR version information when loading file diffs

See merge request !106378



Merged-by: Thomas Randolph <trandolph@gitlab.com>

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

@@ -444,20 +444,27 @@ export const scrollToLineIfNeededParallel = (_, line) => {
   }
 };
 
-export const loadCollapsedDiff = ({ commit, getters, state }, file) =>
-  axios
-    .get(file.load_collapsed_diff_url, {
-      params: {
-        commit_id: getters.commitId,
-        w: state.showWhitespace ? '0' : '1',
-      },
-    })
-    .then((res) => {
-      commit(types.ADD_COLLAPSED_DIFFS, {
-        file,
-        data: res.data,
-      });
+export const loadCollapsedDiff = ({ commit, getters, state }, file) => {
+  const versionPath = state.mergeRequestDiff?.version_path;
+  let loadParams = {
+    commit_id: getters.commitId,
+    w: state.showWhitespace ? '0' : '1',
+  };
+
+  if (versionPath) {
+    const { diffId, startSha } = getDerivedMergeRequestInformation({ endpoint: versionPath });
+
+    loadParams.diff_id = diffId;
+    loadParams.start_sha = startSha;
+  }
+
+  return axios.get(file.load_collapsed_diff_url, { params: loadParams }).then((res) => {
+    commit(types.ADD_COLLAPSED_DIFFS, {
+      file,
+      data: res.data,
     });
+  });
+};
 
 /**
  * Toggles the file discussions after user clicked on the toggle discussions button.

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

 const endpointRE = /^(\/?(.+?)\/(.+?)\/-\/merge_requests\/(\d+)).*$/i;
 
+function getVersionInfo({ endpoint } = {}) {
+  const dummyRoot = 'https://gitlab.com';
+  const endpointUrl = new URL(endpoint, dummyRoot);
+  const params = Object.fromEntries(endpointUrl.searchParams.entries());
+
+  const { start_sha: startSha, diff_id: diffId } = params;
+
+  return {
+    diffId,
+    startSha,
+  };
+}
+
 export function getDerivedMergeRequestInformation({ endpoint } = {}) {
   let mrPath;
   let userOrGroup;
   let project;
   let id;
+  let diffId;
+  let startSha;
   const matches = endpointRE.exec(endpoint);
 
   if (matches) {
     [, mrPath, userOrGroup, project, id] = matches;
+    ({ diffId, startSha } = getVersionInfo({ endpoint }));
   }
 
   return {
@@ -16,5 +32,7 @@ export function getDerivedMergeRequestInformation({ endpoint } = {}) {
     userOrGroup,
     project,
     id,
+    diffId,
+    startSha,
   };
 }

spec/frontend/diffs/utils/merge_request_spec.js

@@ -2,30 +2,70 @@ import { getDerivedMergeRequestInformation } from '~/diffs/utils/merge_request';
 import { diffMetadata } from '../mock_data/diff_metadata';
 
 describe('Merge Request utilities', () => {
-  const derivedMrInfo = {
+  const derivedBaseInfo = {
     mrPath: '/gitlab-org/gitlab-test/-/merge_requests/4',
     userOrGroup: 'gitlab-org',
     project: 'gitlab-test',
     id: '4',
   };
+  const derivedVersionInfo = {
+    diffId: '4',
+    startSha: 'eb227b3e214624708c474bdab7bde7afc17cefcc',
+  };
+  const noVersion = {
+    diffId: undefined,
+    startSha: undefined,
+  };
   const unparseableEndpoint = {
     mrPath: undefined,
     userOrGroup: undefined,
     project: undefined,
     id: undefined,
+    ...noVersion,
   };
 
   describe('getDerivedMergeRequestInformation', () => {
-    const endpoint = `${diffMetadata.latest_version_path}.json?searchParam=irrelevant`;
+    let endpoint = `${diffMetadata.latest_version_path}.json?searchParam=irrelevant`;
 
     it.each`
       argument                   | response
-      ${{ endpoint }}            | ${derivedMrInfo}
+      ${{ endpoint }}            | ${{ ...derivedBaseInfo, ...noVersion }}
       ${{}}                      | ${unparseableEndpoint}
       ${{ endpoint: undefined }} | ${unparseableEndpoint}
       ${{ endpoint: null }}      | ${unparseableEndpoint}
     `('generates the correct derived results based on $argument', ({ argument, response }) => {
       expect(getDerivedMergeRequestInformation(argument)).toStrictEqual(response);
     });
+
+    describe('version information', () => {
+      const bare = diffMetadata.latest_version_path;
+      endpoint = diffMetadata.merge_request_diffs[0].compare_path;
+
+      it('still gets the correct derived information', () => {
+        expect(getDerivedMergeRequestInformation({ endpoint })).toEqual(
+          expect.objectContaining(derivedBaseInfo),
+        );
+      });
+
+      it.each`
+        url                                                   | versionPart
+        ${endpoint}                                           | ${derivedVersionInfo}
+        ${`${bare}?diff_id=4`}                                | ${{ ...derivedVersionInfo, startSha: undefined }}
+        ${`${bare}?start_sha=${derivedVersionInfo.startSha}`} | ${{ ...derivedVersionInfo, diffId: undefined }}
+      `(
+        'generates the correct derived version information based on $url',
+        ({ url, versionPart }) => {
+          expect(getDerivedMergeRequestInformation({ endpoint: url })).toEqual(
+            expect.objectContaining(versionPart),
+          );
+        },
+      );
+
+      it('extracts nothing if there is no available version-like information in the URL', () => {
+        expect(getDerivedMergeRequestInformation({ endpoint: bare })).toEqual(
+          expect.objectContaining(noVersion),
+        );
+      });
+    });
   });
 });