From de7fa7e93b6249fd34f686edababf27dbae5ba59 Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Fri, 13 Mar 2026 12:15:42 -0300 Subject: [PATCH 01/21] ENG-564: Replace YAML editor with node-based dataset UI Swap the Monaco YAML editor for an interactive React Flow graph where each collection and field is a node. Clicking a node opens a drawer to edit its metadata. - Add DatasetNodeEditor with D3 hierarchy layout (LR tree) - Add custom node components (root, collection, field) matching the taxonomy tree visual style with hover context - Add DatasetTreeHoverContext for parent/child/inactive highlighting - Add custom bezier edges that change color on hover - Add DatasetNodeDetailPanel drawer for editing metadata - Enable "Edit dataset" button for SaaS connectors - Remove test results/logs sections for full-canvas layout --- .../forms/ConnectorParametersForm.tsx | 5 +- .../test-datasets/DatasetEditorSection.tsx | 162 ++-------- .../test-datasets/DatasetNodeDetailPanel.tsx | 132 ++++++++ .../test-datasets/DatasetNodeEditor.tsx | 304 ++++++++++++++++++ .../context/DatasetTreeHoverContext.tsx | 148 +++++++++ .../test-datasets/edges/DatasetTreeEdge.tsx | 53 +++ .../nodes/DatasetCollectionNode.tsx | 59 ++++ .../test-datasets/nodes/DatasetFieldNode.tsx | 72 +++++ .../nodes/DatasetNode.module.scss | 75 +++++ .../test-datasets/nodes/DatasetNodeHandle.tsx | 27 ++ .../test-datasets/nodes/DatasetRootNode.tsx | 54 ++++ .../features/test-datasets/useDatasetGraph.ts | 171 ++++++++++ .../test-datasets/useDatasetNodeLayout.ts | 61 ++++ .../systems/configure/[id]/test-datasets.tsx | 21 +- 14 files changed, 1193 insertions(+), 151 deletions(-) create mode 100644 clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx create mode 100644 clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx create mode 100644 clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx create mode 100644 clients/admin-ui/src/features/test-datasets/edges/DatasetTreeEdge.tsx create mode 100644 clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx create mode 100644 clients/admin-ui/src/features/test-datasets/nodes/DatasetFieldNode.tsx create mode 100644 clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss create mode 100644 clients/admin-ui/src/features/test-datasets/nodes/DatasetNodeHandle.tsx create mode 100644 clients/admin-ui/src/features/test-datasets/nodes/DatasetRootNode.tsx create mode 100644 clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts create mode 100644 clients/admin-ui/src/features/test-datasets/useDatasetNodeLayout.ts diff --git a/clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/ConnectorParametersForm.tsx b/clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/ConnectorParametersForm.tsx index d61c23c31e0..b6876254b20 100644 --- a/clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/ConnectorParametersForm.tsx +++ b/clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/ConnectorParametersForm.tsx @@ -291,10 +291,11 @@ export const ConnectorParametersForm = ({ ) : null} {isPlusEnabled && - SystemType.DATABASE === connectionOption.type && + (SystemType.DATABASE === connectionOption.type || + SystemType.SAAS === connectionOption.type) && !_.isEmpty(initialDatasets) && ( )} {connectionOption.authorization_required && !authorized ? ( diff --git a/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx b/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx index 3bf7cae83de..d613d61a58c 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx @@ -1,8 +1,6 @@ import { FetchBaseQueryError } from "@reduxjs/toolkit/query"; import { - Alert, Button, - Card, Flex, Icons, Select, @@ -11,55 +9,29 @@ import { Typography, useMessage, } from "fidesui"; -import yaml, { YAMLException } from "js-yaml"; -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { useAppDispatch, useAppSelector } from "~/app/hooks"; -import ClipboardButton from "~/features/common/ClipboardButton"; import { getErrorMessage, isErrorResult } from "~/features/common/helpers"; -import { Editor } from "~/features/common/yaml/helpers"; import { useUpdateDatasetMutation } from "~/features/dataset"; -import { - useGetConnectionConfigDatasetConfigsQuery, - useGetDatasetReachabilityQuery, -} from "~/features/datastore-connections"; +import { useGetConnectionConfigDatasetConfigsQuery } from "~/features/datastore-connections"; import { Dataset } from "~/types/api"; -import { - selectCurrentDataset, - selectCurrentPolicyKey, - setCurrentDataset, - setReachability, -} from "./dataset-test.slice"; +import { selectCurrentDataset, setCurrentDataset } from "./dataset-test.slice"; +import DatasetNodeEditor from "./DatasetNodeEditor"; import { removeNulls } from "./helpers"; interface EditorSectionProps { connectionKey: string; } -const getReachabilityMessage = (details: any) => { - if (Array.isArray(details)) { - const firstDetail = details[0]; - - if (!firstDetail) { - return ""; - } - - const message = firstDetail.msg || ""; - const location = firstDetail.loc ? ` (${firstDetail.loc})` : ""; - return `${message}${location}`; - } - return details; -}; - const EditorSection = ({ connectionKey }: EditorSectionProps) => { const messageApi = useMessage(); const dispatch = useAppDispatch(); const [updateDataset] = useUpdateDatasetMutation(); - const [editorContent, setEditorContent] = useState(""); + const [localDataset, setLocalDataset] = useState(); const currentDataset = useAppSelector(selectCurrentDataset); - const currentPolicyKey = useAppSelector(selectCurrentPolicyKey); const { data: datasetConfigs, @@ -69,24 +41,6 @@ const EditorSection = ({ connectionKey }: EditorSectionProps) => { skip: !connectionKey, }); - const { data: reachability, refetch: refetchReachability } = - useGetDatasetReachabilityQuery( - { - connectionKey, - datasetKey: currentDataset?.fides_key || "", - policyKey: currentPolicyKey, - }, - { - skip: !connectionKey || !currentDataset?.fides_key || !currentPolicyKey, - }, - ); - - useEffect(() => { - if (reachability) { - dispatch(setReachability(reachability.reachable)); - } - }, [reachability, dispatch]); - const datasetOptions = useMemo( () => (datasetConfigs?.items || []).map((item) => ({ @@ -112,27 +66,10 @@ const EditorSection = ({ connectionKey }: EditorSectionProps) => { useEffect(() => { if (currentDataset?.ctl_dataset) { - setEditorContent(yaml.dump(removeNulls(currentDataset?.ctl_dataset))); + setLocalDataset(removeNulls(currentDataset.ctl_dataset) as Dataset); } }, [currentDataset]); - useEffect(() => { - if (currentPolicyKey && currentDataset?.fides_key && connectionKey) { - refetchReachability(); - } - }, [ - currentPolicyKey, - currentDataset?.fides_key, - connectionKey, - refetchReachability, - ]); - - useEffect(() => { - if (reachability) { - dispatch(setReachability(reachability.reachable)); - } - }, [reachability, dispatch]); - const handleDatasetChange = async (value: string) => { const selectedConfig = datasetConfigs?.items.find( (item) => item.fides_key === value, @@ -142,28 +79,16 @@ const EditorSection = ({ connectionKey }: EditorSectionProps) => { } }; - const handleSave = async () => { - if (!currentDataset) { - return; - } + const handleLocalDatasetChange = useCallback((updated: Dataset) => { + setLocalDataset(updated); + }, []); - // Parse YAML first - let datasetValues: Dataset; - try { - datasetValues = yaml.load(editorContent) as Dataset; - } catch (yamlError) { - messageApi.error( - `YAML Parsing Error: ${ - yamlError instanceof YAMLException - ? `${yamlError.reason} ${yamlError.mark ? `at line ${yamlError.mark.line}` : ""}` - : "Invalid YAML format" - }`, - ); + const handleSave = async () => { + if (!currentDataset || !localDataset) { return; } - // Then handle the API update - const result = await updateDataset(datasetValues); + const result = await updateDataset(localDataset); if (isErrorResult(result)) { messageApi.error(getErrorMessage(result.error)); @@ -178,7 +103,6 @@ const EditorSection = ({ connectionKey }: EditorSectionProps) => { ); messageApi.success("Successfully modified dataset"); await refetchDatasets(); - await refetchReachability(); }; const handleRefresh = async () => { @@ -188,7 +112,7 @@ const EditorSection = ({ connectionKey }: EditorSectionProps) => { (item) => item.fides_key === currentDataset?.fides_key, ); if (refreshedDataset?.ctl_dataset) { - setEditorContent(yaml.dump(removeNulls(refreshedDataset.ctl_dataset))); + setLocalDataset(removeNulls(refreshedDataset.ctl_dataset) as Dataset); } messageApi.success("Successfully refreshed datasets"); } catch (error) { @@ -201,12 +125,14 @@ const EditorSection = ({ connectionKey }: EditorSectionProps) => { align="stretch" flex="1" gap="small" - className="max-h-screen max-w-[70vw]" vertical + style={{ height: "100%", minHeight: 0 }} > - Edit dataset: + + Edit dataset: + + + + + + + + + + + + + ); +}; + +export default AddNodeModal; diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx index 5f63c290eb5..98d1b31f594 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx @@ -1,8 +1,21 @@ -import { Drawer, Form, Input, Select, Tag, Typography } from "fidesui"; -import { useEffect } from "react"; +import { + Button, + Collapse, + Drawer, + Form, + Input, + InputNumber, + Select, + Switch, + Tag, + Typography, + useModal, +} from "fidesui"; +import { useContext, useEffect } from "react"; import { DatasetCollection, DatasetField } from "~/types/api"; +import DatasetEditorActionsContext from "./context/DatasetEditorActionsContext"; import { CollectionNodeData, FieldNodeData } from "./useDatasetGraph"; interface DatasetNodeDetailPanelProps { @@ -20,6 +33,20 @@ interface DatasetNodeDetailPanelProps { ) => void; } +const DATA_TYPE_OPTIONS = [ + "string", + "integer", + "float", + "boolean", + "object_id", + "object", +].map((v) => ({ label: v, value: v })); + +const REDACT_OPTIONS = [ + { label: "None", value: "" }, + { label: "name", value: "name" }, +]; + const DatasetNodeDetailPanel = ({ open, onClose, @@ -28,6 +55,8 @@ const DatasetNodeDetailPanel = ({ onUpdateField, }: DatasetNodeDetailPanelProps) => { const [form] = Form.useForm(); + const modal = useModal(); + const actions = useContext(DatasetEditorActionsContext); useEffect(() => { if (!nodeData || !open) { @@ -35,16 +64,31 @@ const DatasetNodeDetailPanel = ({ } if (nodeData.nodeType === "collection") { + const { collection } = nodeData; form.setFieldsValue({ - name: nodeData.collection.name, - description: nodeData.collection.description ?? "", - data_categories: nodeData.collection.data_categories ?? [], + name: collection.name, + description: collection.description ?? "", + data_categories: collection.data_categories ?? [], + // CollectionMeta fields + after: collection.fides_meta?.after ?? [], + erase_after: collection.fides_meta?.erase_after ?? [], + skip_processing: collection.fides_meta?.skip_processing ?? false, }); } else { + const { field } = nodeData; form.setFieldsValue({ - name: nodeData.field.name, - description: nodeData.field.description ?? "", - data_categories: nodeData.field.data_categories ?? [], + name: field.name, + description: field.description ?? "", + data_categories: field.data_categories ?? [], + // FidesMeta fields + data_type: field.fides_meta?.data_type ?? undefined, + identity: field.fides_meta?.identity ?? "", + primary_key: field.fides_meta?.primary_key ?? false, + read_only: field.fides_meta?.read_only ?? false, + return_all_elements: field.fides_meta?.return_all_elements ?? false, + length: field.fides_meta?.length ?? undefined, + custom_request_field: field.fides_meta?.custom_request_field ?? "", + redact: field.fides_meta?.redact ?? "", }); } }, [nodeData, open, form]); @@ -57,19 +101,80 @@ const DatasetNodeDetailPanel = ({ return; } - const updates = { - description: (allValues.description as string) || null, + const categories = allValues.data_categories as string[] | undefined; + const baseUpdates = { + description: (allValues.description as string) || undefined, data_categories: - (allValues.data_categories as string[])?.length > 0 - ? (allValues.data_categories as string[]) - : null, + categories && categories.length > 0 ? categories : undefined, }; if (nodeData.nodeType === "collection") { - onUpdateCollection(nodeData.collection.name, updates); + const after = allValues.after as string[] | undefined; + const eraseAfter = allValues.erase_after as string[] | undefined; + const skipProcessing = allValues.skip_processing as boolean; + const collectionMeta = { + after: after && after.length > 0 ? after : undefined, + erase_after: + eraseAfter && eraseAfter.length > 0 ? eraseAfter : undefined, + skip_processing: skipProcessing || undefined, + }; + const hasAnyMeta = Object.values(collectionMeta).some( + (v) => v !== undefined, + ); + + onUpdateCollection(nodeData.collection.name, { + ...baseUpdates, + fides_meta: hasAnyMeta ? collectionMeta : undefined, + } as Partial); } else { - onUpdateField(nodeData.collectionName, nodeData.fieldPath, updates); + const redactVal = allValues.redact as string; + const fieldMeta = { + data_type: (allValues.data_type as string) || undefined, + identity: (allValues.identity as string) || undefined, + primary_key: (allValues.primary_key as boolean) || undefined, + read_only: (allValues.read_only as boolean) || undefined, + return_all_elements: + (allValues.return_all_elements as boolean) || undefined, + length: + allValues.length !== null && + allValues.length !== undefined && + allValues.length !== "" + ? (allValues.length as number) + : undefined, + custom_request_field: + (allValues.custom_request_field as string) || undefined, + redact: redactVal === "name" ? ("name" as const) : undefined, + }; + const hasAnyMeta = Object.values(fieldMeta).some((v) => v !== undefined); + + onUpdateField(nodeData.collectionName, nodeData.fieldPath, { + ...baseUpdates, + fides_meta: hasAnyMeta ? fieldMeta : undefined, + } as Partial); + } + }; + + const handleDelete = () => { + if (!nodeData) { + return; } + modal.confirm({ + title: `Delete this ${nodeData.nodeType}?`, + content: + nodeData.nodeType === "collection" + ? "This will remove the collection and all its fields." + : "This will remove the field.", + okText: "Delete", + okButtonProps: { danger: true }, + onOk: () => { + if (nodeData.nodeType === "collection") { + actions.deleteCollection(nodeData.collection.name); + } else { + actions.deleteField(nodeData.collectionName, nodeData.fieldPath); + } + onClose(); + }, + }); }; const isProtected = nodeData?.nodeType === "field" && nodeData.isProtected; @@ -117,12 +222,126 @@ const DatasetNodeDetailPanel = ({ /> + {nodeData.nodeType === "collection" && ( + + + + + + + + + ), + }, + ]} + /> + )} + + {nodeData.nodeType === "field" && ( + + + + + + + + + + + + + + + + + + + + + + + {mode === "field" && ( + <> + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + - + (option?.value as string) + ?.toLowerCase() + .includes(input.toLowerCase()) ?? false + } + /> + ); +}; + /** * Shared Form.Item components for field-level fides_meta editing. * Expects to be rendered inside an Ant Design
. From 95827030f11eec9541ee1c2d9dd4879207d06033 Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Tue, 17 Mar 2026 11:08:54 -0300 Subject: [PATCH 07/21] Fix fitView triggering on metadata edits Only re-fit the viewport when the graph structure changes (node count or focused collection), not when field metadata is edited. --- .../src/features/test-datasets/DatasetNodeEditor.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx index 1febb2f7fe3..5289765a0bb 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx @@ -186,15 +186,16 @@ const DatasetNodeEditorInner = ({ [layoutedNodes, selectedNodeId], ); - // Fit view when layout changes + // Fit view only when the graph structure changes (drill-down or node count), + // not on metadata edits which don't affect layout. + const nodeCount = rawNodes.length; useEffect(() => { - if (rawNodes.length > 0) { + if (nodeCount > 0) { setTimeout(() => { reactFlowInstance.fitView({ padding: 0.2 }); }, 150); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [rawNodes, reactFlowInstance]); + }, [nodeCount, focusedCollection, reactFlowInstance]); // Clear selection when drilling in/out useEffect(() => { From 11fece109cd8f7b635f9f936a91dbf236739dfe3 Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Tue, 17 Mar 2026 11:11:38 -0300 Subject: [PATCH 08/21] Briefly highlight newly added nodes in yellow When a collection or field is added, the new node gets a 1.5s yellow highlight animation that fades out, making it easy to spot in the graph. --- .../test-datasets/DatasetNodeEditor.tsx | 32 +++++++++++++++---- .../nodes/DatasetCollectionNode.tsx | 5 ++- .../test-datasets/nodes/DatasetFieldNode.tsx | 5 ++- .../nodes/DatasetNode.module.scss | 15 +++++++++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx index 5289765a0bb..0a69fc93e74 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx @@ -164,6 +164,21 @@ const DatasetNodeEditorInner = ({ CollectionNodeData | FieldNodeData | null >(null); const [addModal, setAddModal] = useState(CLOSED_MODAL); + const [highlightedNodeId, setHighlightedNodeId] = useState( + null, + ); + const highlightTimerRef = useRef | null>(null); + + const highlightNode = useCallback((nodeId: string) => { + if (highlightTimerRef.current) { + clearTimeout(highlightTimerRef.current); + } + setHighlightedNodeId(nodeId); + highlightTimerRef.current = setTimeout(() => { + setHighlightedNodeId(null); + highlightTimerRef.current = null; + }, 1500); + }, []); const { nodes: rawNodes, edges } = useDatasetGraph( dataset, @@ -176,14 +191,18 @@ const DatasetNodeEditorInner = ({ options: { direction: "LR" }, }); - // Merge selection state into nodes + // Merge selection + highlight state into nodes const nodes = useMemo( () => layoutedNodes.map((node) => ({ ...node, selected: node.id === selectedNodeId, + data: { + ...node.data, + isHighlighted: node.id === highlightedNodeId, + }, })), - [layoutedNodes, selectedNodeId], + [layoutedNodes, selectedNodeId, highlightedNodeId], ); // Fit view only when the graph structure changes (drill-down or node count), @@ -287,8 +306,9 @@ const DatasetNodeEditorInner = ({ ...dataset, collections: [...dataset.collections, newCollection], }); + highlightNode(`${COLLECTION_ROOT_PREFIX}${name}`); }, - [dataset, onDatasetChange], + [dataset, onDatasetChange, highlightNode], ); const handleAddField = useCallback( @@ -306,10 +326,8 @@ const DatasetNodeEditorInner = ({ return c; } if (!parentFieldPath) { - // Top-level field on the collection return { ...c, fields: [...c.fields, newField] }; } - // Nested field: append to the parent's fields array const segments = parentFieldPath.split("."); return { ...c, @@ -317,8 +335,10 @@ const DatasetNodeEditorInner = ({ }; }), }); + const fieldPath = parentFieldPath ? `${parentFieldPath}.${name}` : name; + highlightNode(`field-${collectionName}-${fieldPath}`); }, - [dataset, onDatasetChange], + [dataset, onDatasetChange, highlightNode], ); const handleDeleteCollection = useCallback( diff --git a/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx b/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx index 8d95fe1a1d8..cfd9b0fb80c 100644 --- a/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx +++ b/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx @@ -42,7 +42,10 @@ const DatasetCollectionNode = ({ data, id }: NodeProps) => { type="target" inactive={hoverStatus === DatasetNodeHoverStatus.INACTIVE} /> - + } > )} - + ); }; From 0316e2decb4c6cedebaaddc36d5e966e15d5dec2 Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Tue, 17 Mar 2026 11:38:03 -0300 Subject: [PATCH 11/21] Move Create button to bottom of add node drawer --- .../src/features/test-datasets/AddNodeModal.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/clients/admin-ui/src/features/test-datasets/AddNodeModal.tsx b/clients/admin-ui/src/features/test-datasets/AddNodeModal.tsx index 2f739c0907a..677b5e99044 100644 --- a/clients/admin-ui/src/features/test-datasets/AddNodeModal.tsx +++ b/clients/admin-ui/src/features/test-datasets/AddNodeModal.tsx @@ -76,11 +76,6 @@ const AddNodeModal = ({ open={open} onClose={onCancel} mask={false} - extra={ - - } >
)} + +
+ +
); From 4535cc19846b6edb0f778e379bbe0aae8f0126a9 Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Tue, 17 Mar 2026 12:02:38 -0300 Subject: [PATCH 12/21] Add collapsible YAML editor panel to node-based dataset editor Adds a toggleable YAML code editor at the bottom of the node graph with two-way sync: graph changes update the YAML and YAML edits update the graph (debounced 500ms with error display). --- .../test-datasets/DatasetNodeEditor.tsx | 227 +++++++++++++++--- 1 file changed, 189 insertions(+), 38 deletions(-) diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx index 0a69fc93e74..21d28600e99 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx @@ -12,10 +12,12 @@ import { ReactFlowProvider, useReactFlow, } from "@xyflow/react"; -import { Breadcrumb, Button, Flex, Icons } from "fidesui"; +import { Breadcrumb, Button, Flex, Icons, Typography } from "fidesui"; import palette from "fidesui/src/palette/palette.module.scss"; +import yaml, { YAMLException } from "js-yaml"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { Editor } from "~/features/common/yaml/helpers"; import { Dataset, DatasetCollection, DatasetField } from "~/types/api"; import AddNodeModal from "./AddNodeModal"; @@ -25,6 +27,7 @@ import DatasetEditorActionsContext, { import { DatasetTreeHoverProvider } from "./context/DatasetTreeHoverContext"; import DatasetNodeDetailPanel from "./DatasetNodeDetailPanel"; import DatasetTreeEdge from "./edges/DatasetTreeEdge"; +import { removeNulls } from "./helpers"; import DatasetCollectionNode from "./nodes/DatasetCollectionNode"; import DatasetFieldNode from "./nodes/DatasetFieldNode"; import DatasetRootNode from "./nodes/DatasetRootNode"; @@ -169,6 +172,14 @@ const DatasetNodeEditorInner = ({ ); const highlightTimerRef = useRef | null>(null); + // --- YAML editor state --- + const [yamlPanelOpen, setYamlPanelOpen] = useState(false); + const [yamlContent, setYamlContent] = useState(""); + const [yamlError, setYamlError] = useState(null); + // Tracks who initiated the last change to prevent sync loops + const changeSourceRef = useRef<"graph" | "yaml" | null>(null); + const yamlDebounceRef = useRef | null>(null); + const highlightNode = useCallback((nodeId: string) => { if (highlightTimerRef.current) { clearTimeout(highlightTimerRef.current); @@ -180,6 +191,70 @@ const DatasetNodeEditorInner = ({ }, 1500); }, []); + // Sync dataset → YAML when the graph side changes + useEffect(() => { + if (!yamlPanelOpen) { + return; + } + if (changeSourceRef.current === "yaml") { + changeSourceRef.current = null; + return; + } + const cleaned = removeNulls(dataset); + setYamlContent(yaml.dump(cleaned)); + setYamlError(null); + }, [dataset, yamlPanelOpen]); + + // Initialize YAML content when panel opens + const handleToggleYamlPanel = useCallback(() => { + setYamlPanelOpen((prev) => { + if (!prev) { + const cleaned = removeNulls(dataset); + setYamlContent(yaml.dump(cleaned)); + setYamlError(null); + } + return !prev; + }); + }, [dataset]); + + // Handle YAML editor changes with debounce + const handleYamlChange = useCallback( + (value: string | undefined) => { + const newValue = value || ""; + setYamlContent(newValue); + + if (yamlDebounceRef.current) { + clearTimeout(yamlDebounceRef.current); + } + + yamlDebounceRef.current = setTimeout(() => { + try { + const parsed = yaml.load(newValue) as Dataset; + if ( + parsed && + typeof parsed === "object" && + Array.isArray(parsed.collections) + ) { + setYamlError(null); + changeSourceRef.current = "yaml"; + onDatasetChange(parsed); + } else { + setYamlError("Invalid dataset structure"); + } + } catch (e) { + if (e instanceof YAMLException) { + setYamlError( + `${e.reason}${e.mark ? ` at line ${e.mark.line + 1}` : ""}`, + ); + } else { + setYamlError("Invalid YAML"); + } + } + }, 500); + }, + [onDatasetChange], + ); + const { nodes: rawNodes, edges } = useDatasetGraph( dataset, protectedFields, @@ -435,44 +510,57 @@ const DatasetNodeEditorInner = ({ className="size-full" style={{ display: "flex", flexDirection: "column" }} > - {/* Breadcrumb navigation */} - {focusedCollection && ( - + {focusedCollection ? ( + + + ), + }, + { title: focusedCollection }, + ]} + /> + + ) : ( +
+ )} + - ), - }, - { title: focusedCollection }, - ]} - /> - - )} + YAML + +
+ {/* Collapsible YAML editor panel */} + {yamlPanelOpen && ( +
+ + + + YAML Editor + + {yamlError && ( + + {yamlError} + + )} + +
+ )} { From f5f1b3974aacaeffc5037c146a3b73d41e9638d6 Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Tue, 17 Mar 2026 12:11:55 -0300 Subject: [PATCH 13/21] Fix review findings: stabilize hover context, dedupe types, improve a11y - Hoist layout options to module constant to prevent useMemo busting - Stabilize onMouseEnter with functional state update (removes dep) - Precompute ancestor/descendant sets per hover instead of per-node - Export ProtectedFieldsInfo from useDatasetGraph, remove duplicate - Remove unused DatasetNodeData type export - Remove no-op onMount from Monaco editor - Add aria-label to add field/nested field buttons - Show add buttons on focus-within (keyboard accessibility) --- .../test-datasets/DatasetEditorSection.tsx | 1 - .../test-datasets/DatasetNodeEditor.tsx | 11 ++----- .../context/DatasetTreeHoverContext.tsx | 31 ++++++++++--------- .../nodes/DatasetCollectionNode.tsx | 1 + .../test-datasets/nodes/DatasetFieldNode.tsx | 1 + .../nodes/DatasetNode.module.scss | 3 +- .../features/test-datasets/useDatasetGraph.ts | 4 +-- 7 files changed, 24 insertions(+), 28 deletions(-) diff --git a/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx b/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx index 4018b6f8292..bdfaa3b5413 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx @@ -314,7 +314,6 @@ const EditorSection = ({ value={editorContent} height="100%" onChange={(value) => setEditorContent(value || "")} - onMount={() => {}} options={{ fontFamily: "Menlo", fontSize: 13, diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx index 21d28600e99..315b4b3a26e 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx @@ -36,16 +36,11 @@ import useDatasetGraph, { CollectionNodeData, DATASET_ROOT_ID, FieldNodeData, + ProtectedFieldsInfo, } from "./useDatasetGraph"; import useDatasetNodeLayout from "./useDatasetNodeLayout"; -interface ProtectedFieldsInfo { - immutable_fields: string[]; - protected_collection_fields: Array<{ - collection: string; - field: string; - }>; -} +const LAYOUT_OPTIONS = { direction: "LR" } as const; interface DatasetNodeEditorProps { dataset: Dataset; @@ -263,7 +258,7 @@ const DatasetNodeEditorInner = ({ const { nodes: layoutedNodes, edges: layoutedEdges } = useDatasetNodeLayout({ nodes: rawNodes, edges, - options: { direction: "LR" }, + options: LAYOUT_OPTIONS, }); // Merge selection + highlight state into nodes diff --git a/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx b/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx index 229c1f64547..44e63342214 100644 --- a/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx +++ b/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx @@ -90,19 +90,24 @@ export const DatasetTreeHoverProvider = ({ [edges], ); - const onMouseEnter = useCallback( - (nodeId: string) => { - if (nodeId !== activeNodeId) { - setActiveNodeId(nodeId); - } - }, - [activeNodeId], - ); + const onMouseEnter = useCallback((nodeId: string) => { + setActiveNodeId((prev) => (prev === nodeId ? prev : nodeId)); + }, []); const onMouseLeave = useCallback(() => { setActiveNodeId(null); }, []); + // Precompute ancestor/descendant sets once per hover change + const ancestorSet = useMemo( + () => (activeNodeId ? getAncestors(activeNodeId) : new Set()), + [activeNodeId, getAncestors], + ); + const descendantSet = useMemo( + () => (activeNodeId ? getDescendants(activeNodeId) : new Set()), + [activeNodeId, getDescendants], + ); + const getNodeHoverStatus = useCallback( (nodeId: string): DatasetNodeHoverStatus => { if (!activeNodeId) { @@ -113,21 +118,17 @@ export const DatasetTreeHoverProvider = ({ return DatasetNodeHoverStatus.ACTIVE_HOVER; } - // Is this node an ancestor of the hovered node? - const ancestors = getAncestors(activeNodeId); - if (ancestors.has(nodeId)) { + if (ancestorSet.has(nodeId)) { return DatasetNodeHoverStatus.PARENT_OF_HOVER; } - // Is this node a descendant of the hovered node? - const descendants = getDescendants(activeNodeId); - if (descendants.has(nodeId)) { + if (descendantSet.has(nodeId)) { return DatasetNodeHoverStatus.CHILD_OF_HOVER; } return DatasetNodeHoverStatus.INACTIVE; }, - [activeNodeId, getAncestors, getDescendants], + [activeNodeId, ancestorSet, descendantSet], ); const value = useMemo( diff --git a/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx b/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx index cfd9b0fb80c..b41fe4b3e9f 100644 --- a/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx +++ b/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx @@ -62,6 +62,7 @@ const DatasetCollectionNode = ({ data, id }: NodeProps) => { actions.addField(nodeData.collection.name); }} title="Add field" + aria-label="Add field" > + diff --git a/clients/admin-ui/src/features/test-datasets/nodes/DatasetFieldNode.tsx b/clients/admin-ui/src/features/test-datasets/nodes/DatasetFieldNode.tsx index b2320da65a5..ecd8471e131 100644 --- a/clients/admin-ui/src/features/test-datasets/nodes/DatasetFieldNode.tsx +++ b/clients/admin-ui/src/features/test-datasets/nodes/DatasetFieldNode.tsx @@ -72,6 +72,7 @@ const DatasetFieldNode = ({ data, id }: NodeProps) => { actions.addField(nodeData.collectionName, nodeData.fieldPath); }} title="Add nested field" + aria-label="Add nested field" > + diff --git a/clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss b/clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss index a5333c7c077..132704fc2a0 100644 --- a/clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss +++ b/clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss @@ -102,7 +102,8 @@ } } -.container:hover .add-button { +.container:hover .add-button, +.container:focus-within .add-button { opacity: 1; } diff --git a/clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts b/clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts index 5a7a5b81cea..a1e4676d025 100644 --- a/clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts +++ b/clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts @@ -26,9 +26,7 @@ export type FieldNodeData = { [key: string]: unknown; }; -export type DatasetNodeData = CollectionNodeData | FieldNodeData; - -interface ProtectedFieldsInfo { +export interface ProtectedFieldsInfo { immutable_fields: string[]; protected_collection_fields: Array<{ collection: string; From 880a72028c4b11a935d1d16b10949f034982fefc Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Tue, 17 Mar 2026 12:20:40 -0300 Subject: [PATCH 14/21] Fix stale node data, debounce detail panel, guard YAML parse errors - Derive selectedNodeData from rawNodes via useMemo instead of storing a stale snapshot in state. Now always reflects the latest dataset. - Debounce detail panel onValuesChange (300ms) to avoid triggering full dataset/graph recomputes on every keystroke. - Guard against null/non-array fields and collections from incomplete YAML edits (e.g. bare "-" list items, partial typing). --- .../test-datasets/DatasetNodeDetailPanel.tsx | 95 ++++++++++++------- .../test-datasets/DatasetNodeEditor.tsx | 23 ++--- .../features/test-datasets/useDatasetGraph.ts | 46 ++++----- 3 files changed, 95 insertions(+), 69 deletions(-) diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx index b5778e50310..545d25a0fc3 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx @@ -10,7 +10,7 @@ import { Typography, useModal, } from "fidesui"; -import { useContext, useEffect } from "react"; +import { useCallback, useContext, useEffect, useRef } from "react"; import { DatasetCollection, DatasetField } from "~/types/api"; @@ -82,46 +82,69 @@ const DatasetNodeDetailPanel = ({ } }, [nodeData, open, form]); - const handleValuesChange = ( - _: Record, - allValues: Record, - ) => { - if (!nodeData) { - return; - } + const debounceRef = useRef | null>(null); - const categories = allValues.data_categories as string[] | undefined; - const baseUpdates = { - description: (allValues.description as string) || undefined, - data_categories: - categories && categories.length > 0 ? categories : undefined, - }; + const flushUpdate = useCallback( + (allValues: Record) => { + if (!nodeData) { + return; + } - if (nodeData.nodeType === "collection") { - const after = allValues.after as string[] | undefined; - const eraseAfter = allValues.erase_after as string[] | undefined; - const skipProcessing = allValues.skip_processing as boolean; - const collectionMeta = { - after: after && after.length > 0 ? after : undefined, - erase_after: - eraseAfter && eraseAfter.length > 0 ? eraseAfter : undefined, - skip_processing: skipProcessing || undefined, + const categories = allValues.data_categories as string[] | undefined; + const baseUpdates = { + description: (allValues.description as string) || undefined, + data_categories: + categories && categories.length > 0 ? categories : undefined, }; - const hasAnyMeta = Object.values(collectionMeta).some( - (v) => v !== undefined, - ); - onUpdateCollection(nodeData.collection.name, { - ...baseUpdates, - fides_meta: hasAnyMeta ? collectionMeta : undefined, - } as Partial); - } else { - onUpdateField(nodeData.collectionName, nodeData.fieldPath, { - ...baseUpdates, - fides_meta: buildFieldMeta(allValues), - } as Partial); + if (nodeData.nodeType === "collection") { + const after = allValues.after as string[] | undefined; + const eraseAfter = allValues.erase_after as string[] | undefined; + const skipProcessing = allValues.skip_processing as boolean; + const collectionMeta = { + after: after && after.length > 0 ? after : undefined, + erase_after: + eraseAfter && eraseAfter.length > 0 ? eraseAfter : undefined, + skip_processing: skipProcessing || undefined, + }; + const hasAnyMeta = Object.values(collectionMeta).some( + (v) => v !== undefined, + ); + + onUpdateCollection(nodeData.collection.name, { + ...baseUpdates, + fides_meta: hasAnyMeta ? collectionMeta : undefined, + } as Partial); + } else { + onUpdateField(nodeData.collectionName, nodeData.fieldPath, { + ...baseUpdates, + fides_meta: buildFieldMeta(allValues), + } as Partial); + } + }, + [nodeData, onUpdateCollection, onUpdateField], + ); + + // Flush pending debounced update when the panel closes + useEffect(() => { + if (!open && debounceRef.current) { + clearTimeout(debounceRef.current); + debounceRef.current = null; } - }; + }, [open]); + + const handleValuesChange = useCallback( + (_: Record, allValues: Record) => { + if (debounceRef.current) { + clearTimeout(debounceRef.current); + } + debounceRef.current = setTimeout(() => { + flushUpdate(allValues); + debounceRef.current = null; + }, 300); + }, + [flushUpdate], + ); const handleDelete = () => { if (!nodeData) { diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx index 315b4b3a26e..d8b77a00772 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx @@ -158,9 +158,6 @@ const DatasetNodeEditorInner = ({ null, ); const [selectedNodeId, setSelectedNodeId] = useState(null); - const [selectedNodeData, setSelectedNodeData] = useState< - CollectionNodeData | FieldNodeData | null - >(null); const [addModal, setAddModal] = useState(CLOSED_MODAL); const [highlightedNodeId, setHighlightedNodeId] = useState( null, @@ -230,9 +227,11 @@ const DatasetNodeEditorInner = ({ typeof parsed === "object" && Array.isArray(parsed.collections) ) { - setYamlError(null); + // Validate that the parsed structure won't crash the graph — + // e.g. a bare "-" in YAML produces null array entries. changeSourceRef.current = "yaml"; onDatasetChange(parsed); + setYamlError(null); } else { setYamlError("Invalid dataset structure"); } @@ -261,6 +260,15 @@ const DatasetNodeEditorInner = ({ options: LAYOUT_OPTIONS, }); + // Derive selected node data from the current graph instead of stale state + const selectedNodeData = useMemo(() => { + if (!selectedNodeId) { + return null; + } + const node = rawNodes.find((n) => n.id === selectedNodeId); + return (node?.data as CollectionNodeData | FieldNodeData) ?? null; + }, [rawNodes, selectedNodeId]); + // Merge selection + highlight state into nodes const nodes = useMemo( () => @@ -289,7 +297,6 @@ const DatasetNodeEditorInner = ({ // Clear selection when drilling in/out useEffect(() => { setSelectedNodeId(null); - setSelectedNodeData(null); }, [focusedCollection]); const handleNodeClick = useCallback( @@ -309,18 +316,15 @@ const DatasetNodeEditorInner = ({ node.id === `${COLLECTION_ROOT_PREFIX}${focusedCollection}` ) { setSelectedNodeId(node.id); - setSelectedNodeData(node.data as CollectionNodeData); return; } setSelectedNodeId(node.id); - setSelectedNodeData(node.data as CollectionNodeData | FieldNodeData); }, [focusedCollection], ); const handlePaneClick = useCallback(() => { setSelectedNodeId(null); - setSelectedNodeData(null); }, []); const handleBack = useCallback(() => { @@ -423,7 +427,6 @@ const DatasetNodeEditorInner = ({ setFocusedCollection(null); } setSelectedNodeId(null); - setSelectedNodeData(null); }, [dataset, onDatasetChange, focusedCollection], ); @@ -441,7 +444,6 @@ const DatasetNodeEditorInner = ({ }), }); setSelectedNodeId(null); - setSelectedNodeData(null); }, [dataset, onDatasetChange], ); @@ -658,7 +660,6 @@ const DatasetNodeEditorInner = ({ open={!!selectedNodeData} onClose={() => { setSelectedNodeId(null); - setSelectedNodeData(null); }} nodeData={selectedNodeData} onUpdateCollection={handleUpdateCollection} diff --git a/clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts b/clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts index a1e4676d025..d7aae90331c 100644 --- a/clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts +++ b/clients/admin-ui/src/features/test-datasets/useDatasetGraph.ts @@ -46,7 +46,7 @@ const buildFieldNodes = ( nodes: Node[], edges: Edge[], ) => { - fields.forEach((field) => { + (Array.isArray(fields) ? fields : []).filter(Boolean).forEach((field) => { const fieldPath = pathPrefix ? `${pathPrefix}.${field.name}` : field.name; const nodeId = `field-${collectionName}-${fieldPath}`; const hasChildren = !!(field.fields && field.fields.length > 0); @@ -168,28 +168,30 @@ const useDatasetGraph = ( }, }); - dataset.collections.forEach((collection) => { - const collectionId = `${COLLECTION_ROOT_PREFIX}${collection.name}`; - - nodes.push({ - id: collectionId, - position: { x: 0, y: 0 }, - type: "datasetCollectionNode", - data: { - label: collection.name, - collection, - nodeType: "collection", - isProtected: false, - } satisfies CollectionNodeData, + (Array.isArray(dataset.collections) ? dataset.collections : []) + .filter(Boolean) + .forEach((collection) => { + const collectionId = `${COLLECTION_ROOT_PREFIX}${collection.name}`; + + nodes.push({ + id: collectionId, + position: { x: 0, y: 0 }, + type: "datasetCollectionNode", + data: { + label: collection.name, + collection, + nodeType: "collection", + isProtected: false, + } satisfies CollectionNodeData, + }); + + edges.push({ + id: `${DATASET_ROOT_ID}->${collectionId}`, + source: DATASET_ROOT_ID, + target: collectionId, + type: "datasetTreeEdge", + }); }); - - edges.push({ - id: `${DATASET_ROOT_ID}->${collectionId}`, - source: DATASET_ROOT_ID, - target: collectionId, - type: "datasetTreeEdge", - }); - }); } return { nodes, edges }; From f8764a6389b4d960f94559ba54ba919b9957b069 Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Wed, 18 Mar 2026 10:48:54 -0300 Subject: [PATCH 15/21] Remove return_all_elements, length, and custom_request_field from node editor forms These advanced fields are better edited directly in YAML. --- .../test-datasets/DatasetNodeDetailPanel.tsx | 3 --- .../test-datasets/FieldMetadataFormItems.tsx | 23 +------------------ 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx index 545d25a0fc3..b216ad2f654 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx @@ -74,9 +74,6 @@ const DatasetNodeDetailPanel = ({ identity: field.fides_meta?.identity ?? "", primary_key: field.fides_meta?.primary_key ?? false, read_only: field.fides_meta?.read_only ?? false, - return_all_elements: field.fides_meta?.return_all_elements ?? false, - length: field.fides_meta?.length ?? undefined, - custom_request_field: field.fides_meta?.custom_request_field ?? "", redact: field.fides_meta?.redact ?? "", }); } diff --git a/clients/admin-ui/src/features/test-datasets/FieldMetadataFormItems.tsx b/clients/admin-ui/src/features/test-datasets/FieldMetadataFormItems.tsx index 0c10243da1c..6d08375ee3c 100644 --- a/clients/admin-ui/src/features/test-datasets/FieldMetadataFormItems.tsx +++ b/clients/admin-ui/src/features/test-datasets/FieldMetadataFormItems.tsx @@ -1,4 +1,4 @@ -import { Form, Input, InputNumber, Select, Switch } from "fidesui"; +import { Form, Input, Select, Switch } from "fidesui"; import { useMemo } from "react"; import useTaxonomies from "~/features/common/hooks/useTaxonomies"; @@ -33,14 +33,6 @@ export const buildFieldMeta = (values: Record) => { identity: (values.identity as string) || undefined, primary_key: (values.primary_key as boolean) || undefined, read_only: (values.read_only as boolean) || undefined, - return_all_elements: (values.return_all_elements as boolean) || undefined, - length: - values.length !== null && - values.length !== undefined && - values.length !== "" - ? (values.length as number) - : undefined, - custom_request_field: (values.custom_request_field as string) || undefined, redact: redactVal === "name" ? ("name" as const) : undefined, }; const hasAny = Object.values(meta).some((v) => v !== undefined); @@ -108,19 +100,6 @@ const FieldMetadataFormItems = () => ( - - - - - - - - - - - - - - - - Test results - - - - - ); -}; - -export default TestResultsSection; diff --git a/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx b/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx index 44e63342214..98cc38a7256 100644 --- a/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx +++ b/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx @@ -11,7 +11,6 @@ export enum DatasetNodeHoverStatus { DEFAULT = "DEFAULT", ACTIVE_HOVER = "ACTIVE_HOVER", PARENT_OF_HOVER = "PARENT_OF_HOVER", - CHILD_OF_HOVER = "CHILD_OF_HOVER", INACTIVE = "INACTIVE", } @@ -51,29 +50,7 @@ const buildAncestryMaps = (edges: Edge[]) => { return ancestors; }; - const getDescendants = (nodeId: string): Set => { - const descendants = new Set(); - // children: parent → children[] - const children = new Map(); - edges.forEach((e) => { - if (!children.has(e.source)) { - children.set(e.source, []); - } - children.get(e.source)!.push(e.target); - }); - const stack = children.get(nodeId) ?? []; - while (stack.length > 0) { - const child = stack.pop()!; - descendants.add(child); - const grandchildren = children.get(child); - if (grandchildren) { - stack.push(...grandchildren); - } - } - return descendants; - }; - - return { getAncestors, getDescendants }; + return { getAncestors }; }; export const DatasetTreeHoverProvider = ({ @@ -85,7 +62,7 @@ export const DatasetTreeHoverProvider = ({ }) => { const [activeNodeId, setActiveNodeId] = useState(null); - const { getAncestors, getDescendants } = useMemo( + const { getAncestors } = useMemo( () => buildAncestryMaps(edges), [edges], ); @@ -103,11 +80,6 @@ export const DatasetTreeHoverProvider = ({ () => (activeNodeId ? getAncestors(activeNodeId) : new Set()), [activeNodeId, getAncestors], ); - const descendantSet = useMemo( - () => (activeNodeId ? getDescendants(activeNodeId) : new Set()), - [activeNodeId, getDescendants], - ); - const getNodeHoverStatus = useCallback( (nodeId: string): DatasetNodeHoverStatus => { if (!activeNodeId) { @@ -122,13 +94,9 @@ export const DatasetTreeHoverProvider = ({ return DatasetNodeHoverStatus.PARENT_OF_HOVER; } - if (descendantSet.has(nodeId)) { - return DatasetNodeHoverStatus.CHILD_OF_HOVER; - } - return DatasetNodeHoverStatus.INACTIVE; }, - [activeNodeId, ancestorSet, descendantSet], + [activeNodeId, ancestorSet], ); const value = useMemo( diff --git a/clients/admin-ui/src/features/test-datasets/dataset-test.slice.ts b/clients/admin-ui/src/features/test-datasets/dataset-test.slice.ts index 0e5caf9f6ac..30ad328c1ef 100644 --- a/clients/admin-ui/src/features/test-datasets/dataset-test.slice.ts +++ b/clients/admin-ui/src/features/test-datasets/dataset-test.slice.ts @@ -6,7 +6,6 @@ import { DatasetConfigSchema } from "~/types/api"; interface DatasetTestState { privacyRequestId: string | null; currentDataset: DatasetConfigSchema | null; - isReachable: boolean; testInputs: Record>; testResults: Record; isTestRunning: boolean; @@ -22,7 +21,6 @@ interface DatasetTestState { const initialState: DatasetTestState = { privacyRequestId: null, currentDataset: null, - isReachable: false, testInputs: {}, testResults: {}, isTestRunning: false, @@ -108,9 +106,6 @@ export const datasetTestSlice = createSlice({ draftState.currentDataset = action.payload; draftState.privacyRequestId = null; }, - setReachability: (draftState, action: PayloadAction) => { - draftState.isReachable = action.payload; - }, setTestResults: ( draftState, action: PayloadAction<{ @@ -140,7 +135,6 @@ export const { setTestInputs, setCurrentPolicyKey, setCurrentDataset, - setReachability, setTestResults, setLogs, clearLogs, @@ -152,8 +146,6 @@ export const selectDatasetTestPrivacyRequestId = (state: RootState) => state.datasetTest.privacyRequestId; export const selectCurrentDataset = (state: RootState) => state.datasetTest.currentDataset; -export const selectIsReachable = (state: RootState) => - state.datasetTest.isReachable; export const selectTestInputs = (state: RootState) => { const { currentDataset } = state.datasetTest; return currentDataset diff --git a/clients/admin-ui/src/features/test-datasets/edges/DatasetTreeEdge.tsx b/clients/admin-ui/src/features/test-datasets/edges/DatasetTreeEdge.tsx index fa76d917ea6..7338486ed47 100644 --- a/clients/admin-ui/src/features/test-datasets/edges/DatasetTreeEdge.tsx +++ b/clients/admin-ui/src/features/test-datasets/edges/DatasetTreeEdge.tsx @@ -1,6 +1,6 @@ import { BezierEdge, BezierEdgeProps } from "@xyflow/react"; import palette from "fidesui/src/palette/palette.module.scss"; -import { useCallback, useContext } from "react"; +import { useContext } from "react"; import { DatasetNodeHoverStatus, @@ -11,39 +11,39 @@ interface DatasetTreeEdgeProps extends BezierEdgeProps { target: string; } +const getStrokeColor = (status: DatasetNodeHoverStatus): string => { + switch (status) { + case DatasetNodeHoverStatus.ACTIVE_HOVER: + case DatasetNodeHoverStatus.PARENT_OF_HOVER: + return palette.FIDESUI_MINOS; + case DatasetNodeHoverStatus.INACTIVE: + return palette.FIDESUI_NEUTRAL_400; + default: + return palette.FIDESUI_SANDSTONE; + } +}; + +const getStrokeWidth = (status: DatasetNodeHoverStatus): number => { + switch (status) { + case DatasetNodeHoverStatus.ACTIVE_HOVER: + case DatasetNodeHoverStatus.PARENT_OF_HOVER: + return 2; + default: + return 1; + } +}; + const DatasetTreeEdge = (props: DatasetTreeEdgeProps) => { const { getNodeHoverStatus } = useContext(DatasetTreeHoverContext); const { target } = props; const targetHoverStatus = getNodeHoverStatus(target); - const getStrokeColor = useCallback(() => { - switch (targetHoverStatus) { - case DatasetNodeHoverStatus.ACTIVE_HOVER: - case DatasetNodeHoverStatus.PARENT_OF_HOVER: - return palette.FIDESUI_MINOS; - case DatasetNodeHoverStatus.INACTIVE: - return palette.FIDESUI_NEUTRAL_400; - default: - return palette.FIDESUI_SANDSTONE; - } - }, [targetHoverStatus]); - - const getStrokeWidth = useCallback(() => { - switch (targetHoverStatus) { - case DatasetNodeHoverStatus.ACTIVE_HOVER: - case DatasetNodeHoverStatus.PARENT_OF_HOVER: - return 2; - default: - return 1; - } - }, [targetHoverStatus]); - return ( diff --git a/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx b/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx index b41fe4b3e9f..6e3b46e1273 100644 --- a/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx +++ b/clients/admin-ui/src/features/test-datasets/nodes/DatasetCollectionNode.tsx @@ -10,6 +10,7 @@ import { import { CollectionNodeData } from "../useDatasetGraph"; import styles from "./DatasetNode.module.scss"; import DatasetNodeHandle from "./DatasetNodeHandle"; +import { getNodeHoverClass } from "./getNodeHoverClass"; const DatasetCollectionNode = ({ data, id }: NodeProps) => { const nodeData = data as CollectionNodeData; @@ -19,19 +20,6 @@ const DatasetCollectionNode = ({ data, id }: NodeProps) => { const actions = useContext(DatasetEditorActionsContext); const hoverStatus = getNodeHoverStatus(id); - const getHoverClass = () => { - switch (hoverStatus) { - case DatasetNodeHoverStatus.ACTIVE_HOVER: - return styles["button--hover"]; - case DatasetNodeHoverStatus.PARENT_OF_HOVER: - return styles["button--parent-hover"]; - case DatasetNodeHoverStatus.INACTIVE: - return styles["button--inactive"]; - default: - return ""; - } - }; - return (
{ inactive={hoverStatus === DatasetNodeHoverStatus.INACTIVE} /> {nodeData.isRoot && ( From 8dfc5c16820dea86cf33181a7266e45d859d574f Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Mon, 23 Mar 2026 11:35:36 -0300 Subject: [PATCH 19/21] Fix review issues: stale closures, flush debounce on close, dead code cleanup, tests - Flush pending debounced update on detail panel close instead of discarding - Use datasetRef.current in all mutation handlers to prevent stale closure bugs - Extract tree helpers (updateFieldAtPath, removeFieldAtPath, addNestedField, getFieldsAtPath) into dataset-field-helpers.ts with 16 unit tests - Remove dead Redux state/actions from dataset-test.slice (test runner, logs, privacy request tracking no longer used after TestRunnerSection removal) - Replace hardcoded hex colors with design token CSS variables --- .../dataset-field-helpers.test.ts | 148 ++++++++++++++++++ .../test-datasets/DatasetEditorSection.tsx | 2 +- .../test-datasets/DatasetNodeDetailPanel.tsx | 3 +- .../test-datasets/DatasetNodeEditor.tsx | 132 +++++----------- .../test-datasets/dataset-field-helpers.ts | 73 +++++++++ .../test-datasets/dataset-test.slice.ts | 132 +--------------- .../nodes/DatasetNode.module.scss | 10 +- 7 files changed, 267 insertions(+), 233 deletions(-) create mode 100644 clients/admin-ui/__tests__/features/test-datasets/dataset-field-helpers.test.ts create mode 100644 clients/admin-ui/src/features/test-datasets/dataset-field-helpers.ts diff --git a/clients/admin-ui/__tests__/features/test-datasets/dataset-field-helpers.test.ts b/clients/admin-ui/__tests__/features/test-datasets/dataset-field-helpers.test.ts new file mode 100644 index 00000000000..cbd393b4823 --- /dev/null +++ b/clients/admin-ui/__tests__/features/test-datasets/dataset-field-helpers.test.ts @@ -0,0 +1,148 @@ +import { + addNestedField, + getFieldsAtPath, + removeFieldAtPath, + updateFieldAtPath, +} from "~/features/test-datasets/dataset-field-helpers"; + +import { DatasetField } from "~/types/api"; + +const makeField = ( + name: string, + children?: DatasetField[], +): DatasetField => ({ + name, + ...(children ? { fields: children } : {}), +}); + +describe("dataset-field-helpers", () => { + describe("updateFieldAtPath", () => { + it("updates a top-level field", () => { + const fields = [makeField("a"), makeField("b")]; + const result = updateFieldAtPath(fields, ["b"], { + description: "updated", + }); + expect(result[0]).toEqual(fields[0]); + expect(result[1]).toEqual({ name: "b", description: "updated" }); + }); + + it("updates a nested field", () => { + const fields = [makeField("a", [makeField("b", [makeField("c")])])]; + const result = updateFieldAtPath(fields, ["a", "b", "c"], { + description: "deep", + }); + expect(result[0].fields![0].fields![0]).toEqual({ + name: "c", + description: "deep", + }); + }); + + it("leaves non-matching fields untouched", () => { + const fields = [makeField("a"), makeField("b")]; + const result = updateFieldAtPath(fields, ["a"], { + description: "only a", + }); + expect(result[1]).toBe(fields[1]); + }); + + it("handles missing path gracefully", () => { + const fields = [makeField("a")]; + const result = updateFieldAtPath(fields, ["nonexistent"], { + description: "x", + }); + expect(result).toEqual(fields); + }); + }); + + describe("getFieldsAtPath", () => { + it("returns children of a top-level field", () => { + const child1 = makeField("c1"); + const child2 = makeField("c2"); + const fields = [makeField("parent", [child1, child2])]; + const result = getFieldsAtPath(fields, ["parent"]); + expect(result).toEqual([child1, child2]); + }); + + it("returns children of a nested field", () => { + const leaf = makeField("leaf"); + const fields = [makeField("a", [makeField("b", [leaf])])]; + const result = getFieldsAtPath(fields, ["a", "b"]); + expect(result).toEqual([leaf]); + }); + + it("returns empty array for missing path", () => { + const fields = [makeField("a")]; + expect(getFieldsAtPath(fields, ["missing"])).toEqual([]); + }); + + it("returns empty array for field with no children", () => { + const fields = [makeField("a")]; + expect(getFieldsAtPath(fields, ["a"])).toEqual([]); + }); + }); + + describe("addNestedField", () => { + it("adds a child to a top-level field", () => { + const fields = [makeField("parent", [makeField("existing")])]; + const newField = makeField("new_child"); + const result = addNestedField(fields, ["parent"], newField); + expect(result[0].fields).toHaveLength(2); + expect(result[0].fields![1]).toEqual(newField); + }); + + it("adds a child to a deeply nested field", () => { + const fields = [makeField("a", [makeField("b", [])])]; + const newField = makeField("c"); + const result = addNestedField(fields, ["a", "b"], newField); + expect(result[0].fields![0].fields).toEqual([newField]); + }); + + it("creates fields array if parent has none", () => { + const fields = [makeField("parent")]; + const newField = makeField("child"); + const result = addNestedField(fields, ["parent"], newField); + expect(result[0].fields).toEqual([newField]); + }); + + it("does not modify non-matching siblings", () => { + const sibling = makeField("sibling"); + const fields = [sibling, makeField("target", [])]; + const newField = makeField("child"); + const result = addNestedField(fields, ["target"], newField); + expect(result[0]).toBe(sibling); + }); + }); + + describe("removeFieldAtPath", () => { + it("removes a top-level field", () => { + const fields = [makeField("a"), makeField("b"), makeField("c")]; + const result = removeFieldAtPath(fields, ["b"]); + expect(result).toHaveLength(2); + expect(result.map((f) => f.name)).toEqual(["a", "c"]); + }); + + it("removes a nested field", () => { + const fields = [ + makeField("a", [makeField("b"), makeField("c")]), + ]; + const result = removeFieldAtPath(fields, ["a", "b"]); + expect(result[0].fields).toHaveLength(1); + expect(result[0].fields![0].name).toBe("c"); + }); + + it("removes a deeply nested field", () => { + const fields = [ + makeField("a", [makeField("b", [makeField("c"), makeField("d")])]), + ]; + const result = removeFieldAtPath(fields, ["a", "b", "c"]); + expect(result[0].fields![0].fields).toHaveLength(1); + expect(result[0].fields![0].fields![0].name).toBe("d"); + }); + + it("returns unchanged array if field not found", () => { + const fields = [makeField("a")]; + const result = removeFieldAtPath(fields, ["nonexistent"]); + expect(result).toEqual(fields); + }); + }); +}); diff --git a/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx b/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx index 844ad35cdcb..33e72f5cf1a 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetEditorSection.tsx @@ -278,7 +278,7 @@ const EditorSection = ({ minHeight: 0, borderRadius: 8, overflow: "hidden", - border: "1px solid #E2E8F0", + border: "1px solid var(--fidesui-neutral-200)", }} > {localDataset && ( diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx index 65449984fb1..02a61e9ab5c 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx @@ -127,8 +127,9 @@ const DatasetNodeDetailPanel = ({ if (!open && debounceRef.current) { clearTimeout(debounceRef.current); debounceRef.current = null; + flushUpdate(form.getFieldsValue()); } - }, [open]); + }, [open, flushUpdate, form]); // Clean up debounce timer on unmount useEffect(() => { diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx index 822bc27ac14..af0c6dc7059 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx @@ -30,6 +30,12 @@ import AddNodeModal from "./AddNodeModal"; import DatasetEditorActionsContext, { DatasetEditorActions, } from "./context/DatasetEditorActionsContext"; +import { + addNestedField, + getFieldsAtPath, + removeFieldAtPath, + updateFieldAtPath, +} from "./dataset-field-helpers"; import { DatasetTreeHoverProvider } from "./context/DatasetTreeHoverContext"; import DatasetNodeDetailPanel from "./DatasetNodeDetailPanel"; import DatasetTreeEdge from "./edges/DatasetTreeEdge"; @@ -64,78 +70,6 @@ const edgeTypes: EdgeTypes = { datasetTreeEdge: DatasetTreeEdge, }; -/** Walk nested fields to find and update the one at fieldPath */ -const updateFieldAtPath = ( - fields: DatasetField[], - segments: string[], - updates: Partial, -): DatasetField[] => - fields.map((f) => { - if (f.name !== segments[0]) { - return f; - } - if (segments.length === 1) { - return { ...f, ...updates }; - } - return { - ...f, - fields: updateFieldAtPath(f.fields ?? [], segments.slice(1), updates), - }; - }); - -/** Get the children fields of a field at the given path */ -const getFieldsAtPath = ( - fields: DatasetField[], - segments: string[], -): DatasetField[] => { - const match = fields.find((f) => f.name === segments[0]); - if (!match) { - return []; - } - if (segments.length === 1) { - return match.fields ?? []; - } - return getFieldsAtPath(match.fields ?? [], segments.slice(1)); -}; - -/** Append a new child field to the parent at the given path */ -const addNestedField = ( - fields: DatasetField[], - segments: string[], - newField: DatasetField, -): DatasetField[] => - fields.map((f) => { - if (f.name !== segments[0]) { - return f; - } - if (segments.length === 1) { - return { ...f, fields: [...(f.fields ?? []), newField] }; - } - return { - ...f, - fields: addNestedField(f.fields ?? [], segments.slice(1), newField), - }; - }); - -/** Recursively remove a field at the given path */ -const removeFieldAtPath = ( - fields: DatasetField[], - segments: string[], -): DatasetField[] => { - if (segments.length === 1) { - return fields.filter((f) => f.name !== segments[0]); - } - return fields.map((f) => { - if (f.name !== segments[0]) { - return f; - } - return { - ...f, - fields: removeFieldAtPath(f.fields ?? [], segments.slice(1)), - }; - }); -}; - interface AddModalState { open: boolean; title: string; @@ -341,15 +275,16 @@ const DatasetNodeEditorInner = ({ const handleUpdateCollection = useCallback( (collectionName: string, updates: Partial) => { + const current = datasetRef.current; const updated: Dataset = { - ...dataset, - collections: dataset.collections.map((c) => + ...current, + collections: current.collections.map((c) => c.name === collectionName ? { ...c, ...updates } : c, ), }; onDatasetChange(updated); }, - [dataset, onDatasetChange], + [onDatasetChange], ); const handleUpdateField = useCallback( @@ -358,10 +293,11 @@ const DatasetNodeEditorInner = ({ fieldPath: string, updates: Partial, ) => { + const current = datasetRef.current; const segments = fieldPath.split("."); const updated: Dataset = { - ...dataset, - collections: dataset.collections.map((c) => { + ...current, + collections: current.collections.map((c) => { if (c.name !== collectionName) { return c; } @@ -373,24 +309,28 @@ const DatasetNodeEditorInner = ({ }; onDatasetChange(updated); }, - [dataset, onDatasetChange], + [onDatasetChange], ); // --- Add / Delete handlers --- + const datasetRef = useRef(dataset); + datasetRef.current = dataset; + const handleAddCollection = useCallback( (name: string) => { + const current = datasetRef.current; const newCollection: DatasetCollection = { name, fields: [], }; onDatasetChange({ - ...dataset, - collections: [...dataset.collections, newCollection], + ...current, + collections: [...current.collections, newCollection], }); highlightNode(`${COLLECTION_ROOT_PREFIX}${name}`); }, - [dataset, onDatasetChange, highlightNode], + [onDatasetChange, highlightNode], ); const handleAddField = useCallback( @@ -400,10 +340,11 @@ const DatasetNodeEditorInner = ({ parentFieldPath?: string, fieldData?: Partial, ) => { + const current = datasetRef.current; const newField: DatasetField = { name, ...fieldData }; onDatasetChange({ - ...dataset, - collections: dataset.collections.map((c) => { + ...current, + collections: current.collections.map((c) => { if (c.name !== collectionName) { return c; } @@ -420,14 +361,15 @@ const DatasetNodeEditorInner = ({ const fieldPath = parentFieldPath ? `${parentFieldPath}.${name}` : name; highlightNode(`field-${collectionName}-${fieldPath}`); }, - [dataset, onDatasetChange, highlightNode], + [onDatasetChange, highlightNode], ); const handleDeleteCollection = useCallback( (collectionName: string) => { + const current = datasetRef.current; onDatasetChange({ - ...dataset, - collections: dataset.collections.filter( + ...current, + collections: current.collections.filter( (c) => c.name !== collectionName, ), }); @@ -436,15 +378,16 @@ const DatasetNodeEditorInner = ({ } setSelectedNodeId(null); }, - [dataset, onDatasetChange, focusedCollection], + [onDatasetChange, focusedCollection], ); const handleDeleteField = useCallback( (collectionName: string, fieldPath: string) => { + const current = datasetRef.current; const segments = fieldPath.split("."); onDatasetChange({ - ...dataset, - collections: dataset.collections.map((c) => { + ...current, + collections: current.collections.map((c) => { if (c.name !== collectionName) { return c; } @@ -453,12 +396,9 @@ const DatasetNodeEditorInner = ({ }); setSelectedNodeId(null); }, - [dataset, onDatasetChange], + [onDatasetChange], ); - const datasetRef = useRef(dataset); - datasetRef.current = dataset; - const editorActions: DatasetEditorActions = useMemo( () => ({ addCollection: () => { @@ -525,7 +465,7 @@ const DatasetNodeEditorInner = ({ justify="space-between" style={{ padding: "6px 12px", - borderBottom: "1px solid #E2E8F0", + borderBottom: "1px solid var(--fidesui-neutral-200)", backgroundColor: "white", flexShrink: 0, }} @@ -610,7 +550,7 @@ const DatasetNodeEditorInner = ({ style={{ flexShrink: 0, height: 300, - borderTop: "1px solid #E2E8F0", + borderTop: "1px solid var(--fidesui-neutral-200)", display: "flex", flexDirection: "column", backgroundColor: "white", @@ -621,7 +561,7 @@ const DatasetNodeEditorInner = ({ justify="space-between" style={{ padding: "4px 12px", - borderBottom: "1px solid #E2E8F0", + borderBottom: "1px solid var(--fidesui-neutral-200)", flexShrink: 0, }} > diff --git a/clients/admin-ui/src/features/test-datasets/dataset-field-helpers.ts b/clients/admin-ui/src/features/test-datasets/dataset-field-helpers.ts new file mode 100644 index 00000000000..5f86e588dcb --- /dev/null +++ b/clients/admin-ui/src/features/test-datasets/dataset-field-helpers.ts @@ -0,0 +1,73 @@ +import { DatasetField } from "~/types/api"; + +/** Walk nested fields to find and update the one at fieldPath */ +export const updateFieldAtPath = ( + fields: DatasetField[], + segments: string[], + updates: Partial, +): DatasetField[] => + fields.map((f) => { + if (f.name !== segments[0]) { + return f; + } + if (segments.length === 1) { + return { ...f, ...updates }; + } + return { + ...f, + fields: updateFieldAtPath(f.fields ?? [], segments.slice(1), updates), + }; + }); + +/** Get the children fields of a field at the given path */ +export const getFieldsAtPath = ( + fields: DatasetField[], + segments: string[], +): DatasetField[] => { + const match = fields.find((f) => f.name === segments[0]); + if (!match) { + return []; + } + if (segments.length === 1) { + return match.fields ?? []; + } + return getFieldsAtPath(match.fields ?? [], segments.slice(1)); +}; + +/** Append a new child field to the parent at the given path */ +export const addNestedField = ( + fields: DatasetField[], + segments: string[], + newField: DatasetField, +): DatasetField[] => + fields.map((f) => { + if (f.name !== segments[0]) { + return f; + } + if (segments.length === 1) { + return { ...f, fields: [...(f.fields ?? []), newField] }; + } + return { + ...f, + fields: addNestedField(f.fields ?? [], segments.slice(1), newField), + }; + }); + +/** Recursively remove a field at the given path */ +export const removeFieldAtPath = ( + fields: DatasetField[], + segments: string[], +): DatasetField[] => { + if (segments.length === 1) { + return fields.filter((f) => f.name !== segments[0]); + } + return fields.map((f) => { + if (f.name !== segments[0]) { + return f; + } + return { + ...f, + fields: removeFieldAtPath(f.fields ?? [], segments.slice(1)), + }; + }); +}; diff --git a/clients/admin-ui/src/features/test-datasets/dataset-test.slice.ts b/clients/admin-ui/src/features/test-datasets/dataset-test.slice.ts index 30ad328c1ef..13da9926e1d 100644 --- a/clients/admin-ui/src/features/test-datasets/dataset-test.slice.ts +++ b/clients/admin-ui/src/features/test-datasets/dataset-test.slice.ts @@ -4,98 +4,18 @@ import type { RootState } from "~/app/store"; import { DatasetConfigSchema } from "~/types/api"; interface DatasetTestState { - privacyRequestId: string | null; currentDataset: DatasetConfigSchema | null; - testInputs: Record>; - testResults: Record; - isTestRunning: boolean; currentPolicyKey?: string; - logs: Array<{ - timestamp: string; - level: string; - module_info: string; - message: string; - }>; } const initialState: DatasetTestState = { - privacyRequestId: null, currentDataset: null, - testInputs: {}, - testResults: {}, - isTestRunning: false, - logs: [], }; export const datasetTestSlice = createSlice({ name: "datasetTest", initialState, reducers: { - startTest: (draftState, action: PayloadAction) => { - draftState.testResults = { - ...draftState.testResults, - [action.payload]: "", - }; - draftState.isTestRunning = true; - draftState.logs = []; - }, - setPrivacyRequestId: (draftState, action: PayloadAction) => { - draftState.privacyRequestId = action.payload; - }, - finishTest: (draftState) => { - draftState.privacyRequestId = null; - draftState.isTestRunning = false; - }, - interruptTest: (draftState) => { - if (draftState.currentDataset?.fides_key) { - draftState.testResults = { - ...draftState.testResults, - [draftState.currentDataset.fides_key]: "", - }; - } - draftState.logs = []; - draftState.privacyRequestId = null; - draftState.isTestRunning = false; - }, - setTestInputs: ( - draftState, - action: PayloadAction<{ - datasetKey: string; - values: Record; - }>, - ) => { - const existingValues = - draftState.testInputs[action.payload.datasetKey] || {}; - const inputsData = action.payload.values || {}; - - // Skip processing if we're trying to set empty values over existing ones - if ( - Object.keys(existingValues).length > 0 && - Object.keys(inputsData).length === 0 - ) { - return; - } - - // Start with input data - const mergedValues = { ...inputsData }; - - // For each key in the existing values - Object.entries(existingValues).forEach(([key, existingValue]) => { - // If the new value is null and we have an existing non-null value, keep the existing value - if ( - key in mergedValues && - mergedValues[key] === null && - existingValue !== null - ) { - mergedValues[key] = existingValue; - } - }); - - draftState.testInputs = { - ...draftState.testInputs, - [action.payload.datasetKey]: mergedValues, - }; - }, setCurrentPolicyKey: (draftState, action: PayloadAction) => { draftState.currentPolicyKey = action.payload; }, @@ -104,64 +24,16 @@ export const datasetTestSlice = createSlice({ action: PayloadAction, ) => { draftState.currentDataset = action.payload; - draftState.privacyRequestId = null; - }, - setTestResults: ( - draftState, - action: PayloadAction<{ - datasetKey: string; - values: string; - }>, - ) => { - draftState.testResults = { - ...draftState.testResults, - [action.payload.datasetKey]: action.payload.values, - }; - }, - setLogs: (draftState, action: PayloadAction) => { - draftState.logs = action.payload; - }, - clearLogs: (draftState) => { - draftState.logs = []; }, }, }); -export const { - startTest, - setPrivacyRequestId, - finishTest, - interruptTest, - setTestInputs, - setCurrentPolicyKey, - setCurrentDataset, - setTestResults, - setLogs, - clearLogs, -} = datasetTestSlice.actions; +export const { setCurrentPolicyKey, setCurrentDataset } = + datasetTestSlice.actions; -export const selectPrivacyRequestId = (state: RootState) => - state.datasetTest.privacyRequestId; -export const selectDatasetTestPrivacyRequestId = (state: RootState) => - state.datasetTest.privacyRequestId; export const selectCurrentDataset = (state: RootState) => state.datasetTest.currentDataset; -export const selectTestInputs = (state: RootState) => { - const { currentDataset } = state.datasetTest; - return currentDataset - ? state.datasetTest.testInputs[currentDataset.fides_key] || {} - : {}; -}; export const selectCurrentPolicyKey = (state: RootState) => state.datasetTest.currentPolicyKey; -export const selectTestResults = (state: RootState) => { - const { currentDataset } = state.datasetTest; - return currentDataset - ? state.datasetTest.testResults[currentDataset.fides_key] || "" - : ""; -}; -export const selectIsTestRunning = (state: RootState) => - state.datasetTest.isTestRunning; -export const selectLogs = (state: RootState) => state.datasetTest.logs; export const { reducer } = datasetTestSlice; diff --git a/clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss b/clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss index 132704fc2a0..d323e4a791a 100644 --- a/clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss +++ b/clients/admin-ui/src/features/test-datasets/nodes/DatasetNode.module.scss @@ -38,10 +38,10 @@ &--protected { font-style: italic; opacity: 0.85; - border-color: #d69e2e; + border-color: var(--fidesui-warning); &:hover { - border-color: #d69e2e !important; + border-color: var(--fidesui-warning) !important; } } } @@ -66,7 +66,7 @@ } .badge--warning { - background-color: #d69e2e; + background-color: var(--fidesui-warning); font-size: 9px; } @@ -109,8 +109,8 @@ @keyframes highlight-fade { 0% { - background-color: #fefcbf; - border-color: #d69e2e; + background-color: var(--fidesui-bg-caution); + border-color: var(--fidesui-warning); } 100% { background-color: transparent; From 6126fb6349d8c33c8006379e6463c787dc7b640b Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Mon, 23 Mar 2026 11:50:39 -0300 Subject: [PATCH 20/21] Address PR review feedback - Replace div flex containers with Flex from fidesui in DatasetNodeDetailPanel and DatasetNodeEditor - Rename single-char variable `e` to `edge` in DatasetTreeHoverContext --- .../test-datasets/DatasetNodeDetailPanel.tsx | 5 +++-- .../features/test-datasets/DatasetNodeEditor.tsx | 15 +++++---------- .../context/DatasetTreeHoverContext.tsx | 4 ++-- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx index 02a61e9ab5c..a5ea5f41fef 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeDetailPanel.tsx @@ -2,6 +2,7 @@ import { Button, Collapse, Drawer, + Flex, Form, Input, Select, @@ -181,7 +182,7 @@ const DatasetNodeDetailPanel = ({ return ( + {nodeData?.label} {isProtected && protected} -
+ } placement="right" width={400} diff --git a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx index af0c6dc7059..e4ab9670978 100644 --- a/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx +++ b/clients/admin-ui/src/features/test-datasets/DatasetNodeEditor.tsx @@ -454,11 +454,7 @@ const DatasetNodeEditorInner = ({ return ( -
+ {/* Toolbar / Breadcrumb navigation */} {/* Collapsible YAML editor panel */} {yamlPanelOpen && ( -
@@ -605,7 +600,7 @@ const DatasetNodeEditorInner = ({ theme="light" />
-
+ )} setAddModal(CLOSED_MODAL)} /> -
+
); }; diff --git a/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx b/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx index 98cc38a7256..37855e9edbe 100644 --- a/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx +++ b/clients/admin-ui/src/features/test-datasets/context/DatasetTreeHoverContext.tsx @@ -36,8 +36,8 @@ export const DatasetTreeHoverContext = const buildAncestryMaps = (edges: Edge[]) => { // parentOf: child → parent const parentOf = new Map(); - edges.forEach((e) => { - parentOf.set(e.target, e.source); + edges.forEach((edge) => { + parentOf.set(edge.target, edge.source); }); const getAncestors = (nodeId: string): Set => { From 50d0d72afff26ff66872735bb62964cf5400c368 Mon Sep 17 00:00:00 2001 From: Facundo Lopez Janza Date: Mon, 23 Mar 2026 12:27:30 -0300 Subject: [PATCH 21/21] Remove dead imports for deleted TestLogsSection and TestRunnerSection These components were deleted in d8207d915 but their imports and usages in the test-datasets page were not removed, causing a build failure. --- .../systems/configure/[id]/test-datasets.tsx | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/clients/admin-ui/src/pages/systems/configure/[id]/test-datasets.tsx b/clients/admin-ui/src/pages/systems/configure/[id]/test-datasets.tsx index 2f927d27300..fb59b61e3b3 100644 --- a/clients/admin-ui/src/pages/systems/configure/[id]/test-datasets.tsx +++ b/clients/admin-ui/src/pages/systems/configure/[id]/test-datasets.tsx @@ -7,8 +7,6 @@ import { SYSTEM_ROUTE } from "~/features/common/nav/routes"; import PageHeader from "~/features/common/PageHeader"; import { useGetSystemByFidesKeyQuery } from "~/features/system"; import EditorSection from "~/features/test-datasets/DatasetEditorSection"; -import TestLogsSection from "~/features/test-datasets/TestLogsSection"; -import TestResultsSection from "~/features/test-datasets/TestRunnerSection"; // Helper functions const getSystemId = (query: { id?: string | string[] }): string => { @@ -58,21 +56,10 @@ const TestDatasetPage: NextPage = () => { ]} /> - - - - - - - + );