-
-
Notifications
You must be signed in to change notification settings - Fork 7
fix: authentication, chat history toggle, and UI improvements #443
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: authentication, chat history toggle, and UI improvements #443
Conversation
- Fix unauthorized user error by ensuring user exists in DB on credit fetch and chat save - Implement chat history toggle from plant icon next to QCX - Move credits preview to the top of the chat - Ensure tent tree icon opens usage and billing UI - Toggle chat history from the right side
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 |
|
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
fe8dc87
into
fix/regressions-security-architecture-13947295106479740171
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.
The DB-user auto-provisioning is a good direction, but both the credits endpoint and saveChat use a non-idempotent select → insert flow that can fail under concurrent requests; make inserts conflict-safe to avoid reintroducing auth-related errors. saveChat also adds dynamic imports in a hot path, which is unnecessary overhead for server actions. Finally, History’s trigger API is flexible but can easily be misused with nested/compound children, leading to brittle UI behavior.
Additional notes (2)
- Maintainability |
components/history.tsx:38-41
SheetTriggeris typically expected to wrap a single interactive element. Passing an arbitrarychildrennode that could include multiple elements (or another button) can lead to nested-interactive / click-target issues.
Now that the header uses <History> to wrap a Button, consider enforcing asChild + single-element children (or validating at runtime) to avoid accidental misuse elsewhere.
- Compatibility |
components/header.tsx:36-36
Wrapping the logo button with an<a href="/">can cause nested interactive elements depending on howButtonrenders (many button components render a<button>). If it’s a<button>inside an<a>, that’s invalid HTML and can lead to accessibility/interaction quirks.
In Next.js, prefer next/link and have the button render “as child” (or render the interactive element as the link).
Summary of changes
What changed
API: Credits endpoint (app/api/user/credits/route.ts)
- Changed DB lookup to
let dbUser = ...and auto-creates a DB user row when missing viadb.insert(users).values({ id, credits: 0, tier: 'free' }).returning(). - Removes the prior
404 User not foundbehavior and proceeds to returncredits/tier.
Chat UI (components/chat.tsx)
- Moved
<CreditsDisplay />to render only in the non-calendar view, immediately above<ChatPanel />.
Header / navigation (components/header.tsx)
- Adds
Sprouticon and introduces a dedicated chat-history toggle button next to the QCX logo. - Wraps the logo + title in an
<a href="/">link. - Adds
title="Usage & Billing"to theTentTreebuttons (desktop + mobile).
History panel (components/history.tsx)
- Moves history
SheetContentfrom left to right side and updates corner rounding classes accordingly.
Chat persistence (lib/actions/chat.ts)
- Before saving a chat, ensures the user exists in the DB by querying
usersand inserting a default record if absent. - Implements this using dynamic
await import(...)calls fordb,users, andeq.
Next.js typings (next-env.d.ts)
- Updates the referenced generated routes types path from
./.next/dev/types/routes.d.tsto./.next/types/routes.d.ts.
| // Get user from database | ||
| const dbUser = await db.query.users.findFirst({ | ||
| let dbUser = await db.query.users.findFirst({ | ||
| where: eq(users.id, user.id) | ||
| }); | ||
|
|
||
| if (!dbUser) { | ||
| return NextResponse.json( | ||
| { error: 'User not found' }, | ||
| { status: 404 } | ||
| ); | ||
| // Create user if they don't exist in the database | ||
| const [newUser] = await db.insert(users).values({ | ||
| id: user.id, | ||
| credits: 0, | ||
| tier: 'free' | ||
| }).returning(); | ||
| dbUser = newUser; | ||
| } |
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.
The “create user if missing” flow can still throw a 500 (or fail the request) if two requests race and both attempt to insert the same users.id concurrently (e.g., multiple tabs hitting credits fetch on first login). This can reintroduce the auth/credits error you’re trying to fix.
Consider making the insert idempotent (upsert / on-conflict-do-nothing) and then re-select, or handle unique-constraint violations explicitly.
Suggestion
Make user creation race-safe by using an idempotent insert (if supported by your Drizzle dialect) and then re-fetch:
- Prefer
onConflictDoNothing/onDuplicateKeyUpdatestyle APIs (dialect-dependent) - Or catch unique-violation and continue
Example (Postgres-style Drizzle):
await db
.insert(users)
.values({ id: user.id, credits: 0, tier: 'free' })
.onConflictDoNothing({ target: users.id })
const dbUser = await db.query.users.findFirst({ where: eq(users.id, user.id) })Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this change.
| // Ensure user exists in the database before saving chat | ||
| const { db } = await import('@/lib/db'); | ||
| const { users } = await import('@/lib/db/schema'); | ||
| const { eq } = await import('drizzle-orm'); | ||
|
|
||
| const dbUser = await db.query.users.findFirst({ | ||
| where: eq(users.id, effectiveUserId) | ||
| }); | ||
|
|
||
| if (!dbUser) { | ||
| await db.insert(users).values({ | ||
| id: effectiveUserId, | ||
| credits: 0, | ||
| tier: 'free' | ||
| }); | ||
| } | ||
|
|
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.
Doing a sequence of findFirst then insert has the same race condition risk as the credits endpoint (double insert on concurrent requests). Also, using dynamic await import(...) inside a hot path will add overhead and makes bundling/traceability harder; this is not a great tradeoff for a core action.
Since this is server-side code, prefer static imports at module scope and make the insert idempotent.
Suggestion
Refactor to static imports and an idempotent insert:
- Move imports to top-level:
import { db } from '@/lib/db'
import { users } from '@/lib/db/schema'
import { eq } from 'drizzle-orm'- Use conflict-safe insert + optional re-fetch (dialect-dependent):
await db
.insert(users)
.values({ id: effectiveUserId, credits: 0, tier: 'free' })
.onConflictDoNothing({ target: users.id })(Or catch unique-constraint errors.)
Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this refactor.
User description
PR Type
Bug fix, Enhancement
Description
Ensure user exists in database before credit fetch and chat save
Implement chat history toggle with plant icon in header
Move credits display to top of chat panel conditionally
Add usage and billing button with tent tree icon
Reposition chat history panel from left to right side
Diagram Walkthrough
File Walkthrough
route.ts
Auto-create missing users on credit fetchapp/api/user/credits/route.ts
chat.ts
Ensure user exists before chat savelib/actions/chat.ts
next-env.d.ts
Update Next.js types import pathnext-env.d.ts
chat.tsx
Conditionally position credits displaycomponents/chat.tsx
header.tsx
Add chat history toggle and improve header UIcomponents/header.tsx
history.tsx
Reposition history panel to right sidecomponents/history.tsx