diff options
7 files changed, 92 insertions, 10 deletions
diff --git a/app/assets/javascripts/ide/services/index.js b/app/assets/javascripts/ide/services/index.js index ba33b6826d6..840761f68db 100644 --- a/app/assets/javascripts/ide/services/index.js +++ b/app/assets/javascripts/ide/services/index.js @@ -56,7 +56,13 @@ export default { return Api.branchSingle(projectId, currentBranchId); }, commit(projectId, payload) { - return Api.commitMultiple(projectId, payload); + // Currently the `commit` endpoint does not support `start_sha` so we + // have to make the request in the FE. This is not ideal and will be + // resolved soon. https://gitlab.com/gitlab-org/gitlab-ce/issues/59023 + const { branch, start_sha: ref } = payload; + const branchPromise = ref ? Api.createBranch(projectId, { ref, branch }) : Promise.resolve(); + + return branchPromise.then(() => Api.commitMultiple(projectId, payload)); }, getFiles(projectUrl, branchId) { const url = `${projectUrl}/files/${branchId}`; diff --git a/app/assets/javascripts/ide/stores/modules/commit/actions.js b/app/assets/javascripts/ide/stores/modules/commit/actions.js index 51062f092ad..ff1255ce749 100644 --- a/app/assets/javascripts/ide/stores/modules/commit/actions.js +++ b/app/assets/javascripts/ide/stores/modules/commit/actions.js @@ -142,6 +142,7 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo getters, state, rootState, + rootGetters, }); return service.commit(rootState.currentProjectId, payload); diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index bcc9ca60d9b..4e7a8765abe 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -135,7 +135,14 @@ export const getCommitFiles = stagedFiles => }); }, []); -export const createCommitPayload = ({ branch, getters, newBranch, state, rootState }) => ({ +export const createCommitPayload = ({ + branch, + getters, + newBranch, + state, + rootState, + rootGetters, +}) => ({ branch, commit_message: state.commitMessage || getters.preBuiltCommitMessage, actions: getCommitFiles(rootState.stagedFiles).map(f => ({ @@ -146,7 +153,7 @@ export const createCommitPayload = ({ branch, getters, newBranch, state, rootSta encoding: f.base64 ? 'base64' : 'text', last_commit_id: newBranch || f.deleted || f.prevPath ? undefined : f.lastCommitSha, })), - start_branch: newBranch ? rootState.currentBranchId : undefined, + start_sha: newBranch ? rootGetters.lastCommit.short_id : undefined, }); export const createNewMergeRequestUrl = (projectUrl, source, target) => diff --git a/changelogs/unreleased/59023-fix-web-ide-creating-branches-off-new-commits.yml b/changelogs/unreleased/59023-fix-web-ide-creating-branches-off-new-commits.yml new file mode 100644 index 00000000000..f7e0ee333aa --- /dev/null +++ b/changelogs/unreleased/59023-fix-web-ide-creating-branches-off-new-commits.yml @@ -0,0 +1,5 @@ +--- +title: Fix IDE commit using latest ref in branch and overriding contents +merge_request: 29769 +author: +type: fixed diff --git a/spec/frontend/ide/services/index_spec.js b/spec/frontend/ide/services/index_spec.js new file mode 100644 index 00000000000..499fa8fc012 --- /dev/null +++ b/spec/frontend/ide/services/index_spec.js @@ -0,0 +1,55 @@ +import services from '~/ide/services'; +import Api from '~/api'; + +jest.mock('~/api'); + +const TEST_PROJECT_ID = 'alice/wonderland'; +const TEST_BRANCH = 'master-patch-123'; +const TEST_COMMIT_SHA = '123456789'; + +describe('IDE services', () => { + describe('commit', () => { + let payload; + + beforeEach(() => { + payload = { + branch: TEST_BRANCH, + commit_message: 'Hello world', + actions: [], + start_sha: undefined, + }; + + Api.createBranch.mockReturnValue(Promise.resolve()); + Api.commitMultiple.mockReturnValue(Promise.resolve()); + }); + + describe.each` + startSha | shouldCreateBranch + ${undefined} | ${false} + ${TEST_COMMIT_SHA} | ${true} + `('when start_sha is $startSha', ({ startSha, shouldCreateBranch }) => { + beforeEach(() => { + payload.start_sha = startSha; + + return services.commit(TEST_PROJECT_ID, payload); + }); + + if (shouldCreateBranch) { + it('should create branch', () => { + expect(Api.createBranch).toHaveBeenCalledWith(TEST_PROJECT_ID, { + ref: TEST_COMMIT_SHA, + branch: TEST_BRANCH, + }); + }); + } else { + it('should not create branch', () => { + expect(Api.createBranch).not.toHaveBeenCalled(); + }); + } + + it('should commit', () => { + expect(Api.commitMultiple).toHaveBeenCalledWith(TEST_PROJECT_ID, payload); + }); + }); + }); +}); diff --git a/spec/javascripts/ide/stores/modules/commit/actions_spec.js b/spec/javascripts/ide/stores/modules/commit/actions_spec.js index 5f7272311c8..8a3c132972e 100644 --- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/actions_spec.js @@ -6,9 +6,11 @@ import eventHub from '~/ide/eventhub'; import consts 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 testAction from '../../../../helpers/vuex_action_helper'; import { commitActionTypes } from '~/ide/constants'; import { resetStore, file } from 'spec/ide/helpers'; +import testAction from '../../../../helpers/vuex_action_helper'; + +const TEST_COMMIT_SHA = '123456789'; describe('IDE commit module actions', () => { beforeEach(() => { @@ -139,6 +141,9 @@ describe('IDE commit module actions', () => { branches: { master: { workingReference: '', + commit: { + short_id: TEST_COMMIT_SHA, + }, }, }, }; @@ -239,6 +244,9 @@ describe('IDE commit module actions', () => { branches: { master: { workingReference: '1', + commit: { + short_id: TEST_COMMIT_SHA, + }, }, }, }; @@ -247,7 +255,7 @@ describe('IDE commit module actions', () => { ...file('changed'), type: 'blob', active: true, - lastCommitSha: '123456789', + lastCommitSha: TEST_COMMIT_SHA, }; store.state.stagedFiles.push(f); store.state.changedFiles = [ @@ -307,7 +315,7 @@ describe('IDE commit module actions', () => { previous_path: undefined, }, ], - start_branch: 'master', + start_sha: TEST_COMMIT_SHA, }); done(); @@ -330,11 +338,11 @@ describe('IDE commit module actions', () => { file_path: jasmine.anything(), content: undefined, encoding: jasmine.anything(), - last_commit_id: '123456789', + last_commit_id: TEST_COMMIT_SHA, previous_path: undefined, }, ], - start_branch: undefined, + start_sha: undefined, }); done(); diff --git a/spec/javascripts/ide/stores/utils_spec.js b/spec/javascripts/ide/stores/utils_spec.js index debe1c4acee..e3bf6d40245 100644 --- a/spec/javascripts/ide/stores/utils_spec.js +++ b/spec/javascripts/ide/stores/utils_spec.js @@ -132,7 +132,7 @@ describe('Multi-file store utils', () => { previous_path: undefined, }, ], - start_branch: undefined, + start_sha: undefined, }); }); @@ -187,7 +187,7 @@ describe('Multi-file store utils', () => { previous_path: undefined, }, ], - start_branch: undefined, + start_sha: undefined, }); }); }); |