feat: support updateManyAndReturn #1960
Conversation
📝 WalkthroughWalkthroughThis pull request introduces support for the Changes
Sequence DiagramsequenceDiagram
participant Client
participant ProxyHandler
participant PolicyHandler
participant Database
Client->>ProxyHandler: updateManyAndReturn(args)
ProxyHandler->>PolicyHandler: Apply policy checks
PolicyHandler-->>ProxyHandler: Policy validation result
ProxyHandler->>Database: Execute update operation
Database-->>ProxyHandler: Return updated records
ProxyHandler-->>Client: Return updated records
Possibly related PRs
Finishing Touches
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
🧹 Nitpick comments (5)
tests/regression/tests/issue-1955.test.ts (1)
106-115: Consider adding more diverse test casesWhile the current test verifies basic functionality, consider adding test cases for:
- Partial updates
- Different where conditions
- Error scenarios
tests/integration/tests/enhancements/with-policy/update-many-and-return.test.ts (1)
106-139: LGTM! Well-structured field-level policy testsGood coverage of field-level permissions and partial visibility scenarios.
Consider adding test cases for:
- Error handling when policies are violated
- Invalid update data scenarios
- Concurrent update scenarios
packages/runtime/src/enhancements/node/policy/handler.ts (3)
1278-1278: Add return type annotation todoUpdateMany.The method's return type should be explicitly annotated for better type safety and documentation.
- private doUpdateMany(args: any, action: 'updateMany' | 'updateManyAndReturn'): Promise<any> { + private doUpdateMany(args: any, action: 'updateMany' | 'updateManyAndReturn'): Promise<{ count: number } | unknown[]> {
1331-1332: Fix comment typo in error handling.The error message refers to "create result" but should refer to "update result".
- // throw read-back error if any of create result read-back fails + // throw read-back error if any of the update result read-back fails
1314-1338: Consider extracting transaction-less and transaction paths into separate methods.The logic for handling transaction-less and transaction paths could be clearer if extracted into separate private methods. This would improve readability and make the code structure more maintainable.
Example structure:
private async handleTransactionLessPath(args: any, action: 'updateMany' | 'updateManyAndReturn', origArgs?: any) { // ... transaction-less logic ... } private async handleTransactionPath(args: any, action: 'updateMany' | 'updateManyAndReturn', origArgs?: any) { // ... transaction logic ... }Also applies to: 1389-1407
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/runtime/src/constants.ts(1 hunks)packages/runtime/src/cross/nested-write-visitor.ts(1 hunks)packages/runtime/src/cross/types.ts(1 hunks)packages/runtime/src/enhancements/node/policy/handler.ts(5 hunks)packages/runtime/src/enhancements/node/proxy.ts(2 hunks)packages/runtime/src/types.ts(1 hunks)tests/integration/tests/enhancements/with-policy/update-many-and-return.test.ts(1 hunks)tests/regression/tests/issue-1955.test.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
🔇 Additional comments (10)
packages/runtime/src/cross/types.ts (1)
11-11: LGTM! Addition of updateManyAndReturn actionThe new action is correctly added to
PrismaWriteActionsand maintains logical grouping with other update operations.packages/runtime/src/constants.ts (1)
80-80: LGTM! Consistent addition to write payload actionsThe new action is correctly added to
ACTIONS_WITH_WRITE_PAYLOAD, maintaining consistency with other update operations that return results.tests/regression/tests/issue-1955.test.ts (1)
43-52: LGTM! Basic updateManyAndReturn test coverageThe test properly verifies the basic functionality of updating and returning multiple records.
tests/integration/tests/enhancements/with-policy/update-many-and-return.test.ts (1)
4-104: LGTM! Comprehensive model-level policy testingExcellent test coverage including:
- Policy-based update restrictions
- Read-back policy enforcement
- Relation handling and filtering
packages/runtime/src/types.ts (1)
22-22: LGTM!The method signature is consistent with other similar methods in the interface, particularly matching the pattern of
createManyAndReturn.packages/runtime/src/cross/nested-write-visitor.ts (1)
250-250: LGTM!Correctly reuses the same visitor logic as
updateManysince both operations share the same write payload structure. The difference in return value is appropriately handled at the proxy handler level.packages/runtime/src/enhancements/node/proxy.ts (2)
38-39: LGTM!The method signature in the interface is consistent with other methods and correctly specifies the return type.
137-140: LGTM!The implementation correctly uses the
deferredpattern and matches the interface contract.packages/runtime/src/enhancements/node/policy/handler.ts (2)
1271-1278: LGTM! Good refactoring.Extracting shared logic into
doUpdateManyimproves code maintainability and reduces duplication.
1409-1426: LGTM! Proper error handling.The read-back processing correctly handles errors and results, ensuring that any failures during the read-back process are properly propagated.
No description provided.