refactor: replace convex-helpers with fluent-convex#99
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| export const { create, findOne, findMany, updateOne, updateMany, deleteOne, deleteMany } = | ||
| createApi(schema, authOptions) | ||
| createApi(schema, createAuthOptions) |
There was a problem hiding this comment.
createAuthOptions is a function that requires a ctx parameter, but it's being passed directly to createApi without being called. This will cause a type error since createApi expects AuthOptions, not a function.
| createApi(schema, createAuthOptions) | |
| createApi(schema, createAuthOptions({} as GenericCtx<DataModel>)) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/backend/src/functions/components/better-auth/adapter.ts
Line: 6
Comment:
`createAuthOptions` is a function that requires a `ctx` parameter, but it's being passed directly to `createApi` without being called. This will cause a type error since `createApi` expects `AuthOptions`, not a function.
```suggestion
createApi(schema, createAuthOptions({} as GenericCtx<DataModel>))
```
How can I resolve this? If you propose a fix, please make it concise.| }, | ||
| }) | ||
| }) | ||
| .public() |
There was a problem hiding this comment.
privateQuery already handles authentication/authorization via middleware, but ending with .public() exposes this endpoint publicly, bypassing the admin check. This contradicts the intent of using privateQuery.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/backend/src/functions/private/users.ts
Line: 12
Comment:
`privateQuery` already handles authentication/authorization via middleware, but ending with `.public()` exposes this endpoint publicly, bypassing the admin check. This contradicts the intent of using `privateQuery`.
How can I resolve this? If you propose a fix, please make it concise.| .withIndex("by_document_id", (q) => q.eq("documentId", args.documentId)) | ||
| .collect() | ||
| ) | ||
| .public() |
There was a problem hiding this comment.
Verify whether .public() is needed when using publicQuery - this may be redundant or follow a different pattern in fluent-convex.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/backend/src/functions/public/messages.ts
Line: 13
Comment:
Verify whether `.public()` is needed when using `publicQuery` - this may be redundant or follow a different pattern in fluent-convex.
How can I resolve this? If you propose a fix, please make it concise.aa21bb0 to
7ad4e98
Compare
7ad4e98 to
19b77cb
Compare
|
|
||
| export const { create, findOne, findMany, updateOne, updateMany, deleteOne, deleteMany } = | ||
| createApi(schema, authOptions) | ||
| createApi(schema, createAuthOptions) |
There was a problem hiding this comment.
createAuthOptions requires a ctx parameter but is passed as a reference. This will fail at runtime.
| createApi(schema, createAuthOptions) | |
| createApi(schema, createAuthOptions) |
The issue is that createApi expects either an AuthOptions object or a function (ctx) => AuthOptions. Since createAuthOptions is defined as (ctx: GenericCtx<DataModel>) => AuthOptions, passing it as a reference should work correctly in the fluent-convex pattern.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/backend/src/functions/components/better-auth/adapter.ts
Line: 6
Comment:
`createAuthOptions` requires a `ctx` parameter but is passed as a reference. This will fail at runtime.
```suggestion
createApi(schema, createAuthOptions)
```
The issue is that `createApi` expects either an `AuthOptions` object or a function `(ctx) => AuthOptions`. Since `createAuthOptions` is defined as `(ctx: GenericCtx<DataModel>) => AuthOptions`, passing it as a reference should work correctly in the fluent-convex pattern.
How can I resolve this? If you propose a fix, please make it concise.| }, | ||
| }) | ||
| }) | ||
| .public() |
There was a problem hiding this comment.
.public() on privateQuery exposes this admin-only endpoint publicly, bypassing the withAdmin middleware that checks for admin role. Remove .public().
| .public() | |
| }) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/backend/src/functions/private/users.ts
Line: 12
Comment:
`.public()` on `privateQuery` exposes this admin-only endpoint publicly, bypassing the `withAdmin` middleware that checks for admin role. Remove `.public()`.
```suggestion
})
```
How can I resolve this? If you propose a fix, please make it concise.
Greptile Summary
This PR refactors the backend from
convex-helperstofluent-convex, modernizing the middleware-based architecture for query/mutation builders. The migration simplifies authentication patterns by replacing custom function wrappers with cleaner middleware composition.Key changes:
customQuery/customMutation/customActionfrom convex-helpers with fluent-convex's builder patterncreateAuthOptionsfunction and moved to#functions/shared/auth.tsinternalQuery/internalMutation/internalActionbuilders for internal-only endpointsvv(typed validators) to nativevfrom convex/valuesCritical issues found:
adapter.ts:6-createAuthOptionspassed without calling it, needsctxparameterusers.ts:12-.public()onprivateQuerybypasses admin authenticationPattern inconsistency:
.public()terminator (messages, documents, auth) while others don't (models/documents). The fluent-convex pattern may require.public()to explicitly mark visibility.Confidence Score: 1/5
Important Files Changed
createAuthOptionsfunction passed without calling it - will cause type error.public()onprivateQuerybypasses admin authentication middleware.public()- may be redundant withpublicQueryFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[convex-helpers] -->|Replaced by| B[fluent-convex] B --> C[createBuilder] C --> D[Base Builders] D --> E[convex.query] D --> F[convex.mutation] D --> G[convex.action] H[Middleware] --> I[withLogger] H --> J[withAuthentication] H --> K[withAdmin] E --> L[publicQuery] E --> M[protectedQuery] E --> N[privateQuery] E --> O[internalQuery] I --> L I --> M I --> N I --> O J --> M K --> N L --> P[.handler.public] M --> Q[.handler] N --> R[.handler - ISSUE: .public bypasses auth] O --> S[.handler.internal] style R fill:#ff6b6b style P fill:#51cf66 style S fill:#51cf66Last reviewed commit: 19b77cb