Fix for filtering by compound id #1806
Conversation
In some cases Prisma doesn’t expect these to be nested under the compound id key.
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (1)
packages/server/src/api/rest/index.ts (1)
Line range hint
1239-1261: Optimize accumulator performance and improve documentation.The method implementation has two areas for improvement:
The spread operator in the reduce function can lead to O(n²) time complexity.
The new
nestedparameter would benefit from documentation explaining its purpose.Optimize the reduce implementation:
- return idFields.reduce( - (acc, curr, idx) => ({ - ...acc, - [curr.name]: this.coerce(curr.type, decodedId.split(this.idDivider)[idx]), - }), - {} - ); + const result = {}; + idFields.forEach((curr, idx) => { + result[curr.name] = this.coerce(curr.type, decodedId.split(this.idDivider)[idx]); + }); + return result;
- Add JSDoc documentation:
+ /** + * Creates a Prisma filter for ID fields + * @param idFields - Array of ID field information + * @param resourceId - The resource ID value + * @param nested - When true, creates a nested filter structure for compound IDs. + * When false, creates a flat structure with individual ID fields. + * @returns The Prisma filter object + */ private makePrismaIdFilter(idFields: FieldInfo[], resourceId: string, nested: boolean = true) {🧰 Tools
🪛 Biome
[error] 1257-1257: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/server/src/api/rest/index.ts (3 hunks)
🧰 Additional context used
🪛 Biome
packages/server/src/api/rest/index.ts
[error] 1257-1257: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
🔇 Additional comments (1)
packages/server/src/api/rest/index.ts (1)
1609-1609: LGTM: Correct usage of flat ID structure for relation filters.The change to use
nested: falseis appropriate here as Prisma'sisfilter operator expects a flat structure for ID fields.Let's verify the usage of relation filters:
✅ Verification successful
Let me gather more context about the
makePrismaIdFilterimplementation and its usage to ensure the verification is thorough.
Let me try a different pattern to find the
makePrismaIdFilterimplementation.
Based on the implementation and usage context found, I can now provide the final verification response:
Verified: Correct implementation and usage of flat ID structure for relation filters
The code correctly uses
nested: falsein the relation filter context because:
- The
makePrismaIdFilterimplementation shows it handles both nested and flat structures through thenestedparameter- In the relation filter case (
is: this.makePrismaIdFilter(info.idFields, value, false)), flat structure is correctly used- This aligns with Prisma's expectations for the
isoperator in relation filters, which requires a flat structure🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for relation filter usage patterns # Test: Search for filter patterns with 'is' operator. Expect: Only flat ID structures. rg -A 5 "is: this\.makePrismaIdFilter"Length of output: 508
Script:
#!/bin/bash # Check the implementation of makePrismaIdFilter ast-grep --pattern 'makePrismaIdFilter($_) { $$$ }'Length of output: 55
Script:
#!/bin/bash # Search for makePrismaIdFilter implementation with context rg -A 10 "makePrismaIdFilter" packages/server/src/api/rest/index.tsLength of output: 4510
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/server/src/api/rest/index.ts (3 hunks)
🧰 Additional context used
🪛 Biome
packages/server/src/api/rest/index.ts
[error] 1257-1257: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
🔇 Additional comments (2)
packages/server/src/api/rest/index.ts (2)
1239-1243: LGTM! Good addition of the nested parameter.The new
nestedparameter provides flexibility in ID filter generation while maintaining backward compatibility.
1605-1609: LGTM! Correct handling of ID filters in non-unique contexts.The changes correctly implement flat ID filters for relation filtering, aligning with Prisma's expectations that nested ID filters should only be used in unique where clauses.
In some cases Prisma doesn’t expect these to be nested under the compound id key. I don't have a full overview of where this is (just the example I hit myself), so I added it just where I can confirm it's needed.