From e223953653de9906e6a66aac9ca30799c27290df Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 4 Feb 2025 15:42:32 +0100 Subject: [PATCH 1/6] Do not drill props to BlobButtonGroup Passing the whole blobInfo object to use different properties instead of passed down props. Changelog: changed EE: true --- .../header_area/blob_button_group.vue | 49 +++++------------ .../components/header_area/blob_controls.vue | 3 +- .../header_area/blob_overflow_menu.vue | 10 +--- ee/spec/frontend/repository/mock_data.js | 52 +++++++++++++++++++ .../header_area/blob_button_group_spec.js | 31 +++++------ 5 files changed, 84 insertions(+), 61 deletions(-) diff --git a/app/assets/javascripts/repository/components/header_area/blob_button_group.vue b/app/assets/javascripts/repository/components/header_area/blob_button_group.vue index c233bd2823da68..185f49d20e850c 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_button_group.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_button_group.vue @@ -36,34 +36,11 @@ export default { originalBranch: { default: '', }, - canModifyBlob: { - default: () => false, - }, - canModifyBlobWithWebIde: { - default: () => false, + blobInfo: { + default: () => {}, }, }, props: { - name: { - type: String, - required: true, - }, - path: { - type: String, - required: true, - }, - replacePath: { - type: String, - required: true, - }, - deletePath: { - type: String, - required: true, - }, - canPushToBranch: { - type: Boolean, - required: true, - }, isEmptyRepository: { type: Boolean, required: true, @@ -131,10 +108,10 @@ export default { return uniqueId('delete-modal'); }, replaceCommitMessage() { - return sprintf(__('Replace %{name}'), { name: this.name }); + return sprintf(__('Replace %{name}'), { name: this.blobInfo.name }); }, deleteModalCommitMessage() { - return sprintf(__('Delete %{name}'), { name: this.name }); + return sprintf(__('Delete %{name}'), { name: this.blobInfo.name }); }, canFork() { const { createMergeRequestIn, forkProject } = this.userPermissions; @@ -142,10 +119,10 @@ export default { return this.isLoggedIn && !this.isUsingLfs && createMergeRequestIn && forkProject; }, showSingleFileEditorForkSuggestion() { - return this.canFork && !this.canModifyBlob; + return this.canFork && !this.blobInfo.canModifyBlob; }, showWebIdeForkSuggestion() { - return this.canFork && !this.canModifyBlobWithWebIde; + return this.canFork && !this.blobInfo.canModifyBlobWithWebIde; }, showForkSuggestion() { return this.showSingleFileEditorForkSuggestion || this.showWebIdeForkSuggestion; @@ -168,8 +145,8 @@ export default { diff --git a/app/assets/javascripts/repository/components/header_area/blob_controls.vue b/app/assets/javascripts/repository/components/header_area/blob_controls.vue index 6d83dd15d72bb4..afea51f37ec58e 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_controls.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_controls.vue @@ -64,8 +64,7 @@ export default { }, provide() { return { - canModifyBlob: computed(() => this.blobInfo?.canModifyBlob ?? false), - canModifyBlobWithWebIde: computed(() => this.blobInfo?.canModifyBlobWithWebIde ?? false), + blobInfo: computed(() => this.blobInfo ?? {}), }; }, props: { diff --git a/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue b/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue index fa6954d011d94e..63735a03be64e8 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue @@ -21,11 +21,10 @@ export default { directives: { GlTooltipDirective, }, - inject: ['canModifyBlob', 'canModifyBlobWithWebIde'], + inject: ['blobInfo'], provide() { return { - canModifyBlob: computed(() => this.canModifyBlob), - canModifyBlobWithWebIde: computed(() => this.canModifyBlobWithWebIde), + blobInfo: computed(() => this.blobInfo ?? {}), }; }, props: { @@ -154,11 +153,6 @@ export default { > ({ })); const DEFAULT_PROPS = { - name: 'some name', - path: 'some/path', - replacePath: 'some/replace/path', - deletePath: 'some/delete/path', - canPushToBranch: true, isEmptyRepository: false, projectPath: 'some/project/path', isUsingLfs: true, @@ -32,8 +27,7 @@ const DEFAULT_PROPS = { const DEFAULT_INJECT = { targetBranch: 'master', originalBranch: 'master', - canModifyBlob: true, - canModifyBlobWithWebIde: true, + blobInfo: blobControlsDataMock.repository.blobs.nodes[0], }; describe('BlobButtonGroup component', () => { @@ -105,8 +99,9 @@ describe('BlobButtonGroup component', () => { it('renders component', () => { expect(wrapper.props()).toMatchObject({ - name: 'some name', - path: 'some/path', + isEmptyRepository: false, + isUsingLfs: true, + projectPath: 'some/project/path', }); }); @@ -141,7 +136,13 @@ describe('BlobButtonGroup component', () => { beforeEach(async () => { await createComponent({ props: { isUsingLfs: false }, - inject: { canModifyBlob: false, canModifyBlobWithWebIde: false }, + inject: { + blobInfo: { + ...blobControlsDataMock.repository.blobs.nodes[0], + canModifyBlob: false, + canModifyBlobWithWebIde: false, + }, + }, }); }); @@ -171,18 +172,18 @@ describe('BlobButtonGroup component', () => { it('renders UploadBlobModal', () => { expect(findUploadBlobModal().props()).toMatchObject({ - commitMessage: 'Replace some name', + commitMessage: 'Replace file.js', targetBranch: 'master', originalBranch: 'master', canPushCode: true, - path: 'some/path', - replacePath: 'some/replace/path', + path: 'some/file.js', + replacePath: 'some/replace/file.js', }); }); it('renders CommitChangesModal for delete', () => { expect(findDeleteBlobModal().props()).toMatchObject({ - commitMessage: 'Delete some name', + commitMessage: 'Delete file.js', targetBranch: 'master', originalBranch: 'master', canPushCode: true, -- GitLab From fccd81b69bafe31bc7c09cc3934fdd699407f6e6 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 4 Feb 2025 16:30:26 +0100 Subject: [PATCH 2/6] Use injected blobInfo instead of drilled props Laverage injected blobInfo in BlobOveflowMenu and BlobDefaultActionsGroup components. Changelog: other --- .../components/header_area/blob_controls.vue | 15 +---- .../blob_default_actions_group.vue | 41 +++++-------- .../header_area/blob_overflow_menu.vue | 61 ++----------------- .../header_area/blob_controls_spec.js | 23 ------- .../blob_default_actions_group_spec.js | 36 ++++++++--- .../header_area/blob_overflow_menu_spec.js | 26 +++----- spec/frontend/repository/mock_data.js | 4 +- 7 files changed, 60 insertions(+), 146 deletions(-) diff --git a/app/assets/javascripts/repository/components/header_area/blob_controls.vue b/app/assets/javascripts/repository/components/header_area/blob_controls.vue index afea51f37ec58e..8c112f38094986 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_controls.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_controls.vue @@ -23,7 +23,7 @@ import { FIND_FILE_BUTTON_CLICK } from '~/tracking/constants'; import { updateElementsVisibility } from '~/repository/utils/dom'; import blobControlsQuery from '~/repository/queries/blob_controls.query.graphql'; import { getRefType } from '~/repository/utils/ref_type'; -import { TEXT_FILE_TYPE } from '../../constants'; +import { TEXT_FILE_TYPE, DEFAULT_BLOB_INFO } from '../../constants'; import OverflowMenu from './blob_overflow_menu.vue'; export default { @@ -64,7 +64,7 @@ export default { }, provide() { return { - blobInfo: computed(() => this.blobInfo ?? {}), + blobInfo: computed(() => this.blobInfo ?? DEFAULT_BLOB_INFO.repository.blobs.nodes[0]), }; }, props: { @@ -226,22 +226,11 @@ export default { diff --git a/app/assets/javascripts/repository/components/header_area/blob_default_actions_group.vue b/app/assets/javascripts/repository/components/header_area/blob_default_actions_group.vue index 951d6d2dbfe6a6..357ececd937d49 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_default_actions_group.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_default_actions_group.vue @@ -22,20 +22,11 @@ export default { canDownloadCode: { default: true, }, + blobInfo: { + default: () => {}, + }, }, props: { - name: { - type: String, - required: true, - }, - path: { - type: String, - required: true, - }, - rawPath: { - type: String, - required: true, - }, activeViewerType: { type: String, required: true, @@ -56,16 +47,6 @@ export default { type: Boolean, required: true, }, - environmentName: { - type: String, - required: false, - default: null, - }, - environmentPath: { - type: String, - required: false, - default: null, - }, }, computed: { copyFileContentsItem() { @@ -81,7 +62,7 @@ export default { openRawItem() { return { text: i18n.btnRawTitle, - href: this.rawPath, + href: this.blobInfo.rawPath, extraAttrs: { target: '_blank', }, @@ -100,7 +81,7 @@ export default { environmentItem() { return { text: this.environmentTitle, - href: this.environmentPath, + href: this.blobInfo.environmentExternalUrlForRouteMap, extraAttrs: { target: '_blank', 'data-testid': 'environment', @@ -120,14 +101,20 @@ export default { return `[data-blob-hash="${this.blobHash}"]`; }, downloadUrl() { - return setUrlParams({ inline: false }, relativePathToAbsolute(this.rawPath, getBaseURL())); + return setUrlParams( + { inline: false }, + relativePathToAbsolute(this.blobInfo.rawPath, getBaseURL()), + ); }, showEnvironmentItem() { - return this.environmentName && this.environmentPath; + return ( + this.blobInfo.environmentFormattedExternalUrl && + this.blobInfo.environmentExternalUrlForRouteMap + ); }, environmentTitle() { return sprintf(s__('BlobViewer|View on %{environmentName}'), { - environmentName: this.environmentName, + environmentName: this.blobInfo.environmentFormattedExternalUrl, }); }, }, diff --git a/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue b/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue index 63735a03be64e8..72f47558a46c7b 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue @@ -28,59 +28,15 @@ export default { }; }, props: { - name: { - type: String, - required: true, - }, - archived: { - type: Boolean, - required: true, - }, projectPath: { type: String, required: true, }, - path: { - type: String, - required: true, - }, - rawPath: { - type: String, - required: true, - }, - replacePath: { - type: String, - required: true, - }, - webPath: { - type: String, - required: true, - }, - richViewer: { - type: Object, - required: false, - default: () => {}, - }, - simpleViewer: { - type: Object, - required: false, - default: () => {}, - }, isBinary: { type: Boolean, required: false, default: false, }, - environmentName: { - type: String, - required: false, - default: null, - }, - environmentPath: { - type: String, - required: false, - default: null, - }, isEmpty: { type: Boolean, required: false, @@ -96,10 +52,6 @@ export default { required: false, default: false, }, - canCurrentUserPushToBranch: { - type: Boolean, - required: true, - }, isUsingLfs: { type: Boolean, required: false, @@ -122,14 +74,16 @@ export default { return SIMPLE_BLOB_VIEWER; }, viewer() { - return this.activeViewerType === RICH_BLOB_VIEWER ? this.richViewer : this.simpleViewer; + return this.activeViewerType === RICH_BLOB_VIEWER + ? this.blobInfo.richViewer + : this.blobInfo.simpleViewer; }, hasRenderError() { return Boolean(this.viewer.renderError); }, environmentTitle() { return sprintf(s__('BlobViewer|View on %{environmentName}'), { - environmentName: this.environmentName, + environmentName: this.blobInfo.environmentFormattedExternalUrl, }); }, }, @@ -152,22 +106,17 @@ export default { text-sr-only > diff --git a/spec/frontend/repository/components/header_area/blob_controls_spec.js b/spec/frontend/repository/components/header_area/blob_controls_spec.js index ad98ce073cbb68..14539c594fd878 100644 --- a/spec/frontend/repository/components/header_area/blob_controls_spec.js +++ b/spec/frontend/repository/components/header_area/blob_controls_spec.js @@ -166,33 +166,10 @@ describe('Blob controls component', () => { expect(findOverflowMenu().exists()).toBe(true); expect(findOverflowMenu().props()).toEqual({ - name: 'file.js', projectPath: 'some/project', - path: 'some/file.js', - rawPath: 'https://testing.com/flightjs/flight/snippets/51/raw', isBinary: true, - environmentName: '', - environmentPath: '', isEmpty: false, overrideCopy: true, - archived: false, - replacePath: 'some/replace/file.js', - webPath: 'some/file.js', - canCurrentUserPushToBranch: true, - simpleViewer: { - __typename: 'BlobViewer', - renderError: null, - tooLarge: false, - type: 'simple', - fileType: 'rich', - }, - richViewer: { - __typename: 'BlobViewer', - renderError: 'too big file', - tooLarge: false, - type: 'rich', - fileType: 'rich', - }, isEmptyRepository: false, isUsingLfs: false, }); diff --git a/spec/frontend/repository/components/header_area/blob_default_actions_group_spec.js b/spec/frontend/repository/components/header_area/blob_default_actions_group_spec.js index 54930a587a9cf9..a0ab8e8e8183f8 100644 --- a/spec/frontend/repository/components/header_area/blob_default_actions_group_spec.js +++ b/spec/frontend/repository/components/header_area/blob_default_actions_group_spec.js @@ -1,10 +1,12 @@ import { shallowMount } from '@vue/test-utils'; import { GlDisclosureDropdownItem } from '@gitlab/ui'; import BlobDefaultActionsGroup from '~/repository/components/header_area/blob_default_actions_group.vue'; +import { blobControlsDataMock } from '../../mock_data'; const mockBlobHash = 'foo-bar'; const mockEnvironmentName = 'my.testing.environment'; const mockEnvironmentPath = 'https://my.testing.environment'; +const blobInfoMock = blobControlsDataMock.repository.blobs.nodes[0]; describe('Blob Default Actions Group', () => { let wrapper; @@ -12,9 +14,6 @@ describe('Blob Default Actions Group', () => { const createComponent = (props = {}, provide = {}) => { wrapper = shallowMount(BlobDefaultActionsGroup, { propsData: { - name: 'dummy.md', - path: 'foo/bar/dummy.md', - rawPath: 'https://testing.com/flightjs/flight/snippets/51/raw', blobHash: mockBlobHash, activeViewerType: 'simple', hasRenderError: false, @@ -27,6 +26,10 @@ describe('Blob Default Actions Group', () => { provide: { blobHash: mockBlobHash, canDownloadCode: true, + blobInfo: { + ...blobInfoMock, + ...provide.blobInfo, + }, ...provide, }, }); @@ -38,7 +41,8 @@ describe('Blob Default Actions Group', () => { const findCopyFileContentItem = () => findDropdownItemWithText('Copy file contents'); const findViewRawItem = () => findDropdownItemWithText('Open raw'); const findDownloadItem = () => findDropdownItemWithText('Download'); - const findEnvironmentItem = () => findDropdownItemWithText(`View on ${mockEnvironmentName}`); + const findEnvironmentItem = () => + findDropdownItemWithText(`View on ${blobInfoMock.environmentFormattedExternalUrl}`); beforeEach(() => { createComponent(); @@ -106,7 +110,16 @@ describe('Blob Default Actions Group', () => { 'when environmentName is $environmentName and environmentPath is $environmentPath', ({ environmentName, environmentPath, isVisible }) => { it(`${isVisible ? 'renders' : 'does not render'} the button`, () => { - createComponent({ environmentName, environmentPath }); + createComponent( + {}, + { + blobInfo: { + ...blobInfoMock, + environmentFormattedExternalUrl: environmentName, + environmentExternalUrlForRouteMap: environmentPath, + }, + }, + ); expect(findEnvironmentItem()).toEqual(isVisible); }); @@ -114,10 +127,15 @@ describe('Blob Default Actions Group', () => { ); it('renders the correct props', () => { - createComponent({ - environmentName: mockEnvironmentName, - environmentPath: mockEnvironmentPath, - }); + createComponent( + {}, + { + blobInfo: { + environmentFormattedExternalUrl: mockEnvironmentName, + environmentExternalUrlForRouteMap: mockEnvironmentPath, + }, + }, + ); expect(findEnvironmentItem().props('item')).toMatchObject({ text: `View on ${mockEnvironmentName}`, diff --git a/spec/frontend/repository/components/header_area/blob_overflow_menu_spec.js b/spec/frontend/repository/components/header_area/blob_overflow_menu_spec.js index 6a36820bd735cf..d3d207525b4ffc 100644 --- a/spec/frontend/repository/components/header_area/blob_overflow_menu_spec.js +++ b/spec/frontend/repository/components/header_area/blob_overflow_menu_spec.js @@ -23,23 +23,11 @@ describe('Blob Overflow Menu', () => { wrapper = shallowMountExtended(BlobOverflowMenu, { router, provide: { - canModifyBlob: true, - canModifyBlobWithWebIde: true, + blobInfo: blobControlsDataMock.repository.blobs.nodes[0], ...provided, }, propsData: { - path: blobControlsDataMock.repository.blobs.nodes[0].path, - rawPath: blobControlsDataMock.repository.blobs.nodes[0].rawPath, projectPath, - richViewer: blobControlsDataMock.repository.blobs.nodes[0].richViewer, - simpleViewer: blobControlsDataMock.repository.blobs.nodes[0].simpleViewer, - name: blobControlsDataMock.repository.blobs.nodes[0].name, - isBinary: blobControlsDataMock.repository.blobs.nodes[0].binary, - archived: blobControlsDataMock.repository.blobs.nodes[0].archived, - replacePath: blobControlsDataMock.repository.blobs.nodes[0].replacePath, - webPath: blobControlsDataMock.repository.blobs.nodes[0].webPath, - canCurrentUserPushToBranch: - blobControlsDataMock.repository.blobs.nodes[0].canCurrentUserPushToBranch, ...propsData, }, stub: { @@ -87,9 +75,15 @@ describe('Blob Overflow Menu', () => { }); it('does not render when blob is archived', () => { - createComponent({ - archived: true, - }); + createComponent( + {}, + { + blobInfo: { + ...blobControlsDataMock.repository.blobs.nodes[0], + archived: true, + }, + }, + ); expect(findBlobButtonGroup().exists()).toBe(false); }); diff --git a/spec/frontend/repository/mock_data.js b/spec/frontend/repository/mock_data.js index 585c59ec34e37e..e4f023a6a22778 100644 --- a/spec/frontend/repository/mock_data.js +++ b/spec/frontend/repository/mock_data.js @@ -106,8 +106,8 @@ export const blobControlsDataMock = { path: 'some/file.js', storedExternally: false, externalStorage: 'https://external-storage', - environmentFormattedExternalUrl: '', - environmentExternalUrlForRouteMap: '', + environmentFormattedExternalUrl: 'my.testing.environment', + environmentExternalUrlForRouteMap: 'https://my.testing.environment', rawPath: 'https://testing.com/flightjs/flight/snippets/51/raw', rawTextBlob: 'Example raw text content', archived: false, -- GitLab From 3bb162dc526f61f1d49150235d5acfe57d3fdf25 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 4 Feb 2025 16:45:43 +0100 Subject: [PATCH 3/6] Provide a default value for injected infoBlob --- .../repository/components/header_area/blob_button_group.vue | 2 +- .../components/header_area/blob_default_actions_group.vue | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/repository/components/header_area/blob_button_group.vue b/app/assets/javascripts/repository/components/header_area/blob_button_group.vue index 185f49d20e850c..2e7d45c0216c63 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_button_group.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_button_group.vue @@ -37,7 +37,7 @@ export default { default: '', }, blobInfo: { - default: () => {}, + default: () => DEFAULT_BLOB_INFO.repository.blobs.nodes[0], }, }, props: { diff --git a/app/assets/javascripts/repository/components/header_area/blob_default_actions_group.vue b/app/assets/javascripts/repository/components/header_area/blob_default_actions_group.vue index 357ececd937d49..61c54847a7ef2c 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_default_actions_group.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_default_actions_group.vue @@ -2,6 +2,7 @@ import { GlDisclosureDropdownItem, GlDisclosureDropdownGroup } from '@gitlab/ui'; import { sprintf, s__, __ } from '~/locale'; import { setUrlParams, relativePathToAbsolute, getBaseURL } from '~/lib/utils/url_utility'; +import { DEFAULT_BLOB_INFO } from '~/repository/constants'; export const i18n = { btnCopyContentsTitle: __('Copy file contents'), @@ -23,7 +24,7 @@ export default { default: true, }, blobInfo: { - default: () => {}, + default: () => DEFAULT_BLOB_INFO.repository.blobs.nodes[0], }, }, props: { -- GitLab From 9900365902e949b12119d9b43c630a4e4a1e7e42 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 4 Feb 2025 17:17:55 +0100 Subject: [PATCH 4/6] Add alert when actions from BloButtonGroup are possible in fork This is an alternative notification to ForkSuggestion that was presented for these action inside the BlobContentViewer. --- .../header_area/blob_button_group.vue | 24 +++++++++---- .../queries/blob_controls.query.graphql | 3 ++ ee/spec/frontend/repository/mock_data.js | 3 ++ .../header_area/blob_button_group_spec.js | 36 +++++++++++++------ spec/frontend/repository/mock_data.js | 3 ++ 5 files changed, 53 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/repository/components/header_area/blob_button_group.vue b/app/assets/javascripts/repository/components/header_area/blob_button_group.vue index 2e7d45c0216c63..433a6f011e2cd0 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_button_group.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_button_group.vue @@ -2,7 +2,7 @@ import { GlDisclosureDropdownItem, GlDisclosureDropdownGroup } from '@gitlab/ui'; import { uniqueId } from 'lodash'; import { sprintf, __ } from '~/locale'; -import { createAlert } from '~/alert'; +import { createAlert, VARIANT_INFO } from '~/alert'; import { isLoggedIn } from '~/lib/utils/common_utils'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import projectInfoQuery from 'ee_else_ce/repository/queries/project_info.query.graphql'; @@ -18,6 +18,11 @@ export default { replace: __('Replace'), delete: __('Delete'), fetchError: __('An error occurred while fetching lock information, please try again.'), + forkSuggestion: __( + 'You can’t edit files directly in this project. Fork this project and submit a merge request with your changes.', + ), + fork: __('Fork'), + cancel: __('Cancel'), }, replaceBlobModalId: REPLACE_BLOB_MODAL_ID, components: { @@ -89,8 +94,6 @@ export default { text: this.$options.i18n.replace, extraAttrs: { 'data-testid': 'replace', - // a temporary solution before resolving https://gitlab.com/gitlab-org/gitlab/-/issues/450774#note_2319974833 - disabled: this.showForkSuggestion, }, }; }, @@ -99,8 +102,6 @@ export default { text: this.$options.i18n.delete, extraAttrs: { 'data-testid': 'delete', - // a temporary solution before resolving https://gitlab.com/gitlab-org/gitlab/-/issues/450774#note_2319974833 - disabled: this.showForkSuggestion, }, }; }, @@ -131,7 +132,18 @@ export default { methods: { showModal(modalId) { if (this.showForkSuggestion) { - this.$emit('fork', 'view'); + const alert = createAlert({ + message: this.$options.i18n.forkSuggestion, + variant: VARIANT_INFO, + primaryButton: { + text: this.$options.i18n.fork, + link: this.blobInfo.forkAndViewPath, + }, + secondaryButton: { + text: this.$options.i18n.cancel, + clickHandler: () => alert.dismiss(), + }, + }); return; } diff --git a/app/assets/javascripts/repository/queries/blob_controls.query.graphql b/app/assets/javascripts/repository/queries/blob_controls.query.graphql index 9c50ba6236b9df..5751e39133668d 100644 --- a/app/assets/javascripts/repository/queries/blob_controls.query.graphql +++ b/app/assets/javascripts/repository/queries/blob_controls.query.graphql @@ -26,6 +26,9 @@ query getBlobControls($projectPath: ID!, $filePath: String!, $ref: String!, $ref canCurrentUserPushToBranch canModifyBlob canModifyBlobWithWebIde + ideForkAndEditPath + forkAndEditPath + forkAndViewPath simpleViewer { __typename fileType diff --git a/ee/spec/frontend/repository/mock_data.js b/ee/spec/frontend/repository/mock_data.js index 52efd94c53237a..15380f309d255d 100644 --- a/ee/spec/frontend/repository/mock_data.js +++ b/ee/spec/frontend/repository/mock_data.js @@ -226,6 +226,9 @@ export const blobControlsDataMock = { canCurrentUserPushToBranch: true, canModifyBlob: true, canModifyBlobWithWebIde: true, + ideForkAndEditPath: 'ide/fork/edit/path', + forkAndEditPath: 'fork/edit/path', + forkAndViewPath: 'fork/view/path', simpleViewer: { __typename: 'BlobViewer', collapsed: false, diff --git a/spec/frontend/repository/components/header_area/blob_button_group_spec.js b/spec/frontend/repository/components/header_area/blob_button_group_spec.js index 3eb6e30dcc9468..8d703e4ec56f46 100644 --- a/spec/frontend/repository/components/header_area/blob_button_group_spec.js +++ b/spec/frontend/repository/components/header_area/blob_button_group_spec.js @@ -149,23 +149,39 @@ describe('BlobButtonGroup component', () => { it('does not trigger the UploadBlobModal from the replace item', () => { findReplaceItem().vm.$emit('action'); - expect(findReplaceItem().props('item')).toMatchObject({ - extraAttrs: { disabled: true }, - }); - expect(showUploadBlobModalMock).not.toHaveBeenCalled(); - expect(wrapper.emitted().fork).toHaveLength(1); + expect(createAlert).toHaveBeenCalledWith({ + message: + 'You can’t edit files directly in this project. Fork this project and submit a merge request with your changes.', + primaryButton: { + link: 'fork/view/path', + text: 'Fork', + }, + secondaryButton: { + clickHandler: expect.any(Function), + text: 'Cancel', + }, + variant: 'info', + }); }); it('does not trigger the DeleteBlobModal from the delete item', () => { findDeleteItem().vm.$emit('action'); - expect(findDeleteItem().props('item')).toMatchObject({ - extraAttrs: { disabled: true }, - }); - expect(showDeleteBlobModalMock).not.toHaveBeenCalled(); - expect(wrapper.emitted().fork).toHaveLength(1); + expect(createAlert).toHaveBeenCalledWith({ + message: + 'You can’t edit files directly in this project. Fork this project and submit a merge request with your changes.', + primaryButton: { + link: 'fork/view/path', + text: 'Fork', + }, + secondaryButton: { + clickHandler: expect.any(Function), + text: 'Cancel', + }, + variant: 'info', + }); }); }); }); diff --git a/spec/frontend/repository/mock_data.js b/spec/frontend/repository/mock_data.js index e4f023a6a22778..dbac72845bd29a 100644 --- a/spec/frontend/repository/mock_data.js +++ b/spec/frontend/repository/mock_data.js @@ -116,6 +116,9 @@ export const blobControlsDataMock = { canCurrentUserPushToBranch: true, canModifyBlob: true, canModifyBlobWithWebIde: true, + ideForkAndEditPath: 'ide/fork/edit/path', + forkAndEditPath: 'fork/edit/path', + forkAndViewPath: 'fork/view/path', simpleViewer: { __typename: 'BlobViewer', collapsed: false, -- GitLab From e69e40a4255fe85a4dccb23f832cca2bf2d1d096 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 11 Feb 2025 15:07:35 +0100 Subject: [PATCH 5/6] Extract showForkSuggestion to a util function --- .../repository/utils/fork_suggestion_utils.js | 73 +++++++++ .../utils/fork_suggestion_utils_spec.js | 148 ++++++++++++++++++ 2 files changed, 221 insertions(+) create mode 100644 app/assets/javascripts/repository/utils/fork_suggestion_utils.js create mode 100644 spec/frontend/repository/utils/fork_suggestion_utils_spec.js diff --git a/app/assets/javascripts/repository/utils/fork_suggestion_utils.js b/app/assets/javascripts/repository/utils/fork_suggestion_utils.js new file mode 100644 index 00000000000000..2cbc76118f52c8 --- /dev/null +++ b/app/assets/javascripts/repository/utils/fork_suggestion_utils.js @@ -0,0 +1,73 @@ +import { isLoggedIn } from '~/lib/utils/common_utils'; +import { createAlert, VARIANT_INFO } from '~/alert'; +import { __ } from '~/locale'; + +export function showForkSuggestionAlert(forkAndViewPath) { + const i18n = { + forkSuggestion: __( + "You can't edit files directly in this project. Fork this project and submit a merge request with your changes.", + ), + fork: __('Fork'), + cancel: __('Cancel'), + }; + + return createAlert({ + message: i18n.forkSuggestion, + variant: VARIANT_INFO, + primaryButton: { + text: i18n.fork, + link: forkAndViewPath, + }, + secondaryButton: { + text: i18n.cancel, + clickHandler: (alert) => alert.dismiss(), + }, + }); +} + +/** + * Checks if the user can fork the project + * @param {Object} userPermissions - User permissions object + * @param {boolean} isUsingLfs - Whether the project is using LFS + * @returns {boolean} + */ +export const canFork = (userPermissions, isUsingLfs) => { + const { createMergeRequestIn, forkProject } = userPermissions; + return isLoggedIn() && !isUsingLfs && createMergeRequestIn && forkProject; +}; + +/** + * Checks if the fork suggestion should be shown for single file editor + * @param {Object} userPermissions - User permissions object + * @param {boolean} isUsingLfs - Whether the project is using LFS + * @param {boolean} canModifyBlob - Whether the user can modify the blob + * @returns {boolean} + */ +export const showSingleFileEditorForkSuggestion = (userPermissions, isUsingLfs, canModifyBlob) => { + return canFork(userPermissions, isUsingLfs) && !canModifyBlob; +}; + +/** + * Checks if the fork suggestion should be shown for Web IDE + * @param {Object} userPermissions - User permissions object + * @param {boolean} isUsingLfs - Whether the project is using LFS + * @param {boolean} canModifyBlobWithWebIde - Whether the user can modify the blob with Web IDE + * @returns {boolean} + */ +export const showWebIdeForkSuggestion = (userPermissions, isUsingLfs, canModifyBlobWithWebIde) => { + return canFork(userPermissions, isUsingLfs) && !canModifyBlobWithWebIde; +}; + +/** + * Checks if the fork suggestion should be shown + * @param {Object} userPermissions - User permissions object + * @param {boolean} isUsingLfs - Whether the project is using LFS + * @param {Object} blobInfo - blobInfo object, including canModifyBlob and canModifyBlobWithWebIde + * @returns {boolean} + */ +export const showForkSuggestion = (userPermissions, isUsingLfs, blobInfo) => { + return ( + showSingleFileEditorForkSuggestion(userPermissions, isUsingLfs, blobInfo.canModifyBlob) || + showWebIdeForkSuggestion(userPermissions, isUsingLfs, blobInfo.canModifyBlobWithWebIde) + ); +}; diff --git a/spec/frontend/repository/utils/fork_suggestion_utils_spec.js b/spec/frontend/repository/utils/fork_suggestion_utils_spec.js new file mode 100644 index 00000000000000..d111f65e078e68 --- /dev/null +++ b/spec/frontend/repository/utils/fork_suggestion_utils_spec.js @@ -0,0 +1,148 @@ +import { createAlert, VARIANT_INFO } from '~/alert'; +import * as commonUtils from '~/lib/utils/common_utils'; +import { + showForkSuggestionAlert, + canFork, + showSingleFileEditorForkSuggestion, + showWebIdeForkSuggestion, + showForkSuggestion, +} from '~/repository/utils/fork_suggestion_utils'; + +jest.mock('~/alert'); +jest.mock('~/lib/utils/common_utils'); + +describe('forkSuggestionUtils', () => { + let userPermissions; + const createUserPermissions = (createMergeRequestIn = true, forkProject = true) => ({ + createMergeRequestIn, + forkProject, + }); + + beforeEach(() => { + commonUtils.isLoggedIn.mockReturnValue(true); + userPermissions = createUserPermissions(); + }); + + describe('canFork', () => { + it('returns true when all conditions are met', () => { + expect(canFork(userPermissions, false)).toBe(true); + }); + + it('returns false when user is not logged in', () => { + commonUtils.isLoggedIn.mockReturnValue(false); + expect(canFork(userPermissions, false)).toBe(false); + }); + + it('returns false when project is using LFS', () => { + expect(canFork(userPermissions, true)).toBe(false); + }); + + it('returns false when user cannot create merge request', () => { + userPermissions = createUserPermissions(false, true); + expect(canFork(userPermissions, false)).toBe(false); + }); + + it('returns false when user cannot fork project', () => { + userPermissions = createUserPermissions(true, false); + expect(canFork(userPermissions, false)).toBe(false); + }); + }); + + describe('showSingleFileEditorForkSuggestion', () => { + it('returns true when user can fork but cannot modify blob', () => { + expect(showSingleFileEditorForkSuggestion(userPermissions, false, false)).toBe(true); + }); + + it('returns false when user can fork and can modify blob', () => { + expect(showSingleFileEditorForkSuggestion(userPermissions, false, true)).toBe(false); + }); + }); + + describe('showWebIdeForkSuggestion', () => { + it('returns true when user can fork but cannot modify blob with Web IDE', () => { + expect(showWebIdeForkSuggestion(userPermissions, false, false)).toBe(true); + }); + + it('returns false when user can fork and can modify blob with Web IDE', () => { + expect(showWebIdeForkSuggestion(userPermissions, false, true)).toBe(false); + }); + }); + + describe('showForkSuggestion', () => { + it('returns true when single file editor fork suggestion is true', () => { + expect( + showForkSuggestion(userPermissions, false, { + canModifyBlob: false, + canModifyBlobWithWebIde: true, + }), + ).toBe(true); + }); + + it('returns true when Web IDE fork suggestion is true', () => { + expect( + showForkSuggestion(userPermissions, false, { + canModifyBlob: true, + canModifyBlobWithWebIde: false, + }), + ).toBe(true); + }); + + it('returns false when both fork suggestions are false', () => { + expect( + showForkSuggestion(userPermissions, false, { + canModifyBlob: true, + canModifyBlobWithWebIde: true, + }), + ).toBe(false); + }); + + it('returns false when user cannot fork', () => { + commonUtils.isLoggedIn.mockReturnValue(false); + expect( + showForkSuggestion(userPermissions, false, { + canModifyBlob: false, + canModifyBlobWithWebIde: false, + }), + ).toBe(false); + }); + }); + + describe('showForkSuggestionAlert', () => { + const forkAndViewPath = '/path/to/fork'; + + beforeEach(() => { + createAlert.mockClear(); + }); + + it('calls createAlert with correct parameters', () => { + showForkSuggestionAlert(forkAndViewPath); + + expect(createAlert).toHaveBeenCalledTimes(1); + expect(createAlert).toHaveBeenCalledWith({ + message: + "You can't edit files directly in this project. Fork this project and submit a merge request with your changes.", + variant: VARIANT_INFO, + primaryButton: { + text: 'Fork', + link: forkAndViewPath, + }, + secondaryButton: { + text: 'Cancel', + clickHandler: expect.any(Function), + }, + }); + }); + + it('secondary button click handler dismisses the alert', () => { + const mockAlert = { dismiss: jest.fn() }; + createAlert.mockReturnValue(mockAlert); + + showForkSuggestionAlert(forkAndViewPath); + + const secondaryButtonClickHandler = createAlert.mock.calls[0][0].secondaryButton.clickHandler; + secondaryButtonClickHandler(mockAlert); + + expect(mockAlert.dismiss).toHaveBeenCalledTimes(1); + }); + }); +}); -- GitLab From 50758f187fcfb52273d341c6bffb8e07e4fff92c Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Thu, 6 Feb 2025 19:25:49 +0100 Subject: [PATCH 6/6] Extract Delete file to a separate component --- .../header_area/blob_button_group.vue | 116 +++++------------- .../header_area/blob_delete_file_group.vue | 102 +++++++++++++++ .../header_area/blob_overflow_menu.vue | 63 +++++++++- locale/gitlab.pot | 9 ++ 4 files changed, 205 insertions(+), 85 deletions(-) create mode 100644 app/assets/javascripts/repository/components/header_area/blob_delete_file_group.vue diff --git a/app/assets/javascripts/repository/components/header_area/blob_button_group.vue b/app/assets/javascripts/repository/components/header_area/blob_button_group.vue index 433a6f011e2cd0..af7720374e348a 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_button_group.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_button_group.vue @@ -1,39 +1,33 @@ + + diff --git a/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue b/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue index 72f47558a46c7b..9f34dd7f843e08 100644 --- a/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue +++ b/app/assets/javascripts/repository/components/header_area/blob_overflow_menu.vue @@ -2,13 +2,20 @@ import { GlDisclosureDropdown, GlTooltipDirective } from '@gitlab/ui'; import { computed } from 'vue'; import { sprintf, s__, __ } from '~/locale'; +import { createAlert } from '~/alert'; import { isLoggedIn } from '~/lib/utils/common_utils'; import { SIMPLE_BLOB_VIEWER, RICH_BLOB_VIEWER } from '~/blob/components/constants'; +import { DEFAULT_BLOB_INFO } from '~/repository/constants'; +import getRefMixin from '~/repository/mixins/get_ref'; +import projectInfoQuery from 'ee_else_ce/repository/queries/project_info.query.graphql'; import BlobDefaultActionsGroup from './blob_default_actions_group.vue'; import BlobButtonGroup from './blob_button_group.vue'; +import BlobDeleteFileGroup from './blob_delete_file_group.vue'; export const i18n = { dropdownLabel: __('Actions'), + delete: __('Delete file'), + fetchError: __('An error occurred while fetching lock information, please try again.'), }; export default { @@ -17,11 +24,23 @@ export default { GlDisclosureDropdown, BlobDefaultActionsGroup, BlobButtonGroup, + BlobDeleteFileGroup, }, directives: { GlTooltipDirective, }, - inject: ['blobInfo'], + mixins: [getRefMixin], + inject: { + targetBranch: { + default: '', + }, + originalBranch: { + default: '', + }, + blobInfo: { + default: () => DEFAULT_BLOB_INFO.repository.blobs.nodes[0], + }, + }, provide() { return { blobInfo: computed(() => this.blobInfo ?? {}), @@ -58,12 +77,42 @@ export default { default: false, }, }, + apollo: { + // eslint-disable-next-line @gitlab/vue-no-undef-apollo-properties + projectInfo: { + query: projectInfoQuery, + variables() { + return { + projectPath: this.projectPath, + }; + }, + update({ project }) { + this.pathLocks = project?.pathLocks || DEFAULT_BLOB_INFO.pathLocks; + this.userPermissions = project?.userPermissions; + }, + error() { + createAlert({ message: this.$options.i18n.fetchError }); + }, + }, + }, data() { return { + userPermissions: DEFAULT_BLOB_INFO.userPermissions, isLoggedIn: isLoggedIn(), }; }, computed: { + isLoading() { + return this.$apollo?.queries.projectInfo.loading; + }, + deleteFileItem() { + return { + text: this.$options.i18n.delete, + extraAttrs: { + 'data-testid': 'delete', + }, + }; + }, activeViewerType() { if (this.$route?.query?.plain !== '1') { const richViewer = document.querySelector('.blob-viewer[data-type="rich"]'); @@ -107,9 +156,12 @@ export default { > + diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c85a2c611175a7..cffebf914d52ec 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19162,6 +19162,9 @@ msgstr "" msgid "Delete epic" msgstr "" +msgid "Delete file" +msgstr "" + msgid "Delete group" msgstr "" @@ -47705,6 +47708,9 @@ msgstr "" msgid "Replace all labels" msgstr "" +msgid "Replace file" +msgstr "" + msgid "Replaced all labels with %{label_references} %{label_text}." msgstr "" @@ -66146,6 +66152,9 @@ msgstr "" msgid "You can't approve because you added one or more commits to this merge request." msgstr "" +msgid "You can't edit files directly in this project. Fork this project and submit a merge request with your changes." +msgstr "" + msgid "You can't follow more than %{limit} users. To follow more users, unfollow some others." msgstr "" -- GitLab