From 72023d18c7099da72eac5f43a488d40e18611936 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 11 Feb 2025 15:07:35 +0100 Subject: [PATCH 1/3] 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 b70a282939089c6c06ccec752a06f77416b4ecd7 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Tue, 11 Feb 2025 19:11:30 +0100 Subject: [PATCH 2/3] Update gitlab.pot file --- locale/gitlab.pot | 3 +++ 1 file changed, 3 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7665952cf708be..bb233f6ee2eaf3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -66395,6 +66395,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 From 229c8843e3c283ba4a4edcdf1d71543d9c8565c2 Mon Sep 17 00:00:00 2001 From: psjakubowska Date: Wed, 12 Feb 2025 15:24:32 +0100 Subject: [PATCH 3/3] Address review feedback --- .../javascripts/repository/utils/fork_suggestion_utils.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/repository/utils/fork_suggestion_utils.js b/app/assets/javascripts/repository/utils/fork_suggestion_utils.js index 2cbc76118f52c8..2e5b22a1d9d39b 100644 --- a/app/assets/javascripts/repository/utils/fork_suggestion_utils.js +++ b/app/assets/javascripts/repository/utils/fork_suggestion_utils.js @@ -11,7 +11,7 @@ export function showForkSuggestionAlert(forkAndViewPath) { cancel: __('Cancel'), }; - return createAlert({ + const alert = createAlert({ message: i18n.forkSuggestion, variant: VARIANT_INFO, primaryButton: { @@ -20,9 +20,11 @@ export function showForkSuggestionAlert(forkAndViewPath) { }, secondaryButton: { text: i18n.cancel, - clickHandler: (alert) => alert.dismiss(), + clickHandler: () => alert.dismiss(), }, }); + + return alert; } /** -- GitLab