Conversation
✅ Deploy Preview for flipside1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Caution Review failedThe pull request is closed. WalkthroughThis update transitions the authentication system from Clerk to "better-auth" with Drizzle ORM integration, introducing new authentication modules, database schemas, and API route handlers. It updates the articles UI to use the new auth client, removes Clerk and related middleware, and refactors the articles router to the new session management approach. Several obsolete files and migrations are deleted, and the database schema for articles is consolidated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Navbar
participant authClient
participant ArticlesPage
participant API
participant DB
User->>Navbar: Loads app
Navbar->>authClient: useSession()
authClient-->>Navbar: Session/User info
User->>ArticlesPage: Navigates to articles
ArticlesPage->>authClient: useSession()
alt No session
ArticlesPage-->>User: Show "No session" message
else Session exists
ArticlesPage->>API: Fetch articles (with user session)
API->>DB: Query articles by userId
DB-->>API: Return articles
API-->>ArticlesPage: Article data
ArticlesPage-->>User: Render articles UI
end
User->>Navbar: Clicks "Sign In"
Navbar->>authClient: signIn.social("google", callbackURL)
authClient-->>User: Redirect to Google OAuth
User->>Navbar: Clicks "Sign Out"
Navbar->>authClient: signOut()
authClient-->>User: Session cleared
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
drizzle.config.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.3_eslint@9.28.0_typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. src/app/layout.tsxOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.3_eslint@9.28.0_typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. src/app/articles/page.tsxOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.3_eslint@9.28.0_typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting.
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (26)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
package.json (1)
24-37: 💡 Verification agent🧩 Analysis chain
Potential conflict with existing authentication provider.
The addition of
better-authwhile retaining@clerk/nextjsmay cause conflicts since both are authentication solutions. If you're migrating from Clerk to better-auth, consider removing the Clerk dependency once the migration is complete.
🏁 Script executed:
#!/bin/bash # Check for any remaining Clerk usage in the codebase rg -i "clerk|useAuth|useUser" --type ts --type tsx --type js --type jsxLength of output: 101
🏁 Script executed:
#!/bin/bash # Correctly search for any remaining Clerk usage in JS/TS files rg "@clerk/nextjs" -t ts -t tsx -t js -t jsx rg -i "useAuth|useUser" -t ts -t tsx -t js -t jsxLength of output: 152
🏁 Script executed:
#!/bin/bash # Search for any remaining Clerk imports and hook usage in code without relying on ripgrep file‐type detection rg "from ['\"]@clerk/nextjs" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' rg "@clerk/nextjs" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' rg -i "useAuth|useUser" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Length of output: 999
Remove residual Clerk imports before completing migration to better-auth
The following files still import from
@clerk/nextjs, indicating Clerk hasn’t been fully removed. Please replace or remove these before finalizing your switch tobetter-auth:• src/middleware.ts —
clerkMiddleware
• src/server/api/routers/articles.ts —auth
• src/app/layout.tsx —ClerkProvider
• src/app/(auth)/sign-in/page.tsx —SignIn as ClerkSignInUI
• src/app/(auth)/sign-up/page.tsx —SignUp as ClerkSignUpUIEnsure you’ve updated these to use your new auth solution (or removed them if no longer needed) to avoid bundle bloat and runtime conflicts.
🧹 Nitpick comments (7)
src/lib/auth.ts (1)
6-15: Consider supplying a signing secret & other Better-Auth options
betterAuth()usually expects a top-levelsecret,cookies, etc. Relying on package defaults may produce weak signing keys in non-production environments. Double-check the docs and pass an explicit secret (e.g.process.env.AUTH_SECRET!) to stay compliant with OWASP recommendations.src/app/articles/page.tsx (2)
24-26:signOutshould surface failure states as wellSame rationale as above—wrap the call in
try/catchand give the user visual feedback if it fails.
30-39: UI/UX nit – label buttons descriptively“Test” / “signOut” don’t convey intent in production. Consider “Sign in with Google” and “Sign out”.
src/app/articles/old-page.tsx (2)
118-126: Search placeholder advertises “description” but filter ignores itEither update the placeholder or include description in the predicate:
- return articles.filter((article) => { - const matchesSearch = article.title - .toLowerCase() - .includes(searchQuery.toLowerCase()); - return matchesSearch; - }); + const q = searchQuery.toLowerCase(); + return articles.filter((a) => + a.title.toLowerCase().includes(q) || + (a.description ?? "").toLowerCase().includes(q) + );
50-63:handleNewArticle/handleUpdateArticleonly log – consider cache invalidationThe comments hint at a missing refetch. Call
utils.articles.getAll.invalidate()(or optimistic update) so the UI reflects the mutation.src/server/db/auth-schema.ts (2)
7-7: Use clearer boolean default syntax.The expression
.$defaultFn(() => !1)is unnecessarily obscure. Usefalsedirectly for better readability.- emailVerified: boolean('email_verified').$defaultFn(() => !1).notNull(), + emailVerified: boolean('email_verified').default(false).notNull(),
9-10: Consider using SQL defaults for timestamps and auto-update behavior.The current approach uses JavaScript Date objects for defaults, but SQL-level defaults are generally preferred for consistency and performance. Also consider auto-update behavior for
updatedAtfields.- createdAt: timestamp('created_at').$defaultFn(() => new Date).notNull(), - updatedAt: timestamp('updated_at').$defaultFn(() => new Date).notNull() + createdAt: timestamp('created_at').defaultNow().notNull(), + updatedAt: timestamp('updated_at').defaultNow().notNull().$onUpdate(() => new Date())Note: You may need to handle the auto-update behavior at the application level if Drizzle doesn't support
$onUpdatefor your use case.Also applies to: 17-18, 36-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
drizzle.config.ts(1 hunks)drizzle/0005_freezing_spyke.sql(1 hunks)drizzle/meta/0005_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)package.json(1 hunks)src/app/api/auth/[...all]/route.ts(1 hunks)src/app/articles/old-page.tsx(1 hunks)src/app/articles/page.tsx(1 hunks)src/lib/auth-client.ts(1 hunks)src/lib/auth.ts(1 hunks)src/server/db/auth-schema.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/lib/auth.ts (2)
src/server/db/index.ts (1)
db(18-18)src/server/db/auth-schema.ts (4)
verification(40-47)account(24-38)session(13-22)user(3-11)
src/app/api/auth/[...all]/route.ts (1)
src/lib/auth.ts (1)
auth(6-22)
src/app/articles/old-page.tsx (5)
src/server/db/schema.ts (1)
articles(20-34)src/trpc/react.tsx (1)
api(25-25)src/lib/types.ts (1)
Article(1-11)src/components/ui/button.tsx (1)
Button(59-59)src/components/ui/input.tsx (1)
Input(21-21)
🔇 Additional comments (6)
drizzle/meta/_journal.json (1)
39-46: Migration journal entry looks correct.The new migration entry follows the proper format and is consistent with existing entries.
src/app/api/auth/[...all]/route.ts (1)
1-4: Clean and correct better-auth integration.The API route implementation follows the standard better-auth pattern for Next.js integration. The dynamic catch-all route will properly handle all authentication endpoints.
src/lib/auth-client.ts (1)
1-5: Proper client-side authentication setup.The auth client configuration is clean and follows better-auth best practices. The commented baseURL is appropriate as it defaults to the current domain.
drizzle.config.ts (1)
6-6: Correct schema configuration for multiple files.The update to include both schema files is necessary for Drizzle Kit to process the new authentication tables alongside existing schemas.
drizzle/0005_freezing_spyke.sql (1)
1-50: Authentication schema migration looks well-structured.The migration properly defines the four core authentication tables with appropriate constraints, data types, and referential integrity. The cascade delete relationships ensure orphaned records are cleaned up when users are deleted.
src/server/db/auth-schema.ts (1)
21-21:Details
✅ Verification successful
Verify foreign key relationships match migration.
The foreign key definitions look correct with proper cascade delete behavior. Ensure these match the SQL migration exactly to avoid schema drift.
Also applies to: 28-28
🏁 Script executed:
#!/bin/bash # Verify that the TypeScript schema foreign keys match the SQL migration echo "Checking foreign key consistency between schema and migration..." # Extract foreign key definitions from migration echo "=== Migration Foreign Keys ===" grep -A 2 -B 2 "FOREIGN KEY" drizzle/0005_freezing_spyke.sql # Extract foreign key references from TypeScript schema echo "=== TypeScript Schema References ===" grep -n "references.*onDelete" src/server/db/auth-schema.tsLength of output: 970
Confirmed: Foreign key definitions match migration
- In drizzle/0005_freezing_spyke.sql:
• account.user_id → user.id ON DELETE cascade, ON UPDATE no action
• session.user_id → user.id ON DELETE cascade, ON UPDATE no action- In src/server/db/auth-schema.ts (lines 21 & 28):
•.references(() => user.id, { onDelete: 'cascade' })(Drizzle’s defaultonUpdateisno action)No discrepancies found—schema and migration are in sync.
| google: { | ||
| clientId: process.env.GOOGLE_CLIENT_ID as string, | ||
| clientSecret: process.env.GOOGLE_CLIENT_SECRET as string, | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard against missing Google OAuth env-vars instead of type-casting them away
Using as string coerces potential undefined values to string, hiding mis-configuration until the first runtime call to the provider. Prefer an explicit assertion or validation step so the app fails fast and predictably:
- clientId: process.env.GOOGLE_CLIENT_ID as string,
- clientSecret: process.env.GOOGLE_CLIENT_SECRET as string,
+ clientId: process.env.GOOGLE_CLIENT_ID!,
+ clientSecret: process.env.GOOGLE_CLIENT_SECRET!,or, better, centralise env-var validation with Zod / @bogey/envsafe and import from there.
🏁 Script executed:
#!/bin/bash
set -e
echo "=== Searching for Zod-based env schemas ==="
rg -n "z\\.object" || true
echo "=== Searching for @bogey/envsafe usage ==="
rg -n "envsafe" || true
echo "=== Locating GOOGLE_CLIENT_ID references ==="
rg -n "GOOGLE_CLIENT_ID" || true
echo "=== Inspecting raw process.env access patterns ==="
rg -n "process.env" || trueLength of output: 1612
🏁 Script executed:
sed -n '1,200p' src/env.js || trueLength of output: 1415
Add missing Google OAuth env vars to central env schema and use env import
To ensure fail-fast validation and eliminate unsafe casts, register your Google keys in the shared schema and consume them via the validated env:
• In src/env.js (or src/env.ts), under server and runtimeEnv:
export const env = createEnv({
server: {
DATABASE_URL: z.string().url(),
NODE_ENV: z.enum(["development","test","production"]).default("development"),
+ GOOGLE_CLIENT_ID: z.string(),
+ GOOGLE_CLIENT_SECRET: z.string(),
},
…
runtimeEnv: {
DATABASE_URL: process.env.DATABASE_URL,
NODE_ENV: process.env.NODE_ENV,
+ GOOGLE_CLIENT_ID: process.env.GOOGLE_CLIENT_ID!,
+ GOOGLE_CLIENT_SECRET: process.env.GOOGLE_CLIENT_SECRET!,
},
…
});• In src/lib/auth.ts, import the validated env and replace the raw casts:
-import { … } from "next-auth";
+import { env } from "~/env";
…
google: {
- clientId: process.env.GOOGLE_CLIENT_ID as string,
- clientSecret: process.env.GOOGLE_CLIENT_SECRET as string,
+ clientId: env.GOOGLE_CLIENT_ID,
+ clientSecret: env.GOOGLE_CLIENT_SECRET,
},This removes the unsafe as string, centralizes validation with Zod, and guarantees that missing vars break the build rather than fail at runtime.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| google: { | |
| clientId: process.env.GOOGLE_CLIENT_ID as string, | |
| clientSecret: process.env.GOOGLE_CLIENT_SECRET as string, | |
| }, | |
| google: { | |
| - clientId: process.env.GOOGLE_CLIENT_ID as string, | |
| - clientSecret: process.env.GOOGLE_CLIENT_SECRET as string, | |
| + clientId: env.GOOGLE_CLIENT_ID, | |
| + clientSecret: env.GOOGLE_CLIENT_SECRET, | |
| }, |
🤖 Prompt for AI Agents
In src/lib/auth.ts around lines 17 to 20, avoid using 'as string' to cast Google
OAuth environment variables directly from process.env, as this can hide missing
configuration until runtime. Instead, add these Google OAuth variables to the
central environment schema (e.g., in src/env.js or src/env.ts) using a
validation library like Zod or @bogey/envsafe, then import the validated env
object into auth.ts and use the validated variables from there. This ensures
fail-fast validation and removes unsafe casts, causing the app to fail at build
time if the variables are missing.
src/app/articles/page.tsx
Outdated
| const signIn = async () => { | ||
| const data = await authClient.signIn.social({ | ||
| provider: "google", | ||
| callbackURL: "/articles", | ||
| }); | ||
| }, [articles, searchQuery]); | ||
|
|
||
| const handleNewArticle = () => { | ||
| // Refresh articles list - in real app, this would refetch from API | ||
| console.log("Refreshing articles list..."); | ||
| console.log(data); | ||
| // If ID token is provided no redirection will happen, and the user will be signed in directly. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling around the async signIn flow
signIn.social() may reject (network error, popup blocked, provider error). A swallowed rejection bubbles to the console in development and is silent in prod UI:
- const signIn = async () => {
- const data = await authClient.signIn.social({
- provider: "google",
- callbackURL: "/articles",
- });
- console.log(data);
+ const signIn = async () => {
+ try {
+ const data = await authClient.signIn.social({
+ provider: "google",
+ callbackURL: "/articles",
+ });
+ console.log(data);
+ } catch (err) {
+ console.error("Google sign-in failed", err);
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const signIn = async () => { | |
| const data = await authClient.signIn.social({ | |
| provider: "google", | |
| callbackURL: "/articles", | |
| }); | |
| }, [articles, searchQuery]); | |
| const handleNewArticle = () => { | |
| // Refresh articles list - in real app, this would refetch from API | |
| console.log("Refreshing articles list..."); | |
| console.log(data); | |
| // If ID token is provided no redirection will happen, and the user will be signed in directly. | |
| const signIn = async () => { | |
| try { | |
| const data = await authClient.signIn.social({ | |
| provider: "google", | |
| callbackURL: "/articles", | |
| }); | |
| console.log(data); | |
| } catch (err) { | |
| console.error("Google sign-in failed", err); | |
| } | |
| // If ID token is provided no redirection will happen, and the user will be signed in directly. | |
| }; |
🤖 Prompt for AI Agents
In src/app/articles/page.tsx around lines 8 to 14, the async signIn function
calls signIn.social() without error handling, which can cause unhandled promise
rejections on failures. Wrap the call to signIn.social() in a try-catch block to
catch any errors, log or handle them appropriately, and prevent silent failures
in production.
| const handleDeleteArticle = async (id: string) => { | ||
| try { | ||
| deleteArticle.mutateAsync({ id: id }); | ||
| } catch (error) { | ||
| console.log("Failed to delete article", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
mutateAsync is not awaited, so the catch never triggers
try/catch around an un-awaited promise only captures sync errors. Await the mutation (or attach .catch) to handle API failures correctly:
- deleteArticle.mutateAsync({ id: id });
+ await deleteArticle.mutateAsync({ id });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleDeleteArticle = async (id: string) => { | |
| try { | |
| deleteArticle.mutateAsync({ id: id }); | |
| } catch (error) { | |
| console.log("Failed to delete article", error); | |
| } | |
| }; | |
| const handleDeleteArticle = async (id: string) => { | |
| try { | |
| - deleteArticle.mutateAsync({ id: id }); | |
| + await deleteArticle.mutateAsync({ id }); | |
| } catch (error) { | |
| console.log("Failed to delete article", error); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/app/articles/old-page.tsx around lines 65 to 71, the async function
handleDeleteArticle calls deleteArticle.mutateAsync without awaiting it, so the
try/catch block does not catch errors from the promise. Fix this by adding await
before deleteArticle.mutateAsync to properly handle and catch any asynchronous
errors during the mutation.
drizzle/meta/0005_snapshot.json
Outdated
| "user_id": { | ||
| "name": "user_id", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": true | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing foreign key constraint for flipside_article.user_id.
The flipside_article table has a user_id field but no foreign key constraint to the user table. This could lead to orphaned articles when users are deleted.
Consider adding a foreign key constraint:
ALTER TABLE "flipside_article" ADD CONSTRAINT "flipside_article_user_id_user_id_fk"
FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action;🤖 Prompt for AI Agents
In drizzle/meta/0005_snapshot.json around lines 18 to 23, the user_id field in
the flipside_article table lacks a foreign key constraint referencing the user
table, risking orphaned records. Add a foreign key constraint on user_id that
references the id column of the user table with ON DELETE cascade and ON UPDATE
no action to maintain referential integrity.
| createdAt: timestamp('created_at').$defaultFn(() => new Date), | ||
| updatedAt: timestamp('updated_at').$defaultFn(() => new Date) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent timestamp nullability in verification table.
The verification table has nullable timestamps while other tables use non-null timestamps. This inconsistency could lead to confusion and potential issues.
- createdAt: timestamp('created_at').$defaultFn(() => new Date),
- updatedAt: timestamp('updated_at').$defaultFn(() => new Date)
+ createdAt: timestamp('created_at').defaultNow().notNull(),
+ updatedAt: timestamp('updated_at').defaultNow().notNull()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| createdAt: timestamp('created_at').$defaultFn(() => new Date), | |
| updatedAt: timestamp('updated_at').$defaultFn(() => new Date) | |
| createdAt: timestamp('created_at').defaultNow().notNull(), | |
| updatedAt: timestamp('updated_at').defaultNow().notNull() |
🤖 Prompt for AI Agents
In src/server/db/auth-schema.ts around lines 45 to 46, the timestamps for
createdAt and updatedAt are currently nullable in the verification table, which
is inconsistent with other tables that use non-nullable timestamps. Update the
timestamp definitions to be non-nullable by removing any nullable settings,
ensuring createdAt and updatedAt always have valid timestamp values to maintain
consistency across tables.
Akashamba
changed the title
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores