-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: replace convex-helpers with fluent-convex #99
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||
| import { createApi } from "@convex-dev/better-auth" | ||||||
| import schema from "#functions/components/better-auth/schema.ts" | ||||||
| import { authOptions } from "#functions/shared/auth/options.ts" | ||||||
| import { createAuthOptions } from "#functions/shared/auth.ts" | ||||||
| import schema from "./schema" | ||||||
|
|
||||||
| export const { create, findOne, findMany, updateOne, updateMany, deleteOne, deleteMany } = | ||||||
| createApi(schema, authOptions) | ||||||
| createApi(schema, createAuthOptions) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The issue is that Prompt To Fix With AIThis 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. |
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,9 @@ | ||
| import type { GenericCtx } from "@convex-dev/better-auth" | ||
| import { createAuth } from "@init/auth/server" | ||
| import type { DataModel } from "#functions/_generated/dataModel.js" | ||
| import { authOptions } from "#functions/shared/auth/options.ts" | ||
| import { createAuthOptions } from "#functions/shared/auth.ts" | ||
|
|
||
| // Export a static instance for Better Auth schema generation | ||
| // Static instance for Better Auth schema generation only | ||
| export const auth = createAuth({ | ||
| // Casting as GenericCtx<DataModel> since the Convex component does not need | ||
| // the running context | ||
| ...authOptions({} as GenericCtx<DataModel>), | ||
| ...createAuthOptions({} as GenericCtx<DataModel>), | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { defineComponent } from "convex/server" | ||
|
|
||
| const component = defineComponent("betterAuth") | ||
| const component = defineComponent("auth") | ||
|
|
||
| export default component |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { defineSchema } from "convex/server" | ||
| import { tables } from "#functions/components/better-auth/schema.generated.ts" | ||
|
|
||
| export default defineSchema({ ...tables }) | ||
| export default defineSchema({ ...tables, user: tables.user.index("by_email", ["email"]) }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,13 @@ | ||
| import { httpRouter } from "convex/server" | ||
| import { convexAuth } from "#functions/shared/auth/index.ts" | ||
| import { authComponent } from "#functions/shared/auth/options.ts" | ||
| import { authComponent, convexAuth } from "#functions/auth.ts" | ||
| import env from "#functions/shared/env.ts" | ||
|
|
||
| const http = httpRouter() | ||
|
|
||
| authComponent.registerRoutes(http, convexAuth) | ||
| authComponent.registerRoutes(http, convexAuth, { | ||
| cors: { | ||
| allowedOrigins: env.AUTH_TRUSTED_ORIGINS, | ||
| }, | ||
| }) | ||
|
|
||
| export default http |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { UnauthenticatedError } from "@init/error" | ||
| import { v } from "convex/values" | ||
| import { protectedQuery, publicQuery } from "#functions/shared/convex.ts" | ||
|
|
||
| export const getDocument = publicQuery | ||
| .input({ name: v.string() }) | ||
| .handler(async (ctx, { name }) => { | ||
| const document = await ctx.db | ||
| .query("documents") | ||
| .withIndex("by_name", (q) => q.eq("name", name)) | ||
| .unique() | ||
|
|
||
| return document | ||
| }) | ||
|
|
||
| export const withDocument = protectedQuery.createMiddleware(async (ctx, next) => { | ||
| const identity = await ctx.auth.getUserIdentity() | ||
|
|
||
| if (!identity) { | ||
| throw new UnauthenticatedError() | ||
| } | ||
|
|
||
| const document = await ctx.db | ||
| .query("documents") | ||
| .withIndex("by_name", (q) => q.eq("name", identity.subject)) | ||
| .unique() | ||
|
|
||
| return next({ ...ctx, document }) | ||
| }) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,14 +1,12 @@ | ||||||
| import type { UserWithRole } from "@init/auth/server/plugins" | ||||||
| import { convexAuth } from "#functions/shared/auth/index.ts" | ||||||
| import { authComponent } from "#functions/shared/auth/options.ts" | ||||||
| import { authComponent, convexAuth } from "#functions/auth.ts" | ||||||
| import { privateQuery } from "#functions/shared/convex.ts" | ||||||
|
|
||||||
| export const list = privateQuery({ | ||||||
| handler: async (ctx) => { | ||||||
| export const list = privateQuery | ||||||
| .handler(async (ctx) => { | ||||||
| const { auth, headers } = await authComponent.getAuth(convexAuth, ctx) | ||||||
|
|
||||||
| const result = await auth.api.listUsers({ headers, query: { limit: 100 } }) | ||||||
|
|
||||||
| return result.users as UserWithRole[] | ||||||
| }, | ||||||
| }) | ||||||
| }) | ||||||
| .public() | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis 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.There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis 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. |
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { authComponent } from "#functions/shared/auth.ts" | ||
| import { publicQuery } from "#functions/shared/convex.ts" | ||
|
|
||
| export const getCurrentUser = publicQuery | ||
| .handler(async (ctx) => { | ||
| const user = await authComponent.getAuthUser(ctx) | ||
|
|
||
| return user ?? null | ||
| }) | ||
| .public() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { publicQuery } from "#functions/shared/convex.ts" | ||
|
|
||
| export const list = publicQuery({ | ||
| handler: async (ctx) => await ctx.db.query("documents").collect(), | ||
| }) | ||
| export const list = publicQuery | ||
| .handler(async (ctx) => await ctx.db.query("documents").collect()) | ||
| .public() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,13 @@ | ||
| import { publicQuery, vv } from "#functions/shared/convex.ts" | ||
| import { v } from "convex/values" | ||
| import { publicQuery } from "#functions/shared/convex.ts" | ||
|
|
||
| export const list = publicQuery({ | ||
| args: { documentId: vv.id("documents") }, | ||
| handler: async (ctx, args) => | ||
| await ctx.db | ||
| .query("messages") | ||
| .withIndex("by_document_id", (q) => q.eq("documentId", args.documentId)) | ||
| .collect(), | ||
| }) | ||
| export const list = publicQuery | ||
| .input({ documentId: v.id("documents") }) | ||
| .handler( | ||
| async (ctx, args) => | ||
| await ctx.db | ||
| .query("messages") | ||
| .withIndex("by_document_id", (q) => q.eq("documentId", args.documentId)) | ||
| .collect() | ||
| ) | ||
| .public() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verify whether Prompt To Fix With AIThis 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. |
||
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.
createAuthOptionsis a function that requires actxparameter, but it's being passed directly tocreateApiwithout being called. This will cause a type error sincecreateApiexpectsAuthOptions, not a function.Prompt To Fix With AI