From 7096da58d97d00fca778122d46b617e3aba1d0bc Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 26 Aug 2025 13:34:10 +0200 Subject: [PATCH 01/19] Extend edit_blob_app_data Pass project_id and project_path in order to migrate to REST API call for the update or create action in blob edit. --- app/assets/javascripts/blob_edit/blob_edit_header.js | 4 ++++ app/helpers/blob_helper.rb | 4 +++- spec/helpers/blob_helper_spec.rb | 8 ++++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/blob_edit/blob_edit_header.js b/app/assets/javascripts/blob_edit/blob_edit_header.js index 1a93872f927722..727f771fdda578 100644 --- a/app/assets/javascripts/blob_edit/blob_edit_header.js +++ b/app/assets/javascripts/blob_edit/blob_edit_header.js @@ -21,6 +21,8 @@ export default function initBlobEditHeader(editor) { blobName, branchAllowsCollaboration, lastCommitSha, + projectId, + projectPath, } = el.dataset; return new Vue({ @@ -34,6 +36,8 @@ export default function initBlobEditHeader(editor) { targetBranch, blobName, lastCommitSha, + projectId, + projectPath, emptyRepo: parseBoolean(emptyRepo), canPushCode: parseBoolean(canPushCode), canPushToBranch: parseBoolean(canPushToBranch), diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index bb379a08f0b241..547444eed56c4c 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -351,7 +351,9 @@ 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 } end diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index debb600198a527..8d6d76b472673c 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -508,7 +508,9 @@ 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 }) end @@ -522,7 +524,9 @@ 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 }) end end -- GitLab From ca81985f7cdbd9ec952e41d046cde58a4f824d4d Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 26 Aug 2025 13:41:39 +0200 Subject: [PATCH 02/19] Use rest API for single file edit --- .../javascripts/blob_edit/blob_edit_header.js | 2 + app/assets/javascripts/blob_edit/edit_blob.js | 4 + .../repository/pages/blob_edit_header.vue | 89 +++++++++++++++---- app/helpers/blob_helper.rb | 3 +- .../projects/find_file_controller_spec.rb | 2 +- .../repository/pages/blob_edit_header_spec.js | 60 +++++++++++-- 6 files changed, 131 insertions(+), 29 deletions(-) diff --git a/app/assets/javascripts/blob_edit/blob_edit_header.js b/app/assets/javascripts/blob_edit/blob_edit_header.js index 727f771fdda578..dc76c639ecbe22 100644 --- a/app/assets/javascripts/blob_edit/blob_edit_header.js +++ b/app/assets/javascripts/blob_edit/blob_edit_header.js @@ -23,6 +23,7 @@ export default function initBlobEditHeader(editor) { lastCommitSha, projectId, projectPath, + newMergeRequestPath, } = el.dataset; return new Vue({ @@ -38,6 +39,7 @@ export default function initBlobEditHeader(editor) { 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 ddca90579325c3..e9b98df2916bb5 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 34dc6cb6700a0d..940ac2d1e598b0 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -5,10 +5,12 @@ 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 getRefMixin from '../mixins/get_ref'; export default { + UPDATE_FILE_PATH: '/api/:version/projects/:id/repository/files/:file_path', components: { PageHeading, CommitChangesModal, @@ -28,12 +30,16 @@ export default { 'emptyRepo', 'branchAllowsCollaboration', 'lastCommitSha', + 'projectId', + 'projectPath', + 'newMergeRequestPath', ], data() { return { isCommitChangeModalOpen: false, fileContent: null, filePath: null, + originalFilePath: null, isLoading: false, error: null, }; @@ -75,15 +81,17 @@ 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) { if (!message) return; this.error = 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); @@ -98,30 +106,73 @@ export default { // `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); - return axios({ - method: this.isEditBlob ? 'put' : 'post', - url: this.updatePath, - data, - }) - .then(({ data: responseData }) => { - if (responseData.error) { - this.handleError(responseData.error); - return; - } + try { + let response; + if (this.isEditBlob) { + response = await this.editBlob(originalFormData); + } else { + response = await axios({ + method: 'post', + url: this.updatePath, + data: originalFormData, + }); + } + const { data: responseData } = response; + + if (responseData.error) { + this.handleError(responseData.error); + return; + } + + if (this.isEditBlob) { + if (originalFormData.create_merge_request) { + const mrUrl = mergeUrlParams( + { 'merge_request[source_branch]': responseData.branch }, + this.newMergeRequestPath, + ); + visitUrl(mrUrl); + } else { + visitUrl(this.getUpdatePath(responseData.branch, responseData.file_path)); + } + } else { if (responseData.filePath) { visitUrl(responseData.filePath); return; } - this.handleError(this.errorMessage); - }) - .catch(({ response }) => this.handleError(response?.data?.error)) - .finally(() => { - this.isLoading = false; - }); + } + } catch (error) { + this.handleError( + error.response?.data?.message || error.response?.data?.error || this.errorMessage, + ); + } 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, + start_branch: originalFormData.original_branch, + }; + + return axios.put(url, data); + }, + 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/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 547444eed56c4c..b7e0f568f5efc4 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -353,7 +353,8 @@ def edit_blob_app_data(project, id, blob, ref, action) branch_allows_collaboration: project.branch_allows_collaboration?(current_user, ref).to_s, last_commit_sha: @last_commit_sha, project_id: project.id, - project_path: project.full_path + project_path: project.full_path, + new_merge_request_path: project_new_merge_request_path(project) } end diff --git a/spec/controllers/projects/find_file_controller_spec.rb b/spec/controllers/projects/find_file_controller_spec.rb index 68810bae36879c..62bef9b0a0fedd 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/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js index cd950ff110a562..0bc3425ca7c392 100644 --- a/spec/frontend/repository/pages/blob_edit_header_spec.js +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -24,6 +24,7 @@ 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(), @@ -45,6 +46,9 @@ describe('BlobEditHeader', () => { emptyRepo: false, branchAllowsCollaboration: false, lastCommitSha: '782426692977b2cedb4452ee6501a404410f9b00', + projectId: 123, + projectPath: 'gitlab-org/gitlab', + newMergeRequestPath: 'merge_request/new/123', }, stubs: { PageHeading, @@ -58,6 +62,8 @@ describe('BlobEditHeader', () => { }; beforeEach(() => { + window.gon = { api_version: 'v4' }; + visitUrlSpy = jest.spyOn(urlUtility, 'visitUrl'); mock = new MockAdapter(axios); wrapper = createWrapper(); @@ -75,7 +81,12 @@ describe('BlobEditHeader', () => { const findCancelButton = () => wrapper.findByTestId('blob-edit-header-cancel-button'); 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(); }; @@ -116,22 +127,33 @@ describe('BlobEditHeader', () => { }); it('on submit, redirects to the updated file', async () => { + // First click the commit button to open the modal and set up the file content findCommitChangesButton().vm.$emit('click'); + await nextTick(); - mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); + mock.onPut().replyOnce(HTTP_STATUS_OK, { + branch: 'feature', + file_path: 'test.js', + }); 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.file).toBe(content); - expect(visitUrlSpy).toHaveBeenCalledWith('/update/path'); + expect(putData.content).toBe(content); + expect(visitUrlSpy).toHaveBeenCalledWith( + 'http://test.host/gitlab-org/gitlab/-/blob/feature/test.js', + ); }); describe('error handling', () => { const errorMessage = 'Custom error message'; it('shows error message in modal when response contains error', async () => { - mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { error: errorMessage }); + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + mock.onPut().replyOnce(HTTP_STATUS_OK, { error: errorMessage }); await submitForm(); expect(findCommitChangesModal().props('error')).toBe(errorMessage); @@ -139,21 +161,43 @@ describe('BlobEditHeader', () => { }); it('shows error message in modal when request fails', async () => { - mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { error: errorMessage }); + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + mock.onPut().replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { message: errorMessage }); await submitForm(); expect(findCommitChangesModal().props('error')).toBe(errorMessage); }); it('clears error on successful submission', async () => { - mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY); + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + mock.onPut().replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY); await submitForm(); - mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); + // 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', + ); }); }); }); -- GitLab From 56a18c696e322da0a9dee9646341aed66825d1b7 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Fri, 29 Aug 2025 17:28:58 +0200 Subject: [PATCH 03/19] Refactor hsandleSubmitForm --- .../repository/pages/blob_edit_header.vue | 66 +++++++++++-------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index 940ac2d1e598b0..bc98e8e694a03e 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -109,17 +109,7 @@ export default { const originalFormData = Object.fromEntries(formData); try { - let response; - if (this.isEditBlob) { - response = await this.editBlob(originalFormData); - } else { - response = await axios({ - method: 'post', - url: this.updatePath, - data: originalFormData, - }); - } - + const response = await this.performBlobOperation(originalFormData); const { data: responseData } = response; if (responseData.error) { @@ -127,23 +117,7 @@ export default { return; } - if (this.isEditBlob) { - if (originalFormData.create_merge_request) { - const mrUrl = mergeUrlParams( - { 'merge_request[source_branch]': responseData.branch }, - this.newMergeRequestPath, - ); - visitUrl(mrUrl); - } else { - visitUrl(this.getUpdatePath(responseData.branch, responseData.file_path)); - } - } else { - if (responseData.filePath) { - visitUrl(responseData.filePath); - return; - } - this.handleError(this.errorMessage); - } + this.handleSuccessfulResponse(responseData, originalFormData); } catch (error) { this.handleError( error.response?.data?.message || error.response?.data?.error || this.errorMessage, @@ -152,6 +126,42 @@ export default { this.isLoading = false; } }, + async performBlobOperation(formData) { + if (this.isEditBlob) { + return this.editBlob(formData); + } + + return axios({ + method: 'post', + url: this.updatePath, + data: formData, + }); + }, + handleSuccessfulResponse(responseData, formData) { + if (this.isEditBlob) { + this.handleEditBlobSuccess(responseData, formData); + } else { + this.handleCreateFileSuccess(responseData); + } + }, + handleEditBlobSuccess(responseData, formData) { + if (formData.create_merge_request) { + const mrUrl = mergeUrlParams( + { 'merge_request[source_branch]': responseData.branch }, + this.newMergeRequestPath, + ); + visitUrl(mrUrl); + } else { + visitUrl(this.getUpdatePath(responseData.branch, responseData.file_path)); + } + }, + handleCreateFileSuccess(responseData) { + if (responseData.filePath) { + visitUrl(responseData.filePath); + return; + } + this.handleError(this.errorMessage); + }, editBlob(originalFormData) { const url = buildApiUrl(this.$options.UPDATE_FILE_PATH) .replace(':id', this.projectId) -- GitLab From 91ac19714076699c01246619b7363ab83bff3c28 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Thu, 4 Sep 2025 11:08:48 +0200 Subject: [PATCH 04/19] Add missing data to tests --- spec/helpers/blob_helper_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index 8d6d76b472673c..cc3dc33d5c1afc 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -510,7 +510,8 @@ branch_allows_collaboration: 'false', last_commit_sha: '782426692977b2cedb4452ee6501a404410f9b00', project_id: project.id, - project_path: project.full_path + project_path: project.full_path, + new_merge_request_path: project_new_merge_request_path(project) }) end @@ -526,7 +527,8 @@ empty_repo: 'false', blob_name: nil, project_id: project.id, - project_path: project.full_path + project_path: project.full_path, + new_merge_request_path: project_new_merge_request_path(project) }) end end -- GitLab From 12a7d42552e200f82c5c041877d3a412b894376c Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Thu, 4 Sep 2025 13:21:28 +0200 Subject: [PATCH 05/19] Fix issues with creating new MR --- .../repository/pages/blob_edit_header.vue | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index bc98e8e694a03e..07d3b7ce05ab33 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -9,6 +9,8 @@ import { getParameterByName, visitUrl, joinPaths, mergeUrlParams } from '~/lib/u import { buildApiUrl } from '~/api/api_utils'; 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: { @@ -145,9 +147,9 @@ export default { } }, handleEditBlobSuccess(responseData, formData) { - if (formData.create_merge_request) { + if (formData.create_merge_request && this.originalBranch !== responseData.branch) { const mrUrl = mergeUrlParams( - { 'merge_request[source_branch]': responseData.branch }, + { [MR_SOURCE_BRANCH]: responseData.branch }, this.newMergeRequestPath, ); visitUrl(mrUrl); @@ -174,9 +176,16 @@ export default { file_path: originalFormData.file_path, id: this.projectId, last_commit_id: originalFormData.last_commit_sha, - start_branch: originalFormData.original_branch, }; + // 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); }, getUpdatePath(branch, filePath) { -- GitLab From 7e362650bba979e701562201aa62d6103595ed60 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 9 Sep 2025 12:18:17 +0200 Subject: [PATCH 06/19] Make sure we show custom error messages --- .../javascripts/repository/pages/blob_edit_header.vue | 8 +++++++- locale/gitlab.pot | 9 --------- spec/features/projects/files/editing_a_file_spec.rb | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index 07d3b7ce05ab33..bf7660b8baf5c4 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -90,6 +90,10 @@ export default { if (!message) return; this.error = message; }, + handleGenericErrorMessage(message) { + // eslint-disable-next-line @gitlab/require-i18n-strings + return message === '403 Forbidden' ? this.errorMessage : message; + }, async handleFormSubmit(formData) { this.error = null; this.isLoading = true; @@ -122,7 +126,9 @@ export default { this.handleSuccessfulResponse(responseData, originalFormData); } catch (error) { this.handleError( - error.response?.data?.message || error.response?.data?.error || this.errorMessage, + this.handleGenericErrorMessage(error.response?.data?.message) || + error.response?.data?.error || + this.errorMessage, ); } finally { this.isLoading = false; diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9e807eb57e5c1b..ef8e2ef10baf0f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26808,9 +26808,6 @@ msgstr "" msgid "Error: %{error}" msgstr "" -msgid "Error: Can't edit this file. The fork and upstream project have diverged. %{link_start}Edit the file on the fork %{icon}%{link_end}, and create a merge request." -msgstr "" - msgid "Error: Couldn't load some or all of the changes." msgstr "" @@ -45590,9 +45587,6 @@ msgstr "" msgid "Opens in a new window" msgstr "" -msgid "Opens new window" -msgstr "" - msgid "Operation not allowed" msgstr "" @@ -62551,9 +62545,6 @@ msgstr "" msgid "Someone edited the %{issuableType} at the same time you did. Review %{linkStart}the %{issuableType}%{linkEnd} and make sure you don't unintentionally overwrite their changes." msgstr "" -msgid "Someone edited the file the same time you did. Please check out %{link_start}the file %{icon}%{link_end} and make sure your changes will not unintentionally remove theirs." -msgstr "" - msgid "Someone edited this %{issueType} at the same time you did. The description has been updated and you will need to make your changes again." msgstr "" diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb index 58535c82453177..04726ade445eb4 100644 --- a/spec/features/projects/files/editing_a_file_spec.rb +++ b/spec/features/projects/files/editing_a_file_spec.rb @@ -32,7 +32,7 @@ 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 end -- GitLab From 40085bf6f44775a80c5889b926c3792308ab6d19 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 9 Sep 2025 12:34:43 +0200 Subject: [PATCH 07/19] Remove old notification about editing conflict --- app/views/projects/blob/edit.html.haml | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index ae763407742b17..6f9e0987be440a 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -4,22 +4,6 @@ - webpack_preload_asset_tag('monaco') - add_page_specific_style 'page_bundles/editor' -- if @conflict - = render Pajamas::AlertComponent.new(alert_options: { class: 'gl-mb-5 gl-mt-5' }, - variant: :danger, - dismissible: false) do |c| - - c.with_body do - - blob_link_start = ''.html_safe - - link_end = ''.html_safe - - external_link_icon = content_tag 'span', { aria: { label: _('Opens new window') }} do - - sprite_icon('external-link', css_class: 'gl-icon').html_safe - - if commit_to_fork - = _("Error: Can't edit this file. The fork and upstream project have diverged. %{link_start}Edit the file on the fork %{icon}%{link_end}, and create a merge request.").html_safe % {link_start: blob_link_start % { url: project_blob_path(@project_to_commit_into, @id) } , link_end: link_end, icon: external_link_icon } - - else - - blob_url = project_blob_path(@project, @id) - = _('Someone edited the file the same time you did. Please check out %{link_start}the file %{icon}%{link_end} and make sure your changes will not unintentionally remove theirs.').html_safe % { link_start: blob_link_start % { url: blob_url }, link_end: link_end , icon: external_link_icon } - - .js-blob-edit-header{ data: edit_blob_app_data(@project, @id, @blob, @ref, 'update') } = render ::Layouts::PageHeadingComponent.new(_('Edit file'), options: { class: 'gl-mb-3' }) do |c| - c.with_actions do -- GitLab From 4b65edf6786f5c31c76f6279343aff0cdb94c3ba Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 9 Sep 2025 12:45:00 +0200 Subject: [PATCH 08/19] Refactor into separate edit and create methods --- .../repository/pages/blob_edit_header.vue | 133 ++++++++++++------ app/controllers/projects/blob_controller.rb | 1 + .../feature_flags/wip/blob_edit_refactor.yml | 10 ++ .../projects/files/editing_a_file_spec.rb | 20 +++ .../repository/pages/blob_edit_header_spec.js | 67 ++++++++- 5 files changed, 185 insertions(+), 46 deletions(-) create mode 100644 config/feature_flags/wip/blob_edit_refactor.yml diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index bf7660b8baf5c4..fbbe1ff2815dd4 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -7,6 +7,7 @@ import PageHeading from '~/vue_shared/components/page_heading.vue'; import CommitChangesModal from '~/repository/components/commit_changes_modal.vue'; 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]'; @@ -18,7 +19,7 @@ export default { CommitChangesModal, GlButton, }, - mixins: [getRefMixin], + mixins: [getRefMixin, glFeatureFlagMixin()], inject: [ 'action', 'editor', @@ -86,6 +87,19 @@ export default { this.originalFilePath = this.editor.getOriginalFilePath(); this.$refs[this.updateModalId].show(); }, + prepareFormData(formData) { + // Prepare form data for both controller and API approaches + // Ensure we have the file content from the editor + const fileContent = this.fileContent || this.editor.getFileContent(); + const filePath = this.filePath || this.editor.filepathFormMediator?.$filenameInput?.val(); + + formData.append('file', fileContent); + formData.append('file_path', filePath); + formData.append('last_commit_sha', this.lastCommitSha); + formData.append('from_merge_request_iid', this.fromMergeRequestIid); + + return Object.fromEntries(formData); + }, handleError(message) { if (!message) return; this.error = message; @@ -99,23 +113,21 @@ export default { 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) { + this.error = null; + this.isLoading = true; - // 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 originalFormData = Object.fromEntries(formData); + const originalFormData = this.prepareFormData(formData); try { - const response = await this.performBlobOperation(originalFormData); + const response = this.glFeatures.blobEditRefactor + ? await this.editBlob(originalFormData) + : await this.editBlobWithController(originalFormData); const { data: responseData } = response; if (responseData.error) { @@ -123,7 +135,11 @@ export default { return; } - this.handleSuccessfulResponse(responseData, originalFormData); + if (this.glFeatures.blobEditRefactor) { + this.handleEditBlobSuccess(responseData, originalFormData); + } else { + this.handleEditBlobWithControllerSuccess(responseData); + } } catch (error) { this.handleError( this.handleGenericErrorMessage(error.response?.data?.message) || @@ -134,41 +150,36 @@ export default { this.isLoading = false; } }, - async performBlobOperation(formData) { - if (this.isEditBlob) { - return this.editBlob(formData); - } + async handleCreateFormSubmit(formData) { + formData.append('file_name', this.filePath); + formData.append('content', this.fileContent); + + 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; + } - return axios({ - method: 'post', - url: this.updatePath, - data: formData, - }); - }, - handleSuccessfulResponse(responseData, formData) { - if (this.isEditBlob) { - this.handleEditBlobSuccess(responseData, formData); - } else { this.handleCreateFileSuccess(responseData); - } - }, - handleEditBlobSuccess(responseData, formData) { - if (formData.create_merge_request && this.originalBranch !== responseData.branch) { - const mrUrl = mergeUrlParams( - { [MR_SOURCE_BRANCH]: responseData.branch }, - this.newMergeRequestPath, + } catch (error) { + this.handleError( + this.handleGenericErrorMessage(error.response?.data?.message) || + error.response?.data?.error || + this.errorMessage, ); - visitUrl(mrUrl); - } else { - visitUrl(this.getUpdatePath(responseData.branch, responseData.file_path)); - } - }, - handleCreateFileSuccess(responseData) { - if (responseData.filePath) { - visitUrl(responseData.filePath); - return; + } finally { + this.isLoading = false; } - this.handleError(this.errorMessage); }, editBlob(originalFormData) { const url = buildApiUrl(this.$options.UPDATE_FILE_PATH) @@ -194,6 +205,38 @@ export default { 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 mrUrl = mergeUrlParams( + { [MR_SOURCE_BRANCH]: responseData.branch }, + this.newMergeRequestPath, + ); + visitUrl(mrUrl); + } else { + visitUrl(this.getUpdatePath(responseData.branch, responseData.file_path)); + } + }, + handleEditBlobWithControllerSuccess(responseData) { + if (responseData.filePath) { + visitUrl(responseData.filePath); + return; + } + this.handleError(this.errorMessage); + }, + handleCreateFileSuccess(responseData) { + if (responseData.filePath) { + visitUrl(responseData.filePath); + return; + } + this.handleError(this.errorMessage); + }, getUpdatePath(branch, filePath) { const url = new URL(window.location.href); url.pathname = joinPaths(this.projectPath, '-/blob', branch, filePath); diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 11aa078de6915f..a541e231c67f38 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/config/feature_flags/wip/blob_edit_refactor.yml b/config/feature_flags/wip/blob_edit_refactor.yml new file mode 100644 index 00000000000000..c1476f11a74548 --- /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: +rollout_issue_url: +milestone: '18.4' +group: group::source code +type: wip +default_enabled: false diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb index 04726ade445eb4..f98867c91e6c3d 100644 --- a/spec/features/projects/files/editing_a_file_spec.rb +++ b/spec/features/projects/files/editing_a_file_spec.rb @@ -34,6 +34,26 @@ 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, 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 context 'when the user does not have write access' do diff --git a/spec/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js index 0bc3425ca7c392..3c6cf33f7ad62a 100644 --- a/spec/frontend/repository/pages/blob_edit_header_spec.js +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -9,6 +9,7 @@ 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', () => { @@ -31,7 +32,7 @@ describe('BlobEditHeader', () => { }, }; - const createWrapper = ({ action = 'update' } = {}) => { + const createWrapper = ({ action = 'update', glFeatures = {} } = {}) => { return shallowMountExtended(BlobEditHeader, { provide: { action, @@ -49,7 +50,9 @@ describe('BlobEditHeader', () => { projectId: 123, projectPath: 'gitlab-org/gitlab', newMergeRequestPath: 'merge_request/new/123', + glFeatures: { blobEditRefactor: true, ...glFeatures }, }, + mixins: [glFeatureFlagMixin()], stubs: { PageHeading, CommitChangesModal: stubComponent(CommitChangesModal, { @@ -264,4 +267,66 @@ describe('BlobEditHeader', () => { expect(mockEditor.filepathFormMediator.toggleValidationError).toHaveBeenCalledWith(true); }); }); + + describe('when blobEditRefactor is disabled', () => { + beforeEach(() => { + wrapper = createWrapper({ glFeatures: { blobEditRefactor: false } }); + }); + + it('on submit, redirects to the updated file using controller', async () => { + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + 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('/update'); + const putData = JSON.parse(mock.history.put[0].data); + expect(putData.file).toBe(content); + expect(visitUrlSpy).toHaveBeenCalledWith('/update/path'); + }); + + describe('error handling', () => { + const errorMessage = 'Controller error message'; + + it('shows error message in modal when response contains error', async () => { + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + mock.onPut('/update').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 () => { + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { error: errorMessage }); + await submitForm(); + + expect(findCommitChangesModal().props('error')).toBe(errorMessage); + }); + + it('clears error on successful submission', async () => { + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + 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(); + }); + }); + }); }); -- GitLab From b6a2e4e45c732f4afcb758c61c72a4ae398dc3a4 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Thu, 18 Sep 2025 14:33:32 +0200 Subject: [PATCH 09/19] Use without the flag for flows still under development --- spec/features/projects/blobs/edit_spec.rb | 2 ++ spec/features/projects/files/user_edits_files_spec.rb | 3 +++ 2 files changed, 5 insertions(+) diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index c0be951c343db9..38b8aa46aaf519 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/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb index 12755b244dd0bd..5735be77460939 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 -- GitLab From 6d45ff2596d812f19a763c2dcb5833df5cbd29b3 Mon Sep 17 00:00:00 2001 From: Paulina Sedlak-Jakubowska Date: Thu, 18 Sep 2025 15:39:00 +0200 Subject: [PATCH 10/19] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: DANGER_GITLAB_API_TOKEN --- config/feature_flags/wip/blob_edit_refactor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/wip/blob_edit_refactor.yml b/config/feature_flags/wip/blob_edit_refactor.yml index c1476f11a74548..2a890e3003ae07 100644 --- a/config/feature_flags/wip/blob_edit_refactor.yml +++ b/config/feature_flags/wip/blob_edit_refactor.yml @@ -2,7 +2,7 @@ name: blob_edit_refactor description: feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/509968 -introduced_by_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/205529 rollout_issue_url: milestone: '18.4' group: group::source code -- GitLab From 13648116e7439b1035b7af551e998427093a3790 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Mon, 22 Sep 2025 13:14:03 +0200 Subject: [PATCH 11/19] Address reviewer feedback --- .../repository/pages/blob_edit_header.vue | 67 ++++++++----------- .../feature_flags/wip/blob_edit_refactor.yml | 2 +- .../repository/pages/blob_edit_header_spec.js | 64 +++++++++--------- 3 files changed, 61 insertions(+), 72 deletions(-) diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index fbbe1ff2815dd4..efe27c6a534d89 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -87,26 +87,25 @@ export default { this.originalFilePath = this.editor.getOriginalFilePath(); this.$refs[this.updateModalId].show(); }, - prepareFormData(formData) { - // Prepare form data for both controller and API approaches - // Ensure we have the file content from the editor - const fileContent = this.fileContent || this.editor.getFileContent(); - const filePath = this.filePath || this.editor.filepathFormMediator?.$filenameInput?.val(); + prepareApiFormData(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); - formData.append('file', fileContent); - formData.append('file_path', filePath); + return Object.fromEntries(formData); + }, + prepareControllerFormData(formData) { + formData.append('file', this.editor.getFileContent()); + formData.append('file_path', this.editor.filepathFormMediator?.$filenameInput?.val()); formData.append('last_commit_sha', this.lastCommitSha); formData.append('from_merge_request_iid', this.fromMergeRequestIid); return Object.fromEntries(formData); }, - handleError(message) { + handleError(message, errorCode = null) { if (!message) return; - this.error = message; - }, - handleGenericErrorMessage(message) { - // eslint-disable-next-line @gitlab/require-i18n-strings - return message === '403 Forbidden' ? this.errorMessage : message; + this.error = errorCode === 403 ? this.errorMessage : message; }, async handleFormSubmit(formData) { this.error = null; @@ -119,10 +118,9 @@ export default { } }, async handleEditFormSubmit(formData) { - this.error = null; - this.isLoading = true; - - const originalFormData = this.prepareFormData(formData); + const originalFormData = this.glFeatures.blobEditRefactor + ? this.prepareApiFormData(formData) + : this.prepareControllerFormData(formData); try { const response = this.glFeatures.blobEditRefactor @@ -138,14 +136,12 @@ export default { if (this.glFeatures.blobEditRefactor) { this.handleEditBlobSuccess(responseData, originalFormData); } else { - this.handleEditBlobWithControllerSuccess(responseData); + this.handleControllerSuccess(responseData); } } catch (error) { - this.handleError( - this.handleGenericErrorMessage(error.response?.data?.message) || - error.response?.data?.error || - this.errorMessage, - ); + const errorMessage = + error.response?.data?.message || error.response?.data?.error || this.errorMessage; + this.handleError(errorMessage, error.response?.status); } finally { this.isLoading = false; } @@ -154,6 +150,10 @@ export default { 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 originalFormData = Object.fromEntries(formData); try { @@ -170,13 +170,11 @@ export default { return; } - this.handleCreateFileSuccess(responseData); + this.handleControllerSuccess(responseData); } catch (error) { - this.handleError( - this.handleGenericErrorMessage(error.response?.data?.message) || - error.response?.data?.error || - this.errorMessage, - ); + const errorMessage = + error.response?.data?.message || error.response?.data?.error || this.errorMessage; + this.handleError(errorMessage, error.response?.status); } finally { this.isLoading = false; } @@ -223,19 +221,10 @@ export default { visitUrl(this.getUpdatePath(responseData.branch, responseData.file_path)); } }, - handleEditBlobWithControllerSuccess(responseData) { - if (responseData.filePath) { - visitUrl(responseData.filePath); - return; - } - this.handleError(this.errorMessage); - }, - handleCreateFileSuccess(responseData) { + handleControllerSuccess(responseData) { if (responseData.filePath) { visitUrl(responseData.filePath); - return; } - this.handleError(this.errorMessage); }, getUpdatePath(branch, filePath) { const url = new URL(window.location.href); diff --git a/config/feature_flags/wip/blob_edit_refactor.yml b/config/feature_flags/wip/blob_edit_refactor.yml index 2a890e3003ae07..26ff0fbe525fd4 100644 --- a/config/feature_flags/wip/blob_edit_refactor.yml +++ b/config/feature_flags/wip/blob_edit_refactor.yml @@ -4,7 +4,7 @@ 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.4' +milestone: '18.5' group: group::source code type: wip default_enabled: false diff --git a/spec/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js index 3c6cf33f7ad62a..c6483023aa85d6 100644 --- a/spec/frontend/repository/pages/blob_edit_header_spec.js +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -3,7 +3,11 @@ 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'; @@ -83,6 +87,11 @@ 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 () => { const formData = new FormData(); formData.append('commit_message', 'Test commit'); @@ -103,9 +112,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', @@ -131,9 +139,7 @@ describe('BlobEditHeader', () => { it('on submit, redirects to the updated file', async () => { // First click the commit button to open the modal and set up the file content - findCommitChangesButton().vm.$emit('click'); - await nextTick(); - + clickCommitChangesButton(); mock.onPut().replyOnce(HTTP_STATUS_OK, { branch: 'feature', file_path: 'test.js', @@ -152,10 +158,11 @@ describe('BlobEditHeader', () => { describe('error handling', () => { const errorMessage = 'Custom error message'; - it('shows error message in modal when response contains error', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); + beforeEach(() => { + clickCommitChangesButton(); + }); + it('shows error message in modal when response contains error', async () => { mock.onPut().replyOnce(HTTP_STATUS_OK, { error: errorMessage }); await submitForm(); @@ -164,19 +171,21 @@ describe('BlobEditHeader', () => { }); it('shows error message in modal when request fails', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); - mock.onPut().replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { message: errorMessage }); await submitForm(); expect(findCommitChangesModal().props('error')).toBe(errorMessage); }); - it('clears error on successful submission', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); + 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(); @@ -218,9 +227,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', @@ -245,8 +253,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(); @@ -261,8 +268,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); }); @@ -274,9 +280,7 @@ describe('BlobEditHeader', () => { }); it('on submit, redirects to the updated file using controller', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); - + clickCommitChangesButton(); mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); await submitForm(); @@ -291,9 +295,7 @@ describe('BlobEditHeader', () => { const errorMessage = 'Controller error message'; it('shows error message in modal when response contains error', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); - + clickCommitChangesButton(); mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { error: errorMessage }); await submitForm(); @@ -301,9 +303,7 @@ describe('BlobEditHeader', () => { expect(visitUrlSpy).not.toHaveBeenCalled(); }); it('shows error message in modal when request fails', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); - + clickCommitChangesButton(); mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { error: errorMessage }); await submitForm(); -- GitLab From 77c5c4fadf78bdfaff8dc6d5c8a4c26dd21e8614 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Mon, 22 Sep 2025 13:14:31 +0200 Subject: [PATCH 12/19] Revert "Remove old notification about editing conflict" This reverts commit be1f9bb57e7743007e81d21db88da476ec54ab0b. --- app/views/projects/blob/edit.html.haml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index 6f9e0987be440a..ae763407742b17 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -4,6 +4,22 @@ - webpack_preload_asset_tag('monaco') - add_page_specific_style 'page_bundles/editor' +- if @conflict + = render Pajamas::AlertComponent.new(alert_options: { class: 'gl-mb-5 gl-mt-5' }, + variant: :danger, + dismissible: false) do |c| + - c.with_body do + - blob_link_start = ''.html_safe + - link_end = ''.html_safe + - external_link_icon = content_tag 'span', { aria: { label: _('Opens new window') }} do + - sprite_icon('external-link', css_class: 'gl-icon').html_safe + - if commit_to_fork + = _("Error: Can't edit this file. The fork and upstream project have diverged. %{link_start}Edit the file on the fork %{icon}%{link_end}, and create a merge request.").html_safe % {link_start: blob_link_start % { url: project_blob_path(@project_to_commit_into, @id) } , link_end: link_end, icon: external_link_icon } + - else + - blob_url = project_blob_path(@project, @id) + = _('Someone edited the file the same time you did. Please check out %{link_start}the file %{icon}%{link_end} and make sure your changes will not unintentionally remove theirs.').html_safe % { link_start: blob_link_start % { url: blob_url }, link_end: link_end , icon: external_link_icon } + + .js-blob-edit-header{ data: edit_blob_app_data(@project, @id, @blob, @ref, 'update') } = render ::Layouts::PageHeadingComponent.new(_('Edit file'), options: { class: 'gl-mb-3' }) do |c| - c.with_actions do -- GitLab From 06127aae6ffe52b46b529c93dbf7ad147564be04 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Mon, 22 Sep 2025 17:06:41 +0200 Subject: [PATCH 13/19] Regenerate .pot file --- locale/gitlab.pot | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ef8e2ef10baf0f..9e807eb57e5c1b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26808,6 +26808,9 @@ msgstr "" msgid "Error: %{error}" msgstr "" +msgid "Error: Can't edit this file. The fork and upstream project have diverged. %{link_start}Edit the file on the fork %{icon}%{link_end}, and create a merge request." +msgstr "" + msgid "Error: Couldn't load some or all of the changes." msgstr "" @@ -45587,6 +45590,9 @@ msgstr "" msgid "Opens in a new window" msgstr "" +msgid "Opens new window" +msgstr "" + msgid "Operation not allowed" msgstr "" @@ -62545,6 +62551,9 @@ msgstr "" msgid "Someone edited the %{issuableType} at the same time you did. Review %{linkStart}the %{issuableType}%{linkEnd} and make sure you don't unintentionally overwrite their changes." msgstr "" +msgid "Someone edited the file the same time you did. Please check out %{link_start}the file %{icon}%{link_end} and make sure your changes will not unintentionally remove theirs." +msgstr "" + msgid "Someone edited this %{issueType} at the same time you did. The description has been updated and you will need to make your changes again." msgstr "" -- GitLab From 2c7ce6ae7219899ecd949f907e195df6c2373003 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Wed, 24 Sep 2025 18:09:09 +0200 Subject: [PATCH 14/19] Refactor formData and separate test cases --- .../repository/pages/blob_edit_header.vue | 10 +- .../repository/pages/blob_edit_header_spec.js | 282 +++++++++--------- 2 files changed, 149 insertions(+), 143 deletions(-) diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index efe27c6a534d89..b42e98422beb34 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -87,7 +87,7 @@ export default { this.originalFilePath = this.editor.getOriginalFilePath(); this.$refs[this.updateModalId].show(); }, - prepareApiFormData(formData) { + prepareFormData(formData) { formData.append('file', this.fileContent); formData.append('file_path', this.filePath); formData.append('last_commit_sha', this.lastCommitSha); @@ -96,8 +96,8 @@ export default { return Object.fromEntries(formData); }, prepareControllerFormData(formData) { - formData.append('file', this.editor.getFileContent()); - formData.append('file_path', this.editor.filepathFormMediator?.$filenameInput?.val()); + 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); @@ -118,9 +118,7 @@ export default { } }, async handleEditFormSubmit(formData) { - const originalFormData = this.glFeatures.blobEditRefactor - ? this.prepareApiFormData(formData) - : this.prepareControllerFormData(formData); + const originalFormData = this.prepareFormData(formData); try { const response = this.glFeatures.blobEditRefactor diff --git a/spec/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js index c6483023aa85d6..3139b65cef2b93 100644 --- a/spec/frontend/repository/pages/blob_edit_header_spec.js +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -104,112 +104,176 @@ describe('BlobEditHeader', () => { }; 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', () => { - 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('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 () => { - // 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', + 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(); + + 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', + ); }); - 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', - ); - }); + 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(); + }); - describe('error handling', () => { - const errorMessage = 'Custom error message'; + 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('when blobEditRefactor is disabled', () => { beforeEach(() => { + wrapper = createWrapper({ glFeatures: { blobEditRefactor: false } }); clickCommitChangesButton(); }); - it('shows error message in modal when response contains error', async () => { - mock.onPut().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().replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { message: 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('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(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(); - expect(findCommitChangesModal().props('error')).toBe('An error occurred editing the blob'); - expect(visitUrlSpy).not.toHaveBeenCalled(); - }); + expect(findCommitChangesModal().props('error')).toBe(errorMessage); + }); - it('clears error on successful submission', async () => { - mock.onPut().replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY); - await submitForm(); + it('clears error on successful submission', async () => { + findCommitChangesButton().vm.$emit('click'); + await nextTick(); - // Verify error is set first - expect(findCommitChangesModal().props('error')).toBe('An error occurred editing the blob'); + mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY); + await submitForm(); - // Mock visitUrl to prevent actual navigation - visitUrlSpy.mockImplementation(() => {}); + expect(findCommitChangesModal().props('error')).toBe( + 'An error occurred editing the blob', + ); - mock.onPut().replyOnce(HTTP_STATUS_OK, { - branch: 'feature', - file_path: 'test.js', - }); - jest.spyOn(console, 'error').mockImplementation(() => {}); + visitUrlSpy.mockImplementation(() => {}); + mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); - // Submit the form again - await submitForm(); - await nextTick(); + 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', - ); + expect(findCommitChangesModal().props('error')).toBeNull(); + }); }); }); }); @@ -273,60 +337,4 @@ describe('BlobEditHeader', () => { expect(mockEditor.filepathFormMediator.toggleValidationError).toHaveBeenCalledWith(true); }); }); - - describe('when blobEditRefactor is disabled', () => { - beforeEach(() => { - wrapper = createWrapper({ glFeatures: { blobEditRefactor: false } }); - }); - - it('on submit, redirects to the updated file using controller', async () => { - clickCommitChangesButton(); - 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('/update'); - const putData = JSON.parse(mock.history.put[0].data); - expect(putData.file).toBe(content); - expect(visitUrlSpy).toHaveBeenCalledWith('/update/path'); - }); - - describe('error handling', () => { - const errorMessage = 'Controller error message'; - - it('shows error message in modal when response contains error', async () => { - clickCommitChangesButton(); - mock.onPut('/update').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 () => { - clickCommitChangesButton(); - mock.onPut('/update').replyOnce(HTTP_STATUS_UNPROCESSABLE_ENTITY, { error: errorMessage }); - await submitForm(); - - expect(findCommitChangesModal().props('error')).toBe(errorMessage); - }); - - it('clears error on successful submission', async () => { - findCommitChangesButton().vm.$emit('click'); - await nextTick(); - - 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(); - }); - }); - }); }); -- GitLab From 650dfa7d70f38942a70777ba19b5192d1f777793 Mon Sep 17 00:00:00 2001 From: Paulina Sedlak-Jakubowska Date: Thu, 25 Sep 2025 11:37:23 +0200 Subject: [PATCH 15/19] Apply 1 suggestion(s) to 1 file(s) --- app/assets/javascripts/repository/pages/blob_edit_header.vue | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index b42e98422beb34..c2a5adb58a68e4 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -105,6 +105,8 @@ export default { }, 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; }, async handleFormSubmit(formData) { -- GitLab From 946353f69cae665b38ce2145c8f337b7b24fc02c Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Thu, 25 Sep 2025 13:38:43 +0200 Subject: [PATCH 16/19] Stub ff for fork workflow --- spec/features/merge_request/maintainer_edits_fork_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index f50af8dace9da4..92969a89c067a2 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) -- GitLab From 1a6434521d077d926ccd8ca0acf881f3a6523b71 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Thu, 25 Sep 2025 14:09:19 +0200 Subject: [PATCH 17/19] Use different commit for the second test case --- .../features/projects/files/editing_a_file_spec.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb index f98867c91e6c3d..798155aa1d248a 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 @@ -43,8 +54,7 @@ end it 'file has been updated since the user opened the edit page' do - Files::UpdateService.new(project, user, commit_params).execute - + Files::UpdateService.new(project, user, second_commit_params).execute click_button 'Commit changes' within_testid('commit-change-modal') do -- GitLab From afe44d02224e4ef36deed7bf5f751f25cabf6a6c Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Thu, 25 Sep 2025 16:12:26 +0200 Subject: [PATCH 18/19] Handle scenario when no filePath is returned --- .../javascripts/repository/pages/blob_edit_header.vue | 2 ++ .../frontend/repository/pages/blob_edit_header_spec.js | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index c2a5adb58a68e4..6ac28c7d339581 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -224,6 +224,8 @@ export default { handleControllerSuccess(responseData) { if (responseData.filePath) { visitUrl(responseData.filePath); + } else { + this.handleError(this.errorMessage); } }, getUpdatePath(branch, filePath) { diff --git a/spec/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js index 3139b65cef2b93..c2f8b1a5d8537d 100644 --- a/spec/frontend/repository/pages/blob_edit_header_spec.js +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -255,6 +255,16 @@ describe('BlobEditHeader', () => { 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(); -- GitLab From c7e12c8c3f83b02eeb447e7bf341b457cbd1a9de Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Mon, 29 Sep 2025 09:55:24 +0200 Subject: [PATCH 19/19] Skip E2E tests in gdk-instance-ff-inverse pipeline --- .../3_create/source_editor/source_editor_toolbar_spec.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 0aab3aeda2953a..858e063309bee5 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') -- GitLab