diff --git a/app/assets/javascripts/blob_edit/blob_edit_header.js b/app/assets/javascripts/blob_edit/blob_edit_header.js index 1a93872f927722d57408b154125c6d966e92354b..dc76c639ecbe22f9f7e06efa32ddb900225840eb 100644 --- a/app/assets/javascripts/blob_edit/blob_edit_header.js +++ b/app/assets/javascripts/blob_edit/blob_edit_header.js @@ -21,6 +21,9 @@ export default function initBlobEditHeader(editor) { blobName, branchAllowsCollaboration, lastCommitSha, + projectId, + projectPath, + newMergeRequestPath, } = el.dataset; return new Vue({ @@ -34,6 +37,9 @@ export default function initBlobEditHeader(editor) { targetBranch, blobName, lastCommitSha, + projectId, + projectPath, + newMergeRequestPath, emptyRepo: parseBoolean(emptyRepo), canPushCode: parseBoolean(canPushCode), canPushToBranch: parseBoolean(canPushToBranch), diff --git a/app/assets/javascripts/blob_edit/edit_blob.js b/app/assets/javascripts/blob_edit/edit_blob.js index ddca90579325c33f919facb770229f334c25092e..e9b98df2916bb547c680e199a1d3c762580c1f5b 100644 --- a/app/assets/javascripts/blob_edit/edit_blob.js +++ b/app/assets/javascripts/blob_edit/edit_blob.js @@ -246,4 +246,8 @@ export default class EditBlob { getFileContent() { return this.editor?.getValue(); } + + getOriginalFilePath() { + return this.options.filePath; + } } diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index 34dc6cb6700a0d24805b15cfdd6729c0c1817940..6ac28c7d3395812e6211bd31e51443a7865db416 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -5,16 +5,21 @@ import axios from '~/lib/utils/axios_utils'; import { __, sprintf } from '~/locale'; import PageHeading from '~/vue_shared/components/page_heading.vue'; import CommitChangesModal from '~/repository/components/commit_changes_modal.vue'; -import { getParameterByName, visitUrl } from '~/lib/utils/url_utility'; +import { getParameterByName, visitUrl, joinPaths, mergeUrlParams } from '~/lib/utils/url_utility'; +import { buildApiUrl } from '~/api/api_utils'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import getRefMixin from '../mixins/get_ref'; +const MR_SOURCE_BRANCH = 'merge_request[source_branch]'; + export default { + UPDATE_FILE_PATH: '/api/:version/projects/:id/repository/files/:file_path', components: { PageHeading, CommitChangesModal, GlButton, }, - mixins: [getRefMixin], + mixins: [getRefMixin, glFeatureFlagMixin()], inject: [ 'action', 'editor', @@ -28,12 +33,16 @@ export default { 'emptyRepo', 'branchAllowsCollaboration', 'lastCommitSha', + 'projectId', + 'projectPath', + 'newMergeRequestPath', ], data() { return { isCommitChangeModalOpen: false, fileContent: null, filePath: null, + originalFilePath: null, isLoading: false, error: null, }; @@ -75,53 +84,154 @@ export default { this.error = null; this.fileContent = this.editor.getFileContent(); this.filePath = filePath; + this.originalFilePath = this.editor.getOriginalFilePath(); this.$refs[this.updateModalId].show(); }, - handleError(message) { + prepareFormData(formData) { + formData.append('file', this.fileContent); + formData.append('file_path', this.filePath); + formData.append('last_commit_sha', this.lastCommitSha); + formData.append('from_merge_request_iid', this.fromMergeRequestIid); + + return Object.fromEntries(formData); + }, + prepareControllerFormData(formData) { + formData.append('file', this.fileContent); + formData.append('file_path', this.filePath); + formData.append('last_commit_sha', this.lastCommitSha); + formData.append('from_merge_request_iid', this.fromMergeRequestIid); + + return Object.fromEntries(formData); + }, + handleError(message, errorCode = null) { if (!message) return; - this.error = message; + // 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; }, - handleFormSubmit(formData) { + async handleFormSubmit(formData) { this.error = null; this.isLoading = true; + if (this.isEditBlob) { - formData.append('file', this.fileContent); - formData.append('file_path', this.filePath); - formData.append('last_commit_sha', this.lastCommitSha); - formData.append('from_merge_request_iid', this.fromMergeRequestIid); + this.handleEditFormSubmit(formData); } else { - formData.append('file_name', this.filePath); - formData.append('content', this.fileContent); + this.handleCreateFormSubmit(formData); + } + }, + async handleEditFormSubmit(formData) { + const originalFormData = this.prepareFormData(formData); + + try { + const response = this.glFeatures.blobEditRefactor + ? await this.editBlob(originalFormData) + : await this.editBlobWithController(originalFormData); + const { data: responseData } = response; + + if (responseData.error) { + this.handleError(responseData.error); + return; + } + + if (this.glFeatures.blobEditRefactor) { + this.handleEditBlobSuccess(responseData, originalFormData); + } else { + this.handleControllerSuccess(responseData); + } + } catch (error) { + const errorMessage = + error.response?.data?.message || error.response?.data?.error || this.errorMessage; + this.handleError(errorMessage, error.response?.status); + } finally { + this.isLoading = false; } + }, + async handleCreateFormSubmit(formData) { + formData.append('file_name', this.filePath); + formData.append('content', this.fileContent); // Object.fromEntries is used here to handle potential line ending mutations in `FormData`. // `FormData` uses the "multipart/form-data" format (RFC 2388), which follows MIME data stream rules (RFC 2046). // These specifications require line breaks to be represented as CRLF sequences in the canonical form. // See https://stackoverflow.com/questions/69835705/formdata-textarea-puts-r-carriage-return-when-sent-with-post for more details. - const data = Object.fromEntries(formData); + const originalFormData = Object.fromEntries(formData); + + try { + const response = await axios({ + method: 'post', + url: this.updatePath, + data: originalFormData, + }); + + const { data: responseData } = response; + + if (responseData.error) { + this.handleError(responseData.error); + return; + } + this.handleControllerSuccess(responseData); + } catch (error) { + const errorMessage = + error.response?.data?.message || error.response?.data?.error || this.errorMessage; + this.handleError(errorMessage, error.response?.status); + } finally { + 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: this.isEditBlob ? 'put' : 'post', + method: 'put', url: this.updatePath, - data, - }) - .then(({ data: responseData }) => { - if (responseData.error) { - this.handleError(responseData.error); - return; - } - - if (responseData.filePath) { - visitUrl(responseData.filePath); - return; - } - - this.handleError(this.errorMessage); - }) - .catch(({ response }) => this.handleError(response?.data?.error)) - .finally(() => { - this.isLoading = false; - }); + data: originalFormData, + }); + }, + handleEditBlobSuccess(responseData, formData) { + if (formData.create_merge_request && this.originalBranch !== responseData.branch) { + const mrUrl = mergeUrlParams( + { [MR_SOURCE_BRANCH]: responseData.branch }, + this.newMergeRequestPath, + ); + visitUrl(mrUrl); + } else { + visitUrl(this.getUpdatePath(responseData.branch, responseData.file_path)); + } + }, + handleControllerSuccess(responseData) { + if (responseData.filePath) { + visitUrl(responseData.filePath); + } else { + 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(); }, }, i18n: { diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 11aa078de6915fb7960722eb8f0cca991f487553..a541e231c67f38208659445625d4dce409378136 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -51,6 +51,7 @@ class Projects::BlobController < Projects::ApplicationController push_licensed_feature(:file_locks) if @project.licensed_feature_available?(:file_locks) push_frontend_feature_flag(:directory_code_dropdown_updates, current_user) push_frontend_feature_flag(:repository_file_tree_browser, @project) + push_frontend_feature_flag(:blob_edit_refactor, @project) end def new diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index bb379a08f0b241c5f96af02ee1d8dc5695661d0f..b7e0f568f5efc4310856630ff1017132c709757a 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -351,7 +351,10 @@ def edit_blob_app_data(project, id, blob, ref, action) empty_repo: project.empty_repo?.to_s, blob_name: is_update ? blob.name : nil, branch_allows_collaboration: project.branch_allows_collaboration?(current_user, ref).to_s, - last_commit_sha: @last_commit_sha + last_commit_sha: @last_commit_sha, + project_id: project.id, + project_path: project.full_path, + new_merge_request_path: project_new_merge_request_path(project) } end diff --git a/config/feature_flags/wip/blob_edit_refactor.yml b/config/feature_flags/wip/blob_edit_refactor.yml new file mode 100644 index 0000000000000000000000000000000000000000..26ff0fbe525fd404640ff7e49e906081a9df5649 --- /dev/null +++ b/config/feature_flags/wip/blob_edit_refactor.yml @@ -0,0 +1,10 @@ +--- +name: blob_edit_refactor +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/509968 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/205529 +rollout_issue_url: +milestone: '18.5' +group: group::source code +type: wip +default_enabled: false diff --git a/qa/qa/specs/features/browser_ui/3_create/source_editor/source_editor_toolbar_spec.rb b/qa/qa/specs/features/browser_ui/3_create/source_editor/source_editor_toolbar_spec.rb index 0aab3aeda2953abcfdafac90481a2c1b9f8a61ce..858e063309bee5d749a4b5759b44564ede8ec742 100644 --- a/qa/qa/specs/features/browser_ui/3_create/source_editor/source_editor_toolbar_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/source_editor/source_editor_toolbar_spec.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true module QA - RSpec.describe 'Create', feature_category: :source_code_management do + RSpec.describe 'Create', feature_category: :source_code_management, feature_flag: { + name: :blob_edit_refactor + } do describe 'Source editor toolbar preview' do let(:project) { create(:project, :with_readme, name: 'empty-project-with-md') } let(:edited_readme_content) { 'Here is the edited content.' } @@ -12,6 +14,10 @@ module QA it 'can preview markdown side-by-side while editing', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/367749' do + # Skip test when blob_edit_refactor feature flag is enabled as it is WIP + # https://gitlab.com/gitlab-org/gitlab/-/issues/509968 + skip 'blob_edit_refactor feature flag is WIP' if Runtime::Feature.enabled?(:blob_edit_refactor) + project.visit! Page::Project::Show.perform do |project| project.click_file('README.md') diff --git a/spec/controllers/projects/find_file_controller_spec.rb b/spec/controllers/projects/find_file_controller_spec.rb index 68810bae36879c3162f75da96f131a488b9ec36a..62bef9b0a0fedda26c225502e5cfb33c1a019108 100644 --- a/spec/controllers/projects/find_file_controller_spec.rb +++ b/spec/controllers/projects/find_file_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::FindFileController do +RSpec.describe Projects::FindFileController, feature_category: :source_code_management do let(:project) { create(:project, :repository) } let(:user) { create(:user) } diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index f50af8dace9da4a3880786524140df6e0d6cd8af..92969a89c067a2fbeb48008a49fb802a6a88a8af 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -24,6 +24,7 @@ end before do + stub_feature_flags(blob_edit_refactor: false) target_project.add_maintainer(user) sign_in(user) diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index c0be951c343db9ff04efca737595d8f2ff4c5abb..38b8aa46aaf5196c084b2945ac0814cd124463b5 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -50,6 +50,7 @@ def fill_editor(content: 'class NextFeature\\nend\\n') context 'from MR diff' do before do + stub_feature_flags(blob_edit_refactor: false) visit diffs_project_merge_request_path(project, merge_request) edit_and_commit(is_diff: true) end @@ -114,6 +115,7 @@ def has_toolbar_buttons context 'from blob file path' do before do + stub_feature_flags(blob_edit_refactor: false) visit project_blob_path(project, tree_join(branch, file_path)) end diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb index 58535c82453177f2a186e93cd9ffe6b8ded3fb9d..798155aa1d248a8990d5b1d6877e9ac63b418953 100644 --- a/spec/features/projects/files/editing_a_file_spec.rb +++ b/spec/features/projects/files/editing_a_file_spec.rb @@ -17,6 +17,17 @@ } end + let(:second_commit_params) do + { + start_branch: project.default_branch, + branch_name: project.default_branch, + commit_message: "Committing Second Update", + file_path: ".gitignore", + file_content: "Second Update", + last_commit_sha: Gitlab::Git::Commit.last_for_path(project.repository, project.default_branch, ".gitignore").sha + } + end + context 'when the user has write access' do before do sign_in user @@ -32,7 +43,26 @@ click_button('Commit changes') end - expect(page).to have_content 'An error occurred editing the blob' + expect(page).to have_content 'You are attempting to update a file that has changed since you started editing it.' + end + + context 'and blob_edit_refactor feature flag is false' do + before do + stub_feature_flags(blob_edit_refactor: false) + sign_in user + visit project_edit_blob_path(project, File.join(project.default_branch, '.gitignore')) + end + + it 'file has been updated since the user opened the edit page' do + Files::UpdateService.new(project, user, second_commit_params).execute + click_button 'Commit changes' + + within_testid('commit-change-modal') do + click_button('Commit changes') + end + + expect(page).to have_content 'An error occurred editing the blob' + end end end diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb index 12755b244dd0bd7e28afa21c35967f47fa671f01..5735be774609397ed77b021357293903c56c7554 100644 --- a/spec/features/projects/files/user_edits_files_spec.rb +++ b/spec/features/projects/files/user_edits_files_spec.rb @@ -48,6 +48,7 @@ context 'when an user has write access', :js do before do + stub_feature_flags(blob_edit_refactor: false) project.add_maintainer(user) visit(project_tree_path_root_ref) wait_for_requests @@ -163,6 +164,7 @@ context 'when an user does not have write access', :js do before do + stub_feature_flags(blob_edit_refactor: false) project2.add_reporter(user) visit(project2_tree_path_root_ref) wait_for_requests @@ -262,6 +264,7 @@ def expect_fork_status let!(:forked_project) { fork_project(project2, user, namespace: user.namespace, repository: true) } before do + stub_feature_flags(blob_edit_refactor: false) visit(project2_tree_path_root_ref) wait_for_requests end diff --git a/spec/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js index cd950ff110a56214142c62f4c8e10d245039cc22..c2f8b1a5d8537d76103b46342f8349e959681353 100644 --- a/spec/frontend/repository/pages/blob_edit_header_spec.js +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -3,12 +3,17 @@ import MockAdapter from 'axios-mock-adapter'; import { GlButton } from '@gitlab/ui'; import axios from '~/lib/utils/axios_utils'; import * as urlUtility from '~/lib/utils/url_utility'; -import { HTTP_STATUS_OK, HTTP_STATUS_UNPROCESSABLE_ENTITY } from '~/lib/utils/http_status'; +import { + HTTP_STATUS_OK, + HTTP_STATUS_UNPROCESSABLE_ENTITY, + HTTP_STATUS_FORBIDDEN, +} from '~/lib/utils/http_status'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import CommitChangesModal from '~/repository/components/commit_changes_modal.vue'; import BlobEditHeader from '~/repository/pages/blob_edit_header.vue'; import PageHeading from '~/vue_shared/components/page_heading.vue'; import { stubComponent } from 'helpers/stub_component'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; jest.mock('~/alert'); jest.mock('lodash/uniqueId', () => { @@ -24,13 +29,14 @@ describe('BlobEditHeader', () => { const mockEditor = { getFileContent: jest.fn().mockReturnValue(content), + getOriginalFilePath: jest.fn().mockReturnValue('test.js'), filepathFormMediator: { $filenameInput: { val: jest.fn().mockReturnValue('.gitignore') }, toggleValidationError: jest.fn(), }, }; - const createWrapper = ({ action = 'update' } = {}) => { + const createWrapper = ({ action = 'update', glFeatures = {} } = {}) => { return shallowMountExtended(BlobEditHeader, { provide: { action, @@ -45,7 +51,12 @@ describe('BlobEditHeader', () => { emptyRepo: false, branchAllowsCollaboration: false, lastCommitSha: '782426692977b2cedb4452ee6501a404410f9b00', + projectId: 123, + projectPath: 'gitlab-org/gitlab', + newMergeRequestPath: 'merge_request/new/123', + glFeatures: { blobEditRefactor: true, ...glFeatures }, }, + mixins: [glFeatureFlagMixin()], stubs: { PageHeading, CommitChangesModal: stubComponent(CommitChangesModal, { @@ -58,6 +69,8 @@ describe('BlobEditHeader', () => { }; beforeEach(() => { + window.gon = { api_version: 'v4' }; + visitUrlSpy = jest.spyOn(urlUtility, 'visitUrl'); mock = new MockAdapter(axios); wrapper = createWrapper(); @@ -74,86 +87,203 @@ describe('BlobEditHeader', () => { const findCommitChangesButton = () => wrapper.findByTestId('blob-edit-header-commit-button'); const findCancelButton = () => wrapper.findByTestId('blob-edit-header-cancel-button'); + const clickCommitChangesButton = async () => { + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + }; + const submitForm = async () => { - findCommitChangesModal().vm.$emit('submit-form', new FormData()); + const formData = new FormData(); + formData.append('commit_message', 'Test commit'); + formData.append('branch_name', 'feature'); + formData.append('original_branch', 'main'); + + findCommitChangesModal().vm.$emit('submit-form', formData); await axios.waitForAll(); }; describe('for edit blob', () => { - it('renders title with two buttons', () => { - expect(findTitle().text()).toBe('Edit file'); - const buttons = findButtons(); - expect(buttons).toHaveLength(2); - expect(buttons.at(0).text()).toBe('Cancel'); - expect(buttons.at(1).text()).toBe('Commit changes'); - }); + describe('when blobEditRefactor is enabled', () => { + it('renders title with two buttons', () => { + expect(findTitle().text()).toBe('Edit file'); + const buttons = findButtons(); + expect(buttons).toHaveLength(2); + expect(buttons.at(0).text()).toBe('Cancel'); + expect(buttons.at(1).text()).toBe('Commit changes'); + }); - it('opens commit changes modal with correct props', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); - expect(mockEditor.getFileContent).toHaveBeenCalled(); - expect(findCommitChangesModal().props()).toEqual({ - modalId: 'update-modal1', - canPushCode: true, - canPushToBranch: true, - commitMessage: 'Edit test.js', - emptyRepo: false, - isUsingLfs: false, - originalBranch: 'main', - targetBranch: 'feature', - loading: false, - branchAllowsCollaboration: false, - valid: true, - error: null, + it('opens commit changes modal with correct props', () => { + clickCommitChangesButton(); + expect(mockEditor.getFileContent).toHaveBeenCalled(); + expect(findCommitChangesModal().props()).toEqual({ + modalId: 'update-modal1', + canPushCode: true, + canPushToBranch: true, + commitMessage: 'Edit test.js', + emptyRepo: false, + isUsingLfs: false, + originalBranch: 'main', + targetBranch: 'feature', + loading: false, + branchAllowsCollaboration: false, + valid: true, + error: null, + }); }); - }); - it('shows confirmation message on cancel button', () => { - expect(findCancelButton().attributes('data-confirm')).toBe( - 'Leave edit mode? All unsaved changes will be lost.', - ); - }); + it('shows confirmation message on cancel button', () => { + expect(findCancelButton().attributes('data-confirm')).toBe( + 'Leave edit mode? All unsaved changes will be lost.', + ); + }); - it('on submit, redirects to the updated file', async () => { - findCommitChangesButton().vm.$emit('click'); + it('on submit, 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', + }); + await submitForm(); - mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); - await submitForm(); + expect(mock.history.put).toHaveLength(1); + expect(mock.history.put[0].url).toBe('/api/v4/projects/123/repository/files/test.js'); + const putData = JSON.parse(mock.history.put[0].data); + expect(putData.content).toBe(content); + expect(visitUrlSpy).toHaveBeenCalledWith( + 'http://test.host/gitlab-org/gitlab/-/blob/feature/test.js', + ); + }); - expect(mock.history.put).toHaveLength(1); - const putData = JSON.parse(mock.history.put[0].data); - expect(putData.file).toBe(content); - expect(visitUrlSpy).toHaveBeenCalledWith('/update/path'); + describe('error handling', () => { + const errorMessage = 'Custom error message'; + + beforeEach(() => { + clickCommitChangesButton(); + }); + + it('shows error message in modal when response contains error', async () => { + mock.onPut().replyOnce(HTTP_STATUS_OK, { error: errorMessage }); + await submitForm(); + + expect(findCommitChangesModal().props('error')).toBe(errorMessage); + expect(visitUrlSpy).not.toHaveBeenCalled(); + }); + + it('shows error message in modal when request fails', async () => { + mock.onPut().replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { message: errorMessage }); + await submitForm(); + + expect(findCommitChangesModal().props('error')).toBe(errorMessage); + }); + + it('shows customized error message, when generic 403 error is returned from backend', async () => { + mock.onPut().replyOnce(HTTP_STATUS_FORBIDDEN, { message: 'Access denied' }); + await submitForm(); + + expect(findCommitChangesModal().props('error')).toBe( + 'An error occurred editing the blob', + ); + expect(visitUrlSpy).not.toHaveBeenCalled(); + }); + + it('clears error on successful submission', async () => { + mock.onPut().replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY); + await submitForm(); + + // Verify error is set first + expect(findCommitChangesModal().props('error')).toBe( + 'An error occurred editing the blob', + ); + + // Mock visitUrl to prevent actual navigation + visitUrlSpy.mockImplementation(() => {}); + + mock.onPut().replyOnce(HTTP_STATUS_OK, { + branch: 'feature', + file_path: 'test.js', + }); + jest.spyOn(console, 'error').mockImplementation(() => {}); + + // Submit the form again + await submitForm(); + await nextTick(); + + // The error should be cleared at the start of handleFormSubmit + expect(findCommitChangesModal().props('error')).toBeNull(); + expect(visitUrlSpy).toHaveBeenCalledWith( + 'http://test.host/gitlab-org/gitlab/-/blob/feature/test.js', + ); + }); + }); }); - describe('error handling', () => { - const errorMessage = 'Custom error message'; + describe('when blobEditRefactor is disabled', () => { + beforeEach(() => { + wrapper = createWrapper({ glFeatures: { blobEditRefactor: false } }); + clickCommitChangesButton(); + }); - it('shows error message in modal when response contains error', async () => { - mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { error: errorMessage }); + it('on submit, redirects to the updated file using controller', async () => { + mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); await submitForm(); - expect(findCommitChangesModal().props('error')).toBe(errorMessage); - expect(visitUrlSpy).not.toHaveBeenCalled(); + expect(mock.history.put).toHaveLength(1); + expect(mock.history.put[0].url).toBe('/update'); + const putData = JSON.parse(mock.history.put[0].data); + expect(putData.file).toBe(content); + expect(visitUrlSpy).toHaveBeenCalledWith('/update/path'); }); - it('shows error message in modal when request fails', async () => { - mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { error: errorMessage }); - await submitForm(); + describe('error handling', () => { + const errorMessage = 'Controller error message'; - expect(findCommitChangesModal().props('error')).toBe(errorMessage); - }); + it('shows error message in modal when response contains error', async () => { + mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { error: errorMessage }); + await submitForm(); - it('clears error on successful submission', async () => { - mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY); - await submitForm(); + expect(findCommitChangesModal().props('error')).toBe(errorMessage); + expect(visitUrlSpy).not.toHaveBeenCalled(); + }); + it('shows error message in modal when request fails', async () => { + mock + .onPut('/update') + .replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { error: errorMessage }); + await submitForm(); - mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); - jest.spyOn(console, 'error').mockImplementation(() => {}); - await submitForm(); + expect(findCommitChangesModal().props('error')).toBe(errorMessage); + }); + + it('shows error message when response does not contain filePath', async () => { + mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { message: 'success' }); + await submitForm(); + + expect(findCommitChangesModal().props('error')).toBe( + 'An error occurred editing the blob', + ); + expect(visitUrlSpy).not.toHaveBeenCalled(); + }); + + it('clears error on successful submission', async () => { + findCommitChangesButton().vm.$emit('click'); + await nextTick(); - expect(findCommitChangesModal().props('error')).toBeNull(); + mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY); + await submitForm(); + + expect(findCommitChangesModal().props('error')).toBe( + 'An error occurred editing the blob', + ); + + visitUrlSpy.mockImplementation(() => {}); + mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); + + await submitForm(); + await nextTick(); + + expect(findCommitChangesModal().props('error')).toBeNull(); + }); }); }); }); @@ -171,9 +301,8 @@ describe('BlobEditHeader', () => { expect(buttons.at(1).text()).toBe('Commit changes'); }); - it('opens commit changes modal with correct props', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); + it('opens commit changes modal with correct props', () => { + clickCommitChangesButton(); expect(mockEditor.getFileContent).toHaveBeenCalled(); expect(findCommitChangesModal().props()).toEqual({ modalId: 'update-modal1', @@ -198,8 +327,7 @@ describe('BlobEditHeader', () => { }); it('on submit, redirects to the new file', async () => { - findCommitChangesButton().vm.$emit('click'); - + clickCommitChangesButton(); mock.onPost('/update').reply(HTTP_STATUS_OK, { filePath: '/new/file' }); await submitForm(); @@ -214,8 +342,7 @@ describe('BlobEditHeader', () => { it('toggles validation error when filename is empty', () => { mockEditor.filepathFormMediator.$filenameInput.val.mockReturnValue(null); wrapper = createWrapper(); - - findCommitChangesButton().vm.$emit('click'); + clickCommitChangesButton(); expect(mockEditor.filepathFormMediator.toggleValidationError).toHaveBeenCalledWith(true); }); diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index debb600198a527dab448e2ca026e112abd971361..cc3dc33d5c1afc91abfb4779213cf3407dfdbb78 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -508,7 +508,10 @@ empty_repo: 'false', blob_name: blob.name, branch_allows_collaboration: 'false', - last_commit_sha: '782426692977b2cedb4452ee6501a404410f9b00' + last_commit_sha: '782426692977b2cedb4452ee6501a404410f9b00', + project_id: project.id, + project_path: project.full_path, + new_merge_request_path: project_new_merge_request_path(project) }) end @@ -522,7 +525,10 @@ can_push_code: 'true', can_push_to_branch: 'true', empty_repo: 'false', - blob_name: nil + blob_name: nil, + project_id: project.id, + project_path: project.full_path, + new_merge_request_path: project_new_merge_request_path(project) }) end end