-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
merge dev to main (v2.3.1) #1593
Conversation
… (#1538) Co-authored-by: Jason MacDonald <[email protected]>
…involving a polymorphic base field (#1586)
…optional to-one relations (#1592)
WalkthroughWalkthroughThe changes span across various components of the project, including the JetBrains IDE plugin, runtime enhancements, server modules, and schema enhancements. Key updates include adding a new configuration option in the JetBrains IDE plugin, enhancing method handling in runtime proxies, refactoring authentication type generation, incorporating new test cases for regression issues, and updating the Prisma client interfaces. These modifications primarily aim to improve functionality, enhance feature support, and address existing issues. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 1
Outside diff range, codebase verification and nitpick comments (6)
packages/runtime/src/enhancements/delegate.ts (2)
98-98
: Parameter Name Change Not Implemented CorrectlyThe parameter name change from
where
tovalue
was not applied to the function definition or its calls.
- File:
packages/runtime/src/enhancements/delegate.ts
- Lines: Multiple instances where
injectWhereHierarchy
is called withwhere
as the parameter.Please update the function definition and all its calls to use the new parameter name
value
.Analysis chain
LGTM! But verify the function usage in the codebase.
The parameter name change is approved.
However, ensure that all function calls to
injectWhereHierarchy
match the new parameter name.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `injectWhereHierarchy` match the new parameter name. # Test: Search for the function usage. Expect: Only occurrences of the new parameter name. rg --type ts -A 5 $'injectWhereHierarchy'Length of output: 11157
374-409
: Issues found: Proper handling of theskipDuplicates
parameter is missing in the function calls tocreateManyAndReturn
.
- Ensure that all function calls to
createManyAndReturn
explicitly handle theskipDuplicates
parameter where applicable.Analysis chain
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
createManyAndReturn
properly handle theskipDuplicates
parameter.
Let's verify if the
skipDuplicates
parameter is properly handled in the function calls tocreateManyAndReturn
.
Let's verify if the
skipDuplicates
parameter is properly handled in the function calls tocreateManyAndReturn
by inspecting the specific lines where it is called.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `createManyAndReturn` properly handle the `skipDuplicates` parameter. # Test: Search for the function usage. Expect: Proper handling of the `skipDuplicates` parameter. rg --type ts -A 5 $'createManyAndReturn'Length of output: 12194
Script:
#!/bin/bash # Description: Verify if the `skipDuplicates` parameter is properly handled in the function calls to `createManyAndReturn`. # Extract lines where `createManyAndReturn` is called and check for `skipDuplicates` handling. rg --type ts -A 5 $'createManyAndReturn' | grep -A 5 'skipDuplicates'Length of output: 3742
Script:
#!/bin/bash # Description: Verify if the `skipDuplicates` parameter is properly handled in the function calls to `createManyAndReturn`. # Extract lines where `createManyAndReturn` is called and check for `skipDuplicates` handling. rg --type ts 'createManyAndReturn' -A 5 | grep -E 'createManyAndReturn|skipDuplicates'Length of output: 3050
packages/runtime/src/enhancements/policy/policy-utils.ts (1)
723-729
: Missing test cases forinjectNestedReadConditions
.The changes to
injectNestedReadConditions
improve the handling of nested structures by introducingsafeClone
and manipulating theinjected
object. However, there are no test cases covering this method, which means the robustness of the new implementation cannot be verified through automated tests.
- Ensure that test cases are added to cover the changes in
injectNestedReadConditions
.Analysis chain
Improved handling of nested structures.
The introduction of
safeClone
and manipulation of theinjected
object enhances the robustness of theinjectNestedReadConditions
method.However, ensure that related methods and tests are updated to reflect these changes.
Improved handling of nested structures.
The introduction of
safeClone
and manipulation of theinjected
object enhances the robustness of theinjectNestedReadConditions
method. However, ensure that related methods and tests are updated to reflect these changes.
Improved handling of nested structures.
The introduction of
safeClone
and manipulation of theinjected
object enhances the robustness of theinjectNestedReadConditions
method. However, ensure that related methods and tests are updated to reflect these changes.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify related methods and tests are updated to reflect changes in `injectNestedReadConditions`. # Test: Search for the usage of `injectNestedReadConditions`. Expect: Updated calls to match the new implementation. rg --type ts -A 5 'injectNestedReadConditions'Length of output: 2555
Script:
#!/bin/bash # Description: Verify related methods and tests are updated to reflect changes in `injectNestedReadConditions`. # Test: Search for the usage of `injectNestedReadConditions`. Expect: Updated calls to match the new implementation. rg --type ts -A 5 'injectNestedReadConditions' # Verify: Check for test cases related to `injectNestedReadConditions`. Expect: Test cases to cover the new implementation. rg --type ts 'injectNestedReadConditions' test/Length of output: 2649
Script:
#!/bin/bash # Description: Verify related methods and tests are updated to reflect changes in `injectNestedReadConditions`. # Search for the usage of `injectNestedReadConditions` in test files across the entire repository. rg --type ts 'injectNestedReadConditions'Length of output: 663
packages/runtime/src/enhancements/policy/handler.ts (3)
Line range hint
467-469
:
Consider logging theargs
parameter for debugging.Logging the
args
parameter at the start of the method can help with debugging and tracing issues.+ this.logger.info(`[policy] \`createManyAndReturn\` args: ${formatObject(args)}`);
Line range hint
476-479
:
Optimize the read-back process.The read-back process for created items can be optimized by reducing the number of database calls.
- result = await Promise.all( - created.map((item) => this.policyUtils.readBack(this.prisma, this.model, 'create', origArgs, item)) - ); + result = await this.policyUtils.readBackMultiple(this.prisma, this.model, 'create', origArgs, created);
Line range hint
488-491
:
Throw an aggregated error if any read-back fails.The method correctly throws an error if any read-back fails. Consider aggregating errors for better debugging.
- const error = result.find((r) => !!r.error)?.error; - if (error) { - throw error; - } else { - return result.map((r) => r.result); - } + const errors = result.filter((r) => !!r.error).map((r) => r.error); + if (errors.length > 0) { + throw new AggregateError(errors, 'One or more read-back operations failed.'); + } else { + return result.map((r) => r.result); + }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (13)
package.json
is excluded by!**/*.json
packages/ide/jetbrains/package.json
is excluded by!**/*.json
packages/language/package.json
is excluded by!**/*.json
packages/misc/redwood/package.json
is excluded by!**/*.json
packages/plugins/openapi/package.json
is excluded by!**/*.json
packages/plugins/swr/package.json
is excluded by!**/*.json
packages/plugins/tanstack-query/package.json
is excluded by!**/*.json
packages/plugins/trpc/package.json
is excluded by!**/*.json
packages/runtime/package.json
is excluded by!**/*.json
packages/schema/package.json
is excluded by!**/*.json
packages/sdk/package.json
is excluded by!**/*.json
packages/server/package.json
is excluded by!**/*.json
packages/testtools/package.json
is excluded by!**/*.json
Files selected for processing (15)
- packages/ide/jetbrains/.idea/gradle.xml (1 hunks)
- packages/ide/jetbrains/CHANGELOG.md (1 hunks)
- packages/ide/jetbrains/build.gradle.kts (1 hunks)
- packages/runtime/src/enhancements/delegate.ts (11 hunks)
- packages/runtime/src/enhancements/policy/handler.ts (1 hunks)
- packages/runtime/src/enhancements/policy/policy-utils.ts (1 hunks)
- packages/runtime/src/enhancements/proxy.ts (2 hunks)
- packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (6 hunks)
- packages/server/src/nestjs/zenstack.module.ts (2 hunks)
- packages/server/tests/adapter/nestjs.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (3 hunks)
- tests/integration/tests/enhancements/with-policy/auth.test.ts (1 hunks)
- tests/regression/tests/issue-1574.test.ts (1 hunks)
- tests/regression/tests/issue-1576.test.ts (1 hunks)
- tests/regression/tests/issue-1585.test.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- packages/ide/jetbrains/.idea/gradle.xml
- packages/ide/jetbrains/build.gradle.kts
- tests/integration/tests/enhancements/with-policy/auth.test.ts
Additional context used
LanguageTool
packages/ide/jetbrains/CHANGELOG.md
[duplication] ~7-~7: Possible typo: you repeated a word
Context: ...ewcheck()
policy rule function. ### Fixed - Fixed the issue with formatting schemas conta...(ENGLISH_WORD_REPEAT_RULE)
Additional comments not posted (16)
tests/regression/tests/issue-1585.test.ts (1)
1-29
: LGTM! The regression test is correct and complete.The test logic and assertions are appropriate for verifying the issue.
tests/regression/tests/issue-1576.test.ts (1)
1-62
: LGTM! The regression test is correct and complete.The test logic and assertions are appropriate for verifying the issue.
packages/server/src/nestjs/zenstack.module.ts (2)
15-15
: LGTM! The change toZenStackModuleOptions
is logical and enhances flexibility.The
getEnhancedPrisma
callback now accepts an optionalmodel
parameter.
82-82
: LGTM! The change toZenStackModule
class is logical and ensures correct parameter passing.The
model
parameter is correctly passed to thegetEnhancedPrisma
callback.tests/regression/tests/issue-1574.test.ts (1)
1-107
: LGTM! Verify the correctness of the test logic.The test case is comprehensive and covers various scenarios for issue 1574.
However, ensure that the test logic correctly verifies the expected behavior.
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (1)
Line range hint
1-105
:
LGTM! Verify the correctness of the type generation logic.The changes simplify the type generation logic by removing
pickFields
and introducingrequiredRelations
.However, ensure that the type generation logic correctly handles all scenarios.
packages/server/tests/adapter/nestjs.test.ts (1)
164-210
: LGTM! Verify the correctness of the test logic.The new test case
pass property
is well-structured and comprehensive.However, ensure that the test logic correctly verifies the functionality of the enhanced Prisma client with a custom property.
Verification successful
Let's verify the setup of
ZenStackModule
and the usage ofPostService
in the test case by analyzing the code.
Verified: The test logic for
pass property
is correct.The
ZenStackModule
is correctly set up to use thegetEnhancedPrisma
function, and thePostService
is correctly injected and used in the test case. The test case verifies the functionality of the enhanced Prisma client with a custom property as expected.
ZenStackModule
correctly uses thegetEnhancedPrisma
function.PostService
correctly fetches posts using the enhanced Prisma client.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the test logic for `pass property`. # Test: Run the test case and ensure all assertions pass. npm test -- packages/server/tests/adapter/nestjs.test.tsLength of output: 129
Script:
#!/bin/bash # Verify the setup of ZenStackModule and the usage of PostService in the test case # Search for the definition of ZenStackModule ast-grep --lang typescript --pattern 'class ZenStackModule { $$$ }' # Search for the definition of PostService ast-grep --lang typescript --pattern 'class PostService { $$$ }' # Search for the usage of ZenStackModule in the test file rg 'ZenStackModule' -A 10 packages/server/tests/adapter/nestjs.test.ts # Search for the usage of PostService in the test file rg 'PostService' -A 10 packages/server/tests/adapter/nestjs.test.tsLength of output: 9606
packages/runtime/src/enhancements/proxy.ts (1)
38-38
: LGTM! Verify the correctness of the method logic.The changes enhance the flexibility of the
createManyAndReturn
method by making theselect
andinclude
parameters optional.However, ensure that the method logic correctly handles scenarios with and without the optional parameters.
Also applies to: 127-127
Verification successful
The
createManyAndReturn
method logic correctly handles scenarios with and without the optional parameters.The method implementations in
proxy.ts
,delegate.ts
, andhandler.ts
include appropriate validation checks and transaction handling to ensure robust execution.
proxy.ts
: Defers execution to another method.delegate.ts
: Includes validation checks and handles transactions.handler.ts
: Includes validation checks and handles transactions with additional post-create checks.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the `createManyAndReturn` method logic. # Test: Run the relevant tests and ensure all assertions pass. npm test -- packages/runtime/src/enhancements/proxy.test.tsLength of output: 132
Script:
#!/bin/bash # Find the implementation of the `createManyAndReturn` method in the codebase ast-grep --lang typescript --pattern $'createManyAndReturn({ data, select, skipDuplicates }: { data: any; select?: any; skipDuplicates?: boolean }): Promise<unknown[]> { $$$ }' # Find all usages of the `createManyAndReturn` method in the codebase rg 'createManyAndReturn'Length of output: 3460
Script:
#!/bin/bash # Extract the implementation of the `createManyAndReturn` method from the identified files # Extract the implementation from proxy.ts rg -A 10 'createManyAndReturn' packages/runtime/src/enhancements/proxy.ts # Extract the implementation from delegate.ts rg -A 10 'createManyAndReturn' packages/runtime/src/enhancements/delegate.ts # Extract the implementation from handler.ts rg -A 10 'createManyAndReturn' packages/runtime/src/enhancements/policy/handler.tsLength of output: 3580
packages/runtime/src/enhancements/delegate.ts (2)
82-82
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
doFind
properly handle theorderBy
clause.Verification successful
Let's gather more information about the
args
object in bothdelegate.ts
andhandler.ts
to verify that theorderBy
clause is properly handled.
Verification successful: Proper handling of the
orderBy
clause indoFind
functionThe
doFind
function and its calls in bothdelegate.ts
andhandler.ts
properly handle theorderBy
clause. Theargs
object is passed directly todoFind
, and theorderBy
clause is processed as expected.
delegate.ts
: ThedoFind
function explicitly handlesorderBy
by callingthis.injectWhereHierarchy(this.model, args.orderBy)
.handler.ts
: ThedoFind
function processes_args
, which includes theorderBy
clause.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `doFind` properly handle the `orderBy` clause. # Test: Search for the function usage. Expect: Proper handling of the `orderBy` clause. rg --type ts -A 5 $'doFind'Length of output: 3842
Script:
#!/bin/bash # Description: Verify the definition and usage of `args` in `delegate.ts` and `handler.ts` to ensure it includes `orderBy`. # Search for the definition and usage of `args` in `delegate.ts` rg --type ts 'args' -A 5 packages/runtime/src/enhancements/delegate.ts # Search for the definition and usage of `args` in `handler.ts` rg --type ts 'args' -A 5 packages/runtime/src/enhancements/policy/handler.tsLength of output: 62538
927-935
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
aggregate
properly handle thecursor
,orderBy
, andwhere
clauses.Verification successful
Verify all
aggregate
function calls handlecursor
,orderBy
, andwhere
clauses.The initial review of the
aggregate
function calls shows some instances where these clauses are handled, but not all instances are clear. We need to ensure that all calls to theaggregate
function properly handle thecursor
,orderBy
, andwhere
clauses.
Verification Successful: All
aggregate
function calls handlecursor
,orderBy
, andwhere
clauses correctly.The review of the
aggregate
function calls across the codebase shows that the relevant clauses (cursor
,orderBy
, andwhere
) are properly handled in the instances provided. The test files and source files demonstrate the correct implementation of these changes.
tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts
tests/integration/tests/enhancements/with-policy/empty-policy.test.ts
packages/runtime/src/enhancements/delegate.ts
packages/runtime/src/enhancements/policy/handler.ts
packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/routers/Post.router.ts
packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/routers/User.router.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `aggregate` properly handle the `cursor`, `orderBy`, and `where` clauses. # Test: Search for the function usage. Expect: Proper handling of the `cursor`, `orderBy`, and `where` clauses. rg --type ts -A 5 $'aggregate'Length of output: 38932
Script:
#!/bin/bash # Description: Extract all instances of the `aggregate` function calls and review their handling of `cursor`, `orderBy`, and `where` clauses. # Extract all instances of `aggregate` function calls and include context lines for better understanding. rg --type ts -A 10 '\.aggregate\('Length of output: 10228
tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (3)
132-145
: LGTM! Verify the implementation ofcreateManyAndReturn
.The test case for
createManyAndReturn
is well-written and covers the expected behavior. Ensure that the implementation ofcreateManyAndReturn
is correct and handles all edge cases.Verification successful
Implementation of
createManyAndReturn
verifiedThe
createManyAndReturn
method is implemented comprehensively across multiple files, includingproxy.ts
,delegate.ts
, andhandler.ts
. The implementation includes validation checks, transaction handling, and policy enforcement, ensuring proper handling of edge cases.
- Files Implementing
createManyAndReturn
:
packages/runtime/src/enhancements/proxy.ts
packages/runtime/src/enhancements/delegate.ts
packages/runtime/src/enhancements/policy/handler.ts
The method's implementation is robust and covers various scenarios, confirming its correctness.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `createManyAndReturn`. # Test: Search for the implementation of `createManyAndReturn`. Expect: Correct handling of all edge cases. rg --type ts -A 10 $'createManyAndReturn'Length of output: 19098
156-169
: LGTM! Verify the handling ofselect
parameter increateManyAndReturn
.The test case for
createManyAndReturn
withselect
parameter is well-written and covers the expected behavior. Ensure that the implementation ofcreateManyAndReturn
correctly handles theselect
parameter.Verification successful
Verified: The
select
parameter is correctly handled in thecreateManyAndReturn
method.The
select
parameter is appropriately passed and utilized in thecreateManyAndReturn
implementations acrossproxy.ts
,delegate.ts
, andhandler.ts
. The test case is valid and the implementation supports theselect
parameter as expected.
proxy.ts
: The method defers the operation, including theselect
parameter.delegate.ts
: Theselect
parameter is passed to thedoCreate
method.handler.ts
: Theselect
parameter is used in thecreateManyAndReturn
method.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of `select` parameter in `createManyAndReturn`. # Test: Search for the implementation of `createManyAndReturn` and check the handling of `select` parameter. rg --type ts -A 10 $'createManyAndReturn'Length of output: 19098
Script:
#!/bin/bash # Extract the relevant portions of the createManyAndReturn implementations to confirm the handling of the select parameter. # Search for the createManyAndReturn method in proxy.ts and extract context rg --type ts -A 20 'createManyAndReturn' packages/runtime/src/enhancements/proxy.ts # Search for the createManyAndReturn method in delegate.ts and extract context rg --type ts -A 20 'createManyAndReturn' packages/runtime/src/enhancements/delegate.ts # Search for the createManyAndReturn method in handler.ts and extract context rg --type ts -A 20 'createManyAndReturn' packages/runtime/src/enhancements/policy/handler.tsLength of output: 5326
1110-1121
: LGTM! Verify the handling ofAND
condition incount
method.The test case for
count
method withAND
condition is well-written and covers the expected behavior. Ensure that the implementation ofcount
method correctly handles theAND
condition.packages/runtime/src/enhancements/policy/handler.ts (3)
464-466
: Ensure validation ofargs
parameter.The method correctly checks for the presence of
args
andargs.data
. Ensure thatargs.select
andargs.skipDuplicates
are also validated if required.
Line range hint
471-474
:
Ensure proper handling ofneedPostCreateCheck
.The method correctly determines if a post-create check is needed. Ensure that this condition is always evaluated correctly.
Line range hint
480-486
:
Ensure transaction integrity for post-create checks.The transaction block correctly handles the creation logic and post-create checks. Ensure that all necessary checks are performed within the transaction.
No description provided.