refactor(ui): extract child definition tree traversal to a common method

Remove outdated TODOs. Fix formatting.

Author: a-asaad

ui/src/components/canvas/EipNode.tsx

@@ -87,7 +87,6 @@ const getClassNames = (props: NodeProps<EipNodeData>, role: EipRole) => {
 
 // TODO: Consider separating into Endpoint and Channel custom node types
 export const EipNode = (props: NodeProps<EipNodeData>) => {
-  // TODO: clearSelectedChildNode is used in too many different components. See if that can be reduced (or elimnated).
   const layout = useGetLayout()
   const children = useGetEnabledChildren(props.id)
   const hasChildren = children.length > 0

ui/src/components/config-panel/ChildManagementModal.tsx

@@ -26,6 +26,7 @@ import {
 import { useGetEnabledChildren } from "../../singletons/store/getterHooks"
 import { getEipId } from "../../singletons/store/storeViews"
 import { DraggableList, DraggableListItem } from "./DraggableList"
+import { findChildDefinition } from "./childDefinitions"
 
 interface ChildPathBreadcrumbProps {
   path: string[]
@@ -51,20 +52,13 @@ interface ChildModalProps {
   initPath?: string[]
 }
 
-// TODO: Should this be a utility method in the EipComponentDef module?
-const getEipDefinition = (rootEipDef: EipComponent, path: string[]) => {
-  let children = rootEipDef.childGroup?.children
-
+const getChildEipDefinition = (rootEipDef: EipComponent, path: string[]) => {
   if (path.length == 1) {
-    return children
+    return rootEipDef.childGroup?.children
   }
 
-  for (const id of path.slice(1)) {
-    const name = getEipId(id)?.name
-    const child = children?.find((c) => c.name === name)
-    children = child?.childGroup?.children
-  }
-  return children
+  const child = findChildDefinition(rootEipDef, path)
+  return child?.childGroup?.children
 }
 
 const ChildPathBreadcrumb = ({
@@ -164,7 +158,7 @@ export const ChildManagementModal = ({
   const rootEipId = getEipId(rootId)
   const rootEipDef = rootEipId && lookupEipComponent(rootEipId)
 
-  const childOptions = rootEipDef && getEipDefinition(rootEipDef, path)
+  const childOptions = rootEipDef && getChildEipDefinition(rootEipDef, path)
 
   let modalContent: ReactNode
   if (childOptions && childOptions.length > 0) {

ui/src/components/config-panel/EipConfigSidePanel.tsx

@@ -2,11 +2,7 @@ import { HeaderPanel } from "@carbon/react"
 import { useState } from "react"
 import { Edge, useOnSelectionChange, useStoreApi } from "reactflow"
 import { DYNAMIC_EDGE_TYPE, DynamicEdge, EipFlowNode } from "../../api/flow"
-import {
-  Attribute,
-  EipChildElement,
-  EipComponent,
-} from "../../api/generated/eipComponentDef"
+import { Attribute } from "../../api/generated/eipComponentDef"
 import { EipId } from "../../api/generated/eipFlow"
 import {
   FLOW_CONTROLLED_ATTRIBUTES,
@@ -18,6 +14,7 @@ import { useGetSelectedChildNode } from "../../singletons/store/getterHooks"
 import { getEipId } from "../../singletons/store/storeViews"
 import DynamicEdgeConfig from "./DynamicEdgeConfig"
 import { ChildNodeConfig, RootNodeConfig } from "./NodeConfigPanel"
+import { findChildDefinition } from "./childDefinitions"
 
 const isDynamicRouterAttribute = (attribute: Attribute, eipId?: EipId) => {
   if (!eipId) {
@@ -44,25 +41,6 @@ const filterConfigurableAttributes = (attrs?: Attribute[], eipId?: EipId) => {
 
 const isDynamicEdge = (edge: Edge) => edge?.type === DYNAMIC_EDGE_TYPE
 
-// TODO: Should this be a utility method in the EipComponentDef module?
-// TODO: Handle invalid path error case
-const findChildDefinition = (
-  rootComponent: EipComponent,
-  childPath: string[]
-) => {
-  let children = rootComponent.childGroup?.children
-  let child: EipChildElement | null = null
-
-  for (const id of childPath.slice(1)) {
-    const name = getEipId(id)?.name
-    child = children?.find((c) => c.name === name) ?? null
-    children = child?.childGroup?.children
-  }
-
-  return child
-}
-
-// TODO: Add breadcrumb menu at the top, showing the path to child.
 const EipConfigSidePanel = () => {
   const reactFlowStore = useStoreApi()
   const [selectedNode, setSelectedNode] = useState<EipFlowNode | null>(null)
@@ -81,7 +59,6 @@ const EipConfigSidePanel = () => {
   })
 
   let sidePanelContent
-  // TODO: Simplify conditionals
   if (selectedChildPath) {
     // TODO: Handle error case if childElement or rootComponent is undefined
     const childId = selectedChildPath[selectedChildPath.length - 1]

ui/src/components/config-panel/childDefinitions.ts

@@ -0,0 +1,26 @@
+import {
+  EipChildElement,
+  EipComponent,
+} from "../../api/generated/eipComponentDef"
+import { getEipId } from "../../singletons/store/storeViews"
+
+// Searches the root's child tree to find the child definition referenced by 'childPath'
+export const findChildDefinition = (
+  rootEipComponent: EipComponent,
+  childPath: string[]
+) => {
+  let children = rootEipComponent.childGroup?.children
+  let child: EipChildElement | null = null
+
+  if (childPath.length <= 1) {
+    return null
+  }
+
+  for (const id of childPath.slice(1)) {
+    const name = getEipId(id)?.name
+    child = children?.find((c) => c.name === name) ?? null
+    children = child?.childGroup?.children
+  }
+
+  return child
+}

ui/src/components/toolbar/assistant/AssistantChatPanel.tsx

@@ -128,7 +128,6 @@ const ChatHistory = forwardRef<HTMLParagraphElement, ChatHistoryProps>(
 
 ChatHistory.displayName = "ChatHistory"
 
-// TODO: Create a new store action to set nodes/edges directly from object instead of JSON string.
 const AssistantChatPanel = () => {
   const chatHistoryEndRef = useRef<HTMLParagraphElement | null>(null)
   const [chatEntries, setChatEntries] = useState<ChatEntry[]>([])

ui/src/singletons/store/appStore.ts

@@ -1,60 +1,52 @@
 import isDeepEqual from "fast-deep-equal"
 import { temporal } from "zundo"
 import { create } from "zustand"
-import { createJSONStorage, devtools, persist } from "zustand/middleware"
+import { createJSONStorage, persist } from "zustand/middleware"
 import debounce from "../../utils/debounce"
 
 import { AppStore } from "./api"
 
 export const EXPORTED_FLOW_VERSION = "1.0"
 
 // If app becomes too slow, might need to switch to async persistent storage.
-// TODO: Remove devtools
 export const useAppStore = create<AppStore>()(
-  devtools(
-    persist(
-      temporal(
-        (_) => ({
-          nodes: [],
-          edges: [],
-          eipConfigs: {},
-          selectedChildNode: null,
-          layout: {
-            orientation: "horizontal",
-            density: "comfortable",
-          },
-        }),
-        {
-          limit: 50,
-          partialize: (state) => {
-            const newNodes = state.nodes.map((node) => {
-              const n = { ...node }
-              const {
-                selected,
-                draggable,
-                dragging,
-                positionAbsolute,
-                ...rest
-              } = n
-              return rest
-            })
+  persist(
+    temporal(
+      (_) => ({
+        nodes: [],
+        edges: [],
+        eipConfigs: {},
+        selectedChildNode: null,
+        layout: {
+          orientation: "horizontal",
+          density: "comfortable",
+        },
+      }),
+      {
+        limit: 50,
+        partialize: (state) => {
+          const newNodes = state.nodes.map((node) => {
+            const n = { ...node }
+            const { selected, draggable, dragging, positionAbsolute, ...rest } =
+              n
+            return rest
+          })
 
-            const { eipConfigs, edges, layout } = state
-            return { eipConfigs, edges, layout, nodes: newNodes }
-          },
+          const { eipConfigs, edges, layout } = state
+          return { eipConfigs, edges, layout, nodes: newNodes }
+        },
 
-          equality: (pastState, currentState) =>
-            isDeepEqual(pastState, currentState),
+        equality: (pastState, currentState) =>
+          isDeepEqual(pastState, currentState),
 
-          handleSet: (handleSet) =>
-            debounce<typeof handleSet>((state) => handleSet(state), 1000, true),
-        }
-      ),
-      {
-        name: "eipFlow",
-        version: Number(EXPORTED_FLOW_VERSION.split(".")[0]),
-        storage: createJSONStorage(() => localStorage),
+        handleSet: (handleSet) =>
+          debounce<typeof handleSet>((state) => handleSet(state), 1000, true),
       }
-    )
+    ),
+    {
+      name: "eipFlow",
+      version: Number(EXPORTED_FLOW_VERSION.split(".")[0]),
+      storage: createJSONStorage(() => localStorage),
+    }
   )
 )

ui/src/singletons/store/reactFlowActions.test.ts

@@ -19,7 +19,6 @@ beforeEach(() => {
   })
 })
 
-// TODO: test that child node configs are deleted recursively
 describe("onNodesChange", () => {
   test("top-level config and all child configs are removed when node is deleted", () => {
     // assert configs exist before removing node