-
-
Notifications
You must be signed in to change notification settings - Fork 126
merge dev to main (v2.19.2) #2261
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
📝 WalkthroughWalkthroughUpdates span: REST API now parses fields[] for partial field selection and integrates it into select/include flows; delegate enhancement injects nested where clauses; attribute arg value becomes optional across runtime and generator; build script loads .env files; adds tests for partial fields and a delegation regression; bumps JetBrains plugin version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant RH as REST Handler
participant PS as buildPartialSelect
participant RS as buildRelationSelect
participant DB as Prisma
C->>RH: GET /resources?fields[User]=id,name&include=posts
RH->>PS: parse fields[User]
PS-->>RH: { select: { id: true, name: true } }
RH->>RS: build nested relation select/include (with query)
RS-->>RH: include/select with nested partials/default ids
RH->>DB: findMany({ select/include merged })
DB-->>RH: rows + related
RH-->>C: JSON: data + included (projected fields)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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. Comment |
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: 2
🧹 Nitpick comments (1)
packages/server/tests/api/rest-partial.test.ts (1)
377-471: Fix typo in test names.Test names contain "efect" which should be "affect" (lines 377, 412, 445).
Apply this diff to fix the typos:
- it('does not efect toplevel filtering', async () => { + it('does not affect toplevel filtering', async () => { - it('does not efect toplevel sorting', async () => { + it('does not affect toplevel sorting', async () => { - it('does not efect toplevel pagination', async () => { + it('does not affect toplevel pagination', async () => {Otherwise, the test coverage is comprehensive and validates the feature well across multiple scenarios including interactions with existing functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
package.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/swr/package.jsonis excluded by!**/*.jsonpackages/plugins/tanstack-query/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/package.jsonis excluded by!**/*.jsonpackages/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!**/*.json
📒 Files selected for processing (8)
packages/ide/jetbrains/build.gradle.kts(1 hunks)packages/runtime/src/cross/model-meta.ts(1 hunks)packages/runtime/src/enhancements/node/delegate.ts(1 hunks)packages/schema/build/bundle.js(1 hunks)packages/sdk/src/model-meta-generator.ts(2 hunks)packages/server/src/api/rest/index.ts(12 hunks)packages/server/tests/api/rest-partial.test.ts(1 hunks)tests/regression/tests/issue-2246.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
tests/regression/tests/issue-2246.test.ts (1)
packages/testtools/src/schema.ts (1)
loadSchema(172-248)
packages/server/src/api/rest/index.ts (1)
packages/runtime/src/local-helpers/lower-case-first.ts (1)
lowerCaseFirst(1-3)
packages/server/tests/api/rest-partial.test.ts (2)
packages/testtools/src/schema.ts (1)
loadSchema(172-248)packages/server/src/api/rest/index.ts (2)
makeHandler(2101-2104)makeHandler(2106-2106)
packages/sdk/src/model-meta-generator.ts (1)
packages/sdk/src/code-gen.ts (1)
FastWriter(90-131)
⏰ 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: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: OSSAR-Scan
🔇 Additional comments (12)
packages/schema/build/bundle.js (1)
5-6: LGTM! Environment configuration loaded correctly.The dotenv configuration correctly loads
.env.localfirst (for local overrides) and then.env(for defaults). This is a standard pattern for managing environment-specific configuration.packages/runtime/src/enhancements/node/delegate.ts (1)
231-233: LGTM! Nested where clause injection for delegate models.Correctly propagates
whereconditions into nested delegate hierarchies after the select/include hierarchy is injected. This ensures filtering works properly for nested delegate relations.packages/sdk/src/model-meta-generator.ts (3)
83-88: LGTM! Type-only import added for explicit typing.Adding a type-only import of
ModelMetaand using it to explicitly type the metadata variable improves code clarity without affecting the runtime bundle.
96-96: LGTM! Explicit type annotation improves clarity.Explicitly typing
metadataasModelMetais clearer than relying on type inference fromwriter.result.
374-374: LGTM! Optional value aligns with runtime type changes.Making the
valueproperty optional in attribute args aligns with the corresponding change inpackages/runtime/src/cross/model-meta.tsand correctly handles cases whereexprToValue()returnsundefined.tests/regression/tests/issue-2246.test.ts (1)
1-82: LGTM! Comprehensive regression test for nested delegate filtering.This test thoroughly validates nested relation includes, counts, and filtering across a delegate model hierarchy. It correctly tests:
- Nested
whereclauses in includes (line 52-55)_countwith filtered relations (line 63)- Both positive and negative cases
The test ensures the delegate enhancement changes work correctly with nested filtering scenarios.
packages/server/src/api/rest/index.ts (5)
180-183: LGTM!The new error type for duplicate field parameters is well-defined and consistent with existing error patterns.
519-541: LGTM!The integration of partial field selection in
processSingleReadcorrectly handles the merge of select and include options, following Prisma's constraint that select and include cannot coexist.
577-594: LGTM!The partial select integration correctly avoids overriding existing select objects from includes and applies field filtering to related resources.
739-761: LGTM!The partial field selection integration in
processCollectionReadmirrors the logic inprocessSingleRead, maintaining consistency across the codebase.
1861-1919: LGTM!The enhancement to
buildRelationSelectproperly propagates partial field selection into nested relations while maintaining backward compatibility. The logic correctly handles both intermediate and terminal relations in the include path.packages/server/tests/api/rest-partial.test.ts (1)
1-62: LGTM!The test setup is well-structured with proper schema definition, test isolation via database resets, and clean dependency injection through the handler wrapper.
No description provided.