diff --git a/app/assets/javascripts/blob_edit/blob_bundle.js b/app/assets/javascripts/blob_edit/blob_bundle.js index 730ea9fe80125da4869b2f3bca73bddc62485e93..7e097041beb874744105071045f974db2df605e9 100644 --- a/app/assets/javascripts/blob_edit/blob_bundle.js +++ b/app/assets/javascripts/blob_edit/blob_bundle.js @@ -1,6 +1,6 @@ import $ from 'jquery'; import { createAlert } from '~/alert'; -import NewCommitForm from '../new_commit_form'; +import initBlobEditHeader from '~/blob_edit/blob_edit_header'; export default () => { const editBlobForm = $('.js-edit-blob-form'); @@ -20,8 +20,7 @@ export default () => { import('./edit_blob') .then(({ default: EditBlob } = {}) => { - // eslint-disable-next-line no-new - new EditBlob({ + const editor = new EditBlob({ assetsPath: `${urlRoot}${assetsPath}`, filePath, currentAction, @@ -30,6 +29,8 @@ export default () => { isMarkdown, previewMarkdownPath, }); + + initBlobEditHeader(editor); }) .catch((e) => createAlert({ @@ -47,8 +48,6 @@ export default () => { window.onbeforeunload = null; }); - new NewCommitForm(editBlobForm); // eslint-disable-line no-new - // returning here blocks page navigation window.onbeforeunload = () => ''; } diff --git a/app/assets/javascripts/blob_edit/blob_edit_header.js b/app/assets/javascripts/blob_edit/blob_edit_header.js new file mode 100644 index 0000000000000000000000000000000000000000..83db372ac3fa867d873dafc2dc8ec5e43661c89c --- /dev/null +++ b/app/assets/javascripts/blob_edit/blob_edit_header.js @@ -0,0 +1,42 @@ +import Vue from 'vue'; +import { parseBoolean } from '~/lib/utils/common_utils'; +import BlobEditHeader from '~/repository/pages/blob_edit_header.vue'; + +export default function initBlobEditHeader(editor) { + const el = document.querySelector('.js-blob-edit-header'); + + if (!el) { + return null; + } + + const { + updatePath, + cancelPath, + originalBranch, + targetBranch, + canPushCode, + canPushToBranch, + emptyRepo, + blobName, + branchAllowsCollaboration, + lastCommitSha, + } = el.dataset; + + return new Vue({ + el, + provide: { + editor, + updatePath, + cancelPath, + originalBranch, + targetBranch, + blobName, + lastCommitSha, + emptyRepo: parseBoolean(emptyRepo), + canPushCode: parseBoolean(canPushCode), + canPushToBranch: parseBoolean(canPushToBranch), + branchAllowsCollaboration: parseBoolean(branchAllowsCollaboration), + }, + render: (createElement) => createElement(BlobEditHeader), + }); +} diff --git a/app/assets/javascripts/blob_edit/edit_blob.js b/app/assets/javascripts/blob_edit/edit_blob.js index 123e59bdff0431b9c182166e36c811b86e7b424d..0a35ba64c075d06f8d2412e6a3d945d30e2e5f90 100644 --- a/app/assets/javascripts/blob_edit/edit_blob.js +++ b/app/assets/javascripts/blob_edit/edit_blob.js @@ -216,4 +216,8 @@ export default class EditBlob { this.$toggleButton.toggleClass('soft-wrap-active', this.isSoftWrapped); this.editor.updateOptions({ wordWrap: this.isSoftWrapped ? 'on' : 'off' }); } + + getFileContent() { + return this.editor?.getValue(); + } } diff --git a/app/assets/javascripts/repository/components/commit_changes_modal.vue b/app/assets/javascripts/repository/components/commit_changes_modal.vue index 074cfe1c7668985fcade1000d985b1f55a0af2f0..871226a711a64049810afd9ac8263102a2a29a5a 100644 --- a/app/assets/javascripts/repository/components/commit_changes_modal.vue +++ b/app/assets/javascripts/repository/components/commit_changes_modal.vue @@ -153,6 +153,7 @@ export default { variant: 'confirm', loading: this.loading, disabled: this.loading || !this.form.state || !this.valid, + 'data-testid': 'commit-change-modal-commit-button', }, }; @@ -229,6 +230,7 @@ export default { this.$refs.message?.$el.focus(); }, async handlePrimaryAction(e) { + window.onbeforeunload = null; e.preventDefault(); // Prevent modal from closing if (this.showLfsWarning) { @@ -264,6 +266,7 @@ export default { v-bind="$attrs" :modal-id="modalId" :title="title" + data-testid="commit-change-modal" :action-primary="primaryOptions" :action-cancel="cancelOptions" @primary="handlePrimaryAction" @@ -346,16 +349,18 @@ export default {
- + + + {{ $options.i18n.CREATE_MR_LABEL }} @@ -367,17 +372,20 @@ export default { - + + + + {{ $options.i18n.CREATE_MR_LABEL }} diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue new file mode 100644 index 0000000000000000000000000000000000000000..b932c4e035c179d93bf0742ef4a789ab0cc4e683 --- /dev/null +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -0,0 +1,128 @@ + + + diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 91c82bdea7625819ae9ec66de01844c1d829c2b4..523eb97f1d5901f1169e2ffbe1c327e1d218a031 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -317,6 +317,21 @@ def vue_blob_header_app_data(project, blob, ref) ref: ref } end + + def edit_blob_app_data(project, id, blob, ref) + { + update_path: project_update_blob_path(project, id), + cancel_path: project_blob_path(project, id), + original_branch: ref, + target_branch: selected_branch, + can_push_code: can?(current_user, :push_code, project).to_s, + can_push_to_branch: project.present(current_user: current_user).can_current_user_push_to_branch?(ref).to_s, + empty_repo: project.empty_repo?.to_s, + blob_name: blob.name, + branch_allows_collaboration: project.branch_allows_collaboration?(current_user, ref).to_s, + last_commit_sha: @last_commit_sha + } + end end BlobHelper.prepend_mod_with('BlobHelper') diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index d607a94ca5dbc1ca58dbe2af9d5b0600eb481f89..d6b825621e7ab91168e4d5a9620dc4e0e2d8857b 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -3,6 +3,7 @@ - content_for :prefetch_asset_tags do - 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, @@ -18,8 +19,17 @@ - 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 } -%h1.page-title.gl-text-size-h-display.blob-edit-page-title - Edit file + +.js-blob-edit-header{ data: edit_blob_app_data(@project, @id, @blob, @ref) } + .gl-mb-4.gl-mt-5.gl-items-center.gl-justify-between.md:gl-flex.lg:gl-my-5 + %h1{ class: 'md:!gl-mb-0 gl-heading-1 gl-inline-block' } + = _('Edit file') + .gl-flex.gl-gap-3 + = render Pajamas::ButtonComponent.new do + = _('Cancel') + = render Pajamas::ButtonComponent.new(variant: :confirm) do + = _('Commit changes') + .file-editor = gl_tabs_nav({ class: 'js-edit-mode nav-links gl-border-0'}) do = gl_tab_link_to _('Write'), '#editor', { tab_class: 'active' } @@ -28,8 +38,3 @@ = form_tag(project_update_blob_path(@project, @id), method: :put, class: 'js-quick-submit js-edit-blob-form', data: blob_editor_paths(@project, 'put')) do = render 'projects/blob/editor', ref: @ref, path: @path, blob_data: @blob.data - = render 'shared/new_commit_form', placeholder: "Update #{@blob.name}" - = hidden_field_tag 'last_commit_sha', @last_commit_sha - = hidden_field_tag 'content', '', id: "file-content" - = hidden_field_tag 'from_merge_request_iid', params[:from_merge_request_iid] - = render 'projects/commit_button', ref: @ref, cancel_path: project_blob_path(@project, @id) diff --git a/ee/spec/helpers/ee/blob_helper_spec.rb b/ee/spec/helpers/ee/blob_helper_spec.rb index fbc09fd7a874787a3e09086aa3630f7d271bc844..d9eeefb7d1d30cb155378fe96d9ebfad70666aa7 100644 --- a/ee/spec/helpers/ee/blob_helper_spec.rb +++ b/ee/spec/helpers/ee/blob_helper_spec.rb @@ -68,12 +68,8 @@ let(:project) { build_stubbed(:project) } let(:ref) { 'main' } - before do - allow(helper).to receive(:selected_branch).and_return(ref) - end - it 'returns data related to blob app' do - allow(helper).to receive(:current_user).and_return(nil) + allow(helper).to receive_messages(selected_branch: ref, current_user: nil) expect(helper.vue_blob_app_data(project, blob, ref)).to include({ user_id: '', diff --git a/qa/qa/page/file/shared/editor.rb b/qa/qa/page/file/shared/editor.rb index 86d044bb4fa117adc5ab256544c5667ff4930340..93db7b6807fe15258e0e52e6d26110956c79649a 100644 --- a/qa/qa/page/file/shared/editor.rb +++ b/qa/qa/page/file/shared/editor.rb @@ -13,6 +13,18 @@ def self.included(base) base.view 'app/views/projects/blob/_editor.html.haml' do element 'source-editor-preview-container' end + + base.view 'app/assets/javascripts/repository/components/commit_changes_modal.vue' do + element 'commit-change-modal' + end + + base.view 'app/assets/javascripts/repository/components/commit_changes_modal.vue' do + element 'commit-change-modal-commit-button' + end + + base.view 'app/assets/javascripts/repository/pages/blob_edit_header.vue' do + element 'blob-edit-header-commit-button' + end end def add_content(content) @@ -27,6 +39,20 @@ def remove_content end end + def click_commit_changes_in_header + click_element('blob-edit-header-commit-button') + end + + def commit_changes_through_modal + within_element 'commit-change-modal' do + click_element('commit-change-modal-commit-button') + end + end + + def has_modal_commit_button? + has_element?('commit-change-modal-commit-button') + end + private def text_area diff --git a/qa/qa/page/file/show.rb b/qa/qa/page/file/show.rb index 4637cedad418a5e7dec6d53f56c596c833c963f2..6bb4b9b1b80448afd14691c92d144a092254feaa 100644 --- a/qa/qa/page/file/show.rb +++ b/qa/qa/page/file/show.rb @@ -7,6 +7,7 @@ class Show < Page::Base include Shared::CommitMessage include Layout::Flash include Page::Component::BlobContent + include Shared::Editor view 'app/assets/javascripts/repository/components/blob_button_group.vue' do element 'lock-button' @@ -34,10 +35,6 @@ def highlight_text def explain_code click_element('question-icon') end - - def click_commit_changes - click_on 'Commit changes' - end end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/file/delete_file_via_web_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/file/delete_file_via_web_spec.rb index 4085c4953f64954d5857387b258da394b44da531..aaef69eda20b85c1a8fe13dbea1aa0025f9def12 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/file/delete_file_via_web_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/file/delete_file_via_web_spec.rb @@ -16,9 +16,10 @@ module QA Page::File::Show.perform do |file| file.click_delete file.add_commit_message(commit_message_for_delete) - file.click_commit_changes end + Page::File::Edit.perform(&:commit_changes_through_modal) + Page::Project::Show.perform do |project| aggregate_failures 'file details' do expect(project).to have_notice('The file has been successfully deleted.') diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/file/edit_file_via_web_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/file/edit_file_via_web_spec.rb index 11280c71618854ca1f7f84fa1954583c3b816719..8fa99a55c3b499633015a2926d1b5b5e18a21294 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/file/edit_file_via_web_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/file/edit_file_via_web_spec.rb @@ -19,8 +19,9 @@ module QA Page::File::Form.perform do |file| file.remove_content file.add_content(updated_file_content) + file.click_commit_changes_in_header file.add_commit_message(commit_message_for_update) - file.commit_changes + file.commit_changes_through_modal end Page::File::Show.perform do |file| 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 37e7aa898610a2e3040d02badca13b51843077b7..52cc5ef8629aa0e6dfa1ad149956aaba5475642b 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 @@ -29,7 +29,8 @@ module QA file.add_content("# #{edited_readme_content}") file.preview expect(file.has_markdown_preview?('h1', edited_readme_content)).to be true - file.commit_changes + file.click_commit_changes_in_header + file.commit_changes_through_modal end Page::File::Show.perform do |file| diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/user/user_inherited_access_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/user/user_inherited_access_spec.rb index a49bf4c99c4dc7ee3a515f702a026edb57a9b28d..a5eea8d98492303c23d63c03f862e7d8631c299b 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/user/user_inherited_access_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/user/user_inherited_access_spec.rb @@ -36,8 +36,9 @@ module QA Page::File::Show.perform(&:click_edit) - Page::File::Form.perform do |file_form| - expect(file_form).to have_element('data-testid': 'commit-button') + Page::File::Edit.perform do |file| + file.click_commit_changes_in_header + expect(file).to have_modal_commit_button end end end diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index 8618dca5873dbf69de90c128868c9e7bbf5516bd..f50af8dace9da4a3880786524140df6e0d6cd8af 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -40,6 +40,8 @@ end it 'mentions commits will go to the source branch' do + click_button 'Commit changes' + expect(page).to have_content('Your changes can be committed to fix because a merge request is open.') end @@ -48,6 +50,11 @@ editor_set_value(content) click_button 'Commit changes' + + within_testid('commit-change-modal') do + click_button 'Commit changes' + end + wait_for_requests expect(page).to have_content('Your changes have been committed successfully') diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index 0f5f7db5bf4b335bd0911125afc5478aaf77a35d..b1660b9c06136d7ee4d04c0365b12e1f229f273f 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -34,7 +34,13 @@ def edit_and_commit(commit_changes: true, is_diff: false) # there may be no diff and nothing to render. fill_editor(content: "class NextFeature#{object_id}\\nend\\n") - click_button 'Commit changes' if commit_changes + return unless commit_changes + + click_button('Commit changes') + + within_testid('commit-change-modal') do + click_button('Commit changes') + end end def fill_editor(content: 'class NextFeature\\nend\\n') @@ -204,18 +210,23 @@ def has_toolbar_buttons it 'shows blob editor with same branch' do expect(page).to have_current_path(project_edit_blob_path(project, tree_join(branch, file_path))) - expect(find('.js-branch-name').value).to eq(branch) + + click_button('Commit changes') + + expect(page).to have_selector('code', text: branch) end end context 'with protected branch' do - it 'shows blob editor with patch branch' do + it 'shows blob editor with patch branch and option to create MR' do freeze_time do visit project_edit_blob_path(project, tree_join(protected_branch, file_path)) - epoch = Time.zone.now.strftime('%s%L').last(5) + click_button('Commit changes') - expect(find('.js-branch-name').value).to eq "#{user.username}-protected-branch-patch-#{epoch}" + epoch = Time.zone.now.strftime('%s%L').last(5) + expect(page).to have_checked_field _('Create a merge request for this change') + expect(find_field(_('Commit to a new branch')).value).to eq "#{user.username}-protected-branch-patch-#{epoch}" end end end @@ -232,7 +243,10 @@ def has_toolbar_buttons it 'shows blob editor with same branch' do expect(page).to have_current_path(project_edit_blob_path(project, tree_join(branch, file_path))) - expect(find('.js-branch-name').value).to eq(branch) + + click_button('Commit changes') + + expect(page).to have_selector('code', text: branch) end end end diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb index 39e75f2cee6629ffcafaae255abf674fec5a4dd8..58535c82453177f2a186e93cd9ffe6b8ded3fb9d 100644 --- a/spec/features/projects/files/editing_a_file_spec.rb +++ b/spec/features/projects/files/editing_a_file_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Projects > Files > User wants to edit a file', feature_category: :source_code_management do +RSpec.describe 'Projects > Files > User wants to edit a file', :js, feature_category: :source_code_management do include ProjectForksHelper let(:project) { create(:project, :repository, :public) } let(:user) { project.first_owner } @@ -28,7 +28,11 @@ click_button 'Commit changes' - expect(page).to have_content 'Someone edited the file the same time you did.' + within_testid('commit-change-modal') do + click_button('Commit changes') + end + + expect(page).to have_content 'An error occurred editing the blob' end end @@ -52,8 +56,12 @@ it 'renders an error message' do click_button 'Commit changes' + within_testid('commit-change-modal') do + click_button('Commit changes') + end + expect(page).to have_content( - %(Error: Can't edit this file. The fork and upstream project have diverged. Edit the file on the fork) + 'An error occurred editing the blob' ) end end diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb index 81cebde16ca4a3984d69392ab4e57b1fe7d76dde..45867118071dfe5a554049e1787c35d5b73bb8ed 100644 --- a/spec/features/projects/files/user_edits_files_spec.rb +++ b/spec/features/projects/files/user_edits_files_spec.rb @@ -17,6 +17,11 @@ let(:project_tree_path_root_ref) { project_tree_path(project, project.repository.root_ref) } let(:project2_tree_path_root_ref) { project_tree_path(project2, project2.repository.root_ref) } + let_it_be(:lf_text) { 'Line 1\nLine 2\nLine 3\n' } + let_it_be(:crlf_text) { 'Line 1\r\nLine 2\r\nLine 3\r\n"' } + let_it_be(:project_with_lf) { create(:project, :custom_repo, name: 'Project with lf', files: { 'lf_file.txt' => lf_text }) } + let_it_be(:project_with_crlf) { create(:project, :custom_repo, name: 'Project with crlf', files: { 'crlf_file.txt' => crlf_text }) } + before do stub_feature_flags(vscode_web_ide: false) @@ -81,9 +86,13 @@ find('.file-editor', match: :first) editor_set_value('*.rbca') - fill_in(:commit_message, with: 'New commit message', visible: true) click_button('Commit changes') + within_testid('commit-change-modal') do + fill_in(:commit_message, with: 'New commit message', visible: true) + click_button('Commit changes') + end + expect(page).to have_current_path(project_blob_path(project, 'master/.gitignore'), ignore_query: true) wait_for_requests @@ -97,9 +106,13 @@ find('.file-editor', match: :first) editor_set_value('*.rbca') - fill_in(:commit_message, with: 'New commit message', visible: true) click_button('Commit changes') + within_testid('commit-change-modal') do + fill_in(:commit_message, with: 'New commit message', visible: true) + click_button('Commit changes') + end + expect(page).to have_current_path(project_blob_path(project, 'master/.gitignore'), ignore_query: true) wait_for_requests @@ -117,10 +130,14 @@ find('.file-editor', match: :first) editor_set_value('*.rbca') - fill_in(:commit_message, with: 'New commit message', visible: true) - fill_in(:branch_name, with: 'new_branch_name', visible: true) click_button('Commit changes') + within_testid('commit-change-modal') do + fill_in(:commit_message, with: 'New commit message', visible: true) + choose(option: true) + fill_in(:branch_name, with: 'new_branch_name', visible: true) + click_button('Commit changes') + end expect(page).to have_current_path(project_new_merge_request_path(project), ignore_query: true) click_link('Changes') @@ -206,9 +223,13 @@ def expect_fork_status find('.file-editor', match: :first) editor_set_value('*.rbca') - fill_in(:commit_message, with: 'New commit message', visible: true) click_button('Commit changes') + within_testid('commit-change-modal') do + fill_in(:commit_message, with: 'New commit message', visible: true) + click_button('Commit changes') + end + fork = user.fork_of(project2.reload) expect(page).to have_current_path(project_new_merge_request_path(fork), ignore_query: true) @@ -233,9 +254,13 @@ def expect_fork_status expect(page).not_to have_link('Fork') editor_set_value('*.rbca') - fill_in(:commit_message, with: 'Another commit', visible: true) click_button('Commit changes') + within_testid('commit-change-modal') do + fill_in(:commit_message, with: 'Another commit', visible: true) + click_button('Commit changes') + end + fork = user.fork_of(project2) expect(page).to have_current_path(project_new_merge_request_path(fork), ignore_query: true) @@ -264,4 +289,47 @@ def expect_fork_status expect(find('.monaco-editor')).to have_content(json_text) end end + + context 'for line endings', :js do + before_all do + project_with_lf.add_maintainer(user) + project_with_crlf.add_maintainer(user) + end + + it 'does not mutate LF line endings' do + visit(project_edit_blob_path(project_with_lf, tree_join(project_with_lf.default_branch, 'lf_file.txt'))) + wait_for_requests + + find('.file-editor', match: :first) + + click_button('Commit changes') + + within_testid('commit-change-modal') do + fill_in(:commit_message, with: 'New commit message', visible: true) + choose(option: true) + fill_in(:branch_name, with: 'new_branch_name', visible: true) + click_button('Commit changes') + end + + expect(page).to have_content('Changes 0') + end + + it 'does not mutate CRLF line endings' do + visit(project_edit_blob_path(project_with_crlf, tree_join(project_with_crlf.default_branch, 'crlf_file.txt'))) + wait_for_requests + + find('.file-editor', match: :first) + + click_button('Commit changes') + + within_testid('commit-change-modal') do + fill_in(:commit_message, with: 'New commit message', visible: true) + choose(option: true) + fill_in(:branch_name, with: 'new_branch_name', visible: true) + click_button('Commit changes') + end + + expect(page).to have_content('Changes 0') + end + end end diff --git a/spec/features/projects/files/user_replaces_files_spec.rb b/spec/features/projects/files/user_replaces_files_spec.rb index 926efbd4c2cb8d66db9cdf8f2c368777e8fe19d0..da8bc710e17f3613fca5ee03dfd4e0d3abd2daff 100644 --- a/spec/features/projects/files/user_replaces_files_spec.rb +++ b/spec/features/projects/files/user_replaces_files_spec.rb @@ -48,7 +48,7 @@ click_on('Replace') find(".upload-dropzone-card").drop(File.join(Rails.root, 'spec', 'fixtures', 'doc_sample.txt')) - within_testid('upload-blob-modal') do + page.within('#modal-replace-blob') do fill_in(:commit_message, with: 'Replacement file commit message') click_button('Commit changes') end diff --git a/spec/frontend/blob_edit/blob_bundle_spec.js b/spec/frontend/blob_edit/blob_bundle_spec.js index ca75cb00fbd367437fe03d18a7265cd8b8dbbbcf..b39e471272cc0377114f56e0459db488258f6e99 100644 --- a/spec/frontend/blob_edit/blob_bundle_spec.js +++ b/spec/frontend/blob_edit/blob_bundle_spec.js @@ -2,12 +2,14 @@ import $ from 'jquery'; import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; import waitForPromises from 'helpers/wait_for_promises'; import blobBundle from '~/blob_edit/blob_bundle'; +import initBlobEditHeader from '~/blob_edit/blob_edit_header'; import SourceEditor from '~/blob_edit/edit_blob'; import { createAlert } from '~/alert'; jest.mock('~/blob_edit/edit_blob'); jest.mock('~/alert'); +jest.mock('~/blob_edit/blob_edit_header'); describe('BlobBundle', () => { beforeAll(() => { @@ -27,7 +29,8 @@ describe('BlobBundle', () => { blobBundle(); await waitForPromises(); expect(SourceEditor).toHaveBeenCalled(); - + expect(initBlobEditHeader).toHaveBeenCalledTimes(1); + expect(initBlobEditHeader).toHaveBeenCalledWith(expect.any(SourceEditor)); resetHTMLFixture(); }); diff --git a/spec/frontend/blob_edit/edit_blob_spec.js b/spec/frontend/blob_edit/edit_blob_spec.js index 27e456437586c58a80250743c7ca93a36b046e6a..974f0d57c0ebad05d533b3eaa23f1cd380bafada 100644 --- a/spec/frontend/blob_edit/edit_blob_spec.js +++ b/spec/frontend/blob_edit/edit_blob_spec.js @@ -49,12 +49,14 @@ describe('Blob Editing', () => { const filePath = 'path/to/file.js'; const useMock = jest.fn(() => markdownExtensions); const unuseMock = jest.fn(); + const valueMock = 'test value'; + const getValueMock = jest.fn().mockReturnValue('test value'); const emitter = new Emitter(); const mockInstance = { use: useMock, unuse: unuseMock, setValue: jest.fn(), - getValue: jest.fn().mockReturnValue('test value'), + getValue: getValueMock, focus: jest.fn(), onDidChangeModelLanguage: emitter.event, updateModelLanguage: jest.fn(), @@ -114,6 +116,11 @@ describe('Blob Editing', () => { }), ); }); + + it('returns content from the editor', () => { + expect(blobInstance.getFileContent()).toBe(valueMock); + expect(getValueMock).toHaveBeenCalled(); + }); }); it('loads SourceEditorExtension and FileTemplateExtension by default', async () => { diff --git a/spec/frontend/repository/components/commit_changes_modal_spec.js b/spec/frontend/repository/components/commit_changes_modal_spec.js index 941ae8307c7d0a004fe0baef9aa4113662a1c001..39365b20a193d5c989375ca5467ccc48a7a75ec7 100644 --- a/spec/frontend/repository/components/commit_changes_modal_spec.js +++ b/spec/frontend/repository/components/commit_changes_modal_spec.js @@ -12,6 +12,7 @@ import { mount } from '@vue/test-utils'; import { nextTick } from 'vue'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import { RENDER_ALL_SLOTS_TEMPLATE, stubComponent } from 'helpers/stub_component'; +import setWindowLocation from 'helpers/set_window_location_helper'; import CommitChangesModal from '~/repository/components/commit_changes_modal.vue'; import { sprintf } from '~/locale'; @@ -88,77 +89,77 @@ describe('CommitChangesModal', () => { linkEnd: '', }); - beforeEach(() => createComponent({ props: { isUsingLfs: true } })); + describe('LFS warning', () => { + beforeEach(() => createComponent({ props: { isUsingLfs: true } })); - it('renders a modal containing LFS text', () => { - expect(findModal().props('title')).toBe(lfsTitleText); - expect(findModal().text()).toContain(primaryLfsText); - expect(findModal().text()).toContain(secondaryLfsText); - }); + it('renders a modal containing LFS text', () => { + expect(findModal().props('title')).toBe(lfsTitleText); + expect(findModal().text()).toContain(primaryLfsText); + expect(findModal().text()).toContain(secondaryLfsText); + }); - it('hides the LFS content if the continue button is clicked', async () => { - findModal().vm.$emit('primary', { preventDefault: jest.fn() }); - await nextTick(); + it('hides the LFS content when the continue button is clicked', async () => { + findModal().vm.$emit('primary', { preventDefault: jest.fn() }); + await nextTick(); - expect(findModal().props('title')).not.toBe(lfsTitleText); - expect(findModal().text()).not.toContain(primaryLfsText); - expect(findModal().text()).not.toContain(secondaryLfsText); + expect(findModal().props('title')).not.toBe(lfsTitleText); + expect(findModal().text()).not.toContain(primaryLfsText); + expect(findModal().text()).not.toContain(secondaryLfsText); + }); }); }); - describe('renders modal component', () => { - it('renders with correct props', () => { - createComponent(); + it('renders Modal component', () => { + createComponent(); - expect(findModal().props()).toMatchObject({ - size: 'md', - actionPrimary: { - text: 'Commit changes', - }, - actionCancel: { - text: 'Cancel', - }, - }); - expect(findSlot().exists()).toBe(false); + expect(findModal().props()).toMatchObject({ + size: 'md', + actionPrimary: { + text: 'Commit changes', + }, + actionCancel: { + text: 'Cancel', + }, }); + expect(findSlot().exists()).toBe(false); + }); - it('renders the body slot when one is provided', () => { - createComponent({ - slots: { - body: '
test body slot
', - }, - }); - expect(findSlot().text()).toBe('test body slot'); + it('renders the body slot when one is provided', () => { + createComponent({ + slots: { + body: '
test body slot
', + }, }); + expect(findSlot().text()).toBe('test body slot'); + }); - it('renders the form field slot when one is provided', () => { - createComponent({ - slots: { - body: '
test form fields slot
', - }, - }); - expect(findSlot().text()).toBe('test form fields slot'); + it('renders the form field slot when one is provided', () => { + createComponent({ + slots: { + body: '
test form fields slot
', + }, }); + expect(findSlot().text()).toBe('test form fields slot'); + }); - it('disables actionable while loading', () => { - createComponent({ props: { loading: true } }); + it('disables actionable while loading', () => { + createComponent({ props: { loading: true } }); - expect(findModal().props('actionPrimary').attributes).toEqual( - expect.objectContaining({ disabled: true }), - ); - expect(findModal().props('actionCancel').attributes).toEqual( - expect.objectContaining({ disabled: true }), - ); - expect(findCommitTextarea().attributes()).toEqual( - expect.objectContaining({ disabled: 'true' }), - ); - expect(findCurrentBranchRadioOption().attributes()).toEqual( - expect.objectContaining({ disabled: 'true' }), - ); - expect(findNewBranchRadioOption().attributes()).toEqual( - expect.objectContaining({ disabled: 'true' }), - ); - }); + expect(findModal().props('actionPrimary').attributes).toEqual( + expect.objectContaining({ disabled: true }), + ); + expect(findModal().props('actionCancel').attributes).toEqual( + expect.objectContaining({ disabled: true }), + ); + expect(findCommitTextarea().attributes()).toEqual( + expect.objectContaining({ disabled: 'true' }), + ); + expect(findCurrentBranchRadioOption().attributes()).toEqual( + expect.objectContaining({ disabled: 'true' }), + ); + expect(findNewBranchRadioOption().attributes()).toEqual( + expect.objectContaining({ disabled: 'true' }), + ); }); describe('form', () => { @@ -263,7 +264,12 @@ describe('CommitChangesModal', () => { ${'create_merge_request'} | ${undefined} | ${true} | ${false} | ${true} | ${false} `( 'passes $input as a hidden input with the correct value', - ({ input, value, emptyRepo, canPushCode, canPushToBranch, exist }) => { + ({ input, value, emptyRepo, canPushCode, canPushToBranch, exist, fromMergeRequestIid }) => { + if (fromMergeRequestIid) { + setWindowLocation( + `https://gitlab.test/foo?from_merge_request_iid=${fromMergeRequestIid}`, + ); + } createComponent({ props: { emptyRepo, @@ -337,7 +343,9 @@ describe('CommitChangesModal', () => { }); it('does not submit form', () => { - findModal().vm.$emit('primary', { preventDefault: () => {} }); + findModal().vm.$emit('primary', { + preventDefault: () => {}, + }); expect(wrapper.emitted('submit-form')).toBeUndefined(); }); }); @@ -354,7 +362,9 @@ describe('CommitChangesModal', () => { }); it('does not submit form', () => { - findModal().vm.$emit('primary', { preventDefault: () => {} }); + findModal().vm.$emit('primary', { + preventDefault: () => {}, + }); expect(wrapper.emitted('submit-form')).toBeUndefined(); }); }); diff --git a/spec/frontend/repository/pages/blob_edit_header_spec.js b/spec/frontend/repository/pages/blob_edit_header_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..5f8d37f4a5bf429bf1b4dd9afbc3d7ddcee9e618 --- /dev/null +++ b/spec/frontend/repository/pages/blob_edit_header_spec.js @@ -0,0 +1,142 @@ +import { nextTick } from 'vue'; +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 } from '~/lib/utils/http_status'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import CommitChangesModal from '~/repository/components/commit_changes_modal.vue'; +import BlobEditHeader from '~/repository/pages/blob_edit_header.vue'; +import { createAlert } from '~/alert'; +import { stubComponent } from 'helpers/stub_component'; + +jest.mock('~/alert'); +jest.mock('lodash/uniqueId', () => { + return jest.fn((input) => `${input}1`); +}); + +describe('BlobEditHeader', () => { + let wrapper; + let mock; + let visitUrlSpy; + + const content = 'some \r\n content \n'; + + const mockEditor = { + getFileContent: jest.fn().mockReturnValue(content), + filepathFormMediator: { $filenameInput: { val: jest.fn().mockReturnValue('.gitignore') } }, + }; + + const createWrapper = () => { + return shallowMountExtended(BlobEditHeader, { + provide: { + editor: mockEditor, + updatePath: '/update', + cancelPath: '/cancel', + originalBranch: 'main', + targetBranch: 'feature', + blobName: 'test.js', + canPushCode: true, + canPushToBranch: true, + emptyRepo: false, + branchAllowsCollaboration: false, + lastCommitSha: '782426692977b2cedb4452ee6501a404410f9b00', + }, + stubs: { + CommitChangesModal: stubComponent(CommitChangesModal, { + methods: { + show: jest.fn(), + }, + }), + }, + }); + }; + + beforeEach(() => { + visitUrlSpy = jest.spyOn(urlUtility, 'visitUrl'); + mock = new MockAdapter(axios); + wrapper = createWrapper(); + }); + + afterEach(() => { + jest.clearAllMocks(); + mock.restore(); + }); + + const findTitle = () => wrapper.find('h1'); + const findButtons = () => wrapper.findAllComponents(GlButton); + const findCommitChangesModal = () => wrapper.findComponent(CommitChangesModal); + const findCommitChangesButton = () => wrapper.findByTestId('blob-edit-header-commit-button'); + const findCancelButton = () => wrapper.findByTestId('blob-edit-header-cancel-button'); + + const submitForm = async () => { + findCommitChangesModal().vm.$emit('submit-form', new FormData()); + + await axios.waitForAll(); + }; + + it('renders title with two buttons', () => { + expect(findTitle().text()).toBe('Edit file'); + const buttons = findButtons(); + expect(buttons).toHaveLength(2); + expect(buttons.at(0).text()).toBe('Cancel'); + expect(buttons.at(1).text()).toBe('Commit changes'); + }); + + it('opens commit changes modal with correct props', async () => { + findCommitChangesButton().vm.$emit('click'); + await nextTick(); + expect(mockEditor.getFileContent).toHaveBeenCalled(); + expect(findCommitChangesModal().props()).toEqual({ + modalId: 'update-modal1', + canPushCode: true, + canPushToBranch: true, + commitMessage: 'Edit test.js', + originalBranch: 'main', + targetBranch: 'feature', + isUsingLfs: false, + loading: false, + emptyRepo: false, + branchAllowsCollaboration: false, + valid: true, + }); + }); + + it('shows confirmation message on cancel button', () => { + expect(findCancelButton().attributes('data-confirm')).toBe( + 'Leave edit mode? All unsaved changes will be lost.', + ); + }); + + it('on submit, redirects to the updated file', async () => { + findCommitChangesButton().vm.$emit('click'); + + mock.onPut('/update').replyOnce(HTTP_STATUS_OK, { filePath: '/update/path' }); + await submitForm(); + + expect(mock.history.put).toHaveLength(1); + const putData = JSON.parse(mock.history.put[0].data); + expect(putData.file).toBe(content); + expect(visitUrlSpy).toHaveBeenCalledWith('/update/path'); + }); + + it('creates alert when there is no filePath in response', async () => { + mock.onPut('/update').reply(HTTP_STATUS_OK); + await submitForm(); + + expect(createAlert).toHaveBeenCalledWith({ + captureError: true, + message: 'An error occurred editing the blob', + }); + }); + + it('on error, creates an alert error', async () => { + mock.onPut('/update').timeout(); + await submitForm(); + + expect(createAlert).toHaveBeenCalledWith({ + captureError: true, + message: 'An error occurred editing the blob', + }); + }); +}); diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index 7bf6fc7300e4cbcca5412a225fc6dbfd8012baaa..1f4e870ae7a1f374b09ed553465d56cb9e8b27b5 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BlobHelper do +RSpec.describe BlobHelper, feature_category: :source_code_management do include TreeHelper include FakeBlobHelpers include Devise::Test::ControllerHelpers @@ -211,7 +211,7 @@ end describe '#ide_edit_path' do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } let(:current_user) { create(:user) } let(:can_push_code) { true } @@ -421,7 +421,6 @@ let_it_be(:user) { build_stubbed(:user) } before do - allow(helper).to receive(:current_user).and_return(user) allow(Ability).to receive(:allowed?).and_call_original allow(Ability).to receive(:allowed?).with(user, :download_code, project).and_return(true) end @@ -434,6 +433,87 @@ end end + describe '#edit_blob_app_data' do + let(:project) { build_stubbed(:project) } + let(:user) { build_stubbed(:user) } + let(:blob) { fake_blob(path: 'test.rb', size: 100.bytes) } + let(:ref) { 'main' } + let(:id) { "#{ref}/#{blob.path}" } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:selected_branch).and_return(ref) + end + + it 'returns data related to blob editing' do + project_presenter = instance_double(ProjectPresenter) + + allow(helper).to receive(:can?).with(user, :push_code, project).and_return(true) + allow(project).to receive(:present).and_return(project_presenter) + allow(project_presenter).to receive(:can_current_user_push_to_branch?).with(ref).and_return(true) + allow(project).to receive(:empty_repo?).and_return(false) + allow(blob).to receive(:stored_externally?).and_return(false) + allow(project).to receive(:branch_allows_collaboration?).with(user, ref).and_return(false) + assign(:last_commit_sha, '782426692977b2cedb4452ee6501a404410f9b00') + + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include({ + update_path: project_update_blob_path(project, id), + cancel_path: project_blob_path(project, id), + original_branch: ref, + target_branch: ref, + can_push_code: 'true', + can_push_to_branch: 'true', + empty_repo: 'false', + blob_name: blob.name, + branch_allows_collaboration: 'false', + last_commit_sha: '782426692977b2cedb4452ee6501a404410f9b00' + }) + end + + context 'when user cannot push code' do + it 'returns false for push permissions' do + allow(helper).to receive(:can?).with(user, :push_code, project).and_return(false) + + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include( + can_push_code: 'false' + ) + end + end + + context 'when user cannot push to branch' do + it 'returns false for branch push permissions' do + project_presenter = instance_double(ProjectPresenter) + + allow(project).to receive(:present).and_return(project_presenter) + allow(project_presenter).to receive(:can_current_user_push_to_branch?).with(ref).and_return(false) + + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include( + can_push_to_branch: 'false' + ) + end + end + + context 'when repository is empty' do + it 'returns true for empty_repo' do + allow(project).to receive(:empty_repo?).and_return(true) + + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include( + empty_repo: 'true' + ) + end + end + + context 'branch collaboration' do + it 'returns true when branch allows collaboration' do + allow(project).to receive(:branch_allows_collaboration?).with(user, ref).and_return(true) + + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include( + branch_allows_collaboration: 'true' + ) + end + end + end + describe "#copy_blob_source_button" do let(:project) { build_stubbed(:project) }