feat(orm): support where enum #544
Conversation
📝 WalkthroughWalkthroughAdds an override of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/orm/src/client/executor/name-mapper.ts (1)
207-207: Optional: Verify return types of enum mapping helpers.The type casts to
OperationNodeat line 207 and the implicit type assumptions at line 210 rely on the return types ofprocessEnumMappingForValueandprocessEnumMappingForValues. These should be safe given the existing usage, but confirming the type signatures match expectations would improve type safety.Also applies to: 210-210
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/orm/src/client/executor/name-mapper.tstests/e2e/orm/client-api/name-mapping.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/client-api/name-mapping.test.ts
🧠 Learnings (3)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/e2e/orm/client-api/name-mapping.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/e2e/orm/client-api/name-mapping.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
tests/e2e/orm/client-api/name-mapping.test.tspackages/orm/src/client/executor/name-mapper.ts
⏰ 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). (2)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: build-test (22.x, postgresql)
🔇 Additional comments (2)
packages/orm/src/client/executor/name-mapper.ts (1)
188-218: LGTM! Enum value mapping in WHERE clauses correctly implemented.The
transformWhereoverride properly handles the two key patterns from issue #541:
- Direct equality:
where: { role: UserRole.ADMIN }- IN conditions:
where: { role: { in: [UserRole.ADMIN] } }The implementation correctly:
- Detects binary operations with table-qualified column references
- Delegates to existing enum mapping helpers
- Falls back to base implementation for non-matching patterns
Note on scope: This implementation specifically handles table-qualified column references (e.g.,
User.role). Unqualified references in raw Kysely queries would bypass this transformation, which is expected behavior since the ORM-generated queries always include table qualifiers.tests/e2e/orm/client-api/name-mapping.test.ts (1)
49-57: Excellent test coverage for enum WHERE clause mapping!The test additions comprehensively validate the fix for issue #541:
- Raw queries (lines 49-57, 79-86) verify that mapped enum values are correctly stored in the database (
role_uservsMODERATOR)- ORM queries (lines 168-212) test the transformation logic for both:
- Exact match filtering:
where: { role: 'USER' }- IN operator filtering:
where: { role: { in: ['USER'] } }- Both
findFirstandfindManyoperations are exercisedThe tests correctly demonstrate that the ORM now maps enum identifiers (e.g.,
USER) to their database representations (e.g.,role_user) in WHERE clauses, resolving the reported issue.Also applies to: 79-86, 168-212
DoctorFTB
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/orm/src/client/executor/name-mapper.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
packages/orm/src/client/executor/name-mapper.ts
⏰ 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). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (1)
packages/orm/src/client/executor/name-mapper.ts (1)
4-4: LGTM! Necessary imports for WHERE clause transformation.The new imports from kysely are correctly added to support the
transformWheremethod implementation.Also applies to: 14-14, 22-22, 28-28
DoctorFTB
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/orm/src/client/executor/name-mapper.ts (1)
187-235: Well-structured enum mapping for WHERE clause binary operations.The implementation correctly:
- Detects the pattern (qualified/unqualified column reference with enum value or list)
- Resolves the field's model from scopes (handles both qualified and unqualified references)
- Applies enum mapping via existing helper methods
- Falls back gracefully when the pattern doesn't match
Edge cases are handled defensively—if the field or enum isn't found, the original value is returned.
📝 Optional: Add JSDoc for clarity
Consider adding a brief JSDoc comment above the method to document the supported patterns:
+ /** + * Transforms binary operations to map enum values in WHERE clauses. + * Supports: + * - Single enum values: column = EnumValue + * - IN lists: column IN [EnumValue1, EnumValue2] + * - Both qualified (table.column) and unqualified (column) references + * + * Nested AND/OR conditions are handled automatically through recursive transformation. + */ protected override transformBinaryOperation(node: BinaryOperationNode) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/orm/src/client/executor/name-mapper.tstests/e2e/orm/client-api/name-mapping.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/client-api/name-mapping.test.ts
🧠 Learnings (4)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/e2e/orm/client-api/name-mapping.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/e2e/orm/client-api/name-mapping.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
tests/e2e/orm/client-api/name-mapping.test.tspackages/orm/src/client/executor/name-mapper.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/orm/src/client/executor/name-mapper.ts
⏰ 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). (2)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: build-test (22.x, postgresql)
🔇 Additional comments (3)
packages/orm/src/client/executor/name-mapper.ts (1)
4-4: LGTM: Import additions support the new transformation logic.The new imports (
BinaryOperationNode,type OperationNode,type SimpleReferenceExpressionNode) are appropriate for thetransformBinaryOperationoverride.Also applies to: 14-14, 22-22
tests/e2e/orm/client-api/name-mapping.test.ts (2)
11-14: Good simplification: Removed unnecessary type cast.Directly awaiting
createTestClientwithout theas anycast improves type safety.
49-57: Excellent test coverage for enum mapping in WHERE clauses.The test additions comprehensively verify:
- ✅ Raw queries with both mapped (
'role_user') and unmapped ('MODERATOR') enum values- ✅ ORM queries with direct equality (
where: { role: 'USER' })- ✅ ORM queries with IN operators (
where: { role: { in: ['USER'] } })- ✅ Complex logical conditions (
AND/ORcombinations)- ✅ Query builder with unqualified column refs (
where('role', '=', 'USER'))- ✅ Query builder with qualified/aliased refs (
where('u.role', '=', 'USER'))- ✅ Enum value lists (
where('role', 'in', ['USER', 'ADMIN']))This validates round-trip consistency (create with enum value → raw read confirms database storage) and exercises all code paths in the new
transformBinaryOperationmethod. As per coding guidelines, these tests validate real-world schema compatibility.Also applies to: 79-86, 168-224, 265-295
|
Hi @DoctorFTB , thanks for working on this. I've made a change to resolve a column reference's model name from the scopes (generated during traversing the Kysely AST tree), so that it doesn't rely on the column explicitly having a table name qualification. |
|
Hey! LGTM. Thanks |
2 participants
Use transformWhere to handle mapping of enum values
Closes #541
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.