diff --git a/app/assets/javascripts/blob_edit/blob_edit_header.js b/app/assets/javascripts/blob_edit/blob_edit_header.js index 1a93872f927722d57408b154125c6d966e92354b..76aa3c32eb9d1351f035c9315c0fec13369f919d 100644 --- a/app/assets/javascripts/blob_edit/blob_edit_header.js +++ b/app/assets/javascripts/blob_edit/blob_edit_header.js @@ -21,6 +21,10 @@ export default function initBlobEditHeader(editor) { blobName, branchAllowsCollaboration, lastCommitSha, + projectId, + projectPath, + newMergeRequestPath, + ref, } = el.dataset; return new Vue({ @@ -34,6 +38,10 @@ export default function initBlobEditHeader(editor) { targetBranch, blobName, lastCommitSha, + projectId, + projectPath, + newMergeRequestPath, + ref, 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/components/blob_content_viewer.vue b/app/assets/javascripts/repository/components/blob_content_viewer.vue index 6cde8237a05d9da2f651ca3ba44f9be5f51a6ad9..896991dcce7cb9c08f1f9cf634b6a3765a69d39f 100644 --- a/app/assets/javascripts/repository/components/blob_content_viewer.vue +++ b/app/assets/javascripts/repository/components/blob_content_viewer.vue @@ -8,7 +8,7 @@ import BlobContent from '~/blob/components/blob_content.vue'; import BlobHeader from 'ee_else_ce/blob/components/blob_header.vue'; import BlameHeader from '~/blob/components/blame_header.vue'; import { SIMPLE_BLOB_VIEWER, RICH_BLOB_VIEWER, BLAME_VIEWER } from '~/blob/components/constants'; -import { createAlert } from '~/alert'; +import { createAlert, VARIANT_SUCCESS } from '~/alert'; import axios from '~/lib/utils/axios_utils'; import { isLoggedIn, handleLocationHash } from '~/lib/utils/common_utils'; import { __ } from '~/locale'; @@ -285,6 +285,21 @@ export default { else this.setShowBlame(false); // Always hide blame panel by default }, }, + mounted() { + const urlParams = new URLSearchParams(window.location.search); + if (urlParams.get('success') === 'committed') { + const currentPath = window.location.pathname; + const changesLink = { href: currentPath }; // should match changes_link = ActionController::Base.helpers.link_to _('changes'), success_path, class: 'gl-link' + + createAlert({ + message: __( + 'Your %{changesLinkStart}changes%{changesLinkEnd} have been committed successfully.', + ), + variant: VARIANT_SUCCESS, + messageLinks: { changesLink }, + }); + } + }, methods: { onError() { this.useFallback = true; diff --git a/app/assets/javascripts/repository/constants.js b/app/assets/javascripts/repository/constants.js index 5fa99fa67e9e7c5da139364f0fa5b7997d7697f4..6f0ab0cc2f000c9464c88cf1fc2ce8ac17094a86 100644 --- a/app/assets/javascripts/repository/constants.js +++ b/app/assets/javascripts/repository/constants.js @@ -17,6 +17,8 @@ export const I18N_COMMIT_DATA_FETCH_ERROR = __('An error occurred while fetching export const PDF_MAX_FILE_SIZE = 10000000; // 10 MB export const PDF_MAX_PAGE_LIMIT = 50; +export const MAX_EDIT_SIZE = 10485760; // 10MB in bytes (same as backend) + export const ROW_APPEAR_DELAY = 150; export const DEFAULT_BLOB_INFO = { diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index 34dc6cb6700a0d24805b15cfdd6729c0c1817940..b2abcac2725405eaf220a04e6694d4b6b60c1580 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -5,10 +5,19 @@ 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 * as Sentry from '~/sentry/sentry_browser_wrapper'; +import { MAX_EDIT_SIZE } from '~/repository/constants'; +import blobSizeValidationQuery from '~/repository/queries/blob_size_validation.query.graphql'; +import { getRefType } from '~/repository/utils/ref_type'; import getRefMixin from '../mixins/get_ref'; +const MR_SOURCE_BRANCH = 'merge_request[source_branch]'; + export default { + name: 'BlobEditHeader', + UPDATE_FILE_PATH: '/api/:version/projects/:id/repository/files/:file_path', components: { PageHeading, CommitChangesModal, @@ -28,14 +37,43 @@ export default { 'emptyRepo', 'branchAllowsCollaboration', 'lastCommitSha', + 'projectId', + 'projectPath', + 'newMergeRequestPath', + 'ref', ], + apollo: { + blobSizeData: { + query: blobSizeValidationQuery, + variables() { + return { + projectPath: this.projectPath, + filePath: this.originalFilePath, + ref: this.ref, + refType: getRefType(this.ref), + }; + }, + skip() { + return !this.isEditBlob || !this.originalFilePath; + }, + error(error) { + Sentry.captureException(error, { + tags: { + vue_component: this.$options.name, + }, + }); + }, + }, + }, data() { return { isCommitChangeModalOpen: false, fileContent: null, filePath: null, + originalFilePath: null, isLoading: false, error: null, + blobSizeData: {}, }; }, computed: { @@ -61,6 +99,33 @@ export default { ? __('An error occurred editing the blob') : __('An error occurred creating the blob'); }, + blobInfo() { + return this.blobSizeData?.project?.repository?.blobs?.nodes?.[0] || null; + }, + isFileTooLarge() { + if (!this.isEditBlob || !this.blobInfo) return false; + + const backendTooLarge = + this.blobInfo.simpleViewer?.tooLarge || this.blobInfo.richViewer?.tooLarge; + if (backendTooLarge) return true; + + const fileSize = this.blobInfo.rawSize || this.blobInfo.size || 0; + return fileSize > MAX_EDIT_SIZE; + }, + fileSizeError() { + if (!this.isFileTooLarge) return null; + + return __( + "File exceeds 10MB and can't be edited in the browser. Edit locally and push your changes.", + ); + }, + currentFilePath() { + // Extract file path from current URL for edit mode + if (!this.isEditBlob) return null; + + const pathMatch = window.location.pathname.match(/\/-\/blob\/[^/]+\/(.+)$/); + return pathMatch ? decodeURIComponent(pathMatch[1]) : null; + }, }, methods: { handleCancelButtonClick() { @@ -72,56 +137,153 @@ export default { this.editor.filepathFormMediator?.toggleValidationError(true); return; } + + if (this.isEditBlob && this.isFileTooLarge) { + this.handleError(this.fileSizeError); + return; + } + 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) { + 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; + 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) { + 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); // 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); - - return axios({ - method: this.isEditBlob ? 'put' : 'post', - 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; + const originalFormData = Object.fromEntries(formData); + + try { + const response = await this.editBlob(originalFormData); + const { data: responseData } = response; + + if (responseData.error) { + this.handleError(responseData.error); + return; + } + + this.handleEditBlobSuccess(responseData, originalFormData); + } catch (error) { + this.handleError( + this.handleGenericErrorMessage(error.response?.data?.message) || + error.response?.data?.error || + this.errorMessage, + ); + } finally { + this.isLoading = false; + } + }, + 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; + } + + this.handleCreateFileSuccess(responseData); + } catch (error) { + this.handleError( + this.handleGenericErrorMessage(error.response?.data?.message) || + error.response?.data?.error || + this.errorMessage, + ); + } finally { + this.isLoading = false; + } + }, + + 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 { + const successPath = this.getUpdatePath(responseData.branch, responseData.file_path); + // Add success parameter to URL to trigger flash message on destination page + const urlWithSuccess = mergeUrlParams({ success: 'committed' }, successPath); + visitUrl(urlWithSuccess); + } + }, + 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) + .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, + }; + + if (this.originalFilePath !== originalFormData.file_path) { + data.previous_path = this.originalFilePath; + } + + // 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) { + const url = new URL(window.location.href); + url.pathname = joinPaths(this.projectPath, '-/blob', branch, filePath); + return url.toString(); }, }, i18n: { diff --git a/app/assets/javascripts/repository/queries/blob_size_validation.query.graphql b/app/assets/javascripts/repository/queries/blob_size_validation.query.graphql new file mode 100644 index 0000000000000000000000000000000000000000..bb4ded9cb9dbd727f14e9e58974b7993f6379df1 --- /dev/null +++ b/app/assets/javascripts/repository/queries/blob_size_validation.query.graphql @@ -0,0 +1,32 @@ +query getBlobSizeValidation( + $projectPath: ID! + $filePath: String! + $ref: String! + $refType: RefType +) { + project(fullPath: $projectPath) { + __typename + id + repository { + __typename + blobs(paths: [$filePath], ref: $ref, refType: $refType) { + __typename + nodes { + __typename + id + name + size + rawSize + simpleViewer { + __typename + tooLarge + } + richViewer { + __typename + tooLarge + } + } + } + } + } +} diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index bb379a08f0b241c5f96af02ee1d8dc5695661d0f..a481922f59099c43efceab84dc84be24fdc8bcb6 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -351,7 +351,11 @@ 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), + ref: ref } end diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index ae763407742b1797dbf27d304fc09b1a446bf39e..6f9e0987be440aa19d080acce9eb82d0a549bbb8 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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index df20793a43e6555d3d9a346119aba9b5f605eed6..d529d1f5ccc623cf8ed91f156f6586d3fe13b914 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26740,9 +26740,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 "" @@ -45418,9 +45415,6 @@ msgstr "" msgid "Opens in a new window" msgstr "" -msgid "Opens new window" -msgstr "" - msgid "Operation not allowed" msgstr "" @@ -62268,9 +62262,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 "" @@ -75811,6 +75802,9 @@ msgstr "" msgid "YouTube" msgstr "" +msgid "Your %{changesLinkStart}changes%{changesLinkEnd} have been committed successfully." +msgstr "" + msgid "Your %{changes_link} have been committed successfully." msgstr "" 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/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb index 58535c82453177f2a186e93cd9ffe6b8ded3fb9d..04726ade445eb4e1f3820fba3b6df3dff637b81a 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 diff --git a/spec/frontend/repository/components/blob_content_viewer_spec.js b/spec/frontend/repository/components/blob_content_viewer_spec.js index 7a5be37154e138563eeaa04559d42acfdb51e91f..517be011b2daad69784da7b53cc55c0d12845a38 100644 --- a/spec/frontend/repository/components/blob_content_viewer_spec.js +++ b/spec/frontend/repository/components/blob_content_viewer_spec.js @@ -633,6 +633,19 @@ describe('Blob content viewer component', () => { findBlobHeader().vm.$emit('edit', 'ide'); expect(urlUtility.visitUrl).toHaveBeenCalledWith(simpleViewerMock.ideEditPath); }); + + it('creates an alert when user navigates back from successful edit', async () => { + await createComponent( + { blob: simpleViewerMock, urlParams: '?success=committed' }, + shallowMount, + ); + expect(createAlert).toHaveBeenCalledWith( + expect.objectContaining({ + message: + 'Your %{changesLinkStart}changes%{changesLinkEnd} have been committed successfully.', + }), + ); + }); }); describe('active viewer based on plain attribute', () => { diff --git a/spec/frontend/repository/mock_data.js b/spec/frontend/repository/mock_data.js index 23c730d8c75789a9f60362b0e53eca766d78e65b..eaf0e6ac28afe4c20d798c295fb1a7cfdf2f65f5 100644 --- a/spec/frontend/repository/mock_data.js +++ b/spec/frontend/repository/mock_data.js @@ -363,3 +363,67 @@ export const createCommitData = ({ pipelineEdges = defaultPipelineEdges, signatu }, }; }; + +export const blobSizeValidationMock = { + data: { + project: { + __typename: 'Project', + id: 'gid://gitlab/Project/278964', + repository: { + __typename: 'Repository', + blobs: { + __typename: 'RepositoryBlobConnection', + nodes: [ + { + __typename: 'RepositoryBlob', + id: 'gid://gitlab/RepositoryBlob/1', + name: 'some_file.js', + size: 123, + rawSize: 123, + simpleViewer: { + __typename: 'BlobViewer', + tooLarge: false, + }, + richViewer: { + __typename: 'BlobViewer', + tooLarge: false, + }, + }, + ], + }, + }, + }, + }, +}; + +export const blobSizeValidationTooLargeMock = { + data: { + project: { + __typename: 'Project', + id: 'gid://gitlab/Project/278964', + repository: { + __typename: 'Repository', + blobs: { + __typename: 'RepositoryBlobConnection', + nodes: [ + { + __typename: 'RepositoryBlob', + id: 'gid://gitlab/RepositoryBlob/1', + name: 'large_file.js', + size: 10485760, + rawSize: 10485760, + simpleViewer: { + __typename: 'BlobViewer', + tooLarge: true, + }, + richViewer: { + __typename: 'BlobViewer', + tooLarge: true, + }, + }, + ], + }, + }, + }, + }, +}; diff --git a/spec/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js index cd950ff110a56214142c62f4c8e10d245039cc22..3dc59021af0ec9a5651e1544dc0367f184a34225 100644 --- a/spec/frontend/repository/pages/blob_edit_header_spec.js +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -1,37 +1,59 @@ -import { nextTick } from 'vue'; +import Vue, { nextTick } from 'vue'; import MockAdapter from 'axios-mock-adapter'; +import VueApollo from 'vue-apollo'; import { GlButton } from '@gitlab/ui'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; 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 * as Sentry from '~/sentry/sentry_browser_wrapper'; 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 blobSizeValidationQuery from '~/repository/queries/blob_size_validation.query.graphql'; import PageHeading from '~/vue_shared/components/page_heading.vue'; import { stubComponent } from 'helpers/stub_component'; +import { blobSizeValidationMock, blobSizeValidationTooLargeMock } from '../mock_data'; +Vue.use(VueApollo); jest.mock('~/alert'); jest.mock('lodash/uniqueId', () => { return jest.fn((input) => `${input}1`); }); +jest.mock('~/sentry/sentry_browser_wrapper'); describe('BlobEditHeader', () => { let wrapper; let mock; + let fakeApollo; let visitUrlSpy; const content = 'some \r\n content \n'; 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' } = {}) => { - return shallowMountExtended(BlobEditHeader, { + const blobSizeValidationSuccessResolver = jest.fn().mockResolvedValue(blobSizeValidationMock); + const blobSizeValidationLargeSuccessResolver = jest + .fn() + .mockResolvedValue(blobSizeValidationTooLargeMock); + const blobSizeValidationErrorResolver = jest.fn().mockRejectedValue(new Error('Request failed')); + + const createWrapper = async ({ + action = 'update', + blobSizeValidationResolver = blobSizeValidationSuccessResolver, + } = {}) => { + fakeApollo = createMockApollo([[blobSizeValidationQuery, blobSizeValidationResolver]]); + + wrapper = shallowMountExtended(BlobEditHeader, { + apolloProvider: fakeApollo, provide: { action, editor: mockEditor, @@ -45,6 +67,10 @@ describe('BlobEditHeader', () => { emptyRepo: false, branchAllowsCollaboration: false, lastCommitSha: '782426692977b2cedb4452ee6501a404410f9b00', + projectId: 123, + projectPath: 'gitlab-org/gitlab', + newMergeRequestPath: 'merge_request/new/123', + ref: 'main', }, stubs: { PageHeading, @@ -55,15 +81,20 @@ describe('BlobEditHeader', () => { }), }, }); + + await waitForPromises(); }; - beforeEach(() => { + beforeEach(async () => { + window.gon = { api_version: 'v4' }; + visitUrlSpy = jest.spyOn(urlUtility, 'visitUrl'); mock = new MockAdapter(axios); - wrapper = createWrapper(); + await createWrapper(); }); afterEach(() => { + fakeApollo = null; jest.clearAllMocks(); mock.restore(); }); @@ -75,7 +106,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 +152,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?success=committed', + ); }); 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,28 +186,50 @@ 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?success=committed', + ); }); }); }); describe('for create blob', () => { - beforeEach(() => { - wrapper = createWrapper({ action: 'create' }); + beforeEach(async () => { + await createWrapper({ action: 'create' }); }); it('renders title with two buttons', () => { @@ -211,13 +280,60 @@ describe('BlobEditHeader', () => { }); describe('validation', () => { - it('toggles validation error when filename is empty', () => { + it('toggles validation error when filename is empty', async () => { mockEditor.filepathFormMediator.$filenameInput.val.mockReturnValue(null); - wrapper = createWrapper(); + await createWrapper(); findCommitChangesButton().vm.$emit('click'); expect(mockEditor.filepathFormMediator.toggleValidationError).toHaveBeenCalledWith(true); }); }); + + describe('file size validation', () => { + it('should allow editing when file size is within limit', () => { + expect(wrapper.vm.isFileTooLarge).toBe(false); + expect(wrapper.vm.fileSizeError).toBeNull(); + }); + + it('should not validate file size for create mode', async () => { + expect(wrapper.vm.isFileTooLarge).toBe(false); + + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + expect(wrapper.vm.error).toBeNull(); + }); + + it('should capture exception and quietly fail on error', async () => { + await createWrapper({ blobSizeValidationResolver: blobSizeValidationErrorResolver }); + + expect(Sentry.captureException).toHaveBeenCalledWith(expect.any(Error)); + }); + + describe('when file size is too large', () => { + it('should prevent editing when file size exceeds limit', async () => { + await createWrapper({ blobSizeValidationResolver: blobSizeValidationLargeSuccessResolver }); + + expect(wrapper.vm.isFileTooLarge).toBe(true); + expect(wrapper.vm.fileSizeError).toContain("can't be edited in the browser"); + }); + + it('should prevent editing when backend marks file as too large', async () => { + await createWrapper({ blobSizeValidationResolver: blobSizeValidationLargeSuccessResolver }); + + expect(wrapper.vm.isFileTooLarge).toBe(true); + }); + + it('should show error when trying to open modal for large file', async () => { + await createWrapper({ blobSizeValidationResolver: blobSizeValidationLargeSuccessResolver }); + + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + + expect(wrapper.vm.fileSizeError).toContain("can't be edited in the browser"); + expect(findCommitChangesModal().vm.show).not.toHaveBeenCalled(); + }); + }); + }); }); 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 diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb index 7e43ad144d0c133dd803e7d0430030be246a3f68..92893954f16c0efb109b4268f8e3975391b3c347 100644 --- a/spec/services/files/update_service_spec.rb +++ b/spec/services/files/update_service_spec.rb @@ -38,6 +38,41 @@ allow(project).to receive(:lfs_enabled?).and_return(lfs_enabled) end + context 'when file path is changed' do + let(:original_file_path) { 'files/ruby/popen.rb' } + let(:new_file_path) { 'files/ruby/popen_renamed.rb' } + let(:file_path) { new_file_path } + let(:commit_params) do + { + file_path: new_file_path, + previous_path: original_file_path, + commit_message: "Rename file", + file_content: new_contents, + file_content_encoding: "text", + start_project: project, + start_branch: start_branch, + branch_name: branch_name + } + end + + it 'updates the file with the new file name' do + # Ensure the original file exists + expect(project.repository.blob_at_branch(project.default_branch, original_file_path)).to be_present + + results = update_service.execute + + expect(results[:status]).to match(:success) + + # Original file should no longer exist + expect(project.repository.blob_at_branch(project.default_branch, original_file_path)).to be_nil + + # New file should exist with the updated content + new_blob = project.repository.blob_at_branch(project.default_branch, new_file_path) + expect(new_blob).to be_present + expect(new_blob.data).to eq(new_contents) + end + end + context 'with LFS disabled' do let(:lfs_enabled) { false }