-
-
Couldn't load subscription status.
- Fork 126
merge dev to main (v2.18.0) #2217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis update introduces support for external ID mappings in the REST API handler, allowing models to use alternative unique keys as identifiers. It also updates dependency versions for Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RESTHandler
participant ModelMeta
Client->>RESTHandler: Request resource by external ID (e.g., /user/User1_a)
RESTHandler->>RESTHandler: Normalize externalIdMapping
RESTHandler->>RESTHandler: getIdFields(modelMeta, 'User')
alt External ID mapping exists
RESTHandler->>ModelMeta: Lookup unique constraint fields for external ID
ModelMeta-->>RESTHandler: Return fields (e.g., name, source)
else
RESTHandler->>ModelMeta: Fallback to default ID fields
ModelMeta-->>RESTHandler: Return default ID field(s)
end
RESTHandler->>Client: Return resource with external ID in relationships
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
🔭 Outside diff range comments (2)
packages/server/src/api/rest/index.ts (2)
589-593: Use mapped model name in paginator links for related fetchThe paginator URL should use the mapped model name (consistent with the linkers and other endpoints).
- const url = this.makeNormalizedUrl(`/${type}/${resourceId}/${relationship}`, query); + const mappedType = this.mapModelName(type); + const url = this.makeNormalizedUrl(`/${mappedType}/${resourceId}/${relationship}`, query);
1614-1714: Align filter[id] semantics with external IDsWith externalIdMapping, “id” in the API represents the external identifier, not necessarily the primary key. The current implementation resolves filter[id] via the model’s isId field, which can be inconsistent.
Recommend: when filter key is 'id', build a where-clause using this.typeMap[...].idFields (supporting compound values split by idDivider) similar to makePrismaIdFilter. This keeps filtering aligned with how single/relationship reads resolve IDs.
I can provide a targeted patch if you want to adopt this behavior in this PR.
♻️ Duplicate comments (3)
packages/runtime/src/enhancements/node/policy/policy-utils.ts (1)
5-5: Same concern as other files switching tozod-validation-error/v3.Ensure workspace-wide dependency versions and bundler/Node resolution support this subpath import.
packages/runtime/src/validation.ts (1)
2-2: Same concern forzod-validation-error/v3subpath import.Confirm dependency version and resolution compatibility.
packages/schema/src/cli/config.ts (1)
3-3: Same concern forzod-validation-error/v3subpath import.Validate that all packages declare a compatible
zod-validation-errorand that no root imports remain.
🧹 Nitpick comments (12)
CONTRIBUTING.md (1)
8-8: Align pnpm version with CI and add Corepack instruction for reproducibilityIt looks like our CI/config doesn’t explicitly pin or activate pnpm@9.15.9, nor is Corepack invoked to lock it down. Please confirm in your workflow files that you’re installing or preparing the exact version, and consider adding Corepack steps so everyone (local and CI) uses the same pnpm.
Proposed doc tweak in CONTRIBUTING.md (at line 8):
- [pnpm](https://pnpm.io/): v9.15.9 + [pnpm](https://pnpm.io/): v9.15.9 (use Corepack to activate this exact version)Add after prerequisites:
corepack enable corepack prepare pnpm@9.15.9 --activatePlease verify (manually or via CI) that your GitHub workflows or other build scripts install or prepare pnpm@9.15.9. For example, check .github/workflows/*.yml for any of:
- pnpm/action-setup@… with version: 9.15.9
- explicit
npm install -g pnpm@9.15.9- Corepack commands in CI steps
packages/plugins/openapi/src/rpc-generator.ts (2)
157-163: Guard against missing input types
orderByWithRelationInputfalls back to the SearchRelevance variant if the plain variant is absent, but the second variant may also be missing (e.g. whenfullTextSearchisn’t enabled and the first variant was merely filtered out).
Consider adding a final fallback or an invariant check to avoid emitting$refs that have no matching schema component.
279-286: Add basic value constraints to pagination fieldsNice addition! To make the OpenAPI definition self-documenting and catch obvious client errors, declare lower bounds:
- take: { type: 'integer' }, - skip: { type: 'integer' }, + take: { type: 'integer', minimum: 1 }, + skip: { type: 'integer', minimum: 0 },This mirrors typical Prisma usage and prevents negative numbers from slipping through.
packages/server/tests/api/rpc.test.ts (2)
134-249: Great test coverage – considerbeforeEachfor setup/teardownThe test thoroughly exercises ordering, skip/take and cursor pagination.
Minor: moving the delete-many + data-seeding logic to abeforeEach/afterEachhook keeps the flow DRY and guarantees a clean slate even if an assertion aborts mid-test.
283-286: Consistent data cleanupGood call adding an explicit cleanup to avoid ID collisions with prior tests. You might extract these two
deleteMany()calls into a shared helper orafterEachto prevent repetition across test cases.packages/schema/src/language-server/validator/datamodel-validator.ts (1)
49-56: Exempting views from unique-criteria validation: LGTM; add coverageSkipping the uniqueness requirement for views via !dm.isView is correct and contained. Please add/adjust tests to ensure a view without @id/@unique doesn’t raise this error.
I can add a minimal validator test case asserting no diagnostics for a view without unique criteria—want a patch?
tests/integration/tests/enhancements/with-policy/view.test.ts (1)
4-6: Skip rationale noted; add tracking referenceSkipping until Prisma 6.13 is fine. Consider linking an issue/PR reference in the TODO to make reinstating the suite actionable.
packages/schema/tests/generator/prisma-generator.test.ts (1)
534-536: Skip rationale noted; add tracking referenceSame as the integration suite, add an issue link to revisit after Prisma 6.13 to avoid stale skips.
packages/server/tests/api/rest.test.ts (2)
3165-3165: Fix minor comment typo: unique key nameThe comment says backtick name__source (double underscore) but the mapping uses name_source (single underscore). Update for clarity.
- // user is exposed using the fields from the `name__source` multi-column unique index + // user is exposed using the fields from the `name_source` multi-column unique index
3115-3196: Great coverage; consider adding filter[id] case for external IDsAdd a test asserting that filtering by the resource identifier works with external IDs, e.g. GET /user?filter[id]=User1_a returns 1 record. This guards the contract that “id” in the API means the external identifier.
I can draft the test snippet if you want it included in this suite.
packages/server/src/api/rest/index.ts (2)
56-58: Document externalIdMapping optionAdd JSDoc explaining how to specify the mapping:
- Key: model name (case-insensitive; normalized to lowerCaseFirst)
- Value: the Prisma unique filter key generated by joining fields with '_' (e.g. 'name_source')
- Effect: endpoints expose this unique key as the resource id
This avoids guesswork for users configuring the handler.
export type Options = { @@ - modelNameMapping?: Record<string, string>; + modelNameMapping?: Record<string, string>; + + /** + * Map models to an external identifier (unique key) to be used as the resource ID in the API. + * Key: model name (case-insensitive). Value: the Prisma unique filter key generated from the + * unique fields (e.g. 'email' or 'name_source' for @@unique([name, source])). + * When provided, all ID resolution and link serialization use this external key instead of + * the primary id field(s). + */ externalIdMapping?: Record<string, string>; };
245-245: Normalize and validate externalIdMapping early (optional)Normalization looks good. For faster feedback on misconfiguration, consider validating each mapped model against modelMeta during type map build (it will throw now the first time a request hits). You could surface a clearer configuration error by catching and rethrowing with context in buildTypeMap.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (65)
.changeset/config.jsonis excluded by!**/*.json.github/workflows/build-test.ymlis excluded by!**/*.yml.github/workflows/integration-test.ymlis excluded by!**/*.yml.github/workflows/regression-test.ymlis excluded by!**/*.ymlpackage.jsonis excluded by!**/*.jsonpackages/ide/jetbrains/package.jsonis excluded by!**/*.jsonpackages/language/package.jsonis excluded by!**/*.jsonpackages/misc/redwood/package.jsonis excluded by!**/*.jsonpackages/plugins/openapi/package.jsonis excluded by!**/*.jsonpackages/plugins/openapi/tests/baseline/rpc-3.0.0-omit.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rpc-3.0.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rpc-3.1.0-omit.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rpc-3.1.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rpc-type-coverage-3.0.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rpc-type-coverage-3.1.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/swr/package.jsonis excluded by!**/*.jsonpackages/plugins/tanstack-query/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/nuxt-trpc-v10/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/nuxt-trpc-v10/server/trpc/routers/generated/client/Post.nuxt.type.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v10/server/trpc/routers/generated/client/User.nuxt.type.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v10/server/trpc/routers/generated/client/nuxt.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v10/server/trpc/routers/generated/client/utils.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v10/server/trpc/routers/generated/helper.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v10/server/trpc/routers/generated/routers/Post.router.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v10/server/trpc/routers/generated/routers/User.router.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v10/server/trpc/routers/generated/routers/index.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/nuxt-trpc-v11/server/trpc/routers/generated/client/Post.nuxt.type.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v11/server/trpc/routers/generated/client/User.nuxt.type.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v11/server/trpc/routers/generated/client/nuxt.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v11/server/trpc/routers/generated/client/utils.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v11/server/trpc/routers/generated/helper.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v11/server/trpc/routers/generated/routers/Post.router.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v11/server/trpc/routers/generated/routers/User.router.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/nuxt-trpc-v11/server/trpc/routers/generated/routers/index.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/client/Post.next.type.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/client/User.next.type.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/client/next.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/client/utils.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/helper.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/routers/Post.router.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/routers/User.router.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/routers/index.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v11/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/client/Post.react.type.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/client/User.react.type.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/client/react.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/client/utils.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/helper.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/routers/Post.router.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/routers/User.router.tsis excluded by!**/generated/**,!**/generated/**packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/routers/index.tsis excluded by!**/generated/**,!**/generated/**packages/runtime/package.jsonis excluded by!**/*.jsonpackages/schema/package.jsonis excluded by!**/*.jsonpackages/sdk/package.jsonis excluded by!**/*.jsonpackages/server/package.jsonis excluded by!**/*.jsonpackages/testtools/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yamlpnpm-workspace.yamlis excluded by!**/*.yamltests/integration/package.jsonis excluded by!**/*.jsontests/integration/test-run/package.jsonis excluded by!**/*.jsontests/integration/tests/frameworks/nextjs/test-project/package.jsonis excluded by!**/*.jsontests/integration/tests/frameworks/trpc/test-project/package.jsonis excluded by!**/*.jsontests/regression/package.jsonis excluded by!**/*.json
📒 Files selected for processing (22)
.changeset/README.md(0 hunks)CONTRIBUTING.md(1 hunks)packages/ide/jetbrains/build.gradle.kts(1 hunks)packages/plugins/openapi/src/generator-base.ts(1 hunks)packages/plugins/openapi/src/rpc-generator.ts(2 hunks)packages/plugins/tanstack-query/tests/plugin.test.ts(4 hunks)packages/plugins/tanstack-query/tests/portable.test.ts(2 hunks)packages/runtime/src/enhancements/node/policy/handler.ts(2 hunks)packages/runtime/src/enhancements/node/policy/policy-utils.ts(1 hunks)packages/runtime/src/validation.ts(1 hunks)packages/schema/src/cli/config.ts(1 hunks)packages/schema/src/language-server/validator/datamodel-validator.ts(1 hunks)packages/schema/tests/generator/prisma-generator.test.ts(1 hunks)packages/server/src/api/rest/index.ts(10 hunks)packages/server/src/api/rpc/index.ts(1 hunks)packages/server/tests/api/rest.test.ts(1 hunks)packages/server/tests/api/rpc.test.ts(2 hunks)packages/testtools/src/schema.ts(1 hunks)script/test-scaffold.ts(1 hunks)tests/integration/tests/cli/generate.test.ts(1 hunks)tests/integration/tests/cli/plugins.test.ts(2 hunks)tests/integration/tests/enhancements/with-policy/view.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .changeset/README.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-16T03:09:04.699Z
Learnt from: ymc9
PR: zenstackhq/zenstack#2187
File: packages/server/src/api/rest/index.ts:287-289
Timestamp: 2025-07-16T03:09:04.699Z
Learning: In REST API URL pattern matching with model name mapping, only the `type` parameter (representing model names) should be mapped using `modelNameMapping`. The `relationship` parameter represents field names on models and should not be mapped, as field names are distinct from model names in the API structure.
Applied to files:
packages/server/src/api/rest/index.ts
🧬 Code Graph Analysis (3)
tests/integration/tests/cli/generate.test.ts (1)
packages/testtools/src/schema.ts (1)
installPackage(68-70)
script/test-scaffold.ts (1)
packages/testtools/src/schema.ts (1)
run(44-66)
packages/plugins/openapi/src/rpc-generator.ts (1)
packages/runtime/src/local-helpers/upper-case-first.ts (1)
upperCaseFirst(1-3)
⏰ 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)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: dependency-review
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
🔇 Additional comments (3)
packages/runtime/src/enhancements/node/policy/handler.ts (2)
18-18: LGTM: import reordering is harmlessNo functional impact; consistent with local-helpers usage elsewhere.
4-4: Import consistency verified; please confirm bundler support for v3 exportAll references to
fromZodErrorhave been updated to import from'zod-validation-error/v3'across the repository:
- packages/server/src/api/rpc/index.ts
- packages/server/src/api/rest/index.ts
- packages/schema/src/cli/config.ts
- packages/runtime/src/validation.ts
- packages/runtime/src/enhancements/node/policy/policy-utils.ts
- packages/runtime/src/enhancements/node/policy/handler.ts
- packages/plugins/openapi/src/generator-base.ts
Please verify in your build setup that:
- The v3 package exports a named
fromZodErrorfunction (check the package’sexports/package.jsonor type definitions).- Your bundler/TS configuration properly recognizes and tree-shakes the
/v3entry point.If you run into issues, adjust your bundler aliases or TS
pathsaccordingly.packages/server/src/api/rest/index.ts (1)
19-19: ConfirmfromZodErrorexport inzod-validation-error/v3I ran a quick search and see
fromZodErrorbeing imported and used in multiple modules:
- packages/server/src/api/rest/index.ts (L19, L824, L1025, L1056)
- packages/server/src/api/rpc/index.ts (L13, L254)
- packages/runtime/src/validation.ts (L2, L18)
- packages/runtime/src/enhancements/node/policy/handler.ts (L4, L423, L1270)
- packages/runtime/src/enhancements/node/policy/policy-utils.ts (L5, L938, L1443)
- packages/schema/src/cli/config.ts (L3, L31)
- packages/plugins/openapi/src/generator-base.ts (L6, L95)
Please manually verify that in the published v3 bundle:
fromZodErroris exposed as a named export (not default) in its ESM build.- Your TypeScript compiler and bundler (ESM/Node) correctly resolve
zod-validation-error/v3without fallbacks or synthetic defaults.This check will prevent any unexpected runtime import errors.
No description provided.