diff --git a/src/frontend/src/CustomNodes/GenericNode/components/NodeDialogComponent/index.tsx b/src/frontend/src/CustomNodes/GenericNode/components/NodeDialogComponent/index.tsx index ffe6f313bf..73600dea5f 100644 --- a/src/frontend/src/CustomNodes/GenericNode/components/NodeDialogComponent/index.tsx +++ b/src/frontend/src/CustomNodes/GenericNode/components/NodeDialogComponent/index.tsx @@ -264,8 +264,13 @@ export const NodeDialog: React.FC = ({ editNode={false} handleNodeClass={() => {}} nodeClass={dialogNodeData} + nodeType={ + typeof dialogNodeData?.type === "string" + ? dialogNodeData.type + : undefined + } disabled={ - (fieldValue as { disabled: boolean })?.disabled ?? false + (fieldValue as Partial)?.disabled ?? false } placeholder={ (fieldValue as { placeholder: string })?.placeholder ?? "" diff --git a/src/frontend/src/CustomNodes/GenericNode/components/NodeInputField/index.tsx b/src/frontend/src/CustomNodes/GenericNode/components/NodeInputField/index.tsx index 493fb0e451..51a96630a6 100644 --- a/src/frontend/src/CustomNodes/GenericNode/components/NodeInputField/index.tsx +++ b/src/frontend/src/CustomNodes/GenericNode/components/NodeInputField/index.tsx @@ -208,6 +208,7 @@ export default function NodeInputField({ handleNodeClass={handleNodeClass} showParameter={true} nodeClass={data.node!} + nodeType={data.type} placeholder={ isToolMode ? DEFAULT_TOOLSET_PLACEHOLDER diff --git a/src/frontend/src/CustomNodes/GenericNode/index.test.tsx b/src/frontend/src/CustomNodes/GenericNode/index.test.tsx index bb92b09996..d90b78709d 100644 --- a/src/frontend/src/CustomNodes/GenericNode/index.test.tsx +++ b/src/frontend/src/CustomNodes/GenericNode/index.test.tsx @@ -1,9 +1,9 @@ +import { render, screen } from "@testing-library/react"; import type { ButtonHTMLAttributes, ComponentProps, PropsWithChildren, } from "react"; -import { render, screen } from "@testing-library/react"; let mockCloudOnly = false; type Selector = (state: TState) => TResult; @@ -26,6 +26,11 @@ type TypesStoreState = { templates: Record; }; +const typesStoreState: TypesStoreState = { + types: {}, + templates: {}, +}; + const flowStoreState = { deleteNode: jest.fn(), setNode: jest.fn(), @@ -121,10 +126,7 @@ jest.mock("../../stores/shortcuts", () => ({ jest.mock("../../stores/typesStore", () => ({ useTypesStore: (selector: Selector) => - selector({ - types: {}, - templates: {}, - }), + selector(typesStoreState), })); jest.mock("../helpers/process-node-advanced-fields", () => ({ @@ -206,6 +208,8 @@ describe("GenericNode", () => { flowStoreState.dismissedNodesLegacy = []; flowStoreState.componentsToUpdate = []; flowStoreState.nodes = []; + typesStoreState.types = {}; + typesStoreState.templates = {}; }); it("renders legacy and cloud incompatibility warnings together", () => { @@ -231,4 +235,34 @@ describe("GenericNode", () => { expect(screen.getByText("Legacy")).toBeInTheDocument(); expect(screen.getByTestId("cloud-incompatible-banner")).toBeInTheDocument(); }); + + it("backfills cloud incompatibility from the current catalog for older saved nodes", () => { + mockCloudOnly = true; + typesStoreState.templates = { + Directory: { + description: "Reads local directories", + display_name: "Directory", + documentation: "", + template: {}, + cloud_compatible: false, + }, + }; + + const data: ComponentProps["data"] = { + id: "legacy-directory-node", + type: "Directory", + showNode: true, + node: { + display_name: "Directory", + description: "Reads local directories", + documentation: "", + template: {}, + outputs: [], + }, + }; + + render(); + + expect(screen.getByTestId("cloud-incompatible-banner")).toBeInTheDocument(); + }); }); diff --git a/src/frontend/src/CustomNodes/GenericNode/index.tsx b/src/frontend/src/CustomNodes/GenericNode/index.tsx index 63f968c313..156276ae13 100644 --- a/src/frontend/src/CustomNodes/GenericNode/index.tsx +++ b/src/frontend/src/CustomNodes/GenericNode/index.tsx @@ -14,19 +14,20 @@ import { ICON_STROKE_WIDTH } from "../../constants/constants"; import NodeToolbarComponent from "../../pages/FlowPage/components/nodeToolbarComponent"; import { useChangeOnUnfocus } from "../../shared/hooks/use-change-on-unfocus"; import useAlertStore from "../../stores/alertStore"; +import { useCloudModeStore } from "../../stores/cloudModeStore"; import useFlowStore from "../../stores/flowStore"; import useFlowsManagerStore from "../../stores/flowsManagerStore"; import { useShortcutsStore } from "../../stores/shortcuts"; import { useTypesStore } from "../../stores/typesStore"; import type { OutputFieldType, VertexBuildTypeAPI } from "../../types/api"; import type { NodeDataType } from "../../types/flow"; +import { withCurrentCloudMetadata } from "../../utils/cloudMetadataUtils"; import { scapedJSONStringfy } from "../../utils/reactflowUtils"; import { classNames, cn } from "../../utils/utils"; import { processNodeAdvancedFields } from "../helpers/process-node-advanced-fields"; import useUpdateNodeCode from "../hooks/use-update-node-code"; -import NodeDescription from "./components/NodeDescription"; -import { useCloudModeStore } from "../../stores/cloudModeStore"; import NodeCloudIncompatibleComponent from "./components/NodeCloudIncompatibleComponent"; +import NodeDescription from "./components/NodeDescription"; import NodeLegacyComponent from "./components/NodeLegacyComponent"; import NodeName from "./components/NodeName"; import NodeOutputs from "./components/NodeOutputParameter/NodeOutputs"; @@ -374,9 +375,17 @@ function GenericNode({ ); const cloudOnly = useCloudModeStore((state) => state.cloudOnly); + const effectiveNodeClass = useMemo( + () => + cloudOnly + ? (withCurrentCloudMetadata(data.node, templates[data.type]) ?? + data.node) + : data.node, + [cloudOnly, data.node, data.type, templates], + ); const shouldShowCloudIncompatible = useMemo( - () => cloudOnly && data.node?.cloud_compatible === false, - [cloudOnly, data.node?.cloud_compatible], + () => cloudOnly && effectiveNodeClass?.cloud_compatible === false, + [cloudOnly, effectiveNodeClass?.cloud_compatible], ); const memoizedNodeToolbarComponent = useMemo(() => { diff --git a/src/frontend/src/components/core/parameterRenderComponent/components/tableComponent/components/tableNodeCellRender/index.tsx b/src/frontend/src/components/core/parameterRenderComponent/components/tableComponent/components/tableNodeCellRender/index.tsx index b24f0f3fc3..3082eeb254 100644 --- a/src/frontend/src/components/core/parameterRenderComponent/components/tableComponent/components/tableNodeCellRender/index.tsx +++ b/src/frontend/src/components/core/parameterRenderComponent/components/tableComponent/components/tableNodeCellRender/index.tsx @@ -4,7 +4,6 @@ import useHandleOnNewValue from "@/CustomNodes/hooks/use-handle-new-value"; import useHandleNodeClass from "@/CustomNodes/hooks/use-handle-node-class"; import { ParameterRenderComponent } from "@/components/core/parameterRenderComponent"; import type { NodeInfoType } from "@/components/core/parameterRenderComponent/types"; -import { IS_AUTO_LOGIN } from "@/constants/constants"; import { useIsAutoLogin } from "@/hooks/use-is-auto-login"; import useAuthStore from "@/stores/authStore"; import useFlowStore from "@/stores/flowStore"; @@ -70,6 +69,7 @@ export default function TableNodeCellRender({ editNode={true} handleNodeClass={handleNodeClass} nodeClass={node?.data.node} + nodeType={node?.data?.type} disabled={disabled} nodeInformationMetadata={nodeInformationMetadata} /> diff --git a/src/frontend/src/components/core/parameterRenderComponent/index.test.tsx b/src/frontend/src/components/core/parameterRenderComponent/index.test.tsx index 144278fba9..ce6a2606fe 100644 --- a/src/frontend/src/components/core/parameterRenderComponent/index.test.tsx +++ b/src/frontend/src/components/core/parameterRenderComponent/index.test.tsx @@ -1,12 +1,17 @@ -import type { ComponentProps } from "react"; import { render, screen } from "@testing-library/react"; +import type { ComponentProps } from "react"; +import type { APIClassType } from "@/types/api"; import { ParameterRenderComponent } from "."; let mockCloudOnly = false; +let mockTemplates: Record = {}; type CloudModeState = { cloudOnly: boolean; setCloudOnly: jest.Mock; }; +type TypesStoreState = { + templates: Record; +}; type StrRenderProps = { value?: string | number | readonly string[] | null; placeholder?: string | null; @@ -22,6 +27,11 @@ jest.mock("@/stores/cloudModeStore", () => ({ selector({ cloudOnly: mockCloudOnly, setCloudOnly: jest.fn() }), })); +jest.mock("@/stores/typesStore", () => ({ + useTypesStore: (selector: (state: TypesStoreState) => T) => + selector({ templates: mockTemplates }), +})); + jest.mock("./components/strRenderComponent", () => ({ StrRenderComponent: ({ value, placeholder }: StrRenderProps) => (
({ describe("ParameterRenderComponent", () => { beforeEach(() => { mockCloudOnly = false; + mockTemplates = {}; }); + const createNodeClass = ( + overrides: Record = {}, + ): APIClassType => + ({ + description: "Test component", + template: {}, + display_name: "Test Component", + documentation: "Test component documentation", + ...overrides, + }) as APIClassType; + const baseProps: ComponentProps = { handleOnNewValue: jest.fn(), name: "url", @@ -65,11 +87,8 @@ describe("ParameterRenderComponent", () => { showParameter: true, inspectionPanel: false, handleNodeClass: jest.fn(), - nodeClass: { - description: "Test component", - template: {}, - display_name: "Test Component", - documentation: "Test component documentation", + templateValue: "", + nodeClass: createNodeClass({ metadata: { cloud_default_overrides: { url: { @@ -78,7 +97,7 @@ describe("ParameterRenderComponent", () => { }, }, }, - }, + }), disabled: false, templateData: { type: "str", name: "url", placeholder: "Local URL" }, }; @@ -114,6 +133,38 @@ describe("ParameterRenderComponent", () => { ); }); + it("backfills the cloud placeholder from the current catalog for older saved nodes", () => { + mockCloudOnly = true; + mockTemplates = { + File: createNodeClass({ + display_name: "Read File", + documentation: "", + metadata: { + cloud_default_overrides: { + url: { + placeholder: "Enter your cloud URL", + }, + }, + }, + }), + }; + + render( + , + ); + + const renderedProps = screen.getByTestId("str-render-props"); + expect(renderedProps).toHaveAttribute( + "data-placeholder", + "Enter your cloud URL", + ); + }); + it("preserves incompatible sortable values while filtering them from the chooser in cloud mode", () => { mockCloudOnly = true; @@ -128,14 +179,49 @@ describe("ParameterRenderComponent", () => { options: [{ name: "Local" }, { name: "AWS" }], limit: 1, }} - nodeClass={{ - ...baseProps.nodeClass, + nodeClass={createNodeClass({ metadata: { cloud_incompatible_options: { storage_location: ["Local"], }, }, + })} + />, + ); + + const renderedProps = screen.getByTestId("sortable-list-props"); + expect(renderedProps).toHaveAttribute("data-value", "Local"); + expect(renderedProps).toHaveAttribute("data-options", "AWS"); + expect(renderedProps).toHaveAttribute("data-cloud-incompatible", "Local"); + }); + + it("backfills cloud-incompatible sortable options from the current catalog for older saved nodes", () => { + mockCloudOnly = true; + mockTemplates = { + File: createNodeClass({ + display_name: "Read File", + documentation: "", + metadata: { + cloud_incompatible_options: { + storage_location: ["Local"], + }, + }, + }), + }; + + render( + , ); diff --git a/src/frontend/src/components/core/parameterRenderComponent/index.tsx b/src/frontend/src/components/core/parameterRenderComponent/index.tsx index 00a7989750..68d6d26916 100644 --- a/src/frontend/src/components/core/parameterRenderComponent/index.tsx +++ b/src/frontend/src/components/core/parameterRenderComponent/index.tsx @@ -5,12 +5,14 @@ import SliderComponent from "@/components/core/parameterRenderComponent/componen import TableNodeComponent from "@/components/core/parameterRenderComponent/components/TableNodeComponent"; import TabComponent from "@/components/core/parameterRenderComponent/components/tabComponent"; import { TEXT_FIELD_TYPES } from "@/constants/constants"; -import { useCloudModeStore } from "@/stores/cloudModeStore"; import CustomConnectionComponent from "@/customization/components/custom-connectionComponent"; import CustomInputFileComponent from "@/customization/components/custom-input-file"; import CustomLinkComponent from "@/customization/components/custom-linkComponent"; import { ENABLE_INSPECTION_PANEL } from "@/customization/feature-flags"; +import { useCloudModeStore } from "@/stores/cloudModeStore"; +import { useTypesStore } from "@/stores/typesStore"; import type { APIClassType, InputFieldType } from "@/types/api"; +import { withCurrentCloudMetadata } from "@/utils/cloudMetadataUtils"; import AccordionPromptComponent from "./components/accordionPromptComponent"; import DictComponent from "./components/dictComponent"; import { EmptyParameterComponent } from "./components/emptyParameterComponent"; @@ -36,10 +38,11 @@ export function ParameterRenderComponent({ templateData, templateValue, editNode, - showParameter, + showParameter = true, inspectionPanel = false, handleNodeClass, nodeClass, + nodeType, disabled, placeholder, isToolMode, @@ -53,16 +56,27 @@ export function ParameterRenderComponent({ templateData: Partial; templateValue: unknown; editNode: boolean; - showParameter: boolean; - inspectionPanel: boolean; + showParameter?: boolean; + inspectionPanel?: boolean; handleNodeClass: (value: unknown, code?: string, type?: string) => void; nodeClass: APIClassType; + nodeType?: string; disabled: boolean; placeholder?: string; isToolMode?: boolean; nodeInformationMetadata?: NodeInfoType; }) { const cloudOnly = useCloudModeStore((state) => state.cloudOnly); + const templates = useTypesStore((state) => state.templates); + + const resolvedNodeType = + nodeType ?? + (typeof nodeClass?.type === "string" ? nodeClass.type : undefined); + const effectiveNodeClass = + cloudOnly && resolvedNodeType + ? (withCurrentCloudMetadata(nodeClass, templates[resolvedNodeType]) ?? + nodeClass) + : nodeClass; const id = ( templateData.type + @@ -71,7 +85,7 @@ export function ParameterRenderComponent({ templateData.name ).toLowerCase(); - const nodeMetadata = nodeClass?.metadata as + const nodeMetadata = effectiveNodeClass?.metadata as | { cloud_default_overrides?: Record< string, @@ -98,7 +112,7 @@ export function ParameterRenderComponent({ editNode, handleOnNewValue: handleOnNewValue as handleOnNewValueType, disabled, - nodeClass, + nodeClass: effectiveNodeClass, handleNodeClass, nodeId, helperText: templateData?.helper_text, @@ -143,7 +157,7 @@ export function ParameterRenderComponent({ ) : ( @@ -234,7 +248,7 @@ export function ParameterRenderComponent({ return ENABLE_INSPECTION_PANEL && !baseInputProps.editNode ? ( @@ -269,9 +283,9 @@ export function ParameterRenderComponent({ ); case "slider": { @@ -338,7 +352,7 @@ export function ParameterRenderComponent({ {...baseInputProps} name={name} nodeId={nodeId} - nodeClass={nodeClass} + nodeClass={effectiveNodeClass} helperText={templateData?.helper_text} helperMetadata={templateData?.helper_text_metadata} options={templateData?.options} diff --git a/src/frontend/src/customization/components/custom-parameter.tsx b/src/frontend/src/customization/components/custom-parameter.tsx index e411baf3ff..75bddc4c1a 100644 --- a/src/frontend/src/customization/components/custom-parameter.tsx +++ b/src/frontend/src/customization/components/custom-parameter.tsx @@ -19,6 +19,7 @@ export function CustomParameterComponent({ editNode, handleNodeClass, nodeClass, + nodeType, placeholder, isToolMode = false, nodeInformationMetadata, @@ -29,12 +30,13 @@ export function CustomParameterComponent({ nodeId: string; inputId: targetHandleType; templateData: Partial; - templateValue: any; + templateValue: unknown; showParameter: boolean; - inspectionPanel: boolean; + inspectionPanel?: boolean; editNode: boolean; - handleNodeClass: (value: any, code?: string, type?: string) => void; + handleNodeClass: (value: unknown, code?: string, type?: string) => void; nodeClass: APIClassType; + nodeType?: string; placeholder?: string; isToolMode?: boolean; nodeInformationMetadata?: NodeInfoType; @@ -61,6 +63,7 @@ export function CustomParameterComponent({ inspectionPanel={inspectionPanel} handleNodeClass={handleNodeClass} nodeClass={nodeClass} + nodeType={nodeType} disabled={disabled} placeholder={placeholder} isToolMode={isToolMode} @@ -107,7 +110,7 @@ export function CustomParameterLabel({ }: { name: string; nodeId: string; - templateValue: any; + templateValue: unknown; nodeClass: APIClassType; }) { return <>; diff --git a/src/frontend/src/utils/cloudMetadataUtils.ts b/src/frontend/src/utils/cloudMetadataUtils.ts new file mode 100644 index 0000000000..91b2ead9d3 --- /dev/null +++ b/src/frontend/src/utils/cloudMetadataUtils.ts @@ -0,0 +1,82 @@ +import type { APIClassType } from "@/types/api"; + +type CloudFieldOverride = { + value?: unknown; + placeholder?: string; +}; + +type CloudUiMetadata = Record & { + cloud_default_overrides?: Record; + cloud_incompatible_options?: Record; +}; + +function isCloudUiMetadata(value: unknown): value is CloudUiMetadata { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +export function withCurrentCloudMetadata( + savedNode: APIClassType | undefined, + currentCatalogNode: APIClassType | undefined, +): APIClassType | undefined { + if (!savedNode || !currentCatalogNode) { + return savedNode; + } + + const savedMetadata = isCloudUiMetadata(savedNode.metadata) + ? savedNode.metadata + : undefined; + const currentMetadata = isCloudUiMetadata(currentCatalogNode.metadata) + ? currentCatalogNode.metadata + : undefined; + + const shouldOverlayCloudCompatible = + savedNode.cloud_compatible === undefined && + currentCatalogNode.cloud_compatible !== undefined; + const shouldOverlayCloudDefaultOverrides = + savedMetadata?.cloud_default_overrides === undefined && + currentMetadata?.cloud_default_overrides !== undefined; + const shouldOverlayCloudIncompatibleOptions = + savedMetadata?.cloud_incompatible_options === undefined && + currentMetadata?.cloud_incompatible_options !== undefined; + + if ( + !shouldOverlayCloudCompatible && + !shouldOverlayCloudDefaultOverrides && + !shouldOverlayCloudIncompatibleOptions + ) { + return savedNode; + } + + const nextMetadata = + shouldOverlayCloudDefaultOverrides || shouldOverlayCloudIncompatibleOptions + ? { + ...((savedMetadata ?? {}) as Record), + ...(shouldOverlayCloudDefaultOverrides + ? { + cloud_default_overrides: + currentMetadata?.cloud_default_overrides, + } + : {}), + ...(shouldOverlayCloudIncompatibleOptions + ? { + cloud_incompatible_options: + currentMetadata?.cloud_incompatible_options, + } + : {}), + } + : savedMetadata; + + return { + ...savedNode, + ...(shouldOverlayCloudCompatible + ? { + cloud_compatible: currentCatalogNode.cloud_compatible, + } + : {}), + ...(nextMetadata !== undefined + ? { + metadata: nextMetadata, + } + : {}), + } as APIClassType; +}