feat: more robust resource visualization#618
Merged
adityachoudhari26 merged 2 commits intomainfrom Jul 12, 2025
Merged
Conversation
Contributor
WalkthroughThis update introduces a new resource visualization API endpoint and refactors the frontend resource relationship diagram to use unified data structures. Components for displaying resource nodes are expanded to show detailed deployment and system information, and a new backend procedure provides the latest deployment job status for release targets. Related API and UI components are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebApp
participant API
participant DB
Client->>WebApp: Navigate to resource visualization page
WebApp->>API: Call resource.visualize(resourceId)
API->>DB: Fetch resource by ID
API->>DB: Recursively fetch parent/child relationships, systems, release targets
API->>DB: Group release targets by system
API-->>WebApp: Return { resources, edges }
WebApp->>API: For each release target, call releaseTarget.latestJob(releaseTargetId)
API->>DB: Query latest successful job for release target
API-->>WebApp: Return job and deployment version (or null)
WebApp-->>Client: Render relationships diagram with enriched nodes and edges
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/api/src/router/resource-visualization.ts (1)
44-44: Remove unnecessary empty constructor.The empty constructor serves no purpose and can be removed.
class TreeBuilder { private resources: schema.Resource[] = []; private edges: Edge[] = []; - constructor() {}apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/ResourceNode.tsx (1)
107-138: Consider making the node width more flexible.The hard-coded width of 450px might cause layout issues with different content or screen sizes.
Consider using a min/max width approach or making it configurable:
- <div className="flex w-[450px] flex-col gap-4 rounded-md border bg-background p-3"> + <div className="flex min-w-[350px] max-w-[450px] flex-col gap-4 rounded-md border bg-background p-3">
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/resources/ResourceIcon.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/RelationshipsDiagram.tsx(3 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/ResourceNode.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/page.tsx(1 hunks)packages/api/src/router/release-target.ts(2 hunks)packages/api/src/router/resource-visualization.ts(1 hunks)packages/api/src/router/resources.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
⚙️ CodeRabbit Configuration File
**/*.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.{js,jsx,ts,tsx,json,css,md,yml,yaml}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (8)
📓 Common learnings
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#515
File: apps/webservice/src/app/api/v1/release-targets/[releaseTargetId]/releases/route.ts:103-108
Timestamp: 2025-04-28T18:41:58.813Z
Learning: In this project, full records from the `deployment` and `deployment_version` tables are considered safe for public API consumption, and there's no need to create restricted DTOs for them.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#601
File: e2e/tests/api/policies/retry-policy.spec.ts:117-130
Timestamp: 2025-06-24T23:53:25.398Z
Learning: User adityachoudhari26 prefers to keep non-null assertions in e2e test code without extensive null safety checks, reasoning that test failures serve the same purpose of catching issues and the extra validation doesn't add much value in test contexts.
packages/api/src/router/resources.ts (2)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx:43-50
Timestamp: 2024-11-27T23:16:35.580Z
Learning: In `DeploymentResourceDrawer.tsx`, the `isOpen` variable already checks whether `deploymentId`, `environmentId`, and `resourceId` are non-null, so additional null checks in query `enabled` conditions are not necessary.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#181
File: packages/api/src/router/deployment.ts:116-131
Timestamp: 2024-10-29T02:04:50.312Z
Learning: In `packages/api/src/router/deployment.ts`, the `list.byDeploymentId` procedure requires multiple database queries due to limitations of the `releaseMatchesCondition` function.
apps/webservice/src/app/[workspaceSlug]/(app)/_components/resources/ResourceIcon.tsx (2)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#188
File: apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/Usage.tsx:42-44
Timestamp: 2024-11-01T02:35:07.352Z
Learning: In `apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/Usage.tsx`, within the `Usage` component, the arrays used for filtering inherited environments are expected to remain small. Therefore, performance optimizations for this filtering logic are not necessary.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx:43-50
Timestamp: 2024-11-27T23:16:35.580Z
Learning: In `DeploymentResourceDrawer.tsx`, the `isOpen` variable already checks whether `deploymentId`, `environmentId`, and `resourceId` are non-null, so additional null checks in query `enabled` conditions are not necessary.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/page.tsx (8)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#165
File: apps/webservice/src/app/[workspaceSlug]/_components/relationships/RelationshipsDiagramDependencies.tsx:185-189
Timestamp: 2024-10-24T21:31:17.820Z
Learning: In `RelationshipsDiagramDependencies.tsx`, when using React Flow's `BaseEdge`, the `markerStart` prop may intentionally use the `markerEnd` value to flip the arrow direction because 'source depends on target' logic is applied. These are intrinsic attributes of the React Flow `BaseEdge` component and cannot be renamed or changed.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#188
File: apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/Usage.tsx:42-44
Timestamp: 2024-11-01T02:35:07.352Z
Learning: In `apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/Usage.tsx`, within the `Usage` component, the arrays used for filtering inherited environments are expected to remain small. Therefore, performance optimizations for this filtering logic are not necessary.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx:43-50
Timestamp: 2024-11-27T23:16:35.580Z
Learning: In `DeploymentResourceDrawer.tsx`, the `isOpen` variable already checks whether `deploymentId`, `environmentId`, and `resourceId` are non-null, so additional null checks in query `enabled` conditions are not necessary.
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#363
File: apps/webservice/tsconfig.json:14-15
Timestamp: 2025-03-09T08:56:53.603Z
Learning: In the ctrlplane project, they use Next.js standalone output mode, which means they intentionally exclude `.next/types` in their tsconfig.json despite including `.next/types/**/*.ts` in their include pattern.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#578
File: apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/providers/integrations/github/GithubDialog.tsx:58-58
Timestamp: 2025-05-30T21:48:48.868Z
Learning: In the ctrlplane codebase, the shadcn form UI version allows initializing forms with `useForm({ schema: formSchema })` directly, without needing to import and use `zodResolver` from '@hookform/resolvers/zod'. This is different from standard react-hook-form usage.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#579
File: packages/db/src/schema/rules/concurrency.ts:0-0
Timestamp: 2025-06-01T19:10:11.535Z
Learning: In the ctrlplane codebase, when defining database schemas with Drizzle ORM, it's an intentional pattern to spread base fields (like `basePolicyRuleFields`) and then redefine specific fields to add additional constraints (like unique constraints or foreign key references). The TypeScript field overwriting behavior is deliberately used to override base field definitions with more specific requirements.
Learnt from: CR
PR: ctrlplanedev/ctrlplane#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T21:36:42.935Z
Learning: Applies to **/*.{ts,tsx} : Consistent type imports: `import type { Type } from "module"`
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#382
File: apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts:82-88
Timestamp: 2025-03-16T19:41:44.129Z
Learning: In Next.js 15, dynamic route parameters (params) return Promises instead of direct values in async contexts. When accessing properties like `params.deploymentId` in an async function, use `(await params).deploymentId` to avoid the "params should be awaited before using its properties" error. This applies to API routes, page components, and other async contexts.
packages/api/src/router/resource-visualization.ts (2)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#181
File: packages/api/src/router/deployment.ts:116-131
Timestamp: 2024-10-29T02:04:50.312Z
Learning: In `packages/api/src/router/deployment.ts`, the `list.byDeploymentId` procedure requires multiple database queries due to limitations of the `releaseMatchesCondition` function.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#188
File: packages/api/src/router/deployment.ts:144-161
Timestamp: 2024-11-01T02:37:25.510Z
Learning: In `packages/api/src/router/deployment.ts`, when using Drizzle ORM, there is a limitation when referencing the same table twice in a relational builder query (rbq), requiring separate queries to avoid issues.
packages/api/src/router/release-target.ts (12)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#181
File: packages/api/src/router/deployment.ts:116-131
Timestamp: 2024-10-29T02:04:50.312Z
Learning: In `packages/api/src/router/deployment.ts`, the `list.byDeploymentId` procedure requires multiple database queries due to limitations of the `releaseMatchesCondition` function.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: packages/api/src/router/job.ts:362-365
Timestamp: 2024-11-27T23:18:42.055Z
Learning: In the file `packages/api/src/router/job.ts`, the function `releaseMatchesCondition` returns undefined if the `filter` parameter is null. This behavior ensures that when constructing the query with `and(...)`, the condition is omitted, allowing the query to function correctly even if there is no release channel associated with the environment.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#188
File: packages/api/src/router/deployment.ts:144-161
Timestamp: 2024-11-01T02:37:25.510Z
Learning: In `packages/api/src/router/deployment.ts`, when using Drizzle ORM, there is a limitation when referencing the same table twice in a relational builder query (rbq), requiring separate queries to avoid issues.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#395
File: packages/api/src/router/environment-page/resources/router.ts:40-45
Timestamp: 2025-03-24T18:46:38.894Z
Learning: The `takeFirst` utility function in the codebase (from `@ctrlplane/db`) throws an Error with message "Found non unique or inexistent value" if the result array doesn't contain exactly one element, making additional null/undefined checks unnecessary after its use.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#579
File: packages/db/src/schema/rules/concurrency.ts:0-0
Timestamp: 2025-06-01T19:10:11.535Z
Learning: In the ctrlplane codebase, when defining database schemas with Drizzle ORM, it's an intentional pattern to spread base fields (like `basePolicyRuleFields`) and then redefine specific fields to add additional constraints (like unique constraints or foreign key references). The TypeScript field overwriting behavior is deliberately used to override base field definitions with more specific requirements.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#578
File: apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/providers/integrations/github/GithubDialog.tsx:58-58
Timestamp: 2025-05-30T21:48:48.868Z
Learning: In the ctrlplane codebase, the shadcn form UI version allows initializing forms with `useForm({ schema: formSchema })` directly, without needing to import and use `zodResolver` from '@hookform/resolvers/zod'. This is different from standard react-hook-form usage.
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#280
File: integrations/terraform-cloud-scanner/src/scanner.ts:0-0
Timestamp: 2025-01-14T16:41:56.796Z
Learning: In the Terraform Cloud scanner implementation, functional programming is preferred over imperative approaches, utilizing lodash's chain API and functional methods for data transformation and processing.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#593
File: packages/rule-engine/src/manager/version-manager-rules/version-approval.ts:55-70
Timestamp: 2025-06-18T21:46:51.459Z
Learning: The `takeFirst` utility function from the database layer throws an error if no record is found, rather than returning `undefined`. This means functions using `takeFirst` already have error handling built-in and don't require additional null checks.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#601
File: packages/job-dispatch/src/job-update.ts:264-270
Timestamp: 2025-06-24T23:56:54.799Z
Learning: In this codebase, the `Tx` type is just an alias for the database client type (`Omit<typeof db, "$client">`) and does not necessarily indicate an active transaction context. Functions like `createReleaseJob` need to be called within a transaction, which is why they are wrapped with `db.transaction()` even when the parameter is typed as `Tx`. Drizzle supports nested transactions via breakpoints, so additional transaction wrappers are safe even if already within a transaction.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#579
File: packages/rule-engine/src/rules/concurrency-rule.ts:8-11
Timestamp: 2025-06-01T19:10:47.122Z
Learning: In packages/rule-engine/src/rules/concurrency-rule.ts, the ConcurrencyRule should remain simple without additional validation since database and Zod schemas already handle concurrency validation. The user prefers this rule to be "dumb" and just perform the comparison check rather than duplicating validation logic.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#500
File: packages/db/src/schema/job.ts:117-120
Timestamp: 2025-04-21T18:34:54.764Z
Learning: In the system's job schema, the relationship between job and releaseJob is a true one-to-one relationship - a release job should only ever point to one job and vice versa. The implementation uses `one(releaseJob, ...)` in the jobRelations to reflect this business rule.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#500
File: apps/webservice/src/app/api/v1/jobs/[jobId]/new-engine-job.ts:0-0
Timestamp: 2025-04-21T18:47:47.640Z
Learning: In the job API endpoints, it's acceptable to let internal code fail with TypeErrors when encountering unexpected null values (like missing releaseJob) as long as the errors are caught at the function boundary, logged for debugging, and a consistent response is returned to the caller.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/ResourceNode.tsx (3)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#188
File: apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/Usage.tsx:42-44
Timestamp: 2024-11-01T02:35:07.352Z
Learning: In `apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/Usage.tsx`, within the `Usage` component, the arrays used for filtering inherited environments are expected to remain small. Therefore, performance optimizations for this filtering logic are not necessary.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx:43-50
Timestamp: 2024-11-27T23:16:35.580Z
Learning: In `DeploymentResourceDrawer.tsx`, the `isOpen` variable already checks whether `deploymentId`, `environmentId`, and `resourceId` are non-null, so additional null checks in query `enabled` conditions are not necessary.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#165
File: apps/webservice/src/app/[workspaceSlug]/_components/relationships/RelationshipsDiagramDependencies.tsx:185-189
Timestamp: 2024-10-24T21:31:17.820Z
Learning: In `RelationshipsDiagramDependencies.tsx`, when using React Flow's `BaseEdge`, the `markerStart` prop may intentionally use the `markerEnd` value to flip the arrow direction because 'source depends on target' logic is applied. These are intrinsic attributes of the React Flow `BaseEdge` component and cannot be renamed or changed.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/RelationshipsDiagram.tsx (1)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#165
File: apps/webservice/src/app/[workspaceSlug]/_components/relationships/RelationshipsDiagramDependencies.tsx:185-189
Timestamp: 2024-10-24T21:31:17.820Z
Learning: In `RelationshipsDiagramDependencies.tsx`, when using React Flow's `BaseEdge`, the `markerStart` prop may intentionally use the `markerEnd` value to flip the arrow direction because 'source depends on target' logic is applied. These are intrinsic attributes of the React Flow `BaseEdge` component and cannot be renamed or changed.
🧬 Code Graph Analysis (3)
packages/api/src/router/resources.ts (1)
packages/api/src/router/resource-visualization.ts (1)
resourceVisualization(106-121)
packages/api/src/router/resource-visualization.ts (6)
packages/db/src/client.ts (1)
db(15-15)packages/db/src/common.ts (1)
takeFirst(9-13)packages/db/src/schema/resource.ts (1)
resource(58-86)packages/db/src/queries/relationships/get-resource-parents.ts (1)
getResourceParents(20-84)packages/db/src/queries/relationships/get-resource-children.ts (1)
getResourceChildren(19-72)packages/api/src/trpc.ts (1)
protectedProcedure(173-173)
packages/api/src/router/release-target.ts (3)
packages/api/src/trpc.ts (1)
protectedProcedure(173-173)packages/db/src/schema/job.ts (1)
JobStatus(149-149)packages/db/src/common.ts (1)
takeFirstOrNull(15-20)
🪛 Biome (1.9.4)
packages/api/src/router/resource-visualization.ts
[error] 44-44: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
🔇 Additional comments (14)
packages/api/src/router/release-target.ts (1)
99-141: LGTM! Well-structured query for fetching the latest successful deployment.The implementation correctly joins the necessary tables and filters for successful jobs, providing the latest deployment information for a release target.
packages/api/src/router/resources.ts (1)
36-36: Clean integration of the new visualization endpoint.The resource visualization router is properly imported and integrated following the existing pattern.
Also applies to: 79-79
apps/webservice/src/app/[workspaceSlug]/(app)/_components/resources/ResourceIcon.tsx (1)
15-21: Good enhancement for flexible styling.The addition of the
classNameprop and consistent use ofcnutility across all icon cases enables better component reusability.Also applies to: 23-65
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/page.tsx (1)
27-36: Clean migration to the new visualization API.The code properly handles the null case and adapts to the new unified data model with baseResource, resources, and edges.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/RelationshipsDiagram.tsx (6)
13-22: LGTM! Clean type definition for unified edge model.The simplified
Edgetype effectively consolidates the relationship model and properly references the schema type fordependencyType.
24-28: Good refactoring of the props interface.The simplified props structure with
baseResource,resources, andedgesprovides a cleaner API that aligns well with the unified data model.
30-36: Node creation logic looks good.The function properly transforms
ResourceNodeDatainto ReactFlow nodes and correctly embeds thebaseResourceIdfor reference.
43-55: Well-documented edge reversal logic.The comment clearly explains why source and target are reversed to match ctrlplane's domain model. The implementation correctly handles this transformation.
60-75: Component implementation properly updated for new data model.The hooks correctly use the new unified data structures, and the
focusedNodeIdis appropriately set tobaseResource.id.
95-99: Clean simplification of the provider component.The direct prop spreading makes the provider more maintainable and reduces boilerplate.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/ResourceNode.tsx (4)
3-25: Well-structured type definitions using API outputs.Good use of type aliases for
SystemandReleaseTargetto improve code readability.
27-39: Clean header component implementation.Well-structured presentational component with proper use of the
ResourceIcon.
76-87: Good layout component with proper text truncation.The component effectively handles long deployment slugs with appropriate truncation.
89-105: Well-structured section component for system grouping.Good visual hierarchy with proper styling and text transformation.
Comment on lines
+19
to
+33
| const getRootsOfTree = async ( | ||
| resource: schema.Resource, | ||
| roots: schema.Resource[], | ||
| ): Promise<schema.Resource[]> => { | ||
| const parents = await getResourceParents(db, resource.id); | ||
| const hasNoParents = Object.keys(parents.relationships).length === 0; | ||
| if (hasNoParents) return [...roots, resource]; | ||
|
|
||
| const retrievedRoots = await Promise.all( | ||
| Object.values(parents.relationships).map((r) => | ||
| getRootsOfTree(r.target, roots), | ||
| ), | ||
| ).then((roots) => roots.flat()); | ||
| return [...retrievedRoots, ...roots]; | ||
| }; |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding cycle detection to prevent infinite recursion.
The recursive functions getRootsOfTree and getChildren could run indefinitely if circular dependencies exist in the resource relationships. Consider tracking visited resources to prevent infinite loops.
Also applies to: 58-66
🏁 Script executed:
#!/bin/bash
# Description: Check if circular resource dependencies are possible in the database schema or if there are constraints preventing them
# Search for any unique constraints or checks on resource relationships
rg -A 10 "resourceRelationship" --glob "**/*.ts" | grep -E "(unique|constraint|check)"
# Look for any validation logic around resource relationships
ast-grep --pattern 'function $FUNC($$$) {
$$$
resourceRelationship
$$$
}'Length of output: 765
Add cycle detection to recursive resource traversal
To avoid potential infinite loops if resource relationships form a cycle, enhance both recursive functions to track and short-circuit on already-visited resource IDs.
• getRootsOfTree (packages/api/src/router/resource-visualization.ts:19-33)
• getChildren (packages/api/src/router/resource-visualization.ts:58-66)
Suggested change:
- const getRootsOfTree = async (
- resource: schema.Resource,
- roots: schema.Resource[],
- ): Promise<schema.Resource[]> => {
+ const getRootsOfTree = async (
+ resource: schema.Resource,
+ roots: schema.Resource[],
+ visited = new Set<string>(),
+ ): Promise<schema.Resource[]> => {
+ if (visited.has(resource.id)) {
+ // already seen—break the cycle
+ return [];
+ }
+ visited.add(resource.id);
const parents = await getResourceParents(db, resource.id);
const hasNoParents = Object.keys(parents.relationships).length === 0;
if (hasNoParents) return [...roots, resource];
const retrievedRoots = await Promise.all(
- Object.values(parents.relationships).map((r) =>
- getRootsOfTree(r.target, roots),
+ Object.values(parents.relationships).map((r) =>
+ getRootsOfTree(r.target, roots, visited),
),
).then((roots) => roots.flat());
return [...retrievedRoots, ...roots];
};Apply a similar visited-set pattern to getChildren to ensure both traversals cannot recurse infinitely.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const getRootsOfTree = async ( | |
| resource: schema.Resource, | |
| roots: schema.Resource[], | |
| ): Promise<schema.Resource[]> => { | |
| const parents = await getResourceParents(db, resource.id); | |
| const hasNoParents = Object.keys(parents.relationships).length === 0; | |
| if (hasNoParents) return [...roots, resource]; | |
| const retrievedRoots = await Promise.all( | |
| Object.values(parents.relationships).map((r) => | |
| getRootsOfTree(r.target, roots), | |
| ), | |
| ).then((roots) => roots.flat()); | |
| return [...retrievedRoots, ...roots]; | |
| }; | |
| const getRootsOfTree = async ( | |
| resource: schema.Resource, | |
| roots: schema.Resource[], | |
| visited = new Set<string>(), | |
| ): Promise<schema.Resource[]> => { | |
| if (visited.has(resource.id)) { | |
| // already seen—break the cycle | |
| return []; | |
| } | |
| visited.add(resource.id); | |
| const parents = await getResourceParents(db, resource.id); | |
| const hasNoParents = Object.keys(parents.relationships).length === 0; | |
| if (hasNoParents) return [...roots, resource]; | |
| const retrievedRoots = await Promise.all( | |
| Object.values(parents.relationships).map((r) => | |
| getRootsOfTree(r.target, roots, visited), | |
| ), | |
| ).then((roots) => roots.flat()); | |
| return [...retrievedRoots, ...roots]; | |
| }; |
🤖 Prompt for AI Agents
In packages/api/src/router/resource-visualization.ts lines 19 to 33, the
getRootsOfTree function lacks cycle detection, risking infinite recursion if
resource relationships form cycles. Modify getRootsOfTree to accept a Set of
visited resource IDs, check if the current resource ID is already in this set
before recursing, and add it when first visited. Apply the same visited-set
pattern to the getChildren function (lines 58 to 66) to prevent infinite loops
in both recursive traversals.
Comment on lines
+41
to
+74
| const ReleaseTargetStatus: React.FC<{ releaseTarget: ReleaseTarget }> = ({ | ||
| releaseTarget, | ||
| }) => { | ||
| const { data, isLoading } = api.releaseTarget.latestJob.useQuery( | ||
| releaseTarget.id, | ||
| { refetchInterval: 10_000 }, | ||
| ); | ||
|
|
||
| const { workspaceSlug } = useParams<{ workspaceSlug: string }>(); | ||
|
|
||
| if (isLoading) return <Skeleton className="h-4 w-20" />; | ||
| if (!data) | ||
| return <span className="text-xs text-muted-foreground">Not deployed</span>; | ||
|
|
||
| const versionUrl = urls | ||
| .workspace(workspaceSlug) | ||
| .system(releaseTarget.system.slug) | ||
| .deployment(releaseTarget.deployment.slug) | ||
| .release(data.version.id) | ||
| .jobs(); | ||
|
|
||
| return ( | ||
| <Link | ||
| href={versionUrl} | ||
| className={cn( | ||
| "flex h-6 items-center gap-1 truncate", | ||
| buttonVariants({ variant: "ghost", className: "h-6" }), | ||
| )} | ||
| > | ||
| <JobTableStatusIcon status={data.job.status} className="flex-shrink-0" /> | ||
| {data.version.tag} | ||
| </Link> | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling and making polling interval configurable.
The component lacks error handling for the query. Failed requests will appear as "Not deployed" which could be misleading. Also, the 10-second polling interval might be too frequent for production use.
const ReleaseTargetStatus: React.FC<{ releaseTarget: ReleaseTarget }> = ({
releaseTarget,
}) => {
- const { data, isLoading } = api.releaseTarget.latestJob.useQuery(
+ const { data, isLoading, error } = api.releaseTarget.latestJob.useQuery(
releaseTarget.id,
{ refetchInterval: 10_000 },
);
const { workspaceSlug } = useParams<{ workspaceSlug: string }>();
if (isLoading) return <Skeleton className="h-4 w-20" />;
+ if (error) return <span className="text-xs text-destructive">Error</span>;
if (!data)
return <span className="text-xs text-muted-foreground">Not deployed</span>;Consider making the polling interval configurable through props or environment variables for different environments.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const ReleaseTargetStatus: React.FC<{ releaseTarget: ReleaseTarget }> = ({ | |
| releaseTarget, | |
| }) => { | |
| const { data, isLoading } = api.releaseTarget.latestJob.useQuery( | |
| releaseTarget.id, | |
| { refetchInterval: 10_000 }, | |
| ); | |
| const { workspaceSlug } = useParams<{ workspaceSlug: string }>(); | |
| if (isLoading) return <Skeleton className="h-4 w-20" />; | |
| if (!data) | |
| return <span className="text-xs text-muted-foreground">Not deployed</span>; | |
| const versionUrl = urls | |
| .workspace(workspaceSlug) | |
| .system(releaseTarget.system.slug) | |
| .deployment(releaseTarget.deployment.slug) | |
| .release(data.version.id) | |
| .jobs(); | |
| return ( | |
| <Link | |
| href={versionUrl} | |
| className={cn( | |
| "flex h-6 items-center gap-1 truncate", | |
| buttonVariants({ variant: "ghost", className: "h-6" }), | |
| )} | |
| > | |
| <JobTableStatusIcon status={data.job.status} className="flex-shrink-0" /> | |
| {data.version.tag} | |
| </Link> | |
| ); | |
| }; | |
| const ReleaseTargetStatus: React.FC<{ releaseTarget: ReleaseTarget }> = ({ | |
| releaseTarget, | |
| }) => { | |
| const { data, isLoading, error } = api.releaseTarget.latestJob.useQuery( | |
| releaseTarget.id, | |
| { refetchInterval: 10_000 }, | |
| ); | |
| const { workspaceSlug } = useParams<{ workspaceSlug: string }>(); | |
| if (isLoading) return <Skeleton className="h-4 w-20" />; | |
| if (error) return <span className="text-xs text-destructive">Error</span>; | |
| if (!data) | |
| return <span className="text-xs text-muted-foreground">Not deployed</span>; | |
| const versionUrl = urls | |
| .workspace(workspaceSlug) | |
| .system(releaseTarget.system.slug) | |
| .deployment(releaseTarget.deployment.slug) | |
| .release(data.version.id) | |
| .jobs(); | |
| return ( | |
| <Link | |
| href={versionUrl} | |
| className={cn( | |
| "flex h-6 items-center gap-1 truncate", | |
| buttonVariants({ variant: "ghost", className: "h-6" }), | |
| )} | |
| > | |
| <JobTableStatusIcon status={data.job.status} className="flex-shrink-0" /> | |
| {data.version.tag} | |
| </Link> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(raw)/[resourceId]/visualize/ResourceNode.tsx
between lines 41 and 74, the ReleaseTargetStatus component lacks error handling
for the query, causing failed requests to show misleading "Not deployed"
messages, and the polling interval is hardcoded to 10 seconds which may be too
frequent for production. To fix this, add error handling logic to display an
appropriate message or UI when the query fails, and modify the polling interval
to be configurable via component props or environment variables so it can be
adjusted based on the deployment environment.
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
1 participant
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.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes