diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index 9f396571fc5b116c54c6b3e024ce1e074fe1a933..ff233cbbb2090e427697909a9ebb40970119a465 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -11,12 +11,17 @@ import { VARIANT_INFO } from '~/alert'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { saveAlertToLocalStorage } from '../local_storage_alert/save_alert_to_local_storage'; import getRefMixin from '../mixins/get_ref'; -import { prepareEditFormData, prepareCreateFormData } from '../utils/edit_form_data_utils'; +import { + prepareEditFormData, + prepareCreateFormData, + prepareDataForApiEdit, +} from '../utils/edit_form_data_utils'; const MR_SOURCE_BRANCH = 'merge_request[source_branch]'; export default { UPDATE_FILE_PATH: '/api/:version/projects/:id/repository/files/:file_path', + COMMIT_FILE_PATH: '/api/:version/projects/:id/repository/commits', components: { PageHeading, CommitChangesModal, @@ -109,11 +114,64 @@ export default { this.originalFilePath = this.editor.getOriginalFilePath(); this.$refs[this.updateModalId].show(); }, - handleError(message, errorCode = null) { - if (!message) return; - // Returns generic '403 Forbidden' error message - // Custom message will be added in https://gitlab.com/gitlab-org/gitlab/-/issues/569115 - this.error = errorCode === 403 ? this.errorMessage : message; + getUpdatePath(branch, filePath) { + const url = new URL(window.location.href); + url.pathname = joinPaths(this.projectPath, '-/blob', branch, filePath); + return url.toString(); + }, + getResultingBranch(responseData, formData) { + return responseData.branch || formData.branch_name || formData.original_branch; + }, + editBlob(originalFormData) { + const filePathChanged = originalFormData.file_path !== this.originalFilePath; + + if (filePathChanged) { + // Use Commits API for file rename/move operations + return this.editBlobWithCommitsApi(originalFormData); + } + + // Use Repository Files API for content-only updates + return this.editBlobWithUpdateFileApi(originalFormData); + }, + editBlobWithUpdateFileApi(originalFormData) { + const url = buildApiUrl(this.$options.UPDATE_FILE_PATH) + .replace(':id', this.projectId) + .replace(':file_path', encodeURIComponent(this.originalFilePath)); + + const data = { + ...prepareDataForApiEdit(originalFormData), + content: originalFormData.file, + file_path: originalFormData.file_path, + id: this.projectId, + last_commit_id: originalFormData.last_commit_sha, + }; + + return axios.put(url, data); + }, + editBlobWithCommitsApi(originalFormData) { + const url = buildApiUrl(this.$options.COMMIT_FILE_PATH).replace(':id', this.projectId); + + const action = { + action: 'move', + file_path: originalFormData.file_path, + previous_path: this.originalFilePath, + content: originalFormData.file, + last_commit_id: originalFormData.last_commit_sha, + }; + + const data = { + ...prepareDataForApiEdit(originalFormData), + actions: [action], + }; + + return axios.post(url, data); + }, + editBlobWithController(originalFormData) { + return axios({ + method: 'put', + url: this.updatePath, + data: originalFormData, + }); }, async handleFormSubmit(formData) { this.error = null; @@ -126,19 +184,12 @@ export default { } }, async handleEditFormSubmit(formData) { - const originalFormData = this.glFeatures.blobEditRefactor - ? prepareEditFormData(formData, { - fileContent: this.fileContent, - filePath: this.filePath, - lastCommitSha: this.lastCommitSha, - fromMergeRequestIid: this.fromMergeRequestIid, - }) - : prepareEditFormData(formData, { - fileContent: this.fileContent, - filePath: this.filePath, - lastCommitSha: this.lastCommitSha, - fromMergeRequestIid: this.fromMergeRequestIid, - }); + const originalFormData = prepareEditFormData(formData, { + fileContent: this.fileContent, + filePath: this.filePath, + lastCommitSha: this.lastCommitSha, + fromMergeRequestIid: this.fromMergeRequestIid, + }); try { const response = this.glFeatures.blobEditRefactor @@ -193,47 +244,21 @@ export default { this.isLoading = false; } }, - editBlob(originalFormData) { - const url = buildApiUrl(this.$options.UPDATE_FILE_PATH) - .replace(':id', this.projectId) - .replace(':file_path', encodeURIComponent(this.originalFilePath)); - - const data = { - branch: originalFormData.branch_name || originalFormData.original_branch, - commit_message: originalFormData.commit_message, - content: originalFormData.file, - file_path: originalFormData.file_path, - id: this.projectId, - last_commit_id: originalFormData.last_commit_sha, - }; - - // Only include start_branch when creating a new branch - if ( - originalFormData.branch_name && - originalFormData.branch_name !== originalFormData.original_branch - ) { - data.start_branch = originalFormData.original_branch; - } - - return axios.put(url, data); - }, - editBlobWithController(originalFormData) { - return axios({ - method: 'put', - url: this.updatePath, - data: originalFormData, - }); - }, handleEditBlobSuccess(responseData, formData) { - if (formData.create_merge_request && this.originalBranch !== responseData.branch) { + const resultingBranch = this.getResultingBranch(responseData, formData); + const isNewBranch = this.originalBranch !== resultingBranch; + + if (formData.create_merge_request && isNewBranch) { const mrUrl = mergeUrlParams( - { [MR_SOURCE_BRANCH]: responseData.branch }, + { [MR_SOURCE_BRANCH]: resultingBranch }, this.newMergeRequestPath, ); visitUrl(mrUrl); } else { - const successPath = this.getUpdatePath(responseData.branch, responseData.file_path); - const isNewBranch = this.originalBranch !== responseData.branch; + const successPath = this.getUpdatePath( + resultingBranch, + responseData.file_path || formData.file_path, + ); const createMergeRequestNotChosen = !formData.create_merge_request; const message = this.successMessageForAlert(isNewBranch, createMergeRequestNotChosen); @@ -254,10 +279,11 @@ export default { this.handleError(this.errorMessage); } }, - getUpdatePath(branch, filePath) { - const url = new URL(window.location.href); - url.pathname = joinPaths(this.projectPath, '-/blob', branch, filePath); - return url.toString(); + handleError(message, errorCode = null) { + if (!message) return; + // Returns generic '403 Forbidden' error message + // Custom message will be added in https://gitlab.com/gitlab-org/gitlab/-/issues/569115 + this.error = errorCode === 403 ? this.errorMessage : message; }, }, i18n: { diff --git a/app/assets/javascripts/repository/utils/edit_form_data_utils.js b/app/assets/javascripts/repository/utils/edit_form_data_utils.js index f4bbc651247fb8b9f612afe920bb945b33c38fea..15959f95c1b0f60775a91b28d8209f5e72b67d0c 100644 --- a/app/assets/javascripts/repository/utils/edit_form_data_utils.js +++ b/app/assets/javascripts/repository/utils/edit_form_data_utils.js @@ -49,3 +49,29 @@ export const prepareCreateFormData = (formData, { filePath, fileContent }) => { return Object.fromEntries(formData); }; + +/** + * Prepares common data fields for API edit operations including branch information + * and commit message. Conditionally adds start_branch when creating a new branch. + * This is used when the target branch differs from the original branch, + * indicating that a new branch should be created from the original branch. + * + * @param {Object} formData - The form data containing branch and commit information + * @param {string} formData.branch_name - The target branch name + * @param {string} formData.original_branch - The original branch name + * @param {string} formData.commit_message - The commit message + * @returns {Object} Object with branch, commit_message, and conditionally start_branch + */ +export const prepareDataForApiEdit = (formData) => { + const data = { + branch: formData.branch_name || formData.original_branch, + commit_message: formData.commit_message, + }; + + // Only include start_branch when creating a new branch + if (formData.branch_name && formData.branch_name !== formData.original_branch) { + data.start_branch = formData.original_branch; + } + + return data; +}; diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb index f0b06d7f622b179b57db09849103150170481f2a..987083fef361b9f1a42f8c50398e8916e0826d08 100644 --- a/spec/features/projects/files/user_edits_files_spec.rb +++ b/spec/features/projects/files/user_edits_files_spec.rb @@ -102,6 +102,27 @@ expect(page).to have_content('New commit message') end + it 'commits a renamed file' do + click_link('.gitignore') + edit_in_single_file_editor + find('.file-editor', match: :first) + + fill_in('File path', with: '.gitignore-v1') + click_button('Commit changes') + + within_testid('commit-change-modal') do + fill_in(:commit_message, with: 'New commit message', visible: true) + click_button('Commit changes') + end + + expect(page).to have_current_path(project_blob_path(project, 'master/.gitignore-v1'), ignore_query: true) + + wait_for_requests + + expect(page).to have_content('.gitignore-v1') + expect(page).to have_content('New commit message') + end + it 'displays a flash message with a link when an edited file was committed' do click_link('.gitignore') edit_in_single_file_editor diff --git a/spec/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js index 035eee70232f26ccb968d059e559d61a836d98c8..4f662e885a3692414e371af1a31bfdbc08df9b7a 100644 --- a/spec/frontend/repository/pages/blob_edit_header_spec.js +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -33,7 +33,7 @@ describe('BlobEditHeader', () => { getFileContent: jest.fn().mockReturnValue(content), getOriginalFilePath: jest.fn().mockReturnValue('test.js'), filepathFormMediator: { - $filenameInput: { val: jest.fn().mockReturnValue('.gitignore') }, + $filenameInput: { val: jest.fn().mockReturnValue('test.js') }, toggleValidationError: jest.fn(), }, }; @@ -138,6 +138,10 @@ describe('BlobEditHeader', () => { describe('for edit blob', () => { describe('when blobEditRefactor is enabled', () => { + beforeEach(() => { + clickCommitChangesButton(); + }); + it('shows confirmation message on cancel button', () => { expect(findCancelButton().attributes('data-confirm')).toBe( 'Leave edit mode? All unsaved changes will be lost.', @@ -146,7 +150,6 @@ describe('BlobEditHeader', () => { it('on submit, saves success message to localStorage and redirects to the updated file', async () => { // First click the commit button to open the modal and set up the file content - clickCommitChangesButton(); mock.onPut().replyOnce(HTTP_STATUS_OK, { branch: 'feature', file_path: 'test.js', @@ -206,6 +209,9 @@ describe('BlobEditHeader', () => { provided: { canPushToBranch: false }, }); + // First click the commit button to open the modal and set up the file content + clickCommitChangesButton(); + mock.onPut().replyOnce(HTTP_STATUS_OK, { branch: 'feature', file_path: 'test.js', @@ -283,6 +289,83 @@ describe('BlobEditHeader', () => { ); }); }); + + describe('when renaming a file', () => { + beforeEach(() => { + // Mock the editor to return a different file path to trigger rename logic + mockEditor.filepathFormMediator.$filenameInput.val.mockReturnValue('renamed_test.js'); + mockEditor.getOriginalFilePath.mockReturnValue('test.js'); + clickCommitChangesButton(); + }); + + it('uses commits API when file path changes', async () => { + mock.onPost().replyOnce(HTTP_STATUS_OK, {}); + + await submitForm(); + + expect(mock.history.post).toHaveLength(1); + expect(mock.history.post[0].url).toBe('/api/v4/projects/123/repository/commits'); + + const postData = JSON.parse(mock.history.post[0].data); + expect(postData.branch).toBe('feature'); + expect(postData.commit_message).toBe('Test commit'); + expect(postData.actions).toHaveLength(1); + expect(postData.actions[0]).toEqual({ + action: 'move', + file_path: 'renamed_test.js', + previous_path: 'test.js', + content, + last_commit_id: '782426692977b2cedb4452ee6501a404410f9b00', + }); + }); + + it('uses original_branch when branch_name is not provided', async () => { + mock.onPost().replyOnce(HTTP_STATUS_OK, {}); + + const formData = new FormData(); + formData.append('commit_message', 'Test commit'); + formData.append('original_branch', 'main'); + findCommitChangesModal().vm.$emit('submit-form', formData); + await axios.waitForAll(); + + const postData = JSON.parse(mock.history.post[0].data); + expect(postData.branch).toBe('main'); + }); + + it('redirects to renamed file on successful submission', async () => { + mock.onPost().replyOnce(HTTP_STATUS_OK, {}); + + await submitForm(); + + expect(visitUrlSpy).toHaveBeenCalledWith( + 'http://test.host/gitlab-org/gitlab/-/blob/feature/renamed_test.js', + ); + }); + + it('handles error responses from commits API', async () => { + const errorMessage = 'File rename failed'; + mock.onPost().replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { + message: errorMessage, + }); + + await submitForm(); + + expect(findCommitChangesModal().props('error')).toBe(errorMessage); + expect(visitUrlSpy).not.toHaveBeenCalled(); + }); + + it('handles error in response data from commits API', async () => { + const errorMessage = 'Validation failed'; + mock.onPost().replyOnce(HTTP_STATUS_OK, { + error: errorMessage, + }); + + await submitForm(); + + expect(findCommitChangesModal().props('error')).toBe(errorMessage); + expect(visitUrlSpy).not.toHaveBeenCalled(); + }); + }); }); describe('when blobEditRefactor is disabled', () => { @@ -312,6 +395,7 @@ describe('BlobEditHeader', () => { expect(findCommitChangesModal().props('error')).toBe(errorMessage); expect(visitUrlSpy).not.toHaveBeenCalled(); }); + it('shows error message in modal when request fails', async () => { mock .onPut('/update') diff --git a/spec/frontend/repository/utils/edit_form_data_utils_spec.js b/spec/frontend/repository/utils/edit_form_data_utils_spec.js index 5aec1772658142b819c82a9a0d933ee6f420d124..cf678be3a1f98737be919a3f8e4715f03e0d8521 100644 --- a/spec/frontend/repository/utils/edit_form_data_utils_spec.js +++ b/spec/frontend/repository/utils/edit_form_data_utils_spec.js @@ -1,6 +1,7 @@ import { prepareEditFormData, prepareCreateFormData, + prepareDataForApiEdit, } from '~/repository/utils/edit_form_data_utils'; describe('edit_form_data_utils', () => { @@ -75,4 +76,102 @@ describe('edit_form_data_utils', () => { expect(result.content).toBe(multilineParams.fileContent); }); }); + + describe('prepareDataForApiEdit', () => { + it('returns branch and commit_message when branch_name is not provided', () => { + const originalFormData = { + original_branch: 'main', + commit_message: 'Update file content', + }; + + const result = prepareDataForApiEdit(originalFormData); + + expect(result).toEqual({ + branch: 'main', + commit_message: 'Update file content', + }); + }); + + it('returns branch and commit_message when branch_name equals original_branch', () => { + const originalFormData = { + branch_name: 'main', + original_branch: 'main', + commit_message: 'Update file content', + }; + + const result = prepareDataForApiEdit(originalFormData); + + expect(result).toEqual({ + branch: 'main', + commit_message: 'Update file content', + }); + }); + + it('includes start_branch when creating a new branch', () => { + const originalFormData = { + branch_name: 'feature-branch', + original_branch: 'main', + commit_message: 'Add new feature', + }; + + const result = prepareDataForApiEdit(originalFormData); + + expect(result).toEqual({ + branch: 'feature-branch', + commit_message: 'Add new feature', + start_branch: 'main', + }); + }); + + it('uses branch_name over original_branch when both are provided', () => { + const originalFormData = { + branch_name: 'develop', + original_branch: 'main', + commit_message: 'Merge changes', + }; + + const result = prepareDataForApiEdit(originalFormData); + + expect(result).toEqual({ + branch: 'develop', + commit_message: 'Merge changes', + start_branch: 'main', + }); + }); + + it.each([ + ['empty', ''], + ['null', null], + ['undefined', undefined], + ])('handles %s branch_name by falling back to original_branch', (_, branchName) => { + const originalFormData = { + branch_name: branchName, + original_branch: 'main', + commit_message: 'Fix bug', + }; + + const result = prepareDataForApiEdit(originalFormData); + + expect(result).toEqual({ + branch: 'main', + commit_message: 'Fix bug', + }); + }); + + it('handles special characters in branch names', () => { + const originalFormData = { + branch_name: 'feature/special-chars_123', + original_branch: 'main', + commit_message: 'Add feature with special chars', + }; + + const result = prepareDataForApiEdit(originalFormData); + + expect(result).toEqual({ + branch: 'feature/special-chars_123', + commit_message: 'Add feature with special chars', + start_branch: 'main', + }); + }); + }); });