Feat/dashboard wip #40
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRuntime role fetching was added and consumed across UI and server: the sidebar shows a dynamic Tutor/Student badge, the settings page conditionally renders a Signals block with loading/error states for tutors, onboarding store gained a reset() called on mount, and the signal router's getViewerRole now queries the DB using ctx.user.id instead of always returning STUDENT. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Browser UI
participant TRPC as TRPC API (signalRouter)
participant DB as Database
rect rgb(230,240,255)
UI->>TRPC: api.signal.getViewerRole.query()
TRPC->>DB: findUnique({ where: { clerkUid: ctx.user.id } })
DB-->>TRPC: user (role or null)
alt user.role exists
TRPC-->>UI: { role: "TUTOR" }
else no user
TRPC-->>UI: { role: "STUDENT" }
end
UI->>UI: render badge (🎓 Tutor / 👤 Student) and conditional Signals UI
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/app/dashboard/_components/sidebar.tsx (1)
26-27: Consider lifting the role query to a common parent component.Both the sidebar (this file) and the settings page (
src/app/dashboard/settings/page.tsx) independently fetch the viewer role usingapi.signal.getViewerRole.useQuery(). While tRPC/react-query will cache the result, this creates redundant query definitions and could be simplified.Consider creating a shared hook or lifting the query to a common parent in the dashboard layout to avoid duplication and ensure consistent loading states across components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/dashboard/_components/sidebar.tsx(2 hunks)src/app/dashboard/settings/page.tsx(1 hunks)src/app/onboarding/page.tsx(1 hunks)src/server/api/routers/signal.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/server/api/routers/signal.ts (1)
src/server/api/trpc.ts (1)
protectedProcedure(104-104)
src/app/dashboard/_components/sidebar.tsx (1)
src/trpc/react.tsx (1)
api(25-25)
src/app/dashboard/settings/page.tsx (1)
src/trpc/react.tsx (1)
api(25-25)
src/app/onboarding/page.tsx (1)
src/stores/onboarding.ts (1)
useOnboardingStore(96-220)
🔇 Additional comments (1)
src/app/onboarding/page.tsx (1)
12-15: Verify the reset-on-mount behavior aligns with user experience goals.The
useEffecthook clears all onboarding data every time the component mounts. If a user navigates away (e.g., using the browser back button) and returns to this page, their role selection and all progress will be lost.Consider whether this is the intended behavior, or if you want to preserve selections during a session. If reset is only needed for new sessions, you might want to add a condition or move the reset logic elsewhere.
| const { data: role } = api.signal.getViewerRole.useQuery(); | ||
| const isTutor = role === "TUTOR"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add loading and error states for the role query.
Similar to the settings page, the role query lacks loading and error handling. While loading, role will be undefined, causing isTutor to be false and displaying "👤 Student" as the default. This could confuse tutors who see the wrong badge briefly before it updates.
Add loading and error states:
- const { data: role } = api.signal.getViewerRole.useQuery();
+ const { data: role, isLoading } = api.signal.getViewerRole.useQuery();
const isTutor = role === "TUTOR";Then conditionally render the badge:
<div className="px-3 py-4">
<div className="text-lg font-semibold text-neutral-900">TutorLink</div>
- <div className="mt-2 inline-block px-2.5 py-1 rounded-full text-xs font-semibold bg-indigo-100 text-indigo-900">
- {isTutor ? "🎓 Tutor" : "👤 Student"}
- </div>
+ {!isLoading && (
+ <div className="mt-2 inline-block px-2.5 py-1 rounded-full text-xs font-semibold bg-indigo-100 text-indigo-900">
+ {isTutor ? "🎓 Tutor" : "👤 Student"}
+ </div>
+ )}
</div>Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/api/routers/signal.ts (1)
109-141: Critical: Inconsistent handling ofstudentIdacross endpoints.The
acceptSignalendpoint assumessignal.studentIdis an internal database ID (line 137:{ id: signal.studentId }), butcreateSignal(line 29) setsstudentId: ctx.user.iddirectly without querying the database first.Given that lines 113-114 query by
clerkUid: ctx.user.id, this confirmsctx.user.idis the Clerk UID (external identifier), not the internal database ID. This creates a mismatch:
- Line 29 stores the Clerk UID in
signal.studentId- Line 137 tries to connect a user by
{ id: signal.studentId }, expecting an internal IDThis will cause the Prisma
connectoperation to fail because it cannot find a user withid = <clerkUid>.Solution: Update
createSignalto first resolve the internal database ID:createSignal: protectedProcedure .input(z.object({ message: z.string().min(1), subject: z.string().min(1), urgency: z.number().min(1).max(5), status: z.string() })) .mutation(async ({ input, ctx }) => { - const studentId = ctx.user.id; + // Resolve internal DB ID from Clerk UID + const student = await ctx.db.user.findUnique({ + where: { clerkUid: ctx.user.id }, + select: { id: true }, + }); + if (!student) { + throw new TRPCError({ code: "NOT_FOUND", message: "User not found" }); + } + const studentId = student.id; const signalData = { ...input, studentId, createdAt: new Date().toISOString() };Note: This same issue affects
getMySignals(line 87) anddeleteSignal(line 99), which also query/checkstudentIdusingctx.user.iddirectly. Apply the same fix pattern to those endpoints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/dashboard/settings/page.tsx(1 hunks)src/server/api/routers/signal.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/dashboard/settings/page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/server/api/routers/signal.ts (1)
src/server/api/trpc.ts (1)
protectedProcedure(104-104)
| getViewerRole: protectedProcedure.query(async ({ ctx }) => { | ||
| // Query the DB user to get the canonical role | ||
| const user = await ctx.db.user.findUnique({ | ||
| where: { clerkUid: ctx.user.id }, | ||
| select: { role: true }, | ||
| }); | ||
|
|
||
| // Return the user's role from DB or default to STUDENT if not found | ||
| return user?.role ?? "STUDENT"; |
There was a problem hiding this comment.
🧩 Analysis chain
Good fix for the role consistency issue.
This change standardizes role determination by querying the canonical user.role field, resolving the previous inconsistency with acceptSignal. The fallback to "STUDENT" provides safe default behavior.
However, verify whether ctx.user already contains the role information to avoid an unnecessary DB query on every call:
If the role is already available in ctx.user, consider using it directly instead of querying the database.
🏁 Script executed:
#!/bin/bash
# Check if ctx.user is populated with role during authentication
rg -nP --type=ts -C5 'enforceUserIsAuthed|ctx\.user\s*='Length of output: 907
🏁 Script executed:
#!/bin/bash
# Find where ctx is created/typed in TRPC setup
rg -nP --type=ts -B5 -A10 'export.*context|type.*Context|createContext'Length of output: 4650
🏁 Script executed:
#!/bin/bash
# Find Signal model schema and studentId field type
fd -e prisma | head -5Length of output: 20
🏁 Script executed:
#!/bin/bash
# Check createSignal implementation (line 29) and acceptSignal (lines 112-137)
rg -nP --type=ts -A30 'createSignal.*protectedProcedure\.input'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if studentId is used elsewhere and how it's referenced
rg -nP --type=ts 'studentId' -B2 -A2Length of output: 5492
---
🏁 Script executed:
```shell
#!/bin/bash
# Check Signal model schema in prisma
rg -A20 'model Signal' prisma/schema.prisma
Length of output: 536
🏁 Script executed:
#!/bin/bash
# Check User model to confirm id vs clerkUid fields
rg -A10 'model User' prisma/schema.prismaLength of output: 404
getViewerRole fix is correct, but critical bug found in createSignal that must be fixed.
The DB query in getViewerRole is necessary and correct—ctx.user (from Clerk) lacks the role field, which exists only in the database. However, verification reveals a critical issue:
createSignal (line 29) stores ctx.user.id as studentId. Since ctx.user.id is the Clerk UID (not the internal DB User.id), this causes a data integrity failure: acceptSignal (line 137) attempts to connect the conversation participant using { id: signal.studentId }, which fails because it passes a Clerk UID to a field expecting an internal database ID.
Fix required: createSignal must query the database to get the internal User.id before storing it in studentId, similar to how acceptSignal queries for me.id at line 124.
🤖 Prompt for AI Agents
In src/server/api/routers/signal.ts around lines 10-18 (and specifically
createSignal at ~line 29), createSignal currently writes ctx.user.id (Clerk UID)
into studentId which is the internal DB User.id; change createSignal to query
ctx.db.user.findUnique({ where: { clerkUid: ctx.user.id }, select: { id: true }
}) and use the returned internal id as studentId before creating the signal,
handling the case where the DB user is not found (throw or return an error)
similar to the me lookup used in acceptSignal around line 124 so subsequent
acceptSignal participant connects work correctly.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements