From 66709d96af7012552e0d012c81788b077bd43b0e Mon Sep 17 00:00:00 2001 From: Chaoyue Zhao Date: Wed, 23 Oct 2024 16:19:28 -0400 Subject: [PATCH 01/16] Update edit blob to use commit change modal Changelog: changed --- .../javascripts/blob_edit/blob_bundle.js | 9 +- .../javascripts/blob_edit/blob_edit_header.js | 40 +++++++++ app/assets/javascripts/blob_edit/edit_blob.js | 4 + .../components/blob_button_group.vue | 2 +- .../components/commit_changes_modal.vue | 31 +++++-- .../repository/pages/blob_edit_header.vue | 81 ++++++++++++++++++ app/helpers/blob_helper.rb | 14 ++++ app/views/projects/blob/edit.html.haml | 10 +-- doc/user/project/repository/web_editor.md | 17 +++- spec/frontend/blob_edit/blob_bundle_spec.js | 5 +- .../components/blob_button_group_spec.js | 2 +- .../components/commit_changes_modal_spec.js | 69 +++++++++------ .../repository/pages/blob_edit_header_spec.js | 74 +++++++++++++++++ spec/helpers/blob_helper_spec.rb | 83 +++++++++++++++++++ 14 files changed, 390 insertions(+), 51 deletions(-) create mode 100644 app/assets/javascripts/blob_edit/blob_edit_header.js create mode 100644 app/assets/javascripts/repository/pages/blob_edit_header.vue create mode 100644 spec/frontend/repository/pages/blob_edit_header_spec.js diff --git a/app/assets/javascripts/blob_edit/blob_bundle.js b/app/assets/javascripts/blob_edit/blob_bundle.js index 730ea9fe80125d..7e097041beb874 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 00000000000000..2668a4daf307e2 --- /dev/null +++ b/app/assets/javascripts/blob_edit/blob_edit_header.js @@ -0,0 +1,40 @@ +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, + isUsingLfs, + blobName, + } = el.dataset; + + return new Vue({ + el, + provide: { + editor, + updatePath, + cancelPath, + originalBranch, + targetBranch, + blobName, + emptyRepo: parseBoolean(emptyRepo), + canPushCode: parseBoolean(canPushCode), + canPushToBranch: parseBoolean(canPushToBranch), + isUsingLfs: parseBoolean(isUsingLfs), + }, + 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 123e59bdff0431..0a35ba64c075d0 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/blob_button_group.vue b/app/assets/javascripts/repository/components/blob_button_group.vue index 3aec35ee96000b..c8095c5c3c4c69 100644 --- a/app/assets/javascripts/repository/components/blob_button_group.vue +++ b/app/assets/javascripts/repository/components/blob_button_group.vue @@ -144,7 +144,7 @@ export default {

-
- - +
+ + + +
diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index c7c96ed8631632..57eadcb3358cda 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -22,6 +22,7 @@ export default { 'canPushToBranch', 'emptyRepo', 'isUsingLfs', + 'branchAllowsCollaboration', ], data() { return { @@ -38,6 +39,9 @@ export default { }, }, methods: { + handleCancelButtonClick() { + window.onbeforeunload = null; + }, openModal() { this.fileContent = this.editor.getFileContent(); this.$refs[this.updateModalId].show(); @@ -47,6 +51,7 @@ export default { headerText: __('Edit file'), cancelButtonText: __('Cancel'), commitButtonText: __('Commit changes'), + confirmationMessage: __('Leave edit mode? All unsaved changes will be lost.'), }, }; @@ -57,7 +62,13 @@ export default { {{ $options.i18n.headerText }}
- {{ $options.i18n.cancelButtonText }} + {{ $options.i18n.cancelButtonText }} {{ $options.i18n.commitButtonText }} @@ -74,6 +85,7 @@ export default { :can-push-to-branch="canPushToBranch" :is-using-lfs="isUsingLfs" :file-content="fileContent" + :branch-allows-collaboration="branchAllowsCollaboration" method="put" is-edit /> diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index a5ccccaf226be9..db16ec0b53abb4 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -303,17 +303,18 @@ def vue_blob_app_data(project, blob, ref) } end - def edit_blob_app_data(project) + 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: @ref, + update_path: project_update_blob_path(project, id), + cancel_path: project_blob_path(project, id), + original_branch: ref, + target_branch: ref, 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, + 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, - is_using_lfs: @blob.stored_externally?.to_s, - blob_name: @blob.name + is_using_lfs: blob.stored_externally?.to_s, + blob_name: blob.name, + branch_allows_collaboration: project.branch_allows_collaboration?(current_user, ref).to_s } end end diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index acdab3aa798994..6aea2c2afe560e 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -19,7 +19,7 @@ - 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) } +.js-blob-edit-header{ data: edit_blob_app_data(@project, @id, @blob, @ref) } .file-editor = gl_tabs_nav({ class: 'js-edit-mode nav-links gl-border-0'}) do diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index 8618dca5873dbf..f50af8dace9da4 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 d8b1971007b8a5..afde4b5fd0396e 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -35,7 +35,11 @@ def edit_and_commit(commit_changes: true, is_diff: false) fill_editor(content: "class NextFeature#{object_id}\\nend\\n") if commit_changes - click_button 'Commit changes' + click_button('Commit changes') + + within_testid('commit-change-modal') do + click_button('Commit changes') + end end end @@ -206,18 +210,22 @@ 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 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}" + expect(page).to have_checked_field _('Create a merge request for this change') + expect(page).to have_field _('Commit to a new branch') end end end @@ -234,7 +242,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 39e75f2cee6629..77c5c9b566ddd0 100644 --- a/spec/features/projects/files/editing_a_file_spec.rb +++ b/spec/features/projects/files/editing_a_file_spec.rb @@ -28,6 +28,10 @@ click_button 'Commit changes' + within_testid('commit-change-modal') do + click_button('Commit changes') + end + expect(page).to have_content 'Someone edited the file the same time you did.' end end @@ -52,6 +56,10 @@ 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) ) diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb index 81cebde16ca4a3..2b6e3a7ab753b0 100644 --- a/spec/features/projects/files/user_edits_files_spec.rb +++ b/spec/features/projects/files/user_edits_files_spec.rb @@ -81,9 +81,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 +101,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 +125,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 +218,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 +249,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) diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index 96a06f76cf148b..41277e36e9cab6 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -455,8 +455,9 @@ 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) - expect(helper.edit_blob_app_data(project)).to include({ + 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, @@ -465,7 +466,8 @@ can_push_to_branch: 'true', empty_repo: 'false', is_using_lfs: 'false', - blob_name: blob.name + blob_name: blob.name, + branch_allows_collaboration: 'false' }) end @@ -473,7 +475,7 @@ 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)).to include( + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include( can_push_code: 'false' ) end @@ -486,7 +488,7 @@ 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)).to include( + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include( can_push_to_branch: 'false' ) end @@ -496,7 +498,7 @@ it 'returns true for empty_repo' do allow(project).to receive(:empty_repo?).and_return(true) - expect(helper.edit_blob_app_data(project)).to include( + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include( empty_repo: 'true' ) end @@ -506,11 +508,21 @@ it 'returns true for is_using_lfs' do allow(blob).to receive(:stored_externally?).and_return(true) - expect(helper.edit_blob_app_data(project)).to include( + expect(helper.edit_blob_app_data(project, id, blob, ref)).to include( is_using_lfs: '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 -- GitLab From cec3c477630a44fae2ecf2c81267efd68d8f0c26 Mon Sep 17 00:00:00 2001 From: Chaoyue Zhao Date: Tue, 5 Nov 2024 10:33:06 -0500 Subject: [PATCH 04/16] Address code reviews comments: - Clean up styling - Add supports for small screen - Add supports for loading state - Fix broken jest tests - Add forgotten translation strings --- .../javascripts/repository/pages/blob_edit_header.vue | 8 ++++---- app/views/projects/blob/edit.html.haml | 9 +++++++++ locale/gitlab.pot | 3 +++ .../repository/components/commit_changes_modal_spec.js | 2 +- spec/frontend/repository/pages/blob_edit_header_spec.js | 2 ++ 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/repository/pages/blob_edit_header.vue b/app/assets/javascripts/repository/pages/blob_edit_header.vue index 57eadcb3358cda..d28c994365ad26 100644 --- a/app/assets/javascripts/repository/pages/blob_edit_header.vue +++ b/app/assets/javascripts/repository/pages/blob_edit_header.vue @@ -57,11 +57,11 @@ export default {