From 4e56af3b7df9f61c91a2888c840214ae29d8e5ee Mon Sep 17 00:00:00 2001 From: Phil Hughes <me@iamphill.com> Date: Fri, 13 Apr 2018 09:49:57 +0100 Subject: [PATCH 1/2] Remove confirm box when redirecting to new merge request form in IDE Closes #45326 --- .../ide/stores/modules/commit/actions.js | 70 +++++++------------ .../unreleased/ide-mr-changes-alert-box.yml | 5 ++ .../ide/stores/modules/commit/actions_spec.js | 66 +++++++++-------- 3 files changed, 67 insertions(+), 74 deletions(-) create mode 100644 changelogs/unreleased/ide-mr-changes-alert-box.yml diff --git a/app/assets/javascripts/ide/stores/modules/commit/actions.js b/app/assets/javascripts/ide/stores/modules/commit/actions.js index f536ce6344b2..367c45f7e2d9 100644 --- a/app/assets/javascripts/ide/stores/modules/commit/actions.js +++ b/app/assets/javascripts/ide/stores/modules/commit/actions.js @@ -37,9 +37,9 @@ export const setLastCommitMessage = ({ rootState, commit }, data) => { const commitMsg = sprintf( __('Your changes have been committed. Commit %{commitId} %{commitStats}'), { - commitId: `<a href="${currentProject.web_url}/commit/${ + commitId: `<a href="${currentProject.web_url}/commit/${data.short_id}" class="commit-sha">${ data.short_id - }" class="commit-sha">${data.short_id}</a>`, + }</a>`, commitStats, }, false, @@ -54,9 +54,7 @@ export const checkCommitStatus = ({ rootState }) => .then(({ data }) => { const { id } = data.commit; const selectedBranch = - rootState.projects[rootState.currentProjectId].branches[ - rootState.currentBranchId - ]; + rootState.projects[rootState.currentProjectId].branches[rootState.currentBranchId]; if (selectedBranch.workingReference !== id) { return true; @@ -135,32 +133,15 @@ export const updateFilesAfterCommit = ( if (state.commitAction === consts.COMMIT_TO_NEW_BRANCH) { router.push( - `/project/${rootState.currentProjectId}/blob/${branch}/${ - rootGetters.activeFile.path - }`, + `/project/${rootState.currentProjectId}/blob/${branch}/${rootGetters.activeFile.path}`, ); } - - dispatch('updateCommitAction', consts.COMMIT_TO_CURRENT_BRANCH); }; -export const commitChanges = ({ - commit, - state, - getters, - dispatch, - rootState, -}) => { +export const commitChanges = ({ commit, state, getters, dispatch, rootState }) => { const newBranch = state.commitAction !== consts.COMMIT_TO_CURRENT_BRANCH; - const payload = createCommitPayload( - getters.branchName, - newBranch, - state, - rootState, - ); - const getCommitStatus = newBranch - ? Promise.resolve(false) - : dispatch('checkCommitStatus'); + const payload = createCommitPayload(getters.branchName, newBranch, state, rootState); + const getCommitStatus = newBranch ? Promise.resolve(false) : dispatch('checkCommitStatus'); commit(types.UPDATE_LOADING, true); @@ -182,28 +163,29 @@ export const commitChanges = ({ if (!data.short_id) { flash(data.message, 'alert', document, null, false, true); - return; + return null; } dispatch('setLastCommitMessage', data); dispatch('updateCommitMessage', ''); - - if (state.commitAction === consts.COMMIT_TO_NEW_BRANCH_MR) { - dispatch( - 'redirectToUrl', - createNewMergeRequestUrl( - rootState.projects[rootState.currentProjectId].web_url, - getters.branchName, - rootState.currentBranchId, - ), - { root: true }, - ); - } else { - dispatch('updateFilesAfterCommit', { - data, - branch: getters.branchName, - }); - } + return dispatch('updateFilesAfterCommit', { + data, + branch: getters.branchName, + }) + .then(() => { + if (state.commitAction === consts.COMMIT_TO_NEW_BRANCH_MR) { + dispatch( + 'redirectToUrl', + createNewMergeRequestUrl( + rootState.projects[rootState.currentProjectId].web_url, + getters.branchName, + rootState.currentBranchId, + ), + { root: true }, + ); + } + }) + .then(() => dispatch('updateCommitAction', consts.COMMIT_TO_CURRENT_BRANCH)); }) .catch(err => { let errMsg = __('Error committing changes. Please try again.'); diff --git a/changelogs/unreleased/ide-mr-changes-alert-box.yml b/changelogs/unreleased/ide-mr-changes-alert-box.yml new file mode 100644 index 000000000000..fec2719c2b1e --- /dev/null +++ b/changelogs/unreleased/ide-mr-changes-alert-box.yml @@ -0,0 +1,5 @@ +--- +title: Removed alert box in IDE when redirecting to new merge request +merge_request: +author: +type: fixed diff --git a/spec/javascripts/ide/stores/modules/commit/actions_spec.js b/spec/javascripts/ide/stores/modules/commit/actions_spec.js index 90ded9402273..afaf147d6fd6 100644 --- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/actions_spec.js @@ -133,10 +133,7 @@ describe('IDE commit module actions', () => { store .dispatch('commit/checkCommitStatus') .then(() => { - expect(service.getBranchData).toHaveBeenCalledWith( - 'abcproject', - 'master', - ); + expect(service.getBranchData).toHaveBeenCalledWith('abcproject', 'master'); done(); }) @@ -230,9 +227,7 @@ describe('IDE commit module actions', () => { branch, }) .then(() => { - expect( - store.state.projects.abcproject.branches.master.workingReference, - ).toBe(data.id); + expect(store.state.projects.abcproject.branches.master.workingReference).toBe(data.id); }) .then(done) .catch(done.fail); @@ -317,9 +312,7 @@ describe('IDE commit module actions', () => { branch, }) .then(() => { - expect(router.push).toHaveBeenCalledWith( - `/project/abcproject/blob/master/${f.path}`, - ); + expect(router.push).toHaveBeenCalledWith(`/project/abcproject/blob/master/${f.path}`); }) .then(done) .catch(done.fail); @@ -334,9 +327,7 @@ describe('IDE commit module actions', () => { branch, }) .then(() => { - expect(store.state.commit.commitAction).not.toBe( - consts.COMMIT_TO_NEW_BRANCH, - ); + expect(store.state.commit.commitAction).not.toBe(consts.COMMIT_TO_NEW_BRANCH); }) .then(done) .catch(done.fail); @@ -448,32 +439,47 @@ describe('IDE commit module actions', () => { store .dispatch('commit/commitChanges') .then(() => { - expect(store.state.openFiles[0].lastCommit.message).toBe( - 'test message', - ); + expect(store.state.openFiles[0].lastCommit.message).toBe('test message'); done(); }) .catch(done.fail); }); - it('redirects to new merge request page', done => { - spyOn(eventHub, '$on'); + describe('merge request', () => { + it('redirects to new merge request page', done => { + spyOn(eventHub, '$on'); - store.state.commit.commitAction = '3'; + store.state.commit.commitAction = '3'; - store - .dispatch('commit/commitChanges') - .then(() => { - expect(urlUtils.visitUrl).toHaveBeenCalledWith( - `webUrl/merge_requests/new?merge_request[source_branch]=${ - store.getters['commit/newBranchName'] - }&merge_request[target_branch]=master`, - ); + store + .dispatch('commit/commitChanges') + .then(() => { + expect(urlUtils.visitUrl).toHaveBeenCalledWith( + `webUrl/merge_requests/new?merge_request[source_branch]=${ + store.getters['commit/newBranchName'] + }&merge_request[target_branch]=master`, + ); - done(); - }) - .catch(done.fail); + done(); + }) + .catch(done.fail); + }); + + it('resets changed files before redirecting', done => { + spyOn(eventHub, '$on'); + + store.state.commit.commitAction = '3'; + + store + .dispatch('commit/commitChanges') + .then(() => { + expect(store.state.changedFiles.length).toBe(0); + + done(); + }) + .catch(done.fail); + }); }); }); -- GitLab From f75330781f85d555560f8d79657ef8930ad7d9d5 Mon Sep 17 00:00:00 2001 From: Phil Hughes <me@iamphill.com> Date: Fri, 13 Apr 2018 10:58:42 +0100 Subject: [PATCH 2/2] fixed actions spec --- .../ide/stores/modules/commit/actions_spec.js | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/spec/javascripts/ide/stores/modules/commit/actions_spec.js b/spec/javascripts/ide/stores/modules/commit/actions_spec.js index afaf147d6fd6..1946a0c547ce 100644 --- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/actions_spec.js @@ -317,21 +317,6 @@ describe('IDE commit module actions', () => { .then(done) .catch(done.fail); }); - - it('resets stores commit actions', done => { - store.state.commit.commitAction = consts.COMMIT_TO_NEW_BRANCH; - - store - .dispatch('commit/updateFilesAfterCommit', { - data, - branch, - }) - .then(() => { - expect(store.state.commit.commitAction).not.toBe(consts.COMMIT_TO_NEW_BRANCH); - }) - .then(done) - .catch(done.fail); - }); }); describe('commitChanges', () => { @@ -446,6 +431,18 @@ describe('IDE commit module actions', () => { .catch(done.fail); }); + it('resets stores commit actions', done => { + store.state.commit.commitAction = consts.COMMIT_TO_NEW_BRANCH; + + store + .dispatch('commit/commitChanges') + .then(() => { + expect(store.state.commit.commitAction).not.toBe(consts.COMMIT_TO_NEW_BRANCH); + }) + .then(done) + .catch(done.fail); + }); + describe('merge request', () => { it('redirects to new merge request page', done => { spyOn(eventHub, '$on'); -- GitLab