Merge branch 'backstage/ide-router-refactoring' into 'master'

Backstage/ide router refactoring

See merge request gitlab-org/gitlab-ce!21142

app/assets/javascripts/ide/ide_router.js

@@ -3,7 +3,6 @@ import VueRouter from 'vue-router';
 import { join as joinPath } from 'path';
 import flash from '~/flash';
 import store from './stores';
-import { activityBarViews } from './constants';
 
 Vue.use(VueRouter);
 
@@ -74,102 +73,23 @@ router.beforeEach((to, from, next) => {
         projectId: to.params.project,
       })
       .then(() => {
-        const fullProjectId = `${to.params.namespace}/${to.params.project}`;
-
+        const basePath = to.params[0] || '';
+        const projectId = `${to.params.namespace}/${to.params.project}`;
         const branchId = to.params.branchid;
+        const mergeRequestId = to.params.mrid;
 
         if (branchId) {
-          const basePath = to.params[0] || '';
-
-          store.dispatch('setCurrentBranchId', branchId);
-
-          store.dispatch('getBranchData', {
-            projectId: fullProjectId,
+          store.dispatch('openBranch', {
+            projectId,
             branchId,
+            basePath,
+          });
+        } else if (mergeRequestId) {
+          store.dispatch('openMergeRequest', {
+            projectId,
+            mergeRequestId,
+            targetProjectId: to.query.target_project,
           });
-
-          store
-            .dispatch('getFiles', {
-              projectId: fullProjectId,
-              branchId,
-            })
-            .then(() => {
-              if (basePath) {
-                const path = basePath.slice(-1) === '/' ? basePath.slice(0, -1) : basePath;
-                const treeEntryKey = Object.keys(store.state.entries).find(
-                  key => key === path && !store.state.entries[key].pending,
-                );
-                const treeEntry = store.state.entries[treeEntryKey];
-
-                if (treeEntry) {
-                  store.dispatch('handleTreeEntryAction', treeEntry);
-                }
-              }
-            })
-            .catch(e => {
-              throw e;
-            });
-        } else if (to.params.mrid) {
-          store
-            .dispatch('getMergeRequestData', {
-              projectId: fullProjectId,
-              targetProjectId: to.query.target_project,
-              mergeRequestId: to.params.mrid,
-            })
-            .then(mr => {
-              store.dispatch('setCurrentBranchId', mr.source_branch);
-
-              store.dispatch('getBranchData', {
-                projectId: fullProjectId,
-                branchId: mr.source_branch,
-              });
-
-              return store.dispatch('getFiles', {
-                projectId: fullProjectId,
-                branchId: mr.source_branch,
-              });
-            })
-            .then(() =>
-              store.dispatch('getMergeRequestVersions', {
-                projectId: fullProjectId,
-                targetProjectId: to.query.target_project,
-                mergeRequestId: to.params.mrid,
-              }),
-            )
-            .then(() =>
-              store.dispatch('getMergeRequestChanges', {
-                projectId: fullProjectId,
-                targetProjectId: to.query.target_project,
-                mergeRequestId: to.params.mrid,
-              }),
-            )
-            .then(mrChanges => {
-              if (mrChanges.changes.length) {
-                store.dispatch('updateActivityBarView', activityBarViews.review);
-              }
-
-              mrChanges.changes.forEach((change, ind) => {
-                const changeTreeEntry = store.state.entries[change.new_path];
-
-                if (changeTreeEntry) {
-                  store.dispatch('setFileMrChange', {
-                    file: changeTreeEntry,
-                    mrChange: change,
-                  });
-
-                  if (ind < 10) {
-                    store.dispatch('getFileData', {
-                      path: change.new_path,
-                      makeFileActive: ind === 0,
-                    });
-                  }
-                }
-              });
-            })
-            .catch(e => {
-              flash('Error while loading the merge request. Please try again.');
-              throw e;
-            });
         }
       })
       .catch(e => {

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

-import { __ } from '../../../locale';
+import flash from '~/flash';
+import { __ } from '~/locale';
 import service from '../../services';
 import * as types from '../mutation_types';
+import { activityBarViews } from '../../constants';
 
 export const getMergeRequestData = (
   { commit, dispatch, state },
@@ -104,3 +106,67 @@ export const getMergeRequestVersions = (
       resolve(state.projects[projectId].mergeRequests[mergeRequestId].versions);
     }
   });
+
+export const openMergeRequest = (
+  { dispatch, state },
+  { projectId, targetProjectId, mergeRequestId } = {},
+) =>
+  dispatch('getMergeRequestData', {
+    projectId,
+    targetProjectId,
+    mergeRequestId,
+  })
+  .then(mr => {
+    dispatch('setCurrentBranchId', mr.source_branch);
+
+    dispatch('getBranchData', {
+      projectId,
+      branchId: mr.source_branch,
+    });
+
+    return dispatch('getFiles', {
+      projectId,
+      branchId: mr.source_branch,
+    });
+  })
+  .then(() =>
+    dispatch('getMergeRequestVersions', {
+      projectId,
+      targetProjectId,
+      mergeRequestId,
+    }),
+  )
+  .then(() =>
+    dispatch('getMergeRequestChanges', {
+      projectId,
+      targetProjectId,
+      mergeRequestId,
+    }),
+  )
+  .then(mrChanges => {
+    if (mrChanges.changes.length) {
+      dispatch('updateActivityBarView', activityBarViews.review);
+    }
+
+    mrChanges.changes.forEach((change, ind) => {
+      const changeTreeEntry = state.entries[change.new_path];
+
+      if (changeTreeEntry) {
+        dispatch('setFileMrChange', {
+          file: changeTreeEntry,
+          mrChange: change,
+        });
+
+        if (ind < 10) {
+          dispatch('getFileData', {
+            path: change.new_path,
+            makeFileActive: ind === 0,
+          });
+        }
+      }
+    });
+  })
+  .catch(e => {
+    flash(__('Error while loading the merge request. Please try again.'));
+    throw e;
+  });

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

@@ -124,3 +124,35 @@ export const showBranchNotFoundError = ({ dispatch }, branchId) => {
     actionPayload: branchId,
   });
 };
+
+export const openBranch = (
+  { dispatch, state },
+  { projectId, branchId, basePath },
+) => {
+  dispatch('setCurrentBranchId', branchId);
+
+  dispatch('getBranchData', {
+    projectId,
+    branchId,
+  });
+
+  return (
+    dispatch('getFiles', {
+      projectId,
+      branchId,
+    })
+    .then(() => {
+      if (basePath) {
+        const path = basePath.slice(-1) === '/' ? basePath.slice(0, -1) : basePath;
+        const treeEntryKey = Object.keys(state.entries).find(
+          key => key === path && !state.entries[key].pending,
+        );
+        const treeEntry = state.entries[treeEntryKey];
+
+        if (treeEntry) {
+          dispatch('handleTreeEntryAction', treeEntry);
+        }
+      }
+    })
+  );
+};

app/assets/javascripts/ide/stores/modules/branches/actions.js

@@ -33,7 +33,4 @@ export const fetchBranches = ({ dispatch, rootGetters }, { search = '' }) => {
 
 export const resetBranches = ({ commit }) => commit(types.RESET_BRANCHES);
 
-export const openBranch = ({ rootState, dispatch }, id) =>
-  dispatch('goToRoute', `/project/${rootState.currentProjectId}/edit/${id}`, { root: true });
-
 export default () => {};

locale/gitlab.pot

@@ -2451,6 +2451,9 @@ msgstr ""
 msgid "Error updating todo status."
 msgstr ""
 
+msgid "Error while loading the merge request. Please try again."
+msgstr ""
+
 msgid "Estimated"
 msgstr ""
 

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

 import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
 import store from '~/ide/stores';
-import {
+import actions, {
   getMergeRequestData,
   getMergeRequestChanges,
   getMergeRequestVersions,
+  openMergeRequest,
 } from '~/ide/stores/actions/merge_request';
 import service from '~/ide/services';
+import { activityBarViews } from '~/ide/constants';
 import { resetStore } from '../../helpers';
 
 describe('IDE store merge request actions', () => {
@@ -238,4 +240,101 @@ describe('IDE store merge request actions', () => {
       });
     });
   });
+
+  describe('openMergeRequest', () => {
+    const mr = {
+      projectId: 'abcproject',
+      targetProjectId: 'defproject',
+      mergeRequestId: 2,
+    };
+    let testMergeRequest;
+    let testMergeRequestChanges;
+
+    beforeEach(() => {
+      testMergeRequest = {
+        source_branch: 'abcbranch',
+      };
+      testMergeRequestChanges = {
+        changes: [],
+      };
+      store.state.entries = {
+        foo: {},
+        bar: {},
+      };
+
+      spyOn(store, 'dispatch').and.callFake((type) => {
+        switch (type) {
+          case 'getMergeRequestData':
+            return Promise.resolve(testMergeRequest);
+          case 'getMergeRequestChanges':
+            return Promise.resolve(testMergeRequestChanges);
+          default:
+            return Promise.resolve();
+        }
+      });
+    });
+
+    it('dispatch actions for merge request data', done => {
+      openMergeRequest(store, mr)
+        .then(() => {
+          expect(store.dispatch.calls.allArgs()).toEqual([
+            ['getMergeRequestData', mr],
+            ['setCurrentBranchId', testMergeRequest.source_branch],
+            ['getBranchData', {
+              projectId: mr.projectId,
+              branchId: testMergeRequest.source_branch,
+            }],
+            ['getFiles', {
+              projectId: mr.projectId,
+              branchId: testMergeRequest.source_branch,
+            }],
+            ['getMergeRequestVersions', mr],
+            ['getMergeRequestChanges', mr],
+          ]);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('updates activity bar view and gets file data, if changes are found', done => {
+      testMergeRequestChanges.changes = [
+        { new_path: 'foo' },
+        { new_path: 'bar' },
+      ];
+
+      openMergeRequest(store, mr)
+        .then(() => {
+          expect(store.dispatch).toHaveBeenCalledWith('updateActivityBarView', activityBarViews.review);
+
+          testMergeRequestChanges.changes.forEach((change, i) => {
+            expect(store.dispatch).toHaveBeenCalledWith('setFileMrChange', {
+              file: store.state.entries[change.new_path],
+              mrChange: change,
+            });
+            expect(store.dispatch).toHaveBeenCalledWith('getFileData', {
+              path: change.new_path,
+              makeFileActive: i === 0,
+            });
+          });
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('flashes message, if error', done => {
+      const flashSpy = spyOnDependency(actions, 'flash');
+      store.dispatch.and.returnValue(Promise.reject());
+
+      openMergeRequest(store, mr)
+        .then(() => {
+          fail('Expected openMergeRequest to throw an error');
+        })
+        .catch(() => {
+          expect(flashSpy).toHaveBeenCalledWith(jasmine.any(String));
+        })
+        .then(done)
+        .catch(done.fail);
+
+    });
+  });
 });

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

@@ -5,6 +5,7 @@ import {
   showBranchNotFoundError,
   createNewBranchFromDefault,
   getBranchData,
+  openBranch,
 } from '~/ide/stores/actions';
 import store from '~/ide/stores';
 import service from '~/ide/services';
@@ -224,4 +225,55 @@ describe('IDE store project actions', () => {
       });
     });
   });
+
+  describe('openBranch', () => {
+    const branch = {
+      projectId: 'feature/lorem-ipsum',
+      branchId: '123-lorem',
+    };
+
+    beforeEach(() => {
+      store.state.entries = {
+        foo: { pending: false },
+        'foo/bar-pending': { pending: true },
+        'foo/bar': { pending: false },
+      };
+
+      spyOn(store, 'dispatch').and.returnValue(Promise.resolve());
+    });
+
+    it('dispatches branch actions', done => {
+      openBranch(store, branch)
+        .then(() => {
+          expect(store.dispatch.calls.allArgs()).toEqual([
+            ['setCurrentBranchId', branch.branchId],
+            ['getBranchData', branch],
+            ['getFiles', branch],
+          ]);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('handles tree entry action, if basePath is given', done => {
+      openBranch(store, { ...branch, basePath: 'foo/bar/' })
+        .then(() => {
+          expect(store.dispatch).toHaveBeenCalledWith(
+            'handleTreeEntryAction',
+            store.state.entries['foo/bar'],
+          );
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('does not handle tree entry action, if entry is pending', done => {
+      openBranch(store, { ...branch, basePath: 'foo/bar-pending' })
+        .then(() => {
+          expect(store.dispatch).not.toHaveBeenCalledWith('handleTreeEntryAction', jasmine.anything());
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
 });

spec/javascripts/ide/stores/modules/branches/actions_spec.js

@@ -9,7 +9,6 @@ import {
   receiveBranchesSuccess,
   fetchBranches,
   resetBranches,
-  openBranch,
 } from '~/ide/stores/modules/branches/actions';
 import { branches, projectData } from '../../../mock_data';
 
@@ -174,20 +173,5 @@ describe('IDE branches actions', () => {
         );
       });
     });
-
-    describe('openBranch', () => {
-      it('dispatches goToRoute action with path', done => {
-        const branchId = branches[0].name;
-        const expectedPath = `/project/${projectData.name_with_namespace}/edit/${branchId}`;
-        testAction(
-          openBranch,
-          branchId,
-          mockedState,
-          [],
-          [{ type: 'goToRoute', payload: expectedPath }],
-          done,
-        );
-      });
-    });
   });
 });