feat: support metadata match rule with different keys #617
Conversation
WalkthroughThis change converts the handling of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (Dialog)
participant API
participant DB
User->>UI (Dialog): Fill in sourceKey/targetKey pairs
UI (Dialog)->>API: Submit rule with [{sourceKey, targetKey}, ...]
API->>DB: Store each pair as source_key and target_key
DB-->>API: Confirm insertion
API-->>UI (Dialog): Respond with success
UI (Dialog)-->>User: Show updated rule
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}Instructions used from: Sources:
⚙️ CodeRabbit Configuration File **/*.{js,jsx,ts,tsx}Instructions used from: Sources:
**/*.{js,jsx,ts,tsx,json,css,md,yml,yaml}Instructions used from: Sources:
🧠 Learnings (1)📓 Common learnings⏰ 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). (6)
🔇 Additional comments (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}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
e2e/tests/api/resource-relationships.spec.ts (1)
192-192: Fix inconsistent property name and format.This line uses the old format and property name, which is inconsistent with the schema changes throughout the PR. The property should be
metadataKeysMatches(plural) and should use the new object format.Apply this diff to fix the inconsistency:
- metadataKeysMatch: ["e2e/test", "additional-key"], + metadataKeysMatches: [ + { sourceKey: "e2e/test", targetKey: "e2e/test" }, + { sourceKey: "additional-key", targetKey: "additional-key" } + ],packages/db/src/queries/relationships/get-resource-parents.ts (1)
53-53: Fix typo in variable nameThe variable name has a typo:
relatipnshipTargetsshould berelationshipTargets.- const relatipnshipTargets = async () => + const relationshipTargets = async () =>
🧹 Nitpick comments (1)
packages/logger/src/index.ts (1)
11-11: Shadowedlevelidentifier slightly hurts readability
Theleveldestructured frominfohides thelevelparameter ofcreateLogger. No functional bug, but renaming the outer param (e.g.logLevel) or the inner variable would avoid cognitive overhead.Also applies to: 17-17
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/CreateRelationshipDialog.tsx(5 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/EditRelationshipDialog.tsx(4 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/RelationshipRulesTable.tsx(1 hunks)apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/route.ts(2 hunks)apps/webservice/src/app/api/v1/resource-relationship-rules/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/resource-relationship-rules/route.ts(1 hunks)e2e/api/schema.ts(8 hunks)e2e/tests/api/resource-relationships.spec.ts(4 hunks)e2e/tests/api/resource-variables.spec.ts(4 hunks)openapi.v1.json(1 hunks)packages/api/src/router/resource-relationship-rules.ts(2 hunks)packages/db/drizzle/0125_amused_night_nurse.sql(1 hunks)packages/db/drizzle/meta/_journal.json(1 hunks)packages/db/src/queries/relationships/get-resource-children.ts(1 hunks)packages/db/src/queries/relationships/get-resource-parents.ts(2 hunks)packages/db/src/queries/relationships/queries.ts(1 hunks)packages/db/src/schema/resource-relationship-rule.ts(2 hunks)packages/logger/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx,json,css,md,yml,yaml}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.{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
🧠 Learnings (15)
📓 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/logger/src/index.ts (1)
Learnt from: CR
PR: ctrlplanedev/ctrlplane#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T21:36:42.935Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript with explicit types (prefer interfaces for public APIs)
apps/webservice/src/app/api/v1/resource-relationship-rules/route.ts (3)
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#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.
e2e/tests/api/resource-variables.spec.ts (2)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#601
File: e2e/tests/api/policies/retry-policy.spec.ts:96-99
Timestamp: 2025-06-24T23:53:36.327Z
Learning: In e2e test files, prefer explicit null safety checks and validation over non-null assertions (!). When validating API responses in tests, use type guards and throw descriptive errors rather than assuming values are always present.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#517
File: e2e/tests/api/deployment-variable.spec.ts:70-76
Timestamp: 2025-04-28T21:59:04.723Z
Learning: In the ctrlplane e2e tests, tests can be isolated between runs using prefixed systems with random IDs in YAML templates, but tests within the same file may still need to handle isolation between test cases if they operate on the same resources.
e2e/tests/api/resource-relationships.spec.ts (2)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#601
File: e2e/tests/api/policies/retry-policy.spec.ts:96-99
Timestamp: 2025-06-24T23:53:36.327Z
Learning: In e2e test files, prefer explicit null safety checks and validation over non-null assertions (!). When validating API responses in tests, use type guards and throw descriptive errors rather than assuming values are always present.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#517
File: e2e/tests/api/deployment-variable.spec.ts:70-76
Timestamp: 2025-04-28T21:59:04.723Z
Learning: In the ctrlplane e2e tests, tests can be isolated between runs using prefixed systems with random IDs in YAML templates, but tests within the same file may still need to handle isolation between test cases if they operate on the same resources.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/RelationshipRulesTable.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.
packages/api/src/router/resource-relationship-rules.ts (4)
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#236
File: packages/api/src/router/workspace-integrations.ts:222-231
Timestamp: 2024-12-02T01:47:59.631Z
Learning: In `packages/api/src/router/workspace-integrations.ts`, when creating IAM policies for roles that need to assume customer-provided roles with unknown ARNs, it's acceptable to keep `Resource` as `"*"` in the policy document, while limiting the `Action` to `"sts:AssumeRole"`, since ARNs are not known in advance.
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#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.
packages/db/src/queries/relationships/get-resource-parents.ts (1)
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.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/EditRelationshipDialog.tsx (5)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#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: 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#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/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: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#186
File: apps/webservice/src/app/api/v1/targets/[targetId]/route.ts:28-30
Timestamp: 2024-10-30T20:37:42.751Z
Learning: When adding input validation for `targetId`, consider whether it's worth extracting into a reusable component based on its usage across the codebase.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/CreateRelationshipDialog.tsx (6)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#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: 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#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/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: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#186
File: apps/webservice/src/app/api/v1/targets/[targetId]/route.ts:28-30
Timestamp: 2024-10-30T20:37:42.751Z
Learning: When adding input validation for `targetId`, consider whether it's worth extracting into a reusable component based on its usage across the codebase.
packages/db/src/queries/relationships/get-resource-children.ts (3)
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#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.
apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/route.ts (3)
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#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.
packages/db/src/schema/resource-relationship-rule.ts (3)
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#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#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.
e2e/api/schema.ts (9)
undefined
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: adityachoudhari26
PR: #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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: adityachoudhari26
PR: #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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: adityachoudhari26
PR: #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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
packages/db/src/queries/relationships/queries.ts (4)
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#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.
🧬 Code Graph Analysis (1)
packages/db/src/queries/relationships/queries.ts (1)
packages/db/src/common.ts (1)
Tx(22-22)
⏰ 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). (4)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
🔇 Additional comments (34)
packages/logger/src/index.ts (1)
16-16: Explicit typing forinfoimproves type-safety
Addingwinston.Logform.TransformableInfomakes the callback’s contract clear and lets TS catch mis-destructuring errors. 👍packages/db/drizzle/meta/_journal.json (1)
880-886: LGTM!The migration journal entry is correctly formatted and aligns with the database schema changes for the metadata key structure update.
apps/webservice/src/app/api/v1/resource-relationship-rules/route.ts (1)
65-66: LGTM!The database insertion logic is correctly updated to handle the new
sourceKeyandtargetKeystructure for metadata key matches.apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/RelationshipRulesTable.tsx (1)
65-67: LGTM!The UI correctly displays the new metadata key structure with a clear "sourceKey matches targetKey" format and uses a proper composite React key.
e2e/tests/api/resource-variables.spec.ts (4)
231-233: LGTM!The e2e tests are correctly updated to use the new
metadataKeysMatchesobject structure withsourceKeyandtargetKeyproperties.
321-323: LGTM!Consistent with the schema change to use
sourceKeyandtargetKeyobjects.
478-480: LGTM!Properly updated to match the new metadata key structure.
654-656: LGTM!Final instance correctly updated to use the new object format.
e2e/tests/api/resource-relationships.spec.ts (4)
45-45: LGTM! Correct implementation of the new metadata key matching format.The change from array of strings to objects with
sourceKeyandtargetKeyproperties aligns with the schema updates described in the PR.
64-64: LGTM! Consistent with the new schema.Correctly using the new object format for metadata key matching.
166-166: LGTM! Proper use of the new metadata key matching format.The object structure with
sourceKeyandtargetKeyis correctly implemented.
259-262: LGTM! Excellent test coverage for multiple metadata key matches.The test correctly demonstrates multiple metadata key matching rules using the new format, which helps validate that both rules must be satisfied for the relationship to be established.
packages/db/drizzle/0125_amused_night_nurse.sql (1)
1-17: LGTM! Well-structured migration strategy.The migration follows safe database migration practices by:
- Adding the new column as nullable first
- Populating existing data before making it NOT NULL
- Properly updating the unique index to reflect the new schema
This approach minimizes downtime and ensures data consistency during the transition.
packages/api/src/router/resource-relationship-rules.ts (2)
76-80: LGTM! Correct destructuring for the new key pair structure.The changes properly destructure
sourceKeyandtargetKeyfrom each metadata match item and map them correctly to the database schema.
137-141: LGTM! Consistent handling in update mutation.The update logic correctly mirrors the create mutation pattern, ensuring consistent behavior across both operations.
apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/route.ts (2)
21-21: LGTM! Function parameter updated to support key pairs.The parameter type correctly reflects the new structure with
sourceKeyandtargetKeyproperties.
32-42: LGTM! Proper deduplication and database insertion logic.The composite key approach for deduplication (
${sourceKey}-${targetKey}) is appropriate, and the database insertion correctly maps both fields to the new schema.packages/db/src/queries/relationships/get-resource-children.ts (2)
5-12: LGTM! Good refactoring to use modular query components.Extracting query fragments and aliases to a shared module improves maintainability and reduces code duplication.
20-23: LGTM! Consistent use of imported query functions.The imported query functions maintain the same functionality while supporting the new metadata key pair structure.
apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/EditRelationshipDialog.tsx (4)
54-62: LGTM! Form schema correctly updated for key pairs.The schema properly validates the new
sourceKeyandtargetKeystructure, and the simplified default values approach works well with the updated data format.
89-91: LGTM! Simplified form submission handler.The direct data passing without manual remapping is cleaner and less error-prone than the previous approach.
313-336: LGTM! Intuitive UI for key pair input.The dual input fields with "matches" text clearly convey the relationship between source and target keys, providing good user experience.
356-358: LGTM! Add button correctly creates key pair objects.The button properly initializes both
sourceKeyandtargetKeyas empty strings, maintaining consistency with the new data structure.packages/db/src/queries/relationships/get-resource-parents.ts (1)
115-128: LGTM! Metadata structure updated correctlyThe function now returns
metadataKeysas an array of objects withsourceKeyandtargetKeyproperties, which aligns with the schema changes across the codebase.apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/relationship-rules/components/CreateRelationshipDialog.tsx (2)
49-54: LGTM! UI properly updated for source/target key pairsThe form schema and UI components correctly handle the new metadata match structure with separate
sourceKeyandtargetKeyfields. The implementation provides a clear user interface with "matches" text between the two inputs.Also applies to: 328-352, 372-374
111-111: Good UX improvement for dialog scrollabilityAdding
max-h-[90vh]andoverflow-y-autoensures the dialog remains usable even with many form fields.packages/db/src/schema/resource-relationship-rule.ts (2)
129-139: LGTM! Schema correctly updated for key pairsThe table schema properly splits the single
keycolumn intosourceKeyandtargetKey, with the unique index correctly enforcing uniqueness on the combination of(resourceRelationshipRuleId, sourceKey, targetKey).
193-200: Well-structured validation with clear error messagesThe Zod schema validation for
metadataKeysMatchesprovides appropriate validation for both source and target keys with clear, specific error messages.packages/db/src/queries/relationships/queries.ts (3)
8-11: Good use of aliases to prevent naming conflictsCreating aliases for tables that are joined multiple times is a best practice that prevents naming conflicts and makes the queries more readable.
26-53: Well-structured metadata matching queriesThe
metadataPairSameKeySameValueandunsatisfiedMetadataMatchRulefunctions correctly implement the logic for checking if source and target metadata match according to the new key pair structure. The use ofnotExistsfor finding unsatisfied rules is appropriate.
88-103: Clean separation of matching conditionsExporting
ruleMatchesSourceandruleMatchesTargetas reusable condition arrays promotes consistency across queries and makes the code more maintainable.e2e/api/schema.ts (3)
200-216: LGTM! New API endpoints follow established patterns.The three new API endpoints (
listDeploymentVersions,getLatestJobs,getReleaseTarget) are well-structured and consistent with existing endpoint definitions. The path parameter naming and operation references follow the established conventions.Also applies to: 479-495, 513-529
1133-1138: Schema enhancements align with PR objectives.The schema changes are well-designed:
JobWithTriggerextension with optionallinksandmetadatafields maintains backwards compatibility- The
metadataKeysMatchestransformation fromstring[]to objects withsourceKeyandtargetKeyproperties enables the more sophisticated metadata matching functionality described in the PR titleThese changes provide enhanced functionality while preserving type safety.
Also applies to: 1565-1568
2350-2395: Operation definitions are well-structured and complete.The three new operation definitions demonstrate good API design:
listDeploymentVersionsincludes useful optional status filtering- All operations have appropriate HTTP status codes (200, 404, 500) with proper error response schemas
- Parameter definitions and response types are consistent with existing patterns
- Error messages are descriptive and user-friendly
These operations provide the necessary functionality while maintaining consistency with the existing API structure.
Also applies to: 3333-3379, 3424-3456
| "metadataKeysMatches": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| "type": "object", | ||
| "properties": { | ||
| "sourceKey": { | ||
| "type": "string" | ||
| }, | ||
| "targetKey": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "sourceKey", | ||
| "targetKey" | ||
| ] | ||
| } |
There was a problem hiding this comment.
Inconsistent schema upgrade – metadataKeysMatches updated only for Create flow
You switched metadataKeysMatches from string to { sourceKey, targetKey } objects here, but the companion schemas still expose the old shape:
UpdateResourceRelationshipRule.metadataKeysMatch(singular) is stillstring[].ResourceRelationshipRule.metadataKeysMatches(plural) is stillstring[].
Calling POST /v1/resource-relationship-rules will accept the new structure, but the subsequent PATCH or GET responses will not validate/serialise the same payload, breaking round-trips and client typings.
Suggested fix (partial diff – adjust similar blocks in both schemas):
-"metadataKeysMatch": {
- "type": "array",
- "items": { "type": "string" }
-},
+"metadataKeysMatches": { # keep plural for consistency
+ "type": "array",
+ "items": {
+ "type": "object",
+ "properties": {
+ "sourceKey": { "type": "string" },
+ "targetKey": { "type": "string" }
+ },
+ "required": [ "sourceKey", "targetKey" ]
+ }
+},Then update ResourceRelationshipRule likewise so that GET/PUT/POST use a single, consistent contract.
Failing to do so will surface as 400/500 validation errors in production once clients start using the new feature.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In openapi.v1.json around lines 6788 to 6804, the schema for metadataKeysMatches
was changed from an array of strings to an array of objects with sourceKey and
targetKey only in the Create flow. To fix this inconsistency, update the
UpdateResourceRelationshipRule.metadataKeysMatch and
ResourceRelationshipRule.metadataKeysMatches schemas to match the new object
structure, ensuring all related schemas use the same format for this field to
maintain consistent validation and serialization across POST, PATCH, and GET
operations.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores