Merge branch 'ide-commit-form-improvements' into 'master'

Improve Web IDE commit form

Closes #47307

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

9 files changed, 74 insertions, 28 deletions

diff --git a/app/assets/javascripts/ide/components/commit_sidebar/actions.vue b/app/assets/javascripts/ide/components/commit_sidebar/actions.vue
index b4f3778d946..eb7cb9745ec 100644
--- a/app/assets/javascripts/ide/components/commit_sidebar/actions.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/actions.vue
@@ -10,7 +10,7 @@ export default {
   },
   computed: {
     ...mapState(['currentBranchId', 'changedFiles', 'stagedFiles']),
-    ...mapGetters(['currentProject']),
+    ...mapGetters(['currentProject', 'currentBranch']),
     commitToCurrentBranchText() {
       return sprintf(
         __('Commit to %{branchName} branch'),
@@ -22,17 +22,30 @@ export default {
       return this.changedFiles.length > 0 && this.stagedFiles.length > 0;
     },
   },
+  watch: {
+    disableMergeRequestRadio() {
+      this.updateSelectedCommitAction();
+    },
+  },
   mounted() {
-    if (this.disableMergeRequestRadio) {
-      this.updateCommitAction(consts.COMMIT_TO_CURRENT_BRANCH);
-    }
+    this.updateSelectedCommitAction();
   },
   methods: {
     ...mapActions('commit', ['updateCommitAction']),
+    updateSelectedCommitAction() {
+      if (this.currentBranch && !this.currentBranch.can_push) {
+        this.updateCommitAction(consts.COMMIT_TO_NEW_BRANCH);
+      } else if (this.disableMergeRequestRadio) {
+        this.updateCommitAction(consts.COMMIT_TO_CURRENT_BRANCH);
+      }
+    },
   },
   commitToCurrentBranch: consts.COMMIT_TO_CURRENT_BRANCH,
   commitToNewBranch: consts.COMMIT_TO_NEW_BRANCH,
   commitToNewBranchMR: consts.COMMIT_TO_NEW_BRANCH_MR,
+  currentBranchPermissionsTooltip: __(
+    "This option is disabled as you don't have write permissions for the current branch",
+  ),
 };
 </script>
 
@@ -40,9 +53,11 @@ export default {
   <div class="append-bottom-15 ide-commit-radios">
     <radio-group
       :value="$options.commitToCurrentBranch"
-      :checked="true"
+      :disabled="currentBranch && !currentBranch.can_push"
+      :title="$options.currentBranchPermissionsTooltip"
     >
       <span
+        class="ide-radio-label"
         v-html="commitToCurrentBranchText"
       >
       </span>
@@ -56,6 +71,7 @@ export default {
       v-if="currentProject.merge_requests_enabled"
       :value="$options.commitToNewBranchMR"
       :label="__('Create a new branch and merge request')"
+      :title="__('This option is disabled while you still have unstaged changes')"
       :show-input="true"
       :disabled="disableMergeRequestRadio"
     />
diff --git a/app/assets/javascripts/ide/components/commit_sidebar/form.vue b/app/assets/javascripts/ide/components/commit_sidebar/form.vue
index 14c74687ab4..ee8eb206980 100644
--- a/app/assets/javascripts/ide/components/commit_sidebar/form.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/form.vue
@@ -24,7 +24,7 @@ export default {
     ...mapState(['changedFiles', 'stagedFiles', 'currentActivityView', 'lastCommitMsg']),
     ...mapState('commit', ['commitMessage', 'submitCommitLoading']),
     ...mapGetters(['hasChanges']),
-    ...mapGetters('commit', ['commitButtonDisabled', 'discardDraftButtonDisabled']),
+    ...mapGetters('commit', ['discardDraftButtonDisabled', 'preBuiltCommitMessage']),
     overviewText() {
       return sprintf(
         __(
@@ -36,6 +36,9 @@ export default {
         },
       );
     },
+    commitButtonText() {
+      return this.stagedFiles.length ? __('Commit') : __('Stage & Commit');
+    },
   },
   watch: {
     currentActivityView() {
@@ -136,14 +139,14 @@ export default {
         </transition>
         <commit-message-field
           :text="commitMessage"
+          :placeholder="preBuiltCommitMessage"
           @input="updateCommitMessage"
         />
         <div class="clearfix prepend-top-15">
           <actions />
           <loading-button
             :loading="submitCommitLoading"
-            :disabled="commitButtonDisabled"
-            :label="__('Commit')"
+            :label="commitButtonText"
             container-class="btn btn-success btn-sm float-left"
             @click="commitChanges"
           />
diff --git a/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue b/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue
index 40496c80a46..37ca108fafc 100644
--- a/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue
@@ -16,6 +16,10 @@ export default {
       type: String,
       required: true,
     },
+    placeholder: {
+      type: String,
+      required: true,
+    },
   },
   data() {
     return {
@@ -114,7 +118,7 @@ export default {
         </div>
         <textarea
           ref="textarea"
-          :placeholder="__('Write a commit message...')"
+          :placeholder="placeholder"
           :value="text"
           class="note-textarea ide-commit-message-textarea"
           name="commit-message"
diff --git a/app/assets/javascripts/ide/components/commit_sidebar/radio_group.vue b/app/assets/javascripts/ide/components/commit_sidebar/radio_group.vue
index 35ab3fd11df..969e2aa61c4 100644
--- a/app/assets/javascripts/ide/components/commit_sidebar/radio_group.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/radio_group.vue
@@ -1,6 +1,5 @@
 <script>
 import { mapActions, mapState, mapGetters } from 'vuex';
-import { __ } from '~/locale';
 import tooltip from '~/vue_shared/directives/tooltip';
 
 export default {
@@ -32,14 +31,17 @@ export default {
       required: false,
       default: false,
     },
+    title: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   computed: {
     ...mapState('commit', ['commitAction']),
     ...mapGetters('commit', ['newBranchName']),
     tooltipTitle() {
-      return this.disabled
-        ? __('This option is disabled while you still have unstaged changes')
-        : '';
+      return this.disabled ? this.title : '';
     },
   },
   methods: {
diff --git a/app/assets/javascripts/ide/components/repo_commit_section.vue b/app/assets/javascripts/ide/components/repo_commit_section.vue
index c2c678ff0be..50ab242ba2a 100644
--- a/app/assets/javascripts/ide/components/repo_commit_section.vue
+++ b/app/assets/javascripts/ide/components/repo_commit_section.vue
@@ -28,7 +28,7 @@ export default {
     ]),
     ...mapState('commit', ['commitMessage', 'submitCommitLoading']),
     ...mapGetters(['lastOpenedFile', 'hasChanges', 'someUncommitedChanges', 'activeFile']),
-    ...mapGetters('commit', ['commitButtonDisabled', 'discardDraftButtonDisabled']),
+    ...mapGetters('commit', ['discardDraftButtonDisabled']),
     showStageUnstageArea() {
       return !!(this.someUncommitedChanges || this.lastCommitMsg || !this.unusedSeal);
     },
diff --git a/app/assets/javascripts/ide/stores/getters.js b/app/assets/javascripts/ide/stores/getters.js
index b239a605371..5ce268b0d05 100644
--- a/app/assets/javascripts/ide/stores/getters.js
+++ b/app/assets/javascripts/ide/stores/getters.js
@@ -82,10 +82,13 @@ export const getStagedFilesCountForPath = state => path =>
   getChangesCountForFiles(state.stagedFiles, path);
 
 export const lastCommit = (state, getters) => {
-  const branch = getters.currentProject && getters.currentProject.branches[state.currentBranchId];
+  const branch = getters.currentProject && getters.currentBranch;
 
   return branch ? branch.commit : null;
 };
 
+export const currentBranch = (state, getters) =>
+  getters.currentProject && getters.currentProject.branches[state.currentBranchId];
+
 // 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 7219abc4185..69b6fe2985b 100644
--- a/app/assets/javascripts/ide/stores/modules/commit/actions.js
+++ b/app/assets/javascripts/ide/stores/modules/commit/actions.js
@@ -103,17 +103,24 @@ export const updateFilesAfterCommit = ({ commit, dispatch, rootState }, { data }
 
 export const commitChanges = ({ commit, state, getters, dispatch, rootState, rootGetters }) => {
   const newBranch = state.commitAction !== consts.COMMIT_TO_CURRENT_BRANCH;
-  const payload = createCommitPayload({
-    branch: getters.branchName,
-    newBranch,
-    state,
-    rootState,
-  });
+  const stageFilesPromise = rootState.stagedFiles.length
+    ? Promise.resolve()
+    : dispatch('stageAllChanges', null, { root: true });
 
   commit(types.UPDATE_LOADING, true);
 
-  return service
-    .commit(rootState.currentProjectId, payload)
+  return stageFilesPromise
+    .then(() => {
+      const payload = createCommitPayload({
+        branch: getters.branchName,
+        newBranch,
+        getters,
+        state,
+        rootState,
+      });
+
+      return service.commit(rootState.currentProjectId, payload);
+    })
     .then(({ data }) => {
       commit(types.UPDATE_LOADING, false);
 
diff --git a/app/assets/javascripts/ide/stores/modules/commit/getters.js b/app/assets/javascripts/ide/stores/modules/commit/getters.js
index d01060201f2..3db4b2f903e 100644
--- a/app/assets/javascripts/ide/stores/modules/commit/getters.js
+++ b/app/assets/javascripts/ide/stores/modules/commit/getters.js
@@ -1,3 +1,4 @@
+import { sprintf, n__ } from '../../../../locale';
 import * as consts from './constants';
 
 const BRANCH_SUFFIX_COUNT = 5;
@@ -5,9 +6,6 @@ const BRANCH_SUFFIX_COUNT = 5;
 export const discardDraftButtonDisabled = state =>
   state.commitMessage === '' || state.submitCommitLoading;
 
-export const commitButtonDisabled = (state, getters, rootState) =>
-  getters.discardDraftButtonDisabled || !rootState.stagedFiles.length;
-
 export const newBranchName = (state, _, rootState) =>
   `${gon.current_username}-${rootState.currentBranchId}-patch-${`${new Date().getTime()}`.substr(
     -BRANCH_SUFFIX_COUNT,
@@ -28,5 +26,18 @@ export const branchName = (state, getters, rootState) => {
   return rootState.currentBranchId;
 };
 
+export const preBuiltCommitMessage = (state, _, rootState) => {
+  if (state.commitMessage) return state.commitMessage;
+
+  const files = (rootState.stagedFiles.length
+    ? rootState.stagedFiles
+    : rootState.changedFiles
+  ).reduce((acc, val) => acc.concat(val.path), []);
+
+  return sprintf(n__('Update %{files}', 'Update %{files} files', files.length), {
+    files: files.join(', '),
+  });
+};
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};
diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js
index 10368a4d97c..9e6b86dd844 100644
--- a/app/assets/javascripts/ide/stores/utils.js
+++ b/app/assets/javascripts/ide/stores/utils.js
@@ -105,9 +105,9 @@ export const setPageTitle = title => {
   document.title = title;
 };
 
-export const createCommitPayload = ({ branch, newBranch, state, rootState }) => ({
+export const createCommitPayload = ({ branch, getters, newBranch, state, rootState }) => ({
   branch,
-  commit_message: state.commitMessage,
+  commit_message: state.commitMessage || getters.preBuiltCommitMessage,
   actions: rootState.stagedFiles.map(f => ({
     action: f.tempFile ? 'create' : 'update',
     file_path: f.path,