Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. WalkthroughThis update implements a combined version filtering and search feature for deployment versions, introducing a new UI component and associated hook in the web application. Backend logic is refactored to replace custom SQL condition builders with a selector-based approach. Schema and validation layers are extended to support filtering by deployment version name. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeploymentPageContent
participant VersionFilter
participant useVersionSearchQuery
participant API
participant Selector
User->>DeploymentPageContent: Types in search/filter UI
DeploymentPageContent->>useVersionSearchQuery: Update search string (debounced)
useVersionSearchQuery->>DeploymentPageContent: Provide combined selector condition
DeploymentPageContent->>API: Request deployment versions (with selector)
API->>Selector: Build SQL using selector condition (supports name/tag search)
Selector-->>API: SQL fragment
API-->>DeploymentPageContent: Return filtered deployment versions
DeploymentPageContent-->>User: Display filtered results
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/DeploymentPageContent.tsx(5 hunks)packages/api/src/router/deployment-version.ts(1 hunks)packages/api/src/router/environment-page/deployments/deployment-stats.ts(2 hunks)packages/db/src/schema/deployment-version.ts(1 hunks)packages/db/src/selectors/query/deployment-version-selector.ts(1 hunks)packages/validators/src/releases/conditions/comparison-condition.ts(3 hunks)packages/validators/src/releases/conditions/release-condition.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`**/*.ts`: Use TypeScript with explicit types and prefer interfaces for public A...
**/*.ts: Use TypeScript with explicit types and prefer interfaces for public APIs.
Use named imports and group imports by source: standard library first, then external, then internal.
Use consistent type imports: 'import type { Type } from "module"'.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
packages/db/src/selectors/query/deployment-version-selector.tspackages/api/src/router/deployment-version.tspackages/validators/src/releases/conditions/comparison-condition.tspackages/api/src/router/environment-page/deployments/deployment-stats.tspackages/validators/src/releases/conditions/release-condition.tspackages/db/src/schema/deployment-version.ts
`**/*.{js,ts,tsx}`: Formatting must follow Prettier with '@ctrlplane/prettier-config'.
**/*.{js,ts,tsx}: Formatting must follow Prettier with '@ctrlplane/prettier-config'.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
packages/db/src/selectors/query/deployment-version-selector.tspackages/api/src/router/deployment-version.tspackages/validators/src/releases/conditions/comparison-condition.tspackages/api/src/router/environment-page/deployments/deployment-stats.tspackages/validators/src/releases/conditions/release-condition.tspackages/db/src/schema/deployment-version.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/DeploymentPageContent.tsx
`packages/**`: Adhere to file and directory naming conventions specific to each package.
packages/**: Adhere to file and directory naming conventions specific to each package.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
packages/db/src/selectors/query/deployment-version-selector.tspackages/api/src/router/deployment-version.tspackages/validators/src/releases/conditions/comparison-condition.tspackages/api/src/router/environment-page/deployments/deployment-stats.tspackages/validators/src/releases/conditions/release-condition.tspackages/db/src/schema/deployment-version.ts
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/cat...
**/*.{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.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
packages/db/src/selectors/query/deployment-version-selector.tspackages/api/src/router/deployment-version.tspackages/validators/src/releases/conditions/comparison-condition.tspackages/api/src/router/environment-page/deployments/deployment-stats.tspackages/validators/src/releases/conditions/release-condition.tspackages/db/src/schema/deployment-version.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/DeploymentPageContent.tsx
`apps/**`: Adhere to file and directory naming conventions specific to each package.
apps/**: Adhere to file and directory naming conventions specific to each package.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/DeploymentPageContent.tsx
🧠 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#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: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#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: ctrlplanedev/ctrlplane#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: 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/db/src/selectors/query/deployment-version-selector.ts (5)
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: 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#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: 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#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.
packages/api/src/router/deployment-version.ts (6)
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.
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#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#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: 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.
packages/validators/src/releases/conditions/comparison-condition.ts (1)
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.
packages/api/src/router/environment-page/deployments/deployment-stats.ts (10)
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.
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#187
File: apps/jobs/src/ephemeral-env-checker/index.ts:57-0
Timestamp: 2024-10-30T23:10:58.869Z
Learning: In the codebase, deployments are decoupled from environments. When deleting environments (e.g., in `apps/jobs/src/ephemeral-env-checker/index.ts`), associated deployments should not be deleted.
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#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: 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#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#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#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.
packages/validators/src/releases/conditions/release-condition.ts (3)
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: CR
PR: ctrlplanedev/ctrlplane#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T19:43:23.689Z
Learning: Use 'import type' syntax for importing types in TypeScript to improve clarity and enable better tree-shaking.
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.
packages/db/src/schema/deployment-version.ts (10)
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: 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#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#187
File: apps/jobs/src/ephemeral-env-checker/index.ts:57-0
Timestamp: 2024-10-30T23:10:58.869Z
Learning: In the codebase, deployments are decoupled from environments. When deleting environments (e.g., in `apps/jobs/src/ephemeral-env-checker/index.ts`), associated deployments should not be deleted.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#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: 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#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#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#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#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`.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/DeploymentPageContent.tsx (12)
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#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: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#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: ctrlplanedev/ctrlplane#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: 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#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: 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#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: 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, params in dynamic routes must be awaited before accessing their properties in async contexts. For example, `params.deploymentId` should be accessed as `(await params).deploymentId` in async functions.
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.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#187
File: apps/jobs/src/ephemeral-env-checker/index.ts:57-0
Timestamp: 2024-10-30T23:10:58.869Z
Learning: In the codebase, deployments are decoupled from environments. When deleting environments (e.g., in `apps/jobs/src/ephemeral-env-checker/index.ts`), associated deployments should not be deleted.
🧬 Code Graph Analysis (3)
packages/db/src/selectors/query/deployment-version-selector.ts (2)
packages/db/src/common.ts (1)
ColumnOperatorFn(54-65)packages/db/src/schema/deployment-version.ts (1)
deploymentVersion(80-107)
packages/api/src/router/deployment-version.ts (1)
packages/db/src/selectors/index.ts (1)
selector(13-13)
packages/api/src/router/environment-page/deployments/deployment-stats.ts (1)
packages/db/src/selectors/index.ts (1)
selector(13-13)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
🔇 Additional comments (10)
packages/db/src/selectors/query/deployment-version-selector.ts (1)
135-136: LGTM! Consistent implementation of name-based filtering.The new condition type follows the established pattern used for
VersionandTagconditions, properly applying theColumnOperatorFnto thedeploymentVersion.namecolumn.packages/api/src/router/deployment-version.ts (1)
99-99: LGTM! Successful migration to selector-based SQL generation.This change replaces the removed helper function with the new selector approach, which supports the extended condition types including the new name-based filtering.
packages/api/src/router/environment-page/deployments/deployment-stats.ts (2)
6-13: LGTM! Proper import addition for selector functionality.The
selectorimport is correctly added to support the new SQL generation approach.
144-144: LGTM! Consistent migration to selector-based filtering.This change aligns with the broader refactor, replacing the helper function with the new selector approach that supports extended condition types.
packages/validators/src/releases/conditions/comparison-condition.ts (1)
6-6: LGTM! Proper integration of NameCondition into comparison conditions.The changes consistently add
NameConditionsupport to both the Zod schema validation and TypeScript type definitions, following the established pattern for other condition types.Also applies to: 13-13, 30-30, 46-46
packages/validators/src/releases/conditions/release-condition.ts (1)
6-6: LGTM! Comprehensive integration of name-based filtering support.The changes properly extend the deployment version condition system with
NameConditionsupport across all necessary layers - type imports, value imports, union types, Zod schemas, and enums. The implementation follows the established pattern and maintains consistency with existing condition types.Also applies to: 14-14, 26-26, 34-34, 55-55
packages/db/src/schema/deployment-version.ts (1)
1-3: LGTM!The updated imports align with the refactoring from SQL condition helpers to selector-based query generation.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/DeploymentPageContent.tsx (3)
165-188: LGTM!Clean implementation of the version filter component with proper integration of the condition dialog.
190-219: LGTM!Well-structured helper functions with clear separation of concerns for search query detection and creation.
271-272: LGTM!Clean integration of the search functionality with proper state management and UI components.
Also applies to: 285-286
Comment on lines
+220
to
+256
| const useVersionSearchQuery = () => { | ||
| const [search, setSearch] = useState(""); | ||
| const { selector, setSelector } = useDeploymentVersionSelector(); | ||
|
|
||
| const otherConditions = | ||
| selector?.type === DeploymentVersionConditionType.Comparison | ||
| ? selector.conditions.filter((c) => !isSearchQuery(c)) | ||
| : []; | ||
|
|
||
| useDebounce( | ||
| () => { | ||
| if (search === "") { | ||
| if (selector == null || !isComparisonCondition(selector)) return; | ||
| if (otherConditions.length === 0) { | ||
| setSelector(null); | ||
| return; | ||
| } | ||
| setSelector({ | ||
| type: DeploymentVersionConditionType.Comparison, | ||
| operator: ComparisonOperator.And, | ||
| conditions: otherConditions, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| setSelector({ | ||
| type: DeploymentVersionConditionType.Comparison, | ||
| operator: ComparisonOperator.And, | ||
| conditions: [...otherConditions, getSearchCondition(search)], | ||
| }); | ||
| }, | ||
| 500, | ||
| [search], | ||
| ); | ||
|
|
||
| return { search, setSearch }; | ||
| }; |
Contributor
There was a problem hiding this comment.
Fix potential loss of non-comparison conditions and operator preservation.
The current implementation has two issues:
- Non-comparison conditions are lost when adding search
- Original operator is not preserved when removing search
Apply this refactor to handle all condition types and preserve operators:
const useVersionSearchQuery = () => {
const [search, setSearch] = useState("");
const { selector, setSelector } = useDeploymentVersionSelector();
const otherConditions =
selector?.type === DeploymentVersionConditionType.Comparison
? selector.conditions.filter((c) => !isSearchQuery(c))
- : [];
+ : selector ? [selector] : [];
+
+ const originalOperator =
+ selector?.type === DeploymentVersionConditionType.Comparison
+ ? selector.operator
+ : ComparisonOperator.And;
useDebounce(
() => {
if (search === "") {
if (selector == null || !isComparisonCondition(selector)) return;
if (otherConditions.length === 0) {
setSelector(null);
return;
}
+ if (otherConditions.length === 1) {
+ setSelector(otherConditions[0]);
+ return;
+ }
setSelector({
type: DeploymentVersionConditionType.Comparison,
- operator: ComparisonOperator.And,
+ operator: originalOperator,
conditions: otherConditions,
});
return;
}
setSelector({
type: DeploymentVersionConditionType.Comparison,
operator: ComparisonOperator.And,
conditions: [...otherConditions, getSearchCondition(search)],
});
},
500,
[search],
);
return { search, setSearch };
};📝 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 useVersionSearchQuery = () => { | |
| const [search, setSearch] = useState(""); | |
| const { selector, setSelector } = useDeploymentVersionSelector(); | |
| const otherConditions = | |
| selector?.type === DeploymentVersionConditionType.Comparison | |
| ? selector.conditions.filter((c) => !isSearchQuery(c)) | |
| : []; | |
| useDebounce( | |
| () => { | |
| if (search === "") { | |
| if (selector == null || !isComparisonCondition(selector)) return; | |
| if (otherConditions.length === 0) { | |
| setSelector(null); | |
| return; | |
| } | |
| setSelector({ | |
| type: DeploymentVersionConditionType.Comparison, | |
| operator: ComparisonOperator.And, | |
| conditions: otherConditions, | |
| }); | |
| return; | |
| } | |
| setSelector({ | |
| type: DeploymentVersionConditionType.Comparison, | |
| operator: ComparisonOperator.And, | |
| conditions: [...otherConditions, getSearchCondition(search)], | |
| }); | |
| }, | |
| 500, | |
| [search], | |
| ); | |
| return { search, setSearch }; | |
| }; | |
| const useVersionSearchQuery = () => { | |
| const [search, setSearch] = useState(""); | |
| const { selector, setSelector } = useDeploymentVersionSelector(); | |
| const otherConditions = | |
| selector?.type === DeploymentVersionConditionType.Comparison | |
| ? selector.conditions.filter((c) => !isSearchQuery(c)) | |
| : selector | |
| ? [selector] | |
| : []; | |
| const originalOperator = | |
| selector?.type === DeploymentVersionConditionType.Comparison | |
| ? selector.operator | |
| : ComparisonOperator.And; | |
| useDebounce( | |
| () => { | |
| if (search === "") { | |
| if (selector == null || !isComparisonCondition(selector)) return; | |
| if (otherConditions.length === 0) { | |
| setSelector(null); | |
| return; | |
| } | |
| if (otherConditions.length === 1) { | |
| setSelector(otherConditions[0]); | |
| return; | |
| } | |
| setSelector({ | |
| type: DeploymentVersionConditionType.Comparison, | |
| operator: originalOperator, | |
| conditions: otherConditions, | |
| }); | |
| return; | |
| } | |
| setSelector({ | |
| type: DeploymentVersionConditionType.Comparison, | |
| operator: ComparisonOperator.And, | |
| conditions: [...otherConditions, getSearchCondition(search)], | |
| }); | |
| }, | |
| 500, | |
| [search], | |
| ); | |
| return { search, setSearch }; | |
| }; |
🤖 Prompt for AI Agents
In
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/DeploymentPageContent.tsx
between lines 220 and 256, the useVersionSearchQuery hook currently loses
non-comparison conditions when adding a search condition and does not preserve
the original operator when removing the search. To fix this, update the logic to
retain all existing conditions that are not search queries regardless of the
selector type, and when modifying the selector, preserve the original operator
instead of defaulting to ComparisonOperator.And. This ensures all condition
types are handled correctly and the operator remains consistent.
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
Improvements