Always pre-select "Start a new merge request"

One exception: there is an existing MR for the current branch and the
branch is non-default and non-protected.

Extended mock_data for ide/stores to have different types of branches:
default, protected and regular

Cleaned new MR checkbox view

app/assets/javascripts/ide/components/commit_sidebar/actions.vue

@@ -41,10 +41,16 @@ export default {
   methods: {
     ...mapCommitActions(['updateCommitAction']),
     updateSelectedCommitAction() {
-      if (this.currentBranch && !this.currentBranch.can_push) {
-        this.updateCommitAction(consts.COMMIT_TO_NEW_BRANCH);
-      } else if (this.containsStagedChanges) {
+      if (!this.currentBranch) {
+        return;
+      }
+
+      const { can_push: canPush = false, default: isDefault = false } = this.currentBranch;
+
+      if (canPush && !isDefault) {
         this.updateCommitAction(consts.COMMIT_TO_CURRENT_BRANCH);
+      } else {
+        this.updateCommitAction(consts.COMMIT_TO_NEW_BRANCH);
       }
     },
   },

app/assets/javascripts/ide/components/commit_sidebar/new_merge_request_option.vue

 <script>
-import { mapGetters, createNamespacedHelpers } from 'vuex';
+import { createNamespacedHelpers } from 'vuex';
 
 const {
   mapState: mapCommitState,
-  mapGetters: mapCommitGetters,
   mapActions: mapCommitActions,
+  mapGetters: mapCommitGetters,
 } = createNamespacedHelpers('commit');
 
 export default {
   computed: {
     ...mapCommitState(['shouldCreateMR']),
-    ...mapCommitGetters(['isCommittingToCurrentBranch', 'isCommittingToDefaultBranch']),
-    ...mapGetters(['hasMergeRequest', 'isOnDefaultBranch']),
-    currentBranchHasMr() {
-      return this.hasMergeRequest && this.isCommittingToCurrentBranch;
-    },
-    showNewMrOption() {
-      return (
-        this.isCommittingToDefaultBranch || !this.currentBranchHasMr || this.isCommittingToNewBranch
-      );
-    },
-  },
-  mounted() {
-    this.setShouldCreateMR();
+    ...mapCommitGetters(['shouldHideNewMrOption']),
   },
   methods: {
-    ...mapCommitActions(['toggleShouldCreateMR', 'setShouldCreateMR']),
+    ...mapCommitActions(['toggleShouldCreateMR']),
   },
 };
 </script>
 
 <template>
-  <div v-if="showNewMrOption">
+  <fieldset v-if="!shouldHideNewMrOption">
     <hr class="my-2" />
-    <label class="mb-0">
-      <input :checked="shouldCreateMR" type="checkbox" @change="toggleShouldCreateMR" />
+    <label class="mb-0 js-ide-commit-new-mr">
+      <input
+        :checked="shouldCreateMR"
+        type="checkbox"
+        data-qa-selector="start_new_mr_checkbox"
+        @change="toggleShouldCreateMR"
+      />
       <span class="prepend-left-10">
         {{ __('Start a new merge request') }}
       </span>
     </label>
-  </div>
+  </fieldset>
 </template>

app/assets/javascripts/ide/stores/getters.js

@@ -104,5 +104,8 @@ export const packageJson = state => state.entries[packageJsonPath];
 export const isOnDefaultBranch = (_state, getters) =>
   getters.currentProject && getters.currentProject.default_branch === getters.branchName;
 
+export const canPushToBranch = (_state, getters) =>
+  getters.currentBranch && getters.currentBranch.can_push;
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

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

@@ -18,34 +18,15 @@ export const discardDraft = ({ commit }) => {
   commit(types.UPDATE_COMMIT_MESSAGE, '');
 };
 
-export const updateCommitAction = ({ commit, dispatch }, commitAction) => {
+export const updateCommitAction = ({ commit, getters }, commitAction) => {
   commit(types.UPDATE_COMMIT_ACTION, {
     commitAction,
   });
-  dispatch('setShouldCreateMR');
+  commit(types.TOGGLE_SHOULD_CREATE_MR, !getters.shouldHideNewMrOption);
 };
 
 export const toggleShouldCreateMR = ({ commit }) => {
   commit(types.TOGGLE_SHOULD_CREATE_MR);
-  commit(types.INTERACT_WITH_NEW_MR);
-};
-
-export const setShouldCreateMR = ({
-  commit,
-  getters,
-  rootGetters,
-  state: { interactedWithNewMR },
-}) => {
-  const committingToExistingMR =
-    getters.isCommittingToCurrentBranch &&
-    rootGetters.hasMergeRequest &&
-    !rootGetters.isOnDefaultBranch;
-
-  if ((getters.isCommittingToDefaultBranch && !interactedWithNewMR) || committingToExistingMR) {
-    commit(types.TOGGLE_SHOULD_CREATE_MR, false);
-  } else if (!interactedWithNewMR) {
-    commit(types.TOGGLE_SHOULD_CREATE_MR, true);
-  }
 };
 
 export const updateBranchName = ({ commit }, branchName) => {

app/assets/javascripts/ide/stores/modules/commit/getters.js

@@ -20,7 +20,7 @@ export const placeholderBranchName = (state, _, rootState) =>
   )}`;
 
 export const branchName = (state, getters, rootState) => {
-  if (state.commitAction === consts.COMMIT_TO_NEW_BRANCH) {
+  if (getters.isCreatingNewBranch) {
     if (state.newBranchName === '') {
       return getters.placeholderBranchName;
     }
@@ -48,11 +48,11 @@ export const preBuiltCommitMessage = (state, _, rootState) => {
 
 export const isCreatingNewBranch = state => state.commitAction === consts.COMMIT_TO_NEW_BRANCH;
 
-export const isCommittingToCurrentBranch = state =>
-  state.commitAction === consts.COMMIT_TO_CURRENT_BRANCH;
-
-export const isCommittingToDefaultBranch = (_state, getters, _rootState, rootGetters) =>
-  getters.isCommittingToCurrentBranch && rootGetters.isOnDefaultBranch;
+export const shouldHideNewMrOption = (_state, getters, _rootState, rootGetters) =>
+  !getters.isCreatingNewBranch &&
+  (rootGetters.hasMergeRequest ||
+    (!rootGetters.hasMergeRequest && rootGetters.isOnDefaultBranch)) &&
+  rootGetters.canPushToBranch;
 
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

app/assets/javascripts/ide/stores/modules/commit/mutation_types.js

@@ -3,4 +3,3 @@ export const UPDATE_COMMIT_ACTION = 'UPDATE_COMMIT_ACTION';
 export const UPDATE_NEW_BRANCH_NAME = 'UPDATE_NEW_BRANCH_NAME';
 export const UPDATE_LOADING = 'UPDATE_LOADING';
 export const TOGGLE_SHOULD_CREATE_MR = 'TOGGLE_SHOULD_CREATE_MR';
-export const INTERACT_WITH_NEW_MR = 'INTERACT_WITH_NEW_MR';

app/assets/javascripts/ide/stores/modules/commit/mutations.js

@@ -24,7 +24,4 @@ export default {
       shouldCreateMR: shouldCreateMR === undefined ? !state.shouldCreateMR : shouldCreateMR,
     });
   },
-  [types.INTERACT_WITH_NEW_MR](state) {
-    Object.assign(state, { interactedWithNewMR: true });
-  },
 };

app/assets/javascripts/ide/stores/modules/commit/state.js

@@ -3,6 +3,5 @@ export default () => ({
   commitAction: '1',
   newBranchName: '',
   submitCommitLoading: false,
-  shouldCreateMR: false,
-  interactedWithNewMR: false,
+  shouldCreateMR: true,
 });

changelogs/unreleased/51470-webide-default-commit.yml

+---
+title: Updated WebIDE default commit options
+merge_request: 31449
+author:
+type: changed

qa/qa/page/project/web_ide/edit.rb

@@ -39,6 +39,10 @@ module QA
             element :commit_button
           end
 
+          view 'app/assets/javascripts/ide/components/commit_sidebar/new_merge_request_option.vue' do
+            element :start_new_mr_checkbox
+          end
+
           def has_file?(file_name)
             within_element(:file_list) do
               page.has_content? file_name
@@ -100,6 +104,7 @@ module QA
             # animation is still in process even when the buttons have the
             # expected visibility.
             commit_success_msg_shown = retry_until do
+              uncheck_element :start_new_mr_checkbox
               click_element :commit_button
 
               wait(reload: false) do

spec/features/projects/tree/create_directory_spec.rb

@@ -32,10 +32,12 @@ describe 'Multi-file editor new directory', :js do
       click_button('Create directory')
     end
 
+    expect(page).to have_content('folder name')
+
     first('.ide-tree-actions button').click
 
-    page.within('.modal-dialog') do
-      find('.form-control').set('file name')
+    page.within('.modal') do
+      find('.form-control').set('folder name/file name')
 
       click_button('Create file')
     end
@@ -44,13 +46,18 @@ describe 'Multi-file editor new directory', :js do
 
     find('.js-ide-commit-mode').click
 
-    find('.multi-file-commit-list-item').hover
     click_button 'Stage'
 
     fill_in('commit-message', with: 'commit message ide')
 
+    find(:css, ".js-ide-commit-new-mr input").set(false)
+
+    wait_for_requests
+
     page.within '.multi-file-commit-form' do
       click_button('Commit')
+
+      wait_for_requests
     end
 
     find('.js-ide-edit-mode').click

spec/features/projects/tree/create_file_spec.rb

@@ -36,15 +36,20 @@ describe 'Multi-file editor new file', :js do
 
     find('.js-ide-commit-mode').click
 
-    find('.multi-file-commit-list-item').hover
     click_button 'Stage'
 
     fill_in('commit-message', with: 'commit message ide')
 
+    find(:css, ".js-ide-commit-new-mr input").set(false)
+
     page.within '.multi-file-commit-form' do
       click_button('Commit')
+
+      wait_for_requests
     end
 
+    find('.js-ide-edit-mode').click
+
     expect(page).to have_content('file name')
   end
 end

spec/frontend/ide/stores/modules/commit/mutations_spec.js

@@ -62,12 +62,4 @@ describe('IDE commit module mutations', () => {
       expect(state.shouldCreateMR).toBe(false);
     });
   });
-
-  describe('INTERACT_WITH_NEW_MR', () => {
-    it('sets interactedWithNewMR to true', () => {
-      mutations.INTERACT_WITH_NEW_MR(state);
-
-      expect(state.interactedWithNewMR).toBe(true);
-    });
-  });
 });

spec/javascripts/ide/components/commit_sidebar/actions_spec.js

 import Vue from 'vue';
-import store from '~/ide/stores';
-import consts from '~/ide/stores/modules/commit/constants';
+import { createStore } from '~/ide/stores';
 import commitActions from '~/ide/components/commit_sidebar/actions.vue';
+import consts from '~/ide/stores/modules/commit/constants';
 import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
-import { resetStore } from 'spec/ide/helpers';
-import { projectData } from 'spec/ide/mock_data';
+import { projectData, branches } from 'spec/ide/mock_data';
+
+const ACTION_UPDATE_COMMIT_ACTION = 'commit/updateCommitAction';
 
 describe('IDE commit sidebar actions', () => {
+  let store;
   let vm;
-  const createComponent = ({
-    hasMR = false,
-    commitAction = consts.COMMIT_TO_NEW_BRANCH,
-    mergeRequestsEnabled = true,
-    currentBranchId = 'master',
-    shouldCreateMR = false,
-  } = {}) => {
+
+  const createComponent = ({ hasMR = false, currentBranchId = 'master' } = {}) => {
     const Component = Vue.extend(commitActions);
 
     vm = createComponentWithStore(Component, store);
 
     vm.$store.state.currentBranchId = currentBranchId;
     vm.$store.state.currentProjectId = 'abcproject';
-    vm.$store.state.commit.commitAction = commitAction;
-    Vue.set(vm.$store.state.projects, 'abcproject', { ...projectData });
-    vm.$store.state.projects.abcproject.merge_requests_enabled = mergeRequestsEnabled;
-    vm.$store.state.commit.shouldCreateMR = shouldCreateMR;
+
+    const proj = { ...projectData };
+    proj.branches[currentBranchId] = branches.find(branch => branch.name === currentBranchId);
+
+    Vue.set(vm.$store.state.projects, 'abcproject', proj);
 
     if (hasMR) {
       vm.$store.state.currentMergeRequestId = '1';
@@ -33,13 +31,19 @@ describe('IDE commit sidebar actions', () => {
       ] = { foo: 'bar' };
     }
 
-    return vm.$mount();
+    vm.$mount();
+
+    return vm;
   };
 
+  beforeEach(() => {
+    store = createStore();
+    spyOn(store, 'dispatch');
+  });
+
   afterEach(() => {
     vm.$destroy();
-
-    resetStore(vm.$store);
+    vm = null;
   });
 
   it('renders 2 groups', () => {
@@ -73,4 +77,152 @@ describe('IDE commit sidebar actions', () => {
       expect(vm.commitToCurrentBranchText).not.toContain(injectedSrc);
     });
   });
+
+  describe('updateSelectedCommitAction', () => {
+    it('does not return anything if currentBranch does not exist', () => {
+      createComponent({ currentBranchId: null });
+
+      expect(vm.$store.dispatch).not.toHaveBeenCalled();
+    });
+
+    it('calls again after staged changes', done => {
+      createComponent({ currentBranchId: null });
+
+      vm.$store.state.currentBranchId = 'master';
+      vm.$store.state.changedFiles.push({});
+      vm.$store.state.stagedFiles.push({});
+
+      vm.$nextTick()
+        .then(() => {
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            jasmine.anything(),
+          );
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    describe('default branch', () => {
+      it('dispatches correct action for default branch', () => {
+        createComponent({
+          currentBranchId: 'master',
+        });
+
+        expect(vm.$store.dispatch).toHaveBeenCalledTimes(1);
+        expect(vm.$store.dispatch).toHaveBeenCalledWith(
+          ACTION_UPDATE_COMMIT_ACTION,
+          consts.COMMIT_TO_NEW_BRANCH,
+        );
+      });
+    });
+
+    describe('protected branch', () => {
+      describe('with write access', () => {
+        it('dispatches correct action when MR exists', () => {
+          createComponent({
+            hasMR: true,
+            currentBranchId: 'protected/access',
+          });
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            consts.COMMIT_TO_CURRENT_BRANCH,
+          );
+        });
+
+        it('dispatches correct action when MR does not exists', () => {
+          createComponent({
+            hasMR: false,
+            currentBranchId: 'protected/access',
+          });
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            consts.COMMIT_TO_CURRENT_BRANCH,
+          );
+        });
+      });
+
+      describe('without write access', () => {
+        it('dispatches correct action when MR exists', () => {
+          createComponent({
+            hasMR: true,
+            currentBranchId: 'protected/no-access',
+          });
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            consts.COMMIT_TO_NEW_BRANCH,
+          );
+        });
+
+        it('dispatches correct action when MR does not exists', () => {
+          createComponent({
+            hasMR: false,
+            currentBranchId: 'protected/no-access',
+          });
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            consts.COMMIT_TO_NEW_BRANCH,
+          );
+        });
+      });
+    });
+
+    describe('regular branch', () => {
+      describe('with write access', () => {
+        it('dispatches correct action when MR exists', () => {
+          createComponent({
+            hasMR: true,
+            currentBranchId: 'regular',
+          });
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            consts.COMMIT_TO_CURRENT_BRANCH,
+          );
+        });
+
+        it('dispatches correct action when MR does not exists', () => {
+          createComponent({
+            hasMR: false,
+            currentBranchId: 'regular',
+          });
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            consts.COMMIT_TO_CURRENT_BRANCH,
+          );
+        });
+      });
+
+      describe('without write access', () => {
+        it('dispatches correct action when MR exists', () => {
+          createComponent({
+            hasMR: true,
+            currentBranchId: 'regular/no-access',
+          });
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            consts.COMMIT_TO_NEW_BRANCH,
+          );
+        });
+
+        it('dispatches correct action when MR does not exists', () => {
+          createComponent({
+            hasMR: false,
+            currentBranchId: 'regular/no-access',
+          });
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith(
+            ACTION_UPDATE_COMMIT_ACTION,
+            consts.COMMIT_TO_NEW_BRANCH,
+          );
+        });
+      });
+    });
+  });
 });

spec/javascripts/ide/components/commit_sidebar/new_merge_request_option_spec.js

 import Vue from 'vue';
 import store from '~/ide/stores';
-import consts from '~/ide/stores/modules/commit/constants';
 import NewMergeRequestOption from '~/ide/components/commit_sidebar/new_merge_request_option.vue';
 import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
-import { projectData } from 'spec/ide/mock_data';
+import { projectData, branches } from 'spec/ide/mock_data';
 import { resetStore } from 'spec/ide/helpers';
+import consts from '../../../../../app/assets/javascripts/ide/stores/modules/commit/constants';
 
 describe('create new MR checkbox', () => {
   let vm;
-  const createComponent = ({
-    hasMR = false,
-    commitAction = consts.COMMIT_TO_NEW_BRANCH,
-    currentBranchId = 'master',
-  } = {}) => {
+  const setMR = () => {
+    vm.$store.state.currentMergeRequestId = '1';
+    vm.$store.state.projects[store.state.currentProjectId].mergeRequests[
+      store.state.currentMergeRequestId
+    ] = { foo: 'bar' };
+  };
+
+  const createComponent = ({ currentBranchId = 'master', createNewBranch = false } = {}) => {
     const Component = Vue.extend(NewMergeRequestOption);
 
     vm = createComponentWithStore(Component, store);
 
+    vm.$store.state.commit.commitAction = createNewBranch
+      ? consts.COMMIT_TO_NEW_BRANCH
+      : consts.COMMIT_TO_CURRENT_BRANCH;
+
     vm.$store.state.currentBranchId = currentBranchId;
     vm.$store.state.currentProjectId = 'abcproject';
-    vm.$store.state.commit.commitAction = commitAction;
-    Vue.set(vm.$store.state.projects, 'abcproject', { ...projectData });
 
-    if (hasMR) {
-      vm.$store.state.currentMergeRequestId = '1';
-      vm.$store.state.projects[store.state.currentProjectId].mergeRequests[
-        store.state.currentMergeRequestId
-      ] = { foo: 'bar' };
-    }
+    const proj = JSON.parse(JSON.stringify(projectData));
+    proj.branches[currentBranchId] = branches.find(branch => branch.name === currentBranchId);
+
+    Vue.set(vm.$store.state.projects, 'abcproject', proj);
 
     return vm.$mount();
   };
@@ -38,30 +41,131 @@ describe('create new MR checkbox', () => {
     resetStore(vm.$store);
   });
 
-  it('is hidden when an MR already exists and committing to current branch', () => {
+  describe('for default branch', () => {
+    describe('is rendered when pushing to a new branch', () => {
+      beforeEach(() => {
+        createComponent({
+          currentBranchId: 'master',
+          createNewBranch: true,
+        });
+      });
+
+      it('has NO new MR', () => {
+        expect(vm.$el.textContent).not.toBe('');
+      });
+
+      it('has new MR', done => {
+        setMR();
+
+        vm.$nextTick()
+          .then(() => {
+            expect(vm.$el.textContent).not.toBe('');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+
+    describe('is NOT rendered when pushing to the same branch', () => {
+      beforeEach(() => {
+        createComponent({
+          currentBranchId: 'master',
+          createNewBranch: false,
+        });
+      });
+
+      it('has NO new MR', () => {
+        expect(vm.$el.textContent).toBe('');
+      });
+
+      it('has new MR', done => {
+        setMR();
+
+        vm.$nextTick()
+          .then(() => {
+            expect(vm.$el.textContent).toBe('');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+  });
+
+  describe('for protected branch', () => {
+    describe('when user does not have the write access', () => {
+      beforeEach(() => {
         createComponent({
-      hasMR: true,
-      commitAction: consts.COMMIT_TO_CURRENT_BRANCH,
-      currentBranchId: 'feature',
+          currentBranchId: 'protected/no-access',
+        });
       });
 
+      it('is rendered if MR does not exists', () => {
+        expect(vm.$el.textContent).not.toBe('');
+      });
+
+      it('is rendered if MR exists', done => {
+        setMR();
+
+        vm.$nextTick()
+          .then(() => {
+            expect(vm.$el.textContent).not.toBe('');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+
+    describe('when user has the write access', () => {
+      beforeEach(() => {
+        createComponent({
+          currentBranchId: 'protected/access',
+        });
+      });
+
+      it('is rendered if MR does not exist', () => {
+        expect(vm.$el.textContent).not.toBe('');
+      });
+
+      it('is hidden if MR exists', done => {
+        setMR();
+
+        vm.$nextTick()
+          .then(() => {
             expect(vm.$el.textContent).toBe('');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
   });
 
-  it('does not hide checkbox if MR does not exist', () => {
-    createComponent({ hasMR: false });
+  describe('for regular branch', () => {
+    beforeEach(() => {
+      createComponent({
+        currentBranchId: 'regular',
+      });
+    });
 
-    expect(vm.$el.querySelector('input[type="checkbox"]').hidden).toBe(false);
+    it('is rendered if no MR exists', () => {
+      expect(vm.$el.textContent).not.toBe('');
     });
 
-  it('does not hide checkbox when creating a new branch', () => {
-    createComponent({ commitAction: consts.COMMIT_TO_NEW_BRANCH });
+    it('is hidden if MR exists', done => {
+      setMR();
 
-    expect(vm.$el.querySelector('input[type="checkbox"]').hidden).toBe(false);
+      vm.$nextTick()
+        .then(() => {
+          expect(vm.$el.textContent).toBe('');
+        })
+        .then(done)
+        .catch(done.fail);
+    });
   });
 
   it('dispatches toggleShouldCreateMR when clicking checkbox', () => {
-    createComponent();
+    createComponent({
+      currentBranchId: 'regular',
+    });
     const el = vm.$el.querySelector('input[type="checkbox"]');
     spyOn(vm.$store, 'dispatch');
     el.dispatchEvent(new Event('change'));

spec/javascripts/ide/mock_data.js

@@ -176,23 +176,51 @@ export const branches = [
       committed_date: '2018-08-01T00:20:05Z',
     },
     can_push: true,
+    protected: true,
+    default: true,
   },
   {
     id: 2,
-    name: 'feature/lorem-ipsum',
+    name: 'protected/no-access',
     commit: {
       message: 'Update some stuff',
       committed_date: '2018-08-02T00:00:05Z',
     },
-    can_push: true,
+    can_push: false,
+    protected: true,
+    default: false,
   },
   {
     id: 3,
-    name: 'feature/dolar-amit',
+    name: 'protected/access',
+    commit: {
+      message: 'Update some stuff',
+      committed_date: '2018-08-02T00:00:05Z',
+    },
+    can_push: true,
+    protected: true,
+    default: false,
+  },
+  {
+    id: 4,
+    name: 'regular',
     commit: {
       message: 'Update some more stuff',
       committed_date: '2018-06-30T00:20:05Z',
     },
     can_push: true,
+    protected: false,
+    default: false,
+  },
+  {
+    id: 5,
+    name: 'regular/no-access',
+    commit: {
+      message: 'Update some more stuff',
+      committed_date: '2018-06-30T00:20:05Z',
+    },
+    can_push: false,
+    protected: false,
+    default: false,
   },
 ];

spec/javascripts/ide/stores/getters_spec.js

@@ -221,4 +221,36 @@ describe('IDE store getters', () => {
       });
     });
   });
+
+  describe('canPushToBranch', () => {
+    it('returns false when no currentBranch exists', () => {
+      const localGetters = {
+        currentProject: undefined,
+      };
+
+      expect(getters.canPushToBranch({}, localGetters)).toBeFalsy();
+    });
+
+    it('returns true when can_push to currentBranch', () => {
+      const localGetters = {
+        currentProject: {
+          default_branch: 'master',
+        },
+        currentBranch: { can_push: true },
+      };
+
+      expect(getters.canPushToBranch({}, localGetters)).toBeTruthy();
+    });
+
+    it('returns false when !can_push to currentBranch', () => {
+      const localGetters = {
+        currentProject: {
+          default_branch: 'master',
+        },
+        currentBranch: { can_push: false },
+      };
+
+      expect(getters.canPushToBranch({}, localGetters)).toBeFalsy();
+    });
+  });
 });

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

@@ -57,6 +57,44 @@ describe('IDE commit module actions', () => {
         .then(done)
         .catch(done.fail);
     });
+
+    it('sets shouldCreateMR to true if "Create new MR" option is visible', done => {
+      store.state.shouldHideNewMrOption = false;
+
+      testAction(
+        actions.updateCommitAction,
+        {},
+        store.state,
+        [
+          {
+            type: mutationTypes.UPDATE_COMMIT_ACTION,
+            payload: { commitAction: jasmine.anything() },
+          },
+          { type: mutationTypes.TOGGLE_SHOULD_CREATE_MR, payload: true },
+        ],
+        [],
+        done,
+      );
+    });
+
+    it('sets shouldCreateMR to false if "Create new MR" option is hidden', done => {
+      store.state.shouldHideNewMrOption = true;
+
+      testAction(
+        actions.updateCommitAction,
+        {},
+        store.state,
+        [
+          {
+            type: mutationTypes.UPDATE_COMMIT_ACTION,
+            payload: { commitAction: jasmine.anything() },
+          },
+          { type: mutationTypes.TOGGLE_SHOULD_CREATE_MR, payload: false },
+        ],
+        [],
+        done,
+      );
+    });
   });
 
   describe('updateBranchName', () => {
@@ -541,147 +579,10 @@ describe('IDE commit module actions', () => {
         actions.toggleShouldCreateMR,
         {},
         store.state,
-        [
-          { type: mutationTypes.TOGGLE_SHOULD_CREATE_MR },
-          { type: mutationTypes.INTERACT_WITH_NEW_MR },
-        ],
+        [{ type: mutationTypes.TOGGLE_SHOULD_CREATE_MR }],
         [],
         done,
       );
     });
   });
-
-  describe('setShouldCreateMR', () => {
-    beforeEach(() => {
-      store.state.projects = {
-        project: {
-          default_branch: 'master',
-          branches: {
-            master: {
-              name: 'master',
-            },
-            feature: {
-              name: 'feature',
-            },
-          },
-        },
-      };
-
-      store.state.currentProjectId = 'project';
-    });
-
-    it('sets to false when the current branch already has an MR', done => {
-      store.state.commit.currentMergeRequestId = 1;
-      store.state.commit.commitAction = consts.COMMIT_TO_CURRENT_BRANCH;
-      store.state.currentMergeRequestId = '1';
-      store.state.currentBranchId = 'feature';
-      spyOn(store, 'commit').and.callThrough();
-
-      store
-        .dispatch('commit/setShouldCreateMR')
-        .then(() => {
-          expect(store.commit.calls.allArgs()[0]).toEqual(
-            jasmine.arrayContaining([`commit/${mutationTypes.TOGGLE_SHOULD_CREATE_MR}`, false]),
-          );
-          done();
-        })
-        .catch(done.fail);
-    });
-
-    it('changes to false when current branch is the default branch and user has not interacted', done => {
-      store.state.commit.interactedWithNewMR = false;
-      store.state.currentBranchId = 'master';
-      store.state.commit.commitAction = consts.COMMIT_TO_CURRENT_BRANCH;
-      spyOn(store, 'commit').and.callThrough();
-
-      store
-        .dispatch('commit/setShouldCreateMR')
-        .then(() => {
-          expect(store.commit.calls.allArgs()[0]).toEqual(
-            jasmine.arrayContaining([`commit/${mutationTypes.TOGGLE_SHOULD_CREATE_MR}`, false]),
-          );
-          done();
-        })
-        .catch(done.fail);
-    });
-
-    it('changes to true when "create new branch" is selected and user has not interacted', done => {
-      store.state.commit.commitAction = consts.COMMIT_TO_NEW_BRANCH;
-      store.state.commit.interactedWithNewMR = false;
-      spyOn(store, 'commit').and.callThrough();
-
-      store
-        .dispatch('commit/setShouldCreateMR')
-        .then(() => {
-          expect(store.commit.calls.allArgs()[0]).toEqual(
-            jasmine.arrayContaining([`commit/${mutationTypes.TOGGLE_SHOULD_CREATE_MR}`, true]),
-          );
-          done();
-        })
-        .catch(done.fail);
-    });
-
-    it('does not change anything if user has interacted and comitting to new branch', done => {
-      store.state.commit.commitAction = consts.COMMIT_TO_NEW_BRANCH;
-      store.state.commit.interactedWithNewMR = true;
-      spyOn(store, 'commit').and.callThrough();
-
-      store
-        .dispatch('commit/setShouldCreateMR')
-        .then(() => {
-          expect(store.commit).not.toHaveBeenCalled();
-          done();
-        })
-        .catch(done.fail);
-    });
-
-    it('does not change anything if user has interacted and comitting to branch without MR', done => {
-      store.state.commit.commitAction = consts.COMMIT_TO_CURRENT_BRANCH;
-      store.state.commit.currentMergeRequestId = null;
-      store.state.commit.interactedWithNewMR = true;
-      spyOn(store, 'commit').and.callThrough();
-
-      store
-        .dispatch('commit/setShouldCreateMR')
-        .then(() => {
-          expect(store.commit).not.toHaveBeenCalled();
-          done();
-        })
-        .catch(done.fail);
-    });
-
-    it('still changes to false if hiding the checkbox', done => {
-      store.state.currentBranchId = 'feature';
-      store.state.commit.commitAction = consts.COMMIT_TO_CURRENT_BRANCH;
-      store.state.currentMergeRequestId = '1';
-      store.state.commit.interactedWithNewMR = true;
-      spyOn(store, 'commit').and.callThrough();
-
-      store
-        .dispatch('commit/setShouldCreateMR')
-        .then(() => {
-          expect(store.commit.calls.allArgs()[0]).toEqual(
-            jasmine.arrayContaining([`commit/${mutationTypes.TOGGLE_SHOULD_CREATE_MR}`, false]),
-          );
-          done();
-        })
-        .catch(done.fail);
-    });
-
-    it('does not change to false when on master and user has interacted even if MR exists', done => {
-      store.state.currentBranchId = 'master';
-      store.state.commit.commitAction = consts.COMMIT_TO_CURRENT_BRANCH;
-      store.state.currentMergeRequestId = '1';
-      store.state.commit.interactedWithNewMR = true;
-      spyOn(store, 'commit').and.callThrough();
-
-      store
-        .dispatch('commit/setShouldCreateMR')
-        .then(() => {
-          expect(store.commit).not.toHaveBeenCalled();
-          done();
-        })
-        .catch(done.fail);
-    });
-  });
 });

spec/javascripts/ide/stores/modules/commit/getters_spec.js

 import commitState from '~/ide/stores/modules/commit/state';
-import consts from '~/ide/stores/modules/commit/constants';
 import * as getters from '~/ide/stores/modules/commit/getters';
+import consts from '~/ide/stores/modules/commit/constants';
 
 describe('IDE commit module getters', () => {
   let state;
@@ -55,15 +55,15 @@ describe('IDE commit module getters', () => {
       });
     });
 
-    it('defualts to currentBranchId', () => {
-      expect(getters.branchName(state, null, rootState)).toBe('master');
+    it('defaults to currentBranchId when not committing to a new branch', () => {
+      localGetters.isCreatingNewBranch = false;
+
+      expect(getters.branchName(state, localGetters, rootState)).toBe('master');
     });
 
-    describe('COMMIT_TO_NEW_BRANCH', () => {
+    describe('commit to a new branch', () => {
       beforeEach(() => {
-        Object.assign(state, {
-          commitAction: consts.COMMIT_TO_NEW_BRANCH,
-        });
+        localGetters.isCreatingNewBranch = true;
       });
 
       it('uses newBranchName when not empty', () => {
@@ -144,4 +144,152 @@ describe('IDE commit module getters', () => {
       });
     });
   });
+
+  describe('isCreatingNewBranch', () => {
+    it('returns false if NOT creating a new branch', () => {
+      state.commitAction = consts.COMMIT_TO_CURRENT_BRANCH;
+
+      expect(getters.isCreatingNewBranch(state)).toBeFalsy();
+    });
+
+    it('returns true if creating a new branch', () => {
+      state.commitAction = consts.COMMIT_TO_NEW_BRANCH;
+
+      expect(getters.isCreatingNewBranch(state)).toBeTruthy();
+    });
+  });
+
+  describe('shouldHideNewMrOption', () => {
+    let localGetters = {};
+    let rootGetters = {};
+
+    beforeEach(() => {
+      localGetters = {
+        isCreatingNewBranch: null,
+      };
+      rootGetters = {
+        isOnDefaultBranch: null,
+        hasMergeRequest: null,
+        canPushToBranch: null,
+      };
+    });
+
+    describe('NO existing MR for the branch', () => {
+      beforeEach(() => {
+        rootGetters.hasMergeRequest = false;
+      });
+
+      it('should never hide "New MR" option', () => {
+        expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeFalsy();
+      });
+    });
+
+    describe('existing MR for the branch', () => {
+      beforeEach(() => {
+        rootGetters.hasMergeRequest = true;
+      });
+
+      it('should NOT hide "New MR" option if user can NOT push to the current branch', () => {
+        rootGetters.canPushToBranch = false;
+
+        expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeFalsy();
+      });
+
+      it('should hide "New MR" option if user can push to the current branch', () => {
+        rootGetters.canPushToBranch = true;
+
+        expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeTruthy();
+      });
+    });
+
+    describe('user can NOT push the branch', () => {
+      beforeEach(() => {
+        rootGetters.canPushToBranch = false;
+      });
+
+      it('should never hide "New MR" option', () => {
+        expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeFalsy();
+      });
+    });
+
+    describe('user can push to the branch', () => {
+      beforeEach(() => {
+        rootGetters.canPushToBranch = true;
+      });
+
+      it('should NOT hide "New MR" option if there is NO existing MR for the current branch', () => {
+        rootGetters.hasMergeRequest = false;
+
+        expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeFalsy();
+      });
+
+      it('should hide "New MR" option if there is existing MR for the current branch', () => {
+        rootGetters.hasMergeRequest = true;
+
+        expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeTruthy();
+      });
+    });
+
+    describe('default branch', () => {
+      beforeEach(() => {
+        rootGetters.isOnDefaultBranch = true;
+      });
+
+      describe('committing to the same branch', () => {
+        beforeEach(() => {
+          localGetters.isCreatingNewBranch = false;
+          rootGetters.canPushToBranch = true;
+        });
+
+        it('should hide "New MR" when there is an existing MR', () => {
+          rootGetters.hasMergeRequest = true;
+
+          expect(
+            getters.shouldHideNewMrOption(state, localGetters, null, rootGetters),
+          ).toBeTruthy();
+        });
+
+        it('should hide "New MR" when there is no existing MR', () => {
+          rootGetters.hasMergeRequest = false;
+
+          expect(
+            getters.shouldHideNewMrOption(state, localGetters, null, rootGetters),
+          ).toBeTruthy();
+        });
+      });
+
+      describe('creating a new branch', () => {
+        beforeEach(() => {
+          localGetters.isCreatingNewBranch = true;
+        });
+
+        it('should NOT hide "New MR" option no matter existence of an MR or write access', () => {
+          rootGetters.hasMergeRequest = false;
+          rootGetters.canPushToBranch = true;
+
+          expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeFalsy();
+
+          rootGetters.hasMergeRequest = true;
+          rootGetters.canPushToBranch = true;
+
+          expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeFalsy();
+
+          rootGetters.hasMergeRequest = false;
+          rootGetters.canPushToBranch = false;
+
+          expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeFalsy();
+        });
+      });
+    });
+
+    it('should never hide "New MR" option when creating a new branch', () => {
+      localGetters.isCreatingNewBranch = true;
+
+      rootGetters.isOnDefaultBranch = false;
+      rootGetters.hasMergeRequest = true;
+      rootGetters.canPushToBranch = true;
+
+      expect(getters.shouldHideNewMrOption(state, localGetters, null, rootGetters)).toBeFalsy();
+    });
+  });
 });