diff --git a/app/assets/javascripts/blob/components/blob_header.vue b/app/assets/javascripts/blob/components/blob_header.vue index 5411881a8d2d10dc4b68c3e803dab4e020da4ab0..79ac27ccbe2664bb916cfa6f50236e32c46bb035 100644 --- a/app/assets/javascripts/blob/components/blob_header.vue +++ b/app/assets/javascripts/blob/components/blob_header.vue @@ -75,6 +75,11 @@ export default { required: false, default: false, }, + showWebIdeForkSuggestion: { + type: Boolean, + required: false, + default: false, + }, projectPath: { type: String, required: false, @@ -156,6 +161,7 @@ export default { :edit-url="blob.editBlobPath" :web-ide-url="blob.ideEditPath" :needs-to-fork="showForkSuggestion" + :needs-to-fork-with-web-ide="showWebIdeForkSuggestion" :show-pipeline-editor-button="Boolean(blob.pipelineEditorPath)" :pipeline-editor-url="blob.pipelineEditorPath" :gitpod-url="blob.gitpodBlobUrl" diff --git a/app/assets/javascripts/repository/components/blob_content_viewer.vue b/app/assets/javascripts/repository/components/blob_content_viewer.vue index 4102036d74152ba8c472bb01666ec835de8489c8..cf67fb44c1ffe7f968c5ec263237796511f0df8e 100644 --- a/app/assets/javascripts/repository/components/blob_content_viewer.vue +++ b/app/assets/javascripts/repository/components/blob_content_viewer.vue @@ -174,13 +174,23 @@ export default { return pathLock ? pathLock.user : null; }, - showForkSuggestion() { + canFork() { const { createMergeRequestIn, forkProject } = this.userPermissions; + + return this.isLoggedIn && !this.isUsingLfs && createMergeRequestIn && forkProject; + }, + showSingleFileEditorForkSuggestion() { const { canModifyBlob } = this.blobInfo; - return ( - this.isLoggedIn && !this.isUsingLfs && !canModifyBlob && createMergeRequestIn && forkProject - ); + return this.canFork && !canModifyBlob; + }, + showWebIdeForkSuggestion() { + const { canModifyBlobWithWebIde } = this.blobInfo; + + return this.canFork && !canModifyBlobWithWebIde; + }, + showForkSuggestion() { + return this.showSingleFileEditorForkSuggestion || this.showWebIdeForkSuggestion; }, forkPath() { const forkPaths = { @@ -265,14 +275,24 @@ export default { if (this.$route?.query?.plain === plain) return; this.$router.push({ path: this.$route.path, query: { ...this.$route.query, plain } }); }, + isIdeTarget(target) { + return target === 'ide'; + }, + forkSuggestionForSelectedEditor(target) { + return this.isIdeTarget(target) + ? this.showWebIdeForkSuggestion + : this.showSingleFileEditorForkSuggestion; + }, editBlob(target) { - if (this.showForkSuggestion) { + const { ideEditPath, editBlobPath } = this.blobInfo; + const isIdeTarget = this.isIdeTarget(target); + const showForkSuggestionForSelectedEditor = this.forkSuggestionForSelectedEditor(target); + + if (showForkSuggestionForSelectedEditor) { this.setForkTarget(target); - return; + } else { + visitUrl(isIdeTarget ? ideEditPath : editBlobPath); } - - const { ideEditPath, editBlobPath } = this.blobInfo; - visitUrl(target === 'ide' ? ideEditPath : editBlobPath); }, setForkTarget(target) { this.forkTarget = target; @@ -311,7 +331,8 @@ export default { :has-render-error="hasRenderError" :show-path="false" :override-copy="true" - :show-fork-suggestion="showForkSuggestion" + :show-fork-suggestion="showSingleFileEditorForkSuggestion" + :show-web-ide-fork-suggestion="showWebIdeForkSuggestion" :show-blame-toggle="true" :project-path="projectPath" :project-id="projectId" @@ -334,7 +355,7 @@ export default { :project-path="projectPath" :is-locked="Boolean(pathLockedByUser)" :can-lock="canLock" - :show-fork-suggestion="showForkSuggestion" + :show-fork-suggestion="showSingleFileEditorForkSuggestion" :is-using-lfs="isUsingLfs" @fork="setForkTarget('view')" /> diff --git a/app/assets/javascripts/vue_shared/components/web_ide_link.vue b/app/assets/javascripts/vue_shared/components/web_ide_link.vue index 0f7307d0b51d91df605c929554348a4766913a87..096ba7c00e840a315b323f5695d375ace46c3487 100644 --- a/app/assets/javascripts/vue_shared/components/web_ide_link.vue +++ b/app/assets/javascripts/vue_shared/components/web_ide_link.vue @@ -57,6 +57,11 @@ export default { required: false, default: false, }, + needsToForkWithWebIde: { + type: Boolean, + required: false, + default: false, + }, gitpodEnabled: { type: Boolean, required: false, @@ -221,7 +226,7 @@ export default { webIdeAction() { if (!this.showWebIdeButton) return null; - const handleOptions = this.needsToFork + const handleOptions = this.needsToForkWithWebIde ? { handle: () => { if (this.disableForkModal) { diff --git a/app/graphql/queries/repository/blob_info.query.graphql b/app/graphql/queries/repository/blob_info.query.graphql index 13966d022a93427ed955bae0731c9d927a825110..e320d8701903aa1d0f78e623a7f61dc1b41dd57a 100644 --- a/app/graphql/queries/repository/blob_info.query.graphql +++ b/app/graphql/queries/repository/blob_info.query.graphql @@ -36,6 +36,7 @@ query getBlobInfo( environmentFormattedExternalUrl environmentExternalUrlForRouteMap canModifyBlob + canModifyBlobWithWebIde canCurrentUserPushToBranch archived storedExternally diff --git a/app/graphql/types/repository/blob_type.rb b/app/graphql/types/repository/blob_type.rb index a5c9d6940ceb6a9e1831882ea0cf0d6049b51992..300dab726d4962c2360beee42741cf71cac1b2f5 100644 --- a/app/graphql/types/repository/blob_type.rb +++ b/app/graphql/types/repository/blob_type.rb @@ -127,6 +127,9 @@ class BlobType < BaseObject calls_gitaly: true, description: 'Whether the current user can modify the blob.' + field :can_modify_blob_with_web_ide, GraphQL::Types::Boolean, null: false, method: :can_modify_blob_with_web_ide?, + description: 'Whether the current user can modify the blob with Web IDE.' + field :can_current_user_push_to_branch, GraphQL::Types::Boolean, null: true, method: :can_current_user_push_to_branch?, description: 'Whether the current user can push to the branch.' diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 117b2ea6f80d56c4bf868f649ced2bdb99a035ab..d4c3cb5b5de79a52c5f809b01d1853aed1f703c5 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -76,10 +76,17 @@ def edit_blob_button(project = @project, ref = @ref, path = @path, options = {}) ) end + # Used for single file Web Editor, Delete and Replace UI actions. + # can_edit_tree checks if ref is on top of the branch. def can_modify_blob?(blob, project = @project, ref = @ref) !blob.stored_externally? && can_edit_tree?(project, ref) end + # Used for WebIDE editor where editing is possible even if ref is not on top of the branch. + def can_modify_blob_with_web_ide?(blob, project = @project) + !blob.stored_externally? && can_collaborate_with_project?(project) + end + def leave_edit_message _("Leave edit mode? All unsaved changes will be lost.") end diff --git a/app/presenters/blob_presenter.rb b/app/presenters/blob_presenter.rb index 778043955198f2986c71a2414e24c6b661117a4f..dcaf9de19978eed8d4c4b104d316ee26372af15e 100644 --- a/app/presenters/blob_presenter.rb +++ b/app/presenters/blob_presenter.rb @@ -140,6 +140,10 @@ def can_modify_blob? super(blob, project, commit_id) end + def can_modify_blob_with_web_ide? + super(blob, project) + end + def can_current_user_push_to_branch? return false unless current_user && project.repository.branch_exists?(commit_id) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 1249fe42b7a857d861da9af6573931b31eaf7e3c..1dfcb694fc082ba64cbf28828364a387c26bfc43 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -30753,6 +30753,7 @@ Returns [`RepositoryCodeownerValidation`](#repositorycodeownervalidation). | `blamePath` | [`String`](#string) | Web path to blob blame page. | | `canCurrentUserPushToBranch` | [`Boolean`](#boolean) | Whether the current user can push to the branch. | | `canModifyBlob` | [`Boolean`](#boolean) | Whether the current user can modify the blob. | +| `canModifyBlobWithWebIde` | [`Boolean!`](#boolean) | Whether the current user can modify the blob with Web IDE. | | `codeNavigationPath` | [`String`](#string) | Web path for code navigation. | | `codeOwners` | [`[UserCore!]`](#usercore) | List of code owners for the blob. | | `editBlobPath` | [`String`](#string) | Web path to edit the blob in the old-style editor. | diff --git a/ee/spec/frontend/vulnerabilities/mock_data.js b/ee/spec/frontend/vulnerabilities/mock_data.js index 22fbf1ed834c0a3dfc7473ff8b0e7df03280951c..fbf8abfbdcd26915c3793ba339689bee5ffedd2a 100644 --- a/ee/spec/frontend/vulnerabilities/mock_data.js +++ b/ee/spec/frontend/vulnerabilities/mock_data.js @@ -187,6 +187,7 @@ export const TEST_ALL_BLOBS_INFO_GRAPHQL_SUCCESS_RESPONSE = { environmentFormattedExternalUrl: null, environmentExternalUrlForRouteMap: null, canModifyBlob: false, + canModifyBlobWithWebIde: false, canCurrentUserPushToBranch: false, archived: false, storedExternally: null, diff --git a/spec/frontend/blob/components/blob_header_spec.js b/spec/frontend/blob/components/blob_header_spec.js index e7b2ee7494049a1d13601885fe643a1b8c49ec2a..3faa6e18d05ac49b0cfe4f30bb641c5afc8506f4 100644 --- a/spec/frontend/blob/components/blob_header_spec.js +++ b/spec/frontend/blob/components/blob_header_spec.js @@ -75,13 +75,15 @@ describe('Blob Header Default Actions', () => { it('renders the WebIdeLink component with the correct props', async () => { const { ideEditPath, editBlobPath, gitpodBlobUrl, pipelineEditorPath } = Blob; const showForkSuggestion = false; - await createComponent({ propsData: { showForkSuggestion } }); + const showWebIdeForkSuggestion = false; + await createComponent({ propsData: { showForkSuggestion, showWebIdeForkSuggestion } }); expect(findWebIdeLink().props()).toMatchObject({ showEditButton: true, editUrl: editBlobPath, webIdeUrl: ideEditPath, needsToFork: showForkSuggestion, + needsToForkWithWebIde: showWebIdeForkSuggestion, showPipelineEditorButton: Boolean(pipelineEditorPath), pipelineEditorUrl: pipelineEditorPath, gitpodUrl: gitpodBlobUrl, diff --git a/spec/frontend/repository/components/blob_content_viewer_spec.js b/spec/frontend/repository/components/blob_content_viewer_spec.js index 866655bb2f297c7e7b4d7b719347ac903923c6ca..078b11a91a9683a18e6d7dbc49845e1e76f80566 100644 --- a/spec/frontend/repository/components/blob_content_viewer_spec.js +++ b/spec/frontend/repository/components/blob_content_viewer_spec.js @@ -544,24 +544,27 @@ describe('Blob content viewer component', () => { }); it.each` - loggedIn | canModifyBlob | createMergeRequestIn | forkProject | showForkSuggestion - ${true} | ${false} | ${true} | ${true} | ${true} - ${false} | ${false} | ${true} | ${true} | ${false} - ${true} | ${true} | ${false} | ${true} | ${false} - ${true} | ${true} | ${true} | ${false} | ${false} + loggedIn | canModifyBlob | isUsingLfs | createMergeRequestIn | forkProject | showSingleFileEditorForkSuggestion + ${true} | ${true} | ${false} | ${true} | ${true} | ${false} + ${true} | ${false} | ${false} | ${true} | ${true} | ${true} + ${false} | ${false} | ${false} | ${true} | ${true} | ${false} + ${true} | ${false} | ${false} | ${false} | ${true} | ${false} + ${true} | ${false} | ${false} | ${true} | ${false} | ${false} + ${true} | ${false} | ${true} | ${true} | ${true} | ${false} `( 'shows/hides a fork suggestion according to a set of conditions', async ({ loggedIn, canModifyBlob, + isUsingLfs, createMergeRequestIn, forkProject, - showForkSuggestion, + showSingleFileEditorForkSuggestion, }) => { isLoggedIn.mockReturnValueOnce(loggedIn); await createComponent( { - blob: { ...simpleViewerMock, canModifyBlob }, + blob: { ...simpleViewerMock, canModifyBlob, storedExternally: isUsingLfs }, createMergeRequestIn, forkProject, }, @@ -571,7 +574,42 @@ describe('Blob content viewer component', () => { findBlobHeader().vm.$emit('edit', 'simple'); await nextTick(); - expect(findForkSuggestion().exists()).toBe(showForkSuggestion); + expect(findForkSuggestion().exists()).toBe(showSingleFileEditorForkSuggestion); + }, + ); + + it.each` + loggedIn | canModifyBlobWithWebIde | isUsingLfs | createMergeRequestIn | forkProject | showWebIdeForkSuggestion + ${true} | ${true} | ${false} | ${true} | ${true} | ${false} + ${true} | ${false} | ${false} | ${true} | ${true} | ${true} + ${false} | ${false} | ${false} | ${true} | ${true} | ${false} + ${true} | ${false} | ${false} | ${false} | ${true} | ${false} + ${true} | ${false} | ${false} | ${true} | ${false} | ${false} + ${true} | ${false} | ${true} | ${true} | ${true} | ${false} + `( + 'shows/hides a fork suggestion for WebIDE according to a set of conditions', + async ({ + loggedIn, + canModifyBlobWithWebIde, + isUsingLfs, + createMergeRequestIn, + forkProject, + showWebIdeForkSuggestion, + }) => { + isLoggedIn.mockReturnValueOnce(loggedIn); + await createComponent( + { + blob: { ...simpleViewerMock, canModifyBlobWithWebIde, storedExternally: isUsingLfs }, + createMergeRequestIn, + forkProject, + }, + mount, + ); + + findBlobHeader().vm.$emit('edit', 'ide'); + await nextTick(); + + expect(findForkSuggestion().exists()).toBe(showWebIdeForkSuggestion); }, ); }); diff --git a/spec/frontend/repository/mock_data.js b/spec/frontend/repository/mock_data.js index a5d374c234efe1a2bbbda9d193643bc4647ef982..aa31a12976951477213cb25505d6ff16308fbf81 100644 --- a/spec/frontend/repository/mock_data.js +++ b/spec/frontend/repository/mock_data.js @@ -21,6 +21,7 @@ export const simpleViewerMock = { environmentFormattedExternalUrl: '', environmentExternalUrlForRouteMap: '', canModifyBlob: true, + canModifyBlobWithWebIde: true, canCurrentUserPushToBranch: true, archived: false, storedExternally: false, diff --git a/spec/frontend/vue_shared/components/web_ide_link_spec.js b/spec/frontend/vue_shared/components/web_ide_link_spec.js index 84ecbb431ac404fbf4ba8c8dcb427dc802a2f625..6bf4cd99aa4c2715c329714dcd878750b7813e0a 100644 --- a/spec/frontend/vue_shared/components/web_ide_link_spec.js +++ b/spec/frontend/vue_shared/components/web_ide_link_spec.js @@ -173,7 +173,7 @@ describe('vue_shared/components/web_ide_link', () => { expectedActions: [ACTION_WEB_IDE_EDIT_FORK, ACTION_EDIT], }, { - props: { needsToFork: true }, + props: { needsToFork: true, needsToForkWithWebIde: true }, expectedActions: [ACTION_WEB_IDE_CONFIRM_FORK, ACTION_EDIT_CONFIRM_FORK], }, { @@ -342,7 +342,12 @@ describe('vue_shared/components/web_ide_link', () => { it.each(testActions)( 'emits the correct event when an action handler is called', ({ props, expectedEventPayload }) => { - createComponent({ ...props, needsToFork: true, disableForkModal: true }); + createComponent({ + ...props, + needsToFork: true, + needsToForkWithWebIde: true, + disableForkModal: true, + }); findDisclosureDropdownItems().at(0).props().item.handle(); @@ -351,7 +356,7 @@ describe('vue_shared/components/web_ide_link', () => { ); it.each(testActions)('renders the fork confirmation modal', ({ props }) => { - createComponent({ ...props, needsToFork: true }); + createComponent({ ...props, needsToFork: true, needsToForkWithWebIde: true }); expect(findForkConfirmModal().exists()).toBe(true); expect(findForkConfirmModal().props()).toEqual({ @@ -362,7 +367,10 @@ describe('vue_shared/components/web_ide_link', () => { }); it.each(testActions)('opens the modal when the button is clicked', async ({ props }) => { - createComponent({ ...props, needsToFork: true }, { mountFn: mountExtended }); + createComponent( + { ...props, needsToFork: true, needsToForkWithWebIde: true }, + { mountFn: mountExtended }, + ); findDisclosureDropdownItems().at(0).props().item.handle(); diff --git a/spec/graphql/types/repository/blob_type_spec.rb b/spec/graphql/types/repository/blob_type_spec.rb index 1c27b6fca503f00476764c5aec03fe7ab1c7db61..a76c51a92dcfdcddfbf34d56e75df05398c996af 100644 --- a/spec/graphql/types/repository/blob_type_spec.rb +++ b/spec/graphql/types/repository/blob_type_spec.rb @@ -42,6 +42,7 @@ :rich_viewer, :plain_data, :can_modify_blob, + :can_modify_blob_with_web_ide, :can_current_user_push_to_branch, :archived, :ide_edit_path, diff --git a/spec/presenters/blob_presenter_spec.rb b/spec/presenters/blob_presenter_spec.rb index e6c8b63f2346cb501dd7a7d23f55ca7866f2f5a6..128f3facdac6435140785bd8e3f389f217c0e0fe 100644 --- a/spec/presenters/blob_presenter_spec.rb +++ b/spec/presenters/blob_presenter_spec.rb @@ -119,6 +119,31 @@ end end + describe '#can_modify_blob_with_web_ide?' do + before do + allow(blob).to receive(:stored_externally?).and_return(false) + allow(presenter).to receive(:can_collaborate_with_project?).with(project).and_return(false) + end + + it { expect(presenter.can_modify_blob_with_web_ide?).to be_falsey } + + context 'when blob is stored externally' do + before do + allow(blob).to receive(:stored_externally?).and_return(true) + end + + it { expect(presenter.can_modify_blob_with_web_ide?).to be_falsey } + end + + context 'when user can collaborate with the project' do + before do + allow(presenter).to receive(:can_collaborate_with_project?).with(project).and_return(true) + end + + it { expect(presenter.can_modify_blob_with_web_ide?).to be_truthy } + end + end + describe '#can_current_user_push_to_branch?' do context 'when ref is a branch' do let(:ref) { 'feature' }