summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDenys Mishunov <dmishunov@gitlab.com>2019-08-22 23:23:06 +0000
committerPaul Slaughter <pslaughter@gitlab.com>2019-08-22 23:23:06 +0000
commit371e1ed3716d25f48c6458373e64ea81e0a45946 (patch)
treeb270a0bc9ea40ef7666ef2ae1da3770e2a30ca02
parent16edda3e4db941a8d7122bf1b4fcc854d2aa0927 (diff)
downloadgitlab-ce-371e1ed3716d25f48c6458373e64ea81e0a45946.tar.gz
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
-rw-r--r--app/assets/javascripts/ide/components/commit_sidebar/actions.vue12
-rw-r--r--app/assets/javascripts/ide/components/commit_sidebar/new_merge_request_option.vue33
-rw-r--r--app/assets/javascripts/ide/stores/getters.js3
-rw-r--r--app/assets/javascripts/ide/stores/modules/commit/actions.js23
-rw-r--r--app/assets/javascripts/ide/stores/modules/commit/getters.js12
-rw-r--r--app/assets/javascripts/ide/stores/modules/commit/mutation_types.js1
-rw-r--r--app/assets/javascripts/ide/stores/modules/commit/mutations.js3
-rw-r--r--app/assets/javascripts/ide/stores/modules/commit/state.js3
-rw-r--r--changelogs/unreleased/51470-webide-default-commit.yml5
-rw-r--r--qa/qa/page/project/web_ide/edit.rb5
-rw-r--r--spec/features/projects/tree/create_directory_spec.rb13
-rw-r--r--spec/features/projects/tree/create_file_spec.rb7
-rw-r--r--spec/frontend/ide/stores/modules/commit/mutations_spec.js8
-rw-r--r--spec/javascripts/ide/components/commit_sidebar/actions_spec.js188
-rw-r--r--spec/javascripts/ide/components/commit_sidebar/new_merge_request_option_spec.js160
-rw-r--r--spec/javascripts/ide/mock_data.js34
-rw-r--r--spec/javascripts/ide/stores/getters_spec.js32
-rw-r--r--spec/javascripts/ide/stores/modules/commit/actions_spec.js177
-rw-r--r--spec/javascripts/ide/stores/modules/commit/getters_spec.js162
19 files changed, 619 insertions, 262 deletions
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,
});
diff --git a/changelogs/unreleased/51470-webide-default-commit.yml b/changelogs/unreleased/51470-webide-default-commit.yml
new file mode 100644
index 00000000000..d64111e7ec3
--- /dev/null
+++ b/changelogs/unreleased/51470-webide-default-commit.yml
@@ -0,0 +1,5 @@
+---
+title: Updated WebIDE default commit options
+merge_request: 31449
+author:
+type: changed
diff --git a/qa/qa/page/project/web_ide/edit.rb b/qa/qa/page/project/web_ide/edit.rb
index b5a36862389..37bca97fec7 100644
--- a/qa/qa/page/project/web_ide/edit.rb
+++ b/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
diff --git a/spec/features/projects/tree/create_directory_spec.rb b/spec/features/projects/tree/create_directory_spec.rb
index 7ac5da86702..99285011405 100644
--- a/spec/features/projects/tree/create_directory_spec.rb
+++ b/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
diff --git a/spec/features/projects/tree/create_file_spec.rb b/spec/features/projects/tree/create_file_spec.rb
index 00eefe9db42..780575a5975 100644
--- a/spec/features/projects/tree/create_file_spec.rb
+++ b/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
diff --git a/spec/frontend/ide/stores/modules/commit/mutations_spec.js b/spec/frontend/ide/stores/modules/commit/mutations_spec.js
index 246500a2f34..45ac1a86ab3 100644
--- a/spec/frontend/ide/stores/modules/commit/mutations_spec.js
+++ b/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);
- });
- });
});
diff --git a/spec/javascripts/ide/components/commit_sidebar/actions_spec.js b/spec/javascripts/ide/components/commit_sidebar/actions_spec.js
index b903abe63fc..a3db3ee1b18 100644
--- a/spec/javascripts/ide/components/commit_sidebar/actions_spec.js
+++ b/spec/javascripts/ide/components/commit_sidebar/actions_spec.js
@@ -1,30 +1,28 @@
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,
+ );
+ });
+ });
+ });
+ });
});
diff --git a/spec/javascripts/ide/components/commit_sidebar/new_merge_request_option_spec.js b/spec/javascripts/ide/components/commit_sidebar/new_merge_request_option_spec.js
index 7017bfcd6a6..5f2db695241 100644
--- a/spec/javascripts/ide/components/commit_sidebar/new_merge_request_option_spec.js
+++ b/spec/javascripts/ide/components/commit_sidebar/new_merge_request_option_spec.js
@@ -1,33 +1,36 @@
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', () => {
- createComponent({
- hasMR: true,
- commitAction: consts.COMMIT_TO_CURRENT_BRANCH,
- currentBranchId: 'feature',
+ 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);
+ });
});
- expect(vm.$el.textContent).toBe('');
+ 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);
+ });
+ });
});
- it('does not hide checkbox if MR does not exist', () => {
- createComponent({ hasMR: false });
+ describe('for protected branch', () => {
+ describe('when user does not have the write access', () => {
+ beforeEach(() => {
+ createComponent({
+ 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();
- expect(vm.$el.querySelector('input[type="checkbox"]').hidden).toBe(false);
+ 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 when creating a new branch', () => {
- createComponent({ commitAction: consts.COMMIT_TO_NEW_BRANCH });
+ 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('is hidden if MR exists', done => {
+ setMR();
+
+ 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'));
diff --git a/spec/javascripts/ide/mock_data.js b/spec/javascripts/ide/mock_data.js
index 570a396c5e3..c02c7e5d45e 100644
--- a/spec/javascripts/ide/mock_data.js
+++ b/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,
},
];
diff --git a/spec/javascripts/ide/stores/getters_spec.js b/spec/javascripts/ide/stores/getters_spec.js
index 735bbd47f55..73a8d993a13 100644
--- a/spec/javascripts/ide/stores/getters_spec.js
+++ b/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();
+ });
+ });
});
diff --git a/spec/javascripts/ide/stores/modules/commit/actions_spec.js b/spec/javascripts/ide/stores/modules/commit/actions_spec.js
index 14d861f21d2..091b454c0d2 100644
--- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js
+++ b/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);
- });
- });
});
diff --git a/spec/javascripts/ide/stores/modules/commit/getters_spec.js b/spec/javascripts/ide/stores/modules/commit/getters_spec.js
index 6e71a790deb..07445c22917 100644
--- a/spec/javascripts/ide/stores/modules/commit/getters_spec.js
+++ b/spec/javascripts/ide/stores/modules/commit/getters_spec.js
@@ -1,6 +1,6 @@
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();
+ });
+ });
});