From d4cc92db09a0c765556882943b7508075a0ab0e2 Mon Sep 17 00:00:00 2001 From: Paul Slaughter Date: Wed, 26 Jun 2019 09:07:56 -0500 Subject: [PATCH] FE remove create branch call in IDE commit Previously `start_sha` was intercepted on the frontend to create the correct branch in a separate API call. Now that the commits API supports the `start_sha` parameter directly this workaround is not needed anymore. --- app/assets/javascripts/ide/services/index.js | 8 +---- app/assets/javascripts/ide/stores/utils.js | 2 +- spec/frontend/ide/services/index_spec.js | 32 +++---------------- .../ide/stores/modules/commit/actions_spec.js | 2 +- 4 files changed, 7 insertions(+), 37 deletions(-) diff --git a/app/assets/javascripts/ide/services/index.js b/app/assets/javascripts/ide/services/index.js index 840761f68db..ba33b6826d6 100644 --- a/app/assets/javascripts/ide/services/index.js +++ b/app/assets/javascripts/ide/services/index.js @@ -56,13 +56,7 @@ export default { return Api.branchSingle(projectId, currentBranchId); }, commit(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)); + return Api.commitMultiple(projectId, payload); }, getFiles(projectUrl, branchId) { const url = `${projectUrl}/files/${branchId}`; diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index 01f78a29cf6..04470064c1f 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -155,7 +155,7 @@ export const createCommitPayload = ({ last_commit_id: newBranch || f.deleted || f.prevPath || f.replaces ? undefined : f.lastCommitSha, })), - start_sha: newBranch ? rootGetters.lastCommit.short_id : undefined, + start_sha: newBranch ? rootGetters.lastCommit.id : undefined, }); export const createNewMergeRequestUrl = (projectUrl, source, target) => diff --git a/spec/frontend/ide/services/index_spec.js b/spec/frontend/ide/services/index_spec.js index 499fa8fc012..3d5ed4b5c0c 100644 --- a/spec/frontend/ide/services/index_spec.js +++ b/spec/frontend/ide/services/index_spec.js @@ -16,40 +16,16 @@ describe('IDE services', () => { branch: TEST_BRANCH, commit_message: 'Hello world', actions: [], - start_sha: undefined, + start_sha: TEST_COMMIT_SHA, }; - 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; + it('should commit', () => { + services.commit(TEST_PROJECT_ID, payload); - 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); - }); + 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 8a3c132972e..590af53b3f2 100644 --- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/actions_spec.js @@ -245,7 +245,7 @@ describe('IDE commit module actions', () => { master: { workingReference: '1', commit: { - short_id: TEST_COMMIT_SHA, + id: TEST_COMMIT_SHA, }, }, },