-
-
Notifications
You must be signed in to change notification settings - Fork 126
fix(delegate): issue with filtering concrete relations with fields from base #2260
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
📝 WalkthroughWalkthroughAdds propagation of nested where clauses for delegated fields: after injecting base select/include hierarchy for a delegate field, the handler now calls injectWhereHierarchy on the nested type when a nested Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Prisma as Prisma Client
participant DPH as DelegateProxyHandler
participant ISIH as injectSelectIncludeHierarchy
participant IWH as injectWhereHierarchy
Client->>Prisma: findMany/findFirst with include/select
Prisma->>DPH: process delegated fields
DPH->>ISIH: inject base select/include hierarchy
Note over ISIH: Assemble base field hierarchy
ISIH-->>DPH: base hierarchy injected
alt nested include/select contains where
DPH->>IWH: inject nested where hierarchy (NEW)
Note over IWH: Traverse nested type and augment where clause
IWH-->>DPH: where hierarchy injected
end
DPH-->>Prisma: modified args
Prisma-->>Client: results
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)tests/regression/tests/issue-2246.test.ts (1)
⏰ 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). (5)
🔇 Additional comments (5)
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: 1
🧹 Nitpick comments (1)
tests/regression/tests/issue-2246.test.ts (1)
5-82: Good regression test coverage for the reported issue.The test validates both include and _count scenarios with nested where filters on delegated fields, covering both matching and non-matching cases. This directly addresses the reported issue #2246.
Optional: Consider adding test cases for:
- Using
selectinstead ofincludewith nested where filters.- Complex where clauses with logical operators (AND, OR, NOT) on delegated fields.
- Filtering on fields defined directly in Movie (non-inherited) vs. fields from Media base (inherited) to ensure both scenarios work correctly.
These additions would strengthen confidence in edge cases, but the current coverage is sufficient for validating the fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/runtime/src/enhancements/node/delegate.ts(1 hunks)tests/regression/tests/issue-2246.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/regression/tests/issue-2246.test.ts (1)
packages/testtools/src/schema.ts (1)
loadSchema(172-248)
⏰ 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). (5)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
packages/runtime/src/enhancements/node/delegate.ts (1)
231-233: LGTM! Consistent where-clause propagation for nested delegate fields.The addition correctly propagates where-clause injection for nested delegate fields during select/include processing. This aligns with the existing pattern for orderBy injection (lines 242-247) and fixes the reported issue where nested where filters on delegated fields were not being transformed.
fixes #2246