-
-
Notifications
You must be signed in to change notification settings - Fork 93
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.6.0) #1732
Conversation
Warning Rate limit exceeded@ymc9 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 24 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughWalkthroughThe changes involve multiple updates across various files, primarily focusing on version increments, enhancements to Prisma operation handling, and modifications to API query structures. Key updates include the introduction of optional argument handling in Prisma operations, adjustments to the API query methods, and the addition of new tests to validate these functionalities. Furthermore, there are several version number updates in the Changes
Possibly related PRs
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 (
|
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: 16
Outside diff range and nitpick comments (20)
tests/integration/tests/enhancements/with-delegate/omit-interaction.test.ts (1)
56-73
: LGTM! Consider removing theconsole.log
statement.The test case correctly validates that
@omit
fields are undefined when the discriminator field is not selected in a query.Remove the
console.log
statement at line 70 as it's not necessary for the test.-console.log(foundAsset);
tests/regression/tests/issue-1710.test.ts (3)
29-29
: Security Best Practice: Avoid Hardcoding Passwords in TestsHardcoding passwords, even in tests, can lead to security vulnerabilities and bad practices. Instead, consider using environment variables or mocks.
Modify the test code to use a variable:
- data: { displayName: 'User1', email: '[email protected]', password: '123' }, + const testPassword = 'test-password'; + data: { displayName: 'User1', email: '[email protected]', password: await bcrypt.hash(testPassword, 10) },
18-21
: Clarify Access Control DirectivesThe access control directives
@deny('read', true)
and@omit
on sensitive fields likepassword
are correctly used. However, consider adding comments or documentation to explain their purpose for better maintainability.
24-24
: Suggestion: Add Tests forOrganization
ModelThe
Organization
model is defined but not utilized in any test cases. Adding tests will ensure that the model behaves as expected and maintains consistency with theProfile
inheritance.Consider adding test cases like:
const organization = await db.organization.create({ data: { displayName: 'Org1', type: 'Organization' }, }); expect(organization.displayName).toBe('Org1');tests/regression/tests/issue-1698.test.ts (4)
68-68
: Consider usingfindFirst
instead offindMany
when expecting a single result.On line 68, you are retrieving a single
Skyscraper
record usingfindMany
and accessing the first element of the array. For consistency and clarity, since only one record is expected, consider usingfindFirst
as you did withprivateHouse
on line 62.Apply this diff to use
findFirst
:- const r2 = (await db.skyscraper.findMany({ include: { door: true } }))[0]; + const r2 = await db.skyscraper.findFirst({ include: { door: true } });
45-46
: Optional: Remove console.log statements to clean up test output.The
console.log
statements are useful for debugging but can clutter the test output. If they are no longer needed, consider removing them to keep the test output clean.Apply this diff to remove the
console.log
statements:const door1 = await db.ironDoor.create({ data: { strength: 100, color: 'blue' }, }); - console.log(door1); const door2 = await db.woodenDoor.create({ data: { texture: 'pine', color: 'red' }, }); - console.log(door2); const house1 = await db.privateHouse.create({ data: { size: 5000, door: { connect: { id: door1.id } } }, }); - console.log(house1); const house2 = await db.skyscraper.create({ data: { height: 3000, door: { connect: { id: door2.id } } }, }); - console.log(house2); const r1 = await db.privateHouse.findFirst({ include: { door: true } }); - console.log(r1); expect(r1).toMatchObject({ const r2 = await db.skyscraper.findFirst({ include: { door: true } }); - console.log(r2); expect(r2).toMatchObject({Also applies to: 50-51, 55-56, 60-61, 63-63, 69-70
65-66
: Validate the expected objects in assertions include all relevant properties.When asserting the retrieved
privateHouse
object, consider including all relevant properties in thetoMatchObject
call to ensure comprehensive verification, such assize
forPrivateHouse
.Update the assertion to include the
size
property:expect(r1).toMatchObject({ + size: 5000, door: { color: 'blue', strength: 100 }, });
71-72
: Validate the expected objects in assertions include all relevant properties.Similarly, for the
skyscraper
, consider including theheight
property in the assertion to ensure all critical properties are verified.Update the assertion to include the
height
property:expect(r2).toMatchObject({ + height: 3000, door: { color: 'red', texture: 'pine' }, });
packages/runtime/src/enhancements/node/policy/index.ts (2)
65-69
: Add detailed JSDoc comments for the new functionCurrently, the function lacks detailed documentation for its parameters and return type. Adding JSDoc
@param
and@returns
tags will improve code readability and help other developers understand how to use the function.Here's an example:
/** * Processes a payload for including a relation field in a query. * * @param prisma - The Prisma client contract * @param model - The relation's model name * @param payload - The payload to process * @param options - Internal enhancement options * @param context - The enhancement context */
73-73
: Specify a more precise type thanunknown
forpayload
Using a specific type for
payload
instead ofunknown
enhances type safety and code clarity. If possible, define an interface or type that accurately represents the structure ofpayload
.packages/runtime/src/enhancements/node/query-utils.ts (2)
218-218
: Consider specifying a more precise type for thedata
parameter.Using
any
for thedata
parameter may reduce type safety and potentially introduce runtime errors. Consider defining an interface or a more specific type that describes the expected structure ofdata
.
218-234
: Consider adding unit tests forgetDelegateConcreteModel
.Adding unit tests for this new method will help ensure that it handles all expected cases correctly, especially with various input combinations of
model
anddata
.Do you want me to help create unit tests for this method or open a GitHub issue to track this task?
packages/plugins/trpc/src/helpers.ts (2)
Line range hint
207-243
: Consistent Application of Optional Input ParametersEnsure that
input${inputOptional}
is consistently applied across all relevant procedures. In some cases, the optionality may not be reflected correctly in the parameters ofuseMutation
or other procedures, potentially leading to type inconsistencies.
Line range hint
305-344
: Add.optional()
to Input Schemas for 'aggregate' and 'groupBy'The input schemas for
'aggregate'
and'groupBy'
operations should be made optional since these operations can be invoked without any arguments. Appending.optional()
ensures that the input schema correctly reflects the optional nature of the input.Apply this diff to update the input schemas:
case 'aggregate': - inputType = `$Schema.${capModelName}InputSchema.aggregate`; + inputType = `$Schema.${capModelName}InputSchema.aggregate.optional()`; break; case 'groupBy': - inputType = `$Schema.${capModelName}InputSchema.groupBy`; + inputType = `$Schema.${capModelName}InputSchema.groupBy.optional()`; break;tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts (1)
460-466
: Consistent Use of Models in Direct Database AccessYou use the raw
prisma.post.update
method to modify thedeleted
flag:await prisma.post.update({ where: { id: post.id }, data: { deleted: true } });While this updates the data directly, it bypasses any policy enforcement. Mixing direct database access with policy-enforced operations can lead to inconsistent test results.
Consider using the enhanced
db
client throughout the tests to ensure consistent policy enforcement. If direct access is necessary for setup, clearly document its use to avoid confusion.tests/integration/tests/enhancements/with-policy/auth.test.ts (1)
393-396
: Consider verifying default values in tests forcreateMany
operationsIn line 393, the test checks only the count of the created posts. To ensure that the default values from
auth()
context are correctly set forscore
andauthorName
, consider adding assertions to verify these fields in the created posts.Here's a suggested addition:
await expect(userDb.post.createMany({ data: [{ title: 'def' }] })).resolves.toMatchObject({ count: 1 }); + const createdPosts = await userDb.post.findMany({ where: { title: 'def' } }); + expect(createdPosts[0]).toMatchObject({ title: 'def', score: 10, authorName: 'user1' });tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (3)
1366-1394
: Consider extracting the schema definition for improved readabilityThe inline schema definition within the
loadSchema
call can reduce the readability of the test. Consider extracting the schema into a constant variable or an external file for better code organization and clarity.
1403-1404
: Specify a 'where' clause in 'findFirst' for explicitnessUsing
db.asset.findFirst({ include: { comments: true } })
without awhere
clause may retrieve an unintendedAsset
if multiple assets exist. To ensure the correct asset is retrieved, consider specifying awhere
clause.Apply this diff:
-let r = await db.asset.findFirst({ include: { comments: true } }); +let r = await db.asset.findFirst({ where: { id: post.id }, include: { comments: true } });
1407-1408
: Include a 'where' clause in 'findFirst' for clarity and precisionUsing
db.post.findFirst({ include: { comments: true } })
without awhere
clause may retrieve an unintendedPost
if multiple posts exist. To ensure the correct post is retrieved, consider adding awhere
clause.Apply this diff:
-r = await db.post.findFirst({ include: { comments: true } }); +r = await db.post.findFirst({ where: { id: post.id }, include: { comments: true } });packages/runtime/src/enhancements/node/policy/handler.ts (1)
540-556
: Consider optimizinghasRelationFieldsInPayload
for performanceWhile the current implementation correctly identifies relation fields in the payload, iterating over each item and its fields may impact performance with large datasets. Consider short-circuiting the loops once a relation field is found or optimizing the iteration logic.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (18)
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/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/routers/Post.router.ts
is excluded by!**/generated/**
,!**/generated/**
packages/plugins/trpc/tests/projects/t3-trpc-v10/src/server/api/routers/generated/routers/User.router.ts
is excluded by!**/generated/**
,!**/generated/**
packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/routers/Post.router.ts
is excluded by!**/generated/**
,!**/generated/**
packages/plugins/trpc/tests/projects/t3-trpc-v11/src/server/api/routers/generated/routers/User.router.ts
is excluded by!**/generated/**
,!**/generated/**
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
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
,!**/*.yaml
Files selected for processing (32)
- packages/ide/jetbrains/build.gradle.kts (1 hunks)
- packages/plugins/trpc/src/helpers.ts (8 hunks)
- packages/plugins/trpc/tests/projects/t3-trpc-v10/src/pages/index.tsx (2 hunks)
- packages/plugins/trpc/tests/projects/t3-trpc-v10/src/utils/api.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/create-enhancement.ts (4 hunks)
- packages/runtime/src/enhancements/node/default-auth.ts (1 hunks)
- packages/runtime/src/enhancements/node/delegate.ts (25 hunks)
- packages/runtime/src/enhancements/node/omit.ts (3 hunks)
- packages/runtime/src/enhancements/node/policy/handler.ts (7 hunks)
- packages/runtime/src/enhancements/node/policy/index.ts (2 hunks)
- packages/runtime/src/enhancements/node/policy/policy-utils.ts (6 hunks)
- packages/runtime/src/enhancements/node/proxy.ts (2 hunks)
- packages/runtime/src/enhancements/node/query-utils.ts (1 hunks)
- packages/schema/src/language-server/zmodel-scope.ts (1 hunks)
- packages/schema/src/plugins/zod/generator.ts (6 hunks)
- packages/schema/src/plugins/zod/transformer.ts (13 hunks)
- packages/schema/src/res/stdlib.zmodel (1 hunks)
- tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-delegate/omit-interaction.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-delegate/password-interaction.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-delegate/validation.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-password/with-password.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-policy/auth.test.ts (4 hunks)
- tests/integration/tests/plugins/zod.test.ts (1 hunks)
- tests/regression/tests/issue-1681.test.ts (1 hunks)
- tests/regression/tests/issue-1693.test.ts (1 hunks)
- tests/regression/tests/issue-1695.test.ts (1 hunks)
- tests/regression/tests/issue-1698.test.ts (1 hunks)
- tests/regression/tests/issue-1710.test.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/schema/src/res/stdlib.zmodel
Additional context used
Biome
packages/runtime/src/enhancements/node/delegate.ts
[error] 361-361: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (72)
packages/runtime/src/cross/types.ts (1)
7-7
: LGTM! Verify the usage of the new action in the codebase.The addition of the
'createManyAndReturn'
action to thePrismaWriteActions
array is consistent with the existing actions and expands the functionality of the Prisma write actions.Please ensure that the corresponding changes have been made in the Prisma client and server to handle the
'createManyAndReturn'
operation.Run the following script to verify the usage of the new action:
Verification successful
Verified the integration of
'createManyAndReturn'
across the codebase.The
'createManyAndReturn'
action has been appropriately integrated and is actively used in both test suites and the runtime code. No issues were found regarding its implementation and usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the 'createManyAndReturn' action in the codebase. # Test: Search for the action usage. Expect: Occurrences in the Prisma client and server code. rg --type typescript -A 5 $'createManyAndReturn'Length of output: 82
Script:
#!/bin/bash # Description: Verify the usage of the 'createManyAndReturn' action in the codebase. # Test: Search for the action usage in TypeScript files. rg --type ts -A 5 'createManyAndReturn'Length of output: 20258
tests/regression/tests/issue-1693.test.ts (1)
1-20
: LGTM!The test file is well-structured and follows the standard pattern for writing tests using the Jest testing framework. The use of the
loadSchema
function from the@zenstackhq/testtools
package is appropriate for loading the schema definition. The schema definition is valid and includes the necessary fields and directives for theAnimal
andDog
models. The test is correctly marked as asynchronous using theasync
keyword, and theawait
keyword is used to wait for the schema loading process to complete before the test concludes.packages/plugins/trpc/tests/projects/t3-trpc-v10/src/pages/index.tsx (3)
7-10
: LGTM!The changes to the method signature and the query transformation logic look good.
17-17
: LGTM!The simplified rendering of posts looks good.
6-6
: Verify the usage of the updated method signature across the codebase.The changes to the method signature look good. However, ensure that all calls to
api.post.findMany
have been updated toapi.post.post.findMany
across the codebase to maintain consistency and avoid any potential issues.Run the following script to verify the usage of the updated method signature:
Verification successful
✅ Method Signature Update Verified
All instances of
api.post.findMany
within thet3-trpc-v10
project have been successfully updated toapi.post.post.findMany
. No remaining references were found, ensuring consistency and preventing potential issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `api.post.findMany` have been updated to `api.post.post.findMany`. # Test: Search for the old method signature. Expect: No occurrences. rg --type typescript $'api\.post\.findMany'Length of output: 79
Script:
#!/bin/bash # Description: Verify all calls to `api.post.findMany` have been updated to `api.post.post.findMany`. # Test: Search for the old method signature. Expect: No occurrences. rg --type ts 'api\.post\.findMany'Length of output: 154
Script:
#!/bin/bash # Description: Verify all calls to `api.post.findMany` have been updated to `api.post.post.findMany` within t3-trpc-v10. # Test: Search for the old method signature in the specific project directory. Expect: No occurrences. rg --type ts 'api\.post\.findMany' packages/plugins/trpc/tests/projects/t3-trpc-v10/Length of output: 86
tests/integration/tests/enhancements/with-delegate/validation.test.ts (1)
3-25
: LGTM!The test case is well-structured and provides good coverage for the validation logic of auxiliary fields in mutations. It correctly asserts the expected error message when attempting to update an auxiliary relation field directly.
tests/regression/tests/issue-1681.test.ts (1)
1-29
: LGTM!The test case is well-structured, follows a logical flow, and covers the scenario of creating a post with a default
authorId
value. It uses appropriate assertions to verify the expected behavior.tests/integration/tests/enhancements/with-delegate/password-interaction.test.ts (2)
30-37
: LGTM!The test case is well-structured and follows the AAA (Arrange, Act, Assert) pattern. It correctly asserts that the hashed passwords match the original plaintext passwords when creating a post directly.
39-49
: LGTM!The test case is well-structured and follows the AAA (Arrange, Act, Assert) pattern. It correctly asserts that the hashed passwords match the original plaintext passwords when creating a post nested within a user.
packages/plugins/trpc/tests/projects/t3-trpc-v10/src/utils/api.ts (1)
21-21
: Explicit API method configuration looks good!The change to explicitly specify the HTTP method as
'post'
in thecreateTRPCNext
function call is a good practice. It makes the API configuration more explicit and can help catch type errors if the wrong method is used.Please verify that all API calls in the codebase are using the POST method to ensure compatibility with this change. You can use the following script to search for API calls and check their method:
packages/ide/jetbrains/build.gradle.kts (1)
12-12
: LGTM!The version update from "2.5.1" to "2.6.0" is consistent with the PR objective of merging dev to main for v2.6.0. The version increment follows semantic versioning and does not introduce any functional changes or impact the build process.
tests/integration/tests/enhancements/with-delegate/omit-interaction.test.ts (3)
29-40
: LGTM!The test case correctly validates that
@omit
fields are undefined when querying a concrete model directly.
42-54
: LGTM!The test case correctly validates that
@omit
fields are undefined when querying a base model for a concrete instance, while non-omitted fields liketitle
are still accessible.
75-90
: LGTM!The test case correctly validates that
@omit
fields are undefined when querying a related model that includes the concrete model, while non-omitted fields liketitle
are still accessible.tests/integration/tests/enhancements/with-password/with-password.test.ts (1)
97-127
: LGTM!The test case is well-structured and provides good coverage for the scenario where the password field is marked as non-readable. It correctly verifies that:
- The password is not included in the retrieved user object.
- Attempts to query the user by the password or a substring of the password return
null
.This helps ensure that sensitive information like passwords cannot be accidentally exposed or used for enumeration.
packages/runtime/src/enhancements/node/default-auth.ts (1)
52-59
: LGTM!The addition of the
createManyAndReturn
action type to theactionsOfInterest
array is a logical enhancement. It ensures that thepreprocessWritePayload
method is triggered for this action, allowing the default value preprocessing to be applied consistently across all relevant actions.packages/schema/src/language-server/zmodel-scope.ts (1)
207-207
: LGTM!The change refactors the code to call
createScopeForModel
instead ofcreateScopeForNodes
directly. This makes the code more readable and maintainable, ascreateScopeForModel
encapsulates the logic of checking if the node is aDataModel
and getting its fields (including base fields).The behavior remains the same, so this is a safe refactor.
packages/runtime/src/enhancements/node/proxy.ts (2)
87-87
: Verify the method signature change in the codebase.The change in the method signature to use
PrismaProxyActions
instead ofkeyof PrismaProxyHandler
for themethod
parameter is a good enhancement to improve type specificity.Please ensure that all calls to this method have been updated to pass the correct type for the
method
argument. You can use the following script to search for the method usage:#!/bin/bash # Description: Verify all calls to `deferred` method pass the correct type for `method`. # Test: Search for the method usage. Expect: No occurrences of incorrect type usage. rg --type typescript -A 5 $'deferred\('
72-72
: Verify the method signature change in the codebase.The change in the method signature to use
PrismaProxyActions
instead ofkeyof PrismaProxyHandler
for themethod
parameter is a good enhancement to improve type specificity.Please ensure that all calls to this method have been updated to pass the correct type for the
method
argument. You can use the following script to search for the method usage:packages/runtime/src/cross/nested-write-visitor.ts (1)
172-172
: LGTM!The addition of the new case
createManyAndReturn
to handle a new Prisma operation looks good. Based on the name and the placement alongside the existingcreateMany
case, it seems that the new operation is similar tocreateMany
but with the additional functionality of returning the created records.The logic for the new case is the same as the
createMany
case, which is appropriate given the similarity between the two operations. This addition enhances the functionality of theNestedWriteVisitor
class without affecting the existing cases.packages/schema/src/plugins/zod/transformer.ts (5)
294-294
: LGTM!The change to pass
options.mode
as the second argument towrapWithZodObject
is consistent with the introduction of themode
parameter to determine the object schema wrapping behavior.
405-425
: LGTM!The update to
wrapWithZodObject
to accept amode
parameter and append the corresponding method based on the mode value is a good enhancement. It allows for greater control over the behavior of unknown properties in the object schema.
474-475
: LGTM!Retrieving the
mode
value fromoptions.mode
with a default value of 'strict' is consistent with the changes made towrapWithZodObject
. It ensures that themode
is properly passed down to the object schema generation.
487-488
: LGTM!Passing the
mode
value towrapWithZodObject
when generating the schema for thefindUnique
operation is consistent with the changes made towrapWithZodObject
. It ensures that the specifiedmode
is applied to the object schema wrapping.
499-500
: LGTM!Passing the
mode
value towrapWithZodObject
when generating the schema for thefindFirst
operation is consistent with the changes made towrapWithZodObject
. It ensures that the specifiedmode
is applied to the object schema wrapping.tests/regression/tests/issue-1710.test.ts (5)
34-36
: Good Practice: Verifying Access Control EnforcementGreat job verifying that sensitive fields like
password
remain inaccessible after retrieval. This ensures that access control policies are properly enforced.
38-51
: Properly Testing Error HandlingThe test correctly expects an error when attempting to update the
role
field viadelegate_aux_user
. This ensures that unauthorized updates are adequately prevented.
29-29
: Consistency in Data TypesEnsure that the
displayName
,password
fields conform to expected formats. For instance, validate thatYou can enhance validation by adding constraints or using validation libraries.
19-19
: Security Issue: Store Passwords SecurelyThe
password
field is currently stored as plain text, which poses a significant security risk. It's essential to hash passwords before storing them to protect user data.Consider applying this change to hash the password before saving:
+import bcrypt from 'bcrypt'; const db = enhance(); const user = await db.user.create({ data: { displayName: 'User1', email: '[email protected]', password: '123' }, });Modify the user creation to hash the password:
- data: { displayName: 'User1', email: '[email protected]', password: '123' }, + data: { displayName: 'User1', email: '[email protected]', password: await bcrypt.hash('123', 10) },Ensure you have installed bcrypt and handle asynchronous operations appropriately.
Likely invalid or redundant comment.
12-13
: Verify the Implementation of@@delegate(type)
The use of
@@delegate(type)
in theProfile
model indicates that delegation is based on thetype
field. Ensure that this delegation works as intended across all related operations.Run the following script to verify delegation implementations:
tests/regression/tests/issue-1698.test.ts (1)
41-41
: Verify that theenhance
function is correctly applied.Ensure that the
enhance
function fromloadSchema
is properly enhancing the Prisma client with the expected functionalities, especially thedelegate
enhancement specified.Run the following script to verify the enhancements applied to the Prisma client:
packages/runtime/src/enhancements/node/omit.ts (3)
8-8
: ImportingQueryUtils
module correctlyThe import statement appropriately adds
QueryUtils
from'./query-utils'
, which is necessary for the subsequent usage within the class.
25-26
: Declaration of private memberqueryUtils
The private member
queryUtils
of typeQueryUtils
is correctly declared, enabling encapsulation and proper usage within theOmitHandler
class.
29-29
: Initialization ofqueryUtils
in constructorThe initialization of
this.queryUtils
with a new instance ofQueryUtils
is correctly placed within the constructor, ensuring it's available for use in class methods.packages/runtime/src/enhancements/node/create-enhancement.ts (3)
4-10
: Imports are correctly updatedThe necessary types are properly imported from
'../../types'
to support the enhancements.
16-16
: ImportedpolicyProcessIncludeRelationPayload
successfullyThe import statement correctly includes
policyProcessIncludeRelationPayload
from'./policy'
, which is essential for processing inclusion payloads in policy enhancements.
110-110
: Correct application ofwithDelegate
enhancementThe
withDelegate
function is appropriately applied withresult
,options
, andcontext
parameters, ensuring the delegate enhancement is correctly integrated.tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts (4)
383-392
: Accessing Undefined Properties Due to Policy RestrictionsAfter updating the
deleted
flag, you attempt to access thetitle
property:expect(readAsset1.title).toBeUndefined(); expect(userWithAssets1.assets[0].title).toBeUndefined();Since the
title
field is part of thePost
model and is denied by policy whendeleted
istrue
, accessing it returnsundefined
. This behavior is expected due to the policy, but it might lead to confusion.Please confirm that the policies are correctly applied and that attempting to access restricted fields behaves as intended. If necessary, adjust the test assertions to account for the policy restrictions.
518-526
: Field-Level Policy Enforcement VerificationIn this test, when using
db2
withauth().id != 1
, you observe that thefoo
andbar
fields areundefined
due to field-level policies:expect(post2.foo).toBeUndefined(); expect(post2.bar).toBeUndefined();This confirms that the field-level policies are correctly enforced.
The test accurately verifies that field-level policies are working as intended for different authentication contexts.
313-319
: Data Created Despite Policy RejectionIn this test case, you expect the
db.post.create
operation to be rejected by policy:await expect( db.post.create({ data: { title: 'Post2', deleted: true, userId: user.id } }) ).toBeRejectedByPolicy();However, you then check that the total number of posts is 2:
// actually created await expect(prisma.post.count()).resolves.toBe(2);This suggests that despite the policy rejection, the post was created in the database. This could indicate an issue with policy enforcement or a misconfiguration in the test setup.
Please verify that the policy correctly prevents the creation of the post when it is supposed to be rejected. Ensure that the policy enforcement is properly configured and that the test is accurately assessing the policy behavior.
320-324
: Data Updated Despite Policy RejectionYou attempt to update a post and expect the operation to be rejected by policy:
await expect(db.post.update({ where: { id: 2 }, data: { title: 'Post2-1' } })).toBeRejectedByPolicy();Yet, you assert that the post was actually updated:
// actually updated await expect(prisma.post.findUnique({ where: { id: 2 } })).resolves.toMatchObject({ title: 'Post2-1' });This indicates that the update occurred despite the expected policy rejection. This could point to an issue with the policy not effectively preventing the update operation.
Please ensure that the policy enforcement correctly blocks the update operation when it is supposed to be rejected. Verify that the policy conditions are properly defined and that the test accurately reflects the intended behavior.
packages/schema/src/plugins/zod/generator.ts (4)
338-349
: Verify default behavior whenmode
is undefinedWhen
this.options.mode
is not specified or does not match 'strip' or 'passthrough', the switch statement defaults towriter.writeLine(').strict();');
(line 347), enforcing 'strict' mode. Please confirm if 'strict' mode is intended as the default behavior. If 'strip' mode is meant to be the default, consider adjusting the default case accordingly to ensure the expected schema generation behavior.
399-401
: Approved: Correct handling of discriminator fieldsThe code correctly constructs the
omitDiscriminators
string based on the presence of delegate discriminator fields, ensuring they are omitted appropriately.
490-490
: Confirm the order of applyingmakePartial
andmakePassthrough
At line 490,
prismaUpdateSchema
is wrapped withmakePartial
first and thenmakePassthrough
. Verify that this order is intentional and aligns with the desired schema behavior. Swapping the order may alter how additional properties are handled in the schema.
512-519
: Verify the exclusion of discriminator fields from partial fieldsIn the block starting at line 512, when making
createSchema
partial with specific fields, discriminator fields are excluded (lines 515-518). Ensure this logic is correct to prevent the schema compilation errors mentioned in the comment and that it aligns with the intended schema structure.tests/integration/tests/enhancements/with-policy/auth.test.ts (3)
423-427
: Tests correctly verify explicit values override defaultsThe tests appropriately verify that explicitly provided
authorName
values are not overridden by the default values fromauth()
when usingcreateMany
andcreateManyAndReturn
. The assertions confirm that the providedauthorName
(overrideName
) is preserved.
655-666
: Tests for batch creation with associations are accurateThe code correctly tests the creation of
post
entities associated withstats
entities usingcreateMany
andcreateManyAndReturn
. The assertions verify that thestatsId
field is properly set, ensuring that relationships are correctly established.
668-670
: Validation of nested relation creation is correctThe test effectively verifies that a
post
can be created with a connectedstats
entity using nestedconnect
. Theawait expect
assertion confirms successful creation without policy rejections.tests/integration/tests/plugins/zod.test.ts (4)
854-912
: Great Work Adding Comprehensive Tests for 'strict' ModeThe test
'is strict by default'
accurately verifies that in default'strict'
mode, additional fields are rejected as expected. The assertions correctly confirm that extra fields cause validation to fail, ensuring the schema behaves properly in strict mode.
913-955
: Effective Testing of 'strip' Mode BehaviorThe test
'works in strip mode'
correctly ensures that additional fields are stripped from the parsed data. The assertions validate that while the parsing succeeds, extra fields likex
are removed, confirming the expected behavior in'strip'
mode.
956-998
: Validation of 'passthrough' Mode FunctionalityThe test
'works in passthrough mode'
effectively checks that additional fields are retained in the parsed data. By asserting thatparsed.data.x
equals the input value, it confirms that'passthrough'
mode allows extra fields without validation errors.
999-1027
: Appropriate Error Handling for Invalid Mode ConfigurationThe test
'complains about invalid mode'
correctly verifies that providing an invalid mode ('xyz'
) results in an error. The use oftoThrow(/Invalid mode/)
ensures that the error message is as expected, enhancing the robustness of the plugin's configuration handling.packages/runtime/src/enhancements/node/delegate.ts (8)
5-5
: Added import oftraverse
moduleThe
traverse
module is imported to enable deep traversal of objects, necessary for the implementation ofsanitizeMutationPayload
.
18-18
: ImportedEnhancementContext
for context managementThe
EnhancementContext
type is imported to pass contextual information throughout delegate operations.
25-29
: UpdatedwithDelegate
function to includecontext
parameterThe addition of the
context
parameter to thewithDelegate
function and its propagation toDelegateProxyHandler
allows for enhanced contextual operations within delegates.Also applies to: 33-33
42-47
: ModifiedDelegateProxyHandler
constructor to acceptcontext
The constructor now accepts
context
as aprivate readonly
parameter, enabling the handler to utilize the enhancement context during operations.
88-92
: Ensured discriminator field is selected in queriesThe
ensureDiscriminatorSelection
method ensures that the discriminator field is included inselect
oromit
arguments, which is necessary for correctly assembling the hierarchy of inherited models during post-processing.Also applies to: 111-126
385-386
: IntroducedsanitizeMutationPayload
to validate mutation payloadsThe
sanitizeMutationPayload
method traverses mutation payloads to ensure that auxiliary relation fields are not set directly, throwing an error if they are. This prevents unintended modifications to auxiliary relations.Also applies to: 402-418, 432-433, 473-474, 661-662, 682-683, 711-713
1246-1246
: Updated recursive assembly of hierarchical modelsIn
assembleHierarchy
, recursive calls to assemble base and sub-models are correctly updated to reflect changes in the model hierarchy.Also applies to: 1300-1300
178-178
: Updated methods to be asynchronous to handle async operationsMethods like
injectSelectIncludeHierarchy
,buildSelectIncludeHierarchy
, andinjectConcreteIncludeRecursively
are now asynchronous to handle potentially asynchronous operations, such as processing include relation payloads. Ensure that all calls to these methods are updated withawait
.Please run the following script to verify that all calls to the updated asynchronous methods are using
await
:Also applies to: 198-198, 220-220, 232-232, 236-236, 260-260, 331-331, 345-357, 360-360, 363-363, 365-365, 506-506, 728-728, 747-747, 941-941, 993-993
tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (1)
1365-1409
: Well-structured test case verifying hierarchy and delegationThe test
'merges hierarchy correctly'
effectively validates the functionality of polymorphic relationships and delegation within the schema. The creation ofPost
andTextComment
instances and the subsequent queries and assertions appropriately test the merging of hierarchies.packages/runtime/src/enhancements/node/policy/handler.ts (6)
Line range hint
15-19
: Imports added are appropriate and necessaryThe added imports (
getModelInfo
,requireField
,resolveField
,FieldInfo
,ModelMeta
) are necessary for the new methods and functionalities implemented in the class.
439-449
: Logic for handlingcreateMany
operations is correctly updatedThe introduction of
preprocessCreateManyPayload(args)
effectively determines whether to executecreateMany
directly or convert the operation into individualcreate
transactions with post-write checks. This ensures proper handling of relation fields and post-create validations.
Line range hint
475-497
: Update tocreateManyAndReturn
method enhances operation controlBy implementing
preprocessCreateManyPayload(args)
, the method now accurately decides between direct execution ofcreateManyAndReturn
or processing as individual creates with necessary post-write checks. This modification improves reliability when dealing with complex payloads.
518-538
: New methodpreprocessCreateManyPayload
centralizes preprocessing logicThe addition of
preprocessCreateManyPayload
encapsulates the decision-making process forcreateMany
operations, enhancing readability and maintainability. It checks for post-create validation needs and the presence of relation fields efficiently.
585-590
: Addition ofaction
parameter todoCreateMany
enhances clarityIntroducing the
action
parameter to thedoCreateMany
method allows differentiation betweencreateMany
andcreateManyAndReturn
operations. This improves code clarity and aids in debugging and logging.
606-606
: Enhanced logging provides better traceabilityUpdating the logging statement to include the specific action (
createMany
orcreateManyAndReturn
) enhances traceability and makes it easier to debug issues related to these operations.packages/runtime/src/enhancements/node/policy/policy-utils.ts (5)
611-611
: Ensureargs.where
is updated with merged conditionsThe assignment of
args.where = this.mergeWhereClause(...)
ensures that the merged conditions are correctly applied toargs.where
. This change fixes potential issues where the merged conditions were not being reflected inargs.where
, leading to incorrect query results.Also applies to: 617-617, 633-633
1101-1103
: Add logging for failed read-back operationsIntroducing logging when a read-back operation fails enhances debuggability. The log message
[policy] cannot read back ${model}
provides valuable information for diagnosing issues related to access policies not allowing a read-back of the specified model.
1387-1390
: Handle polymorphic entities using concrete modelsBy obtaining the
realModel
usingthis.getDelegateConcreteModel(model, data)
and passing it todoPostProcessForRead
, the code now accurately processes polymorphic entities. This ensures that field-level policy checks are performed on the correct concrete model, enhancing the correctness of access control.
1555-1559
: OptimizemergeWhereClause
for trivial conditionsAdding checks for
this.isTrue(extra)
andthis.isFalse(extra)
inmergeWhereClause
improves efficiency by handling trivial cases upfront. Ifextra
is always true, it returnswhere
directly. Ifextra
is always false, it simplifies the condition by returningthis.makeFalse()
.
1572-1572
: Preserve query structure when merging conditionsThe updates at these lines ensure that when merging where clauses, the structure necessary for unique queries is preserved. By properly wrapping conditions under
AND
, the code maintains the integrity of unique filters, preventing potential issues with query execution.Also applies to: 1575-1575
No description provided.