From 371e1ed3716d25f48c6458373e64ea81e0a45946 Mon Sep 17 00:00:00 2001
From: Denys Mishunov <dmishunov@gitlab.com>
Date: Thu, 22 Aug 2019 23:23:06 +0000
Subject: 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
---
 .../ide/components/commit_sidebar/actions.vue      | 12 ++++++--
 .../commit_sidebar/new_merge_request_option.vue    | 33 +++++++++-------------
 app/assets/javascripts/ide/stores/getters.js       |  3 ++
 .../ide/stores/modules/commit/actions.js           | 23 ++-------------
 .../ide/stores/modules/commit/getters.js           | 12 ++++----
 .../ide/stores/modules/commit/mutation_types.js    |  1 -
 .../ide/stores/modules/commit/mutations.js         |  3 --
 .../javascripts/ide/stores/modules/commit/state.js |  3 +-
 8 files changed, 34 insertions(+), 56 deletions(-)

(limited to 'app')

diff --git a/app/assets/javascripts/ide/components/commit_sidebar/actions.vue b/app/assets/javascripts/ide/components/commit_sidebar/actions.vue
index 685d8a6b245..8b356ee6e97 100644
--- a/app/assets/javascripts/ide/components/commit_sidebar/actions.vue
+++ b/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);
       }
     },
   },
diff --git a/app/assets/javascripts/ide/components/commit_sidebar/new_merge_request_option.vue b/app/assets/javascripts/ide/components/commit_sidebar/new_merge_request_option.vue
index b2e7b15089c..daa44a42765 100644
--- a/app/assets/javascripts/ide/components/commit_sidebar/new_merge_request_option.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/new_merge_request_option.vue
@@ -1,43 +1,36 @@
 <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>
diff --git a/app/assets/javascripts/ide/stores/getters.js b/app/assets/javascripts/ide/stores/getters.js
index 406903129db..85fd45358be 100644
--- a/app/assets/javascripts/ide/stores/getters.js
+++ b/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 () => {};
diff --git a/app/assets/javascripts/ide/stores/modules/commit/actions.js b/app/assets/javascripts/ide/stores/modules/commit/actions.js
index ac34491c1ad..23caf2d48ed 100644
--- a/app/assets/javascripts/ide/stores/modules/commit/actions.js
+++ b/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) => {
diff --git a/app/assets/javascripts/ide/stores/modules/commit/getters.js b/app/assets/javascripts/ide/stores/modules/commit/getters.js
index 64779e9e4df..de289e27199 100644
--- a/app/assets/javascripts/ide/stores/modules/commit/getters.js
+++ b/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 () => {};
diff --git a/app/assets/javascripts/ide/stores/modules/commit/mutation_types.js b/app/assets/javascripts/ide/stores/modules/commit/mutation_types.js
index b81918156b0..7ad8f3570b7 100644
--- a/app/assets/javascripts/ide/stores/modules/commit/mutation_types.js
+++ b/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';
diff --git a/app/assets/javascripts/ide/stores/modules/commit/mutations.js b/app/assets/javascripts/ide/stores/modules/commit/mutations.js
index 14957d283bb..73b618e250f 100644
--- a/app/assets/javascripts/ide/stores/modules/commit/mutations.js
+++ b/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 });
-  },
 };
diff --git a/app/assets/javascripts/ide/stores/modules/commit/state.js b/app/assets/javascripts/ide/stores/modules/commit/state.js
index 53647a7e3e3..259577e48e0 100644
--- a/app/assets/javascripts/ide/stores/modules/commit/state.js
+++ b/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,
 });
-- 
cgit v1.2.1