-
-
Notifications
You must be signed in to change notification settings - Fork 127
fix: default auth without user context #1015
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
fix: default auth without user context #1015
Conversation
WalkthroughWalkthroughThe recent update enhances authentication handling by initializing context as an empty object by default and refining user context checks. It eliminates the preliminary check for 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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/runtime/src/enhancements/default-auth.ts (3 hunks)
- tests/integration/tests/enhancements/with-policy/auth.test.ts (1 hunks)
Additional comments: 4
packages/runtime/src/enhancements/default-auth.ts (3)
- 18-18: The modification to default the
contextparameter to an empty object in thewithDefaultAuthfunction is a good practice. It ensures that the function can be called without explicitly passing a context, which aligns with the PR's objective to support actions that do not require user context.- 35-35: The change in the
DefaultAuthHandlerconstructor to directly assignthis.context.usertothis.userContextwithout checking for its existence is aligned with the PR's objective. However, it's important to ensure that downstream code that relies onthis.userContextis prepared to handle it being potentiallyundefined.Verification successful
The verification process has confirmed that the code in
default-auth.tsis prepared to handlethis.userContextbeing potentiallyundefined. Specifically, there is a conditional check to ensurethis.userContextexists before proceeding with certain logic, and its usage in a function call suggests that the function is expected to handleundefinedas a valid input. Therefore, the change in theDefaultAuthHandlerconstructor aligns with the PR's objective, and the downstream code is indeed prepared forthis.userContextbeing potentiallyundefined.* 94-96: The addition of a check for `this.userContext` in the `getDefaultValueFromAuth` method before throwing an error is crucial for the PR's objective. This ensures that errors related to the absence of a user context are only thrown when necessary, specifically when attempting to use `auth()` in `@default` without a user context. This change improves the flexibility of the `withDefaultAuth` enhancer by allowing it to be used in scenarios that do not involve user authentication.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all usages of `this.userContext` properly handle the case where it might be undefined. ast-grep --lang typescript --pattern $'this.userContext'Length of output: 366
tests/integration/tests/enhancements/with-policy/auth.test.ts (1)
- 509-536: The addition of the test case
Default auth() without user contextis a valuable contribution to the test suite. It verifies that an error is thrown when attempting to useauth()in@defaultwithout a user context, which aligns with the changes made in the PR. This test ensures that the new behavior works as expected and helps prevent regressions in the future.
|
As tests failed anyway, I could also improve the error message. What about: "Using `auth()` in `@default` for the field `${fieldInfo.name}` requires a user context" or just "`${fieldInfo.name}` requires a user context"? |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- packages/plugins/tanstack-query/src/runtime/vue.ts (2 hunks)
- packages/runtime/src/enhancements/default-auth.ts (3 hunks)
- tests/integration/tests/enhancements/with-policy/auth.test.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/runtime/src/enhancements/default-auth.ts
- tests/integration/tests/enhancements/with-policy/auth.test.ts
Additional comments: 2
packages/plugins/tanstack-query/src/runtime/vue.ts (2)
- 64-64: The update to the
optionsparameter type inuseModelQueryfunction to exclude thequeryKeyproperty is a good practice. It ensures that thequeryKeyis managed internally and not overridden by external inputs, which could potentially lead to unexpected behavior. This change enhances type safety and clarity in the function's usage.- 90-90: Similarly, the modification in the
useInfiniteModelQueryfunction to exclude thequeryKeyfrom theoptionsparameter type aligns with best practices for managing internal state and parameters. By preventing external overriding of thequeryKey, it maintains the integrity of the query management within the function, contributing to more predictable and safer code.
e1979bd to
32414f4
Compare
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/runtime/src/enhancements/default-auth.ts (3 hunks)
- tests/integration/tests/enhancements/with-policy/auth.test.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/runtime/src/enhancements/default-auth.ts
- tests/integration/tests/enhancements/with-policy/auth.test.ts
The withDefaultAuth enhancer should allow actions that do not use this feature. However, it throws an error when there is a @default(auth()) in the schema for an anonymous user, regardless of whether the action involves it.
Summary by CodeRabbit
useModelQueryanduseInfiniteModelQueryfunctions to excludequeryKeyproperty.