diff --git a/app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx b/app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx index d5eaf80dc941..8db9b910c9b1 100644 --- a/app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx +++ b/app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx @@ -775,6 +775,9 @@ const PackageIcon = importSvg( const ModuleIcon = importSvg( async () => import("../__assets__/icons/ads/module.svg"), ); +const CreateModuleIcon = importSvg( + async () => import("../__assets__/icons/ads/create-module.svg"), +); const WorkflowsIcon = importSvg( async () => import("../__assets__/icons/ads/workflows.svg"), ); @@ -1187,6 +1190,7 @@ const ICON_LOOKUP = { "contract-right-line": ContractRight, "copy-control": CopyIcon, "copy2-control": Copy2Icon, + "create-module": CreateModuleIcon, "cut-control": CutIcon, "dashboard-line": DashboardLineIcon, "database-2-line": Database2Line, diff --git a/app/client/packages/design-system/ads/src/__assets__/icons/ads/create-module.svg b/app/client/packages/design-system/ads/src/__assets__/icons/ads/create-module.svg new file mode 100644 index 000000000000..1fb829ae250d --- /dev/null +++ b/app/client/packages/design-system/ads/src/__assets__/icons/ads/create-module.svg @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/app/client/packages/design-system/widgets/src/components/Sidebar/src/SidebarContent.tsx b/app/client/packages/design-system/widgets/src/components/Sidebar/src/SidebarContent.tsx index 7f88187391f6..9c88ff3cf81a 100644 --- a/app/client/packages/design-system/widgets/src/components/Sidebar/src/SidebarContent.tsx +++ b/app/client/packages/design-system/widgets/src/components/Sidebar/src/SidebarContent.tsx @@ -46,6 +46,7 @@ const _SidebarContent = ( )} {!isMobile && ( + + + + Copy below SSH key and paste it in your{" "} + {!!repositorySettingsUrl && value.gitProvider !== "others" ? ( + + repository settings. + + ) : ( + "repository settings." + )}{" "} + Now, give write access to it. + + + + + + + {!loading ? ( + + + {sshKeyType} + {sshKeyPair} + {!connectLoading && ( + + )} + + ) : ( + + )} + + {value?.gitProvider !== "others" && ( + + + + {createMessage(HOW_TO_ADD_DEPLOY_KEY)} + + + + + + )} + + + + {createMessage(CONSENT_ADDED_DEPLOY_KEY)} + +  * + + + + + ); +} + +export default AddDeployKey; diff --git a/app/client/src/git/components/ConnectModal/ChooseGitProvider.test.tsx b/app/client/src/git/components/ConnectModal/ChooseGitProvider.test.tsx new file mode 100644 index 000000000000..8de7501e9602 --- /dev/null +++ b/app/client/src/git/components/ConnectModal/ChooseGitProvider.test.tsx @@ -0,0 +1,234 @@ +/* eslint-disable react-perf/jsx-no-new-object-as-prop */ +import React from "react"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { GIT_DEMO_GIF } from "./constants"; +import "@testing-library/jest-dom"; +import ChooseGitProvider, { type GitProvider } from "./ChooseGitProvider"; +import { BrowserRouter as Router } from "react-router-dom"; + +jest.mock("utils/hooks/useDeviceDetect", () => ({ + useIsMobileDevice: jest.fn(() => false), +})); + +const defaultProps = { + artifactId: "123", + artifactType: "application", + onChange: jest.fn(), + onImportFromCalloutLinkClick: jest.fn(), + value: { + gitProvider: undefined as GitProvider | undefined, + gitEmptyRepoExists: "", + gitExistingRepoExists: false, + }, + isImport: false, + canCreateNewArtifact: true, +}; + +describe("ChooseGitProvider Component", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("renders the component and initial fields", () => { + render(); + expect(screen.getByText("Choose a Git provider")).toBeInTheDocument(); + expect( + screen.getByText("i. To begin with, choose your Git service provider"), + ).toBeInTheDocument(); + + // Provider radios + expect( + screen.getByTestId("t--git-provider-radio-github"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("t--git-provider-radio-gitlab"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("t--git-provider-radio-bitbucket"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("t--git-provider-radio-others"), + ).toBeInTheDocument(); + }); + + it("allows selecting a git provider and updates state via onChange", () => { + const onChange = jest.fn(); + + render(); + + const githubRadio = screen.getByTestId("t--git-provider-radio-github"); + + fireEvent.click(githubRadio); + expect(onChange).toHaveBeenCalledWith({ gitProvider: "github" }); + }); + + it("disables the second question (empty repo) if no git provider selected", () => { + render(); + // The empty repo radios should be disabled initially + const yesRadio = screen.getByTestId( + "t--existing-empty-repo-yes", + ) as HTMLInputElement; + const noRadio = screen.getByTestId( + "t--existing-empty-repo-no", + ) as HTMLInputElement; + + expect(yesRadio).toBeDisabled(); + expect(noRadio).toBeDisabled(); + }); + + it("enables empty repo question after provider is selected", () => { + const onChange = jest.fn(); + + render( + , + ); + + const yesRadio = screen.getByTestId( + "t--existing-empty-repo-yes", + ) as HTMLInputElement; + const noRadio = screen.getByTestId( + "t--existing-empty-repo-no", + ) as HTMLInputElement; + + expect(yesRadio).not.toBeDisabled(); + expect(noRadio).not.toBeDisabled(); + }); + + it("calls onChange when empty repo question changes", () => { + const onChange = jest.fn(); + + render( + , + ); + fireEvent.click(screen.getByTestId("t--existing-empty-repo-no")); + expect(onChange).toHaveBeenCalledWith({ gitEmptyRepoExists: "no" }); + }); + + it("displays the collapsible instructions if gitEmptyRepoExists = no and provider != others", () => { + render( + + + , + ); + expect( + screen.getByText("How to create a new repository?"), + ).toBeInTheDocument(); + + // Check if DemoImage is rendered + const img = screen.getByAltText("Create an empty repo in github"); + + expect(img).toBeInTheDocument(); + expect(img).toHaveAttribute("src", GIT_DEMO_GIF.create_repo.github); + }); + + it("displays a warning callout if gitEmptyRepoExists = no and provider = others", () => { + render( + + + , + ); + expect( + screen.getByText( + "You need an empty repository to connect to Git on Appsmith, please create one on your Git service provider to continue.", + ), + ).toBeInTheDocument(); + }); + + it("shows the import callout if gitEmptyRepoExists = no and not in import mode", () => { + render( + + + , + ); + expect( + screen.getByText( + "If you already have an application connected to Git, you can import it to the workspace.", + ), + ).toBeInTheDocument(); + expect(screen.getByText("Import via git")).toBeInTheDocument(); + }); + + it("clicking on 'Import via git' link calls onImportFromCalloutLinkClick", () => { + const onImportFromCalloutLinkClick = jest.fn(); + + render( + + + , + ); + fireEvent.click(screen.getByText("Import via git")); + expect(onImportFromCalloutLinkClick).toHaveBeenCalledTimes(1); + }); + + it("when isImport = true, shows a checkbox for existing repo", () => { + render(); + expect(screen.getByTestId("t--existing-repo-checkbox")).toBeInTheDocument(); + expect( + screen.getByText( + "I have an existing appsmith application connected to Git", + ), + ).toBeInTheDocument(); + }); + + it("toggles existing repo checkbox and calls onChange", () => { + const onChange = jest.fn(); + + render( + , + ); + const checkbox = screen.getByTestId("t--existing-repo-checkbox"); + + fireEvent.click(checkbox); + expect(onChange).toHaveBeenCalledWith({ gitExistingRepoExists: true }); + }); + + it("does not show second question if isImport = true", () => { + render(); + expect( + screen.queryByText("ii. Does an empty repository exist?"), + ).not.toBeInTheDocument(); + }); + + it("respects canCreateNewArtifact and device conditions for links", () => { + // If canCreateNewArtifact is false, "Import via git" should not appear even if conditions are met + render( + , + ); + // This should be null because we have no permission to create new artifact + expect(screen.queryByText("Import via git")).not.toBeInTheDocument(); + }); + + it("if provider is not chosen and user tries to select empty repo option, it remains disabled", () => { + render(); + const yesRadio = screen.getByTestId("t--existing-empty-repo-yes"); + + fireEvent.click(yesRadio); + // onChange should not be called because it's disabled + expect(defaultProps.onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/app/client/src/git/components/ConnectModal/ChooseGitProvider.tsx b/app/client/src/git/components/ConnectModal/ChooseGitProvider.tsx new file mode 100644 index 000000000000..3fcd9349c2ff --- /dev/null +++ b/app/client/src/git/components/ConnectModal/ChooseGitProvider.tsx @@ -0,0 +1,233 @@ +import React, { useCallback, useMemo } from "react"; +import { + DemoImage, + FieldContainer, + FieldControl, + FieldQuestion, + WellContainer, + WellTitle, + WellTitleContainer, +} from "./common"; +import { + Callout, + Checkbox, + Collapsible, + CollapsibleContent, + CollapsibleHeader, + Icon, + Radio, + RadioGroup, + Text, +} from "@appsmith/ads"; +import styled from "styled-components"; +import { GIT_DEMO_GIF } from "./constants"; +import noop from "lodash/noop"; +import { useIsMobileDevice } from "utils/hooks/useDeviceDetect"; +import { + CHOOSE_A_GIT_PROVIDER_STEP, + CHOOSE_GIT_PROVIDER_QUESTION, + HOW_TO_CREATE_EMPTY_REPO, + IMPORT_ARTIFACT_IF_NOT_EMPTY, + IS_EMPTY_REPO_QUESTION, + I_HAVE_EXISTING_ARTIFACT_REPO, + NEED_EMPTY_REPO_MESSAGE, + createMessage, +} from "ee/constants/messages"; +import log from "loglevel"; + +const WellInnerContainer = styled.div` + padding-left: 16px; +`; + +const CheckboxTextContainer = styled.div` + display: flex; + justify-content: flex-start; +`; + +const GIT_PROVIDERS = ["github", "gitlab", "bitbucket", "others"] as const; + +export type GitProvider = (typeof GIT_PROVIDERS)[number]; + +interface ChooseGitProviderState { + gitProvider?: GitProvider; + gitEmptyRepoExists: string; + gitExistingRepoExists: boolean; +} +interface ChooseGitProviderProps { + artifactId: string; + artifactType: string; + onChange: (args: Partial) => void; + value: Partial; + isImport?: boolean; + // Replaces handleImport in original ChooseGitProvider.tsx + onImportFromCalloutLinkClick: () => void; + // Replaces hasCreateNewApplicationPermission = hasCreateNewAppPermission(workspace.userPermissions) + canCreateNewArtifact: boolean; +} + +function ChooseGitProvider({ + artifactType, + canCreateNewArtifact, + isImport = false, + onChange = noop, + onImportFromCalloutLinkClick, + value = {}, +}: ChooseGitProviderProps) { + const isMobile = useIsMobileDevice(); + + const hasCreateNewArtifactPermission = canCreateNewArtifact && !isMobile; + + const onGitProviderChange = useCallback( + (value: string) => { + const gitProvider = GIT_PROVIDERS.includes(value as GitProvider) + ? (value as GitProvider) + : undefined; + + if (gitProvider) { + onChange({ gitProvider }); + } else { + log.error(`Invalid git provider: ${value}`); + } + }, + [onChange], + ); + + const onEmptyRepoOptionChange = useCallback( + (gitEmptyRepoExists) => onChange({ gitEmptyRepoExists }), + [onChange], + ); + + const onExistingRepoOptionChange = useCallback( + (gitExistingRepoExists) => onChange({ gitExistingRepoExists }), + [onChange], + ); + + const importCalloutLinks = useMemo(() => { + return hasCreateNewArtifactPermission + ? [{ children: "Import via git", onClick: onImportFromCalloutLinkClick }] + : []; + }, [hasCreateNewArtifactPermission, onImportFromCalloutLinkClick]); + + return ( + <> + + + + {createMessage(CHOOSE_A_GIT_PROVIDER_STEP)} + + + + + + i. {createMessage(CHOOSE_GIT_PROVIDER_QUESTION)}{" "} + * + + + + + Github + + + Gitlab + + + Bitbucket + + + Others + + + + + {!isImport && ( + + + ii. {createMessage(IS_EMPTY_REPO_QUESTION)}{" "} + * + + + + + Yes + + + No + + + + + )} + {!isImport && + value?.gitProvider !== "others" && + value?.gitEmptyRepoExists === "no" && ( + + + + {createMessage(HOW_TO_CREATE_EMPTY_REPO)} + + + + + + )} + {!isImport && + value?.gitProvider === "others" && + value?.gitEmptyRepoExists === "no" && ( + + {createMessage(NEED_EMPTY_REPO_MESSAGE)} + + )} + + + {!isImport && value?.gitEmptyRepoExists === "no" ? ( + + {createMessage(IMPORT_ARTIFACT_IF_NOT_EMPTY, artifactType)} + + ) : null} + {isImport && ( + + + + {createMessage(I_HAVE_EXISTING_ARTIFACT_REPO, artifactType)} + + +  * + + + + )} + + ); +} + +export default ChooseGitProvider; diff --git a/app/client/src/git/components/ConnectModal/CopyButton.tsx b/app/client/src/git/components/ConnectModal/CopyButton.tsx new file mode 100644 index 000000000000..e1ac17da33f2 --- /dev/null +++ b/app/client/src/git/components/ConnectModal/CopyButton.tsx @@ -0,0 +1,99 @@ +import type { CSSProperties } from "react"; +import React, { useCallback, useEffect, useRef, useState } from "react"; +import { Button, Icon, Tooltip } from "@appsmith/ads"; +import styled from "styled-components"; +import copy from "copy-to-clipboard"; +import noop from "lodash/noop"; +import log from "loglevel"; + +export const TooltipWrapper = styled.div` + display: flex; + justify-content: center; + align-items: center; +`; + +export const IconContainer = styled.div``; + +interface CopyButtonProps { + style?: CSSProperties; + value?: string; + delay?: number; + onCopy?: () => void; + tooltipMessage?: string; + isDisabled?: boolean; + testIdSuffix?: string; +} + +function CopyButton({ + delay = 2000, + isDisabled = false, + onCopy = noop, + style, + testIdSuffix = "generic", + tooltipMessage, + value, +}: CopyButtonProps) { + const timerRef = useRef(); + const [showCopied, setShowCopied] = useState(false); + + useEffect(function clearShowCopiedTimeout() { + return () => { + if (timerRef.current) { + clearTimeout(timerRef.current); + } + }; + }, []); + + const stopShowingCopiedAfterDelay = useCallback(() => { + timerRef.current = setTimeout(() => { + setShowCopied(false); + }, delay); + }, [delay]); + + const copyToClipboard = useCallback(() => { + if (value) { + try { + const success = copy(value); + + if (success) { + setShowCopied(true); + stopShowingCopiedAfterDelay(); + onCopy(); + } + } catch (error) { + log.error("Failed to copy to clipboard:", error); + } + } + }, [onCopy, stopShowingCopiedAfterDelay, value]); + + return ( + <> + {showCopied ? ( + + + + ) : ( + + + + + {createMessage(COPY_SSH_URL_MESSAGE)} + + + + {value?.gitProvider !== "others" && ( + + + + {createMessage(HOW_TO_COPY_REMOTE_URL)} + + + + + + )} + + + ); +} + +export default GenerateSSH; diff --git a/app/client/src/git/components/ConnectModal/Steps.tsx b/app/client/src/git/components/ConnectModal/Steps.tsx new file mode 100644 index 000000000000..a67af1e1c4fd --- /dev/null +++ b/app/client/src/git/components/ConnectModal/Steps.tsx @@ -0,0 +1,121 @@ +import { Button, Divider, Text } from "@appsmith/ads"; +import noop from "lodash/noop"; +import React, { Fragment } from "react"; +import styled from "styled-components"; + +const Container = styled.div` + display: flex; + margin-bottom: 16px; + align-items: center; +`; + +const StepButton = styled(Button)` + display: flex; + align-items: center; + + .ads-v2-button__content { + padding: 4px; + } + + .ads-v2-button__content-children { + font-weight: var(--ads-v2-font-weight-bold); + } + + .ads-v2-button__content-children > * { + font-weight: var(--ads-v2-font-weight-bold); + } + + opacity: ${({ isDisabled }) => (isDisabled ? "0.6" : "1")}; +`; + +interface StepNumberProps { + active: boolean; +} + +const StepNumber = styled.div` + width: 20px; + height: 20px; + border-radius: 10px; + border-style: solid; + border-width: 1px; + border-color: ${(p) => + p.active + ? "var(--ads-v2-color-border-success)" + : "var(--ads-v2-color-border-emphasis)"}; + background-color: ${(p) => + p.active + ? "var(--ads-v2-color-bg-success)" + : "var(--ads-v2-color-bg-subtle)"}; + color: ${(p) => + p.active + ? "var(--ads-v2-color-border-success)" + : "var(--ads-v2-color-text)"}; + display: flex; + justify-content: center; + align-items: center; + line-height: 1; + margin-right: 4px; + flex-shrink: 0; +`; + +const StepText = styled(Text)` + font-weight: 500; +`; + +const StepLine = styled(Divider)` + width: initial; + margin-left: 8px; + margin-right: 8px; + flex: 1; +`; + +interface Step { + key: string; + text: string; +} + +interface StepsProps { + steps: Step[]; + activeKey: string; + onActiveKeyChange: (activeKey: string) => void; +} + +function Steps({ + activeKey, + onActiveKeyChange = noop, + steps = [], +}: StepsProps) { + const activeIndex = steps.findIndex((s) => s.key === activeKey); + + const onClick = (step: Step, index: number) => () => { + if (index < activeIndex) { + onActiveKeyChange(step.key); + } + }; + + return ( + + {steps.map((step, index) => { + return ( + + {index > 0 && } + activeIndex} + kind="tertiary" + onClick={onClick(step, index)} + role="button" + size="md" + > + + {index + 1} + + {step.text} + + + ); + })} + + ); +} + +export default Steps; diff --git a/app/client/src/git/components/ConnectModal/common.tsx b/app/client/src/git/components/ConnectModal/common.tsx new file mode 100644 index 000000000000..cd69abf40ea3 --- /dev/null +++ b/app/client/src/git/components/ConnectModal/common.tsx @@ -0,0 +1,52 @@ +import { Callout, Text } from "@appsmith/ads"; +import styled from "styled-components"; + +export const WellContainer = styled.div` + padding: 16px; + border-radius: 4px; + background-color: var(--ads-v2-color-gray-100); + margin-bottom: 16px; + flex: 1; + flex-shrink: 1; + overflow-y: auto; +`; + +export const WellTitleContainer = styled.div` + display: flex; + align-items: center; + margin-bottom: 16px; + justify-content: space-between; +`; + +export const WellTitle = styled(Text)` + font-weight: 600; +`; + +export const WellText = styled(Text)` + margin-bottom: 16px; +`; + +export const FieldContainer = styled.div` + margin-bottom: 16px; +`; + +export const FieldControl = styled.div` + padding-left: 24px; +`; + +export const FieldQuestion = styled(Text)<{ isDisabled?: boolean }>` + opacity: ${({ isDisabled = false }) => (isDisabled ? "0.5" : "1")}; + margin-bottom: 16px; +`; + +export const DemoImage = styled.img` + width: 100%; + height: 300px; + object-fit: cover; + object-position: 50% 0; + background-color: var(--ads-color-black-200); +`; + +export const ErrorCallout = styled(Callout)` + margin-bottom: 16px; +`; diff --git a/app/client/src/git/components/ConnectModal/constants.ts b/app/client/src/git/components/ConnectModal/constants.ts new file mode 100644 index 000000000000..a15001af865d --- /dev/null +++ b/app/client/src/git/components/ConnectModal/constants.ts @@ -0,0 +1,26 @@ +import { getAssetUrl } from "ee/utils/airgapHelpers"; +import { ASSETS_CDN_URL } from "constants/ThirdPartyConstants"; + +export const GIT_CONNECT_STEPS = { + CHOOSE_PROVIDER: "choose-provider", + GENERATE_SSH_KEY: "generate-ssh-key", + ADD_DEPLOY_KEY: "add-deploy-key", +}; + +export const GIT_DEMO_GIF = { + create_repo: { + github: getAssetUrl(`${ASSETS_CDN_URL}/Github_create_empty_repo.gif`), + gitlab: getAssetUrl(`${ASSETS_CDN_URL}/Gitlab_create_a_repo.gif`), + bitbucket: getAssetUrl(`${ASSETS_CDN_URL}/Bitbucket_create_a_repo.gif`), + }, + copy_remoteurl: { + github: getAssetUrl(`${ASSETS_CDN_URL}/Github_SSHkey.gif`), + gitlab: getAssetUrl(`${ASSETS_CDN_URL}/Gitlab_SSHKey.gif`), + bitbucket: getAssetUrl(`${ASSETS_CDN_URL}/Bitbucket_Copy_SSHKey.gif`), + }, + add_deploykey: { + github: getAssetUrl(`${ASSETS_CDN_URL}/Github_add_deploykey.gif`), + gitlab: getAssetUrl(`${ASSETS_CDN_URL}/Gitlab_add_deploy_key.gif`), + bitbucket: getAssetUrl(`${ASSETS_CDN_URL}/Bitbucket_add_a_deploykey.gif`), + }, +}; diff --git a/app/client/src/git/components/ConnectModal/index.test.tsx b/app/client/src/git/components/ConnectModal/index.test.tsx new file mode 100644 index 000000000000..71d5b27d7a65 --- /dev/null +++ b/app/client/src/git/components/ConnectModal/index.test.tsx @@ -0,0 +1,216 @@ +import React from "react"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { isValidGitRemoteUrl } from "../utils"; +import ConnectModal from "."; +import "@testing-library/jest-dom"; + +jest.mock("ee/utils/AnalyticsUtil", () => ({ + logEvent: jest.fn(), +})); + +jest.mock("../utils", () => ({ + isValidGitRemoteUrl: jest.fn(), +})); + +const defaultProps = { + artifactId: "artifact-123", + artifactType: "application", + canCreateNewArtifact: true, + connectTo: jest.fn(), + importFrom: jest.fn(), + isConnecting: false, + isImport: false, + isImporting: false, + onImportFromCalloutLinkClick: jest.fn(), + deployKeyDocUrl: "https://docs.example.com", + fetchSSHKeyPair: jest.fn(), + generateSSHKey: jest.fn(), + isFetchingSSHKeyPair: false, + isGeneratingSSHKey: false, + sshKeyPair: "ssh-rsa AAAAB3...", + isModalOpen: true, +}; + +function completeChooseProviderStep(isImport = false) { + fireEvent.click(screen.getByTestId("t--git-provider-radio-github")); + + if (isImport) { + fireEvent.click(screen.getByTestId("t--existing-repo-checkbox")); + } else { + fireEvent.click(screen.getByTestId("t--existing-empty-repo-yes")); + } + + fireEvent.click(screen.getByTestId("t--git-connect-next-button")); +} + +function completeGenerateSSHKeyStep() { + fireEvent.change(screen.getByTestId("git-connect-remote-url-input"), { + target: { value: "git@example.com:user/repo.git" }, + }); + fireEvent.click(screen.getByTestId("t--git-connect-next-button")); +} + +function completeAddDeployKeyStep() { + fireEvent.click(screen.getByTestId("t--added-deploy-key-checkbox")); + fireEvent.click(screen.getByTestId("t--git-connect-next-button")); +} + +describe("ConnectModal Component", () => { + beforeEach(() => { + jest.clearAllMocks(); + (isValidGitRemoteUrl as jest.Mock).mockImplementation((url) => + url.startsWith("git@"), + ); + }); + + it("renders the initial step (ChooseGitProvider)", () => { + render(); + expect( + screen.getByText("i. To begin with, choose your Git service provider"), + ).toBeInTheDocument(); + expect(screen.getByTestId("t--git-connect-next-button")).toHaveTextContent( + "Configure Git", + ); + }); + + it("disables the next button when form data is incomplete in ChooseGitProvider step", () => { + render(); + expect(screen.getByTestId("t--git-connect-next-button")).toBeDisabled(); + }); + + it("navigates to the next step (GenerateSSH) and validates SSH URL input", () => { + render(); + + completeChooseProviderStep(); + + const sshInput = screen.getByTestId("git-connect-remote-url-input"); + + fireEvent.change(sshInput, { target: { value: "invalid-url" } }); + fireEvent.blur(sshInput); + + expect( + screen.getByText("Please enter a valid SSH URL of your repository"), + ).toBeInTheDocument(); + + fireEvent.change(sshInput, { + target: { value: "git@example.com:user/repo.git" }, + }); + fireEvent.blur(sshInput); + + expect( + screen.queryByText("Please enter a valid SSH URL of your repository"), + ).not.toBeInTheDocument(); + }); + + it("renders AddDeployKey step and validates state transitions", () => { + render(); + + completeChooseProviderStep(); + completeGenerateSSHKeyStep(); + + expect( + screen.getByText("Add deploy key & give write access"), + ).toBeInTheDocument(); + }); + + it("calls connectTo on completing AddDeployKey step in connect mode", async () => { + render(); + completeChooseProviderStep(); + completeGenerateSSHKeyStep(); + completeAddDeployKeyStep(); + + await waitFor(() => { + expect(defaultProps.connectTo).toHaveBeenCalledWith( + expect.objectContaining({ + payload: { + remoteUrl: "git@example.com:user/repo.git", + gitProfile: { + authorName: "", + authorEmail: "", + useGlobalProfile: true, + }, + }, + }), + ); + }); + }); + + it("calls importFrom on completing AddDeployKey step in import mode", async () => { + render(); + completeChooseProviderStep(true); + completeGenerateSSHKeyStep(); + completeAddDeployKeyStep(); + + await waitFor(() => { + expect(defaultProps.importFrom).toHaveBeenCalledWith( + expect.objectContaining({ + payload: { + remoteUrl: "git@example.com:user/repo.git", + gitProfile: { + authorName: "", + authorEmail: "", + useGlobalProfile: true, + }, + }, + }), + ); + }); + }); + + it("shows an error callout when an error occurs during connectTo", async () => { + const mockConnectTo = jest.fn((props) => { + props.onErrorCallback(new Error("Error"), { + responseMeta: { error: { code: "AE-GIT-4033" } }, + }); + }); + + render(); + completeChooseProviderStep(); + completeGenerateSSHKeyStep(); + + expect( + screen.queryByText("The repo you added isn't empty"), + ).not.toBeInTheDocument(); + + completeAddDeployKeyStep(); + + await waitFor(() => { + expect( + screen.getByText("The repo you added isn't empty"), + ).toBeInTheDocument(); + }); + }); + + it("renders the previous step when Previous button is clicked", () => { + render(); + expect( + screen.getByText("i. To begin with, choose your Git service provider"), + ).toBeInTheDocument(); + completeChooseProviderStep(); + expect( + screen.queryByText("i. To begin with, choose your Git service provider"), + ).not.toBeInTheDocument(); + fireEvent.click(screen.getByTestId("t--git-connect-prev-button")); // Back to ChooseGitProvider step + expect( + screen.getByText("i. To begin with, choose your Git service provider"), + ).toBeInTheDocument(); + }); + + it("disables next button when form data is invalid in any step", () => { + render(); + const nextButton = screen.getByTestId("t--git-connect-next-button"); + + fireEvent.click(nextButton); // Try to move to next step + expect(nextButton).toBeDisabled(); + }); + + it("renders loading state and removes buttons when connecting", () => { + render(); + expect( + screen.getByText("Please wait while we connect to Git..."), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("t--git-connect-next-button"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/app/client/src/git/components/ConnectModal/index.tsx b/app/client/src/git/components/ConnectModal/index.tsx new file mode 100644 index 000000000000..f7c8d097213d --- /dev/null +++ b/app/client/src/git/components/ConnectModal/index.tsx @@ -0,0 +1,339 @@ +import React, { useCallback, useState } from "react"; +import styled from "styled-components"; + +import AddDeployKey, { type AddDeployKeyProps } from "./AddDeployKey"; +import AnalyticsUtil from "ee/utils/AnalyticsUtil"; +import ChooseGitProvider from "./ChooseGitProvider"; +import GenerateSSH from "./GenerateSSH"; +import Steps from "./Steps"; +import Statusbar from "../Statusbar"; +import { Button, ModalBody, ModalFooter } from "@appsmith/ads"; +import { GIT_CONNECT_STEPS } from "./constants"; +import type { GitProvider } from "./ChooseGitProvider"; +import { + ADD_DEPLOY_KEY_STEP, + CHOOSE_A_GIT_PROVIDER_STEP, + CONFIGURE_GIT, + CONNECT_GIT_TEXT, + GENERATE_SSH_KEY_STEP, + GIT_CONNECT_WAITING, + GIT_IMPORT_WAITING, + IMPORT_APP_CTA, + PREVIOUS_STEP, + createMessage, +} from "ee/constants/messages"; +import { isValidGitRemoteUrl } from "../utils"; +import type { ApiResponse } from "api/ApiResponses"; + +const OFFSET = 200; +const OUTER_PADDING = 32; +const FOOTER = 56; +const HEADER = 44; + +const StyledModalBody = styled(ModalBody)` + flex: 1; + overflow-y: initial; + display: flex; + flex-direction: column; + max-height: calc( + 100vh - ${OFFSET}px - ${OUTER_PADDING}px - ${FOOTER}px - ${HEADER}px + ); +`; + +const StyledModalFooter = styled(ModalFooter)` + justify-content: space-between; + flex-direction: ${(p) => (!p.loading ? "row-reverse" : "row")}; +`; + +const steps = [ + { + key: GIT_CONNECT_STEPS.CHOOSE_PROVIDER, + text: createMessage(CHOOSE_A_GIT_PROVIDER_STEP), + }, + { + key: GIT_CONNECT_STEPS.GENERATE_SSH_KEY, + text: createMessage(GENERATE_SSH_KEY_STEP), + }, + { + key: GIT_CONNECT_STEPS.ADD_DEPLOY_KEY, + text: createMessage(ADD_DEPLOY_KEY_STEP), + }, +]; + +const possibleSteps = steps.map((s) => s.key); + +interface StyledModalFooterProps { + loading?: boolean; +} + +interface FormDataState { + gitProvider?: GitProvider; + gitEmptyRepoExists?: string; + gitExistingRepoExists?: boolean; + remoteUrl?: string; + isAddedDeployKey?: boolean; + sshKeyType?: "RSA" | "ECDSA"; +} + +interface GitProfile { + authorName: string; + authorEmail: string; + useDefaultProfile?: boolean; +} + +interface ConnectOrImportPayload { + remoteUrl: string; + gitProfile: GitProfile; +} + +interface ConnectOrImportProps { + payload: ConnectOrImportPayload; + onErrorCallback: (error: Error, response: ApiResponse) => void; +} + +// Remove comments after integration +interface ConnectModalProps { + isImport?: boolean; + // It replaces const isImportingViaGit in GitConnectionV2/index.tsx + isImporting?: boolean; + // Replaces dispatch(importAppFromGit) + importFrom: (props: ConnectOrImportProps) => void; + // Replaces connectToGit from useGitConnect hook + connectTo: (props: ConnectOrImportProps) => void; + // Replaces isConnectingToGit + isConnectingTo?: boolean; + isConnecting: boolean; + artifactId: string; + artifactType: string; + // Replaces handleImport in original ChooseGitProvider.tsx + onImportFromCalloutLinkClick: () => void; + // Replaces hasCreateNewApplicationPermission = hasCreateNewAppPermission(workspace.userPermissions) + canCreateNewArtifact: boolean; + isModalOpen: boolean; + deployKeyDocUrl: AddDeployKeyProps["deployKeyDocUrl"]; + isFetchingSSHKeyPair: AddDeployKeyProps["isFetchingSSHKeyPair"]; + fetchSSHKeyPair: AddDeployKeyProps["fetchSSHKeyPair"]; + generateSSHKey: AddDeployKeyProps["generateSSHKey"]; + isGeneratingSSHKey: AddDeployKeyProps["isGeneratingSSHKey"]; + sshKeyPair: AddDeployKeyProps["sshKeyPair"]; +} + +function ConnectModal({ + artifactId, + artifactType, + canCreateNewArtifact, + connectTo, + deployKeyDocUrl, + fetchSSHKeyPair, + generateSSHKey, + importFrom, + isConnecting = false, + isFetchingSSHKeyPair, + isGeneratingSSHKey, + isImport = false, + isImporting = false, + isModalOpen, + onImportFromCalloutLinkClick, + sshKeyPair, +}: ConnectModalProps) { + const [errorData, setErrorData] = useState>(); + + const nextStepText = { + [GIT_CONNECT_STEPS.CHOOSE_PROVIDER]: createMessage(CONFIGURE_GIT), + [GIT_CONNECT_STEPS.GENERATE_SSH_KEY]: createMessage(GENERATE_SSH_KEY_STEP), + [GIT_CONNECT_STEPS.ADD_DEPLOY_KEY]: createMessage( + isImport ? IMPORT_APP_CTA : CONNECT_GIT_TEXT, + ), + }; + + const [formData, setFormData] = useState({ + gitProvider: undefined, + gitEmptyRepoExists: undefined, + gitExistingRepoExists: false, + remoteUrl: undefined, + isAddedDeployKey: false, + sshKeyType: "ECDSA", + }); + + const handleChange = (partialFormData: Partial) => { + setFormData((s) => ({ ...s, ...partialFormData })); + }; + + const [activeStep, setActiveStep] = useState( + GIT_CONNECT_STEPS.CHOOSE_PROVIDER, + ); + const currentIndex = steps.findIndex((s) => s.key === activeStep); + + const isDisabled = { + [GIT_CONNECT_STEPS.CHOOSE_PROVIDER]: !isImport + ? !formData.gitProvider || + !formData.gitEmptyRepoExists || + formData.gitEmptyRepoExists === "no" + : !formData.gitProvider || !formData.gitExistingRepoExists, + [GIT_CONNECT_STEPS.GENERATE_SSH_KEY]: + typeof formData?.remoteUrl !== "string" || + !isValidGitRemoteUrl(formData?.remoteUrl), + [GIT_CONNECT_STEPS.ADD_DEPLOY_KEY]: !formData.isAddedDeployKey, + }; + + const handlePreviousStep = useCallback(() => { + if (currentIndex > 0) { + setActiveStep(steps[currentIndex - 1].key); + } + }, [currentIndex]); + + const handleNextStep = useCallback(() => { + if (currentIndex < steps.length) { + switch (activeStep) { + case GIT_CONNECT_STEPS.CHOOSE_PROVIDER: { + setActiveStep(GIT_CONNECT_STEPS.GENERATE_SSH_KEY); + AnalyticsUtil.logEvent("GS_CONFIGURE_GIT"); + break; + } + case GIT_CONNECT_STEPS.GENERATE_SSH_KEY: { + setActiveStep(GIT_CONNECT_STEPS.ADD_DEPLOY_KEY); + AnalyticsUtil.logEvent("GS_GENERATE_KEY_BUTTON_CLICK", { + repoUrl: formData?.remoteUrl, + connectFlow: "v2", + }); + break; + } + case GIT_CONNECT_STEPS.ADD_DEPLOY_KEY: { + const gitProfile = { + authorName: "", + authorEmail: "", + useGlobalProfile: true, + }; + + if (formData.remoteUrl) { + if (!isImport) { + connectTo({ + payload: { + remoteUrl: formData.remoteUrl, + gitProfile, + }, + onErrorCallback: (error, response) => { + // AE-GIT-4033 is repo not empty error + if (response?.responseMeta?.error?.code === "AE-GIT-4033") { + setActiveStep(GIT_CONNECT_STEPS.GENERATE_SSH_KEY); + } + + setErrorData(response); + }, + }); + AnalyticsUtil.logEvent( + "GS_CONNECT_BUTTON_ON_GIT_SYNC_MODAL_CLICK", + { repoUrl: formData?.remoteUrl, connectFlow: "v2" }, + ); + } else { + importFrom({ + payload: { + remoteUrl: formData.remoteUrl, + gitProfile, + // isDefaultProfile: true, + }, + onErrorCallback(error, response) { + setErrorData(response); + }, + }); + } + } + + break; + } + } + } + }, [ + activeStep, + connectTo, + currentIndex, + formData.remoteUrl, + importFrom, + isImport, + ]); + + const stepProps = { + onChange: handleChange, + value: formData, + isImport, + errorData, + }; + + const loading = (!isImport && isConnecting) || (isImport && isImporting); + + return ( + <> + + {possibleSteps.includes(activeStep) && ( + + )} + {activeStep === GIT_CONNECT_STEPS.CHOOSE_PROVIDER && ( + + )} + {activeStep === GIT_CONNECT_STEPS.GENERATE_SSH_KEY && ( + + )} + {activeStep === GIT_CONNECT_STEPS.ADD_DEPLOY_KEY && ( + + )} + + + {loading && ( + + )} + {!loading && ( + + )} + {possibleSteps.includes(activeStep) && currentIndex > 0 && !loading && ( + + )} + + + ); +} + +export default ConnectModal; diff --git a/app/client/src/git/components/CtxAwareGitQuickActions/index.tsx b/app/client/src/git/components/CtxAwareGitQuickActions/index.tsx index 9deb2f2d7a55..824350f3b195 100644 --- a/app/client/src/git/components/CtxAwareGitQuickActions/index.tsx +++ b/app/client/src/git/components/CtxAwareGitQuickActions/index.tsx @@ -5,9 +5,13 @@ import useStatusChangeCount from "./hooks/useStatusChangeCount"; function CtxAwareGitQuickActions() { const { + autocommitEnabled, + autocommitPolling, discard, discardLoading, fetchStatusLoading, + gitConnected, + protectedMode, pull, pullError, pullLoading, @@ -17,12 +21,7 @@ function CtxAwareGitQuickActions() { toggleGitSettingsModal, } = useGitContext(); - const isGitConnected = false; - const isAutocommitEnabled = true; - const isAutocommitPolling = false; - const isConnectPermitted = true; - const isProtectedMode = false; - + const connectPermitted = true; const isPullFailing = !!pullError; const isStatusClean = status?.isClean ?? false; const statusBehindCount = status?.behindCount ?? 0; @@ -31,13 +30,13 @@ function CtxAwareGitQuickActions() { return ( ({ artifactType, baseArtifactId }), [artifactType, baseArtifactId], ); + const useGitMetadataReturnValue = useGitMetadata(basePayload); const useGitConnectReturnValue = useGitConnect(basePayload); const useGitOpsReturnValue = useGitOps(basePayload); const useGitBranchesReturnValue = useGitBranches(basePayload); const useGitSettingsReturnValue = useGitSettings(basePayload); return { + ...useGitMetadataReturnValue, ...useGitOpsReturnValue, ...useGitBranchesReturnValue, ...useGitConnectReturnValue, diff --git a/app/client/src/git/components/GitContextProvider/hooks/useGitMetadata.ts b/app/client/src/git/components/GitContextProvider/hooks/useGitMetadata.ts new file mode 100644 index 000000000000..b1a10de4a1a5 --- /dev/null +++ b/app/client/src/git/components/GitContextProvider/hooks/useGitMetadata.ts @@ -0,0 +1,45 @@ +import type { GitArtifactType } from "git/constants/enums"; +import type { FetchGitMetadataResponseData } from "git/requests/fetchGitMetadataRequest.types"; +import { + selectGitConnected, + selectGitMetadata, +} from "git/store/selectors/gitSingleArtifactSelectors"; +import type { GitRootState } from "git/store/types"; +import { useMemo } from "react"; +import { useSelector } from "react-redux"; + +interface UseGitMetadataParams { + artifactType: keyof typeof GitArtifactType; + baseArtifactId: string; +} + +export interface UseGitMetadataReturnValue { + gitMetadata: FetchGitMetadataResponseData | null; + fetchGitMetadataLoading: boolean; + fetchGitMetadataError: string | null; + gitConnected: boolean; +} + +export default function useGitMetadata({ + artifactType, + baseArtifactId, +}: UseGitMetadataParams): UseGitMetadataReturnValue { + const basePayload = useMemo( + () => ({ artifactType, baseArtifactId }), + [artifactType, baseArtifactId], + ); + + const gitMetadataState = useSelector((state: GitRootState) => + selectGitMetadata(state, basePayload), + ); + const gitConnected = useSelector((state: GitRootState) => + selectGitConnected(state, basePayload), + ); + + return { + gitMetadata: gitMetadataState.value, + fetchGitMetadataLoading: gitMetadataState.loading ?? false, + fetchGitMetadataError: gitMetadataState.error, + gitConnected: gitConnected ?? false, + }; +} diff --git a/app/client/src/git/components/GitContextProvider/hooks/useGitOps.ts b/app/client/src/git/components/GitContextProvider/hooks/useGitOps.ts index 3c9f3990d6e9..1d0b59f71cd0 100644 --- a/app/client/src/git/components/GitContextProvider/hooks/useGitOps.ts +++ b/app/client/src/git/components/GitContextProvider/hooks/useGitOps.ts @@ -133,24 +133,24 @@ export default function useGitOps({ return { commitLoading: commitState?.loading ?? false, - commitError: commitState?.error ?? null, + commitError: commitState?.error, commit, discardLoading: discardState?.loading ?? false, - discardError: discardState?.error ?? null, + discardError: discardState?.error, discard, - status: statusState?.value ?? null, + status: statusState?.value, fetchStatusLoading: statusState?.loading ?? false, - fetchStatusError: statusState?.error ?? null, + fetchStatusError: statusState?.error, fetchStatus, mergeLoading: mergeState?.loading ?? false, - mergeError: mergeState?.error ?? null, + mergeError: mergeState?.error, merge, - mergeStatus: mergeStatusState?.value ?? null, + mergeStatus: mergeStatusState?.value, fetchMergeStatusLoading: mergeStatusState?.loading ?? false, - fetchMergeStatusError: mergeStatusState?.error ?? null, + fetchMergeStatusError: mergeStatusState?.error, fetchMergeStatus, pullLoading: pullState?.loading ?? false, - pullError: pullState?.error ?? null, + pullError: pullState?.error, pull, toggleGitOpsModal, }; diff --git a/app/client/src/git/components/GitContextProvider/hooks/useGitSettings.ts b/app/client/src/git/components/GitContextProvider/hooks/useGitSettings.ts index 096e934d7d5e..2e63f8e60325 100644 --- a/app/client/src/git/components/GitContextProvider/hooks/useGitSettings.ts +++ b/app/client/src/git/components/GitContextProvider/hooks/useGitSettings.ts @@ -1,7 +1,15 @@ import type { GitArtifactType, GitSettingsTab } from "git/constants/enums"; +import type { FetchProtectedBranchesResponseData } from "git/requests/fetchProtectedBranchesRequest.types"; import { gitArtifactActions } from "git/store/gitArtifactSlice"; +import { + selectAutocommitEnabled, + selectAutocommitPolling, + selectProtectedBranches, + selectProtectedMode, +} from "git/store/selectors/gitSingleArtifactSelectors"; +import type { GitRootState } from "git/store/types"; import { useMemo } from "react"; -import { useDispatch } from "react-redux"; +import { useDispatch, useSelector } from "react-redux"; interface UseGitSettingsParams { artifactType: keyof typeof GitArtifactType; @@ -9,6 +17,13 @@ interface UseGitSettingsParams { } export interface UseGitSettingsReturnValue { + autocommitEnabled: boolean; + autocommitPolling: boolean; + protectedBranches: FetchProtectedBranchesResponseData | null; + fetchProtectedBranchesLoading: boolean; + fetchProtectedBranchesError: string | null; + fetchProtectedBranches: () => void; + protectedMode: boolean; toggleGitSettingsModal: ( open: boolean, tab: keyof typeof GitSettingsTab, @@ -25,6 +40,33 @@ export default function useGitSettings({ [artifactType, baseArtifactId], ); + // autocommit + const autocommitEnabled = useSelector((state: GitRootState) => + selectAutocommitEnabled(state, basePayload), + ); + + const autocommitPolling = useSelector((state: GitRootState) => + selectAutocommitPolling(state, basePayload), + ); + + // branch protection + const protectedBranchesState = useSelector((state: GitRootState) => + selectProtectedBranches(state, basePayload), + ); + + const fetchProtectedBranches = () => { + dispatch( + gitArtifactActions.fetchProtectedBranchesInit({ + ...basePayload, + }), + ); + }; + + const protectedMode = useSelector((state: GitRootState) => + selectProtectedMode(state, basePayload), + ); + + // ui const toggleGitSettingsModal = ( open: boolean, tab: keyof typeof GitSettingsTab, @@ -39,6 +81,13 @@ export default function useGitSettings({ }; return { + autocommitEnabled: autocommitEnabled ?? false, + autocommitPolling: autocommitPolling ?? false, + protectedBranches: protectedBranchesState.value, + fetchProtectedBranchesLoading: protectedBranchesState.loading ?? false, + fetchProtectedBranchesError: protectedBranchesState.error, + fetchProtectedBranches, + protectedMode: protectedMode ?? false, toggleGitSettingsModal, }; } diff --git a/app/client/src/git/components/GitContextProvider/index.tsx b/app/client/src/git/components/GitContextProvider/index.tsx index 2d057c30670b..afb9f010a50c 100644 --- a/app/client/src/git/components/GitContextProvider/index.tsx +++ b/app/client/src/git/components/GitContextProvider/index.tsx @@ -1,4 +1,4 @@ -import React, { createContext, useContext, useEffect } from "react"; +import React, { createContext, useContext } from "react"; import type { GitArtifactType } from "git/constants/enums"; import type { GitContextValue } from "./hooks/useGitContextValue"; import useGitContextValue from "./hooks/useGitContextValue"; @@ -15,24 +15,18 @@ interface GitContextProviderProps { artifactType: keyof typeof GitArtifactType; baseArtifactId: string; children: React.ReactNode; + // extra + // connectPermitted?: boolean; } export default function GitContextProvider({ artifactType, baseArtifactId, children, + // connectPermitted = true, }: GitContextProviderProps) { const contextValue = useGitContextValue({ artifactType, baseArtifactId }); - const { fetchBranches } = contextValue; - - useEffect( - function gitInitEffect() { - fetchBranches(); - }, - [fetchBranches], - ); - return ( {children} ); diff --git a/app/client/src/git/components/GitQuickActions/QuickActionButton.test.tsx b/app/client/src/git/components/GitQuickActions/QuickActionButton.test.tsx index 6bd94a586f56..037db38a04d2 100644 --- a/app/client/src/git/components/GitQuickActions/QuickActionButton.test.tsx +++ b/app/client/src/git/components/GitQuickActions/QuickActionButton.test.tsx @@ -50,7 +50,7 @@ describe("QuickActionButton", () => { , ); - const btn = container.getElementsByClassName("t--test-btn")[0]; + const btn = container.querySelectorAll(".t--test-btn button")[0]; fireEvent.click(btn); expect(defaultProps.onClick).toHaveBeenCalledTimes(1); diff --git a/app/client/src/git/components/GitQuickActions/QuickActionButton.tsx b/app/client/src/git/components/GitQuickActions/QuickActionButton.tsx index c22306b748fa..71c11dbe65ad 100644 --- a/app/client/src/git/components/GitQuickActions/QuickActionButton.tsx +++ b/app/client/src/git/components/GitQuickActions/QuickActionButton.tsx @@ -19,12 +19,11 @@ const SpinnerContainer = styled.div` padding: 0 10px; `; -const QuickActionButtonContainer = styled.button<{ disabled?: boolean }>` +const QuickActionButtonContainer = styled.div<{ disabled?: boolean }>` margin: 0 ${(props) => props.theme.spaces[1]}px; display: block; position: relative; overflow: visible; - cursor: ${({ disabled = false }) => (disabled ? "not-allowed" : "pointer")}; opacity: ${({ disabled = false }) => (disabled ? 0.6 : 1)}; `; @@ -57,11 +56,7 @@ function QuickActionButton({ const content = capitalizeFirstLetter(tooltipText); return ( - + {loading ? ( @@ -73,6 +68,7 @@ function QuickActionButton({ isDisabled={disabled} isIconButton kind="tertiary" + onClick={onClick} size="md" startIcon={icon} /> diff --git a/app/client/src/git/components/GitQuickActions/index.test.tsx b/app/client/src/git/components/GitQuickActions/index.test.tsx index 72138f9b9b38..9beebcc7593f 100644 --- a/app/client/src/git/components/GitQuickActions/index.test.tsx +++ b/app/client/src/git/components/GitQuickActions/index.test.tsx @@ -15,8 +15,8 @@ jest.mock("./ConnectButton", () => () => (
ConnectButton
)); -jest.mock("./AutocommitStatusbar", () => () => ( -
AutocommitStatusbar
+jest.mock("./../Statusbar", () => () => ( +
Statusbar
)); describe("QuickActions Component", () => { @@ -79,7 +79,7 @@ describe("QuickActions Component", () => { ).toBe(1); }); - it("should render AutocommitStatusbar when isAutocommitEnabled and isPollingAutocommit are true", () => { + it("should render Statusbar when isAutocommitEnabled and isPollingAutocommit are true", () => { const props = { ...defaultProps, isGitConnected: true, @@ -110,8 +110,8 @@ describe("QuickActions Component", () => { , ); - const commitButton = container.getElementsByClassName( - "t--bottom-bar-commit", + const commitButton = container.querySelectorAll( + ".t--bottom-bar-commit button", )[0]; fireEvent.click(commitButton); @@ -145,8 +145,9 @@ describe("QuickActions Component", () => { , ); - const pullButton = - container.getElementsByClassName("t--bottom-bar-pull")[0]; + const pullButton = container.querySelectorAll( + ".t--bottom-bar-pull button", + )[0]; fireEvent.click(pullButton); expect(AnalyticsUtil.logEvent).toHaveBeenCalledWith("GS_PULL_GIT_CLICK", { @@ -165,8 +166,8 @@ describe("QuickActions Component", () => { , ); - const mergeButton = container.getElementsByClassName( - "t--bottom-bar-merge", + const mergeButton = container.querySelectorAll( + ".t--bottom-bar-merge button", )[0]; fireEvent.click(mergeButton); @@ -190,8 +191,8 @@ describe("QuickActions Component", () => { , ); - const settingsButton = container.getElementsByClassName( - "t--bottom-git-settings", + const settingsButton = container.querySelectorAll( + ".t--bottom-git-settings button", )[0]; fireEvent.click(settingsButton); @@ -216,8 +217,8 @@ describe("QuickActions Component", () => { , ); - const commitButton = container.getElementsByClassName( - "t--bottom-bar-commit", + const commitButton = container.querySelectorAll( + ".t--bottom-bar-commit button", )[0]; expect(commitButton).toBeDisabled(); @@ -298,8 +299,9 @@ describe("QuickActions Component", () => { , ); - const pullButton = - container.getElementsByClassName("t--bottom-bar-pull")[0]; + const pullButton = container.querySelectorAll( + ".t--bottom-bar-pull button", + )[0]; expect(pullButton).toBeDisabled(); }); diff --git a/app/client/src/git/components/GitQuickActions/index.tsx b/app/client/src/git/components/GitQuickActions/index.tsx index e43bf2f0d3d1..42d172839f11 100644 --- a/app/client/src/git/components/GitQuickActions/index.tsx +++ b/app/client/src/git/components/GitQuickActions/index.tsx @@ -13,7 +13,7 @@ import { GitOpsTab } from "../../constants/enums"; import { GitSettingsTab } from "../../constants/enums"; import ConnectButton from "./ConnectButton"; import QuickActionButton from "./QuickActionButton"; -import AutocommitStatusbar from "./AutocommitStatusbar"; +import Statusbar from "../Statusbar"; import getPullBtnStatus from "./helpers/getPullButtonStatus"; import noop from "lodash/noop"; @@ -95,6 +95,7 @@ function GitQuickActions({ if (isProtectedMode) { discard(); } else { + // ! case: why is triggeredFromBottomBar this needed? // pull({ triggeredFromBottomBar: true }); pull(); } @@ -127,7 +128,7 @@ function GitQuickActions({ {/* */} {isAutocommitEnabled && isAutocommitPolling ? ( - + ) : ( <> { +describe("Statusbar Component", () => { afterEach(() => { jest.clearAllTimers(); }); it("should render with initial percentage 0 when completed is false", () => { - render(); + render(); const statusbar = screen.getByTestId("statusbar"); expect(statusbar).toBeInTheDocument(); @@ -31,7 +31,7 @@ describe("AutocommitStatusbar Component", () => { }); it("should increment percentage over time when completed is false", () => { - render(); + render(); const statusbar = screen.getByTestId("statusbar"); // Initial percentage @@ -57,7 +57,7 @@ describe("AutocommitStatusbar Component", () => { }); it("should not increment percentage beyond 90 when completed is false", () => { - render(); + render(); const statusbar = screen.getByTestId("statusbar"); // Advance time beyond the total interval duration @@ -74,7 +74,7 @@ describe("AutocommitStatusbar Component", () => { }); it("should set percentage to 100 when completed is true", () => { - render(); + render(); const statusbar = screen.getByTestId("statusbar"); expect(statusbar).toHaveTextContent("100%"); @@ -83,7 +83,7 @@ describe("AutocommitStatusbar Component", () => { it("should call onHide after 1 second when completed is true", () => { const onHide = jest.fn(); - render(); + render(); expect(onHide).not.toHaveBeenCalled(); // Advance timer by 1 second @@ -96,9 +96,7 @@ describe("AutocommitStatusbar Component", () => { it("should clean up intervals and timeouts on unmount", () => { const onHide = jest.fn(); - const { unmount } = render( - , - ); + const { unmount } = render(); // Start the interval act(() => { @@ -118,7 +116,7 @@ describe("AutocommitStatusbar Component", () => { it("should handle transition from false to true for completed prop", () => { const onHide = jest.fn(); const { rerender } = render( - , + , ); const statusbar = screen.getByTestId("statusbar"); @@ -129,7 +127,7 @@ describe("AutocommitStatusbar Component", () => { expect(statusbar).toHaveTextContent("10%"); // Update the completed prop to true - rerender(); + rerender(); expect(statusbar).toHaveTextContent("100%"); // Ensure onHide is called after 1 second @@ -140,13 +138,13 @@ describe("AutocommitStatusbar Component", () => { }); it("should not reset percentage when completed changes from true to false", () => { - const { rerender } = render(); + const { rerender } = render(); const statusbar = screen.getByTestId("statusbar"); expect(statusbar).toHaveTextContent("100%"); // Change completed to false - rerender(); + rerender(); expect(statusbar).toHaveTextContent("100%"); // Advance timer to check if percentage increments beyond 100% diff --git a/app/client/src/git/components/GitQuickActions/AutocommitStatusbar.tsx b/app/client/src/git/components/Statusbar/index.tsx similarity index 85% rename from app/client/src/git/components/GitQuickActions/AutocommitStatusbar.tsx rename to app/client/src/git/components/Statusbar/index.tsx index 88a3eb460734..a65332643780 100644 --- a/app/client/src/git/components/GitQuickActions/AutocommitStatusbar.tsx +++ b/app/client/src/git/components/Statusbar/index.tsx @@ -1,14 +1,13 @@ import React, { useEffect, useRef, useState } from "react"; -import { Statusbar } from "@appsmith/ads-old"; +import { Statusbar as ADSStatusBar } from "@appsmith/ads-old"; import styled from "styled-components"; -import { - AUTOCOMMIT_IN_PROGRESS_MESSAGE, - createMessage, -} from "ee/constants/messages"; -interface AutocommitStatusbarProps { +interface StatusbarProps { + message?: string; completed: boolean; onHide?: () => void; + className?: string; + testId?: string; } const PROGRESSBAR_WIDTH = 150; @@ -36,10 +35,11 @@ const StatusbarWrapper = styled.div` } `; -export default function AutocommitStatusbar({ +export default function Statusbar({ completed, + message, onHide, -}: AutocommitStatusbarProps) { +}: StatusbarProps) { const intervalRef = useRef(null); const timeoutRef = useRef(null); const [percentage, setPercentage] = useState(0); @@ -74,7 +74,7 @@ export default function AutocommitStatusbar({ }; }, [completed], - ); // Removed 'percentage' from dependencies + ); // Effect for setting percentage to 100% when completed useEffect( @@ -106,10 +106,10 @@ export default function AutocommitStatusbar({ ); return ( - - + diff --git a/app/client/src/git/components/utils.ts b/app/client/src/git/components/utils.ts new file mode 100644 index 000000000000..903f59918684 --- /dev/null +++ b/app/client/src/git/components/utils.ts @@ -0,0 +1,12 @@ +const GIT_REMOTE_URL_PATTERN = + /^((git|ssh)|([\w\-\.]+@[\w\-\.]+))(:(\/\/)?)([\w\.@\:\/\-~\(\)%]+)[^\/]$/im; + +const gitRemoteUrlRegExp = new RegExp(GIT_REMOTE_URL_PATTERN); + +/** + * isValidGitRemoteUrl: returns true if a url follows valid SSH/git url scheme, see GIT_REMOTE_URL_PATTERN + * @param url {string} remote url input + * @returns {boolean} true if valid remote url, false otherwise + */ +export const isValidGitRemoteUrl = (url: string) => + gitRemoteUrlRegExp.test(url); diff --git a/app/client/src/git/requests/checkoutBranchRequest.ts b/app/client/src/git/requests/checkoutBranchRequest.ts index 4df5da6112d3..e0c2f7e990ac 100644 --- a/app/client/src/git/requests/checkoutBranchRequest.ts +++ b/app/client/src/git/requests/checkoutBranchRequest.ts @@ -1,4 +1,4 @@ -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { CheckoutBranchRequestParams, CheckoutBranchResponse, @@ -9,7 +9,7 @@ import Api from "api/Api"; export default async function checkoutBranchRequest( branchedApplicationId: string, params: CheckoutBranchRequestParams, -): Promise> { +): AxiosPromise { return Api.get( `${GIT_BASE_URL}/checkout-branch/app/${branchedApplicationId}`, params, diff --git a/app/client/src/git/requests/commitRequest.ts b/app/client/src/git/requests/commitRequest.ts index 69541d030368..2cbd8aff65a6 100644 --- a/app/client/src/git/requests/commitRequest.ts +++ b/app/client/src/git/requests/commitRequest.ts @@ -4,12 +4,12 @@ import type { CommitResponse, } from "./commitRequest.types"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; export default async function commitRequest( branchedApplicationId: string, params: CommitRequestParams, -): Promise> { +): AxiosPromise { return Api.post( `${GIT_BASE_URL}/commit/app/${branchedApplicationId}`, params, diff --git a/app/client/src/git/requests/createBranchRequest.ts b/app/client/src/git/requests/createBranchRequest.ts index a67b5ee04099..9445ad2c3a55 100644 --- a/app/client/src/git/requests/createBranchRequest.ts +++ b/app/client/src/git/requests/createBranchRequest.ts @@ -1,4 +1,4 @@ -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { CreateBranchRequestParams, CreateBranchResponse, @@ -9,7 +9,7 @@ import Api from "api/Api"; export default async function createBranchRequest( branchedApplicationId: string, params: CreateBranchRequestParams, -): Promise> { +): AxiosPromise { return Api.post( `${GIT_BASE_URL}/create-branch/app/${branchedApplicationId}`, params, diff --git a/app/client/src/git/requests/deleteBranchRequest.ts b/app/client/src/git/requests/deleteBranchRequest.ts index 63f718506d48..cba2463d2019 100644 --- a/app/client/src/git/requests/deleteBranchRequest.ts +++ b/app/client/src/git/requests/deleteBranchRequest.ts @@ -1,4 +1,4 @@ -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { DeleteBranchRequestParams, DeleteBranchResponse, @@ -9,6 +9,6 @@ import Api from "api/Api"; export default async function deleteBranchRequest( baseApplicationId: string, params: DeleteBranchRequestParams, -): Promise> { +): AxiosPromise { return Api.delete(`${GIT_BASE_URL}/branch/app/${baseApplicationId}`, params); } diff --git a/app/client/src/git/requests/discardRequest.ts b/app/client/src/git/requests/discardRequest.ts index fda452fc206c..6dffe7513caf 100644 --- a/app/client/src/git/requests/discardRequest.ts +++ b/app/client/src/git/requests/discardRequest.ts @@ -1,9 +1,9 @@ import Api from "api/Api"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; export default async function discardRequest( branchedApplicationId: string, -): Promise> { +): AxiosPromise { return Api.put(`${GIT_BASE_URL}/discard/app/${branchedApplicationId}`); } diff --git a/app/client/src/git/requests/disconnectRequest.ts b/app/client/src/git/requests/disconnectRequest.ts index 9ec9b3a4e2b9..93c16be07baa 100644 --- a/app/client/src/git/requests/disconnectRequest.ts +++ b/app/client/src/git/requests/disconnectRequest.ts @@ -1,10 +1,10 @@ -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import { GIT_BASE_URL } from "./constants"; import type { DisconnectResponse } from "./disconnectRequest.types"; import Api from "api/Api"; export default async function disconnectRequest( baseApplicationId: string, -): Promise> { +): AxiosPromise { return Api.post(`${GIT_BASE_URL}/disconnect/app/${baseApplicationId}`); } diff --git a/app/client/src/git/requests/fetchAutocommitProgressRequest.ts b/app/client/src/git/requests/fetchAutocommitProgressRequest.ts index 8ad1c71d22c8..1f68624b780c 100644 --- a/app/client/src/git/requests/fetchAutocommitProgressRequest.ts +++ b/app/client/src/git/requests/fetchAutocommitProgressRequest.ts @@ -1,11 +1,11 @@ import Api from "api/Api"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { FetchAutocommitProgressResponse } from "./fetchAutocommitProgressRequest.types"; export default async function fetchAutocommitProgressRequest( baseApplicationId: string, -): Promise> { +): AxiosPromise { return Api.get( `${GIT_BASE_URL}/auto-commit/progress/app/${baseApplicationId}`, ); diff --git a/app/client/src/git/requests/fetchAutocommitProgressRequest.types.ts b/app/client/src/git/requests/fetchAutocommitProgressRequest.types.ts index 60f10b5fc6b3..e59d7a64d547 100644 --- a/app/client/src/git/requests/fetchAutocommitProgressRequest.types.ts +++ b/app/client/src/git/requests/fetchAutocommitProgressRequest.types.ts @@ -1,7 +1,11 @@ +import type { ApiResponse } from "api/types"; import type { AutocommitStatus } from "../constants/enums"; -export interface FetchAutocommitProgressResponse { +export interface FetchAutocommitProgressResponseData { autoCommitResponse: AutocommitStatus; progress: number; branchName: string; } + +export type FetchAutocommitProgressResponse = + ApiResponse; diff --git a/app/client/src/git/requests/fetchBranchesRequest.ts b/app/client/src/git/requests/fetchBranchesRequest.ts index 6dd616e9b0fc..c6f267179096 100644 --- a/app/client/src/git/requests/fetchBranchesRequest.ts +++ b/app/client/src/git/requests/fetchBranchesRequest.ts @@ -8,7 +8,7 @@ import type { AxiosPromise } from "axios"; export default async function fetchBranchesRequest( branchedApplicationId: string, - params: FetchBranchesRequestParams, + params: FetchBranchesRequestParams = { pruneBranches: true }, ): AxiosPromise { const queryParams = {} as FetchBranchesRequestParams; diff --git a/app/client/src/git/requests/fetchBranchesRequest.types.ts b/app/client/src/git/requests/fetchBranchesRequest.types.ts index dadab374a03b..0ea1aeed64cf 100644 --- a/app/client/src/git/requests/fetchBranchesRequest.types.ts +++ b/app/client/src/git/requests/fetchBranchesRequest.types.ts @@ -1,7 +1,7 @@ import type { ApiResponse } from "api/ApiResponses"; export interface FetchBranchesRequestParams { - pruneBranches: boolean; + pruneBranches?: boolean; } interface SingleBranch { diff --git a/app/client/src/git/requests/fetchGitMetadataRequest.ts b/app/client/src/git/requests/fetchGitMetadataRequest.ts index 136f5776f557..7b3376b8ef0e 100644 --- a/app/client/src/git/requests/fetchGitMetadataRequest.ts +++ b/app/client/src/git/requests/fetchGitMetadataRequest.ts @@ -1,10 +1,10 @@ import Api from "api/Api"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { FetchGitMetadataResponse } from "./fetchGitMetadataRequest.types"; export default async function fetchGitMetadataRequest( baseApplicationId: string, -): Promise> { +): AxiosPromise { return Api.get(`${GIT_BASE_URL}/metadata/app/${baseApplicationId}`); } diff --git a/app/client/src/git/requests/fetchGitMetadataRequest.types.ts b/app/client/src/git/requests/fetchGitMetadataRequest.types.ts index 95ef9f6ec3ab..c532052e9793 100644 --- a/app/client/src/git/requests/fetchGitMetadataRequest.types.ts +++ b/app/client/src/git/requests/fetchGitMetadataRequest.types.ts @@ -1,4 +1,6 @@ -export interface FetchGitMetadataResponse { +import type { ApiResponse } from "api/types"; + +export interface FetchGitMetadataResponseData { branchName: string; defaultBranchName: string; remoteUrl: string; @@ -13,3 +15,6 @@ export interface FetchGitMetadataResponse { }; isAutoDeploymentEnabled?: boolean; } + +export type FetchGitMetadataResponse = + ApiResponse; diff --git a/app/client/src/git/requests/fetchMergeStatusRequest.ts b/app/client/src/git/requests/fetchMergeStatusRequest.ts index 95701d5cadc8..402ac66de587 100644 --- a/app/client/src/git/requests/fetchMergeStatusRequest.ts +++ b/app/client/src/git/requests/fetchMergeStatusRequest.ts @@ -1,4 +1,4 @@ -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { FetchMergeStatusRequestParams, FetchMergeStatusResponse, @@ -9,7 +9,7 @@ import { GIT_BASE_URL } from "./constants"; export default async function fetchMergeStatusRequest( branchedApplicationId: string, params: FetchMergeStatusRequestParams, -): Promise> { +): AxiosPromise { return Api.post( `${GIT_BASE_URL}/merge/status/app/${branchedApplicationId}`, params, diff --git a/app/client/src/git/requests/fetchProtectedBranchesRequest.ts b/app/client/src/git/requests/fetchProtectedBranchesRequest.ts index 492a23c5eeca..78f54d9d7140 100644 --- a/app/client/src/git/requests/fetchProtectedBranchesRequest.ts +++ b/app/client/src/git/requests/fetchProtectedBranchesRequest.ts @@ -1,10 +1,10 @@ import Api from "api/Api"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; -import type { FetchProtectedBranches } from "./fetchProtectedBranchesRequest.types"; +import type { AxiosPromise } from "axios"; +import type { FetchProtectedBranchesResponse } from "./fetchProtectedBranchesRequest.types"; export default async function fetchProtectedBranchesRequest( baseApplicationId: string, -): Promise> { +): AxiosPromise { return Api.get(`${GIT_BASE_URL}/branch/app/${baseApplicationId}/protected`); } diff --git a/app/client/src/git/requests/fetchProtectedBranchesRequest.types.ts b/app/client/src/git/requests/fetchProtectedBranchesRequest.types.ts index 166cc05322ed..eb298bd43407 100644 --- a/app/client/src/git/requests/fetchProtectedBranchesRequest.types.ts +++ b/app/client/src/git/requests/fetchProtectedBranchesRequest.types.ts @@ -1 +1,6 @@ -export type FetchProtectedBranches = string[]; +import type { ApiResponse } from "api/types"; + +export type FetchProtectedBranchesResponseData = string[]; + +export type FetchProtectedBranchesResponse = + ApiResponse; diff --git a/app/client/src/git/requests/fetchSSHKeyRequest.ts b/app/client/src/git/requests/fetchSSHKeyRequest.ts index e61884e28a3b..29c295dc6d02 100644 --- a/app/client/src/git/requests/fetchSSHKeyRequest.ts +++ b/app/client/src/git/requests/fetchSSHKeyRequest.ts @@ -1,10 +1,10 @@ -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { FetchSSHKeyResponse } from "./fetchSSHKeyRequest.types"; import Api from "api/Api"; import { APPLICATION_BASE_URL } from "./constants"; export default async function fetchSSHKeyRequest( baseApplicationId: string, -): Promise> { +): AxiosPromise { return Api.get(`${APPLICATION_BASE_URL}/ssh-keypair/${baseApplicationId}`); } diff --git a/app/client/src/git/requests/fetchStatusRequest.ts b/app/client/src/git/requests/fetchStatusRequest.ts index 587b4f66ea0b..a0a68703f09c 100644 --- a/app/client/src/git/requests/fetchStatusRequest.ts +++ b/app/client/src/git/requests/fetchStatusRequest.ts @@ -4,11 +4,11 @@ import type { FetchStatusResponse, } from "./fetchStatusRequest.types"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; export default async function fetchStatusRequest( branchedApplicationId: string, - params: FetchStatusRequestParams, -): Promise> { + params: FetchStatusRequestParams = { compareRemote: true }, +): AxiosPromise { return Api.get(`${GIT_BASE_URL}/status/app/${branchedApplicationId}`, params); } diff --git a/app/client/src/git/requests/fetchStatusRequest.types.ts b/app/client/src/git/requests/fetchStatusRequest.types.ts index 975f94b2b483..c4400688b794 100644 --- a/app/client/src/git/requests/fetchStatusRequest.types.ts +++ b/app/client/src/git/requests/fetchStatusRequest.types.ts @@ -1,7 +1,7 @@ import type { ApiResponse } from "api/types"; export interface FetchStatusRequestParams { - compareRemote: boolean; + compareRemote?: boolean; } export interface FetchStatusResponseData { added: string[]; diff --git a/app/client/src/git/requests/generateSSHKeyRequest.ts b/app/client/src/git/requests/generateSSHKeyRequest.ts index 525c0423a16f..7c747f94371e 100644 --- a/app/client/src/git/requests/generateSSHKeyRequest.ts +++ b/app/client/src/git/requests/generateSSHKeyRequest.ts @@ -1,4 +1,4 @@ -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { GenerateSSHKeyRequestParams, GenerateSSHKeyResponse, @@ -9,7 +9,7 @@ import Api from "api/Api"; export default async function generateSSHKeyRequest( baseApplicationId: string, params: GenerateSSHKeyRequestParams, -): Promise> { +): AxiosPromise { const url = params.isImporting ? `${GIT_BASE_URL}/import/keys?keyType=${params.keyType}` : `${APPLICATION_BASE_URL}/ssh-keypair/${baseApplicationId}?keyType=${params.keyType}`; diff --git a/app/client/src/git/requests/importGitRequest.ts b/app/client/src/git/requests/importGitRequest.ts index e9378361da88..522cd187ebe3 100644 --- a/app/client/src/git/requests/importGitRequest.ts +++ b/app/client/src/git/requests/importGitRequest.ts @@ -4,11 +4,11 @@ import type { ImportGitRequestParams, ImportGitResponse, } from "./importGitRequest.types"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; export default async function importGitRequest( workspaceId: string, params: ImportGitRequestParams, -): Promise> { +): AxiosPromise { return Api.post(`${GIT_BASE_URL}/import/${workspaceId}`, params); } diff --git a/app/client/src/git/requests/mergeRequest.ts b/app/client/src/git/requests/mergeRequest.ts index ee30566c4936..daee44fad21d 100644 --- a/app/client/src/git/requests/mergeRequest.ts +++ b/app/client/src/git/requests/mergeRequest.ts @@ -1,11 +1,11 @@ import Api from "api/Api"; import type { MergeRequestParams, MergeResponse } from "./mergeRequest.types"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; export default async function mergeRequest( branchedApplicationId: string, params: MergeRequestParams, -): Promise> { +): AxiosPromise { return Api.post(`${GIT_BASE_URL}/merge/app/${branchedApplicationId}`, params); } diff --git a/app/client/src/git/requests/pullRequest.ts b/app/client/src/git/requests/pullRequest.ts index 21bd6f4f2a3c..18870918c0cd 100644 --- a/app/client/src/git/requests/pullRequest.ts +++ b/app/client/src/git/requests/pullRequest.ts @@ -1,10 +1,10 @@ import Api from "api/Api"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { PullRequestResponse } from "./pullRequest.types"; export default async function pullRequest( branchedApplicationId: string, -): Promise> { +): AxiosPromise { return Api.get(`${GIT_BASE_URL}/pull/app/${branchedApplicationId}`); } diff --git a/app/client/src/git/requests/toggleAutocommitRequest.ts b/app/client/src/git/requests/toggleAutocommitRequest.ts index deba662ded3f..85131550f108 100644 --- a/app/client/src/git/requests/toggleAutocommitRequest.ts +++ b/app/client/src/git/requests/toggleAutocommitRequest.ts @@ -1,11 +1,11 @@ import Api from "api/Api"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { ToggleAutocommitResponse } from "./toggleAutocommitRequest.types"; export default async function toggleAutocommitRequest( baseApplicationId: string, -): Promise> { +): AxiosPromise { return Api.patch( `${GIT_BASE_URL}/auto-commit/toggle/app/${baseApplicationId}`, ); diff --git a/app/client/src/git/requests/triggerAutocommitRequest.ts b/app/client/src/git/requests/triggerAutocommitRequest.ts index 01c603cb4ce5..7a2085df2d31 100644 --- a/app/client/src/git/requests/triggerAutocommitRequest.ts +++ b/app/client/src/git/requests/triggerAutocommitRequest.ts @@ -1,10 +1,10 @@ import Api from "api/Api"; import { GIT_BASE_URL } from "./constants"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { TriggerAutocommitResponse } from "./triggerAutocommitRequest.types"; export default async function triggerAutocommitRequest( branchedApplicationId: string, -): Promise> { +): AxiosPromise { return Api.post(`${GIT_BASE_URL}/auto-commit/app/${branchedApplicationId}`); } diff --git a/app/client/src/git/requests/triggerAutocommitRequest.types.ts b/app/client/src/git/requests/triggerAutocommitRequest.types.ts index 6abf80ecf226..7a3959478280 100644 --- a/app/client/src/git/requests/triggerAutocommitRequest.types.ts +++ b/app/client/src/git/requests/triggerAutocommitRequest.types.ts @@ -1,7 +1,11 @@ +import type { ApiResponse } from "api/types"; import type { AutocommitStatus } from "../constants/enums"; -export interface TriggerAutocommitResponse { +export interface TriggerAutocommitResponseData { autoCommitResponse: AutocommitStatus; progress: number; branchName: string; } + +export type TriggerAutocommitResponse = + ApiResponse; diff --git a/app/client/src/git/requests/updateGlobalProfileRequest.ts b/app/client/src/git/requests/updateGlobalProfileRequest.ts index 647ff15426f7..62d11a931a70 100644 --- a/app/client/src/git/requests/updateGlobalProfileRequest.ts +++ b/app/client/src/git/requests/updateGlobalProfileRequest.ts @@ -1,4 +1,4 @@ -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; import type { UpdateGlobalProfileRequestParams, UpdateGlobalProfileResponse, @@ -8,6 +8,6 @@ import { GIT_BASE_URL } from "./constants"; export default async function updateGlobalProfileRequest( params: UpdateGlobalProfileRequestParams, -): Promise> { +): AxiosPromise { return Api.post(`${GIT_BASE_URL}/profile/default`, params); } diff --git a/app/client/src/git/requests/updateProtectedBranchesRequest.ts b/app/client/src/git/requests/updateProtectedBranchesRequest.ts index 5af603ecaa7d..aa3822c1b5e2 100644 --- a/app/client/src/git/requests/updateProtectedBranchesRequest.ts +++ b/app/client/src/git/requests/updateProtectedBranchesRequest.ts @@ -4,12 +4,12 @@ import type { UpdateProtectedBranchesRequestParams, UpdateProtectedBranchesResponse, } from "./updateProtectedBranchesRequest.types"; -import type { AxiosResponse } from "axios"; +import type { AxiosPromise } from "axios"; export default async function updateProtectedBranchesRequest( baseApplicationId: string, params: UpdateProtectedBranchesRequestParams, -): Promise> { +): AxiosPromise { return Api.post( `${GIT_BASE_URL}/branch/app/${baseApplicationId}/protected`, params, diff --git a/app/client/src/git/sagas/fetchGitMetadataSaga.ts b/app/client/src/git/sagas/fetchGitMetadataSaga.ts new file mode 100644 index 000000000000..9f694ff10f36 --- /dev/null +++ b/app/client/src/git/sagas/fetchGitMetadataSaga.ts @@ -0,0 +1,35 @@ +import fetchGitMetadataRequest from "git/requests/fetchGitMetadataRequest"; +import type { FetchGitMetadataResponse } from "git/requests/fetchGitMetadataRequest.types"; +import { gitArtifactActions } from "git/store/gitArtifactSlice"; +import type { GitArtifactPayloadAction } from "git/store/types"; +import { call, put } from "redux-saga/effects"; +import { validateResponse } from "sagas/ErrorSagas"; + +export default function* fetchGitMetadataSaga( + action: GitArtifactPayloadAction, +) { + const { artifactType, baseArtifactId } = action.payload; + const basePayload = { artifactType, baseArtifactId }; + let response: FetchGitMetadataResponse | undefined; + + try { + response = yield call(fetchGitMetadataRequest, baseArtifactId); + const isValidResponse: boolean = yield validateResponse(response, false); + + if (response && isValidResponse) { + yield put( + gitArtifactActions.fetchGitMetadataSuccess({ + ...basePayload, + responseData: response.data, + }), + ); + } + } catch (error) { + yield put( + gitArtifactActions.fetchGitMetadataError({ + ...basePayload, + error: error as string, + }), + ); + } +} diff --git a/app/client/src/git/sagas/fetchProtectedBranchesSaga.ts b/app/client/src/git/sagas/fetchProtectedBranchesSaga.ts new file mode 100644 index 000000000000..c0c5d70e7314 --- /dev/null +++ b/app/client/src/git/sagas/fetchProtectedBranchesSaga.ts @@ -0,0 +1,36 @@ +import fetchProtectedBranchesRequest from "git/requests/fetchProtectedBranchesRequest"; +import type { FetchProtectedBranchesResponse } from "git/requests/fetchProtectedBranchesRequest.types"; +import { gitArtifactActions } from "git/store/gitArtifactSlice"; +import type { GitArtifactPayloadAction } from "git/store/types"; +import { call, put } from "redux-saga/effects"; +import { validateResponse } from "sagas/ErrorSagas"; + +export default function* fetchProtectedBranchesSaga( + action: GitArtifactPayloadAction, +) { + const { artifactType, baseArtifactId } = action.payload; + const basePayload = { artifactType, baseArtifactId }; + let response: FetchProtectedBranchesResponse | undefined; + + try { + response = yield call(fetchProtectedBranchesRequest, baseArtifactId); + + const isValidResponse: boolean = yield validateResponse(response); + + if (response && isValidResponse) { + yield put( + gitArtifactActions.fetchProtectedBranchesSuccess({ + ...basePayload, + responseData: response.data, + }), + ); + } + } catch (error) { + yield put( + gitArtifactActions.fetchProtectedBranchesError({ + ...basePayload, + error: error as string, + }), + ); + } +} diff --git a/app/client/src/git/sagas/fetchStatusSaga.ts b/app/client/src/git/sagas/fetchStatusSaga.ts new file mode 100644 index 000000000000..b0bf5d97e4da --- /dev/null +++ b/app/client/src/git/sagas/fetchStatusSaga.ts @@ -0,0 +1,43 @@ +import fetchStatusRequest from "git/requests/fetchStatusRequest"; +import type { FetchStatusResponse } from "git/requests/fetchStatusRequest.types"; +import type { FetchStatusInitPayload } from "git/store/actions/fetchStatusActions"; +import { gitArtifactActions } from "git/store/gitArtifactSlice"; +import type { GitArtifactPayloadAction } from "git/store/types"; +import { call, put } from "redux-saga/effects"; +import { validateResponse } from "sagas/ErrorSagas"; + +export default function* fetchStatusSaga( + action: GitArtifactPayloadAction, +) { + const { artifactType, baseArtifactId } = action.payload; + const basePayload = { artifactType, baseArtifactId }; + let response: FetchStatusResponse | undefined; + + try { + response = yield call(fetchStatusRequest, baseArtifactId); + const isValidResponse: boolean = yield validateResponse(response); + + if (response && isValidResponse) { + yield put( + gitArtifactActions.fetchStatusSuccess({ + ...basePayload, + responseData: response.data, + }), + ); + } + } catch (error) { + yield put( + gitArtifactActions.fetchStatusError({ + ...basePayload, + error: error as string, + }), + ); + + // ! case: BETTER ERROR HANDLING + // if ((error as Error)?.message?.includes("Auth fail")) { + // payload.error = new Error(createMessage(ERROR_GIT_AUTH_FAIL)); + // } else if ((error as Error)?.message?.includes("Invalid remote: origin")) { + // payload.error = new Error(createMessage(ERROR_GIT_INVALID_REMOTE)); + // } + } +} diff --git a/app/client/src/git/sagas/index.ts b/app/client/src/git/sagas/index.ts index 076853a8612a..60108c4d068b 100644 --- a/app/client/src/git/sagas/index.ts +++ b/app/client/src/git/sagas/index.ts @@ -1,37 +1,99 @@ -import { gitArtifactActions } from "git/store/gitArtifactSlice"; -import { all, takeLatest } from "redux-saga/effects"; +import { + actionChannel, + call, + fork, + take, + takeLatest, +} from "redux-saga/effects"; +import type { TakeableChannel } from "redux-saga"; +import type { PayloadAction } from "@reduxjs/toolkit"; +import { objectKeys } from "@appsmith/utils"; +import { gitConfigActions } from "../store/gitConfigSlice"; +import { gitArtifactActions } from "../store/gitArtifactSlice"; import connectSaga from "./connectSaga"; import commitSaga from "./commitSaga"; -import { gitConfigActions } from "git/store/gitConfigSlice"; import fetchGlobalProfileSaga from "./fetchGlobalProfileSaga"; import fetchBranchesSaga from "./fetchBranchesSaga"; import fetchLocalProfileSaga from "./fetchLocalProfileSaga"; import updateLocalProfileSaga from "./updateLocalProfileSaga"; import updateGlobalProfileSaga from "./updateGlobalProfileSaga"; +import initGitForEditorSaga from "./initGitSaga"; +import fetchGitMetadataSaga from "./fetchGitMetadataSaga"; +import triggerAutocommitSaga from "./triggerAutocommitSaga"; +import fetchStatusSaga from "./fetchStatusSaga"; +import fetchProtectedBranchesSaga from "./fetchProtectedBranchesSaga"; -export function* gitSagas() { - yield all([ - takeLatest(gitArtifactActions.connectInit.type, connectSaga), - - // branches - takeLatest(gitArtifactActions.fetchBranchesInit.type, fetchBranchesSaga), - - takeLatest(gitArtifactActions.commitInit.type, commitSaga), - takeLatest( - gitArtifactActions.fetchLocalProfileInit.type, - fetchLocalProfileSaga, - ), - takeLatest( - gitArtifactActions.updateLocalProfileInit.type, - updateLocalProfileSaga, - ), - takeLatest( - gitConfigActions.fetchGlobalProfileInit.type, - fetchGlobalProfileSaga, - ), - takeLatest( - gitConfigActions.updateGlobalProfileInit.type, - updateGlobalProfileSaga, - ), - ]); +const gitRequestBlockingActions: Record< + string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (action: PayloadAction) => Generator +> = { + // init + [gitArtifactActions.fetchGitMetadataInit.type]: fetchGitMetadataSaga, + + // connect + [gitArtifactActions.connectInit.type]: connectSaga, + + // ops + [gitArtifactActions.commitInit.type]: commitSaga, + [gitArtifactActions.fetchStatusInit.type]: fetchStatusSaga, + + // branches + [gitArtifactActions.fetchBranchesInit.type]: fetchBranchesSaga, + + // settings + [gitArtifactActions.fetchLocalProfileInit.type]: fetchLocalProfileSaga, + [gitArtifactActions.updateLocalProfileInit.type]: updateLocalProfileSaga, + [gitConfigActions.fetchGlobalProfileInit.type]: fetchGlobalProfileSaga, + [gitConfigActions.updateGlobalProfileInit.type]: updateGlobalProfileSaga, + + // autocommit + [gitArtifactActions.triggerAutocommitInit.type]: triggerAutocommitSaga, +}; + +const gitRequestNonBlockingActions: Record< + string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (action: PayloadAction) => Generator +> = { + // init + [gitArtifactActions.initGitForEditor.type]: initGitForEditorSaga, + + // settings + [gitArtifactActions.fetchProtectedBranchesInit.type]: + fetchProtectedBranchesSaga, +}; + +/** + * All git actions on the server are behind a lock, + * that means that only one action can be performed at once. + * + * To follow the same principle, we will queue all actions from the client + * as well and only perform one action at a time. + * + * This will ensure that client is not running parallel requests to the server for git + * */ +function* watchGitBlockingRequests() { + const gitActionChannel: TakeableChannel = yield actionChannel( + objectKeys(gitRequestBlockingActions), + ); + + while (true) { + const action: PayloadAction = yield take(gitActionChannel); + + yield call(gitRequestBlockingActions[action.type], action); + } +} + +function* watchGitNonBlockingRequests() { + const keys = objectKeys(gitRequestNonBlockingActions); + + for (const actionType of keys) { + yield takeLatest(actionType, gitRequestNonBlockingActions[actionType]); + } +} + +export default function* gitSagas() { + yield fork(watchGitNonBlockingRequests); + yield fork(watchGitBlockingRequests); } diff --git a/app/client/src/git/sagas/initGitSaga.ts b/app/client/src/git/sagas/initGitSaga.ts new file mode 100644 index 000000000000..c0177f32f5bd --- /dev/null +++ b/app/client/src/git/sagas/initGitSaga.ts @@ -0,0 +1,30 @@ +import { GitArtifactType } from "git/constants/enums"; +import type { InitGitForEditorPayload } from "git/store/actions/initGitActions"; +import { gitArtifactActions } from "git/store/gitArtifactSlice"; +import type { GitArtifactPayloadAction } from "git/store/types"; +import { put, take } from "redux-saga/effects"; + +export default function* initGitForEditorSaga( + action: GitArtifactPayloadAction, +) { + const { artifact, artifactType, baseArtifactId } = action.payload; + const basePayload = { artifactType, baseArtifactId }; + + yield put(gitArtifactActions.mount(basePayload)); + + if (artifactType === GitArtifactType.Application) { + if (!!artifact.gitApplicationMetadata) { + yield put(gitArtifactActions.fetchGitMetadataInit(basePayload)); + yield take(gitArtifactActions.fetchGitMetadataSuccess.type); + yield put( + gitArtifactActions.triggerAutocommitInit({ + ...basePayload, + artifactId: artifact.id, + }), + ); + yield put(gitArtifactActions.fetchBranchesInit(basePayload)); + yield put(gitArtifactActions.fetchProtectedBranchesInit(basePayload)); + yield put(gitArtifactActions.fetchStatusInit(basePayload)); + } + } +} diff --git a/app/client/src/git/sagas/triggerAutocommitSaga.ts b/app/client/src/git/sagas/triggerAutocommitSaga.ts new file mode 100644 index 000000000000..ac50a6b03c08 --- /dev/null +++ b/app/client/src/git/sagas/triggerAutocommitSaga.ts @@ -0,0 +1,131 @@ +import { triggerAutocommitSuccessAction } from "actions/gitSyncActions"; +import { AutocommitStatus, type GitArtifactType } from "git/constants/enums"; +import fetchAutocommitProgressRequest from "git/requests/fetchAutocommitProgressRequest"; +import type { + FetchAutocommitProgressResponse, + FetchAutocommitProgressResponseData, +} from "git/requests/fetchAutocommitProgressRequest.types"; +import triggerAutocommitRequest from "git/requests/triggerAutocommitRequest"; +import type { + TriggerAutocommitResponse, + TriggerAutocommitResponseData, +} from "git/requests/triggerAutocommitRequest.types"; +import type { TriggerAutocommitInitPayload } from "git/store/actions/triggerAutocommitActions"; +import { gitArtifactActions } from "git/store/gitArtifactSlice"; +import { selectAutocommitEnabled } from "git/store/selectors/gitSingleArtifactSelectors"; +import type { GitArtifactPayloadAction } from "git/store/types"; +import { + call, + cancel, + delay, + fork, + put, + select, + take, +} from "redux-saga/effects"; +import type { Task } from "redux-saga"; +import { validateResponse } from "sagas/ErrorSagas"; + +const AUTOCOMMIT_POLL_DELAY = 1000; +const AUTOCOMMIT_WHITELISTED_STATES = [ + AutocommitStatus.PUBLISHED, + AutocommitStatus.IN_PROGRESS, + AutocommitStatus.LOCKED, +]; + +interface PollAutocommitProgressParams { + artifactType: keyof typeof GitArtifactType; + baseArtifactId: string; + artifactId: string; +} + +function isAutocommitHappening( + responseData: + | TriggerAutocommitResponseData + | FetchAutocommitProgressResponseData + | undefined, +): boolean { + return ( + !!responseData && + AUTOCOMMIT_WHITELISTED_STATES.includes(responseData.autoCommitResponse) + ); +} + +function* pollAutocommitProgressSaga(params: PollAutocommitProgressParams) { + const { artifactId, artifactType, baseArtifactId } = params; + const basePayload = { artifactType, baseArtifactId }; + let triggerResponse: TriggerAutocommitResponse | undefined; + + try { + triggerResponse = yield call(triggerAutocommitRequest, artifactId); + const isValidResponse: boolean = yield validateResponse(triggerResponse); + + if (triggerResponse && isValidResponse) { + yield put(gitArtifactActions.triggerAutocommitSuccess(basePayload)); + } + } catch (error) { + yield put( + gitArtifactActions.triggerAutocommitError({ + ...basePayload, + error: error as string, + }), + ); + } + + try { + if (isAutocommitHappening(triggerResponse?.data)) { + yield put(gitArtifactActions.pollAutocommitProgressStart(basePayload)); + + while (true) { + yield put(gitArtifactActions.fetchAutocommitProgressInit(basePayload)); + const progressResponse: FetchAutocommitProgressResponse = yield call( + fetchAutocommitProgressRequest, + baseArtifactId, + ); + const isValidResponse: boolean = + yield validateResponse(progressResponse); + + if (isValidResponse && !isAutocommitHappening(progressResponse?.data)) { + yield put(gitArtifactActions.pollAutocommitProgressStop(basePayload)); + } + + if (!isValidResponse) { + yield put(gitArtifactActions.pollAutocommitProgressStop(basePayload)); + } + + yield delay(AUTOCOMMIT_POLL_DELAY); + } + } else { + yield put(gitArtifactActions.pollAutocommitProgressStop(basePayload)); + } + } catch (error) { + yield put(gitArtifactActions.pollAutocommitProgressStop(basePayload)); + yield put( + gitArtifactActions.fetchAutocommitProgressError({ + ...basePayload, + error: error as string, + }), + ); + } +} + +export default function* triggerAutocommitSaga( + action: GitArtifactPayloadAction, +) { + const { artifactId, artifactType, baseArtifactId } = action.payload; + const basePayload = { artifactType, baseArtifactId }; + const isAutocommitEnabled: boolean = yield select( + selectAutocommitEnabled, + basePayload, + ); + + if (isAutocommitEnabled) { + const params = { artifactType, baseArtifactId, artifactId }; + const pollTask: Task = yield fork(pollAutocommitProgressSaga, params); + + yield take(gitArtifactActions.pollAutocommitProgressStop.type); + yield cancel(pollTask); + } else { + yield put(triggerAutocommitSuccessAction()); + } +} diff --git a/app/client/src/git/store/actions/fetchAutocommitProgressActions.ts b/app/client/src/git/store/actions/fetchAutocommitProgressActions.ts index 2736b0c72cc4..a7d92793e4aa 100644 --- a/app/client/src/git/store/actions/fetchAutocommitProgressActions.ts +++ b/app/client/src/git/store/actions/fetchAutocommitProgressActions.ts @@ -1,8 +1,4 @@ -import type { - GitArtifactPayloadAction, - GitArtifactErrorPayloadAction, - GitAutocommitProgress, -} from "../types"; +import type { GitAsyncErrorPayload } from "../types"; import { createSingleArtifactAction } from "../helpers/createSingleArtifactAction"; export const fetchAutocommitProgressInitAction = createSingleArtifactAction( @@ -15,27 +11,19 @@ export const fetchAutocommitProgressInitAction = createSingleArtifactAction( ); export const fetchAutocommitProgressSuccessAction = createSingleArtifactAction( - ( - state, - action: GitArtifactPayloadAction<{ - autocommitProgress: GitAutocommitProgress; - }>, - ) => { + (state) => { state.apiResponses.autocommitProgress.loading = false; - state.apiResponses.autocommitProgress.value = - action.payload.autocommitProgress; return state; }, ); -export const fetchAutocommitProgressErrorAction = createSingleArtifactAction( - (state, action: GitArtifactErrorPayloadAction) => { +export const fetchAutocommitProgressErrorAction = + createSingleArtifactAction((state, action) => { const { error } = action.payload; state.apiResponses.autocommitProgress.loading = false; state.apiResponses.autocommitProgress.error = error; return state; - }, -); + }); diff --git a/app/client/src/git/store/actions/fetchGitMetadataActions.ts b/app/client/src/git/store/actions/fetchGitMetadataActions.ts new file mode 100644 index 000000000000..6bbb8d41d380 --- /dev/null +++ b/app/client/src/git/store/actions/fetchGitMetadataActions.ts @@ -0,0 +1,31 @@ +import type { GitAsyncErrorPayload, GitAsyncSuccessPayload } from "../types"; +import { createSingleArtifactAction } from "../helpers/createSingleArtifactAction"; +import type { FetchGitMetadataResponseData } from "git/requests/fetchGitMetadataRequest.types"; + +export const fetchGitMetadataInitAction = createSingleArtifactAction( + (state) => { + state.apiResponses.metadata.loading = true; + state.apiResponses.metadata.error = null; + + return state; + }, +); + +export const fetchGitMetadataSuccessAction = createSingleArtifactAction< + GitAsyncSuccessPayload +>((state, action) => { + state.apiResponses.metadata.loading = false; + state.apiResponses.metadata.value = action.payload.responseData; + + return state; +}); + +export const fetchGitMetadataErrorAction = + createSingleArtifactAction((state, action) => { + const { error } = action.payload; + + state.apiResponses.metadata.loading = false; + state.apiResponses.metadata.error = error; + + return state; + }); diff --git a/app/client/src/git/store/actions/fetchMetadataActions.ts b/app/client/src/git/store/actions/fetchMetadataActions.ts deleted file mode 100644 index f4d1fd9da2cb..000000000000 --- a/app/client/src/git/store/actions/fetchMetadataActions.ts +++ /dev/null @@ -1,33 +0,0 @@ -import type { - GitArtifactPayloadAction, - GitArtifactErrorPayloadAction, - GitMetadata, -} from "../types"; -import { createSingleArtifactAction } from "../helpers/createSingleArtifactAction"; - -export const fetchMetadataInitAction = createSingleArtifactAction((state) => { - state.apiResponses.metadata.loading = true; - state.apiResponses.metadata.error = null; - - return state; -}); - -export const fetchMetadataSuccessAction = createSingleArtifactAction( - (state, action: GitArtifactPayloadAction<{ metadata: GitMetadata }>) => { - state.apiResponses.metadata.loading = false; - state.apiResponses.metadata.value = action.payload.metadata; - - return state; - }, -); - -export const fetchMetadataErrorAction = createSingleArtifactAction( - (state, action: GitArtifactErrorPayloadAction) => { - const { error } = action.payload; - - state.apiResponses.metadata.loading = false; - state.apiResponses.metadata.error = error; - - return state; - }, -); diff --git a/app/client/src/git/store/actions/fetchProtectedBranchesActions.ts b/app/client/src/git/store/actions/fetchProtectedBranchesActions.ts index 223952a3962b..dc66ff2290c0 100644 --- a/app/client/src/git/store/actions/fetchProtectedBranchesActions.ts +++ b/app/client/src/git/store/actions/fetchProtectedBranchesActions.ts @@ -1,9 +1,6 @@ -import type { - GitArtifactPayloadAction, - GitArtifactErrorPayloadAction, - GitProtectedBranches, -} from "../types"; +import type { GitAsyncSuccessPayload, GitAsyncErrorPayload } from "../types"; import { createSingleArtifactAction } from "../helpers/createSingleArtifactAction"; +import type { FetchProtectedBranchesResponseData } from "git/requests/fetchProtectedBranchesRequest.types"; export const fetchProtectedBranchesInitAction = createSingleArtifactAction( (state) => { @@ -14,28 +11,21 @@ export const fetchProtectedBranchesInitAction = createSingleArtifactAction( }, ); -export const fetchProtectedBranchesSuccessAction = createSingleArtifactAction( - ( - state, - action: GitArtifactPayloadAction<{ - protectedBranches: GitProtectedBranches; - }>, - ) => { - state.apiResponses.protectedBranches.loading = false; - state.apiResponses.protectedBranches.value = - action.payload.protectedBranches; +export const fetchProtectedBranchesSuccessAction = createSingleArtifactAction< + GitAsyncSuccessPayload +>((state, action) => { + state.apiResponses.protectedBranches.loading = false; + state.apiResponses.protectedBranches.value = action.payload.responseData; - return state; - }, -); + return state; +}); -export const fetchProtectedBranchesErrorAction = createSingleArtifactAction( - (state, action: GitArtifactErrorPayloadAction) => { +export const fetchProtectedBranchesErrorAction = + createSingleArtifactAction((state, action) => { const { error } = action.payload; state.apiResponses.protectedBranches.loading = false; state.apiResponses.protectedBranches.error = error; return state; - }, -); + }); diff --git a/app/client/src/git/store/actions/initGitActions.ts b/app/client/src/git/store/actions/initGitActions.ts new file mode 100644 index 000000000000..5ba453b0edaf --- /dev/null +++ b/app/client/src/git/store/actions/initGitActions.ts @@ -0,0 +1,15 @@ +import type { FetchGitMetadataResponseData } from "git/requests/fetchGitMetadataRequest.types"; +import { createSingleArtifactAction } from "../helpers/createSingleArtifactAction"; + +export interface InitGitForEditorPayload { + artifact: { + id: string; + baseId: string; + gitApplicationMetadata?: Partial; + }; +} + +export const initGitForEditorAction = + createSingleArtifactAction((state) => { + return state; + }); diff --git a/app/client/src/git/store/actions/triggerAutocommitActions.ts b/app/client/src/git/store/actions/triggerAutocommitActions.ts index 414de9ed68c8..28e88ecd2f16 100644 --- a/app/client/src/git/store/actions/triggerAutocommitActions.ts +++ b/app/client/src/git/store/actions/triggerAutocommitActions.ts @@ -1,14 +1,17 @@ import { createSingleArtifactAction } from "../helpers/createSingleArtifactAction"; -import type { GitArtifactErrorPayloadAction } from "../types"; +import type { GitAsyncErrorPayload } from "../types"; -export const triggerAutocommitInitAction = createSingleArtifactAction( - (state) => { +export interface TriggerAutocommitInitPayload { + artifactId: string; +} + +export const triggerAutocommitInitAction = + createSingleArtifactAction((state) => { state.apiResponses.triggerAutocommit.loading = true; state.apiResponses.triggerAutocommit.error = null; return state; - }, -); + }); export const triggerAutocommitSuccessAction = createSingleArtifactAction( (state) => { @@ -18,13 +21,28 @@ export const triggerAutocommitSuccessAction = createSingleArtifactAction( }, ); -export const triggerAutocommitErrorAction = createSingleArtifactAction( - (state, action: GitArtifactErrorPayloadAction) => { +export const triggerAutocommitErrorAction = + createSingleArtifactAction((state, action) => { const { error } = action.payload; state.apiResponses.triggerAutocommit.loading = false; state.apiResponses.triggerAutocommit.error = error; + return state; + }); + +export const pollAutocommitProgressStartAction = createSingleArtifactAction( + (state) => { + state.ui.autocommitPolling = true; + + return state; + }, +); + +export const pollAutocommitProgressStopAction = createSingleArtifactAction( + (state) => { + state.ui.autocommitPolling = false; + return state; }, ); diff --git a/app/client/src/git/store/gitArtifactSlice.ts b/app/client/src/git/store/gitArtifactSlice.ts index da29a9f54b4a..425b6421d403 100644 --- a/app/client/src/git/store/gitArtifactSlice.ts +++ b/app/client/src/git/store/gitArtifactSlice.ts @@ -8,10 +8,10 @@ import { connectSuccessAction, } from "./actions/connectActions"; import { - fetchMetadataErrorAction, - fetchMetadataInitAction, - fetchMetadataSuccessAction, -} from "./actions/fetchMetadataActions"; + fetchGitMetadataErrorAction, + fetchGitMetadataInitAction, + fetchGitMetadataSuccessAction, +} from "./actions/fetchGitMetadataActions"; import { fetchBranchesErrorAction, fetchBranchesInitAction, @@ -79,6 +79,34 @@ import { mergeInitAction, mergeSuccessAction, } from "./actions/mergeActions"; +import { + pollAutocommitProgressStopAction, + pollAutocommitProgressStartAction, + triggerAutocommitErrorAction, + triggerAutocommitInitAction, + triggerAutocommitSuccessAction, +} from "./actions/triggerAutocommitActions"; +import { + toggleAutocommitErrorAction, + toggleAutocommitInitAction, + toggleAutocommitSuccessAction, +} from "./actions/toggleAutocommitActions"; +import { + fetchProtectedBranchesErrorAction, + fetchProtectedBranchesInitAction, + fetchProtectedBranchesSuccessAction, +} from "./actions/fetchProtectedBranchesActions"; +import { + updateProtectedBranchesErrorAction, + updateProtectedBranchesInitAction, + updateProtectedBranchesSuccessAction, +} from "./actions/updateProtectedBranchesActions"; +import { initGitForEditorAction } from "./actions/initGitActions"; +import { + fetchAutocommitProgressErrorAction, + fetchAutocommitProgressInitAction, + fetchAutocommitProgressSuccessAction, +} from "./actions/fetchAutocommitProgressActions"; const initialState: GitArtifactReduxState = {}; @@ -87,8 +115,13 @@ export const gitArtifactSlice = createSlice({ reducerPath: "git.artifact", initialState, reducers: { + // init + initGitForEditor: initGitForEditorAction, mount: mountAction, unmount: unmountAction, + fetchGitMetadataInit: fetchGitMetadataInitAction, + fetchGitMetadataSuccess: fetchGitMetadataSuccessAction, + fetchGitMetadataError: fetchGitMetadataErrorAction, // connect connectInit: connectInitAction, @@ -135,17 +168,31 @@ export const gitArtifactSlice = createSlice({ // settings toggleGitSettingsModal: toggleGitSettingsModalAction, - - // metadata - fetchMetadataInit: fetchMetadataInitAction, - fetchMetadataSuccess: fetchMetadataSuccessAction, - fetchMetadataError: fetchMetadataErrorAction, fetchLocalProfileInit: fetchLocalProfileInitAction, fetchLocalProfileSuccess: fetchLocalProfileSuccessAction, fetchLocalProfileError: fetchLocalProfileErrorAction, updateLocalProfileInit: updateLocalProfileInitAction, updateLocalProfileSuccess: updateLocalProfileSuccessAction, updateLocalProfileError: updateLocalProfileErrorAction, + fetchProtectedBranchesInit: fetchProtectedBranchesInitAction, + fetchProtectedBranchesSuccess: fetchProtectedBranchesSuccessAction, + fetchProtectedBranchesError: fetchProtectedBranchesErrorAction, + updateProtectedBranchesInit: updateProtectedBranchesInitAction, + updateProtectedBranchesSuccess: updateProtectedBranchesSuccessAction, + updateProtectedBranchesError: updateProtectedBranchesErrorAction, + + // autocommit + toggleAutocommitInit: toggleAutocommitInitAction, + toggleAutocommitSuccess: toggleAutocommitSuccessAction, + toggleAutocommitError: toggleAutocommitErrorAction, + triggerAutocommitInit: triggerAutocommitInitAction, + triggerAutocommitSuccess: triggerAutocommitSuccessAction, + triggerAutocommitError: triggerAutocommitErrorAction, + fetchAutocommitProgressInit: fetchAutocommitProgressInitAction, + fetchAutocommitProgressSuccess: fetchAutocommitProgressSuccessAction, + fetchAutocommitProgressError: fetchAutocommitProgressErrorAction, + pollAutocommitProgressStart: pollAutocommitProgressStartAction, + pollAutocommitProgressStop: pollAutocommitProgressStopAction, }, }); diff --git a/app/client/src/git/store/helpers/gitSingleArtifactInitialState.ts b/app/client/src/git/store/helpers/gitSingleArtifactInitialState.ts index a194106914a3..589eb44be8de 100644 --- a/app/client/src/git/store/helpers/gitSingleArtifactInitialState.ts +++ b/app/client/src/git/store/helpers/gitSingleArtifactInitialState.ts @@ -33,6 +33,8 @@ const gitSingleArtifactInitialUIState: GitSingleArtifactUIReduxState = { repoLimitErrorModal: { open: false, }, + autocommitModalOpen: false, + autocommitPolling: false, }; const gitSingleArtifactInitialAPIResponses: GitSingleArtifactAPIResponsesReduxState = @@ -112,7 +114,6 @@ const gitSingleArtifactInitialAPIResponses: GitSingleArtifactAPIResponsesReduxSt error: null, }, autocommitProgress: { - value: null, loading: false, error: null, }, diff --git a/app/client/src/git/store/selectors/gitSingleArtifactSelectors.ts b/app/client/src/git/store/selectors/gitSingleArtifactSelectors.ts index e67bcdc99b4a..6e36c250d148 100644 --- a/app/client/src/git/store/selectors/gitSingleArtifactSelectors.ts +++ b/app/client/src/git/store/selectors/gitSingleArtifactSelectors.ts @@ -15,6 +15,17 @@ export const selectSingleArtifact = ( ]; }; +// metadata +export const selectGitMetadata = ( + state: GitRootState, + artifactDef: GitArtifactDef, +) => selectSingleArtifact(state, artifactDef)?.apiResponses.metadata; + +export const selectGitConnected = ( + state: GitRootState, + artifactDef: GitArtifactDef, +) => !!selectGitMetadata(state, artifactDef).value; + // git ops export const selectCommit = ( state: GitRootState, @@ -43,6 +54,16 @@ export const selectPull = (state: GitRootState, artifactDef: GitArtifactDef) => selectSingleArtifact(state, artifactDef)?.apiResponses?.pull; // git branches + +export const selectCurrentBranch = ( + state: GitRootState, + artifactDef: GitArtifactDef, +) => { + const gitMetadataState = selectGitMetadata(state, artifactDef).value; + + return gitMetadataState?.branchName; +}; + export const selectBranches = ( state: GitRootState, artifactDef: GitArtifactDef, @@ -62,3 +83,34 @@ export const selectCheckoutBranch = ( state: GitRootState, artifactDef: GitArtifactDef, ) => selectSingleArtifact(state, artifactDef)?.apiResponses.checkoutBranch; + +// autocommit +export const selectAutocommitEnabled = ( + state: GitRootState, + artifactDef: GitArtifactDef, +) => { + const gitMetadata = selectGitMetadata(state, artifactDef).value; + + return gitMetadata?.autoCommitConfig?.enabled; +}; + +export const selectAutocommitPolling = ( + state: GitRootState, + artifactDef: GitArtifactDef, +) => selectSingleArtifact(state, artifactDef)?.ui.autocommitPolling; + +// protected branches +export const selectProtectedBranches = ( + state: GitRootState, + artifactDef: GitArtifactDef, +) => selectSingleArtifact(state, artifactDef)?.apiResponses.protectedBranches; + +export const selectProtectedMode = ( + state: GitRootState, + artifactDef: GitArtifactDef, +) => { + const currentBranch = selectCurrentBranch(state, artifactDef); + const protectedBranches = selectProtectedBranches(state, artifactDef).value; + + return protectedBranches?.includes(currentBranch ?? ""); +}; diff --git a/app/client/src/git/store/types.ts b/app/client/src/git/store/types.ts index 695572840d93..b3a8c18fc02a 100644 --- a/app/client/src/git/store/types.ts +++ b/app/client/src/git/store/types.ts @@ -11,13 +11,8 @@ import type { FetchBranchesResponseData } from "../requests/fetchBranchesRequest import type { FetchLocalProfileResponseData } from "../requests/fetchLocalProfileRequest.types"; import type { FetchStatusResponseData } from "git/requests/fetchStatusRequest.types"; import type { FetchMergeStatusResponseData } from "git/requests/fetchMergeStatusRequest.types"; - -// These will be updated when contracts are finalized -export type GitMetadata = Record; - -export type GitProtectedBranches = Record; - -export type GitAutocommitProgress = Record; +import type { FetchGitMetadataResponseData } from "git/requests/fetchGitMetadataRequest.types"; +import type { FetchProtectedBranchesResponseData } from "git/requests/fetchProtectedBranchesRequest.types"; export type GitSSHKey = Record; @@ -32,7 +27,7 @@ interface AsyncStateWithoutValue { error: string | null; } export interface GitSingleArtifactAPIResponsesReduxState { - metadata: AsyncState; + metadata: AsyncState; connect: AsyncStateWithoutValue; status: AsyncState; commit: AsyncStateWithoutValue; @@ -47,9 +42,9 @@ export interface GitSingleArtifactAPIResponsesReduxState { localProfile: AsyncState; updateLocalProfile: AsyncStateWithoutValue; disconnect: AsyncStateWithoutValue; - protectedBranches: AsyncState; + protectedBranches: AsyncState; updateProtectedBranches: AsyncStateWithoutValue; - autocommitProgress: AsyncState; + autocommitProgress: AsyncStateWithoutValue; toggleAutocommit: AsyncStateWithoutValue; triggerAutocommit: AsyncStateWithoutValue; sshKey: AsyncState; @@ -79,6 +74,8 @@ export interface GitSingleArtifactUIReduxState { repoLimitErrorModal: { open: boolean; }; + autocommitPolling: boolean; + autocommitModalOpen: boolean; } export interface GitSingleArtifactReduxState { ui: GitSingleArtifactUIReduxState; diff --git a/app/client/src/index.tsx b/app/client/src/index.tsx index 1b21d5f0f738..6688d1f2071a 100755 --- a/app/client/src/index.tsx +++ b/app/client/src/index.tsx @@ -1,5 +1,7 @@ // This file must be executed as early as possible to ensure the preloads are triggered ASAP import "./preload-route-chunks"; +// Initialise eval worker instance +import "utils/workerInstances"; import React from "react"; import "./wdyr"; diff --git a/app/client/src/modules/ui-builder/ui/wds/WDSCustomWidget/component/index.tsx b/app/client/src/modules/ui-builder/ui/wds/WDSCustomWidget/component/index.tsx index 9b0e7d4523be..93c7cedabb7a 100644 --- a/app/client/src/modules/ui-builder/ui/wds/WDSCustomWidget/component/index.tsx +++ b/app/client/src/modules/ui-builder/ui/wds/WDSCustomWidget/component/index.tsx @@ -80,7 +80,7 @@ export function CustomWidgetComponent(props: CustomWidgetComponentProps) { // the iframe can make changes to the model, when it needs to // this is done by sending a CUSTOM_WIDGET_UPDATE_MODEL message to the parent window const handleModelUpdate = (message: Record) => { - onUpdateModel(message.model as Record); + onUpdateModel(message as Record); }; // the iframe elements can trigger events. Triggered events here would mean diff --git a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts index 1eeaed39a8aa..fdc5fb83dc68 100644 --- a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts +++ b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts @@ -103,7 +103,8 @@ import log from "loglevel"; import { EMPTY_RESPONSE } from "components/editorComponents/emptyResponse"; import type { AppState } from "ee/reducers"; import { DEFAULT_EXECUTE_ACTION_TIMEOUT_MS } from "ee/constants/ApiConstants"; -import { evaluateActionBindings, evalWorker } from "sagas/EvaluationsSaga"; +import { evaluateActionBindings } from "sagas/EvaluationsSaga"; +import { evalWorker } from "utils/workerInstances"; import { isBlobUrl, parseBlobUrl } from "utils/AppsmithUtils"; import { getType, Types } from "utils/TypeHelpers"; import { matchPath } from "react-router"; diff --git a/app/client/src/sagas/ActionExecution/geolocationSaga.ts b/app/client/src/sagas/ActionExecution/geolocationSaga.ts index 92873a25ebc8..c2391f1f80dd 100644 --- a/app/client/src/sagas/ActionExecution/geolocationSaga.ts +++ b/app/client/src/sagas/ActionExecution/geolocationSaga.ts @@ -5,7 +5,7 @@ import { showToastOnExecutionError } from "sagas/ActionExecution/errorUtils"; import { setUserCurrentGeoLocation } from "actions/browserRequestActions"; import type { Channel } from "redux-saga"; import { channel } from "redux-saga"; -import { evalWorker } from "sagas/EvaluationsSaga"; +import { evalWorker } from "utils/workerInstances"; import type { TGetGeoLocationDescription, TWatchGeoLocationDescription, diff --git a/app/client/src/sagas/EvalWorkerActionSagas.ts b/app/client/src/sagas/EvalWorkerActionSagas.ts index 4485b67a36bd..1d12806fe858 100644 --- a/app/client/src/sagas/EvalWorkerActionSagas.ts +++ b/app/client/src/sagas/EvalWorkerActionSagas.ts @@ -12,10 +12,10 @@ import type { TMessage } from "utils/MessageUtil"; import { MessageType } from "utils/MessageUtil"; import type { ResponsePayload } from "../sagas/EvaluationsSaga"; import { - evalWorker, executeTriggerRequestSaga, updateDataTreeHandler, } from "../sagas/EvaluationsSaga"; +import { evalWorker } from "utils/workerInstances"; import { handleStoreOperations } from "./ActionExecution/StoreActionSaga"; import type { EvalTreeResponseData } from "workers/Evaluation/types"; import isEmpty from "lodash/isEmpty"; diff --git a/app/client/src/sagas/EvaluationsSaga.test.ts b/app/client/src/sagas/EvaluationsSaga.test.ts index 2cad962369ad..9e97aab028d2 100644 --- a/app/client/src/sagas/EvaluationsSaga.test.ts +++ b/app/client/src/sagas/EvaluationsSaga.test.ts @@ -2,8 +2,8 @@ import { defaultAffectedJSObjects, evalQueueBuffer, evaluateTreeSaga, - evalWorker, } from "./EvaluationsSaga"; +import { evalWorker } from "utils/workerInstances"; import { expectSaga } from "redux-saga-test-plan"; import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions"; import { select } from "redux-saga/effects"; diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index eb624fee5c8a..d1dc2dfe075d 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -25,7 +25,7 @@ import { import { getMetaWidgets, getWidgets, getWidgetsMeta } from "sagas/selectors"; import type { WidgetTypeConfigMap } from "WidgetProvider/factory"; import WidgetFactory from "WidgetProvider/factory"; -import { GracefulWorkerService } from "utils/WorkerUtil"; +import { evalWorker } from "utils/workerInstances"; import type { EvalError, EvaluationError } from "utils/DynamicBindingUtils"; import { PropertyEvaluationErrorType } from "utils/DynamicBindingUtils"; import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions"; @@ -120,18 +120,6 @@ import { getInstanceId } from "ee/selectors/tenantSelectors"; const APPSMITH_CONFIGS = getAppsmithConfigs(); -export const evalWorker = new GracefulWorkerService( - new Worker( - new URL("../workers/Evaluation/evaluation.worker.ts", import.meta.url), - { - type: "module", - // Note: the `Worker` part of the name is slightly important – LinkRelPreload_spec.js - // relies on it to find workers in the list of all requests. - name: "evalWorker", - }, - ), -); - let widgetTypeConfigMap: WidgetTypeConfigMap; export function* updateDataTreeHandler( @@ -902,5 +890,3 @@ export default function* evaluationSagaListeners() { } } } - -export { evalWorker as EvalWorker }; diff --git a/app/client/src/sagas/JSLibrarySaga.ts b/app/client/src/sagas/JSLibrarySaga.ts index 5a880cb92564..52fc076cca54 100644 --- a/app/client/src/sagas/JSLibrarySaga.ts +++ b/app/client/src/sagas/JSLibrarySaga.ts @@ -21,7 +21,7 @@ import { getCurrentApplicationId } from "selectors/editorSelectors"; import CodemirrorTernService from "utils/autocomplete/CodemirrorTernService"; import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions"; import { validateResponse } from "./ErrorSagas"; -import { EvalWorker } from "./EvaluationsSaga"; +import { evalWorker as EvalWorker } from "utils/workerInstances"; import log from "loglevel"; import { APP_MODE } from "entities/App"; import { getAppMode } from "ee/selectors/applicationSelectors"; diff --git a/app/client/src/sagas/PostEvaluationSagas.ts b/app/client/src/sagas/PostEvaluationSagas.ts index 841b031f59d7..39ea5b20938f 100644 --- a/app/client/src/sagas/PostEvaluationSagas.ts +++ b/app/client/src/sagas/PostEvaluationSagas.ts @@ -41,6 +41,7 @@ import type { EvalTreeResponseData } from "workers/Evaluation/types"; import { endSpan, startRootSpan } from "UITelemetry/generateTraces"; import { getJSActionPathNameToDisplay } from "ee/utils/actionExecutionUtils"; import { showToastOnExecutionError } from "./ActionExecution/errorUtils"; +import { waitForFetchEnvironments } from "ee/sagas/EnvironmentSagas"; let successfulBindingsMap: SuccessfulBindingMap | undefined; @@ -190,6 +191,9 @@ export function* logSuccessfulBindings( } export function* postEvalActionDispatcher(actions: Array) { + // Wait for environments api fetch before dispatching actions + yield call(waitForFetchEnvironments); + for (const action of actions) { yield put(action); } diff --git a/app/client/src/utils/workerInstances.ts b/app/client/src/utils/workerInstances.ts new file mode 100644 index 000000000000..e3edd6625017 --- /dev/null +++ b/app/client/src/utils/workerInstances.ts @@ -0,0 +1,13 @@ +import { GracefulWorkerService } from "./WorkerUtil"; + +export const evalWorker = new GracefulWorkerService( + new Worker( + new URL("../workers/Evaluation/evaluation.worker.ts", import.meta.url), + { + type: "module", + // Note: the `Worker` part of the name is slightly important – LinkRelPreload_spec.js + // relies on it to find workers in the list of all requests. + name: "evalWorker", + }, + ), +); diff --git a/app/client/src/workers/Evaluation/JSObject/index.ts b/app/client/src/workers/Evaluation/JSObject/index.ts index 79dce10b39bb..315eacb6f10d 100644 --- a/app/client/src/workers/Evaluation/JSObject/index.ts +++ b/app/client/src/workers/Evaluation/JSObject/index.ts @@ -5,7 +5,7 @@ import { EvalErrorTypes, getEvalValuePath } from "utils/DynamicBindingUtils"; import type { JSUpdate, ParsedJSSubAction } from "utils/JSPaneUtils"; import { parseJSObject, isJSFunctionProperty } from "@shared/ast"; import type DataTreeEvaluator from "workers/common/DataTreeEvaluator"; -import evaluateSync from "workers/Evaluation/evaluate"; +import { evaluateSync } from "workers/Evaluation/evaluate"; import type { DataTreeDiff } from "ee/workers/Evaluation/evaluationUtils"; import { DataTreeDiffEvent, diff --git a/app/client/src/workers/Evaluation/JSObject/test.ts b/app/client/src/workers/Evaluation/JSObject/test.ts index ccfb1c6c969c..960e790a4b17 100644 --- a/app/client/src/workers/Evaluation/JSObject/test.ts +++ b/app/client/src/workers/Evaluation/JSObject/test.ts @@ -195,9 +195,6 @@ describe("saveResolvedFunctionsAndJSUpdates", function () { { key: "myFun2", }, - { - key: "myFun2", - }, ], bindingPaths: { body: "SMART_SUBSTITUTE", @@ -216,6 +213,7 @@ describe("saveResolvedFunctionsAndJSUpdates", function () { pluginType: "JS", name: "JSObject1", actionId: "64013546b956c26882acc587", + actionNames: new Set(["myFun1", "myFun2"]), } as JSActionEntityConfig, }; const entityName = "JSObject1"; diff --git a/app/client/src/workers/Evaluation/__tests__/Actions.test.ts b/app/client/src/workers/Evaluation/__tests__/Actions.test.ts index 87dea7c35556..8300a7d5f43f 100644 --- a/app/client/src/workers/Evaluation/__tests__/Actions.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/Actions.test.ts @@ -6,7 +6,7 @@ import type { EvalContext } from "workers/Evaluation/evaluate"; import { createEvaluationContext } from "workers/Evaluation/evaluate"; import { MessageType } from "utils/MessageUtil"; import { - addDataTreeToContext, + getDataTreeContext, addPlatformFunctionsToEvalContext, } from "ee/workers/Evaluation/Actions"; import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter"; @@ -548,12 +548,13 @@ describe("Test addDataTreeToContext method", () => { const evalContext: EvalContext = {}; beforeAll(() => { - addDataTreeToContext({ - EVAL_CONTEXT: evalContext, + const EVAL_CONTEXT = getDataTreeContext({ dataTree: dataTree as unknown as DataTree, configTree, isTriggerBased: true, }); + + Object.assign(evalContext, EVAL_CONTEXT); addPlatformFunctionsToEvalContext(evalContext); }); diff --git a/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts b/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts index 65da53a793fa..e905500327a8 100644 --- a/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts @@ -1,4 +1,5 @@ -import evaluate, { +import { + evaluateSync, createEvaluationContext, evaluateAsync, } from "workers/Evaluation/evaluate"; @@ -54,29 +55,29 @@ describe("evaluateSync", () => { }); it("unescapes string before evaluation", () => { const js = '\\"Hello!\\"'; - const response = evaluate(js, {}, false); + const response = evaluateSync(js, {}, false); expect(response.result).toBe("Hello!"); }); it("evaluate string post unescape in v1", () => { const js = '[1, 2, 3].join("\\\\n")'; - const response = evaluate(js, {}, false); + const response = evaluateSync(js, {}, false); expect(response.result).toBe("1\n2\n3"); }); it("evaluate string without unescape in v2", () => { self.evaluationVersion = 2; const js = '[1, 2, 3].join("\\n")'; - const response = evaluate(js, {}, false); + const response = evaluateSync(js, {}, false); expect(response.result).toBe("1\n2\n3"); }); it("throws error for undefined js", () => { // @ts-expect-error: Types are not available - expect(() => evaluate(undefined, {})).toThrow(TypeError); + expect(() => evaluateSync(undefined, {})).toThrow(TypeError); }); it("Returns for syntax errors", () => { - const response1 = evaluate("wrongJS", {}, false); + const response1 = evaluateSync("wrongJS", {}, false); expect(response1).toStrictEqual({ result: undefined, @@ -100,7 +101,7 @@ describe("evaluateSync", () => { }, ], }); - const response2 = evaluate("{}.map()", {}, false); + const response2 = evaluateSync("{}.map()", {}, false); expect(response2).toStrictEqual({ result: undefined, @@ -130,21 +131,21 @@ describe("evaluateSync", () => { }); it("evaluates value from data tree", () => { const js = "Input1.text"; - const response = evaluate(js, dataTree, false); + const response = evaluateSync(js, dataTree, false); expect(response.result).toBe("value"); }); it("disallows unsafe function calls", () => { const js = "setImmediate(() => {}, 100)"; - const response = evaluate(js, dataTree, false); + const response = evaluateSync(js, dataTree, false); expect(response).toStrictEqual({ result: undefined, errors: [ { errorMessage: { - name: "ReferenceError", - message: "setImmediate is not defined", + name: "TypeError", + message: "setImmediate is not a function", }, errorType: "PARSE", kind: { @@ -166,51 +167,51 @@ describe("evaluateSync", () => { }); it("has access to extra library functions", () => { const js = "_.add(1,2)"; - const response = evaluate(js, dataTree, false); + const response = evaluateSync(js, dataTree, false); expect(response.result).toBe(3); }); it("evaluates functions with callback data", () => { const js = "(arg1, arg2) => arg1.value + arg2"; const callbackData = [{ value: "test" }, "1"]; - const response = evaluate(js, dataTree, false, {}, callbackData); + const response = evaluateSync(js, dataTree, false, {}, callbackData); expect(response.result).toBe("test1"); }); it("handles EXPRESSIONS with new lines", () => { let js = "\n"; - let response = evaluate(js, dataTree, false); + let response = evaluateSync(js, dataTree, false); expect(response.errors.length).toBe(0); js = "\n\n\n"; - response = evaluate(js, dataTree, false); + response = evaluateSync(js, dataTree, false); expect(response.errors.length).toBe(0); }); it("handles TRIGGERS with new lines", () => { let js = "\n"; - let response = evaluate(js, dataTree, false, undefined, undefined); + let response = evaluateSync(js, dataTree, false, undefined, undefined); expect(response.errors.length).toBe(0); js = "\n\n\n"; - response = evaluate(js, dataTree, false, undefined, undefined); + response = evaluateSync(js, dataTree, false, undefined, undefined); expect(response.errors.length).toBe(0); }); it("handles ANONYMOUS_FUNCTION with new lines", () => { let js = "\n"; - let response = evaluate(js, dataTree, false, undefined, undefined); + let response = evaluateSync(js, dataTree, false, undefined, undefined); expect(response.errors.length).toBe(0); js = "\n\n\n"; - response = evaluate(js, dataTree, false, undefined, undefined); + response = evaluateSync(js, dataTree, false, undefined, undefined); expect(response.errors.length).toBe(0); }); it("has access to this context", () => { const js = "this.contextVariable"; const thisContext = { contextVariable: "test" }; - const response = evaluate(js, dataTree, false, { thisContext }); + const response = evaluateSync(js, dataTree, false, { thisContext }); expect(response.result).toBe("test"); // there should not be any error when accessing "this" variables @@ -220,7 +221,7 @@ describe("evaluateSync", () => { it("has access to additional global context", () => { const js = "contextVariable"; const globalContext = { contextVariable: "test" }; - const response = evaluate(js, dataTree, false, { globalContext }); + const response = evaluateSync(js, dataTree, false, { globalContext }); expect(response.result).toBe("test"); expect(response.errors).toHaveLength(0); diff --git a/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts b/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts index 258a39c8fbf2..88fa7c5a5698 100644 --- a/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts @@ -18,7 +18,6 @@ import WidgetFactory from "WidgetProvider/factory"; import { generateDataTreeWidget } from "entities/DataTree/dataTreeWidget"; import { sortObjectWithArray } from "../../../utils/treeUtils"; import klona from "klona"; - import { APP_MODE } from "entities/App"; const klonaFullSpy = jest.fn(); diff --git a/app/client/src/workers/Evaluation/evaluate.ts b/app/client/src/workers/Evaluation/evaluate.ts index 96a851e89870..b8ca7cfcdc0b 100644 --- a/app/client/src/workers/Evaluation/evaluate.ts +++ b/app/client/src/workers/Evaluation/evaluate.ts @@ -23,7 +23,7 @@ import { PrimitiveErrorModifier, TypeErrorModifier, } from "./errorModifier"; -import { addDataTreeToContext } from "ee/workers/Evaluation/Actions"; +import { getDataTreeContext } from "ee/workers/Evaluation/Actions"; import { set } from "lodash"; import { klona } from "klona"; import { getEntityNameAndPropertyPath } from "ee/workers/Evaluation/evaluationUtils"; @@ -103,7 +103,7 @@ const ignoreGlobalObjectKeys = new Set([ "location", ]); -function resetWorkerGlobalScope() { +export function resetWorkerGlobalScope() { const jsLibraryAccessorSet = JSLibraryAccessor.getSet(); for (const key of Object.keys(self)) { @@ -273,14 +273,15 @@ export const createEvaluationContext = (args: createEvaluationContextArgs) => { Object.assign(EVAL_CONTEXT, context.globalContext); } - addDataTreeToContext({ - EVAL_CONTEXT, + const dataTreeContext = getDataTreeContext({ dataTree, configTree, removeEntityFunctions: !!removeEntityFunctions, isTriggerBased, }); + Object.assign(EVAL_CONTEXT, dataTreeContext); + overrideEvalContext(EVAL_CONTEXT, context?.overrideContext); return EVAL_CONTEXT; @@ -365,7 +366,7 @@ export function setEvalContext({ Object.assign(self, evalContext); } -export default function evaluateSync( +export function evaluateSync( userScript: string, dataTree: DataTree, isJSCollection: boolean, @@ -373,7 +374,8 @@ export default function evaluateSync( // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any evalArguments?: Array, - configTree?: ConfigTree, + configTree: ConfigTree = {}, + evalContextCache?: EvalContext, ): EvalResult { return (function () { const errors: EvaluationError[] = []; @@ -394,16 +396,34 @@ export default function evaluateSync( }; } - resetWorkerGlobalScope(); + self["$isDataField"] = true; + const EVAL_CONTEXT: EvalContext = {}; - setEvalContext({ - dataTree, - configTree, - isDataField: true, - isTriggerBased: isJSCollection, - context, - evalArguments, - }); + ///// Adding callback data + EVAL_CONTEXT.ARGUMENTS = evalArguments; + //// Adding contextual data not part of data tree + EVAL_CONTEXT.THIS_CONTEXT = context?.thisContext || {}; + + if (context?.globalContext) { + Object.assign(EVAL_CONTEXT, context.globalContext); + } + + if (evalContextCache) { + Object.assign(EVAL_CONTEXT, evalContextCache); + } else { + const dataTreeContext = getDataTreeContext({ + dataTree, + configTree, + removeEntityFunctions: false, + isTriggerBased: isJSCollection, + }); + + Object.assign(EVAL_CONTEXT, dataTreeContext); + } + + overrideEvalContext(EVAL_CONTEXT, context?.overrideContext); + + Object.assign(self, EVAL_CONTEXT); try { result = indirectEval(script); diff --git a/app/client/src/workers/Evaluation/fns/resetWidget.ts b/app/client/src/workers/Evaluation/fns/resetWidget.ts index ffa8cfb3a0e9..cea91ca76a91 100644 --- a/app/client/src/workers/Evaluation/fns/resetWidget.ts +++ b/app/client/src/workers/Evaluation/fns/resetWidget.ts @@ -15,7 +15,7 @@ import type { import { isWidget } from "ee/workers/Evaluation/evaluationUtils"; import { klona } from "klona"; import { getDynamicBindings, isDynamicValue } from "utils/DynamicBindingUtils"; -import evaluateSync, { setEvalContext } from "../evaluate"; +import { evaluateSync, setEvalContext } from "../evaluate"; import type { DescendantWidgetMap } from "sagas/WidgetOperationUtils"; import type { MetaState } from "reducers/entityReducers/metaReducer"; import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidgetsReducer"; diff --git a/app/client/src/workers/Evaluation/handlers/jsLibrary.ts b/app/client/src/workers/Evaluation/handlers/jsLibrary.ts index ce81c2d7bf5d..56821485f172 100644 --- a/app/client/src/workers/Evaluation/handlers/jsLibrary.ts +++ b/app/client/src/workers/Evaluation/handlers/jsLibrary.ts @@ -1,4 +1,3 @@ -import { createMessage, customJSLibraryMessages } from "ee/constants/messages"; import difference from "lodash/difference"; import type { Def } from "tern"; import { invalidEntityIdentifiers } from "workers/common/DependencyMap/utils"; @@ -34,7 +33,7 @@ enum LibraryInstallError { class ImportError extends Error { code = LibraryInstallError.ImportError; constructor(url: string) { - super(createMessage(customJSLibraryMessages.IMPORT_URL_ERROR, url)); + super(`The script at ${url} cannot be installed.`); this.name = "ImportError"; } } @@ -42,7 +41,7 @@ class ImportError extends Error { class TernDefinitionError extends Error { code = LibraryInstallError.TernDefinitionError; constructor(name: string) { - super(createMessage(customJSLibraryMessages.DEFS_FAILED_ERROR, name)); + super(`Failed to generate autocomplete definitions for ${name}.`); this.name = "TernDefinitionError"; } } diff --git a/app/client/src/workers/Evaluation/handlers/workerEnv.ts b/app/client/src/workers/Evaluation/handlers/workerEnv.ts index c9286a3b5d06..0646334fab23 100644 --- a/app/client/src/workers/Evaluation/handlers/workerEnv.ts +++ b/app/client/src/workers/Evaluation/handlers/workerEnv.ts @@ -1,9 +1,7 @@ import type { FeatureFlags } from "ee/entities/FeatureFlag"; export class WorkerEnv { - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - static flags: any; + static flags: FeatureFlags = {} as FeatureFlags; static cloudHosting: boolean; static setFeatureFlags(featureFlags: FeatureFlags) { diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index f14873229a2d..56ac7c712e87 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -38,6 +38,7 @@ import type { DataTreeDiff } from "ee/workers/Evaluation/evaluationUtils"; import { convertMicroDiffToDeepDiff, getAllPathsBasedOnDiffPaths, + isPropertyAnEntityAction, } from "ee/workers/Evaluation/evaluationUtils"; import { @@ -86,8 +87,11 @@ import { EXECUTION_PARAM_REFERENCE_REGEX, THIS_DOT_PARAMS_KEY, } from "constants/AppsmithActionConstants/ActionConstants"; -import type { EvalResult, EvaluateContext } from "workers/Evaluation/evaluate"; -import evaluateSync, { +import { + evaluateSync, + resetWorkerGlobalScope, + type EvalResult, + type EvaluateContext, evaluateAsync, setEvalContext, } from "workers/Evaluation/evaluate"; @@ -148,6 +152,8 @@ import { EComputationCacheName, type ICacheProps, } from "../AppComputationCache/types"; +import { getDataTreeContext } from "ee/workers/Evaluation/Actions"; +import { WorkerEnv } from "workers/Evaluation/handlers/workerEnv"; type SortedDependencies = Array; export interface EvalProps { @@ -1059,6 +1065,8 @@ export default class DataTreeEvaluator { staleMetaIds: string[]; contextTree: DataTree; } { + resetWorkerGlobalScope(); + const safeTree = klonaJSON(unEvalTree); const dataStore = DataStore.getDataStore(); const dataStoreClone = klonaJSON(dataStore); @@ -1084,6 +1092,16 @@ export default class DataTreeEvaluator { const { isFirstTree, metaWidgets, unevalUpdates } = options; let staleMetaIds: string[] = []; + let evalContextCache; + + if (WorkerEnv.flags.release_evaluation_scope_cache) { + evalContextCache = getDataTreeContext({ + dataTree: contextTree, + configTree: oldConfigTree, + isTriggerBased: false, + }); + } + try { for (const fullPropertyPath of evaluationOrder) { const { entityName, propertyPath } = @@ -1093,6 +1111,11 @@ export default class DataTreeEvaluator { if (!isWidgetActionOrJsObject(entity)) continue; + // Skip evaluations for actions in JSObjects + if (isPropertyAnEntityAction(entity, propertyPath, entityConfig)) { + continue; + } + // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any let unEvalPropertyValue = get(contextTree as any, fullPropertyPath); @@ -1144,6 +1167,7 @@ export default class DataTreeEvaluator { contextData, undefined, fullPropertyPath, + evalContextCache, ); } catch (error) { this.errors.push({ @@ -1209,6 +1233,13 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, parsedValue); set(safeTree, fullPropertyPath, klonaJSON(parsedValue)); + if ( + WorkerEnv.flags.release_evaluation_scope_cache && + evalContextCache + ) { + set(evalContextCache, fullPropertyPath, klonaJSON(parsedValue)); + } + staleMetaIds = staleMetaIds.concat( getStaleMetaStateIds({ entity: widgetEntity, @@ -1254,6 +1285,18 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klonaJSON(evalPropertyValue)); + + if ( + WorkerEnv.flags.release_evaluation_scope_cache && + evalContextCache + ) { + set( + evalContextCache, + fullPropertyPath, + klonaJSON(evalPropertyValue), + ); + } + break; } case ENTITY_TYPE.JSACTION: { @@ -1294,6 +1337,18 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalValue); set(safeTree, fullPropertyPath, valueForSafeTree); + + if ( + WorkerEnv.flags.release_evaluation_scope_cache && + evalContextCache + ) { + set( + evalContextCache, + fullPropertyPath, + klonaJSON(evalPropertyValue), + ); + } + JSObjectCollection.setVariableValue(evalValue, fullPropertyPath); JSObjectCollection.setPrevUnEvalState({ fullPath: fullPropertyPath, @@ -1306,6 +1361,17 @@ export default class DataTreeEvaluator { default: set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klonaJSON(evalPropertyValue)); + + if ( + WorkerEnv.flags.release_evaluation_scope_cache && + evalContextCache + ) { + set( + evalContextCache, + fullPropertyPath, + klonaJSON(evalPropertyValue), + ); + } } } } catch (error) { @@ -1417,6 +1483,7 @@ export default class DataTreeEvaluator { // eslint-disable-next-line @typescript-eslint/no-explicit-any callBackData?: Array, fullPropertyPath?: string, + evalContextCache?: EvaluateContext, ) { // Get the {{binding}} bound values let entity: DataTreeEntity | undefined = undefined; @@ -1467,6 +1534,7 @@ export default class DataTreeEvaluator { !!entity && isAnyJSAction(entity), contextData, callBackData, + evalContextCache, ); if (fullPropertyPath && evalErrors.length) { @@ -1560,6 +1628,7 @@ export default class DataTreeEvaluator { // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any callbackData?: Array, + evalContextCache?: EvaluateContext, ): EvalResult { let evalResponse: EvalResult; @@ -1574,6 +1643,8 @@ export default class DataTreeEvaluator { isJSObject, contextData, callbackData, + {}, + evalContextCache, ); } catch (error) { evalResponse = { diff --git a/app/client/src/workers/common/DataTreeEvaluator/test.ts b/app/client/src/workers/common/DataTreeEvaluator/test.ts index 8c77aeaea266..ee6a876ee267 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/test.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/test.ts @@ -768,6 +768,7 @@ describe("isDataField", () => { dependencyMap: { body: ["myFun2", "myFun1"], }, + actionNames: new Set(["myFun1", "myFun2"]), }, JSObject2: { actionId: "644242aeadc0936a9b0e71cc", @@ -821,6 +822,7 @@ describe("isDataField", () => { dependencyMap: { body: ["myFun2", "myFun1"], }, + actionNames: new Set(["myFun1", "myFun2"]), }, MainContainer: { defaultProps: {}, diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java index 52b4ea3e7335..dcd2639ec34a 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java @@ -25,6 +25,7 @@ import lombok.ToString; import lombok.experimental.FieldNameConstants; import org.springframework.data.annotation.Transient; +import org.springframework.util.StringUtils; import java.time.Instant; import java.util.HashMap; @@ -199,11 +200,11 @@ public void setPolicies(Set policies) { @Override @JsonView({Views.Internal.class}) public String getValidName() { - if (this.fullyQualifiedName == null) { - return this.name; - } else { + if (StringUtils.hasText(this.fullyQualifiedName)) { return this.fullyQualifiedName; } + + return this.name; } @Override diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java index 64bf970ce994..7a15b41f63c6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java @@ -371,27 +371,7 @@ public Mono connectArtifactToGit( return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ORIGIN)); } - Mono currentUserMono = userDataService - .getForCurrentUser() - .filter(userData -> !CollectionUtils.isEmpty(userData.getGitProfiles())) - .switchIfEmpty( - Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION, GIT_PROFILE_ERROR))); - - Mono gitUserMono = currentUserMono - .map(userData -> { - GitProfile profile = userData.getGitProfileByKey(baseArtifactId); - if (profile == null - || Boolean.TRUE.equals(profile.getUseGlobalProfile()) - || !StringUtils.hasText(profile.getAuthorName())) { - profile = userData.getGitProfileByKey(DEFAULT); - } - - GitUser gitUser = new GitUser(); - gitUser.setName(profile.getAuthorName()); - gitUser.setEmail(profile.getAuthorEmail()); - return gitUser; - }) - .cache(); + Mono gitUserMono = getGitUserForArtifactId(baseArtifactId); Mono> profileMono = gitProfileUtils .updateOrCreateGitProfileForCurrentUser(gitConnectDTO.getGitProfile(), baseArtifactId) @@ -441,9 +421,10 @@ public Mono connectArtifactToGit( .onErrorResume(error -> { log.error("Error while cloning the remote repo, ", error); - AppsmithException appsmithException = - new AppsmithException(AppsmithError.GIT_GENERIC_ERROR, error.getMessage()); - if (error instanceof TransportException) { + AppsmithException appsmithException = null; + if (error instanceof AppsmithException e) { + appsmithException = e; + } else if (error instanceof TransportException) { appsmithException = new AppsmithException(AppsmithError.INVALID_GIT_SSH_CONFIGURATION); } else if (error instanceof InvalidRemoteException) { @@ -458,6 +439,9 @@ public Mono connectArtifactToGit( appsmithException = new AppsmithException(AppsmithError.INVALID_GIT_SSH_URL); } + } else { + appsmithException = new AppsmithException( + AppsmithError.GIT_GENERIC_ERROR, error.getMessage()); } ArtifactJsonTransformationDTO jsonTransformationDTO = diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java index ffd8c26e4f50..e84ecd9f7a49 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java @@ -215,8 +215,8 @@ public Mono fetchRemoteRepository( } else if (error instanceof TimeoutException) { return Mono.error(new AppsmithException(AppsmithError.GIT_EXECUTION_TIMEOUT)); } - return Mono.error( - new AppsmithException(AppsmithError.GIT_ACTION_FAILED, "clone", error)); + return Mono.error(new AppsmithException( + AppsmithError.GIT_ACTION_FAILED, "clone", error.getMessage())); }); }); } @@ -325,13 +325,16 @@ public Mono createFirstCommit(ArtifactJsonTransformationDTO jsonTransfor jsonTransformationDTO.getBaseArtifactId(), jsonTransformationDTO.getRepoName()); - return fsGitHandler.commitArtifact( - repoSuffix, - commitDTO.getMessage(), - commitDTO.getAuthor().getName(), - commitDTO.getAuthor().getEmail(), - true, - commitDTO.getIsAmendCommit()); + return fsGitHandler + .commitArtifact( + repoSuffix, + commitDTO.getMessage(), + commitDTO.getAuthor().getName(), + commitDTO.getAuthor().getEmail(), + true, + commitDTO.getIsAmendCommit()) + .onErrorResume(error -> Mono.error( + new AppsmithException(AppsmithError.GIT_ACTION_FAILED, "commit", error.getMessage()))); } @Override diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java index 3d87c2fdcb99..3755272f84c6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java @@ -2,6 +2,7 @@ import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.Datasource; +import com.appsmith.external.models.PluginType; import com.appsmith.external.models.Policy; import com.appsmith.server.actioncollections.base.ActionCollectionService; import com.appsmith.server.constants.FieldName; @@ -482,11 +483,17 @@ private void updateActionNameBeforeMerge( } String oldId = newAction.getId().split("_")[1]; newAction.setId(newNameAction + "_" + oldId); + + if (PluginType.JS.equals(newAction.getPluginType())) { + newAction.getUnpublishedAction().setFullyQualifiedName(newNameAction); + } + newAction.getUnpublishedAction().setName(newNameAction); - newAction.getUnpublishedAction().setFullyQualifiedName(newNameAction); if (newAction.getPublishedAction() != null) { newAction.getPublishedAction().setName(newNameAction); - newAction.getPublishedAction().setFullyQualifiedName(newNameAction); + if (PluginType.JS.equals(newAction.getPluginType())) { + newAction.getPublishedAction().setFullyQualifiedName(newNameAction); + } } mappedImportableResourcesDTO.getRefactoringNameReference().put(oldNameAction, newNameAction); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/refactors/NewActionRefactoringServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/refactors/NewActionRefactoringServiceCEImpl.java index be47951274b8..eed4aa4b86de 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/refactors/NewActionRefactoringServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/refactors/NewActionRefactoringServiceCEImpl.java @@ -130,9 +130,14 @@ public Mono updateRefactoredEntity(RefactorEntityNameDTO refactorEntityNam .map(branchedAction -> newActionService.generateActionByViewMode(branchedAction, false)) .flatMap(action -> { action.setName(refactorEntityNameDTO.getNewName()); - if (StringUtils.hasLength(refactorEntityNameDTO.getCollectionName())) { + if (!PluginType.JS.equals(action.getPluginType())) { + return newActionService.updateUnpublishedAction(action.getId(), action); + } + + if (StringUtils.hasText(refactorEntityNameDTO.getCollectionName())) { action.setFullyQualifiedName(refactorEntityNameDTO.getNewFullyQualifiedName()); } + return newActionService.updateUnpublishedAction(action.getId(), action); }) .then(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationRepositoryCE.java index 4974bb4425b4..1f3f311c121f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationRepositoryCE.java @@ -6,24 +6,9 @@ import com.appsmith.server.repositories.CustomApplicationRepository; import org.springframework.stereotype.Repository; import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; - -import java.util.List; @Repository public interface ApplicationRepositoryCE extends BaseRepository, CustomApplicationRepository { - - Flux findByIdIn(List ids); - - Flux findByWorkspaceId(String workspaceId); - - Mono countByWorkspaceId(String workspaceId); - + // All methods moved to CustomApplicationRepositoryCE Flux findIdsByWorkspaceId(String workspaceId); - - Flux findByClonedFromApplicationId(String clonedFromApplicationId); - - Mono countByDeletedAtNull(); - - Mono findByIdAndExportWithConfiguration(String id, boolean exportWithConfiguration); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCE.java index 33d45af9ece7..d37a6ab7650c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCE.java @@ -81,4 +81,16 @@ Mono getAllApplicationsCountAccessibleToARoleWithPermission( Flux findAllBranchedApplicationIdsByBranchedApplicationId( String branchedApplicationId, AclPermission permission); + + Flux findByIdIn(List ids); + + Flux findByWorkspaceId(String workspaceId); + + Mono countByWorkspaceId(String workspaceId); + + Flux findByClonedFromApplicationId(String clonedFromApplicationId); + + Mono countByDeletedAtNull(); + + Mono findByIdAndExportWithConfiguration(String id, boolean exportWithConfiguration); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java index fa4b56c351af..426f57d80057 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java @@ -298,4 +298,42 @@ public Flux findAllBranchedApplicationIdsByBranchedApplicationId( } }); } + + @Override + public Flux findByIdIn(List ids) { + final BridgeQuery q = Bridge.in(Application.Fields.id, ids); + return queryBuilder().criteria(q).all(); + } + + @Override + public Flux findByWorkspaceId(String workspaceId) { + final BridgeQuery q = Bridge.equal(Application.Fields.workspaceId, workspaceId); + return queryBuilder().criteria(q).all(); + } + + @Override + public Mono countByWorkspaceId(String workspaceId) { + final BridgeQuery q = Bridge.equal(Application.Fields.workspaceId, workspaceId); + return queryBuilder().criteria(q).count(); + } + + @Override + public Flux findByClonedFromApplicationId(String clonedFromApplicationId) { + final BridgeQuery q = + Bridge.equal(Application.Fields.clonedFromApplicationId, clonedFromApplicationId); + return queryBuilder().criteria(q).all(); + } + + @Override + public Mono countByDeletedAtNull() { + final BridgeQuery q = Bridge.isNull(Application.Fields.deletedAt); + return queryBuilder().criteria(q).count(); + } + + @Override + public Mono findByIdAndExportWithConfiguration(String id, boolean exportWithConfiguration) { + final BridgeQuery q = Bridge.equal(Application.Fields.id, id) + .equal(Application.Fields.exportWithConfiguration, exportWithConfiguration); + return queryBuilder().criteria(q).one(); + } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/git/ops/GitConnectTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/git/ops/GitConnectTests.java new file mode 100644 index 000000000000..d1d576bbcad8 --- /dev/null +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/git/ops/GitConnectTests.java @@ -0,0 +1,485 @@ +package com.appsmith.server.git.ops; + +import com.appsmith.external.git.handler.FSGitHandler; +import com.appsmith.server.acl.AclPermission; +import com.appsmith.server.constants.ArtifactType; +import com.appsmith.server.constants.FieldName; +import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.GitArtifactMetadata; +import com.appsmith.server.domains.GitAuth; +import com.appsmith.server.domains.GitProfile; +import com.appsmith.server.domains.User; +import com.appsmith.server.domains.Workspace; +import com.appsmith.server.dtos.GitConnectDTO; +import com.appsmith.server.exceptions.AppsmithError; +import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.git.central.CentralGitService; +import com.appsmith.server.git.central.GitHandlingService; +import com.appsmith.server.git.central.GitType; +import com.appsmith.server.helpers.CommonGitFileUtils; +import com.appsmith.server.helpers.GitCloudServicesUtils; +import com.appsmith.server.repositories.ApplicationRepository; +import com.appsmith.server.services.ApplicationPageService; +import com.appsmith.server.services.UserService; +import com.appsmith.server.services.WorkspaceService; +import com.appsmith.server.solutions.ApplicationPermission; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.mockito.Mockito; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.security.test.context.support.WithUserDetails; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Instant; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; + +@SpringBootTest +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class GitConnectTests { + + @Autowired + CentralGitService centralGitService; + + @Autowired + ApplicationPermission applicationPermission; + + @Autowired + UserService userService; + + @Autowired + ApplicationPageService applicationPageService; + + @Autowired + ApplicationRepository applicationRepository; + + @Autowired + WorkspaceService workspaceService; + + @SpyBean + FSGitHandler fsGitHandler; + + @SpyBean + CommonGitFileUtils commonGitFileUtils; + + @SpyBean + GitCloudServicesUtils gitCloudServicesUtils; + + @SpyBean + GitHandlingService gitHandlingService; + + @Test + @WithUserDetails(value = "api_user") + public void connectArtifactToGit_withEmptyRemoteUrl_throwsInvalidParameterException() { + + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl(null); + gitConnectDTO.setGitProfile(new GitProfile()); + Mono applicationMono = centralGitService + .connectArtifactToGit("testID", gitConnectDTO, "baseUrl", ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .expectErrorSatisfies(throwable -> { + assertThat(throwable).isInstanceOf(AppsmithException.class); + assertThat(throwable) + .message() + .containsAnyOf( + AppsmithError.INVALID_PARAMETER.getMessage("browserSupportedRemoteUrl"), + AppsmithError.INVALID_PARAMETER.getMessage("remoteUrl")); + }) + .verify(); + } + + @Test + @WithUserDetails(value = "api_user") + public void connectArtifactToGit_withEmptyOriginHeader_throwsInvalidParameterException() { + + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl("git@github.com:test/testRepo.git"); + gitConnectDTO.setGitProfile(new GitProfile()); + + Mono applicationMono = centralGitService + .connectArtifactToGit("testID", gitConnectDTO, null, ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException + && throwable.getMessage().contains(AppsmithError.INVALID_PARAMETER.getMessage("origin"))) + .verify(); + } + + @Test + @WithUserDetails(value = "api_user") + public void connectArtifactToGit_whenConnectingMorePrivateReposThanSupported_throwsException() { + Workspace workspace = new Workspace(); + workspace.setName("Limit Private Repo Test Workspace"); + String limitPrivateRepoTestWorkspaceId = + workspaceService.create(workspace).map(Workspace::getId).block(); + + Mockito.doReturn(Mono.just(0)) + .when(gitCloudServicesUtils) + .getPrivateRepoLimitForOrg(Mockito.anyString(), Mockito.anyBoolean()); + + Application testApplication = new Application(); + GitArtifactMetadata gitArtifactMetadata = new GitArtifactMetadata(); + GitAuth gitAuth = new GitAuth(); + gitAuth.setPublicKey("testkey"); + gitAuth.setPrivateKey("privatekey"); + gitAuth.setGeneratedAt(Instant.now()); + gitAuth.setDocUrl("docUrl"); + gitArtifactMetadata.setGitAuth(gitAuth); + testApplication.setGitApplicationMetadata(gitArtifactMetadata); + testApplication.setName("connectArtifactToGit_WithNonEmptyPublishedPages"); + testApplication.setWorkspaceId(limitPrivateRepoTestWorkspaceId); + Application application = + applicationPageService.createApplication(testApplication).block(); + + GitProfile gitProfile = new GitProfile(); + gitProfile.setAuthorEmail("test@email.com"); + gitProfile.setAuthorName("testUser"); + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl("git@github.com:test/testRepo.git"); + gitConnectDTO.setGitProfile(gitProfile); + Mono applicationMono = centralGitService + .connectArtifactToGit( + application.getId(), gitConnectDTO, "baseUrl", ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .expectErrorMatches(error -> error instanceof AppsmithException + && error.getMessage().equals(AppsmithError.GIT_APPLICATION_LIMIT_ERROR.getMessage())) + .verify(); + } + + @WithUserDetails("api_user") + @Test + public void connectArtifactToGit_whenUserDoesNotHaveRequiredPermission_operationFails() { + Application application = + createApplicationAndRemovePermissionFromApplication(applicationPermission.getGitConnectPermission()); + + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl("git@github.com:test/testRepo.git"); + gitConnectDTO.setGitProfile(new GitProfile()); + Mono applicationMono = centralGitService + .connectArtifactToGit( + application.getId(), gitConnectDTO, "baseUrl", ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .expectErrorMessage( + AppsmithError.ACL_NO_RESOURCE_FOUND.getMessage(FieldName.APPLICATION, application.getId())) + .verify(); + } + + /** + * This method creates a workspace, creates an application in the workspace and removes the + * create application permission from the workspace for the api_user. + * + * @return Created Application + */ + private Application createApplicationAndRemovePermissionFromApplication(AclPermission permission) { + User apiUser = userService.findByEmail("api_user").block(); + + Workspace toCreate = new Workspace(); + toCreate.setName("Workspace_" + UUID.randomUUID()); + Workspace workspace = + workspaceService.create(toCreate, apiUser, Boolean.FALSE).block(); + assertThat(workspace).isNotNull(); + + Application testApplication = new Application(); + testApplication.setWorkspaceId(workspace.getId()); + testApplication.setName("Test App"); + Application application1 = + applicationPageService.createApplication(testApplication).block(); + + assertThat(application1).isNotNull(); + + // remove permission from the application for the api user + application1.getPolicyMap().remove(permission.getValue()); + + return applicationRepository.save(application1).block(); + } + + @Test + @WithUserDetails(value = "api_user") + public void connectArtifactToGit_whenCloneOperationFails_throwsGitException() { + + Mockito.doReturn(Mono.error(new Exception("error message"))) + .when(fsGitHandler) + .cloneRemoteIntoArtifactRepo(any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()); + Mockito.doReturn(Mono.just(true)).when(commonGitFileUtils).deleteLocalRepo(any(Path.class)); + + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl("git@github.com:test/testRepo.git"); + GitProfile testUserProfile = new GitProfile(); + testUserProfile.setAuthorEmail("test@email.com"); + testUserProfile.setAuthorName("testUser"); + gitConnectDTO.setGitProfile(testUserProfile); + + User apiUser = userService.findByEmail("api_user").block(); + Workspace toCreate = new Workspace(); + toCreate.setName("Workspace_" + UUID.randomUUID()); + Workspace workspace = + workspaceService.create(toCreate, apiUser, Boolean.FALSE).block(); + assertThat(workspace).isNotNull(); + + // Create application + Application testApplication = new Application(); + testApplication.setWorkspaceId(workspace.getId()); + testApplication.setName("Test App"); + GitArtifactMetadata gitArtifactMetadata = new GitArtifactMetadata(); + GitAuth gitAuth = new GitAuth(); + gitAuth.setPublicKey("testkey"); + gitAuth.setPrivateKey("privatekey"); + gitArtifactMetadata.setGitAuth(gitAuth); + gitArtifactMetadata.setDefaultApplicationId(testApplication.getId()); + gitArtifactMetadata.setRepoName("testRepo"); + testApplication.setGitApplicationMetadata(gitArtifactMetadata); + Application application1 = + applicationPageService.createApplication(testApplication).block(); + + assertThat(application1).isNotNull(); + + Mono applicationMono = centralGitService + .connectArtifactToGit( + application1.getId(), gitConnectDTO, "baseUrl", ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException + && throwable + .getMessage() + .equals(AppsmithError.GIT_ACTION_FAILED.getMessage("clone", "error message"))) + .verify(); + } + + @Test + @WithUserDetails(value = "api_user") + public void connectArtifactToGit_whenUsingGlobalProfile_completesSuccessfully() throws IOException { + + Mockito.doReturn(Mono.just("defaultBranchName")) + .when(fsGitHandler) + .cloneRemoteIntoArtifactRepo(any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()); + Mockito.doReturn(Mono.just("commit")) + .when(fsGitHandler) + .commitArtifact( + any(Path.class), + Mockito.anyString(), + Mockito.anyString(), + Mockito.anyString(), + Mockito.anyBoolean(), + Mockito.anyBoolean()); + Mockito.doReturn(Mono.just(true)).when(fsGitHandler).checkoutToBranch(any(Path.class), Mockito.anyString()); + Mockito.doReturn(Mono.just("success")) + .when(fsGitHandler) + .pushApplication( + any(Path.class), + Mockito.anyString(), + Mockito.anyString(), + Mockito.anyString(), + Mockito.anyString()); + Mockito.doReturn(Mono.just(Paths.get(""))) + .when(commonGitFileUtils) + .saveArtifactToLocalRepoWithAnalytics(any(Path.class), any(), Mockito.anyString()); + Mockito.doReturn(Mono.just(true)).when(commonGitFileUtils).checkIfDirectoryIsEmpty(any(Path.class)); + Mockito.doReturn(Mono.just(Paths.get("textPath"))) + .when(commonGitFileUtils) + .initializeReadme(any(Path.class), Mockito.anyString(), Mockito.anyString()); + Mockito.doReturn(Mono.just(true)).when(commonGitFileUtils).deleteLocalRepo(any(Path.class)); + + User apiUser = userService.findByEmail("api_user").block(); + Workspace toCreate = new Workspace(); + toCreate.setName("Workspace_" + UUID.randomUUID()); + Workspace workspace = + workspaceService.create(toCreate, apiUser, Boolean.FALSE).block(); + assertThat(workspace).isNotNull(); + + GitProfile gitProfile = new GitProfile(); + gitProfile.setAuthorName(null); + gitProfile.setAuthorEmail(null); + gitProfile.setUseGlobalProfile(true); + Application testApplication = new Application(); + GitArtifactMetadata gitArtifactMetadata = new GitArtifactMetadata(); + GitAuth gitAuth = new GitAuth(); + gitAuth.setPublicKey("testkey"); + gitAuth.setPrivateKey("privatekey"); + gitArtifactMetadata.setGitAuth(gitAuth); + testApplication.setGitApplicationMetadata(gitArtifactMetadata); + testApplication.setName("emptyDefaultProfileConnectTest"); + testApplication.setWorkspaceId(workspace.getId()); + Application application1 = + applicationPageService.createApplication(testApplication).block(); + assertThat(application1).isNotNull(); + + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl("git@github.com:test/testRepo.git"); + gitConnectDTO.setGitProfile(gitProfile); + Mono applicationMono = centralGitService + .connectArtifactToGit( + application1.getId(), gitConnectDTO, "baseUrl", ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .assertNext(application -> { + GitArtifactMetadata gitArtifactMetadata1 = application.getGitApplicationMetadata(); + assertThat(gitArtifactMetadata1.getRemoteUrl()).isEqualTo(gitConnectDTO.getRemoteUrl()); + assertThat(gitArtifactMetadata1.getBranchName()).isEqualTo("defaultBranchName"); + }) + .verifyComplete(); + } + + @Test + @WithUserDetails(value = "api_user") + public void connectArtifactToGit_whenUsingIncompleteLocalProfile_throwsAuthorNameUnavailableError() { + User apiUser = userService.findByEmail("api_user").block(); + Workspace toCreate = new Workspace(); + toCreate.setName("Workspace_" + UUID.randomUUID()); + Workspace workspace = + workspaceService.create(toCreate, apiUser, Boolean.FALSE).block(); + assertThat(workspace).isNotNull(); + + GitProfile gitProfile = new GitProfile(); + gitProfile.setAuthorName(null); + gitProfile.setAuthorEmail(null); + // Use repo specific git profile but as this is empty default profile will be used as a fallback + gitProfile.setUseGlobalProfile(false); + Application testApplication = new Application(); + GitArtifactMetadata gitArtifactMetadata = new GitArtifactMetadata(); + GitAuth gitAuth = new GitAuth(); + gitAuth.setPublicKey("testkey"); + gitAuth.setPrivateKey("privatekey"); + gitAuth.setGeneratedAt(Instant.now()); + gitAuth.setDocUrl("docUrl"); + gitArtifactMetadata.setGitAuth(gitAuth); + gitArtifactMetadata.setRemoteUrl("git@github.com:test/testRepo.git"); + gitArtifactMetadata.setBranchName("defaultBranchNameFromRemote"); + gitArtifactMetadata.setRepoName("testRepo"); + testApplication.setGitApplicationMetadata(gitArtifactMetadata); + testApplication.setName("localGitProfile"); + testApplication.setWorkspaceId(workspace.getId()); + Application application1 = + applicationPageService.createApplication(testApplication).block(); + assertThat(application1).isNotNull(); + + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl("git@github.com:test/testRepo.git"); + gitConnectDTO.setGitProfile(gitProfile); + Mono applicationMono = centralGitService + .connectArtifactToGit( + application1.getId(), gitConnectDTO, "baseUrl", ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException + && throwable.getMessage().contains(AppsmithError.INVALID_PARAMETER.getMessage("Author Name"))) + .verify(); + } + + @Test + @WithUserDetails(value = "api_user") + public void connectArtifactToGit_whenClonedRepoIsNotEmpty_throwsException() throws IOException { + + Mockito.doReturn(Mono.just("defaultBranchName")) + .when(fsGitHandler) + .cloneRemoteIntoArtifactRepo(any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()); + Mockito.doReturn(Mono.just(false)).when(commonGitFileUtils).checkIfDirectoryIsEmpty(any(Path.class)); + + User apiUser = userService.findByEmail("api_user").block(); + Workspace toCreate = new Workspace(); + toCreate.setName("Workspace_" + UUID.randomUUID()); + Workspace workspace = + workspaceService.create(toCreate, apiUser, Boolean.FALSE).block(); + assertThat(workspace).isNotNull(); + + Application testApplication = new Application(); + GitArtifactMetadata gitArtifactMetadata = new GitArtifactMetadata(); + GitAuth gitAuth = new GitAuth(); + gitAuth.setPublicKey("testkey"); + gitAuth.setPrivateKey("privatekey"); + gitArtifactMetadata.setGitAuth(gitAuth); + testApplication.setGitApplicationMetadata(gitArtifactMetadata); + testApplication.setName("ValidTest TestApp"); + testApplication.setWorkspaceId(workspace.getId()); + Application application1 = + applicationPageService.createApplication(testApplication).block(); + assertThat(application1).isNotNull(); + + GitProfile gitProfile = new GitProfile(); + gitProfile.setAuthorEmail("test@email.com"); + gitProfile.setAuthorName("testUser"); + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl("git@github.com:test/testRepo.git"); + gitConnectDTO.setGitProfile(gitProfile); + Mono applicationMono = centralGitService + .connectArtifactToGit( + application1.getId(), gitConnectDTO, "baseUrl", ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException + && throwable.getMessage().contains(AppsmithError.INVALID_GIT_REPO.getMessage())) + .verify(); + } + + @Test + @WithUserDetails(value = "api_user") + public void connectArtifactToGit_whenDefaultCommitFails_throwsException() throws IOException { + + Mockito.doReturn(Mono.just("defaultBranchName")) + .when(fsGitHandler) + .cloneRemoteIntoArtifactRepo(any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()); + Mockito.doReturn(Mono.just(true)).when(commonGitFileUtils).checkIfDirectoryIsEmpty(any(Path.class)); + Mockito.doReturn(Mono.error(new Exception("default commit error"))) + .when(gitHandlingService) + .createFirstCommit(Mockito.any(), Mockito.any()); + + User apiUser = userService.findByEmail("api_user").block(); + Workspace toCreate = new Workspace(); + toCreate.setName("Workspace_" + UUID.randomUUID()); + Workspace workspace = + workspaceService.create(toCreate, apiUser, Boolean.FALSE).block(); + assertThat(workspace).isNotNull(); + + Application testApplication = new Application(); + GitArtifactMetadata gitArtifactMetadata = new GitArtifactMetadata(); + GitAuth gitAuth = new GitAuth(); + gitAuth.setPublicKey("testkey"); + gitAuth.setPrivateKey("privatekey"); + gitArtifactMetadata.setGitAuth(gitAuth); + testApplication.setGitApplicationMetadata(gitArtifactMetadata); + testApplication.setName("ValidTest TestApp"); + testApplication.setWorkspaceId(workspace.getId()); + Application application1 = + applicationPageService.createApplication(testApplication).block(); + assertThat(application1).isNotNull(); + + GitProfile gitProfile = new GitProfile(); + gitProfile.setAuthorEmail("test@email.com"); + gitProfile.setAuthorName("testUser"); + GitConnectDTO gitConnectDTO = new GitConnectDTO(); + gitConnectDTO.setRemoteUrl("git@github.com:test/testRepo.git"); + gitConnectDTO.setGitProfile(gitProfile); + Mono applicationMono = centralGitService + .connectArtifactToGit( + application1.getId(), gitConnectDTO, "baseUrl", ArtifactType.APPLICATION, GitType.FILE_SYSTEM) + .map(artifact -> (Application) artifact); + + StepVerifier.create(applicationMono) + .expectErrorMatches(throwable -> throwable.getMessage().contains("default commit error")) + .verify(); + } + + // TODO : write them as templatized integration tests + // - commit failed (no write permission perhaps, or protected branch) + // - successful connection + // - connectArtifactToGit_cancelledMidway_cloneSuccess +} diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java index afdc45a785b5..8bfc73e2f441 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java @@ -921,7 +921,6 @@ public void getActionsExecuteOnLoadWithAstLogic() { Mono pageMono = createPage(app, testPage).cache(); Mono testMono = createComplexAppForExecuteOnLoad(pageMono); - Mockito.when(astService.getPossibleReferencesFromDynamicBinding( List.of("\"anIgnoredAction.data:\" + aGetAction.data"), EVALUATION_VERSION)) .thenReturn(Flux.just(Tuples.of( diff --git a/deploy/docker/tests/pg-test-utils.sh b/deploy/docker/tests/pg-test-utils.sh new file mode 100644 index 000000000000..1cbde82eb7f8 --- /dev/null +++ b/deploy/docker/tests/pg-test-utils.sh @@ -0,0 +1,70 @@ +#!/bin/bash + +# Default container name if not provided +container_name=${container_name:-appsmith-docker-test} + +# Function to check if the Appsmith instance is up +is_appsmith_instance_ready() { + local max_retries=200 + local retry_count=0 + local response_code + + while [ $retry_count -lt $max_retries ]; do + response_code=$(curl -s -o /dev/null -w "%{http_code}" http://localhost/health) + if [[ $response_code -eq 200 ]]; then + echo "Appsmith instance is ready." + return 0 + fi + echo "Waiting for Appsmith instance to be ready... (Attempt: $((retry_count + 1)))" + retry_count=$((retry_count + 1)) + sleep 2 + done + return 1 +} + +# Function to wait until the postgres is ready +wait_for_postgres() { + local max_retries=200 + local retry_count=0 + + while [ $retry_count -lt $max_retries ]; do + if docker exec "${container_name}" pg_isready; then + echo "Postgres is ready." + return 0 + fi + echo "Waiting for Postgres to be ready... (Attempt: $((retry_count + 1)))" + retry_count=$((retry_count + 1)) + sleep 2 + done +} + +# Function to check if the user exists in the database +check_user_exists() { + local user + user=$1 + local max_retries=200 + local retry_count=0 + while [ $retry_count -lt $max_retries ]; do + if docker exec "${container_name}" bash -c "psql -p 5432 -U postgres -c \"SELECT 1 FROM pg_roles WHERE rolname='$user';\" | grep -q 1"; then + echo "$user user exists." + return 0 + fi + echo "Waiting for $user user to be created... (Attempt: $((retry_count + 1)))" + retry_count=$((retry_count + 1)) + sleep 1 + done + echo "$user user does not exist." + return 1 +} + +# Function to check if the Appsmith user has read access to databases +check_user_datasource_access_with_host_port_wo_auth() { + docker exec "${container_name}" bash -c "psql -h 127.0.0.1 -p 5432 -U postgres -c '\l'" + return $? +} + +# Function to check if the Appsmith user has read access to databases +check_user_datasource_access_with_local_port_wo_auth() { + docker exec "${container_name}" bash -c "psql -p 5432 -U postgres -c '\l'" + return $? +} \ No newline at end of file diff --git a/deploy/docker/tests/test-pg-auth.sh b/deploy/docker/tests/test-pg-auth.sh index 854de8a962eb..c5237b1c2031 100755 --- a/deploy/docker/tests/test-pg-auth.sh +++ b/deploy/docker/tests/test-pg-auth.sh @@ -1,8 +1,8 @@ #!/bin/bash set -o errexit -# set -x source ./composes.sh +source ./pg-test-utils.sh # Function to update the APPSMITH_DB_URL in docker.env @@ -13,41 +13,6 @@ update_db_url() { docker exec "${container_name}" bash -c "sed -i 's|^APPSMITH_POSTGRES_DB_URL=|APPSMITH_DB_URL=|' /appsmith-stacks/configuration/docker.env" } -# Function to check if the Appsmith instance is up -is_appsmith_instance_ready() { - local max_retries=200 - local retry_count=0 - local response_code - - while [ $retry_count -lt $max_retries ]; do - response_code=$(curl -s -o /dev/null -w "%{http_code}" http://localhost/health) - if [[ $response_code -eq 200 ]]; then - echo "Appsmith instance is ready." - return 0 - fi - echo "Waiting for Appsmith instance to be ready... (Attempt: $((retry_count + 1)))" - retry_count=$((retry_count + 1)) - sleep 2 - done - return 1 -} - -# Function to wait until the postgres is ready -wait_for_postgres() { - local max_retries=200 - local retry_count=0 - - while [ $retry_count -lt $max_retries ]; do - if docker exec "${container_name}" pg_isready; then - echo "Postgres is ready." - return 0 - fi - echo "Waiting for Postgres to be ready... (Attempt: $((retry_count + 1)))" - retry_count=$((retry_count + 1)) - sleep 2 - done -} - # Function to read the password from the PostgreSQL URL in docker.env.sh get_appsmith_password() { local password @@ -96,18 +61,6 @@ EOF return 1 } -# Function to check if the Appsmith user has read access to databases -check_user_datasource_access_with_host_port_wo_auth() { - docker exec "${container_name}" bash -c "psql -h 127.0.0.1 -p 5432 -U postgres -c '\l'" - return $? -} - -# Function to check if the Appsmith user has read access to databases -check_user_datasource_access_with_local_port_wo_auth() { - docker exec "${container_name}" bash -c "psql -p 5432 -U postgres -c '\l'" - return $? -} - # Test to check if the postgres auth is enabled after upgrading from 1.50 to local image # Expectation: # 1. Appsmith instance should be able to upgrade from v1.50 to local image @@ -167,31 +120,23 @@ test_postgres_auth_enabled_upgrade_from_150tolocal() { RETRYSECONDS=5 retry_count=0 - while true; do - retry_count=$((retry_count + 1)) - if docker exec "${container_name}" pg_isready && - [ "$(docker exec "${container_name}" bash -c 'cat /appsmith-stacks/data/postgres/main/PG_VERSION')" = "14" ]; then - break - fi - if [ $retry_count -le $MAX_RETRIES ]; then - echo "Waiting for postgres to be up..." - sleep $RETRYSECONDS - else - echo "Test ${FUNCNAME[0]} Failed" - exit 1 - fi - done + # Wait until postgres to come up + wait_for_postgres # Check if the Appsmith instance is up if is_appsmith_instance_ready; then - - # Check if the Appsmith user has read access to databases - if check_user_datasource_access_with_auth; then - echo "Test ${FUNCNAME[0]} Passed ✅" - else - echo "Test ${FUNCNAME[0]} Failed ❌" - exit 1 - fi + if ! check_user_exists appsmith; then + echo "Appsmith user does not exist" + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 + fi + # Check if the Appsmith user has read access to databases + if check_user_datasource_access_with_auth; then + echo "Test ${FUNCNAME[0]} Passed ✅" + else + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 + fi else echo "Appsmith instance failed to start." echo "Test ${FUNCNAME[0]} Failed ❌" @@ -253,23 +198,26 @@ test_postgres_auth_enabled_restart_localtolocal() { echo "Starting Appsmith local to check the auth" compose_appsmith_local - wait_for_postgres # Check if the Appsmith instance is up if is_appsmith_instance_ready; then - - # Check if the Appsmith user has read access to databases - if check_user_datasource_access_with_auth; then - echo "Test ${FUNCNAME[0]} Passed ✅" - else - echo "Test ${FUNCNAME[0]} Failed ❌" - exit 1 - fi + if ! check_user_exists appsmith; then + echo "Appsmith user does not exist" + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 + fi + # Check if the Appsmith user has read access to databases + if check_user_datasource_access_with_auth; then + echo "Test ${FUNCNAME[0]} Passed ✅" + else + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 + fi else - echo "Appsmith instance failed to start." - echo "Test ${FUNCNAME[0]} Failed ❌" - exit 1 + echo "Appsmith instance failed to start." + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 fi }