Merge branch '213581-step-1-clean-up-unit-test' into 'master'

Step 1 - Clean up `commit_sidebar/form_spec` and commit constants

See merge request gitlab-org/gitlab!51816

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

@@ -3,7 +3,10 @@ import { escape } from 'lodash';
 import { mapState, mapGetters, createNamespacedHelpers } from 'vuex';
 import { GlSprintf } from '@gitlab/ui';
 import { s__ } from '~/locale';
-import consts from '../../stores/modules/commit/constants';
+import {
+  COMMIT_TO_CURRENT_BRANCH,
+  COMMIT_TO_NEW_BRANCH,
+} from '../../stores/modules/commit/constants';
 import RadioGroup from './radio_group.vue';
 import NewMergeRequestOption from './new_merge_request_option.vue';
 
@@ -53,14 +56,14 @@ export default {
       }
 
       if (this.shouldDefaultToCurrentBranch) {
-        this.updateCommitAction(consts.COMMIT_TO_CURRENT_BRANCH);
+        this.updateCommitAction(COMMIT_TO_CURRENT_BRANCH);
       } else {
-        this.updateCommitAction(consts.COMMIT_TO_NEW_BRANCH);
+        this.updateCommitAction(COMMIT_TO_NEW_BRANCH);
       }
     },
   },
-  commitToCurrentBranch: consts.COMMIT_TO_CURRENT_BRANCH,
-  commitToNewBranch: consts.COMMIT_TO_NEW_BRANCH,
+  commitToCurrentBranch: COMMIT_TO_CURRENT_BRANCH,
+  commitToNewBranch: COMMIT_TO_NEW_BRANCH,
   currentBranchPermissionsTooltip: s__(
     "IDE|This option is disabled because you don't have write permissions for the current branch.",
   ),

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

 <script>
 import { mapState, mapActions, mapGetters } from 'vuex';
 import { GlModal, GlSafeHtmlDirective, GlButton } from '@gitlab/ui';
-import { n__, __ } from '~/locale';
+import { n__ } from '~/locale';
 import CommitMessageField from './message_field.vue';
 import Actions from './actions.vue';
 import SuccessMessage from './success_message.vue';
@@ -35,10 +35,6 @@ export default {
     overviewText() {
       return n__('%d changed file', '%d changed files', this.stagedFiles.length);
     },
-    commitButtonText() {
-      return this.stagedFiles.length ? __('Commit') : __('Stage & Commit');
-    },
-
     currentViewIsCommitView() {
       return this.currentActivityView === leftSidebarViews.commit.name;
     },
@@ -160,13 +156,19 @@ export default {
           <gl-button
             :loading="submitCommitLoading"
             class="float-left qa-commit-button"
+            data-testid="commit-button"
             category="primary"
             variant="success"
             @click="commit"
           >
             {{ __('Commit') }}
           </gl-button>
-          <gl-button v-if="!discardDraftButtonDisabled" class="float-right" @click="discardDraft">
+          <gl-button
+            v-if="!discardDraftButtonDisabled"
+            class="float-right"
+            data-testid="discard-draft"
+            @click="discardDraft"
+          >
             {{ __('Discard draft') }}
           </gl-button>
           <gl-button

app/assets/javascripts/ide/lib/errors.js

 import { escape } from 'lodash';
 import { __ } from '~/locale';
-import consts from '../stores/modules/commit/constants';
+import { COMMIT_TO_NEW_BRANCH } from '../stores/modules/commit/constants';
 
 const CODEOWNERS_REGEX = /Push.*protected branches.*CODEOWNERS/;
 const BRANCH_CHANGED_REGEX = /changed.*since.*start.*edit/;
@@ -8,7 +8,7 @@ const BRANCH_ALREADY_EXISTS = /branch.*already.*exists/;
 
 const createNewBranchAndCommit = (store) =>
   store
-    .dispatch('commit/updateCommitAction', consts.COMMIT_TO_NEW_BRANCH)
+    .dispatch('commit/updateCommitAction', COMMIT_TO_NEW_BRANCH)
     .then(() => store.dispatch('commit/commitChanges'));
 
 export const createUnexpectedCommitError = (message) => ({

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

@@ -4,7 +4,7 @@ import * as rootTypes from '../../mutation_types';
 import { createCommitPayload, createNewMergeRequestUrl } from '../../utils';
 import service from '../../../services';
 import * as types from './mutation_types';
-import consts from './constants';
+import { COMMIT_TO_CURRENT_BRANCH } from './constants';
 import { leftSidebarViews } from '../../../constants';
 import eventHub from '../../../eventhub';
 import { parseCommitError } from '../../../lib/errors';
@@ -112,7 +112,7 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo
   // Pull commit options out because they could change
   // During some of the pre and post commit processing
   const { shouldCreateMR, shouldHideNewMrOption, isCreatingNewBranch, branchName } = getters;
-  const newBranch = state.commitAction !== consts.COMMIT_TO_CURRENT_BRANCH;
+  const newBranch = state.commitAction !== COMMIT_TO_CURRENT_BRANCH;
   const stageFilesPromise = rootState.stagedFiles.length
     ? Promise.resolve()
     : dispatch('stageAllChanges', null, { root: true });
@@ -206,7 +206,7 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo
             dispatch('updateViewer', 'editor', { root: true });
           }
         })
-        .then(() => dispatch('updateCommitAction', consts.COMMIT_TO_CURRENT_BRANCH))
+        .then(() => dispatch('updateCommitAction', COMMIT_TO_CURRENT_BRANCH))
         .then(() => {
           if (newBranch) {
             const path = rootGetters.activeFile ? rootGetters.activeFile.path : '';

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

-const COMMIT_TO_CURRENT_BRANCH = '1';
-const COMMIT_TO_NEW_BRANCH = '2';
-
-export default {
-  COMMIT_TO_CURRENT_BRANCH,
-  COMMIT_TO_NEW_BRANCH,
-};
+export const COMMIT_TO_CURRENT_BRANCH = '1';
+export const COMMIT_TO_NEW_BRANCH = '2';

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

 import { sprintf, n__, __ } from '../../../../locale';
-import consts from './constants';
+import { COMMIT_TO_NEW_BRANCH } from './constants';
 
 const BRANCH_SUFFIX_COUNT = 5;
 const createTranslatedTextForFiles = (files, text) => {
@@ -48,7 +48,7 @@ export const preBuiltCommitMessage = (state, _, rootState) => {
     .join('\n');
 };
 
-export const isCreatingNewBranch = (state) => state.commitAction === consts.COMMIT_TO_NEW_BRANCH;
+export const isCreatingNewBranch = (state) => state.commitAction === COMMIT_TO_NEW_BRANCH;
 
 export const shouldHideNewMrOption = (_state, getters, _rootState, rootGetters) =>
   !getters.isCreatingNewBranch &&

locale/gitlab.pot

@@ -26815,9 +26815,6 @@ msgstr ""
 msgid "Stage"
 msgstr ""
 
-msgid "Stage & Commit"
-msgstr ""
-
 msgid "Stage data updated"
 msgstr ""
 

spec/frontend/__helpers__/stub_component.js

+/**
+ * Returns a new object with keys pointing to stubbed methods
+ *
+ * This is helpful for stubbing components like GlModal where it's supported
+ * in the API to call `.show()` and `.hide()` ([Bootstrap Vue docs][1]).
+ *
+ * [1]: https://bootstrap-vue.org/docs/components/modal#using-show-hide-and-toggle-component-methods
+ *
+ * @param {Object} methods - Object whose keys will be in the returned object.
+ */
+const createStubbedMethods = (methods = {}) => {
+  if (!methods) {
+    return {};
+  }
+
+  return Object.keys(methods).reduce(
+    (acc, key) =>
+      Object.assign(acc, {
+        [key]: () => {},
+      }),
+    {},
+  );
+};
+
 export function stubComponent(Component, options = {}) {
   return {
     props: Component.props,
     model: Component.model,
+    methods: createStubbedMethods(Component.methods),
     // Do not render any slots/scoped slots except default
     // This differs from VTU behavior which renders all slots
     template: '<div><slot></slot></div>',

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

@@ -3,7 +3,10 @@ import { createComponentWithStore } from 'helpers/vue_mount_component_helper';
 import { projectData, branches } from 'jest/ide/mock_data';
 import { createStore } from '~/ide/stores';
 import commitActions from '~/ide/components/commit_sidebar/actions.vue';
-import consts from '~/ide/stores/modules/commit/constants';
+import {
+  COMMIT_TO_NEW_BRANCH,
+  COMMIT_TO_CURRENT_BRANCH,
+} from '~/ide/stores/modules/commit/constants';
 
 const ACTION_UPDATE_COMMIT_ACTION = 'commit/updateCommitAction';
 
@@ -126,16 +129,16 @@ describe('IDE commit sidebar actions', () => {
 
     it.each`
       input                                                            | expectedOption
-      ${{ currentBranchId: BRANCH_DEFAULT }}                           | ${consts.COMMIT_TO_NEW_BRANCH}
-      ${{ currentBranchId: BRANCH_DEFAULT, emptyRepo: true }}          | ${consts.COMMIT_TO_CURRENT_BRANCH}
-      ${{ currentBranchId: BRANCH_PROTECTED, hasMR: true }}            | ${consts.COMMIT_TO_CURRENT_BRANCH}
-      ${{ currentBranchId: BRANCH_PROTECTED, hasMR: false }}           | ${consts.COMMIT_TO_CURRENT_BRANCH}
-      ${{ currentBranchId: BRANCH_PROTECTED_NO_ACCESS, hasMR: true }}  | ${consts.COMMIT_TO_NEW_BRANCH}
-      ${{ currentBranchId: BRANCH_PROTECTED_NO_ACCESS, hasMR: false }} | ${consts.COMMIT_TO_NEW_BRANCH}
-      ${{ currentBranchId: BRANCH_REGULAR, hasMR: true }}              | ${consts.COMMIT_TO_CURRENT_BRANCH}
-      ${{ currentBranchId: BRANCH_REGULAR, hasMR: false }}             | ${consts.COMMIT_TO_CURRENT_BRANCH}
-      ${{ currentBranchId: BRANCH_REGULAR_NO_ACCESS, hasMR: true }}    | ${consts.COMMIT_TO_NEW_BRANCH}
-      ${{ currentBranchId: BRANCH_REGULAR_NO_ACCESS, hasMR: false }}   | ${consts.COMMIT_TO_NEW_BRANCH}
+      ${{ currentBranchId: BRANCH_DEFAULT }}                           | ${COMMIT_TO_NEW_BRANCH}
+      ${{ currentBranchId: BRANCH_DEFAULT, emptyRepo: true }}          | ${COMMIT_TO_CURRENT_BRANCH}
+      ${{ currentBranchId: BRANCH_PROTECTED, hasMR: true }}            | ${COMMIT_TO_CURRENT_BRANCH}
+      ${{ currentBranchId: BRANCH_PROTECTED, hasMR: false }}           | ${COMMIT_TO_CURRENT_BRANCH}
+      ${{ currentBranchId: BRANCH_PROTECTED_NO_ACCESS, hasMR: true }}  | ${COMMIT_TO_NEW_BRANCH}
+      ${{ currentBranchId: BRANCH_PROTECTED_NO_ACCESS, hasMR: false }} | ${COMMIT_TO_NEW_BRANCH}
+      ${{ currentBranchId: BRANCH_REGULAR, hasMR: true }}              | ${COMMIT_TO_CURRENT_BRANCH}
+      ${{ currentBranchId: BRANCH_REGULAR, hasMR: false }}             | ${COMMIT_TO_CURRENT_BRANCH}
+      ${{ currentBranchId: BRANCH_REGULAR_NO_ACCESS, hasMR: true }}    | ${COMMIT_TO_NEW_BRANCH}
+      ${{ currentBranchId: BRANCH_REGULAR_NO_ACCESS, hasMR: false }}   | ${COMMIT_TO_NEW_BRANCH}
     `(
       'with $input, it dispatches update commit action with $expectedOption',
       ({ input, expectedOption }) => {

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

@@ -4,7 +4,10 @@ import { projectData, branches } from 'jest/ide/mock_data';
 import NewMergeRequestOption from '~/ide/components/commit_sidebar/new_merge_request_option.vue';
 import { createStore } from '~/ide/stores';
 import { PERMISSION_CREATE_MR } from '~/ide/constants';
-import consts from '~/ide/stores/modules/commit/constants';
+import {
+  COMMIT_TO_CURRENT_BRANCH,
+  COMMIT_TO_NEW_BRANCH,
+} from '~/ide/stores/modules/commit/constants';
 
 describe('create new MR checkbox', () => {
   let store;
@@ -27,8 +30,8 @@ describe('create new MR checkbox', () => {
     vm = createComponentWithStore(Component, store);
 
     vm.$store.state.commit.commitAction = createNewBranch
-      ? consts.COMMIT_TO_NEW_BRANCH
-      : consts.COMMIT_TO_CURRENT_BRANCH;
+      ? COMMIT_TO_NEW_BRANCH
+      : COMMIT_TO_CURRENT_BRANCH;
 
     vm.$store.state.currentBranchId = currentBranchId;
 

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

@@ -7,7 +7,10 @@ import { createStore } from '~/ide/stores';
 import service from '~/ide/services';
 import { createRouter } from '~/ide/ide_router';
 import eventHub from '~/ide/eventhub';
-import consts from '~/ide/stores/modules/commit/constants';
+import {
+  COMMIT_TO_CURRENT_BRANCH,
+  COMMIT_TO_NEW_BRANCH,
+} from '~/ide/stores/modules/commit/constants';
 import * as mutationTypes from '~/ide/stores/modules/commit/mutation_types';
 import * as actions from '~/ide/stores/modules/commit/actions';
 import { createUnexpectedCommitError } from '~/ide/lib/errors';
@@ -425,12 +428,12 @@ describe('IDE commit module actions', () => {
       });
 
       it('resets stores commit actions', (done) => {
-        store.state.commit.commitAction = consts.COMMIT_TO_NEW_BRANCH;
+        store.state.commit.commitAction = COMMIT_TO_NEW_BRANCH;
 
         store
           .dispatch('commit/commitChanges')
           .then(() => {
-            expect(store.state.commit.commitAction).not.toBe(consts.COMMIT_TO_NEW_BRANCH);
+            expect(store.state.commit.commitAction).not.toBe(COMMIT_TO_NEW_BRANCH);
           })
           .then(done)
           .catch(done.fail);
@@ -450,7 +453,7 @@ describe('IDE commit module actions', () => {
         it('redirects to new merge request page', (done) => {
           jest.spyOn(eventHub, '$on').mockImplementation();
 
-          store.state.commit.commitAction = consts.COMMIT_TO_NEW_BRANCH;
+          store.state.commit.commitAction = COMMIT_TO_NEW_BRANCH;
           store.state.commit.shouldCreateMR = true;
 
           store
@@ -468,7 +471,7 @@ describe('IDE commit module actions', () => {
         it('does not redirect to new merge request page when shouldCreateMR is not checked', (done) => {
           jest.spyOn(eventHub, '$on').mockImplementation();
 
-          store.state.commit.commitAction = consts.COMMIT_TO_NEW_BRANCH;
+          store.state.commit.commitAction = COMMIT_TO_NEW_BRANCH;
           store.state.commit.shouldCreateMR = false;
 
           store
@@ -483,7 +486,7 @@ describe('IDE commit module actions', () => {
         it('does not redirect to merge request page if shouldCreateMR is checked, but branch is the default branch', async () => {
           jest.spyOn(eventHub, '$on').mockImplementation();
 
-          store.state.commit.commitAction = consts.COMMIT_TO_CURRENT_BRANCH;
+          store.state.commit.commitAction = COMMIT_TO_CURRENT_BRANCH;
           store.state.commit.shouldCreateMR = true;
 
           await store.dispatch('commit/commitChanges');

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

 import commitState from '~/ide/stores/modules/commit/state';
 import * as getters from '~/ide/stores/modules/commit/getters';
-import consts from '~/ide/stores/modules/commit/constants';
+import {
+  COMMIT_TO_CURRENT_BRANCH,
+  COMMIT_TO_NEW_BRANCH,
+} from '~/ide/stores/modules/commit/constants';
 
 describe('IDE commit module getters', () => {
   let state;
@@ -147,13 +150,13 @@ describe('IDE commit module getters', () => {
 
   describe('isCreatingNewBranch', () => {
     it('returns false if NOT creating a new branch', () => {
-      state.commitAction = consts.COMMIT_TO_CURRENT_BRANCH;
+      state.commitAction = COMMIT_TO_CURRENT_BRANCH;
 
       expect(getters.isCreatingNewBranch(state)).toBeFalsy();
     });
 
     it('returns true if creating a new branch', () => {
-      state.commitAction = consts.COMMIT_TO_NEW_BRANCH;
+      state.commitAction = COMMIT_TO_NEW_BRANCH;
 
       expect(getters.isCreatingNewBranch(state)).toBeTruthy();
     });