refactor: move icon fetching from the canvas layer to the mapping layer#3100
Open
shivamG640 wants to merge 1 commit intoKaotoIO:mainfrom
Open
refactor: move icon fetching from the canvas layer to the mapping layer#3100shivamG640 wants to merge 1 commit intoKaotoIO:mainfrom
shivamG640 wants to merge 1 commit intoKaotoIO:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request refactors icon handling across the codebase by removing the Changes
Sequence Diagram(s)sequenceDiagram
participant Comp as Component
participant VizHook as useVisibleVizNodes
participant Ctx as VisibleFlowsContext
participant Entity as BaseVisualEntity
participant Enricher as NodeEnrichmentService
participant IconResolver as getIconRequest
participant NodeIconResolver as NodeIconResolver
Comp->>VizHook: Call with entities & visibleFlows
VizHook->>Ctx: Read visibleFlows from context
VizHook->>Entity: For each visible entity: toVizNode()
Entity->>Entity: Create IVisualizationNode (async)
Entity->>Enricher: await enrichNodeFromCatalog(vizNode, kind)
Enricher->>IconResolver: getIconRequest(catalogKind, name)
IconResolver->>NodeIconResolver: getIcon(name, catalogKind)
NodeIconResolver-->>IconResolver: Returns icon URL
IconResolver-->>Enricher: Returns {icon, alt}
Enricher->>Entity: Set vizNode.data.iconUrl & iconAlt
Entity-->>VizHook: Resolved IVisualizationNode
VizHook-->>Comp: Returns IVisualizationNode[] (filtered & enriched)
Comp->>Comp: Render with vizNodes
sequenceDiagram
participant TileAdapter as camelComponentToTile
participant Promise as Promise.all()
participant IconReq as getIconRequest
participant CatalogRegistry as DynamicCatalogRegistry
TileAdapter->>TileAdapter: Collect conversion promises
TileAdapter->>IconReq: getIconRequest(CatalogKind, name)
IconReq->>CatalogRegistry: NodeIconResolver.getIcon()
CatalogRegistry-->>IconReq: icon URL
IconReq-->>TileAdapter: {icon, alt}
TileAdapter->>Promise: await Promise.all([...promises])
Promise-->>TileAdapter: All ITiles with iconUrl populated
TileAdapter-->>TileAdapter: Return combined tiles array
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
lordrip
reviewed
Apr 9, 2026
Member
lordrip
left a comment
lordrip
left a comment
There was a problem hiding this comment.
For the most part is good, the only pending topic is the getIconRequest function. I think is a good opportunity to create a file next to the IconResolver.tsx component, this will allow us to have proper unit tests and cover the cases.
Also, the API for this function feels a bit strange, it looks like we're returning a Promise and a string in the same object, I think we can make it more ergonomic by resolving the internal promises using async/await and return the object entirely, this way, the function will return a Promise, then we can put it in the use API to get both values at once.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/components/IconResolver/getIconRequest.ts (1)
26-34: Make test-kind alt labels explicit for readability.
substring('test'.length)can produce labels likeActionGroup/ValidationMatcherwithout spaces. Consider explicit label mapping so alt text stays human-readable.♻️ Proposed refactor
+const TEST_ALT_LABELS: Partial<Record<CatalogKind, string>> = { + [CatalogKind.TestAction]: 'Test Action icon', + [CatalogKind.TestActionGroup]: 'Test Action Group icon', + [CatalogKind.TestContainer]: 'Test Container icon', + [CatalogKind.TestEndpoint]: 'Test Endpoint icon', + [CatalogKind.TestFunction]: 'Test Function icon', + [CatalogKind.TestValidationMatcher]: 'Test Validation Matcher icon', +}; ... case CatalogKind.TestAction: case CatalogKind.TestActionGroup: case CatalogKind.TestContainer: case CatalogKind.TestEndpoint: case CatalogKind.TestFunction: case CatalogKind.TestValidationMatcher: iconName = name; - alt = altProps ?? `Test ${catalogKind.substring('test'.length)} icon`; + alt = altProps ?? TEST_ALT_LABELS[catalogKind] ?? 'Test icon'; break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/IconResolver/getIconRequest.ts` around lines 26 - 34, The alt text generation for test catalog kinds in getIconRequest.ts uses catalogKind.substring('test'.length), producing concatenated labels like "ActionGroup" which are not human-readable; update the switch handling for CatalogKind.TestAction, TestActionGroup, TestContainer, TestEndpoint, TestFunction, and TestValidationMatcher to set alt explicitly (or via a small mapping object keyed by catalogKind) when altProps is absent—e.g., map TestActionGroup -> "Test Action Group", TestValidationMatcher -> "Test Validation Matcher", etc.—and assign iconName = name as before but use the mapped human-friendly string for the alt variable instead of the substring expression.packages/ui/src/components/IconResolver/IconResolver.tsx (1)
19-19: Consider excludingaltPropsfrom promise memoization.Including
altPropsin the dependency array causes a new promise (and re-fetch) when only the alt text changes, even though the icon URL depends solely oncatalogKindandname. This can trigger unnecessary Suspense fallback flashes.♻️ Proposed optimization to separate alt text from fetch
- const iconPromise = useMemo(() => getIconRequest(catalogKind, name, altProps), [catalogKind, name, altProps]); - const { icon, alt } = use(iconPromise); + const iconPromise = useMemo(() => getIconRequest(catalogKind, name), [catalogKind, name]); + const { icon, alt: defaultAlt } = use(iconPromise); + const alt = altProps ?? defaultAlt;This would require updating
getIconRequestto makealtPropsoptional with a default, but would prevent re-fetching when only the alt text prop changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/IconResolver/IconResolver.tsx` at line 19, The memoized icon fetch is being restarted when only alt text changes because altProps is included in the dependency array; update IconResolver.tsx to call useMemo(() => getIconRequest(catalogKind, name, altProps?), [catalogKind, name]) by removing altProps from the dependency list and make getIconRequest accept altProps as optional (provide a sensible default inside getIconRequest or treat undefined as no-alt change) so the icon URL/fetch depends only on catalogKind and name and alt text updates won't re-trigger the promise or re-fetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ui/src/components/IconResolver/getIconRequest.ts`:
- Around line 26-34: The alt text generation for test catalog kinds in
getIconRequest.ts uses catalogKind.substring('test'.length), producing
concatenated labels like "ActionGroup" which are not human-readable; update the
switch handling for CatalogKind.TestAction, TestActionGroup, TestContainer,
TestEndpoint, TestFunction, and TestValidationMatcher to set alt explicitly (or
via a small mapping object keyed by catalogKind) when altProps is absent—e.g.,
map TestActionGroup -> "Test Action Group", TestValidationMatcher -> "Test
Validation Matcher", etc.—and assign iconName = name as before but use the
mapped human-friendly string for the alt variable instead of the substring
expression.
In `@packages/ui/src/components/IconResolver/IconResolver.tsx`:
- Line 19: The memoized icon fetch is being restarted when only alt text changes
because altProps is included in the dependency array; update IconResolver.tsx to
call useMemo(() => getIconRequest(catalogKind, name, altProps?), [catalogKind,
name]) by removing altProps from the dependency list and make getIconRequest
accept altProps as optional (provide a sensible default inside getIconRequest or
treat undefined as no-alt change) so the icon URL/fetch depends only on
catalogKind and name and alt text updates won't re-trigger the promise or
re-fetch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 042b16ad-cbc9-49ea-8879-9a40ac29c0dc
📒 Files selected for processing (10)
packages/ui/src/components/Catalog/DataListItem.tsxpackages/ui/src/components/Catalog/Tile.tsxpackages/ui/src/components/IconResolver/IconResolver.test.tsxpackages/ui/src/components/IconResolver/IconResolver.tsxpackages/ui/src/components/IconResolver/getIconRequest.test.tspackages/ui/src/components/IconResolver/getIconRequest.tspackages/ui/src/components/PropertiesModal/PropertiesModal.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.tsxpackages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3100 +/- ##
==========================================
+ Coverage 91.88% 91.91% +0.03%
==========================================
Files 606 606
Lines 23350 23375 +25
Branches 5534 5534
==========================================
+ Hits 21454 21485 +31
- Misses 1786 1888 +102
+ Partials 110 2 -108 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lordrip
reviewed
Apr 16, 2026
Member
lordrip
left a comment
lordrip
left a comment
There was a problem hiding this comment.
Great job @shivamG640, I like that we could remove a good portion of code related to <Suspense>. Please find below a few topics that I think needs our attention.
I reckon that the blast radius of this change wasn't as big as I anticipated, and after implementing the changes in the related issue, any similar change would have an even smaller blast radius, enhancing the isolation layers in Kaoto 🙆♂️
Thanks Shivam 💪
Comment on lines
+273
to
+276
| // Enrich route group node with catalog properties | ||
| await NodeEnrichmentService.enrichNodeFromCatalog(routeGroupNode, CatalogKind.Entity); | ||
|
|
||
| const fromNode = await NodeMapperService.getVizNode( |
Member
There was a problem hiding this comment.
I think this is ok for now, in a separate PR, we should create a Route, Pipe, Kamelet, errorHandler, etc... mappers, so we don't need to enhance the vizNode here, it should just be a matter of calling the NodeMapperService.getVizNode() and that's it, so we have the mapping logic in a single place.
PVinaches
reviewed
Apr 20, 2026
shivamG640
force-pushed
the
fix_3095
branch
2 times, most recently
from
d4cb53e to
7cd7fec
Compare
shivamG640
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/ui/src/dynamic-catalog/catalog-tiles.provider.tsx (1)
61-98:⚠️ Potential issue | 🟠 MajorAvoid making one failed tile reject the whole catalog.
With
Promise.all(tilePromises), one failed async tile/icon conversion prevents every tile from being cached or returned. Since the effect call drops the rejected promise, this can also surface as an unhandled rejection.🛡️ Proposed fix: keep successful tiles and isolate failures
- const combinedTiles = await Promise.all(tilePromises); + const combinedTiles = ( + await Promise.all( + tilePromises.map((tilePromise) => + tilePromise.catch((error) => { + console.warn('Failed to build catalog tile', error); + return undefined; + }), + ), + ) + ).filter(isDefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/dynamic-catalog/catalog-tiles.provider.tsx` around lines 61 - 98, The current use of Promise.all(tilePromises) will reject the whole batch if any tile conversion (e.g., camelComponentToTile, camelProcessorToTile, camelEntityToTile, kameletToTile, citrusComponentToTile) fails; replace it with Promise.allSettled on the tilePromises, then collect only the fulfilled results (mapping settled entries to their .value) and optionally log errors from rejected entries so a single failed tile does not prevent caching or returning the rest of the catalog. Ensure the new result is assigned to combinedTiles and downstream code expects an array of successful ITile results only.packages/ui/src/models/visualization/flows/camel-rest-visual-entity.ts (1)
146-157:⚠️ Potential issue | 🟡 MinorAdd missing
catalogKindassignment totoVizNode()for consistency with peer entity implementations.This method does not set
restGroupNode.data.catalogKind = CatalogKind.Entity, while all peer entities explicitly do:
CamelErrorHandlerVisualEntity(line 164)CamelOnExceptionVisualEntity(line 86)CamelInterceptSendToEndpointVisualEntity(line 113)CamelInterceptVisualEntity,CamelInterceptFromVisualEntity,CamelOnCompletionVisualEntity, and othersNeither
NodeMapperService.getVizNode()norNodeEnrichmentServicepopulatecatalogKind, so each entity must set it explicitly. AddrestGroupNode.data.catalogKind = CatalogKind.Entity;after line 152 to match the pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/camel-rest-visual-entity.ts` around lines 146 - 157, The toVizNode() implementation is missing setting the catalog kind on the node data; update the method so after obtaining restGroupNode (from NodeMapperService.getVizNode) you assign restGroupNode.data.catalogKind = CatalogKind.Entity (alongside the existing restGroupNode.data.entity/isGroup/name assignments) to match peer entities like CamelErrorHandlerVisualEntity and ensure catalogKind is populated for downstream consumers.packages/ui/src/models/visualization/flows/abstract-camel-visual-entity.ts (1)
260-320:⚠️ Potential issue | 🔴 CriticalAdd error handling to prevent unhandled promise rejections in enrichment.
toVizNode()awaitsNodeEnrichmentService.enrichNodeFromCatalog()without error handling, and the enrichment chain (getIconRequest→NodeIconResolver.getIcon→ potentiallyDynamicCatalogRegistry.get().getEntity()) has no fallbacks. If the async catalog lookup fails, the rejection propagates uncaught throughuseVisibleVizNodes()because the ErrorBoundary inVisualization.tsxonly catches render-time errors, not unhandled promise rejections from async operations.Wrap the enrichment in a try/catch block (lines 273–276) to gracefully degrade: on failure, the routeGroupNode renders with the fallback empty
iconUrl/title/descriptionrather than silently failing to update the canvas state.try { await NodeEnrichmentService.enrichNodeFromCatalog(routeGroupNode, CatalogKind.Entity); } catch (error) { // Log and continue with fallback empty values console.warn('Failed to enrich route node from catalog:', error); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/abstract-camel-visual-entity.ts` around lines 260 - 320, The call to NodeEnrichmentService.enrichNodeFromCatalog(...) inside toVizNode() can reject and cause unhandled promise rejections; wrap that await in a try/catch around the enrichNodeFromCatalog(routeGroupNode, CatalogKind.Entity) call, log the error (e.g., console.warn or processLogger) and let execution continue so routeGroupNode keeps its fallback empty iconUrl/title/description values instead of failing the whole async chain.packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.tsx (1)
37-97:⚠️ Potential issue | 🟡 MinorRemove the now-unused
visibleFlowscontext read and stale effect dependency.With the prop flip to
vizNodes, filtering by visibility has moved upstream (intouseVisibleVizNodes/ the callers). In this componentvisibleFlowsis no longer referenced inside the effect body, yet:
- Line 37 still reads it from context (and still asserts the context is non-null via
!, couplingHiddenCanvastoVisibleFlowsContextfor no functional reason).- Line 97 still lists it as a dep, so the effect will re-run (and re-layout) whenever
visibleFlowschanges even ifvizNodesis unchanged — wasteful work on every flow show/hide toggle during an export.🔧 Proposed fix
-import { FunctionComponent, useContext, useEffect, useRef, useState } from 'react'; +import { FunctionComponent, useEffect, useRef, useState } from 'react'; import { IVisualizationNode } from '../../../../models/visualization/base-visual-entity'; -import { VisibleFlowsContext } from '../../../../providers/visible-flows.provider'; import { CanvasEdge, CanvasNode, LayoutType } from '../../Canvas/canvas.models'; @@ const containerRef = useRef<HTMLDivElement>(null); const [layoutComplete, setLayoutComplete] = useState(false); - const { visibleFlows } = useContext(VisibleFlowsContext)!; const hasExportedRef = useRef(false); @@ - }, [controller, layout, visibleFlows, vizNodes]); + }, [controller, layout, vizNodes]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.tsx` around lines 37 - 97, HiddenCanvas still reads visibleFlows from VisibleFlowsContext and lists visibleFlows in the effect deps even though vizNodes now contains the already-filtered nodes; remove the unnecessary context read (the line that destructures visibleFlows from useContext(VisibleFlowsContext)) and delete visibleFlows from the dependency array of the effect that builds the model (the useEffect that references controller, layout, vizNodes), ensuring HiddenCanvas only depends on vizNodes, layout, and controller; leave callers using useVisibleVizNodes unchanged.packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsx (1)
3-15:⚠️ Potential issue | 🟡 MinorUpdate the mock node to exercise resolved icon metadata.
The fixture still bypasses the new node data contract with
unknown as IVisualizationNode, so this snapshot can pass even ificonUrl/title rendering regresses. Seed the mock with the resolved metadata fields and drop the oldcatalogKinddependency if it is no longer used here.Suggested fixture update
-import { CatalogKind, IVisualizationNode } from '../../../../models'; +import { IVisualizationNode } from '../../../../models'; @@ id: 'test-node', data: { - catalogKind: CatalogKind.Component, name: 'log', path: 'route.from.steps.0.log', + isPlaceholder: false, + isGroup: false, + iconUrl: '/src/assets/components/log.svg', + title: 'Log', + description: 'Log endpoint', },Also applies to: 25-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsx` around lines 3 - 15, The test fixture createMockVizNode currently casts an incomplete object to IVisualizationNode and only sets catalogKind/name/path, which bypasses the resolved icon metadata checks; update createMockVizNode to return a fully shaped IVisualizationNode (no "unknown as IVisualizationNode" cast) that includes the resolved metadata fields used by CustomNodeContainer — e.g., provide data.resolved?.iconUrl and data.resolved?.title (and any other resolved fields your component reads) and remove or stop relying on catalogKind if it's no longer used, so the snapshot and assertions exercise icon/title rendering in CustomNodeContainer.
♻️ Duplicate comments (2)
packages/ui/src/components/Visualization/ContextToolbar/Flows/FlowsList.tsx (1)
208-218:⚠️ Potential issue | 🟡 MinorRemove the unused
toVizNode()await before showing the delete modal.Line 211 now resolves a viz node but discards it; the title uses
flow.idandflow.getRootPath()directly. This can delay or block the delete confirmation on unrelated async node/icon resolution.♻️ Proposed cleanup
onClick={async (_event) => { //close the dropdown if it is open to not to interfere with the delete modal onClose?.(); - await flow.toVizNode(); const isDeleteConfirmed = await deleteModalContext?.actionConfirmation({ title: "Do you want to delete the '" +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/ContextToolbar/Flows/FlowsList.tsx` around lines 208 - 218, The click handler currently awaits flow.toVizNode() but discards the result, causing unnecessary async delay before showing the delete confirmation; remove the await flow.toVizNode() call (and make the onClick handler synchronous if nothing else needs to await) so the sequence becomes onClose?.(); then call deleteModalContext?.actionConfirmation({ title: "Do you want to delete the '" + flow.id + "' " + camelCaseToSpaces(flow.getRootPath(), { capitalize: true }) + '?' ... }); keep references to flow.id, flow.getRootPath(), deleteModalContext.actionConfirmation and onClose to locate and update the handler.packages/ui/src/models/visualization/flows/nodes/node-enrichment.service.ts (1)
1-1: 🛠️ Refactor suggestion | 🟠 MajorMove icon resolution out of
componentsbefore models depend on it.
node-enrichment.service.tslives undermodels, but imports fromcomponents/IconResolver. That inverts the layer dependency and can create model ⇄ component cycles. PutgetIconRequestin a neutral module, or inject the resolver into this service.Run this read-only check to confirm the dependency direction and any resulting cycle surface:
#!/bin/bash # Description: Inspect model-to-component imports and getIconRequest dependencies. set -euo pipefail echo "Model files importing from components:" rg -n -C2 "from ['\"][.\/]*.*components/" packages/ui/src/models --type ts --type tsx || true echo echo "getIconRequest imports:" sed -n '1,120p' packages/ui/src/components/IconResolver/getIconRequest.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/nodes/node-enrichment.service.ts` at line 1, The model layer (NodeEnrichmentService in node-enrichment.service.ts) imports getIconRequest from components/IconResolver which inverts layering; move icon resolution into a neutral module or make it injectable: either relocate getIconRequest into a shared utility (e.g., a new neutral module like ui-shared/iconResolver with a named export getIconRequest and update imports) or add a dependency to NodeEnrichmentService (e.g., accept an IconResolver/getIconRequest function via constructor or setter and use that instead of importing from components); update all imports to the new neutral module or injection point and run the provided read-only RG/SED checks to confirm no model -> components imports remain and no cycles are introduced.
🧹 Nitpick comments (16)
packages/ui/src/components/Visualization/Custom/Node/PlaceholderNode.test.tsx (1)
101-104: Centralize completeIVisualizationNodeDatatest defaults.Lines 101-104 update one fixture with the new required presentation fields, but
setupWithVizNodestill casts partial data toIVisualizationNodeData. Consider using one fixture factory so all placeholder-node tests exercise the same complete contract.♻️ Proposed refactor
+const createPlaceholderVizNodeData = ( + overrides: Partial<IVisualizationNodeData> = {}, +): IVisualizationNodeData => ({ + name: PlaceholderType.Placeholder, + path: 'test.placeholder', + isPlaceholder: true, + isGroup: false, + title: '', + description: '', + iconUrl: '', + ...overrides, +}); + describe('PlaceholderNode', () => { @@ const vizNode = createVisualizationNode('route.from.steps.1.placeholder', { - name: PlaceholderType.Placeholder, - path: 'route.from.steps.1.placeholder', - isPlaceholder: true, - isGroup: false, - title: '', - description: '', - iconUrl: '', - }) as IVisualizationNode; + ...createPlaceholderVizNodeData({ + path: 'route.from.steps.1.placeholder', + }), + }) as IVisualizationNode; @@ const vizNode = createVisualizationNode('test-placeholder', { - path: 'test.placeholder', - isPlaceholder: true, - ...vizNodeData, - } as IVisualizationNodeData); + ...createPlaceholderVizNodeData(vizNodeData), + });Also applies to: 143-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/Custom/Node/PlaceholderNode.test.tsx` around lines 101 - 104, The tests add new required presentation fields but some fixtures still cast partial objects to IVisualizationNodeData; create a single fixture factory (e.g., createTestVisualizationNodeData) that returns a complete IVisualizationNodeData object with all required fields (isGroup, title, description, iconUrl, and existing fields) and replace ad-hoc partial casts in tests and setupWithVizNode usages with calls to that factory so every PlaceholderNode test (including the cases around setupWithVizNode and the instances at lines referenced) uses the same full contract.packages/ui/src/models/visualization/flows/nodes/node-enrichment.service.ts (1)
5-15: Update the JSDoc to match the implementation.The service only enriches
iconUrlandiconAlt; it does not set title or description, andcomponentLookupis not a parameter.📝 Proposed documentation fix
/** * Service for enriching visualization nodes with catalog-derived properties. - * This includes resolving icons, titles, and descriptions from the catalog. + * This includes resolving icon presentation data from the catalog. */ export class NodeEnrichmentService { /** - * Enriches a visualization node with catalog properties (icon, title, description). + * Enriches a visualization node with catalog icon properties. * `@param` vizNode - The visualization node to enrich - * `@param` componentLookup - The component lookup result containing processor/component names + * `@param` catalogKind - The catalog kind used to resolve the node icon */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/nodes/node-enrichment.service.ts` around lines 5 - 15, The JSDoc for NodeEnrichmentService.enrichNodeFromCatalog is inaccurate: update the comment to reflect the actual signature and behavior — remove any mention of a componentLookup parameter and remove references to setting title/description; state that enrichNodeFromCatalog accepts (vizNode: IVisualizationNode, catalogKind: CatalogKind) and only resolves/enriches iconUrl and iconAlt on the vizNode using catalog data. Also update the method description and `@param` tags to match these exact symbols (NodeEnrichmentService, enrichNodeFromCatalog, vizNode, catalogKind).packages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.ts (1)
30-36: Consider parallelizing enrichment and child resolution.
enrichNodeFromCatalogandgetChildrenFromBranchare independent operations. The enrichment only modifiesvizNode.dataproperties (icon metadata), while child resolution only reads fromentityDefinitionand creates new nodes. Awaiting them sequentially adds unnecessary latency per node, which compounds across the tree. UsingPromise.allwould reduce render time for large routes.♻️ Suggested change
- const vizNode = createVisualizationNode(path, data); - await NodeEnrichmentService.enrichNodeFromCatalog(vizNode, CatalogKind.Processor); - - const children = await this.getChildrenFromBranch(`${path}.steps`, entityDefinition); + const vizNode = createVisualizationNode(path, data); + const [, children] = await Promise.all([ + NodeEnrichmentService.enrichNodeFromCatalog(vizNode, CatalogKind.Processor), + this.getChildrenFromBranch(`${path}.steps`, entityDefinition), + ]); children.forEach((child) => { vizNode.addChild(child); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.ts` around lines 30 - 36, enrichNodeFromCatalog and getChildrenFromBranch can run in parallel to reduce latency: after creating the node with createVisualizationNode(path, data) start both tasks concurrently (call NodeEnrichmentService.enrichNodeFromCatalog(vizNode, CatalogKind.Processor) and this.getChildrenFromBranch(`${path}.steps`, entityDefinition)) and await them together (e.g., Promise.all). Once both complete, iterate the returned children and call vizNode.addChild(child) as before; keep createVisualizationNode and vizNode references unchanged.packages/ui/src/models/visualization/base-visual-entity.ts (1)
173-181: Empty-stringiconUrlis a design fragility risk, though mitigated in current implementation.Making
iconUrla requiredstringand defaulting to''creates a theoretical risk:<img src="">can trigger page re-requests in some browsers. While the current code mitigates this through immediateenrichNodeFromCatalogcalls (which populate the icon synchronously) andgetIconRequestalways returning valid icons, the design is fragile—missing enrichment or test scenarios using fixtures directly could expose this.Consider either:
- Typing as
iconUrl?: stringand guarding<img>rendering with a truthiness check, or- Using a data URI placeholder (e.g., transparent pixel) instead of
''.This would make the code more resilient to future changes and clearer in intent for maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/base-visual-entity.ts` around lines 173 - 181, The current required string field iconUrl on BaseVisualEntity is fragile because a default empty string may cause <img src=""> reloads; update the model and rendering to be resilient by either making iconUrl optional (change iconUrl: string to iconUrl?: string in BaseVisualEntity) and ensure callers/renderers check truthiness before rendering an <img>, or initialize iconUrl to a safe placeholder data URI (e.g., a transparent pixel) and keep enrichNodeFromCatalog and getIconRequest behavior intact; update any code that assumes a non-empty iconUrl (including places that render the image) to use the new optional type or to skip rendering when iconUrl is falsy.packages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.test.ts (1)
65-91: LGTM — asynctoVizNode()and expandedIVisualizationNodeDatafixtures look correct.The test matrix entries and the new
isPlaceholder/isGroup/iconUrl/title/descriptionfields align with the updated data contract, and theasync/awaitconversion at Line 104/108 matches the Promise-returning API.Note (pre-existing, not introduced by this PR): Lines 71 and 73 both declare
{ processorName: 'onCompletion', path: 'onCompletion' }, so theonCompletioncase runs twice and the snapshot name collides. Worth cleaning up in a follow-up (e.g. replacing Line 73 with the previously-missing'choice'/'when'case or removing the duplicate).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.test.ts` around lines 65 - 91, The test matrix in the it.each entries contains a duplicate { processorName: 'onCompletion', path: 'onCompletion' } which causes the onCompletion case to run twice and collide snapshot names; update the array used by the it.each (the entries passed into CamelInterceptFromVisualEntity.getNodeInteraction) to remove or replace the duplicate entry (for example replace the second duplicate with { processorName: 'choice', path: 'choice' } or another missing processor like 'when') so each processorName/path pair is unique and snapshots no longer collide.packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/FlowExportImage.tsx (1)
14-15: Defer visible-node resolution until export starts.Line 15 resolves export nodes on every toolbar render, even when
isExportingis false. With asynctoVizNode()work, this triggers unnecessary async operations before the user clicks Export. Gate the hook input to pass an empty array until export is needed:Suggested fix
-import { useContext, useState } from 'react'; +import { useContext, useMemo, useState } from 'react'; @@ - const vizNodes = useVisibleVizNodes(visualEntities, visibleFlows); + const exportVisualEntities = useMemo(() => (isExporting ? visualEntities : []), [isExporting, visualEntities]); + const vizNodes = useVisibleVizNodes(exportVisualEntities, visibleFlows);HiddenCanvas at line 39 handles empty
vizNodeswithout issue, making this optimization safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/FlowExportImage.tsx` around lines 14 - 15, The toolbar currently calls useVisibleVizNodes(visualEntities, visibleFlows) on every render, causing async toViz() work even when not exporting; change the hook input so it receives an empty array until export begins: in FlowExportImage.tsx gate the call to useVisibleVizNodes by passing [] (or visibleFlows when isExporting is true) so vizNodes remains empty when isExporting is false; keep the rest of the export logic (including HiddenCanvas consumption of vizNodes) unchanged.packages/ui/src/components/Visualization/Custom/hooks/add-step.hook.test.tsx (1)
67-246: Repeated fixture literal — consider extracting a helper.The same 6-field
createVisualizationNode('test', { name: EntityType.Route, isPlaceholder: false, isGroup: false, iconUrl: '', title: '', description: '' })literal is now duplicated across 8 test cases. A smallmakeTestVizNode()helper in this file (or a shared test util) would reduce churn the next timeIVisualizationNodeDatagains a required field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/Custom/hooks/add-step.hook.test.tsx` around lines 67 - 246, Extract a small helper function (e.g., makeTestVizNode) that returns the repeated createVisualizationNode('test', { name: EntityType.Route, isPlaceholder: false, isGroup: false, iconUrl: '', title: '', description: '' }) fixture and replace all duplicated calls in the tests with calls to makeTestVizNode(); update all places using createVisualizationNode in this file (tests that call useAddStep and reference vizNode) so future additions to IVisualizationNodeData only require editing the helper.packages/ui/src/components/Visualization/Custom/hooks/replace-step.hook.test.tsx (1)
84-305: Fixture updates LGTM; same extraction opportunity asadd-step.hook.test.tsx.Same repeated literal appears across many test bodies here; consider a shared
makeTestVizNode()helper to keep futureIVisualizationNodeDatashape changes to a single site.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/Custom/hooks/replace-step.hook.test.tsx` around lines 84 - 305, Tests duplicate creation of visualization nodes (createVisualizationNode(...) and manual child nodes) across many it blocks; extract a shared factory to reduce repetition. Add a helper function makeTestVizNode (and optionally makeChildNode) used by the tests in this file to encapsulate the IVisualizationNodeData shape and common mocks (e.g., isPlaceholder, isGroup, EntityType, empty strings for icon/title/description), replace inline createVisualizationNode calls and manual child creation with calls to these helpers, and update mocks (mockVizNode.getChildren, placeholderChild, nonPlaceholderChild) to use the new helpers so future shape changes require updating only the factory.packages/ui/src/camel-utils/camel-to-tile.adapter.test.ts (1)
17-203: Stale test descriptions: "correct type" tests no longer assert ontile.type.Several tests titled
'should return a tile with the correct type'(lines 17, 98, 157, 175) no longer checktile.type— they only assert onname/title/description. Only the kamelet case (line 252) still validatestile.type. Consider renaming the remaining cases to reflect what they actually verify (e.g.,'should map name and description') so a future reader doesn't add a missingtile.typeassertion assuming the test already covers it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/camel-utils/camel-to-tile.adapter.test.ts` around lines 17 - 203, Several "should return a tile with the correct type" tests no longer assert tile.type; update their test titles to reflect the actual assertions and avoid misleading readers. Rename the camelComponentToTile test (the one that asserts tile.name and tile.description) to something like "should map name and description"; rename the camelProcessorToTile test (that asserts tile.name and tile.description) to "should map name and description"; rename the camelEntityToTile test (that asserts tile.name and tile.description) similarly; and rename the citrusComponentToTile test(s) to "should map name, title and description" and "should return defaults" respectively so the it() descriptions match the assertions in the tests that call camelComponentToTile, camelProcessorToTile, camelEntityToTile, and citrusComponentToTile.packages/ui/src/models/visualization/flows/nodes/node-mapper.service.ts (1)
23-29: Unnecessaryasyncon a pass-through wrapper.
getVizNodeonly forwards togetVizNodeFromProcessor(which is itself already async). Marking this methodasyncadds an extra microtask without anyawait. Droppingasync(the return type remainsPromise<IVisualizationNode>) keeps the contract identical and slightly leaner.♻️ Proposed refactor
- static async getVizNode( + static getVizNode( path: string, componentLookup: ICamelElementLookupResult, entityDefinition: unknown, ): Promise<IVisualizationNode> { return this.getInstance().getVizNodeFromProcessor(path, componentLookup, entityDefinition); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/nodes/node-mapper.service.ts` around lines 23 - 29, The getVizNode wrapper is marked async unnecessarily; remove the async keyword from the static method declaration for getVizNode so it directly returns the Promise from getVizNodeFromProcessor without creating an extra microtask. Keep the signature as returning Promise<IVisualizationNode> and leave the implementation as a single return of this.getInstance().getVizNodeFromProcessor(path, componentLookup, entityDefinition); no other changes needed.packages/ui/src/components/Visualization/hooks/use-visible-viz-nodes.ts (2)
16-27: Consider surfacing errors fromtoVizNode()rather than swallowing them viavoid resolve().If any
entity.toVizNode()rejects (e.g., catalog lookup failure during icon enrichment), the rejection becomes an unhandled promise rejection andvizNodessilently stays empty/stale. A minimal.catchthat logs (and optionally leaves priorvizNodesin place) will make export/preview failures diagnosable. This also aligns with the earlier reviewer feedback about makingawait entity.toVizNode()easier to debug.- void resolve(); + resolve().catch((err) => { + if (!cancelled) console.error('Failed to resolve visible viz nodes', err); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/hooks/use-visible-viz-nodes.ts` around lines 16 - 27, The resolve function currently awaits entity.toVizNode() inside a loop without handling rejections, causing unhandled promise rejections and silent failures; update the loop in resolve to wrap each await entity.toVizNode() in a try/catch (or catch the whole per-entity promise) so errors are logged (use existing logger or console.error) and the function continues to the next entity, and ensure setVizNodes(nodes) is only called when not cancelled so prior vizNodes remain if errors occur; reference the resolve function, entity.toVizNode(), setVizNodes, and cancelled when making the change.
13-33: Resolve visible entities in parallel instead of sequentially.The
for…of+awaitserializesentity.toVizNode()for every visible flow. Since each resolution is independent, this multiplies latency with the number of visible flows — noticeable when enrichment hits the catalog/icon pipeline for multiple routes. PreferPromise.allfor parallel resolution while keeping the same cancellation semantics.♻️ Proposed refactor
useEffect(() => { let cancelled = false; const resolve = async () => { - const nodes: IVisualizationNode[] = []; - for (const entity of entities) { - if (visibleFlows[entity.id]) { - const vizNode = await entity.toVizNode(); - nodes.push(vizNode); - } - } + const nodes = await Promise.all( + entities.filter((entity) => visibleFlows[entity.id]).map((entity) => entity.toVizNode()), + ); if (!cancelled) { setVizNodes(nodes); } }; void resolve(); return () => { cancelled = true; }; }, [entities, visibleFlows]);Also worth noting: if upstream callers produce a new
visibleFlowsobject reference on each render (common when derived from reducers/selectors), this effect will re-run and re-await on every render. Consider memoizingvisibleFlowsat the call site or passing a stable key if you observe thrash.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/hooks/use-visible-viz-nodes.ts` around lines 13 - 33, The effect currently resolves entity.toVizNode() sequentially (for…of + await) causing latency; change the resolve function in use-visible-viz-nodes.ts to build an array of promises for visible entities (filter entities by visibleFlows[entity.id] then map to entity.toVizNode()), await Promise.all on that array, and then call setVizNodes with the resolved nodes only if the cancelled flag is false; keep the existing cancelled sentinel and useEffect dependencies (entities, visibleFlows) unchanged so cancellation semantics remain the same.packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.test.tsx (1)
85-405: Optional: hoist common setup intobeforeEach.The
jest.spyOn(console, 'error').mockImplementation(() => {})line andconst { Provider } = TestProvidersWrapper(); const vizNode = await entity.toVizNode();trio are repeated in nearly every test. Moving them intobeforeEach(and storingvizNode/Provideron closure variables) would shrink each test to its actual assertions without changing behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.test.tsx` around lines 85 - 405, Tests repeat setup lines (jest.spyOn(console, 'error').mockImplementation(() => {}), const { Provider } = TestProvidersWrapper(); and const vizNode = await entity.toVizNode()) — hoist them into a shared beforeEach: create closure-scoped variables (e.g., let Provider, vizNode) and in beforeEach call Provider = TestProvidersWrapper().Provider; jest.spyOn(console, 'error').mockImplementation(() => {}); vizNode = await entity.toVizNode(); and reset/restore any spies/mocks (console.error mockRestore in afterEach if needed); then remove the duplicated lines from each it(...) so tests only contain their assertions. Ensure references to TestProvidersWrapper, entity.toVizNode, jest.spyOn(console, 'error'), Provider, and vizNode are updated accordingly.packages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.ts (1)
85-177: Optional: parallelize independent child node construction.
getChildrenFromBranchandgetChildrenFromArrayClauseawait each child’sgetVizNodeFromProcessorserially, so catalog enrichment per child is effectively sequential. Since each child resolution is independent, they can be produced withPromise.all(...)and linked afterwards in a deterministic loop. For deep or wide routes this can noticeably cut initial graph resolution latency. Same rationale applies togetVizNodesFromChildrenwhen multiplechildrenStepsPropertiesexist ingetVizNodeFromProcessor(Lines 52-58).♻️ Suggested parallelization (branch)
- const branchVizNodes: IVisualizationNode[] = []; - for (let index = 0; index < stepsList.length; index++) { - const step = stepsList[index]; - const singlePropertyName = Object.keys(step)[0]; - const childPath = `${path}.${index}.${singlePropertyName}`; - const childComponentLookup = CamelComponentSchemaService.getCamelComponentLookup( - childPath, - getValue(step, singlePropertyName), - ); - - const vizNode = await this.rootNodeMapper.getVizNodeFromProcessor( - childPath, - childComponentLookup, - entityDefinition, - ); - - const previousVizNode = branchVizNodes[branchVizNodes.length - 1]; - if (previousVizNode !== undefined) { - previousVizNode.setNextNode(vizNode); - vizNode.setPreviousNode(previousVizNode); - } - - branchVizNodes.push(vizNode); - } + const branchVizNodes: IVisualizationNode[] = await Promise.all( + stepsList.map((step, index) => { + const singlePropertyName = Object.keys(step)[0]; + const childPath = `${path}.${index}.${singlePropertyName}`; + const childComponentLookup = CamelComponentSchemaService.getCamelComponentLookup( + childPath, + getValue(step, singlePropertyName), + ); + return this.rootNodeMapper.getVizNodeFromProcessor(childPath, childComponentLookup, entityDefinition); + }), + ); + for (let i = 1; i < branchVizNodes.length; i++) { + branchVizNodes[i - 1].setNextNode(branchVizNodes[i]); + branchVizNodes[i].setPreviousNode(branchVizNodes[i - 1]); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.ts` around lines 85 - 177, The child node creation in getChildrenFromBranch and getChildrenFromArrayClause should be parallelized: replace the sequential awaits of this.rootNodeMapper.getVizNodeFromProcessor(...) with a Promise.all over a mapped array that constructs all viz node promises (preserving index order), then iterate deterministically to link nodes using setNextNode/setPreviousNode and insert the placeholder as before; apply the same Promise.all pattern to the multi-property aggregation in getVizNodesFromChildren/getVizNodeFromProcessor so each independent child resolution runs concurrently and linking happens afterwards.packages/ui/src/models/visualization/flows/pipe-visual-entity.ts (1)
236-339: Consider parallelizing independent catalog/icon resolutions.
toVizNodenow awaits three independent node constructions sequentially (source → steps → sink, Lines 250-252), andgetVizNodesFromStepsresolves each step’s catalog enrichment one after another. SinceNodeEnrichmentService.enrichNodeFromCatalogis the main async cost and each step’s enrichment is independent, the end-to-end latency scales linearly with the number of steps. You can still perform the prev/next linking deterministically after all nodes resolve.♻️ Suggested parallelization
- await NodeEnrichmentService.enrichNodeFromCatalog(pipeGroupNode, CatalogKind.Entity); - - const sourceNode = await this.getVizNodeFromStep(this.pipe.spec!.source, 'source', true); - const stepNodes = await this.getVizNodesFromSteps(this.pipe.spec!.steps); - const sinkNode = await this.getVizNodeFromStep(this.pipe.spec!.sink, 'sink'); + const [, sourceNode, stepNodes, sinkNode] = await Promise.all([ + NodeEnrichmentService.enrichNodeFromCatalog(pipeGroupNode, CatalogKind.Entity), + this.getVizNodeFromStep(this.pipe.spec!.source, 'source', true), + this.getVizNodesFromSteps(this.pipe.spec!.steps), + this.getVizNodeFromStep(this.pipe.spec!.sink, 'sink'), + ]);private async getVizNodesFromSteps(steps: PipeStep[] = []): Promise<IVisualizationNode[]> { - const vizNodes: IVisualizationNode[] = []; - for (let index = 0; index < steps.length; index++) { - const kamelet = steps[index]; - const path = `steps.${index}`; - const vizNode = await this.getVizNodeFromStep(kamelet, path); - - const previousVizNode = vizNodes[vizNodes.length - 1]; - if (previousVizNode !== undefined) { - previousVizNode.setNextNode(vizNode); - vizNode.setPreviousNode(previousVizNode); - } - - vizNodes.push(vizNode); - } - return vizNodes; + const vizNodes = await Promise.all( + steps.map((kamelet, index) => this.getVizNodeFromStep(kamelet, `steps.${index}`)), + ); + for (let i = 1; i < vizNodes.length; i++) { + vizNodes[i - 1].setNextNode(vizNodes[i]); + vizNodes[i].setPreviousNode(vizNodes[i - 1]); + } + return vizNodes; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/pipe-visual-entity.ts` around lines 236 - 339, The code is awaiting independent enrichments sequentially which slows down node construction; modify to run enrichments in parallel by changing toVizNode to kick off getVizNodeFromStep for source, sink and getVizNodesFromSteps concurrently (use Promise.all on [sourcePromise, stepsPromise, sinkPromise]) and then attach children and link nodes after all resolve; similarly, refactor getVizNodesFromSteps to map steps to an array of promises calling getVizNodeFromStep (e.g., steps.map((s,i)=>this.getVizNodeFromStep(s, `steps.${i}`))) and await Promise.all to get vizNodes, then perform the deterministic previous/next linking on the resolved array; keep usage of NodeEnrichmentService.enrichNodeFromCatalog inside getVizNodeFromStep unchanged.packages/ui/src/models/visualization/flows/citrus-test-visual-entity.ts (1)
468-600: Optional: parallelize step/child node creation for consistency.Same observation as for
PipeVisualEntity/BaseNodeMapper:getVizNodesFromSteps(Lines 475-487) andgetChildrenFromArrayClause(Lines 574-579) await per-action catalog enrichment serially. Resolving the nodes withPromise.all(...)and then wiring prev/next links afterwards keeps ordering deterministic while reducing latency proportional to the number of actions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/citrus-test-visual-entity.ts` around lines 468 - 600, getVizNodesFromSteps and getChildrenFromArrayClause create viz nodes awaiting each getVizNodeFromStep serially which wastes time; change both to map actions to promises, call Promise.all(...) to resolve all vizNodes concurrently, then iterate the resulting array to wire previous/next links and finally append the placeholder node (keeping same placeholder creation logic and path calculation). Refer to getVizNodesFromSteps, getChildrenFromArrayClause, getVizNodeFromStep, and the placeholder wiring (setNextNode/setPreviousNode) when making the change to preserve ordering and linkage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71ba209c-d2ea-499f-bf0b-c11cd4b87ebe
⛔ Files ignored due to path filters (14)
packages/ui/src/components/Catalog/__snapshots__/BaseCatalog.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Catalog/__snapshots__/DataListItem.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Catalog/__snapshots__/Tile.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/Form/__snapshots__/CanvasForm.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/Form/__snapshots__/CanvasFormHeader.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/Canvas.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/CanvasSideBar.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/flow.service.test.ts.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Custom/Node/__snapshots__/CustomNodeContainer.test.tsx.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-from-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-send-to-endpoint-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-on-completion-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/tests/__snapshots__/nodes-edges.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (137)
packages/ui/src/camel-utils/camel-to-tile.adapter.test.tspackages/ui/src/camel-utils/camel-to-tile.adapter.tspackages/ui/src/components/Catalog/Catalog.models.tspackages/ui/src/components/Catalog/DataListItem.test.tsxpackages/ui/src/components/Catalog/DataListItem.tsxpackages/ui/src/components/Catalog/Tile.test.tsxpackages/ui/src/components/Catalog/Tile.tsxpackages/ui/src/components/Catalog/filter-tiles.test.tspackages/ui/src/components/Catalog/sort-tags.test.tspackages/ui/src/components/DataMapper/on-delete-datamapper.test.tspackages/ui/src/components/GroupAutoStartupSwitch/GroupAutoStartupSwitch.test.tsxpackages/ui/src/components/IconResolver/IconResolver.test.tsxpackages/ui/src/components/IconResolver/IconResolver.tsxpackages/ui/src/components/IconResolver/getIconRequest.test.tspackages/ui/src/components/IconResolver/getIconRequest.tspackages/ui/src/components/IconResolver/index.tspackages/ui/src/components/IconResolver/node-icon-resolver.tspackages/ui/src/components/PropertiesModal/PropertiesModal.test.tsxpackages/ui/src/components/PropertiesModal/PropertiesModal.tsxpackages/ui/src/components/RenderingAnchor/RenderingAnchor.test.tsxpackages/ui/src/components/RenderingAnchor/rendering.provider.test.tsxpackages/ui/src/components/Visualization/Canvas/Canvas.test.tsxpackages/ui/src/components/Visualization/Canvas/Canvas.tsxpackages/ui/src/components/Visualization/Canvas/CanvasSideBar.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasForm.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasForm.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormBody.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.tsxpackages/ui/src/components/Visualization/Canvas/flow.service.test.tspackages/ui/src/components/Visualization/ContextToolbar/ExportDocument/ExportDocumentPreviewModal.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/FlowExportImage.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.test.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.tsxpackages/ui/src/components/Visualization/ContextToolbar/Flows/FlowsList.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemCopyStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteGroup.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDuplicateStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemEnableAllSteps.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemHideOtherFlows.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemMoveStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemPasteStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/NodeContextMenu.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/get-move-icons.util.test.tspackages/ui/src/components/Visualization/Custom/ContextMenu/item-interaction-helper.test.tspackages/ui/src/components/Visualization/Custom/Edge/CustomEdge.test.tsxpackages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.test.tsxpackages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNode.test.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.tsxpackages/ui/src/components/Visualization/Custom/Node/PlaceholderNode.test.tsxpackages/ui/src/components/Visualization/Custom/customComponentUtils.test.tspackages/ui/src/components/Visualization/Custom/hooks/add-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/delete-group.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/delete-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/disable-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/duplicate-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/enable-all-steps.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/insert-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/move-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/paste-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/replace-step.hook.test.tsxpackages/ui/src/components/Visualization/Visualization.tsxpackages/ui/src/components/Visualization/hooks/use-visible-viz-nodes.tspackages/ui/src/dynamic-catalog/catalog-modal.provider.test.tsxpackages/ui/src/dynamic-catalog/catalog-tiles.provider.tsxpackages/ui/src/models/camel/camel-route-resource.test.tspackages/ui/src/models/camel/kamelet-resource.test.tspackages/ui/src/models/citrus/citrus-test-resource.test.tspackages/ui/src/models/visualization/base-visual-entity.tspackages/ui/src/models/visualization/flows/abstract-camel-visual-entity.test.tspackages/ui/src/models/visualization/flows/abstract-camel-visual-entity.tspackages/ui/src/models/visualization/flows/camel-error-handler-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-error-handler-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-visual-entity.tspackages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.tspackages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.tspackages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.tspackages/ui/src/models/visualization/flows/camel-rest-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-rest-visual-entity.tspackages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.tspackages/ui/src/models/visualization/flows/camel-route-visual-entity.test.tspackages/ui/src/models/visualization/flows/citrus-test-visual-entity.test.tspackages/ui/src/models/visualization/flows/citrus-test-visual-entity.tspackages/ui/src/models/visualization/flows/kamelet-visual-entity.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/choice-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/choice-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/parallel-processor-base-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/parallel-processor-base-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/route-configuration-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/route-configuration-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/step-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/step-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/testing/noop-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/node-enrichment.service.tspackages/ui/src/models/visualization/flows/nodes/node-mapper.service.tspackages/ui/src/models/visualization/flows/nodes/node-mapper.tspackages/ui/src/models/visualization/flows/nodes/root-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/root-node-mapper.tspackages/ui/src/models/visualization/flows/pipe-visual-entity.test.tspackages/ui/src/models/visualization/flows/pipe-visual-entity.tspackages/ui/src/models/visualization/flows/support/camel-component-filter.service.test.tspackages/ui/src/models/visualization/visualization-node.test.tspackages/ui/src/pages/Catalog/CatalogPage.test.tsxpackages/ui/src/services/datamapper-step.service.test.tspackages/ui/src/stubs/tiles.tspackages/ui/src/testing-api.tspackages/ui/src/tests/nodes-edges.test.tspackages/ui/src/utils/get-viznodes-from-graph.test.ts
💤 Files with no reviewable changes (8)
- packages/ui/src/components/IconResolver/index.ts
- packages/ui/src/testing-api.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.ts
- packages/ui/src/components/IconResolver/IconResolver.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.ts
- packages/ui/src/components/IconResolver/IconResolver.tsx
✅ Files skipped from review due to trivial changes (13)
- packages/ui/src/components/Catalog/Tile.tsx
- packages/ui/src/components/Catalog/DataListItem.test.tsx
- packages/ui/src/components/Catalog/filter-tiles.test.ts
- packages/ui/src/components/IconResolver/node-icon-resolver.ts
- packages/ui/src/models/camel/camel-route-resource.test.ts
- packages/ui/src/components/Visualization/Custom/ContextMenu/item-interaction-helper.test.ts
- packages/ui/src/components/Visualization/Custom/customComponentUtils.test.ts
- packages/ui/src/components/RenderingAnchor/RenderingAnchor.test.tsx
- packages/ui/src/pages/Catalog/CatalogPage.test.tsx
- packages/ui/src/components/Catalog/sort-tags.test.ts
- packages/ui/src/components/IconResolver/getIconRequest.test.ts
- packages/ui/src/dynamic-catalog/catalog-modal.provider.test.tsx
- packages/ui/src/components/Catalog/DataListItem.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/ui/src/components/PropertiesModal/PropertiesModal.tsx
- packages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.tsx
- packages/ui/src/components/IconResolver/getIconRequest.ts
- packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.tsx
igarashitm
previously approved these changes
Apr 20, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ui/src/components/Visualization/Custom/customComponentUtils.test.ts (1)
240-249:⚠️ Potential issue | 🟡 MinorMark the group fixture as a group.
getMockGroupVizNode()now populates the required data shape, but setsisGroup: false; this makes these drag-group tests less representative of production group nodes.Proposed fix
- data: { path, name, isPlaceholder: false, isGroup: false, iconUrl: '', title: '', description: '' }, + data: { path, name, isPlaceholder: false, isGroup: true, iconUrl: '', title: '', description: '' },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/Custom/customComponentUtils.test.ts` around lines 240 - 249, The test fixture getMockGroupVizNode returns a mock IVisualizationNode with data.isGroup set to false which makes group-related tests inaccurate; update getMockGroupVizNode (and its returned data shape) to set isGroup: true and ensure any other group-specific fields remain populated (e.g., keep path, name, isPlaceholder, iconUrl, title, description) so the mock represents a real group node for tests that exercise drag-group behavior.packages/ui/src/models/visualization/flows/abstract-camel-visual-entity.ts (1)
276-285:⚠️ Potential issue | 🟠 MajorGuard the missing root URI before resolving the component name.
Line 280 asserts
this.getRootUri()is present, but lines 285-288 explicitly handle the missing-root-URI placeholder case. IfgetRootUri()is undefined,getComponentNameFromUri(...)can fail before that fallback runs.Proposed fix
+ const rootUri = this.getRootUri(); const fromNode = await NodeMapperService.getVizNode( `${this.getRootPath()}.from`, { processorName: 'from' as keyof ProcessorDefinition, - componentName: CamelComponentSchemaService.getComponentNameFromUri(this.getRootUri()!), + componentName: rootUri ? CamelComponentSchemaService.getComponentNameFromUri(rootUri) : undefined, }, this.entityDef, ); - if (!this.getRootUri()) { + if (!rootUri) { fromNode.data.catalogKind = CatalogKind.Entity; fromNode.data.name = PlaceholderType.Placeholder; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/abstract-camel-visual-entity.ts` around lines 276 - 285, The call to CamelComponentSchemaService.getComponentNameFromUri(this.getRootUri()!) is executed before checking for a missing root URI, which can throw when getRootUri() is undefined; update the NodeMapperService.getVizNode call in the code that constructs fromNode (the block using this.getRootPath(), processorName: 'from', and entityDef) to compute componentName conditionally — if this.getRootUri() is present call CamelComponentSchemaService.getComponentNameFromUri(this.getRootUri()), otherwise pass a safe fallback (e.g., a placeholder or undefined) so the missing-root-URI branch remains the guard and no function is invoked on undefined.
♻️ Duplicate comments (1)
packages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.ts (1)
60-67:⚠️ Potential issue | 🟡 MinorRestore coverage for
interceptFrom.The matrix still skips the
interceptFromprocessor, so that interaction branch is not snapshotted.Proposed fix
{ processorName: 'onCompletion', path: 'onCompletion' }, { processorName: 'intercept', path: 'intercept' }, + { processorName: 'interceptFrom', path: 'interceptFrom' }, { processorName: 'interceptSendToEndpoint', path: 'interceptSendToEndpoint' },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.ts` around lines 60 - 67, The test matrix is missing the interceptFrom processor entry so the interceptFrom branch isn't covered; update the array in camel-intercept-visual-entity.test.ts (the processor list used by the matrix) to include { processorName: 'interceptFrom', path: 'interceptFrom' } alongside the other processor entries (e.g., 'intercept', 'interceptSendToEndpoint') so the test matrix will snapshot the interceptFrom interaction branch.
🧹 Nitpick comments (5)
packages/ui/src/camel-utils/camel-to-tile.adapter.test.ts (1)
16-313: Consider asserting the newiconUrlbehavior on each adapter.Async conversion LGTM, but the central new responsibility of these adapters — resolving
iconUrlviagetIconRequest(...)— isn't covered here. Adapter-level tests should at minimum mockgetIconRequest(orNodeIconResolver.getIcon) and assert thattile.iconUrlis populated and that the correctCatalogKindis passed (particularly forcamelEntityToTile, which intentionally overrides the processor icon with an entity icon, and forcitrusComponentToTile, whose call site currently hardcodesCatalogKind.TestAction— see comment oncamel-to-tile.adapter.tsline 116).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/camel-utils/camel-to-tile.adapter.test.ts` around lines 16 - 313, Add tests to assert icon resolution: mock getIconRequest (or NodeIconResolver.getIcon) in the tests for camelComponentToTile, camelProcessorToTile, camelEntityToTile, citrusComponentToTile and kameletToTile, verify that tile.iconUrl is populated from the mock, and assert the resolver was called with the expected CatalogKind (ensure camelEntityToTile uses the entity-kind override and verify citrusComponentToTile uses the intended CatalogKind rather than the hardcoded TestAction). Locate the calls by referencing the adapter functions camelComponentToTile, camelProcessorToTile, camelEntityToTile, citrusComponentToTile and kameletToTile and make the mock return a predictable URL to assert tile.iconUrl equals that value.packages/ui/src/icon-resolver/getIconRequest.test.ts (1)
7-58: Add coverage forProcessor/Patternand otherTest*kinds.The current suite exercises
Component,Kamelet,Entity,TestAction, and an unknown kind, but skips theProcessor/Patternbranch atgetIconRequest.ts:35-40and the otherTest*kinds (TestActionGroup,TestContainer,TestEndpoint,TestFunction,TestValidationMatcher). These are worth pinning because the alt text for test kinds is synthesized viacatalogKind.substring('test'.length), which is silently dependent on the enum's string value shape — a regression there wouldn't be caught today.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/icon-resolver/getIconRequest.test.ts` around lines 7 - 58, Test coverage is missing for the Processor/Pattern branch and for all other Test* catalog kinds; update the getIconRequest tests to call getIconRequest with CatalogKind.Processor and CatalogKind.Pattern (verifying the prefixed/pattern logic) and add tests for each Test* enum value (TestActionGroup, TestContainer, TestEndpoint, TestFunction, TestValidationMatcher) to assert NodeIconResolver.getIcon is called with the correct name and that the returned alt text matches the synthesized value produced by getIconRequest (which strips the "test" prefix from CatalogKind to form the alt text); reuse the existing jest mocks for NodeIconResolver.getIcon and NodeIconResolver.getDefaultCamelIcon and add assertions that NodeIconResolver.getDefaultCamelIcon is not called for these known kinds.packages/ui/src/icon-resolver/getIconRequest.ts (1)
26-40: Consider extracting alt-text labels to a map for better maintainability.Currently,
alt = altProps ?? \Test ${catalogKind.substring('test'.length)} icon`assumes allCatalogKind.Test*enum values start with exactly'test'followed by a PascalCase suffix. While this pattern holds today (e.g.,'testAction','testActionGroup'`), the approach is fragile—if an enum value changes naming in the future, the substring operation will silently produce malformed alt text without triggering a test failure.A more robust approach would extract these labels into a map, making the intent explicit and ensuring type safety:
♻️ Optional refactor
+const TEST_KIND_LABELS: Record<CatalogKind, string> = { + [CatalogKind.TestAction]: 'Test Action', + [CatalogKind.TestActionGroup]: 'Test Action Group', + [CatalogKind.TestContainer]: 'Test Container', + [CatalogKind.TestEndpoint]: 'Test Endpoint', + [CatalogKind.TestFunction]: 'Test Function', + [CatalogKind.TestValidationMatcher]: 'Test Validation Matcher', +}; + case CatalogKind.TestAction: case CatalogKind.TestActionGroup: case CatalogKind.TestContainer: case CatalogKind.TestEndpoint: case CatalogKind.TestFunction: case CatalogKind.TestValidationMatcher: iconName = name; - alt = altProps ?? `Test ${catalogKind.substring('test'.length)} icon`; + alt = altProps ?? `${TEST_KIND_LABELS[catalogKind]} icon`; break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/icon-resolver/getIconRequest.ts` around lines 26 - 40, Replace the fragile substring-based alt-text logic in getIconRequest by extracting explicit alt-labels into a map keyed by CatalogKind: create a TestAltLabelMap mapping entries like CatalogKind.TestAction, CatalogKind.TestActionGroup, CatalogKind.TestContainer, CatalogKind.TestEndpoint, CatalogKind.TestFunction, CatalogKind.TestValidationMatcher to their human-readable suffixes (e.g. "Action", "Action Group", "Container", "Endpoint", "Function", "Validation Matcher"); then set alt = altProps ?? `Test ${TestAltLabelMap[catalogKind]} icon` for those test kinds (and similarly introduce a small map or reuse for Processor/Pattern/Component if desired), ensuring you reference catalogKind, altProps, and CatalogKind and fall back to a sensible default if a key is missing.packages/ui/src/components/Visualization/Canvas/Canvas.test.tsx (1)
313-313: Nit: awkward test description wording."no visual viznode" reads oddly; consider "no viznode" or "no visualization node" for clarity.
✏️ Proposed wording
- it('should render empty state when there is no visual viznode', async () => { + it('should render empty state when there are no viznodes', async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/Canvas/Canvas.test.tsx` at line 313, The test description string in the test case it('should render empty state when there is no visual viznode', ...) is awkward; update the description to clearer wording such as "should render empty state when there is no viznode" or "should render empty state when there is no visualization node" so the intent is obvious—locate the test with the exact description string and replace it with one of the proposed clearer phrasings.packages/ui/src/models/visualization/flows/pipe-visual-entity.ts (1)
248-252: Consider parallelizing independent node construction.
sourceNode,stepNodes, andsinkNodeare built independently before any linkage is established. Running them sequentially multiplies the enrichment latency;Promise.allwould cut it roughly to the cost of the slowest branch without altering downstream wiring. Sequential execution insidegetVizNodesFromStepsstill needs to be preserved because of thesetPreviousNode/setNextNodechain.♻️ Proposed refactor
- await NodeEnrichmentService.enrichNodeFromCatalog(pipeGroupNode, CatalogKind.Entity); - - const sourceNode = await this.getVizNodeFromStep(this.pipe.spec!.source, 'source', true); - const stepNodes = await this.getVizNodesFromSteps(this.pipe.spec!.steps); - const sinkNode = await this.getVizNodeFromStep(this.pipe.spec!.sink, 'sink'); + const [, sourceNode, stepNodes, sinkNode] = await Promise.all([ + NodeEnrichmentService.enrichNodeFromCatalog(pipeGroupNode, CatalogKind.Entity), + this.getVizNodeFromStep(this.pipe.spec!.source, 'source', true), + this.getVizNodesFromSteps(this.pipe.spec!.steps), + this.getVizNodeFromStep(this.pipe.spec!.sink, 'sink'), + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/pipe-visual-entity.ts` around lines 248 - 252, sourceNode, stepNodes, and sinkNode are built independently and can be awaited in parallel to reduce latency; change the current sequential awaits to start the three async operations concurrently and await them with Promise.all — call NodeEnrichmentService.enrichNodeFromCatalog(pipeGroupNode, CatalogKind.Entity) as before, then kick off getVizNodeFromStep(this.pipe.spec!.source, 'source', true), getVizNodesFromSteps(this.pipe.spec!.steps) (which must keep its internal sequential setPreviousNode/setNextNode logic), and getVizNodeFromStep(this.pipe.spec!.sink, 'sink') without awaiting each, and finally await Promise.all to collect [sourceNode, stepNodes, sinkNode] before continuing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui-tests/stories/canvas/Canvas.stories.tsx`:
- Around line 88-123: Top-level await is used in story args (e.g.,
CamelRouteVisualization.args with await camelRouteEntity.toVizNode()) which
breaks Storybook/Vite builds; move async entity resolution into Storybook
loaders: change the Template (StoryFn) to accept loaded from the second param
and render <Canvas {...args} vizNodes={loaded.vizNodes}
entitiesCount={loaded.vizNodes.length} />, then for each story
(CamelRouteVisualization, PipeVisualization, KameletVisualization,
EmptyPipeVisualization, EmptyCamelRouteVisualization, IconsRouteVisualization)
remove the await from args and add a loaders array that returns { vizNodes:
[await <entity>.toVizNode()] } so viz nodes are resolved in loaders instead of
top-level awaits.
In `@packages/ui/src/camel-utils/camel-to-tile.adapter.ts`:
- Around line 110-117: The call in citrusComponentToTile hardcodes
CatalogKind.TestAction when requesting the icon; change the getIconRequest
invocation to pass the actual component kind (derived from
componentDefinition.kind) instead of CatalogKind.TestAction so the icon lookup
reflects the real kind used elsewhere (e.g., headerTags and tile.type) — update
the getIconRequest call in the citrusComponentToTile function to use the
appropriate CatalogKind based on the kind variable.
In
`@packages/ui/src/components/Visualization/ContextToolbar/ExportDocument/ExportDocumentPreviewModal.tsx`:
- Line 53: useVisibleVizNodes() currently initializes vizNodes as [] and
resolves asynchronously, but HiddenCanvas is mounted when isGeneratingImage
toggles true which can produce a stale/empty image; update the hook to expose a
readiness flag (e.g., isLoading or isReady) that is true when inputs change and
set to false only after the async toVizNode() loop completes, then change the
ExportDocumentPreviewModal to wait for that flag (render HiddenCanvas only when
isGeneratingImage && !isLoading/isReady) so HiddenCanvas only mounts after
vizNodes are fully populated; reference useVisibleVizNodes, HiddenCanvas,
isGeneratingImage, and vizNodes when making the changes.
In
`@packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.tsx`:
- Around line 14-21: HiddenCanvas still imports and uses VisibleFlowsContext
even though it now receives pre-filtered vizNodes via the HiddenCanvasProps;
remove the VisibleFlowsContext import and any
React.useContext(VisibleFlowsContext) usage in the HiddenCanvas component (and
any reference to that context within functions like the component body or
effects), and rely solely on the vizNodes prop and existing symbols such as
HiddenCanvas, HiddenCanvasProps, and any
CanvasEdge/CanvasNode/LayoutType/ControllerService/FlowService usages so the
component no longer depends on the provider.
In
`@packages/ui/src/components/Visualization/Custom/ContextMenu/get-move-icons.util.test.ts`:
- Around line 149-159: The test "should handle vizNode without processorName" no
longer exercises the !processorName branch because CamelRouteVisualEntityData in
the test sets processorName; update the test to omit processorName from the
constructed viz node (or use a base type like CamelRouteVisualEntityBase /
Omit<CamelRouteVisualEntityData,'processorName'>) so the value is actually
undefined, ensuring isSpecialChildNode's missing-processorName path is hit; also
update the import at the top to bring in the base/omitted type you use for the
test data.
In
`@packages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.tsx`:
- Line 226: The img alt fallback uses nullish coalescing so an empty
tooltipContent won't fall back and also assumes a non-typed iconAlt; update the
alt selection in CustomGroupExpanded to use a truthy check (e.g., prefer
tooltipContent if non-empty, otherwise fallback) and defensively handle iconAlt
from groupVizNode.data (check it's a non-empty string before using it) instead
of casting; also add iconAlt to the IVisualizationNodeData type (or at least
document/validate it where NodeEnrichmentService.enrichNodeFromCatalog sets it)
so TypeScript knows the field exists.
In
`@packages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.test.ts`:
- Around line 66-74: The test matrix for CamelInterceptFromVisualEntity contains
a duplicate case for 'onCompletion' and should instead include an
'interceptFrom' case; update the array used in the camel intercept-from test
(the list of objects with processorName/path entries) by replacing the second {
processorName: 'onCompletion', path: 'onCompletion' } entry with {
processorName: 'interceptFrom', path: 'interceptFrom' } so the test exercises
the interceptFrom interaction.
In
`@packages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.test.ts`:
- Line 101: Update the test title string in the unit test that currently reads
"should return the vizualization node" to the corrected spelling "should return
the visualization node" so Jest output is clear; locate the it(...) declaration
with that exact title in the test named
camel-on-completion-visual-entity.test.ts and replace the misspelled
"vizualization" with "visualization".
---
Outside diff comments:
In
`@packages/ui/src/components/Visualization/Custom/customComponentUtils.test.ts`:
- Around line 240-249: The test fixture getMockGroupVizNode returns a mock
IVisualizationNode with data.isGroup set to false which makes group-related
tests inaccurate; update getMockGroupVizNode (and its returned data shape) to
set isGroup: true and ensure any other group-specific fields remain populated
(e.g., keep path, name, isPlaceholder, iconUrl, title, description) so the mock
represents a real group node for tests that exercise drag-group behavior.
In `@packages/ui/src/models/visualization/flows/abstract-camel-visual-entity.ts`:
- Around line 276-285: The call to
CamelComponentSchemaService.getComponentNameFromUri(this.getRootUri()!) is
executed before checking for a missing root URI, which can throw when
getRootUri() is undefined; update the NodeMapperService.getVizNode call in the
code that constructs fromNode (the block using this.getRootPath(),
processorName: 'from', and entityDef) to compute componentName conditionally —
if this.getRootUri() is present call
CamelComponentSchemaService.getComponentNameFromUri(this.getRootUri()),
otherwise pass a safe fallback (e.g., a placeholder or undefined) so the
missing-root-URI branch remains the guard and no function is invoked on
undefined.
---
Duplicate comments:
In
`@packages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.ts`:
- Around line 60-67: The test matrix is missing the interceptFrom processor
entry so the interceptFrom branch isn't covered; update the array in
camel-intercept-visual-entity.test.ts (the processor list used by the matrix) to
include { processorName: 'interceptFrom', path: 'interceptFrom' } alongside the
other processor entries (e.g., 'intercept', 'interceptSendToEndpoint') so the
test matrix will snapshot the interceptFrom interaction branch.
---
Nitpick comments:
In `@packages/ui/src/camel-utils/camel-to-tile.adapter.test.ts`:
- Around line 16-313: Add tests to assert icon resolution: mock getIconRequest
(or NodeIconResolver.getIcon) in the tests for camelComponentToTile,
camelProcessorToTile, camelEntityToTile, citrusComponentToTile and
kameletToTile, verify that tile.iconUrl is populated from the mock, and assert
the resolver was called with the expected CatalogKind (ensure camelEntityToTile
uses the entity-kind override and verify citrusComponentToTile uses the intended
CatalogKind rather than the hardcoded TestAction). Locate the calls by
referencing the adapter functions camelComponentToTile, camelProcessorToTile,
camelEntityToTile, citrusComponentToTile and kameletToTile and make the mock
return a predictable URL to assert tile.iconUrl equals that value.
In `@packages/ui/src/components/Visualization/Canvas/Canvas.test.tsx`:
- Line 313: The test description string in the test case it('should render empty
state when there is no visual viznode', ...) is awkward; update the description
to clearer wording such as "should render empty state when there is no viznode"
or "should render empty state when there is no visualization node" so the intent
is obvious—locate the test with the exact description string and replace it with
one of the proposed clearer phrasings.
In `@packages/ui/src/icon-resolver/getIconRequest.test.ts`:
- Around line 7-58: Test coverage is missing for the Processor/Pattern branch
and for all other Test* catalog kinds; update the getIconRequest tests to call
getIconRequest with CatalogKind.Processor and CatalogKind.Pattern (verifying the
prefixed/pattern logic) and add tests for each Test* enum value
(TestActionGroup, TestContainer, TestEndpoint, TestFunction,
TestValidationMatcher) to assert NodeIconResolver.getIcon is called with the
correct name and that the returned alt text matches the synthesized value
produced by getIconRequest (which strips the "test" prefix from CatalogKind to
form the alt text); reuse the existing jest mocks for NodeIconResolver.getIcon
and NodeIconResolver.getDefaultCamelIcon and add assertions that
NodeIconResolver.getDefaultCamelIcon is not called for these known kinds.
In `@packages/ui/src/icon-resolver/getIconRequest.ts`:
- Around line 26-40: Replace the fragile substring-based alt-text logic in
getIconRequest by extracting explicit alt-labels into a map keyed by
CatalogKind: create a TestAltLabelMap mapping entries like
CatalogKind.TestAction, CatalogKind.TestActionGroup, CatalogKind.TestContainer,
CatalogKind.TestEndpoint, CatalogKind.TestFunction,
CatalogKind.TestValidationMatcher to their human-readable suffixes (e.g.
"Action", "Action Group", "Container", "Endpoint", "Function", "Validation
Matcher"); then set alt = altProps ?? `Test ${TestAltLabelMap[catalogKind]}
icon` for those test kinds (and similarly introduce a small map or reuse for
Processor/Pattern/Component if desired), ensuring you reference catalogKind,
altProps, and CatalogKind and fall back to a sensible default if a key is
missing.
In `@packages/ui/src/models/visualization/flows/pipe-visual-entity.ts`:
- Around line 248-252: sourceNode, stepNodes, and sinkNode are built
independently and can be awaited in parallel to reduce latency; change the
current sequential awaits to start the three async operations concurrently and
await them with Promise.all — call
NodeEnrichmentService.enrichNodeFromCatalog(pipeGroupNode, CatalogKind.Entity)
as before, then kick off getVizNodeFromStep(this.pipe.spec!.source, 'source',
true), getVizNodesFromSteps(this.pipe.spec!.steps) (which must keep its internal
sequential setPreviousNode/setNextNode logic), and
getVizNodeFromStep(this.pipe.spec!.sink, 'sink') without awaiting each, and
finally await Promise.all to collect [sourceNode, stepNodes, sinkNode] before
continuing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b15992e6-81ea-4e43-93a7-a73c34cead50
⛔ Files ignored due to path filters (14)
packages/ui/src/components/Catalog/__snapshots__/BaseCatalog.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Catalog/__snapshots__/DataListItem.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Catalog/__snapshots__/Tile.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/Form/__snapshots__/CanvasForm.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/Form/__snapshots__/CanvasFormHeader.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/Canvas.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/CanvasSideBar.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/flow.service.test.ts.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Custom/Node/__snapshots__/CustomNodeContainer.test.tsx.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-from-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-send-to-endpoint-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-on-completion-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/tests/__snapshots__/nodes-edges.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (140)
packages/ui-tests/stories/Icons/Icons.stories.scsspackages/ui-tests/stories/Icons/Icons.stories.tsxpackages/ui-tests/stories/canvas/Canvas.stories.tsxpackages/ui/src/camel-utils/camel-to-tile.adapter.test.tspackages/ui/src/camel-utils/camel-to-tile.adapter.tspackages/ui/src/components/Catalog/Catalog.models.tspackages/ui/src/components/Catalog/DataListItem.test.tsxpackages/ui/src/components/Catalog/DataListItem.tsxpackages/ui/src/components/Catalog/Tile.test.tsxpackages/ui/src/components/Catalog/Tile.tsxpackages/ui/src/components/Catalog/filter-tiles.test.tspackages/ui/src/components/Catalog/sort-tags.test.tspackages/ui/src/components/DataMapper/on-delete-datamapper.test.tspackages/ui/src/components/GroupAutoStartupSwitch/GroupAutoStartupSwitch.test.tsxpackages/ui/src/components/IconResolver/IconResolver.test.tsxpackages/ui/src/components/IconResolver/IconResolver.tsxpackages/ui/src/components/IconResolver/index.tspackages/ui/src/components/PropertiesModal/PropertiesModal.test.tsxpackages/ui/src/components/PropertiesModal/PropertiesModal.tsxpackages/ui/src/components/RenderingAnchor/RenderingAnchor.test.tsxpackages/ui/src/components/RenderingAnchor/rendering.provider.test.tsxpackages/ui/src/components/Visualization/Canvas/Canvas.test.tsxpackages/ui/src/components/Visualization/Canvas/Canvas.tsxpackages/ui/src/components/Visualization/Canvas/CanvasSideBar.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasForm.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasForm.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormBody.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.tsxpackages/ui/src/components/Visualization/Canvas/flow.service.test.tspackages/ui/src/components/Visualization/ContextToolbar/ExportDocument/ExportDocumentPreviewModal.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/FlowExportImage.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.test.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.tsxpackages/ui/src/components/Visualization/ContextToolbar/Flows/FlowsList.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemCopyStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteGroup.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDuplicateStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemEnableAllSteps.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemHideOtherFlows.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemMoveStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemPasteStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/NodeContextMenu.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/get-move-icons.util.test.tspackages/ui/src/components/Visualization/Custom/ContextMenu/item-interaction-helper.test.tspackages/ui/src/components/Visualization/Custom/Edge/CustomEdge.test.tsxpackages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.test.tsxpackages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNode.test.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.tsxpackages/ui/src/components/Visualization/Custom/Node/PlaceholderNode.test.tsxpackages/ui/src/components/Visualization/Custom/customComponentUtils.test.tspackages/ui/src/components/Visualization/Custom/hooks/add-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/delete-group.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/delete-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/disable-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/duplicate-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/enable-all-steps.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/insert-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/move-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/paste-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/replace-step.hook.test.tsxpackages/ui/src/components/Visualization/Visualization.tsxpackages/ui/src/components/Visualization/hooks/use-visible-viz-nodes.tspackages/ui/src/dynamic-catalog/catalog-modal.provider.test.tsxpackages/ui/src/dynamic-catalog/catalog-tiles.provider.tsxpackages/ui/src/icon-resolver/getIconRequest.test.tspackages/ui/src/icon-resolver/getIconRequest.tspackages/ui/src/icon-resolver/node-icon-resolver.tspackages/ui/src/models/camel/camel-route-resource.test.tspackages/ui/src/models/camel/kamelet-resource.test.tspackages/ui/src/models/citrus/citrus-test-resource.test.tspackages/ui/src/models/visualization/base-visual-entity.tspackages/ui/src/models/visualization/flows/abstract-camel-visual-entity.test.tspackages/ui/src/models/visualization/flows/abstract-camel-visual-entity.tspackages/ui/src/models/visualization/flows/camel-error-handler-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-error-handler-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-visual-entity.tspackages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.tspackages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.tspackages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.tspackages/ui/src/models/visualization/flows/camel-rest-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-rest-visual-entity.tspackages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.tspackages/ui/src/models/visualization/flows/camel-route-visual-entity.test.tspackages/ui/src/models/visualization/flows/citrus-test-visual-entity.test.tspackages/ui/src/models/visualization/flows/citrus-test-visual-entity.tspackages/ui/src/models/visualization/flows/kamelet-visual-entity.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/choice-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/choice-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/parallel-processor-base-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/parallel-processor-base-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/route-configuration-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/route-configuration-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/step-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/step-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/testing/noop-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/node-enrichment.service.tspackages/ui/src/models/visualization/flows/nodes/node-mapper.service.tspackages/ui/src/models/visualization/flows/nodes/node-mapper.tspackages/ui/src/models/visualization/flows/nodes/root-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/root-node-mapper.tspackages/ui/src/models/visualization/flows/pipe-visual-entity.test.tspackages/ui/src/models/visualization/flows/pipe-visual-entity.tspackages/ui/src/models/visualization/flows/support/camel-component-filter.service.test.tspackages/ui/src/models/visualization/visualization-node.test.tspackages/ui/src/pages/Catalog/CatalogPage.test.tsxpackages/ui/src/services/datamapper-step.service.test.tspackages/ui/src/stubs/tiles.tspackages/ui/src/testing-api.tspackages/ui/src/tests/nodes-edges.test.tspackages/ui/src/utils/get-viznodes-from-graph.test.ts
💤 Files with no reviewable changes (10)
- packages/ui-tests/stories/Icons/Icons.stories.scss
- packages/ui/src/testing-api.ts
- packages/ui/src/components/IconResolver/index.ts
- packages/ui/src/components/IconResolver/IconResolver.test.tsx
- packages/ui-tests/stories/Icons/Icons.stories.tsx
- packages/ui/src/components/IconResolver/IconResolver.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.test.ts
✅ Files skipped from review due to trivial changes (24)
- packages/ui/src/components/Catalog/Catalog.models.ts
- packages/ui/src/dynamic-catalog/catalog-modal.provider.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDuplicateStep.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/item-interaction-helper.test.ts
- packages/ui/src/tests/nodes-edges.test.ts
- packages/ui/src/components/Visualization/Custom/hooks/delete-group.hook.test.tsx
- packages/ui/src/components/Catalog/Tile.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.test.ts
- packages/ui/src/components/Catalog/DataListItem.tsx
- packages/ui/src/components/Catalog/DataListItem.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.test.ts
- packages/ui/src/components/Visualization/Custom/hooks/paste-step.hook.test.tsx
- packages/ui/src/components/PropertiesModal/PropertiesModal.test.tsx
- packages/ui/src/components/PropertiesModal/PropertiesModal.tsx
- packages/ui/src/models/camel/camel-route-resource.test.ts
- packages/ui/src/components/Catalog/filter-tiles.test.ts
- packages/ui/src/models/visualization/flows/kamelet-visual-entity.test.ts
- packages/ui/src/pages/Catalog/CatalogPage.test.tsx
- packages/ui/src/models/visualization/flows/camel-route-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.test.ts
- packages/ui/src/components/Visualization/Custom/hooks/add-step.hook.test.tsx
- packages/ui/src/services/datamapper-step.service.test.ts
- packages/ui/src/components/Catalog/sort-tags.test.ts
- packages/ui/src/stubs/tiles.ts
🚧 Files skipped from review as they are similar to previous changes (63)
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemEnableAllSteps.test.tsx
- packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.tsx
- packages/ui/src/components/RenderingAnchor/rendering.provider.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemPasteStep.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemHideOtherFlows.test.tsx
- packages/ui/src/components/Catalog/Tile.tsx
- packages/ui/src/components/RenderingAnchor/RenderingAnchor.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.test.ts
- packages/ui/src/components/Visualization/Canvas/CanvasSideBar.test.tsx
- packages/ui/src/components/Visualization/Custom/Node/CustomNode.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/choice-node-mapper.test.ts
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemMoveStep.test.tsx
- packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/FlowExportImage.tsx
- packages/ui/src/components/Visualization/Custom/hooks/move-step.hook.test.tsx
- packages/ui/src/components/Visualization/Custom/Node/PlaceholderNode.test.tsx
- packages/ui/src/components/Visualization/Custom/hooks/enable-all-steps.hook.test.tsx
- packages/ui/src/models/camel/kamelet-resource.test.ts
- packages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.test.tsx
- packages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.test.ts
- packages/ui/src/components/Visualization/Canvas/Form/CanvasFormBody.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.test.ts
- packages/ui/src/utils/get-viznodes-from-graph.test.ts
- packages/ui/src/components/Visualization/ContextToolbar/Flows/FlowsList.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemCopyStep.test.tsx
- packages/ui/src/components/GroupAutoStartupSwitch/GroupAutoStartupSwitch.test.tsx
- packages/ui/src/components/Visualization/Canvas/Form/CanvasForm.tsx
- packages/ui/src/models/visualization/flows/nodes/node-mapper.ts
- packages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.ts
- packages/ui/src/components/Visualization/Custom/hooks/replace-step.hook.test.tsx
- packages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.ts
- packages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.test.ts
- packages/ui/src/components/DataMapper/on-delete-datamapper.test.ts
- packages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.ts
- packages/ui/src/models/visualization/flows/camel-intercept-visual-entity.ts
- packages/ui/src/components/Visualization/hooks/use-visible-viz-nodes.ts
- packages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/testing/noop-node-mapper.ts
- packages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.tsx
- packages/ui/src/models/visualization/flows/camel-rest-visual-entity.ts
- packages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.ts
- packages/ui/src/models/visualization/flows/nodes/node-enrichment.service.ts
- packages/ui/src/models/visualization/flows/nodes/root-node-mapper.ts
- packages/ui/src/components/Visualization/Canvas/flow.service.test.ts
- packages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.ts
- packages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.ts
- packages/ui/src/models/visualization/visualization-node.test.ts
- packages/ui/src/components/Visualization/Visualization.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/parallel-processor-base-node-mapper.ts
- packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.ts
- packages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/camel-rest-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/citrus-test-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.ts
- packages/ui/src/models/visualization/flows/abstract-camel-visual-entity.test.ts
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteGroup.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.ts
- packages/ui/src/components/Visualization/Custom/hooks/delete-step.hook.test.tsx
- packages/ui/src/components/Visualization/Canvas/Form/CanvasForm.test.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui-tests/stories/canvas/CanvasFormBody.stories.tsx`:
- Around line 58-60: Update the CanvasFormBody story fixtures so they use
meaningful placeholder metadata instead of empty strings: set iconUrl to a
stable placeholder like 'https://example.com/icon.svg', set title to the
processor name (e.g., 'aggregate', 'marshal', etc.), and provide a short
description string for each fixture; modify the objects passed into the
CanvasFormBody stories (the fixtures referenced in CanvasFormBody.stories.tsx
and rendered by CanvasFormHeader) to use these values so the icon and header
text render correctly in all six stories.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d41c4777-e504-4b89-9d9d-9813650aeef6
📒 Files selected for processing (2)
packages/ui-tests/stories/canvas/Canvas.stories.tsxpackages/ui-tests/stories/canvas/CanvasFormBody.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui-tests/stories/canvas/Canvas.stories.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.ts (1)
59-68:⚠️ Potential issue | 🟡 MinorRestore
interceptFrominteraction coverage.The table still omits
interceptFrom, so this test does not exercise that interaction path.Proposed fix
{ processorName: 'onException', path: 'onException' }, { processorName: 'onCompletion', path: 'onCompletion' }, { processorName: 'intercept', path: 'intercept' }, + { processorName: 'interceptFrom', path: 'interceptFrom' }, { processorName: 'interceptSendToEndpoint', path: 'interceptSendToEndpoint' },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.ts` around lines 59 - 68, The parameterized test array in camel-intercept-visual-entity.test.ts is missing the 'interceptFrom' case so the interaction path isn't exercised; update the it.each(...) data to include an entry with processorName 'interceptFrom' and path 'interceptFrom' (matching the other entries) so the test executes the interceptFrom interaction path inside the test that asserts interactions for processorName/path.
🧹 Nitpick comments (8)
packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsx (1)
10-15: Stale mock shape:catalogKindreplaced byiconUrlin viz node data.Per the PR's data contract change,
VisualizationNodeDatanow carriesiconUrl/iconAltinstead ofcatalogKind. Although the cast viaas unknown as IVisualizationNodehides this, consider updating the mock to reflect the new shape so future readers aren't misled and so a potential tooltip-alt fallback path (vizNode.data.iconAlt) can be exercised where relevant.Proposed update
data: { - catalogKind: CatalogKind.Component, + iconUrl: 'data:image/svg+xml;base64,PHN2Zy8+', + iconAlt: 'log', name: 'log', path: 'route.from.steps.0.log', },And drop the now-unused
CatalogKindimport on line 3.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsx` around lines 10 - 15, The test mock for the visualization node uses the old `catalogKind` field; update the mocked node data in CustomNodeContainer.test.tsx (the object cast to IVisualizationNode) to include `iconUrl` (a test URL) and `iconAlt` (a descriptive alt string) instead of `catalogKind`, and adjust the mock so any tooltip/alt-fallback path that reads `vizNode.data.iconAlt` is exercised; also remove the now-unused `CatalogKind` import at the top of the file.packages/ui/src/camel-utils/camel-to-tile.adapter.ts (1)
69-78: Minor: redundant Processor icon fetch incamelEntityToTile.
camelProcessorToTilealready awaitsgetIconRequest(CatalogKind.Processor, ...)and setsiconUrl, and then lines 74–75 immediately overwrite it with the Entity icon. That's one wasted async round-trip per entity on every catalog build. Since the Processor-kind icon is never surfaced here, consider refactoring to avoid the dual fetch (e.g., extract the shared tile-shaping logic and request the Entity icon directly), rather than relying oncamelProcessorToTilefor the shape.♻️ Sketch
-export const camelEntityToTile = async (processorDef: ICamelProcessorDefinition): Promise<ITile> => { - const entityTile = await camelProcessorToTile(processorDef); - entityTile.type = CatalogKind.Entity; - entityTile.headerTags = ['Entity']; - - const { icon: iconUrl } = await getIconRequest(CatalogKind.Entity, entityTile.name); - entityTile.iconUrl = iconUrl; - - return entityTile; -}; +export const camelEntityToTile = async (processorDef: ICamelProcessorDefinition): Promise<ITile> => { + const { name, title, description, supportLevel, label, provider } = processorDef.model; + const headerTags: string[] = ['Entity']; + if (supportLevel) headerTags.push(supportLevel); + + const { icon: iconUrl } = await getIconRequest(CatalogKind.Entity, name); + + return { + type: CatalogKind.Entity, + name, + title, + description, + headerTags, + tags: label.split(','), + provider, + iconUrl, + }; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/camel-utils/camel-to-tile.adapter.ts` around lines 69 - 78, camelEntityToTile is doing a redundant icon fetch because camelProcessorToTile already calls getIconRequest for Processor and sets iconUrl, then camelEntityToTile overwrites it by calling getIconRequest for Entity; fix by changing the shared shaping so only one icon request happens: either update camelProcessorToTile to accept an options argument (e.g., skipIcon: boolean) and call camelProcessorToTile(processorDef, { skipIcon: true }) from camelEntityToTile, then call getIconRequest(CatalogKind.Entity, entityTile.name) once and set entityTile.iconUrl, or extract the common tile-shaping logic into a helper (e.g., buildCamelTileFromProcessor) that returns the tile without fetching an icon and have camelEntityToTile call getIconRequest for CatalogKind.Entity itself; ensure you update references to camelProcessorToTile, getIconRequest, and entityTile.iconUrl accordingly.packages/ui/src/components/Visualization/Custom/hooks/paste-step.hook.test.tsx (1)
71-78: Consider centralizing the repeated visualization-node metadata defaults.The updated fixtures look aligned with the new node shape, but the same five default fields are repeated in every
createVisualizationNode()call. A small test helper/default object would make future metadata-shape changes less error-prone.♻️ Possible cleanup
+const visualizationNodeMetadataDefaults = { + isPlaceholder: false, + isGroup: false, + iconUrl: '', + title: '', + description: '', +}; + ... const vizNode = createVisualizationNode('test', { name: 'test', - isPlaceholder: false, - isGroup: false, - iconUrl: '', - title: '', - description: '', + ...visualizationNodeMetadataDefaults, });Apply the same spread to the other test fixtures that use the same defaults.
Also applies to: 91-98, 178-186, 232-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/Custom/hooks/paste-step.hook.test.tsx` around lines 71 - 78, Centralize the repeated visualization-node metadata by creating a single default object (e.g., visualizationNodeDefaults) and use object spread when calling createVisualizationNode in tests (e.g., createVisualizationNode('test', { ...visualizationNodeDefaults, name: 'test', isPlaceholder: false })) to avoid duplicating the five fields; update every test fixture that currently repeats those fields (calls to createVisualizationNode at the shown locations) to spread the new defaults and override only differing properties.packages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.ts (1)
52-58: Optional: parallelize child node creation.The sequential
awaitpattern ingetVizNodeFromProcessor(lines 52-58),getChildrenFromBranch(89-111), andgetChildrenFromArrayClause(154-174) serializes what are otherwise independent icon-enrichment I/O operations. Since prev/next linking is synchronous and only depends on the resulting array, each loop can be refactored toPromise.all(...)followed by a linking pass. This reduces latency on deep/wide route trees from O(N) to O(1) icon-fetch roundtrips.Not a correctness issue — defer if it complicates downstream diffs.
♻️ Example: getChildrenFromBranch
- const branchVizNodes: IVisualizationNode[] = []; - for (let index = 0; index < stepsList.length; index++) { - const step = stepsList[index]; - const singlePropertyName = Object.keys(step)[0]; - const childPath = `${path}.${index}.${singlePropertyName}`; - const childComponentLookup = CamelComponentSchemaService.getCamelComponentLookup( - childPath, - getValue(step, singlePropertyName), - ); - - const vizNode = await this.rootNodeMapper.getVizNodeFromProcessor( - childPath, - childComponentLookup, - entityDefinition, - ); - - const previousVizNode = branchVizNodes[branchVizNodes.length - 1]; - if (previousVizNode !== undefined) { - previousVizNode.setNextNode(vizNode); - vizNode.setPreviousNode(previousVizNode); - } - - branchVizNodes.push(vizNode); - } + const branchVizNodes = await Promise.all( + stepsList.map((step, index) => { + const singlePropertyName = Object.keys(step)[0]; + const childPath = `${path}.${index}.${singlePropertyName}`; + const childComponentLookup = CamelComponentSchemaService.getCamelComponentLookup( + childPath, + getValue(step, singlePropertyName), + ); + return this.rootNodeMapper.getVizNodeFromProcessor(childPath, childComponentLookup, entityDefinition); + }), + ); + for (let i = 1; i < branchVizNodes.length; i++) { + branchVizNodes[i - 1].setNextNode(branchVizNodes[i]); + branchVizNodes[i].setPreviousNode(branchVizNodes[i - 1]); + }Also applies to: 85-133, 150-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.ts` around lines 52 - 58, The child node creation is currently performed sequentially; update the loops in getVizNodeFromProcessor, getChildrenFromBranch, and getChildrenFromArrayClause to run icon-enrichment I/O in parallel by mapping childrenStepsProperties (or the equivalent children collection) to promises that call getVizNodesFromChildren, await Promise.all(...) to get the arrays, then flatten the results and perform the synchronous prev/next/linking pass (e.g., calling vizNode.addChild or the existing linking logic) over the resolved children; ensure error handling/ordering is preserved when flattening and linking.packages/ui/src/models/visualization/flows/pipe-visual-entity.ts (1)
250-252: Optional: parallelize independent icon enrichment.
getVizNodeFromStepfor source/sink andgetVizNodesFromStepseach trigger icon I/O viaNodeEnrichmentService.enrichNodeFromCatalog. Since source, step list, and sink are independent, they can run concurrently. For larger pipes this turns N sequential icon fetches into one parallel batch.♻️ Proposed refactor
- const sourceNode = await this.getVizNodeFromStep(this.pipe.spec!.source, 'source', true); - const stepNodes = await this.getVizNodesFromSteps(this.pipe.spec!.steps); - const sinkNode = await this.getVizNodeFromStep(this.pipe.spec!.sink, 'sink'); + const [sourceNode, stepNodes, sinkNode] = await Promise.all([ + this.getVizNodeFromStep(this.pipe.spec!.source, 'source', true), + this.getVizNodesFromSteps(this.pipe.spec!.steps), + this.getVizNodeFromStep(this.pipe.spec!.sink, 'sink'), + ]);And for
getVizNodesFromSteps(linking is purely synchronous, so it can happen after the parallel creation):- private async getVizNodesFromSteps(steps: PipeStep[] = []): Promise<IVisualizationNode[]> { - const vizNodes: IVisualizationNode[] = []; - for (let index = 0; index < steps.length; index++) { - const kamelet = steps[index]; - const path = `steps.${index}`; - const vizNode = await this.getVizNodeFromStep(kamelet, path); - - const previousVizNode = vizNodes[vizNodes.length - 1]; - if (previousVizNode !== undefined) { - previousVizNode.setNextNode(vizNode); - vizNode.setPreviousNode(previousVizNode); - } - - vizNodes.push(vizNode); - } - return vizNodes; - } + private async getVizNodesFromSteps(steps: PipeStep[] = []): Promise<IVisualizationNode[]> { + const vizNodes = await Promise.all( + steps.map((kamelet, index) => this.getVizNodeFromStep(kamelet, `steps.${index}`)), + ); + for (let i = 1; i < vizNodes.length; i++) { + vizNodes[i - 1].setNextNode(vizNodes[i]); + vizNodes[i].setPreviousNode(vizNodes[i - 1]); + } + return vizNodes; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/models/visualization/flows/pipe-visual-entity.ts` around lines 250 - 252, The source/sink node creation and the step-list enrichment are independent and currently run sequentially; modify the code that calls getVizNodeFromStep and getVizNodesFromSteps so they run concurrently (use Promise.all) to parallelize icon I/O from NodeEnrichmentService.enrichNodeFromCatalog: kick off getVizNodeFromStep(this.pipe.spec!.source, 'source', true), getVizNodesFromSteps(this.pipe.spec!.steps), and getVizNodeFromStep(this.pipe.spec!.sink, 'sink') in parallel, await all results, then perform any synchronous linking/graph wiring after the Promise.all completes so linking logic remains unchanged.packages/ui/src/hooks/use-visible-viz-nodes.ts (1)
13-33: Resolve entities in parallel rather than sequentially.The current
for…ofwithawaitserializestoVizNode()across entities, which can noticeably delay the canvas render when there are multiple visible flows (each call awaits catalog-driven enrichment). Since the results are independent,Promise.allover the filtered list would resolve them concurrently while preserving order.Also note that because
entitiesandvisibleFlowsare used as-is in the dependency array, callers passing freshly-constructed arrays/objects on every render will re-trigger resolution unnecessarily. Worth confirming callers memoize these (or deriving a stable key).♻️ Proposed refactor
useEffect(() => { let cancelled = false; const resolve = async () => { - const nodes: IVisualizationNode[] = []; - for (const entity of entities) { - if (visibleFlows[entity.id]) { - const vizNode = await entity.toVizNode(); - nodes.push(vizNode); - } - } + const visibleEntities = entities.filter((entity) => visibleFlows[entity.id]); + const nodes = await Promise.all(visibleEntities.map((entity) => entity.toVizNode())); if (!cancelled) { setVizNodes(nodes); } }; void resolve(); return () => { cancelled = true; }; }, [entities, visibleFlows]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/hooks/use-visible-viz-nodes.ts` around lines 13 - 33, The effect in use-visible-viz-nodes.ts serially awaits each entity.toVizNode() inside resolve, slowing rendering; change resolve to first filter entities by visibleFlows[entity.id], then call Promise.all on the mapped entity.toVizNode() calls to run them in parallel (preserve order by mapping the filtered array), then setVizNodes with the resulting array only if not cancelled; also ensure the dependency array for the useEffect uses stable references (or derive a stable key) for entities and visibleFlows so callers passing new array/object instances every render don't retrigger the effect unnecessarily.packages/ui-tests/stories/canvas/Canvas.stories.tsx (1)
87-140: Optional: extract the repeatedloadersfactory to reduce duplication.All six stories share the exact same loader shape, differing only by entity. A small helper makes intent clearer and removes the repeated
Promise<Pick<CanvasProps, 'vizNodes' | 'entitiesCount'>>annotation.♻️ Proposed fix
+const vizNodeLoader = (entity: { toVizNode: () => Promise<IVisualizationNode> }) => + async (): Promise<Pick<CanvasProps, 'vizNodes' | 'entitiesCount'>> => ({ + vizNodes: [await entity.toVizNode()], + entitiesCount: 1, + }); + export const CamelRouteVisualization: StoryObj<typeof Canvas> = { - loaders: [ - async (): Promise<Pick<CanvasProps, 'vizNodes' | 'entitiesCount'>> => ({ - vizNodes: [await camelRouteEntity.toVizNode()], - entitiesCount: 1, - }), - ], + loaders: [vizNodeLoader(camelRouteEntity)], };(apply the same change to the other five stories)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-tests/stories/canvas/Canvas.stories.tsx` around lines 87 - 140, Several Story objects (CamelRouteVisualization, PipeVisualization, KameletVisualization, EmptyPipeVisualization, EmptyCamelRouteVisualization, IconsRouteVisualization) duplicate the same async loader shape; create a reusable helper (e.g., makeVizLoader or vizLoaderFactory) that accepts an entity (the objects with toVizNode method) and returns the async loader Promise<Pick<CanvasProps,'vizNodes'|'entitiesCount'>>; replace each story's inline loaders with loaders: [makeVizLoader(entity)] to remove duplication and avoid repeating the type annotation across CanvasProps-related stories.packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.test.tsx (1)
34-98: Consolidate the repeatedconsole.errormocking intobeforeEach.
jest.spyOn(console, 'error').mockImplementation(() => {})is now repeated in nearly every test in this file. Moving it intobeforeEach(alongside the existing setup) removes the duplication and keeps each test focused on its own assertions. Tests that need to assert onconsole.errorcalls (e.g. lines 241-243, 260-263, 285-289) can still create a local spy on top.♻️ Proposed fix
beforeEach(() => { jest.useFakeTimers(); + jest.spyOn(console, 'error').mockImplementation(() => {});and then remove the per-test
jest.spyOn(console, 'error').mockImplementation(() => {});calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.test.tsx` around lines 34 - 98, Move the repeated console.error mock into the existing beforeEach: add jest.spyOn(console, 'error').mockImplementation(() => {}) alongside the other setup in HiddenCanvas.test.tsx beforeEach, and remove the per-test jest.spyOn(console, 'error').mockImplementation(() => {}) calls from individual it blocks (e.g., the tests starting "renders the hidden canvas container" and "creates a controller on mount"); for tests that need to assert console.error calls, create a local spy inside that test to override the global mock and perform assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/Visualization/Canvas/Canvas.tsx`:
- Around line 79-83: The empty-state flash occurs because useVisibleVizNodes
initializes vizNodes to [] while async toVizNode() resolves, causing
shouldShowEmptyState (computed from vizNodes.length and entitiesCount) to
briefly treat non-empty data as "all hidden"; update useVisibleVizNodes to
return a resolving flag (e.g., { vizNodes, isResolving } or [vizNodes,
isResolving]) that is true while toVizNode promises are outstanding, then change
Canvas.tsx's shouldShowEmptyState logic to treat "all flows hidden" only when
isResolving is false (or gate rendering of VisualizationEmptyState on
isResolving === false) so the empty-state is not shown during initial async
resolution.
In
`@packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.tsx`:
- Around line 56-62: HiddenCanvas currently passes vizNode.id into
FlowService.getFlowDiagram which causes inconsistent scope resolution compared
to Canvas.tsx; update the call in HiddenCanvas to use the same fallback used in
Canvas (vizNode.getId() ?? vizNode.id) when computing the id passed to
FlowService.getFlowDiagram so exported node IDs use the same scope prefix;
locate the loop that iterates vizNodes and replace the direct vizNode.id usage
with the getId() fallback to keep node identity consistent across canvases.
In
`@packages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.test.ts`:
- Around line 65-75: The parameterized test array in the it.each block contains
a duplicated entry { processorName: 'onCompletion', path: 'onCompletion' }
causing the same assertion to run twice; edit the test data in
camel-intercept-send-to-endpoint-visual-entity.test.ts (the it.each(...) array)
to remove the duplicate or replace it with the intended distinct case (e.g.,
change one to { processorName: 'interceptFrom', path: 'interceptFrom' } or
another correct processorName/path) so each row in the matrix is unique.
In
`@packages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.ts`:
- Around line 18-28: Remove the dead catalogKind assignment from the node data
creation in OnFallbackNodeMapper (and mirror the same change in
DataMapperNodeMapper): update where CamelRouteVisualEntityData is constructed
(the object that currently sets catalogKind: CatalogKind.Processor) to no longer
include catalogKind, since NodeEnrichmentService.enrichNodeFromCatalog() now
resolves icon/title/description; leave other fields (name, path, processorName,
isPlaceholder, isGroup, iconUrl, title, description) intact for consistency with
StepNodeMapper/CircuitBreakerNodeMapper/ParallelProcessorBaseNodeMapper.
---
Duplicate comments:
In
`@packages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.ts`:
- Around line 59-68: The parameterized test array in
camel-intercept-visual-entity.test.ts is missing the 'interceptFrom' case so the
interaction path isn't exercised; update the it.each(...) data to include an
entry with processorName 'interceptFrom' and path 'interceptFrom' (matching the
other entries) so the test executes the interceptFrom interaction path inside
the test that asserts interactions for processorName/path.
---
Nitpick comments:
In `@packages/ui-tests/stories/canvas/Canvas.stories.tsx`:
- Around line 87-140: Several Story objects (CamelRouteVisualization,
PipeVisualization, KameletVisualization, EmptyPipeVisualization,
EmptyCamelRouteVisualization, IconsRouteVisualization) duplicate the same async
loader shape; create a reusable helper (e.g., makeVizLoader or vizLoaderFactory)
that accepts an entity (the objects with toVizNode method) and returns the async
loader Promise<Pick<CanvasProps,'vizNodes'|'entitiesCount'>>; replace each
story's inline loaders with loaders: [makeVizLoader(entity)] to remove
duplication and avoid repeating the type annotation across CanvasProps-related
stories.
In `@packages/ui/src/camel-utils/camel-to-tile.adapter.ts`:
- Around line 69-78: camelEntityToTile is doing a redundant icon fetch because
camelProcessorToTile already calls getIconRequest for Processor and sets
iconUrl, then camelEntityToTile overwrites it by calling getIconRequest for
Entity; fix by changing the shared shaping so only one icon request happens:
either update camelProcessorToTile to accept an options argument (e.g.,
skipIcon: boolean) and call camelProcessorToTile(processorDef, { skipIcon: true
}) from camelEntityToTile, then call getIconRequest(CatalogKind.Entity,
entityTile.name) once and set entityTile.iconUrl, or extract the common
tile-shaping logic into a helper (e.g., buildCamelTileFromProcessor) that
returns the tile without fetching an icon and have camelEntityToTile call
getIconRequest for CatalogKind.Entity itself; ensure you update references to
camelProcessorToTile, getIconRequest, and entityTile.iconUrl accordingly.
In
`@packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.test.tsx`:
- Around line 34-98: Move the repeated console.error mock into the existing
beforeEach: add jest.spyOn(console, 'error').mockImplementation(() => {})
alongside the other setup in HiddenCanvas.test.tsx beforeEach, and remove the
per-test jest.spyOn(console, 'error').mockImplementation(() => {}) calls from
individual it blocks (e.g., the tests starting "renders the hidden canvas
container" and "creates a controller on mount"); for tests that need to assert
console.error calls, create a local spy inside that test to override the global
mock and perform assertions.
In
`@packages/ui/src/components/Visualization/Custom/hooks/paste-step.hook.test.tsx`:
- Around line 71-78: Centralize the repeated visualization-node metadata by
creating a single default object (e.g., visualizationNodeDefaults) and use
object spread when calling createVisualizationNode in tests (e.g.,
createVisualizationNode('test', { ...visualizationNodeDefaults, name: 'test',
isPlaceholder: false })) to avoid duplicating the five fields; update every test
fixture that currently repeats those fields (calls to createVisualizationNode at
the shown locations) to spread the new defaults and override only differing
properties.
In
`@packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsx`:
- Around line 10-15: The test mock for the visualization node uses the old
`catalogKind` field; update the mocked node data in CustomNodeContainer.test.tsx
(the object cast to IVisualizationNode) to include `iconUrl` (a test URL) and
`iconAlt` (a descriptive alt string) instead of `catalogKind`, and adjust the
mock so any tooltip/alt-fallback path that reads `vizNode.data.iconAlt` is
exercised; also remove the now-unused `CatalogKind` import at the top of the
file.
In `@packages/ui/src/hooks/use-visible-viz-nodes.ts`:
- Around line 13-33: The effect in use-visible-viz-nodes.ts serially awaits each
entity.toVizNode() inside resolve, slowing rendering; change resolve to first
filter entities by visibleFlows[entity.id], then call Promise.all on the mapped
entity.toVizNode() calls to run them in parallel (preserve order by mapping the
filtered array), then setVizNodes with the resulting array only if not
cancelled; also ensure the dependency array for the useEffect uses stable
references (or derive a stable key) for entities and visibleFlows so callers
passing new array/object instances every render don't retrigger the effect
unnecessarily.
In
`@packages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.ts`:
- Around line 52-58: The child node creation is currently performed
sequentially; update the loops in getVizNodeFromProcessor,
getChildrenFromBranch, and getChildrenFromArrayClause to run icon-enrichment I/O
in parallel by mapping childrenStepsProperties (or the equivalent children
collection) to promises that call getVizNodesFromChildren, await
Promise.all(...) to get the arrays, then flatten the results and perform the
synchronous prev/next/linking pass (e.g., calling vizNode.addChild or the
existing linking logic) over the resolved children; ensure error
handling/ordering is preserved when flattening and linking.
In `@packages/ui/src/models/visualization/flows/pipe-visual-entity.ts`:
- Around line 250-252: The source/sink node creation and the step-list
enrichment are independent and currently run sequentially; modify the code that
calls getVizNodeFromStep and getVizNodesFromSteps so they run concurrently (use
Promise.all) to parallelize icon I/O from
NodeEnrichmentService.enrichNodeFromCatalog: kick off
getVizNodeFromStep(this.pipe.spec!.source, 'source', true),
getVizNodesFromSteps(this.pipe.spec!.steps), and
getVizNodeFromStep(this.pipe.spec!.sink, 'sink') in parallel, await all results,
then perform any synchronous linking/graph wiring after the Promise.all
completes so linking logic remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77e30862-0f21-4c2f-913c-acae30965333
⛔ Files ignored due to path filters (14)
packages/ui/src/components/Catalog/__snapshots__/BaseCatalog.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Catalog/__snapshots__/DataListItem.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Catalog/__snapshots__/Tile.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/Form/__snapshots__/CanvasForm.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/Form/__snapshots__/CanvasFormHeader.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/Canvas.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/CanvasSideBar.test.tsx.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Canvas/__snapshots__/flow.service.test.ts.snapis excluded by!**/*.snappackages/ui/src/components/Visualization/Custom/Node/__snapshots__/CustomNodeContainer.test.tsx.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-from-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-send-to-endpoint-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-intercept-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/models/visualization/flows/__snapshots__/camel-on-completion-visual-entity.test.ts.snapis excluded by!**/*.snappackages/ui/src/tests/__snapshots__/nodes-edges.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (145)
packages/ui-tests/stories/Icons/Icons.stories.scsspackages/ui-tests/stories/Icons/Icons.stories.tsxpackages/ui-tests/stories/canvas/Canvas.stories.tsxpackages/ui-tests/stories/canvas/CanvasFormBody.stories.tsxpackages/ui-tests/stories/catalog/Catalog.stories.tsxpackages/ui-tests/stories/catalog/PropertiesModal.stories.tsxpackages/ui/src/camel-utils/camel-to-tile.adapter.test.tspackages/ui/src/camel-utils/camel-to-tile.adapter.tspackages/ui/src/components/Catalog/Catalog.models.tspackages/ui/src/components/Catalog/DataListItem.test.tsxpackages/ui/src/components/Catalog/DataListItem.tsxpackages/ui/src/components/Catalog/Tile.test.tsxpackages/ui/src/components/Catalog/Tile.tsxpackages/ui/src/components/Catalog/filter-tiles.test.tspackages/ui/src/components/Catalog/sort-tags.test.tspackages/ui/src/components/DataMapper/on-delete-datamapper.test.tspackages/ui/src/components/GroupAutoStartupSwitch/GroupAutoStartupSwitch.test.tsxpackages/ui/src/components/IconResolver/IconResolver.test.tsxpackages/ui/src/components/IconResolver/IconResolver.tsxpackages/ui/src/components/IconResolver/index.tspackages/ui/src/components/PropertiesModal/PropertiesModal.test.tsxpackages/ui/src/components/PropertiesModal/PropertiesModal.tsxpackages/ui/src/components/RenderingAnchor/RenderingAnchor.test.tsxpackages/ui/src/components/RenderingAnchor/rendering.provider.test.tsxpackages/ui/src/components/Visualization/Canvas/Canvas.test.tsxpackages/ui/src/components/Visualization/Canvas/Canvas.tsxpackages/ui/src/components/Visualization/Canvas/CanvasSideBar.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasForm.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasForm.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormBody.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.test.tsxpackages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.tsxpackages/ui/src/components/Visualization/Canvas/flow.service.test.tspackages/ui/src/components/Visualization/ContextToolbar/ExportDocument/ExportDocumentPreviewModal.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/FlowExportImage.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.test.tsxpackages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/HiddenCanvas.tsxpackages/ui/src/components/Visualization/ContextToolbar/Flows/FlowsList.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemCopyStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteGroup.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemDuplicateStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemEnableAllSteps.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemHideOtherFlows.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemMoveStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemPasteStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/NodeContextMenu.test.tsxpackages/ui/src/components/Visualization/Custom/ContextMenu/get-move-icons.util.test.tspackages/ui/src/components/Visualization/Custom/ContextMenu/item-interaction-helper.test.tspackages/ui/src/components/Visualization/Custom/Edge/CustomEdge.test.tsxpackages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.test.tsxpackages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNode.test.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.test.tsxpackages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.tsxpackages/ui/src/components/Visualization/Custom/Node/PlaceholderNode.test.tsxpackages/ui/src/components/Visualization/Custom/customComponentUtils.test.tspackages/ui/src/components/Visualization/Custom/hooks/add-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/delete-group.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/delete-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/disable-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/duplicate-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/enable-all-steps.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/insert-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/move-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/paste-step.hook.test.tsxpackages/ui/src/components/Visualization/Custom/hooks/replace-step.hook.test.tsxpackages/ui/src/components/Visualization/Visualization.test.tsxpackages/ui/src/components/Visualization/Visualization.tsxpackages/ui/src/dynamic-catalog/catalog-modal.provider.test.tsxpackages/ui/src/dynamic-catalog/catalog-tiles.provider.tsxpackages/ui/src/hooks/use-visible-viz-nodes.test.tspackages/ui/src/hooks/use-visible-viz-nodes.tspackages/ui/src/icon-resolver/getIconRequest.test.tspackages/ui/src/icon-resolver/getIconRequest.tspackages/ui/src/icon-resolver/node-icon-resolver.tspackages/ui/src/models/camel/camel-route-resource.test.tspackages/ui/src/models/camel/kamelet-resource.test.tspackages/ui/src/models/citrus/citrus-test-resource.test.tspackages/ui/src/models/visualization/base-visual-entity.tspackages/ui/src/models/visualization/flows/abstract-camel-visual-entity.test.tspackages/ui/src/models/visualization/flows/abstract-camel-visual-entity.tspackages/ui/src/models/visualization/flows/camel-error-handler-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-error-handler-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.tspackages/ui/src/models/visualization/flows/camel-intercept-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-intercept-visual-entity.tspackages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.tspackages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.tspackages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.tspackages/ui/src/models/visualization/flows/camel-rest-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-rest-visual-entity.tspackages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.test.tspackages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.tspackages/ui/src/models/visualization/flows/camel-route-visual-entity.test.tspackages/ui/src/models/visualization/flows/citrus-test-visual-entity.test.tspackages/ui/src/models/visualization/flows/citrus-test-visual-entity.tspackages/ui/src/models/visualization/flows/kamelet-visual-entity.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/base-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/choice-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/choice-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/parallel-processor-base-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/parallel-processor-base-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/route-configuration-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/route-configuration-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/step-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/step-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/testing/noop-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.tspackages/ui/src/models/visualization/flows/nodes/node-enrichment.service.tspackages/ui/src/models/visualization/flows/nodes/node-mapper.service.tspackages/ui/src/models/visualization/flows/nodes/node-mapper.tspackages/ui/src/models/visualization/flows/nodes/root-node-mapper.test.tspackages/ui/src/models/visualization/flows/nodes/root-node-mapper.tspackages/ui/src/models/visualization/flows/pipe-visual-entity.test.tspackages/ui/src/models/visualization/flows/pipe-visual-entity.tspackages/ui/src/models/visualization/flows/support/camel-component-filter.service.test.tspackages/ui/src/models/visualization/visualization-node.test.tspackages/ui/src/pages/Catalog/CatalogPage.test.tsxpackages/ui/src/services/datamapper-step.service.test.tspackages/ui/src/stubs/tiles.tspackages/ui/src/testing-api.tspackages/ui/src/tests/nodes-edges.test.tspackages/ui/src/utils/get-viznodes-from-graph.test.ts
💤 Files with no reviewable changes (9)
- packages/ui/src/components/IconResolver/index.ts
- packages/ui-tests/stories/Icons/Icons.stories.scss
- packages/ui/src/components/IconResolver/IconResolver.test.tsx
- packages/ui-tests/stories/Icons/Icons.stories.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.test.ts
- packages/ui/src/components/IconResolver/IconResolver.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-node-mapper.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/rest-dsl-verbs-node-mapper.ts
✅ Files skipped from review due to trivial changes (36)
- packages/ui/src/components/Catalog/Tile.tsx
- packages/ui/src/components/Visualization/Custom/customComponentUtils.test.ts
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemMoveStep.test.tsx
- packages/ui/src/components/RenderingAnchor/RenderingAnchor.test.tsx
- packages/ui/src/components/Catalog/Catalog.models.ts
- packages/ui/src/dynamic-catalog/catalog-modal.provider.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.test.tsx
- packages/ui/src/components/Catalog/Tile.test.tsx
- packages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.test.tsx
- packages/ui/src/components/Catalog/sort-tags.test.ts
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemCopyStep.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/on-fallback-node-mapper.test.ts
- packages/ui/src/components/Visualization/ContextToolbar/Flows/FlowsList.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemEnableAllSteps.test.tsx
- packages/ui/src/tests/nodes-edges.test.ts
- packages/ui/src/components/Catalog/filter-tiles.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/otherwise-node-mapper.test.ts
- packages/ui/src/components/Catalog/DataListItem.test.tsx
- packages/ui/src/components/GroupAutoStartupSwitch/GroupAutoStartupSwitch.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDuplicateStep.test.tsx
- packages/ui/src/components/Visualization/Custom/hooks/insert-step.hook.test.tsx
- packages/ui/src/components/Visualization/Custom/Node/CustomNode.test.tsx
- packages/ui/src/models/camel/kamelet-resource.test.ts
- packages/ui/src/components/PropertiesModal/PropertiesModal.test.tsx
- packages/ui/src/models/camel/camel-route-resource.test.ts
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsx
- packages/ui/src/pages/Catalog/CatalogPage.test.tsx
- packages/ui/src/models/visualization/flows/support/camel-component-filter.service.test.ts
- packages/ui/src/models/visualization/flows/kamelet-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.test.ts
- packages/ui/src/components/Visualization/Custom/hooks/delete-step.hook.test.tsx
- packages/ui/src/components/DataMapper/on-delete-datamapper.test.ts
- packages/ui/src/services/datamapper-step.service.test.ts
- packages/ui-tests/stories/canvas/CanvasFormBody.stories.tsx
- packages/ui/src/stubs/tiles.ts
- packages/ui/src/icon-resolver/getIconRequest.test.ts
🚧 Files skipped from review as they are similar to previous changes (61)
- packages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.test.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemPasteStep.test.tsx
- packages/ui/src/components/Visualization/ContextToolbar/FlowExportImage/FlowExportImage.tsx
- packages/ui/src/components/Visualization/Custom/Edge/CustomEdge.test.tsx
- packages/ui/src/components/Visualization/Canvas/Form/CanvasFormBody.test.tsx
- packages/ui/src/components/Visualization/Canvas/CanvasSideBar.test.tsx
- packages/ui/src/components/Visualization/Custom/Node/CustomNodeContainer.tsx
- packages/ui/src/components/RenderingAnchor/rendering.provider.test.tsx
- packages/ui/src/components/Visualization/ContextToolbar/ExportDocument/ExportDocumentPreviewModal.tsx
- packages/ui/src/components/Visualization/Custom/hooks/disable-step.hook.test.tsx
- packages/ui/src/icon-resolver/node-icon-resolver.ts
- packages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.test.ts
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemHideOtherFlows.test.tsx
- packages/ui/src/models/visualization/flows/camel-error-handler-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/when-node-mapper.test.ts
- packages/ui/src/components/Visualization/Custom/hooks/replace-step.hook.test.tsx
- packages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.ts
- packages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.test.ts
- packages/ui/src/components/Visualization/Custom/ContextMenu/get-move-icons.util.test.ts
- packages/ui/src/components/Visualization/Custom/hooks/move-step.hook.test.tsx
- packages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.ts
- packages/ui/src/components/Visualization/Canvas/Form/CanvasFormHeader.tsx
- packages/ui/src/components/Catalog/DataListItem.tsx
- packages/ui/src/dynamic-catalog/catalog-tiles.provider.tsx
- packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteGroup.test.tsx
- packages/ui/src/testing-api.ts
- packages/ui/src/models/visualization/flows/nodes/root-node-mapper.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/route-configuration-node-mapper.test.ts
- packages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.tsx
- packages/ui/src/models/visualization/flows/abstract-camel-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/nodes/node-mapper.ts
- packages/ui/src/components/Visualization/Custom/hooks/duplicate-step.hook.test.tsx
- packages/ui/src/models/visualization/flows/camel-route-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/camel-rest-configuration-visual-entity.ts
- packages/ui/src/components/Visualization/Canvas/flow.service.test.ts
- packages/ui/src/models/visualization/flows/camel-intercept-send-to-endpoint-visual-entity.ts
- packages/ui/src/models/visualization/flows/nodes/node-enrichment.service.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/testing/noop-node-mapper.ts
- packages/ui/src/models/visualization/flows/camel-rest-visual-entity.ts
- packages/ui/src/components/Visualization/Custom/hooks/add-step.hook.test.tsx
- packages/ui/src/models/visualization/flows/camel-intercept-visual-entity.ts
- packages/ui/src/models/visualization/flows/camel-intercept-from-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/pipe-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/abstract-camel-visual-entity.ts
- packages/ui/src/models/visualization/flows/camel-on-completion-visual-entity.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/choice-node-mapper.ts
- packages/ui/src/components/Visualization/Canvas/Canvas.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/circuit-breaker-node-mapper.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/route-configuration-node-mapper.ts
- packages/ui/src/models/visualization/flows/camel-route-configuration-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/nodes/mappers/parallel-processor-base-node-mapper.ts
- packages/ui/src/components/Visualization/Custom/hooks/enable-all-steps.hook.test.tsx
- packages/ui/src/models/visualization/flows/nodes/mappers/datamapper-node-mapper.ts
- packages/ui/src/models/visualization/flows/citrus-test-visual-entity.test.ts
- packages/ui/src/icon-resolver/getIconRequest.ts
- packages/ui/src/models/citrus/citrus-test-resource.test.ts
- packages/ui/src/models/visualization/flows/camel-rest-visual-entity.test.ts
- packages/ui/src/models/visualization/flows/camel-on-exception-visual-entity.ts
- packages/ui/src/models/visualization/flows/citrus-test-visual-entity.ts
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #3095
What We Did
We performed a major architectural refactoring that moved icon resolution logic from the Canvas/UI layer to the Data Mapping layer. This change fundamentally altered how visualization nodes acquire their icon URLs.
Before (Canvas Layer)
Icons were fetched lazily when nodes were rendered in the Canvas
The IconResolver component handled icon fetching at the UI level
Each node would request its icon during render time
Icon URLs were not part of the node's core data structure
After (Mapping Layer)
Icons are fetched eagerly during node creation/mapping
The NodeEnrichmentService handles icon fetching at the data model level
Icon URLs are embedded in IVisualizationNodeData as required properties
Icons are resolved before nodes reach the Canvas
Summary by CodeRabbit
Bug Fixes
Refactor