diff --git a/src/components/shared/ComponentDiff/ComponentEditSummary.test.tsx b/src/components/shared/ComponentDiff/ComponentEditSummary.test.tsx new file mode 100644 index 000000000..636f89361 --- /dev/null +++ b/src/components/shared/ComponentDiff/ComponentEditSummary.test.tsx @@ -0,0 +1,40 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import type { EntityDiff } from "@/utils/componentSpecDiff"; + +import { ComponentEditSummary } from "./ComponentEditSummary"; + +const empty: EntityDiff<{ name: string }> = { + lostEntities: [], + newEntities: [], + changedEntities: [], +}; + +const BREAKING = "Some inputs or outputs will be removed"; + +describe("ComponentEditSummary", () => { + it("shows the breaking-change warning and removed input when inputs are lost", () => { + render( + , + ); + + expect(screen.getByText(new RegExp(BREAKING))).toBeInTheDocument(); + expect(screen.getByText("Removed: threshold")).toBeInTheDocument(); + }); + + it("omits the breaking-change warning when nothing is removed", () => { + render( + , + ); + + expect(screen.queryByText(new RegExp(BREAKING))).not.toBeInTheDocument(); + expect(screen.getByText("Added: max_rows")).toBeInTheDocument(); + }); +}); diff --git a/src/components/shared/ComponentDiff/ComponentEditSummary.tsx b/src/components/shared/ComponentDiff/ComponentEditSummary.tsx new file mode 100644 index 000000000..feb8ada7e --- /dev/null +++ b/src/components/shared/ComponentDiff/ComponentEditSummary.tsx @@ -0,0 +1,80 @@ +import { DiffSection } from "@/components/shared/ComponentDiff/DiffSection"; +import { TrimmedDigest } from "@/components/shared/ManageComponent/TrimmedDigest"; +import { Icon } from "@/components/ui/icon"; +import { BlockStack, InlineStack } from "@/components/ui/layout"; +import { Text } from "@/components/ui/typography"; +import type { EntityDiff } from "@/utils/componentSpecDiff"; + +interface ComponentEditSummaryProps { + /** Digest of the component currently on the task. */ + currentDigest?: string; + /** Digest of the edited component being applied. */ + newDigest?: string; + inputDiff: EntityDiff<{ name: string }>; + outputDiff: EntityDiff<{ name: string }>; + /** + * When false, the lost/new/changed `DiffSection`s are omitted (the digest + * transition + breaking warning still show). Used where a richer preview + * renders the per-port diff instead. Defaults to true. + */ + showDiffList?: boolean; +} + +/** + * Summarizes what an edited component changes, in the same visual language as + * the component-upgrade / replace flows: a current → new digest transition, a + * breaking-change warning when inputs/outputs are removed, and the shared + * lost/new/changed `DiffSection`s. Shared by the legacy and v2 save modals. + */ +export function ComponentEditSummary({ + currentDigest, + newDigest, + inputDiff, + outputDiff, + showDiffList = true, +}: ComponentEditSummaryProps) { + const hasBreakingChanges = + inputDiff.lostEntities.length > 0 || outputDiff.lostEntities.length > 0; + const showDigests = + !!currentDigest && !!newDigest && currentDigest !== newDigest; + + return ( + + {showDigests && ( + + + + + + )} + + {hasBreakingChanges && ( + + + + Some inputs or outputs will be removed, and their connections will + be lost. + + + )} + + {showDiffList && ( + <> + + + + )} + + ); +} diff --git a/src/components/shared/ComponentEditor/SaveActionsView.tsx b/src/components/shared/ComponentEditor/SaveActionsView.tsx index db3284bc5..a7c315092 100644 --- a/src/components/shared/ComponentEditor/SaveActionsView.tsx +++ b/src/components/shared/ComponentEditor/SaveActionsView.tsx @@ -1,20 +1,27 @@ import type { ReactNode } from "react"; -import { DiffSection } from "@/components/shared/ComponentDiff/DiffSection"; +import { ComponentEditSummary } from "@/components/shared/ComponentDiff/ComponentEditSummary"; import { Button } from "@/components/ui/button"; import { Icon, type IconName } from "@/components/ui/icon"; import { BlockStack } from "@/components/ui/layout"; import { Text } from "@/components/ui/typography"; -import { type EntityDiff, hasIODiff } from "@/utils/componentSpecDiff"; +import type { EntityDiff } from "@/utils/componentSpecDiff"; type ChooseableAction = "update" | "import" | "place"; export interface SaveActionsViewProps { taskName: string; + currentDigest?: string; + newDigest?: string; inputDiff: EntityDiff<{ name: string }>; outputDiff: EntityDiff<{ name: string }>; /** Whether to offer "Place as a new task". */ allowPlace?: boolean; + /** + * When false, the lost/new/changed diff list is hidden (e.g. the v2 editor + * shows a ghost-diff preview card instead). Defaults to true. + */ + showDiffList?: boolean; /** Extra content shown above the actions (e.g. v2 predicted issues + preview). */ children?: ReactNode; onChoose: (action: ChooseableAction) => void; @@ -29,14 +36,15 @@ export interface SaveActionsViewProps { */ export function SaveActionsView({ taskName, + currentDigest, + newDigest, inputDiff, outputDiff, allowPlace = false, + showDiffList = true, children, onChoose, }: SaveActionsViewProps) { - const showDiff = hasIODiff(inputDiff, outputDiff); - return (
@@ -44,12 +52,13 @@ export function SaveActionsView({ Choose what to do with your changes to “{taskName}”. - {showDiff && ( - - - - - )} + {children} diff --git a/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.test.ts b/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.test.ts index 58dc1555a..1cd3075e2 100644 --- a/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.test.ts +++ b/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.test.ts @@ -145,6 +145,39 @@ describe("replaceTaskComponentRef", () => { expect(updatedGraphSpec.tasks.train.arguments).toEqual({}); }); + it("seeds default arguments for newly added inputs", () => { + const graphSpec = baseGraphSpec(); + const refWithNewInput: ComponentReference = { + name: "Chicago Taxi Trips dataset", + digest: "new-digest", + spec: { + name: "Chicago Taxi Trips dataset", + inputs: [ + { name: "Limit", type: "Integer", default: "1000" }, + { name: "Select", type: "String" }, + { name: "Format", type: "String", default: "csv" }, + { name: "NoDefault", type: "String" }, + ], + outputs: [{ name: "Table" }], + implementation: { container: { image: "alpine/curl" } }, + }, + }; + + const { updatedGraphSpec } = replaceTaskComponentRef( + "dataset", + refWithNewInput, + graphSpec, + ); + + // New input with a default is seeded; one without a default is not; and + // existing arguments are preserved. + expect(updatedGraphSpec.tasks.dataset.arguments).toEqual({ + Limit: { graphInput: { inputName: "Input" } }, + Select: "tips,trip_seconds", + Format: "csv", + }); + }); + it("returns the graph unchanged when the task does not exist", () => { const graphSpec = baseGraphSpec(); const fixedRef = { ...taxiRef("fixed"), digest: "new-digest" }; diff --git a/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.ts b/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.ts index 5a448cac7..2940b24c4 100644 --- a/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.ts +++ b/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.ts @@ -19,9 +19,10 @@ import { deepClone } from "@/utils/deepClone"; * renaming would re-key the task to a new unique id derived from the component * name and is surprising for an in-place edit. * - * Wiring is preserved with two safe exceptions, mirroring the upgrade flow: + * Wiring is reconciled the same way the upgrade flow's `replaceTask` does: * - argument bindings on this task for inputs that no longer exist in the * edited component are dropped (and reported via `lostInputs`); + * - newly added inputs are seeded with their default argument value; * - argument bindings on downstream tasks that consume an output this * component no longer produces are dropped, to avoid dangling references. * @@ -41,9 +42,9 @@ export const replaceTaskComponentRef = ( } const oldInputs = task.componentRef.spec?.inputs ?? []; - const newInputNames = new Set( - (newComponentRef.spec?.inputs ?? []).map((input) => input.name), - ); + const oldInputNames = new Set(oldInputs.map((input) => input.name)); + const newInputs = newComponentRef.spec?.inputs ?? []; + const newInputNames = new Set(newInputs.map((input) => input.name)); const newOutputNames = new Set( (newComponentRef.spec?.outputs ?? []).map((output) => output.name), ); @@ -68,6 +69,21 @@ export const replaceTaskComponentRef = ( ); } + // Seed default argument values for newly added inputs (parity with the + // upgrade flow's `replaceTask`). + const newlyAddedInputs = newInputs.filter( + (input) => !oldInputNames.has(input.name), + ); + if (newlyAddedInputs.some((input) => input.default !== undefined)) { + const args = task.arguments ?? {}; + for (const input of newlyAddedInputs) { + if (input.default !== undefined && args[input.name] === undefined) { + args[input.name] = input.default; + } + } + task.arguments = args; + } + // Drop downstream argument bindings that consume an output this component no // longer produces. Outputs that still exist keep working because the taskId // is unchanged. diff --git a/src/components/shared/TaskDetails/Actions/EditComponentButton.tsx b/src/components/shared/TaskDetails/Actions/EditComponentButton.tsx index 9729918a8..d1a908de5 100644 --- a/src/components/shared/TaskDetails/Actions/EditComponentButton.tsx +++ b/src/components/shared/TaskDetails/Actions/EditComponentButton.tsx @@ -7,6 +7,7 @@ import type { Bounds } from "@/components/shared/ReactFlow/FlowCanvas/utils/geom import { replaceTaskComponentRef } from "@/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef"; import { useNodesOverlay } from "@/components/shared/ReactFlow/NodesOverlay/NodesOverlayProvider"; import useToastNotification from "@/hooks/useToastNotification"; +import { useAnalytics } from "@/providers/AnalyticsProvider"; import { useComponentSpec } from "@/providers/ComponentSpecProvider"; import { extractPositionFromAnnotations } from "@/utils/annotations"; import { @@ -16,6 +17,7 @@ import { type TaskSpec, } from "@/utils/componentSpec"; import { diffComponentIO } from "@/utils/componentSpecDiff"; +import { componentMetadata } from "@/utils/componentTracking"; import { DEFAULT_NODE_DIMENSIONS } from "@/utils/constants"; import { taskIdToNodeId } from "@/utils/nodes/nodeIdUtils"; import { tracking } from "@/utils/tracking"; @@ -39,6 +41,7 @@ export const EditComponentButton = ({ }: EditComponentButtonProps) => { const [isEditDialogOpen, setIsEditDialogOpen] = useState(false); const notify = useToastNotification(); + const { track } = useAnalytics(); const { currentGraphSpec, updateGraphSpec } = useComponentSpec(); const { getNodes } = useReactFlow(); const { fitNodeIntoView, selectNode, notifyNode } = useNodesOverlay(); @@ -62,6 +65,12 @@ export const EditComponentButton = ({ updateGraphSpec(updatedGraphSpec); + track("pipeline_editor.component.edited", { + ...componentMetadata(hydratedComponent, "user"), + action: "update", + lost_inputs_count: lostInputs.length, + }); + if (lostInputs.length > 0) { const inputNames = lostInputs.map((input) => input.name).join(", "); notify( @@ -147,6 +156,12 @@ export const EditComponentButton = ({ } updateGraphSpec(updatedWrapper.implementation.graph); + + track("pipeline_editor.component.edited", { + ...componentMetadata(hydratedComponent, "user"), + action: "place", + }); + notify("Task added", "success"); // The new node mounts asynchronously; wait for it, then reveal + spotlight. @@ -194,6 +209,8 @@ export const EditComponentButton = ({ return (