Make getFiles not assume it has branch available

Currently, the getFiles action assumes that the state has details
available for the branch passed to it. It uses it to fetch its last
commit ID. This may or may not be the case. The action should
instead take another parameter `ref` to fetch commits by if
necessary.

app/assets/javascripts/ide/stores/actions/merge_request.js

@@ -141,7 +141,7 @@ export const getMergeRequestVersions = (
   });
 
 export const openMergeRequest = (
-  { dispatch, state },
+  { dispatch, state, getters },
   { projectId, targetProjectId, mergeRequestId } = {},
 ) =>
   dispatch('getMergeRequestData', {
@@ -152,17 +152,18 @@ export const openMergeRequest = (
     .then(mr => {
       dispatch('setCurrentBranchId', mr.source_branch);
 
-      // getFiles needs to be called after getting the branch data
-      // since files are fetched using the last commit sha of the branch
       return dispatch('getBranchData', {
         projectId,
         branchId: mr.source_branch,
-      }).then(() =>
-        dispatch('getFiles', {
+      }).then(() => {
+        const branch = getters.findBranch(projectId, mr.source_branch);
+
+        return dispatch('getFiles', {
           projectId,
           branchId: mr.source_branch,
-        }),
-      );
+          ref: branch.commit.id,
+        });
+      });
     })
     .then(() =>
       dispatch('getMergeRequestVersions', {

app/assets/javascripts/ide/stores/actions/project.js

@@ -111,7 +111,7 @@ export const loadFile = ({ dispatch, state }, { basePath }) => {
   }
 };
 
-export const loadBranch = ({ dispatch }, { projectId, branchId }) =>
+export const loadBranch = ({ dispatch, getters }, { projectId, branchId }) =>
   dispatch('getBranchData', {
     projectId,
     branchId,
@@ -121,9 +121,13 @@ export const loadBranch = ({ dispatch }, { projectId, branchId }) =>
         projectId,
         branchId,
       });
+
+      const branch = getters.findBranch(projectId, branchId);
+
       return dispatch('getFiles', {
         projectId,
         branchId,
+        ref: branch.commit.id,
       });
     })
     .catch(() => {

app/assets/javascripts/ide/stores/actions/tree.js

@@ -46,19 +46,20 @@ export const setDirectoryData = ({ state, commit }, { projectId, branchId, treeL
   });
 };
 
-export const getFiles = ({ state, commit, dispatch, getters }, { projectId, branchId } = {}) =>
+export const getFiles = ({ state, commit, dispatch }, payload = {}) =>
   new Promise((resolve, reject) => {
+    const { projectId, branchId, ref = branchId } = payload;
+
     if (
       !state.trees[`${projectId}/${branchId}`] ||
       (state.trees[`${projectId}/${branchId}`].tree &&
         state.trees[`${projectId}/${branchId}`].tree.length === 0)
     ) {
       const selectedProject = state.projects[projectId];
-      const selectedBranch = getters.findBranch(projectId, branchId);
 
       commit(types.CREATE_TREE, { treePath: `${projectId}/${branchId}` });
       service
-        .getFiles(selectedProject.web_url, selectedBranch.commit.id)
+        .getFiles(selectedProject.web_url, ref)
         .then(({ data }) => {
           const { entries, treeList } = decorateFiles({
             data,
@@ -77,8 +78,8 @@ export const getFiles = ({ state, commit, dispatch, getters }, { projectId, bran
         .catch(e => {
           dispatch('setErrorMessage', {
             text: __('An error occurred whilst loading all the files.'),
-            action: payload =>
-              dispatch('getFiles', payload).then(() => dispatch('setErrorMessage', null)),
+            action: actionPayload =>
+              dispatch('getFiles', actionPayload).then(() => dispatch('setErrorMessage', null)),
             actionText: __('Please try again'),
             actionPayload: { projectId, branchId },
           });

spec/javascripts/ide/stores/actions/merge_request_spec.js

@@ -348,6 +348,8 @@ describe('IDE store merge request actions', () => {
     let testMergeRequest;
     let testMergeRequestChanges;
 
+    const mockGetters = { findBranch: () => ({ commit: { id: 'abcd2322' } }) };
+
     beforeEach(() => {
       testMergeRequest = {
         source_branch: 'abcbranch',
@@ -406,8 +408,8 @@ describe('IDE store merge request actions', () => {
       );
     });
 
-    it('dispatch actions for merge request data', done => {
-      openMergeRequest(store, mr)
+    it('dispatches actions for merge request data', done => {
+      openMergeRequest({ state: store.state, dispatch: store.dispatch, getters: mockGetters }, mr)
         .then(() => {
           expect(store.dispatch.calls.allArgs()).toEqual([
             ['getMergeRequestData', mr],
@@ -424,6 +426,7 @@ describe('IDE store merge request actions', () => {
               {
                 projectId: mr.projectId,
                 branchId: testMergeRequest.source_branch,
+                ref: 'abcd2322',
               },
             ],
             ['getMergeRequestVersions', mr],
@@ -449,7 +452,7 @@ describe('IDE store merge request actions', () => {
         { new_path: 'bar', path: 'bar' },
       ];
 
-      openMergeRequest(store, mr)
+      openMergeRequest({ state: store.state, dispatch: store.dispatch, getters: mockGetters }, mr)
         .then(() => {
           expect(store.dispatch).toHaveBeenCalledWith(
             'updateActivityBarView',

spec/javascripts/ide/stores/actions/project_spec.js

@@ -285,16 +285,21 @@ describe('IDE store project actions', () => {
   describe('loadBranch', () => {
     const projectId = 'abc/def';
     const branchId = '123-lorem';
+    const ref = 'abcd2322';
 
     it('fetches branch data', done => {
+      const mockGetters = { findBranch: () => ({ commit: { id: ref } }) };
       spyOn(store, 'dispatch').and.returnValue(Promise.resolve());
 
-      loadBranch(store, { projectId, branchId })
+      loadBranch(
+        { getters: mockGetters, state: store.state, dispatch: store.dispatch },
+        { projectId, branchId },
+      )
         .then(() => {
           expect(store.dispatch.calls.allArgs()).toEqual([
             ['getBranchData', { projectId, branchId }],
             ['getMergeRequestsForBranch', { projectId, branchId }],
-            ['getFiles', { projectId, branchId }],
+            ['getFiles', { projectId, branchId, ref }],
           ]);
         })
         .then(done)

spec/javascripts/ide/stores/actions/tree_spec.js

@@ -17,6 +17,7 @@ describe('Multi-file store tree actions', () => {
     projectId: 'abcproject',
     branch: 'master',
     branchId: 'master',
+    ref: '12345678',
   };
 
   beforeEach(() => {
@@ -29,14 +30,6 @@ describe('Multi-file store tree actions', () => {
     store.state.currentBranchId = 'master';
     store.state.projects.abcproject = {
       web_url: '',
-      branches: {
-        master: {
-          workingReference: '12345678',
-          commit: {
-            id: '12345678',
-          },
-        },
-      },
     };
   });