Conversation
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
…reSQL schemas (#2606) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a regression test for nested collection predicates in access policies generating valid SQL (issue #2538). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis pull request performs a monorepo-wide version bump from 3.6.1 to 3.6.2, adds PostgreSQL schema support for implicit many-to-many join tables, updates policy handler logic for delegate sub-models, declares h3 as an optional peer dependency, and introduces three regression test cases. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
|
Claude finished @ymc9's task —— View job Code Review for v3.6.2 Release PR ✅Comprehensive review completed for this merge from dev to main for v3.6.2 release. Review Tasks
✅ APPROVE - This PR is ready for mergeKey Functional Changes1. PostgreSQL Schema Support Enhancement (packages/orm/src/client/executor/name-mapper.ts:87-102)
2. Delegate Sub-model Policy Enhancement (packages/plugins/policy/src/policy-handler.ts:590-618)
Regression Test Coverage ✅3 comprehensive regression tests added:
Code Quality Assessment ✅Strengths:
Performance Considerations ✅
Security Review ✅
Version Management ✅
Test Coverage ✅
Recommendations
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/orm/src/client/executor/name-mapper.ts (1)
567-585:⚠️ Potential issue | 🟡 MinorSubtle behavior change: unknown tables now force
'public'instead ofdefaultSchema.Previously,
getTableSchema(model)would fall through todefaultSchema ?? 'public'for any name, including tables not in$schema.models. Now, any name not in$schema.modelsreturnsjoinTableSchemaMap.get(model) ?? 'public', ignoringdefaultSchema.For implicit m2m join tables this is correct (they're precomputed). But for any other non-model table name that might flow through
processTableRef(e.g., raw-query table refs, future extensions, or any model name that doesn't exist as a key for some reason), the schema will now incorrectly default to'public'instead of the user-configureddefaultSchema. Consider falling back todefaultSchemawhen the name is not injoinTableSchemaMap:Suggested fallback
if (!this.schema.models[model]) { - return this.joinTableSchemaMap.get(model) ?? 'public'; + return this.joinTableSchemaMap.get(model) ?? this.schema.provider.defaultSchema ?? 'public'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orm/src/client/executor/name-mapper.ts` around lines 567 - 585, The getTableSchema function currently returns joinTableSchemaMap.get(model) ?? 'public' when model is not found in this.schema.models, which ignores the configured defaultSchema; update getTableSchema (in name-mapper.ts) so that for non-model names you first check joinTableSchemaMap.get(model) and if that is undefined you fall back to this.schema.provider.defaultSchema ?? 'public' instead of always 'public'. Keep the existing logic for models (respecting @@schema map args) and only change the fallback path for the not-in-this.schema.models branch to use defaultSchema.
🧹 Nitpick comments (1)
tests/regression/test/issue-2603.test.ts (1)
8-8: Nit: describe title missing#for consistency.The sibling regression tests use
Regression for issue#2538`` /#2595; consider aligning to `'Regression for issue `#2603`'`.Proposed tweak
-describe('Regression for issue 2603', () => { +describe('Regression for issue `#2603`', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/regression/test/issue-2603.test.ts` at line 8, Update the test suite title string in the describe call so it matches the sibling tests' format by adding a '#' before the issue number; change the describe labeled "Regression for issue 2603" to "Regression for issue `#2603`" (locate the describe invocation in tests/regression/test/issue-2603.test.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/orm/src/client/executor/name-mapper.ts`:
- Around line 567-585: The getTableSchema function currently returns
joinTableSchemaMap.get(model) ?? 'public' when model is not found in
this.schema.models, which ignores the configured defaultSchema; update
getTableSchema (in name-mapper.ts) so that for non-model names you first check
joinTableSchemaMap.get(model) and if that is undefined you fall back to
this.schema.provider.defaultSchema ?? 'public' instead of always 'public'. Keep
the existing logic for models (respecting @@schema map args) and only change the
fallback path for the not-in-this.schema.models branch to use defaultSchema.
---
Nitpick comments:
In `@tests/regression/test/issue-2603.test.ts`:
- Line 8: Update the test suite title string in the describe call so it matches
the sibling tests' format by adding a '#' before the issue number; change the
describe labeled "Regression for issue 2603" to "Regression for issue `#2603`"
(locate the describe invocation in tests/regression/test/issue-2603.test.ts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8b88e82a-a0df-490c-9752-bc356aec7517
📒 Files selected for processing (30)
package.jsonpackages/auth-adapters/better-auth/package.jsonpackages/cli/package.jsonpackages/clients/client-helpers/package.jsonpackages/clients/tanstack-query/package.jsonpackages/common-helpers/package.jsonpackages/config/eslint-config/package.jsonpackages/config/tsdown-config/package.jsonpackages/config/typescript-config/package.jsonpackages/config/vitest-config/package.jsonpackages/create-zenstack/package.jsonpackages/ide/vscode/package.jsonpackages/language/package.jsonpackages/orm/package.jsonpackages/orm/src/client/executor/name-mapper.tspackages/plugins/policy/package.jsonpackages/plugins/policy/src/policy-handler.tspackages/schema/package.jsonpackages/sdk/package.jsonpackages/server/package.jsonpackages/testtools/package.jsonpackages/zod/package.jsonsamples/orm/package.jsontests/e2e/package.jsontests/regression/package.jsontests/regression/test/issue-2538.test.tstests/regression/test/issue-2595.test.tstests/regression/test/issue-2603.test.tstests/runtimes/bun/package.jsontests/runtimes/edge-runtime/package.json
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
2 participants
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
Bug Fixes
public.Chores
h3peer dependency support.