chore: version dependency ui #630
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis change introduces backend and frontend support for evaluating and displaying blocking version dependencies in policy evaluations. It adds new helper functions and a UI badge component to show missing dependencies, enriches policy evaluation logic with dependency context, and updates component props and data flows to support these features. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as PolicyEvaluationsCell
participant API as API /evaluateReleaseTarget
participant DB as Database
UI->>API: Request policy evaluation for resource/version
API->>DB: Fetch policy, version, and release target data
API->>DB: Query version dependencies and related resources
API-->>UI: Return evaluation results with version dependency info
UI->>UI: Compute blocking dependencies
UI->>UI: Render VersionDependencyBadge if blocking dependencies exist
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/PolicyEvaluationsCell.tsx(6 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/ReleaseTargetRow.tsx(2 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/policy-evaluations/VersionDependencyBadge.tsx(1 hunks)packages/api/src/router/policy/evaluate.ts(5 hunks)packages/rule-engine/src/manager/version-manager-rules/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
packages/rule-engine/src/manager/version-manager-rules/index.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/ReleaseTargetRow.tsxapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/policy-evaluations/VersionDependencyBadge.tsxpackages/api/src/router/policy/evaluate.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/PolicyEvaluationsCell.tsx
⚙️ CodeRabbit Configuration File
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
packages/rule-engine/src/manager/version-manager-rules/index.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/ReleaseTargetRow.tsxapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/policy-evaluations/VersionDependencyBadge.tsxpackages/api/src/router/policy/evaluate.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/PolicyEvaluationsCell.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
packages/rule-engine/src/manager/version-manager-rules/index.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/ReleaseTargetRow.tsxapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/policy-evaluations/VersionDependencyBadge.tsxpackages/api/src/router/policy/evaluate.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/PolicyEvaluationsCell.tsx
🧠 Learnings (6)
📓 Common learnings
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/rule-engine/src/manager/version-manager-rules/index.ts (4)
Learnt from: adityachoudhari26
PR: #604
File: packages/rule-engine/src/manager/version-manager.ts:124-139
Timestamp: 2025-06-30T21:19:43.866Z
Learning: In packages/rule-engine/src/manager/version-manager.ts, the findDesiredVersion method should use takeFirst (not takeFirstOrNull) because if a desiredVersionId is set but the query fails to find exactly one matching version, it indicates a data integrity or configuration issue that should fail loudly rather than be handled silently.
Learnt from: CR
PR: ctrlplanedev/ctrlplane#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T22:34:20.754Z
Learning: Applies to **/*.{ts,tsx} : Consistent type imports: import type { Type } from "module"
Learnt from: CR
PR: ctrlplanedev/ctrlplane#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T22:34:20.754Z
Learning: Applies to **/*.{ts,tsx} : Import styles: Use named imports, group imports by source (std lib > external > internal)
Learnt from: CR
PR: ctrlplanedev/ctrlplane#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T22:34:20.754Z
Learning: Applies to **/*.{ts,tsx} : Prefer async/await over raw promises
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/ReleaseTargetRow.tsx (4)
Learnt from: adityachoudhari26
PR: #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: #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: #627
File: apps/event-worker/src/workers/job-update/trigger-dependendent-targets.ts:0-0
Timestamp: 2025-07-23T22:42:20.933Z
Learning: In apps/event-worker/src/workers/job-update/trigger-dependendent-targets.ts, when implementing error handling for the triggerDependentTargets function, only a try-catch wrapper is needed around the entire function body. Additional null checks after takeFirst() calls are unnecessary since takeFirst throws an error if no record is found.
Learnt from: adityachoudhari26
PR: #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.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/policy-evaluations/VersionDependencyBadge.tsx (6)
Learnt from: adityachoudhari26
PR: #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: #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: zacharyblasczyk
PR: #408
File: apps/webservice/src/components/form/job-agent/JobAgentJenkinsPipelineConfig.tsx:26-31
Timestamp: 2025-04-12T22:08:13.790Z
Learning: For Jenkins job configuration, two approaches are needed: (1) a simple URL input form for airgapped environments (current focus) and (2) a dropdown selection interface for non-airgapped environments where the Jenkins server is accessible. A component similar to DeploymentJobAgentGithubConfig.tsx is preferred.
Learnt from: zacharyblasczyk
PR: #408
File: apps/webservice/src/components/form/job-agent/JobAgentJenkinsPipelineConfig.tsx:26-31
Timestamp: 2025-04-12T22:08:13.790Z
Learning: For Jenkins job configuration, two approaches are needed: (1) a simple URL input form for airgapped environments (current focus) and (2) a dropdown selection interface for non-airgapped environments where the Jenkins server is accessible. A component similar to DeploymentJobAgentGithubConfig.tsx is preferred.
Learnt from: adityachoudhari26
PR: #183
File: apps/webservice/src/app/[workspaceSlug]/_components/environment-drawer/Overview.tsx:46-57
Timestamp: 2024-10-30T00:03:57.878Z
Learning: Accessibility and validation feedback improvements are not desired for form fields in the Overview component within apps/webservice/src/app/[workspaceSlug]/_components/environment-drawer/Overview.tsx.
Learnt from: adityachoudhari26
PR: #188
File: apps/webservice/src/app/[workspaceSlug]/_components/environment-drawer/EnvironmentDrawer.tsx:139-154
Timestamp: 2024-11-01T02:36:23.101Z
Learning: In EnvironmentDrawer.tsx, for the EnvironmentDrawer component, when there are only a few TabButton instances (e.g., 3), it's acceptable to define them individually without extracting them into a configuration to reduce repetition.
packages/api/src/router/policy/evaluate.ts (11)
Learnt from: adityachoudhari26
PR: #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: #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: #604
File: packages/rule-engine/src/manager/version-manager.ts:124-139
Timestamp: 2025-06-30T21:19:43.866Z
Learning: In packages/rule-engine/src/manager/version-manager.ts, the findDesiredVersion method should use takeFirst (not takeFirstOrNull) because if a desiredVersionId is set but the query fails to find exactly one matching version, it indicates a data integrity or configuration issue that should fail loudly rather than be handled silently.
Learnt from: adityachoudhari26
PR: #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: #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: #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: #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: #181
File: packages/auth/src/utils/rbac.ts:102-118
Timestamp: 2024-10-29T02:05:46.185Z
Learning: The releaseChannel scope type is included in the scopeType enum in packages/db/src/schema/rbac.ts.
Learnt from: adityachoudhari26
PR: #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: #515
File: apps/event-worker/src/workers/compute-systems-release-targets.ts:86-110
Timestamp: 2025-04-28T18:38:21.163Z
Learning: In SQL queries that use inArray() with arrays like deploymentIds or environmentIds, if these arrays are empty, it will generate an invalid IN () clause that PostgreSQL rejects. Adding condition checks (e.g., if (array.length > 0)) before executing such queries prevents SQL syntax errors.
Learnt from: adityachoudhari26
PR: #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.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/PolicyEvaluationsCell.tsx (4)
Learnt from: adityachoudhari26
PR: #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: #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: #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: #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.
🔇 Additional comments (7)
packages/rule-engine/src/manager/version-manager-rules/index.ts (1)
5-5: LGTM!The new export follows the existing pattern and properly exposes the version dependency module.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/ReleaseTargetRow.tsx (1)
176-176: Props correctly extended to support version dependenciesThe version prop type extension and the addition of the resource prop to PolicyEvaluationsCell are properly implemented to support the new version dependency feature.
Also applies to: 225-226
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/policy-evaluations/VersionDependencyBadge.tsx (1)
1-171: Well-structured component with proper edge case handlingThe VersionDependencyBadge component is well-implemented with:
- Clear separation of concerns through sub-components
- Proper handling of edge cases (no resources, single vs multiple resources)
- Good use of TypeScript types and UI primitives
- Accessible dialog implementation
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(raw)/releases/[releaseId]/(sidebar)/jobs/_components/PolicyEvaluationsCell.tsx (2)
213-219: Version dependency integration properly implementedThe component correctly:
- Extracts blocking version dependencies from policy evaluations
- Updates props to accept enriched resource and version data
- Integrates dependencies into the blocking condition checks
- Renders the VersionDependencyBadge when dependencies are blocking
Also applies to: 222-226, 250-259, 304-310
119-124: Rollout logic changes look intentionalThe updated logic properly handles:
- Removing the
passingflag when rolloutTime is null- Returning null when the current time is after the rollout time
This appears to be an intentional refinement of the rollout behavior.
packages/api/src/router/policy/evaluate.ts (2)
68-71: Async rule getter support properly implementedThe function signature and implementation correctly handle both synchronous and asynchronous rule getters.
Also applies to: 75-75
355-359: Version dependency evaluation correctly integratedThe version dependency evaluation is properly added to the releaseTarget evaluation flow and included in the response.
Also applies to: 385-385
Summary by CodeRabbit