Feat/dashboard wip #28
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR extends the signal management system with a complete UI and backend implementation. It adds a new SignalForm component, signal lifecycle endpoints (create, list, manage, delete, accept), tab-based signal management views, toast notifications via sonner, and various UI refinements including updated navigation labels and a settings page with sign-out functionality. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant SignalForm as SignalForm<br/>(Client)
participant TRPC as TRPC<br/>(signal)
participant Backend as Backend<br/>(Router)
participant DB as Database
participant Tutors as Tutors<br/>(WebSocket)
User->>SignalForm: Fill form (subject,<br/>message, urgency)
SignalForm->>SignalForm: Validate (subject & message<br/>length > 0)
User->>SignalForm: Submit
SignalForm->>TRPC: createSignal mutation
TRPC->>Backend: Call createSignal
Backend->>DB: Create signal record
DB-->>Backend: Success
Backend->>Tutors: broadcastToTutors<br/>(signal payload)
Backend-->>TRPC: Return response
TRPC-->>SignalForm: Success
SignalForm->>SignalForm: Show success banner<br/>(4s timeout)
SignalForm->>SignalForm: Reset form fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
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: 4
🧹 Nitpick comments (3)
src/app/dashboard/signal/_components/signal-form.tsx (1)
40-54: Add TODO comment for API implementation.The simulated API call with
setTimeoutis acceptable for a WIP feature but should be tracked for future implementation.Consider adding:
const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); if (!selectedSubject || !message.trim()) return; setIsSubmitting(true); - // Simulate API call + // TODO: Replace with actual API call to create signal setTimeout(() => {src/app/dashboard/signal/page.tsx (2)
18-46: Add TODO comment for real data fetching.Mock data is appropriate for WIP, but track the need to replace it with actual API calls.
Consider adding a comment:
// Mock active signals for demo +// TODO: Replace with real-time data fetching from API type Signal = {
48-54: Extract urgency configuration to shared constants.The urgency configuration here duplicates similar data in
src/app/dashboard/signal/_components/signal-form.tsx(lines 25-31). This violates DRY principles and can lead to inconsistencies.Create a shared constants file:
// src/app/dashboard/signal/_lib/constants.ts export const URGENCY_LEVELS = [ { level: 1, label: "Low", bg: "bg-blue-100", text: "text-blue-900", badge: "border-blue-300" }, { level: 2, label: "Moderate", bg: "bg-yellow-100", text: "text-yellow-900", badge: "border-yellow-300" }, { level: 3, label: "High", bg: "bg-orange-100", text: "text-orange-900", badge: "border-orange-300" }, { level: 4, label: "Very High", bg: "bg-red-100", text: "text-red-900", badge: "border-red-300" }, { level: 5, label: "Critical", bg: "bg-red-200", text: "text-red-950", badge: "border-red-400" }, ] as const; export const urgencyConfig = Object.fromEntries( URGENCY_LEVELS.map(level => [level.level, level]) ) as Record<1 | 2 | 3 | 4 | 5, typeof URGENCY_LEVELS[number]>;Then import in both files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/dashboard/_components/sidebar.tsx(1 hunks)src/app/dashboard/settings/page.tsx(1 hunks)src/app/dashboard/signal/_components/signal-form.tsx(1 hunks)src/app/dashboard/signal/page.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/dashboard/signal/page.tsx (1)
src/app/dashboard/signal/_components/signal-form.tsx (1)
SignalForm(33-172)
🔇 Additional comments (7)
src/app/dashboard/_components/sidebar.tsx (1)
18-18: LGTM! Label clarifies the feature's purpose.The rename from "Calls" to "Live Sessions" improves user understanding while maintaining the existing route.
src/app/dashboard/signal/_components/signal-form.tsx (3)
6-31: LGTM! Constants are well-structured.The subjects list and urgency level configuration provide a good foundation for the form UI.
33-38: LGTM! State management is properly initialized.The form state covers all necessary fields with sensible defaults.
147-168: LGTM! Submit button properly handles validation and loading states.The button correctly disables when validation fails or submission is in progress, with clear visual feedback.
src/app/dashboard/signal/page.tsx (3)
8-16: LGTM! Type definition is well-structured.The Signal type provides good type safety with the union type for urgency levels.
93-135: LGTM! Tab navigation is well-implemented.The state management and UI for tab switching between Create Signal and Active Signals is clean and user-friendly.
175-197: LGTM! Tips section provides helpful user guidance.The Pro Tips and Current Stats sections add value by setting user expectations and providing best practices.
| import "server-only"; | ||
| import { SignOutButton } from "@clerk/nextjs"; |
There was a problem hiding this comment.
Remove "server-only" import—incompatible with client components.
SignOutButton from Clerk is a client component requiring interactivity, but "server-only" enforces this file as a server component. This combination will cause a runtime error.
Apply this diff to fix:
-import "server-only";
import { SignOutButton } from "@clerk/nextjs";📝 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.
| import "server-only"; | |
| import { SignOutButton } from "@clerk/nextjs"; | |
| import { SignOutButton } from "@clerk/nextjs"; |
🤖 Prompt for AI Agents
In src/app/dashboard/settings/page.tsx lines 1-2, the file currently imports
"server-only" which forces a server component but also imports Clerk's
SignOutButton (a client component); remove the "server-only" import and instead
make the file a client component by adding the "use client" directive at the top
OR move the SignOutButton into a separate client-only child component and import
that; ensure no server-only enforcement remains when rendering SignOutButton so
the component runs on the client.
src/app/dashboard/signal/page.tsx
Outdated
| function SignalCard({ signal }: { signal: (typeof mockActiveSignals)[0] }) { | ||
| const config = urgencyConfig[signal.urgency]; | ||
|
|
||
| return ( | ||
| <div className="p-4 border border-neutral-200 rounded-lg hover:shadow-md transition-shadow duration-200 bg-white"> | ||
| <div className="flex items-start justify-between mb-3"> | ||
| <div className="flex items-center gap-2 flex-1"> | ||
| <div className="w-10 h-10 rounded-full bg-linear-to-br from-indigo-400 to-indigo-600 flex items-center justify-center text-white font-semibold"> | ||
| {signal.studentName.charAt(0)} | ||
| </div> | ||
| <div className="flex-1 min-w-0"> | ||
| <p className="font-medium text-neutral-900 text-sm">{signal.studentName}</p> | ||
| <p className="text-xs text-neutral-500 flex items-center gap-1"> | ||
| <Clock size={12} /> | ||
| {signal.timeAgo} ago | ||
| </p> | ||
| </div> | ||
| </div> | ||
| <div className={`px-2 py-1 rounded text-xs font-semibold ${config.badge} border-2 ${config.bg} ${config.text}`}> | ||
| {config.label} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="mb-3"> | ||
| <span className="inline-block px-2.5 py-1 bg-indigo-100 text-indigo-900 text-xs font-semibold rounded-full mb-2"> | ||
| {signal.subject} | ||
| </span> | ||
| <p className="text-sm text-neutral-700 line-clamp-2">{signal.message}</p> | ||
| </div> | ||
|
|
||
| <button className="w-full mt-3 px-3 py-2 bg-indigo-600 text-white text-sm font-semibold rounded-lg hover:bg-indigo-700 transition-colors duration-200 active:scale-95"> | ||
| Accept Signal | ||
| </button> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Fix invalid Tailwind CSS class.
Line 63 uses bg-linear-to-br which is not a valid Tailwind class. This will not apply any gradient styling.
Apply this diff:
<div className="w-10 h-10 rounded-full bg-linear-to-br from-indigo-400 to-indigo-600 flex items-center justify-center text-white font-semibold">
+ <div className="w-10 h-10 rounded-full bg-gradient-to-br from-indigo-400 to-indigo-600 flex items-center justify-center text-white font-semibold">📝 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.
| function SignalCard({ signal }: { signal: (typeof mockActiveSignals)[0] }) { | |
| const config = urgencyConfig[signal.urgency]; | |
| return ( | |
| <div className="p-4 border border-neutral-200 rounded-lg hover:shadow-md transition-shadow duration-200 bg-white"> | |
| <div className="flex items-start justify-between mb-3"> | |
| <div className="flex items-center gap-2 flex-1"> | |
| <div className="w-10 h-10 rounded-full bg-linear-to-br from-indigo-400 to-indigo-600 flex items-center justify-center text-white font-semibold"> | |
| {signal.studentName.charAt(0)} | |
| </div> | |
| <div className="flex-1 min-w-0"> | |
| <p className="font-medium text-neutral-900 text-sm">{signal.studentName}</p> | |
| <p className="text-xs text-neutral-500 flex items-center gap-1"> | |
| <Clock size={12} /> | |
| {signal.timeAgo} ago | |
| </p> | |
| </div> | |
| </div> | |
| <div className={`px-2 py-1 rounded text-xs font-semibold ${config.badge} border-2 ${config.bg} ${config.text}`}> | |
| {config.label} | |
| </div> | |
| </div> | |
| <div className="mb-3"> | |
| <span className="inline-block px-2.5 py-1 bg-indigo-100 text-indigo-900 text-xs font-semibold rounded-full mb-2"> | |
| {signal.subject} | |
| </span> | |
| <p className="text-sm text-neutral-700 line-clamp-2">{signal.message}</p> | |
| </div> | |
| <button className="w-full mt-3 px-3 py-2 bg-indigo-600 text-white text-sm font-semibold rounded-lg hover:bg-indigo-700 transition-colors duration-200 active:scale-95"> | |
| Accept Signal | |
| </button> | |
| </div> | |
| ); | |
| } | |
| function SignalCard({ signal }: { signal: (typeof mockActiveSignals)[0] }) { | |
| const config = urgencyConfig[signal.urgency]; | |
| return ( | |
| <div className="p-4 border border-neutral-200 rounded-lg hover:shadow-md transition-shadow duration-200 bg-white"> | |
| <div className="flex items-start justify-between mb-3"> | |
| <div className="flex items-center gap-2 flex-1"> | |
| <div className="w-10 h-10 rounded-full bg-gradient-to-br from-indigo-400 to-indigo-600 flex items-center justify-center text-white font-semibold"> | |
| {signal.studentName.charAt(0)} | |
| </div> | |
| <div className="flex-1 min-w-0"> | |
| <p className="font-medium text-neutral-900 text-sm">{signal.studentName}</p> | |
| <p className="text-xs text-neutral-500 flex items-center gap-1"> | |
| <Clock size={12} /> | |
| {signal.timeAgo} ago | |
| </p> | |
| </div> | |
| </div> | |
| <div className={`px-2 py-1 rounded text-xs font-semibold ${config.badge} border-2 ${config.bg} ${config.text}`}> | |
| {config.label} | |
| </div> | |
| </div> | |
| <div className="mb-3"> | |
| <span className="inline-block px-2.5 py-1 bg-indigo-100 text-indigo-900 text-xs font-semibold rounded-full mb-2"> | |
| {signal.subject} | |
| </span> | |
| <p className="text-sm text-neutral-700 line-clamp-2">{signal.message}</p> | |
| </div> | |
| <button className="w-full mt-3 px-3 py-2 bg-indigo-600 text-white text-sm font-semibold rounded-lg hover:bg-indigo-700 transition-colors duration-200 active:scale-95"> | |
| Accept Signal | |
| </button> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/app/dashboard/signal/page.tsx around lines 56 to 91, the Tailwind class
bg-linear-to-br on line ~63 is invalid; replace it with bg-gradient-to-br and
ensure the gradient color utility classes from-indigo-400 and to-indigo-600
remain on the same element (or add them if missing) so the avatar uses
Tailwind's gradient syntax (e.g., className should include "bg-gradient-to-br
from-indigo-400 to-indigo-600 ..."); update that single class accordingly and
keep the other utility classes intact.
| <div className="mb-12"> | ||
| {tabActive === "create" ? ( | ||
| <div className="space-y-8"> | ||
| <div className="p-6 bg-linear-to-r from-indigo-50 to-blue-50 border border-indigo-100 rounded-lg"> | ||
| <h2 className="text-xl font-bold text-neutral-900 mb-2"> | ||
| 🚀 Need help fast? | ||
| </h2> | ||
| <p className="text-neutral-700"> | ||
| Send a signal and connect with available tutors in seconds. The more urgent your request, the faster you'll get matched. | ||
| </p> | ||
| </div> | ||
| <SignalForm /> | ||
| </div> | ||
| ) : ( | ||
| <div className="space-y-4"> | ||
| {mockActiveSignals.length > 0 ? ( | ||
| <> | ||
| <p className="text-sm text-neutral-600 mb-4"> | ||
| Showing {mockActiveSignals.length} active signals from students | ||
| </p> | ||
| <div className="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-4"> | ||
| {mockActiveSignals.map((signal) => ( | ||
| <SignalCard key={signal.id} signal={signal} /> | ||
| ))} | ||
| </div> | ||
| </> | ||
| ) : ( | ||
| <div className="text-center py-12"> | ||
| <Activity size={48} className="mx-auto text-neutral-300 mb-4" /> | ||
| <p className="text-neutral-600 text-lg">No active signals right now</p> | ||
| <p className="text-neutral-500 text-sm">Check back soon!</p> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Fix invalid Tailwind CSS class.
Line 141 uses bg-linear-to-r which is not a valid Tailwind class. This will not apply any gradient styling.
Apply this diff:
- <div className="p-6 bg-linear-to-r from-indigo-50 to-blue-50 border border-indigo-100 rounded-lg">
+ <div className="p-6 bg-gradient-to-r from-indigo-50 to-blue-50 border border-indigo-100 rounded-lg">🤖 Prompt for AI Agents
In src/app/dashboard/signal/page.tsx around lines 138 to 173, the div uses the
invalid Tailwind class "bg-linear-to-r"; replace it with the correct Tailwind
gradient utility "bg-gradient-to-r" so the gradient from "from-indigo-50" to
"to-blue-50" is applied (keep the existing from- and to- classes and the rest of
the class list unchanged).
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Tailwind configuration files
fd -e js -e ts -e cjs -e mjs tailwind.config
# Search for usages of bg-linear-to-r in the codebase
rg -n "bg-linear-to-r" --type=tsx --type=ts --type=jsx --type=js -C2
# Also search in CSS files for custom definitions
fd -e css | xargs grep -n "bg-linear-to-r" 2>/dev/null || true
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for Tailwind configuration files
fd tailwind.config
# Search for usages of bg-linear-to-r in TypeScript/JavaScript files
rg -n "bg-linear-to-r" -g "*.tsx" -g "*.ts" -g "*.jsx" -g "*.js" -C2
# Also search in the specific file mentioned
rg -n "bg-linear" src/app/dashboard/signal/page.tsx
# Check for any custom Tailwind plugin or CSS files
fd -e css -e scss | xargs cat 2>/dev/null | grep -i "linear" || echo "No CSS files found or no linear references"
Length of output: 878
🌐 Web query:
Tailwind CSS bg-linear-to-r class valid
💡 Result:
Yes — Tailwind provides bg-linear-to-r (produces background-image: linear-gradient(to right, var(--tw-gradient-stops));). The older/bg-compatible utility bg-gradient-to-r also exists. See Tailwind docs. [1][2]
Sources:
[1] Tailwind docs — Background image / gradient utilities (shows bg-linear-to-r).
[2] Tailwind blog v1.7 (introducing bg-gradient-to-r historically).
@AJ-505, you're absolutely right, and I apologize for the error. bg-linear-to-r is indeed a valid Tailwind CSS utility class that generates background-image: linear-gradient(to right, var(--tw-gradient-stops));. The class works correctly with the from-indigo-50 and to-blue-50 utilities.
My comment was incorrect. Thank you for catching that!
🧬 Code Graph Analysis Results
<src/app/dashboard/signal/_components/signal-form.tsx> (lines 33-172)
export function SignalForm() {
const [selectedSubject, setSelectedSubject] = useState("");
const [message, setMessage] = useState("");
const [urgency, setUrgency] = useState(3);
const [isSubmitting, setIsSubmitting] = useState(false);
const [showSuccess, setShowSuccess] = useState(false);
const handleSubmit = async (e: React.FormEvent) => {
e.preventDefault();
if (!selectedSubject || !message.trim()) return;
setIsSubmitting(true);
// Simulate API call
setTimeout(() => {
setIsSubmitting(false);
setShowSuccess(true);
setSelectedSubject("");
setMessage("");
setUrgency(3);
setTimeout(() => setShowSuccess(false), 4000);
}, 600);
};
return (
<div className="w-full max-w-2xl mx-auto">
{/* Success Message */}
{showSuccess && (
<div className="mb-6 p-4 bg-green-50 border border-green-200 rounded-lg flex items-center gap-3 animate-in slide-in-from-top-2 duration-300">
<div className="w-2 h-2 rounded-full bg-green-600"></div>
<span className="text-green-900 font-medium">
Signal sent! Tutors are being notified...
</span>
</div>
)}
<form onSubmit={handleSubmit} className="space-y-6">
{/* Subject Selection */}
<div>
<label className="block text-sm font-semibold text-neutral-900 mb-3">
What subject do you need help with?
</label>
<div className="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-4 gap-2">
{subjects.map((subject) => (
<button
key={subject}
type="button"
onClick={() => setSelectedSubject(subject)}
className={`px-3 py-2 rounded-lg font-medium text-sm transition-all duration-200 ${selectedSubject === subject
? "bg-indigo-600 text-white shadow-lg scale-105"
: "bg-neutral-100 text-neutral-700 hover:bg-neutral-200"
}`}
>
{subject}
</button>
))}
</div>
</div>
{/* Problem/Question */}
<div>
<label className="block text-sm font-semibold text-neutral-900 mb-2">
What's your problem or question?
</label>
<textarea
value={message}
onChange={(e) => setMessage(e.target.value)}
placeholder="e.g., I need help solving question 5 WAEC past paper on projectile motion"
className="w-full px-4 py-3 border border-neutral-300 rounded-lg focus:outline-none focus:ring-2 focus:ring-indigo-600 focus:border-transparent resize-none placeholder-neutral-400 text-neutral-900"
rows={4}
/>
<p className="mt-2 text-xs text-neutral-500">
{message.length}/500 characters
</p>
</div>
{/* Urgency Level */}
<div>
<div className="flex items-center gap-2 mb-3">
<Zap size={18} className="text-indigo-600" />
<label className="block text-sm font-semibold text-neutral-900">
How urgent is your request?
</label>
</div>
<div className="grid grid-cols-2 sm:grid-cols-5 gap-2">
{urgencyLevels.map(({ level, label, color }) => (
<button
key={level}
type="button"
onClick={() => setUrgency(level)}
className={`px-3 py-2 rounded-lg font-medium text-sm border-2 transition-all duration-200 ${urgency === level
? `${color} shadow-lg scale-105 border-current`
: `${color} border-transparent opacity-60 hover:opacity-100`
}`}
>
{label}
</button>
))}
</div>
</div>
{/* Alert Box */}
<div className="p-4 bg-blue-50 border-l-4 border-blue-500 rounded flex gap-3">
<AlertCircle size={20} className="text-blue-600 shrink-0 mt-0.5" />
<div>
<p className="text-sm font-medium text-blue-900">
💬 Your signal will be broadcast to nearby tutors
</p>
<p className="text-xs text-blue-700 mt-1">
Average response time: 2-5 minutes. Higher urgency gets priority.
</p>
</div>
</div>
{/* Submit Button */}
<button
type="submit"
disabled={!selectedSubject || !message.trim() || isSubmitting}
className={`w-full py-3 px-4 rounded-lg font-semibold flex items-center justify-center gap-2 transition-all duration-200 ${isSubmitting
? "bg-indigo-400 text-white cursor-not-allowed"
: selectedSubject && message.trim()
? "bg-indigo-600 text-white hover:bg-indigo-700 hover:shadow-lg active:scale-95"
: "bg-neutral-200 text-neutral-400 cursor-not-allowed"
}`}
>
{isSubmitting ? (
<>
<div className="w-4 h-4 border-2 border-white border-t-transparent rounded-full animate-spin"></div>
Sending Signal...
</>
) : (
<>
<Send size={18} />
Send Signal Now
</>
)}
</button>
</form>
</div>
);
}
✏️ Learnings added
Learnt from: AJ-505
PR: Tobshub/tutorlink#28
File: src/app/dashboard/signal/page.tsx:138-173
Timestamp: 2025-10-25T06:09:14.372Z
Learning: `bg-linear-to-r`, `bg-linear-to-br`, and other `bg-linear-to-*` variants are valid Tailwind CSS utility classes for applying linear gradients, alongside the `bg-gradient-to-*` utilities.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
…gout functionality
…dd signal router to root API; (WIP) Update SignalForm
…on signalForm textArea
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
src/app/dashboard/settings/page.tsx (1)
1-18: Add "use client" directive for client component.
SignOutButtonfrom Clerk requires client-side interactivity, but this file lacks the"use client"directive at the top. This will cause a Next.js error at runtime.Apply this diff to fix:
+"use client"; + import { SignOutButton } from "@clerk/nextjs";src/app/dashboard/signal/_components/signal-form.tsx (1)
124-130: Also enforce 500 chars in onChange to cover paste/programmatic input.You added maxLength; slice in onChange for belt-and-suspenders.
- onChange={(e) => setMessage(e.target.value)} + onChange={(e) => setMessage(e.target.value.slice(0, 500))}
🧹 Nitpick comments (9)
webrtc-server/index.js (2)
23-29: Consider structured logging for production.Console.log statements are suitable for development but consider using a structured logging library (e.g., winston, pino) for production to enable better observability and log management.
42-45: Validate room parameter.The room parameter is used directly without validation. Consider validating the room format and checking user permissions before allowing them to join rooms to prevent unauthorized access.
src/server/api/root.ts (1)
4-5: Consider consistent import style.Line 4 uses a relative import (
'./routers/chat') while line 5 uses a path alias ('@/server/api/routers/signal'). For consistency with other imports in this file, consider using the path alias format for both.Apply this diff for consistency:
-import { chatRouter } from './routers/chat'; +import { chatRouter } from '@/server/api/routers/chat'; import { signalRouter } from "@/server/api/routers/signal";src/server/api/routers/chat.ts (1)
6-23: Minimize data returned and sort conversations by recency.Select only needed fields and sort by updatedAt (or last message) to back the UI list efficiently. Consider adding pagination later.
return ctx.db.conversation.findMany({ where: { User: { some: { id: ctx.user.id, }, }, }, + orderBy: { updatedAt: 'desc' }, include: { - User: true, - messages: { + User: { select: { id: true, name: true } }, + messages: { orderBy: { createdAt: 'desc', }, take: 1, + select: { id: true, content: true, createdAt: true }, }, }, });Optionally, add cursor/limit inputs to getMessages to avoid unbounded result sets.
src/app/dashboard/_components/conversation-list.tsx (1)
3-4: Use Next.js Link for navigation and add fallbacks.Improves a11y, prefetching, and avoids empty renders when data is missing.
import { api } from "@/trpc/react"; -import { useRouter } from "next/navigation"; +import Link from "next/link"; @@ - <div - key={conversation.id} - className="cursor-pointer rounded-lg border p-4" - onClick={() => - router.push(`/dashboard/messages/${conversation.id}`) - } - > + <Link + key={conversation.id} + href={`/dashboard/messages/${conversation.id}`} + className="block rounded-lg border p-4 hover:bg-neutral-50" + > <p className="font-semibold"> - {conversation.User.map((user) => user.name).join(", ")} + {conversation.User.map((user) => user.name ?? "Unknown").join(", ")} </p> <p className="text-sm text-gray-500"> - {conversation.messages[0]?.content} + {conversation.messages[0]?.content ?? "No messages yet"} </p> - </div> + </Link>Also applies to: 19-33
src/app/dashboard/messages/[conversationId]/page.tsx (2)
25-35: Manage socket lifecycle: connect with auth, join/leave room, and disconnect.Prevents stale room membership and leaks on navigation.
- useEffect(() => { - socket.emit('join', conversationId); - - socket.on('message', () => { - void refetch(); - }); - - return () => { - socket.off('message'); - }; - }, [conversationId, refetch]); + const { getToken, isLoaded } = useAuth(); + const [socket, setSocket] = useState<Socket | null>(null); + + useEffect(() => { + if (!isLoaded) return; + let active = true; + (async () => { + const token = await getToken(); + const s = io(process.env.NEXT_PUBLIC_SOCKET_URL as string, { + auth: { token }, + }); + if (!active) return; + setSocket(s); + s.emit('join', conversationId); + s.on('message', () => void refetch()); + })(); + return () => { + active = false; + setSocket((s) => { + s?.emit('leave', conversationId); + s?.off('message'); + s?.disconnect(); + return null; + }); + }; + }, [conversationId, refetch, getToken, isLoaded]);
21-24: Handle user loading when deriving otherUser.Avoid showing the wrong participant before Clerk loads.
- const otherUser = messages?.[0]?.conversation?.User.find( - (u) => u.id !== user?.id - ); + const otherUser = user + ? messages?.[0]?.conversation?.User.find((u) => u.id !== user.id) + : undefined; @@ - <h1 className="text-xl font-semibold">{otherUser?.name}</h1> + <h1 className="text-xl font-semibold">{otherUser?.name ?? "Conversation"}</h1>Also applies to: 60-67
prisma/schema.prisma (2)
22-22: Clarify the Conversation-User many-to-many relation.The
Conversationmodel has a many-to-many relation toUservia@relation("UserConversations")(line 87), but it's unclear:
- How are users added to a conversation (no explicit join table visible)?
- Should a conversation have membership constraints (e.g., min 2 users)?
- Is this relation truly many-to-many, or should it be explicit with a join table for tracking member roles or permissions?
Consider making the relation explicit if membership metadata is needed, or add a comment explaining the intent.
Also applies to: 82-88
35-35: Add cascade delete to TutorCredentials and StudentGrades relations.For consistency with the recommendation above, add
onDelete: CascadetoTutorProfile→TutorCredentialsandStudentProfile→StudentGradesrelations to prevent orphaned credential/grade records.model TutorProfile { // ... - tutorCredentials TutorCredentials[] + tutorCredentials TutorCredentials[] @relation(onDelete: Cascade) // ... } model TutorCredentials { // ... - profile TutorProfile @relation(fields: [profileId], references: [id]) + profile TutorProfile @relation(fields: [profileId], references: [id], onDelete: Cascade) } model StudentProfile { // ... - studentGrades StudentGrades[] + studentGrades StudentGrades[] @relation(onDelete: Cascade) // ... } model StudentGrades { // ... - profile StudentProfile @relation(fields: [profileId], references: [id]) + profile StudentProfile @relation(fields: [profileId], references: [id], onDelete: Cascade) }Also applies to: 45-45, 58-58, 69-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlwebrtc-server/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
package.json(1 hunks)prisma/schema.prisma(2 hunks)src/app/_components/hero-illustration.tsx(1 hunks)src/app/dashboard/_components/conversation-list.tsx(1 hunks)src/app/dashboard/messages/[conversationId]/page.tsx(1 hunks)src/app/dashboard/messages/page.tsx(1 hunks)src/app/dashboard/settings/page.tsx(1 hunks)src/app/dashboard/signal/_components/signal-form.tsx(1 hunks)src/app/dashboard/signal/page.tsx(1 hunks)src/app/page.tsx(1 hunks)src/server/api/root.ts(2 hunks)src/server/api/routers/chat.ts(1 hunks)tsconfig.json(1 hunks)webrtc-server/index.js(2 hunks)webrtc-server/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-25T06:09:14.372Z
Learnt from: AJ-505
PR: Tobshub/tutorlink#28
File: src/app/dashboard/signal/page.tsx:138-173
Timestamp: 2025-10-25T06:09:14.372Z
Learning: `bg-linear-to-r`, `bg-linear-to-br`, and other `bg-linear-to-*` variants are valid Tailwind CSS utility classes for applying linear gradients, alongside the `bg-gradient-to-*` utilities.
Applied to files:
src/app/dashboard/signal/page.tsx
🧬 Code graph analysis (7)
src/server/api/root.ts (2)
src/server/api/routers/chat.ts (1)
chatRouter(4-59)src/server/api/routers/signal.ts (1)
signalRouter(7-66)
src/app/dashboard/signal/_components/signal-form.tsx (1)
src/trpc/react.tsx (2)
api(25-25)TRPCReactProvider(41-72)
src/app/dashboard/messages/page.tsx (1)
src/app/dashboard/_components/conversation-list.tsx (1)
ConversationList(6-37)
src/server/api/routers/chat.ts (1)
src/server/api/trpc.ts (3)
createTRPCRouter(78-78)protectedProcedure(104-104)ctx(85-96)
src/app/dashboard/_components/conversation-list.tsx (1)
src/trpc/react.tsx (2)
api(25-25)TRPCReactProvider(41-72)
src/app/dashboard/signal/page.tsx (1)
src/app/dashboard/signal/_components/signal-form.tsx (1)
SignalForm(34-200)
src/app/dashboard/messages/[conversationId]/page.tsx (2)
webrtc-server/index.js (1)
io(33-37)src/trpc/react.tsx (1)
api(25-25)
🔇 Additional comments (9)
src/app/_components/hero-illustration.tsx (2)
1-2: LGTM!Standard Next.js Image import follows best practices.
6-13: Great use of Next.js Image optimization!The implementation correctly uses Next.js Image with appropriate accessibility (alt text) and performance optimizations (priority loading for above-the-fold content). Image file verified to exist at the specified path.
package.json (1)
48-48: LGTM! Client-server Socket.IO versions match.The socket.io-client version (^4.8.1) is compatible with the server-side socket.io version specified in webrtc-server/package.json, ensuring proper communication between client and server.
tsconfig.json (1)
41-41: LGTM! Appropriate exclusion of webrtc-server.Excluding the webrtc-server directory is correct since it uses CommonJS module syntax while the main project uses ESM, preventing type-checking conflicts.
webrtc-server/package.json (1)
16-17: LGTM! Server-side Socket.IO dependency added.The socket.io version (^4.8.1) matches the client-side socket.io-client version, ensuring compatibility.
src/app/dashboard/messages/page.tsx (1)
1-12: LGTM! Proper client component setup.The page correctly uses the
"use client"directive and integrates the ConversationList component for displaying conversations.src/app/page.tsx (1)
31-34: LGTM! Clear and actionable tutor steps.The updated tutor step labels are clear, action-oriented, and effectively communicate the value proposition for tutors.
src/server/api/root.ts (1)
16-17: LGTM! Chat and signal routers properly registered.The new routers are correctly integrated into the appRouter, maintaining type safety and following the established pattern.
prisma/schema.prisma (1)
27-27: Review the impact ofuserId @uniqueconstraints.Adding
@uniqueconstraints touserIdinTutorProfile(line 27) andStudentProfile(line 50) enforces a one-to-one mapping. This breaks existing migrations if:
- A user has previously created multiple tutor/student profiles.
- The migration backfill strategy doesn't account for duplicates.
Verify that:
- No existing users have multiple profiles.
- The migration includes a
--forceflag or backfill logic to handle duplicates.- Application code enforces single-profile creation going forward.
If multiple profiles per user should be supported, remove the
@uniqueconstraint.Also applies to: 50-50
| supabaseUid String @unique | ||
| dateOfBirth DateTime? | ||
| role UserRole @default(STUDENT) | ||
| messages Message[] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Clarify the User-Message relation semantics.
The User model has a messages Message[] relation (line 19), but the Message model already connects to User via the sender relation (line 97). This creates an ambiguous dual relationship: are user messages only those they sent, or should they include received messages?
If messages should only represent sent messages, add an explicit relation name to clarify:
model User {
// ...
messages Message[]
// ...
}
model Message {
// ...
- sender User @relation(fields: [senderId], references: [id], onDelete: Cascade)
+ sender User @relation("UserSentMessages", fields: [senderId], references: [id], onDelete: Cascade)
}Then update the User model:
model User {
// ...
- messages Message[]
+ messages Message[] @relation("UserSentMessages")
// ...
}If users should have access to both sent and received messages, document this or reconsider the schema design.
Also applies to: 97-97
🤖 Prompt for AI Agents
In prisma/schema.prisma around lines 19 and 97, the User.messages relation is
ambiguous with Message.sender; decide the intended semantics and make the
relation explicit: if "messages" means only messages a user sent, rename
User.messages to sentMessages and add an explicit relation name on both sides
(e.g. relation("UserSentMessages")) and ensure the Message model has matching
relation(...) and foreign key fields/references; if users should access both
sent and received messages, remove the ambiguous single messages field and
replace it with two relations on User (sentMessages and receivedMessages) each
with distinct relation names and corresponding fields/references on Message
(e.g. senderId/recipientId), or add clear comments documenting the intent.
Ensure relation names, fields, and references match on both models to remove
ambiguity.
| model Conversation { | ||
| id String @id @default(cuid()) | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
| messages Message[] | ||
| User User[] @relation("UserConversations") | ||
| } | ||
|
|
||
| model Message { | ||
| id String @id @default(cuid()) | ||
| createdAt DateTime @default(now()) | ||
| content String | ||
| conversationId String | ||
| senderId String | ||
| conversation Conversation @relation(fields: [conversationId], references: [id]) | ||
| sender User @relation(fields: [senderId], references: [id]) | ||
| } |
There was a problem hiding this comment.
Add cascade delete behavior to foreign key relations.
The new Conversation and Message models lack explicit onDelete constraints. When a conversation or user is deleted, orphaned records could accumulate in the database, violating referential integrity and complicating future queries.
For example, deleting a conversation should cascade-delete its messages, and deleting a user should handle their messages appropriately.
Apply this diff to add cascade delete behavior:
model Conversation {
id String @id @default(cuid())
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt
messages Message[]
User User[] @relation("UserConversations")
}
model Message {
id String @id @default(cuid())
createdAt DateTime @default(now())
content String
conversationId String
senderId String
- conversation Conversation @relation(fields: [conversationId], references: [id])
- sender User @relation(fields: [senderId], references: [id])
+ conversation Conversation @relation(fields: [conversationId], references: [id], onDelete: Cascade)
+ sender User @relation(fields: [senderId], references: [id], onDelete: Cascade)
}Also add cascade delete to other foreign key relations (TutorProfile, TutorCredentials, StudentProfile, StudentGrades) for consistency.
📝 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.
| model Conversation { | |
| id String @id @default(cuid()) | |
| createdAt DateTime @default(now()) | |
| updatedAt DateTime @updatedAt | |
| messages Message[] | |
| User User[] @relation("UserConversations") | |
| } | |
| model Message { | |
| id String @id @default(cuid()) | |
| createdAt DateTime @default(now()) | |
| content String | |
| conversationId String | |
| senderId String | |
| conversation Conversation @relation(fields: [conversationId], references: [id]) | |
| sender User @relation(fields: [senderId], references: [id]) | |
| } | |
| model Conversation { | |
| id String @id @default(cuid()) | |
| createdAt DateTime @default(now()) | |
| updatedAt DateTime @updatedAt | |
| messages Message[] | |
| User User[] @relation("UserConversations") | |
| } | |
| model Message { | |
| id String @id @default(cuid()) | |
| createdAt DateTime @default(now()) | |
| content String | |
| conversationId String | |
| senderId String | |
| conversation Conversation @relation(fields: [conversationId], references: [id], onDelete: Cascade) | |
| sender User @relation(fields: [senderId], references: [id], onDelete: Cascade) | |
| } |
🤖 Prompt for AI Agents
In prisma/schema.prisma around lines 82 to 98, the Conversation/Message
relations do not specify onDelete behavior so deleting a Conversation or User
can leave orphaned Message rows; update the relation attributes to add onDelete:
Cascade on the foreign key relations (e.g., Message.conversation and
Message.sender) so messages are removed when their conversation or sender is
deleted, and apply the same pattern (add onDelete: Cascade on the @relation
attributes that reference User or Conversation) to other FK relations
(TutorProfile, TutorCredentials, StudentProfile, StudentGrades) to ensure
cascade deletes are enforced consistently.
| import { io } from 'socket.io-client'; | ||
| import { useUser } from '@clerk/nextjs'; | ||
|
|
||
| const socket = io('http://localhost:9000'); |
There was a problem hiding this comment.
Avoid hardcoded localhost and add auth to the socket.
Use an env URL and attach a token/user to authenticate.
-import { io } from 'socket.io-client';
+import { io, type Socket } from 'socket.io-client';
+import { useAuth } from '@clerk/nextjs';
@@
-const socket = io('http://localhost:9000');
+// will be created per page mount with auth in useEffectCommittable suggestion skipped: line range outside the PR's diff.
| const handleSendMessage = () => { | ||
| sendMessage.mutate( | ||
| { | ||
| conversationId: conversationId as string, | ||
| content, | ||
| }, | ||
| { | ||
| onSuccess: () => { | ||
| socket.emit('message', conversationId, content); | ||
| setContent(''); | ||
| }, | ||
| } | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Guard against empty messages and disable Send accordingly.
Prevents sending blanks; matches server validation.
- const handleSendMessage = () => {
+ const handleSendMessage = () => {
+ const trimmed = content.trim();
+ if (!trimmed) return;
sendMessage.mutate(
{
conversationId: conversationId as string,
- content,
+ content: trimmed,
},
{
onSuccess: () => {
socket.emit('message', conversationId, content);
setContent('');
},
}
);
};
@@
- <button
+ <button
className="px-4 py-2 bg-blue-500 text-white rounded-lg"
- onClick={handleSendMessage}
+ onClick={handleSendMessage}
+ disabled={!content.trim()}
>
Send
</button>Also applies to: 82-93
🤖 Prompt for AI Agents
In src/app/dashboard/messages/[conversationId]/page.tsx around lines 37-50 (and
apply same change to lines 82-93), the send flow doesn’t guard against empty or
whitespace-only messages and the Send control isn’t disabled; update
handleSendMessage to check if content.trim() is non-empty (and optionally return
early if sendMessage.isLoading) before calling sendMessage.mutate, and only emit
to the socket and clear content in the onSuccess callback; also update the Send
button/input control JSX to pass disabled={sendMessage.isLoading ||
!content.trim()} so the UI prevents sending blanks.
| getMessages: protectedProcedure | ||
| .input(z.object({ conversationId: z.string() })) | ||
| .query(async ({ ctx, input }) => { | ||
| return ctx.db.message.findMany({ | ||
| where: { | ||
| conversationId: input.conversationId, | ||
| }, | ||
| include: { | ||
| sender: true, | ||
| conversation: { include: { User: true }}, | ||
| }, | ||
| orderBy: { | ||
| createdAt: 'asc', | ||
| }, | ||
| }); | ||
| }), |
There was a problem hiding this comment.
Enforce conversation membership in getMessages/sendMessage.
Missing authZ allows reading/sending to conversations the user doesn’t belong to. Add a membership filter and forbid otherwise.
+import { TRPCError } from '@trpc/server';
@@
- getMessages: protectedProcedure
- .input(z.object({ conversationId: z.string() }))
+ getMessages: protectedProcedure
+ .input(z.object({ conversationId: z.string().min(1) }))
.query(async ({ ctx, input }) => {
- return ctx.db.message.findMany({
+ return ctx.db.message.findMany({
where: {
- conversationId: input.conversationId,
+ conversationId: input.conversationId,
+ conversation: {
+ User: { some: { id: ctx.user.id } },
+ },
},
- include: {
- sender: true,
- conversation: { include: { User: true }},
- },
+ include: {
+ sender: { select: { id: true, name: true } },
+ conversation: { include: { User: { select: { id: true, name: true } } } },
+ },
orderBy: {
createdAt: 'asc',
},
});
}),
@@
- sendMessage: protectedProcedure
- .input(
- z.object({
- conversationId: z.string(),
- content: z.string(),
- })
- )
+ sendMessage: protectedProcedure
+ .input(z.object({
+ conversationId: z.string().min(1),
+ content: z.string().trim().min(1).max(1000),
+ }))
.mutation(async ({ ctx, input }) => {
+ const authorized = await ctx.db.conversation.findFirst({
+ where: { id: input.conversationId, User: { some: { id: ctx.user.id } } },
+ select: { id: true },
+ });
+ if (!authorized) throw new TRPCError({ code: 'FORBIDDEN' });
return ctx.db.message.create({
data: {
conversationId: input.conversationId,
- content: input.content,
+ content: input.content.trim(),
senderId: ctx.user.id,
},
});
}),Also applies to: 43-59
🤖 Prompt for AI Agents
In src/server/api/routers/chat.ts around lines 26-41 (and similarly for 43-59),
the handlers do not enforce that the requesting user is a member of the
conversation; add an authorization check that the current user
(ctx.session.user.id) belongs to the conversation before returning messages or
creating a message. Fix by either 1) querying the conversation with a where that
matches both id and a participant/user relation containing ctx.session.user.id
and throw a 403/unauthorized if not found, then proceed to find/create messages;
or 2) perform a simple preliminary db lookup (conversation by id and membership)
and return/throw unauthorized when missing, then run the existing message
queries. Ensure the membership check uses the correct relation name in your
Prisma schema and apply the same pattern to both getMessages and sendMessage
(lines 43-59).
| const io = new Server(server, { | ||
| cors: { | ||
| origin: '*', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Restrict CORS origin for production.
The CORS configuration allows all origins (origin: '*'), which poses a security risk in production. This should be restricted to specific trusted origins.
Consider applying this pattern:
const io = new Server(server, {
cors: {
- origin: '*',
+ origin: process.env.ALLOWED_ORIGINS?.split(',') ?? ['http://localhost:3000'],
},
});| socket.on('message', (room, message) => { | ||
| socket.to(room).emit('message', message); | ||
| }); |
There was a problem hiding this comment.
Add message validation and rate limiting.
Messages are broadcast without content validation, sanitization, or rate limiting. This could enable spam or injection of malicious content.
Consider adding:
- Content validation and sanitization
- Rate limiting per socket/user
- Message size limits
- Authentication verification
🤖 Prompt for AI Agents
In webrtc-server/index.js around lines 47 to 49, the message handler broadcasts
raw messages without validation, sanitization, rate limiting, size limits, or
authentication checks; update the handler to first verify the socket/user is
authenticated, then validate message shape and content (required fields, type
checks), enforce a maximum message size, sanitize strings to remove/escape HTML
or scripts, and implement per-socket rate limiting (token bucket or simple
timestamp+counter) that drops or rejects messages when exceeded; return an error
event to the sender for invalid/oversized/rate-limited messages instead of
broadcasting.
There was a problem hiding this comment.
Actionable comments posted: 6
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)
47-59: Critical security issue: arbitrary field updates via spread operator.Line 55 uses
...inputto spread all input fields into the database update. Since the input schema only validatessignalIdandstatus, a malicious client could pass additional fields (e.g.,studentId,message,urgency) to overwrite protected data.Explicitly specify which fields to update:
.mutation(async ({ ctx, input }) => { const updatedSignal = await ctx.db.signal.update({ where: { id: input.signalId }, data: { - ...input, + status: input.status, } }); return { success: true, message: "Tutor profile updated successfully", }; }),Also, the success message says "Tutor profile updated" but this updates a signal—fix the message for clarity.
♻️ Duplicate comments (1)
src/app/dashboard/signal/page.tsx (1)
61-63: Wire the "Accept Signal" action or mark TODO.The Accept Signal button currently has no onClick handler, leaving it non-functional.
As noted in previous reviews, this button needs to be wired to an accept mutation or marked with a TODO comment.
🧹 Nitpick comments (5)
src/app/onboarding/tutor/summary/page.tsx (2)
3-4: Client-side role guard implemented correctly.The redirect logic properly waits for store hydration before checking the user's role, and the useEffect dependencies are complete. Using
router.replace(rather thanpush) is the right choice for unauthorized access redirects.However, consider adding a loading state while
hydratedis false to prevent users from briefly seeing the summary content before the redirect executes.For example:
export default function TutorSummary() { const role = useOnboardingStore((s) => s.role); const hydrated = useOnboardingStore((s) => s.hydrated); // ... other store selectors const router = useRouter(); useEffect(() => { if (!hydrated) return; if (role !== "tutor") router.replace("/onboarding"); }, [hydrated, role, router]); + if (!hydrated) { + return <div className="min-h-screen w-full flex items-center justify-center">Loading...</div>; + } + return ( <div className="min-h-screen w-full bg-linear-to-b from-white via-blue-50 to-[#43A8FF]">Also applies to: 10-10, 16-16, 18-21
61-63: Consider using Link for consistency with the Back button.The Finish button uses
onClickwithrouter.pushwhile the Back button uses aLinkcomponent. For consistency and better semantics, consider wrapping the Finish button in a Link as well (unless you plan to add additional logic like form submission in the future).- <Button variant="brand" className="rounded-full px-8 py-6 text-black" onClick={() => router.push("/dashboard")}> - Finish - </Button> + <Button asChild variant="brand" className="rounded-full px-8 py-6 text-black"> + <Link href="/dashboard">Finish</Link> + </Button>src/app/dashboard/signal/page.tsx (2)
69-70: Add error handling for the signals query.The query can fail but there's no error state displayed. Users would see an empty list with no explanation if the API call fails.
Add error handling:
- const { data: activeSignals = [], isLoading } = api.signal.getSignals.useQuery(); + const { data: activeSignals = [], isLoading, error } = api.signal.getSignals.useQuery();Then in the Active Signals tab content (around line 129), add an error state before the loading check:
{error ? ( <div className="text-center py-12"> <p className="text-red-600 text-lg">Failed to load signals</p> <p className="text-neutral-500 text-sm">{error.message}</p> </div> ) : isLoading ? ( // existing loading state...
172-175: Consider making stats dynamic.The "Current Stats" section displays hardcoded values that may become inaccurate over time.
If these metrics are important to users, consider fetching them from the backend or adding a disclaimer like "approximate" to set expectations.
src/server/api/routers/signal.ts (1)
60-66: Consider adding pagination for scalability.The query fetches all pending signals without pagination. As the platform grows, this could return a large dataset and impact performance.
Add cursor-based or offset pagination:
getSignals: protectedProcedure .input(z.object({ limit: z.number().min(1).max(100).default(20), cursor: z.string().optional(), }).optional()) .query(async ({ ctx, input }) => { const signals = await ctx.db.signal.findMany({ where: { status: "pending" }, take: input?.limit ?? 20, cursor: input?.cursor ? { id: input.cursor } : undefined, orderBy: { createdAt: 'desc' }, }); return signals; }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/dashboard/signal/page.tsx(1 hunks)src/app/onboarding/tutor/summary/page.tsx(2 hunks)src/server/api/routers/signal.ts(2 hunks)src/server/wsBridge.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-25T06:09:14.372Z
Learnt from: AJ-505
PR: Tobshub/tutorlink#28
File: src/app/dashboard/signal/page.tsx:138-173
Timestamp: 2025-10-25T06:09:14.372Z
Learning: `bg-linear-to-r`, `bg-linear-to-br`, and other `bg-linear-to-*` variants are valid Tailwind CSS utility classes for applying linear gradients, alongside the `bg-gradient-to-*` utilities.
Applied to files:
src/app/dashboard/signal/page.tsx
🧬 Code graph analysis (3)
src/app/dashboard/signal/page.tsx (2)
src/trpc/react.tsx (1)
api(25-25)src/app/dashboard/signal/_components/signal-form.tsx (1)
SignalForm(34-200)
src/app/onboarding/tutor/summary/page.tsx (1)
src/stores/onboarding.ts (1)
useOnboardingStore(96-220)
src/server/api/routers/signal.ts (2)
src/server/wsBridge.ts (2)
broadcastToTutors(6-14)notifyStudent(16-22)src/server/api/trpc.ts (2)
protectedProcedure(104-104)publicProcedure(87-87)
🔇 Additional comments (1)
src/server/api/routers/signal.ts (1)
60-66: Status values are already properly aligned—no changes needed.Verification shows the Signal model defines
statusasString @default("pending"), and bothcreateSignal(signal-form.tsx line 56) andgetSignals(signal.ts line 63) consistently use"pending". No instances of"awaiting"were found in the codebase, so the premise of the review comment is incorrect. The code alignment is sound.Likely an incorrect or invalid review comment.
| function SignalCard({ signal }: { signal: SignalData }) { | ||
| const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig]; | ||
| const timeAgo = new Date(signal.createdAt); | ||
| const now = new Date(); | ||
| const diffMs = now.getTime() - timeAgo.getTime(); | ||
| const diffMins = Math.floor(diffMs / 60000); | ||
| const displayTime = diffMins < 1 ? "just now" : `${diffMins}m ago`; |
There was a problem hiding this comment.
Add fallback for out-of-range urgency values.
The type assertion on Line 27 assumes signal.urgency is always 1-5, but if the API returns an unexpected value, config will be undefined and accessing config.label will throw a runtime error.
Apply this diff to add a safe fallback:
function SignalCard({ signal }: { signal: SignalData }) {
- const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig];
+ const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig] ?? urgencyConfig[3];
const timeAgo = new Date(signal.createdAt);🤖 Prompt for AI Agents
In src/app/dashboard/signal/page.tsx around lines 26 to 32, the lookup
urgencyConfig[signal.urgency as keyof typeof urgencyConfig] can return undefined
for out-of-range or unexpected values causing runtime errors when reading config
properties; replace the direct assertion with a safe lookup that falls back to a
default config (e.g., an "unknown" or lowest-priority config) when urgency is
missing or not a valid key—validate signal.urgency (typeof number or string),
attempt to get urgencyConfig[key], and if undefined use the default object
before using config.label or other properties.
| const signalData = { ...input, studentId, createdAt: new Date().toISOString() }; | ||
| await ctx.db.signal.create({ | ||
| data:signalData}); | ||
| data: signalData | ||
| }); | ||
| console.log("Signal created:", signalData); |
There was a problem hiding this comment.
Avoid logging potentially sensitive user data.
Line 21 logs the complete signal data including the student's question (message) and identifier (studentId). In production, this could expose PII or sensitive educational content in server logs.
Consider logging only the signal ID or removing the log:
- console.log("Signal created:", signalData);
+ console.log("Signal created:", { signalId: signalData.id });Or remove it entirely if it's only for debugging.
📝 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 signalData = { ...input, studentId, createdAt: new Date().toISOString() }; | |
| await ctx.db.signal.create({ | |
| data:signalData}); | |
| data: signalData | |
| }); | |
| console.log("Signal created:", signalData); | |
| const signalData = { ...input, studentId, createdAt: new Date().toISOString() }; | |
| const signal = await ctx.db.signal.create({ | |
| data: signalData | |
| }); | |
| console.log("Signal created:", { signalId: signal.id }); |
🤖 Prompt for AI Agents
In src/server/api/routers/signal.ts around lines 17 to 21, the code logs the
full signalData (including studentId and message), which may expose PII; remove
the full console.log or replace it with a safe log that only records the created
signal's non-sensitive identifier. To fix, capture the result returned by
ctx.db.signal.create and either delete the console.log line entirely or change
it to log only result.id (and any non-sensitive metadata), ensuring no studentId
or message content is written to logs.
| updateSignal: protectedProcedure | ||
| .input(z.object({ signalId: z.string(), studentId: z.string(), status: z.string()})) | ||
| .mutation(async ({ input, ctx }) => { | ||
| const signal = await ctx.db.signal.findUnique({where:{id: input.signalId }}); | ||
| if (input.status=="accepted"){ | ||
| await notifyStudent(input.studentId, { | ||
| event: "signal-accepted", | ||
| data: { signalId: input.signalId, tutorId: ctx.user.id }, | ||
| }); | ||
| await ctx.db.signal.update({ where: { id: input.signalId}, data: { status: "accepted"}}) | ||
| return { success: true, message: "Signal accepted" }; | ||
| } else{ | ||
| await notifyStudent(input.studentId, { | ||
| event: "signal-rejected", | ||
| data: { signalId: input.signalId, tutorId: ctx.user.id }, | ||
| }); | ||
| await ctx.db.signal.update({ where: { id: input.signalId}, data: { status: "rejected"}}); | ||
| } | ||
| return { success: true, message: "Tutor profile updated successfully", }; | ||
| }), | ||
| .input(z.object({ signalId: z.string(), studentId: z.string(), status: z.string() })) | ||
| .mutation(async ({ input, ctx }) => { | ||
| const signal = await ctx.db.signal.findUnique({ where: { id: input.signalId } }); | ||
| if (input.status == "accepted") { | ||
| await notifyStudent(input.studentId, { | ||
| event: "signal-accepted", | ||
| data: { signalId: input.signalId, tutorId: ctx.user.id }, | ||
| }); | ||
| await ctx.db.signal.update({ where: { id: input.signalId }, data: { status: "accepted" } }) | ||
| return { success: true, message: "Signal accepted" }; | ||
| } else { | ||
| await notifyStudent(input.studentId, { | ||
| event: "signal-rejected", | ||
| data: { signalId: input.signalId, tutorId: ctx.user.id }, | ||
| }); | ||
| await ctx.db.signal.update({ where: { id: input.signalId }, data: { status: "rejected" } }); | ||
| } | ||
| return { success: true, message: "Tutor profile updated successfully", }; | ||
| }), |
There was a problem hiding this comment.
Add input validation and authorization checks.
The updateSignal mutation allows any authenticated user to update any signal without verifying:
- The signal exists (fetched on Line 29 but not checked for null)
- The user is authorized (e.g., is the accepting tutor)
- The status value is valid
Add validation before processing:
.mutation(async ({ input, ctx }) => {
const signal = await ctx.db.signal.findUnique({ where: { id: input.signalId } });
if (!signal) {
throw new Error("Signal not found");
}
// Validate status
if (!["accepted", "rejected"].includes(input.status)) {
throw new Error("Invalid status value");
}
// Add authorization check if needed
// e.g., verify ctx.user.role === "tutor"
if (input.status === "accepted") {
// ... rest of logic🤖 Prompt for AI Agents
In src/server/api/routers/signal.ts around lines 26 to 45, the updateSignal
mutation fetches a signal but does not check for null, does not validate the
incoming status, and lacks authorization checks allowing any authenticated user
to modify any signal; fix by: after fetching signal, throw a NotFound error if
null; validate input.status is one of ["accepted","rejected"] and throw a
BadRequest error if not; enforce authorization (e.g., confirm ctx.user.role ===
"tutor" and/or ctx.user.id matches the signal.tutorId) before performing
notify/update; then proceed with the existing notify and update logic.
| .input(z.object({ signalId: z.string(), studentId: z.string(), status: z.string() })) | ||
| .mutation(async ({ input, ctx }) => { | ||
| const signal = await ctx.db.signal.findUnique({ where: { id: input.signalId } }); | ||
| if (input.status == "accepted") { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use strict equality for string comparison.
Line 30 uses loose equality (==) instead of strict equality (===) for string comparison. While it works for strings, strict equality is more predictable and a JavaScript best practice.
- if (input.status == "accepted") {
+ if (input.status === "accepted") {📝 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.
| if (input.status == "accepted") { | |
| if (input.status === "accepted") { |
🤖 Prompt for AI Agents
In src/server/api/routers/signal.ts around line 30, the code uses loose equality
(==) to compare input.status to the string "accepted"; change this to strict
equality (===) so the comparison uses both value and type equality (i.e.,
replace `input.status == "accepted"` with `input.status === "accepted"`),
ensuring consistent and idiomatic JavaScript string comparison.
| console.log("Broadcasting to tutors:", payload); | ||
| await fetch(WEBSOCKET_URL, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(payload), | ||
| }); | ||
| console.log("Broadcast to tutors sent."); |
There was a problem hiding this comment.
Add error handling and use structured logging.
The added console logs raise several concerns:
- Sensitive data exposure: Logging the entire payload may expose sensitive information (PII, authentication tokens, etc.).
- Misleading logs: The fetch call lacks error handling, so "Broadcast to tutors sent." will print even if the request fails.
- Console.log in production: Consider using a structured logging framework (e.g., winston, pino) for better observability and log management in production environments.
Apply this diff to add error handling and safer logging:
- console.log("Broadcasting to tutors:", payload);
- await fetch(WEBSOCKET_URL, {
- method: "POST",
- headers: { "Content-Type": "application/json" },
- body: JSON.stringify(payload),
- });
- console.log("Broadcast to tutors sent.");
+ try {
+ console.log("Broadcasting to tutors...");
+ const response = await fetch(WEBSOCKET_URL, {
+ method: "POST",
+ headers: { "Content-Type": "application/json" },
+ body: JSON.stringify(payload),
+ });
+ if (!response.ok) {
+ console.error(`WebSocket broadcast failed: ${response.status} ${response.statusText}`);
+ return;
+ }
+ console.log("Broadcast to tutors sent successfully.");
+ } catch (error) {
+ console.error("Failed to broadcast to tutors:", error);
+ }📝 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.
| console.log("Broadcasting to tutors:", payload); | |
| await fetch(WEBSOCKET_URL, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify(payload), | |
| }); | |
| console.log("Broadcast to tutors sent."); | |
| try { | |
| console.log("Broadcasting to tutors..."); | |
| const response = await fetch(WEBSOCKET_URL, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify(payload), | |
| }); | |
| if (!response.ok) { | |
| console.error(`WebSocket broadcast failed: ${response.status} ${response.statusText}`); | |
| return; | |
| } | |
| console.log("Broadcast to tutors sent successfully."); | |
| } catch (error) { | |
| console.error("Failed to broadcast to tutors:", error); | |
| } |
🤖 Prompt for AI Agents
In src/server/wsBridge.ts around lines 7 to 13, replace the raw console logs and
unguarded fetch with structured logging and error handling: do not log the full
payload (mask or log only non-sensitive fields or a safe summary), use the
application's logger (e.g., logger.info/logger.error) instead of console.log,
wrap the fetch in a try/catch, await the response and check response.ok before
logging success, and log failures including HTTP status/text or the caught
error; ensure no sensitive fields (tokens, PII) are included in logs by
redacting or omitting them.
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)
57-69: Add validation and authorization checks.This procedure mirrors the issues in
updateSignal:
- No validation that the signal exists
- No authorization check (anyone can verify any signal)
- Direct spread of input could allow unintended field updates
- Success message mentions "Tutor profile" which is misleading
Consider adding validation and restricting which fields can be updated:
verifySignal: protectedProcedure - .input(z.object({ - signalId: z.string(), status: z.string() - })) + .input(z.object({ + signalId: z.string(), + status: z.enum(["pending", "accepted", "rejected"]) + })) .mutation(async ({ ctx, input }) => { + const signal = await ctx.db.signal.findUnique({ + where: { id: input.signalId } + }); + if (!signal) { + throw new TRPCError({ code: "NOT_FOUND", message: "Signal not found" }); + } + // Add authorization check based on your business logic + await ctx.db.signal.update({ where: { id: input.signalId }, - data: { - ...input, - } + data: { status: input.status } }); - return { success: true, message: "Tutor profile updated successfully", }; + return { success: true, message: "Signal updated successfully" }; }),
♻️ Duplicate comments (4)
src/server/api/routers/signal.ts (2)
32-32: Avoid logging sensitive user data.Line 32 logs the complete signal data including potentially sensitive information like
studentIdandmessage. This was flagged in a previous review but remains unaddressed.
37-55: Critical: Authorization and validation still missing.This procedure still lacks the critical safeguards flagged in previous reviews:
- No check that the signal exists
- No authorization (any authenticated user can update any signal)
- No validation of status value
- Line 40 uses loose equality (
==) instead of strict (===)src/app/dashboard/signal/page.tsx (2)
27-35: Add fallback for invalid urgency values.Line 30 can return
undefinedifsignal.urgencyis outside the 1-5 range, causing runtime errors when accessingconfig.labelor other properties. This was flagged in a previous review but remains unaddressed.Apply this diff:
- const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig]; + const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig] ?? urgencyConfig[3];
194-202: Add fallback for invalid urgency values.Line 196 has the same issue as line 30: the urgency config lookup can return
undefinedfor out-of-range values.Apply this diff:
- const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig]; + const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig] ?? urgencyConfig[3];
🧹 Nitpick comments (2)
src/app/dashboard/signal/page.tsx (2)
9-17: Consider narrowing the createdAt type.The
createdAtfield is typed asDate | string, which reduces type safety. Since the TRPC API likely returns a consistent type (probably string in JSON), consider narrowing this to match the actual return type.type SignalData = { id: string; studentId: string; subject: string; message: string; urgency: number; status: string; - createdAt: Date | string; + createdAt: string; };
249-271: Consider fetching real-time stats from the API.The "Current Stats" section displays hardcoded values (150+ active tutors, 98% satisfaction rate). Consider fetching these from the backend for accuracy.
This could be a future enhancement if real-time statistics become important for the user experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/dashboard/signal/page.tsx(1 hunks)src/server/api/routers/signal.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-25T06:09:14.372Z
Learnt from: AJ-505
PR: Tobshub/tutorlink#28
File: src/app/dashboard/signal/page.tsx:138-173
Timestamp: 2025-10-25T06:09:14.372Z
Learning: `bg-linear-to-r`, `bg-linear-to-br`, and other `bg-linear-to-*` variants are valid Tailwind CSS utility classes for applying linear gradients, alongside the `bg-gradient-to-*` utilities.
Applied to files:
src/app/dashboard/signal/page.tsx
🧬 Code graph analysis (2)
src/server/api/routers/signal.ts (2)
src/server/api/trpc.ts (3)
createTRPCRouter(78-78)protectedProcedure(104-104)publicProcedure(87-87)src/server/wsBridge.ts (2)
broadcastToTutors(6-14)notifyStudent(16-22)
src/app/dashboard/signal/page.tsx (2)
src/trpc/react.tsx (1)
api(25-25)src/app/dashboard/signal/_components/signal-form.tsx (1)
SignalForm(34-200)
🔇 Additional comments (7)
src/server/api/routers/signal.ts (5)
9-17: LGTM: Clean role retrieval with proper authorization.The endpoint correctly validates user existence and returns the role for conditional UI rendering.
70-86: LGTM: Well-implemented with proper role-based access control.The procedure correctly restricts access to tutors, filters for pending signals, and includes appropriate limits. Good use of TRPCError for authorization failures.
88-95: LGTM: Clean implementation with implicit authorization.The procedure correctly filters signals by the current user's ID, ensuring users can only see their own signals.
97-111: Excellent: Comprehensive validation and authorization.This procedure demonstrates proper implementation with all necessary checks:
- Validates signal existence (NOT_FOUND)
- Verifies ownership (FORBIDDEN)
- Checks status constraints (CONFLICT)
This serves as a good pattern for the other procedures in this file.
171-171: LGTM.Simple health check endpoint is appropriate.
src/app/dashboard/signal/page.tsx (2)
64-80: LGTM: Accept Signal action properly wired.The button now correctly calls the
acceptSignalmutation with proper loading state handling and navigation on success. This addresses the previous review concern about the dead UI.
85-96: LGTM: Clean data fetching with role-based conditional queries.The component properly:
- Fetches viewer role for conditional rendering
- Conditionally enables queries based on user role
- Handles cache invalidation on mutations
| acceptSignal: protectedProcedure | ||
| .input(z.object({ signalId: z.string() })) | ||
| .mutation(async ({ ctx, input }) => { | ||
| // Guard: Only tutors can accept signals | ||
| const me = await ctx.db.user.findUnique({ | ||
| where: { id: ctx.user.id }, | ||
| select: { id: true, role: true }, | ||
| }); | ||
| if (me?.role !== "TUTOR") { | ||
| throw new TRPCError({ code: "FORBIDDEN", message: "Only tutors can accept signals" }); | ||
| } | ||
|
|
||
| // Fetch signal | ||
| const signal = await ctx.db.signal.findUnique({ where: { id: input.signalId } }); | ||
| if (!signal) { | ||
| throw new TRPCError({ code: "NOT_FOUND", message: "Signal not found" }); | ||
| } | ||
| if (signal.status !== "pending") { | ||
| throw new TRPCError({ code: "CONFLICT", message: "Signal already handled" }); | ||
| } | ||
|
|
||
| // TODO: Add transactional handling, concurrency locks to avoid double-accept | ||
| // Create or reuse a conversation between tutor (me.id) and student (signal.studentId) | ||
| const conversation = await ctx.db.conversation.create({ | ||
| data: { | ||
| User: { | ||
| connect: [ | ||
| { id: me.id }, | ||
| { id: signal.studentId }, | ||
| ], | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| // Update signal status | ||
| await ctx.db.signal.update({ | ||
| where: { id: input.signalId }, | ||
| data: { status: "accepted" }, | ||
| }); | ||
|
|
||
| // Notify student via WS bridge (best-effort) | ||
| try { | ||
| await notifyStudent(signal.studentId, { | ||
| event: "signal-accepted", | ||
| data: { signalId: input.signalId, tutorId: me.id, conversationId: conversation.id }, | ||
| }); | ||
| } catch (e) { | ||
| // non-fatal | ||
| console.warn("notifyStudent failed", e); | ||
| } | ||
|
|
||
| return { | ||
| success: true, | ||
| message: "Signal accepted", | ||
| conversationId: conversation.id, | ||
| redirectTo: `/dashboard/messages/${conversation.id}`, | ||
| }; | ||
| }), |
There was a problem hiding this comment.
Critical: Add transaction and concurrency control.
The TODO on line 134 identifies a critical issue: without a transaction and concurrency lock, race conditions can occur. Two tutors accepting the same signal simultaneously could result in:
- Multiple conversations created for one signal
- Inconsistent state if conversation succeeds but signal update fails
Wrap the operation in a transaction with optimistic locking:
acceptSignal: protectedProcedure
.input(z.object({ signalId: z.string() }))
.mutation(async ({ ctx, input }) => {
const me = await ctx.db.user.findUnique({
where: { id: ctx.user.id },
select: { id: true, role: true },
});
if (me?.role !== "TUTOR") {
throw new TRPCError({ code: "FORBIDDEN", message: "Only tutors can accept signals" });
}
// Use a transaction to ensure atomicity
const result = await ctx.db.$transaction(async (tx) => {
// Fetch and lock the signal in one operation
const signal = await tx.signal.findUnique({
where: { id: input.signalId },
});
if (!signal) {
throw new TRPCError({ code: "NOT_FOUND", message: "Signal not found" });
}
if (signal.status !== "pending") {
throw new TRPCError({ code: "CONFLICT", message: "Signal already handled" });
}
// Update signal first to claim it (prevents race condition)
await tx.signal.update({
where: { id: input.signalId },
data: { status: "accepted" },
});
// Create conversation
const conversation = await tx.conversation.create({
data: {
User: {
connect: [
{ id: me.id },
{ id: signal.studentId },
],
},
},
});
return { signal, conversation };
});
// Notify after transaction succeeds (best-effort)
try {
await notifyStudent(result.signal.studentId, {
event: "signal-accepted",
data: {
signalId: input.signalId,
tutorId: me.id,
conversationId: result.conversation.id
},
});
} catch (e) {
console.warn("notifyStudent failed", e);
}
return {
success: true,
message: "Signal accepted",
conversationId: result.conversation.id,
redirectTo: `/dashboard/messages/${result.conversation.id}`,
};
}),
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)
54-66: Add signal existence and authorization validation.The mutation updates a signal without verifying it exists or checking authorization.
Apply this diff:
verifySignal: protectedProcedure .input(z.object({ signalId: z.string(), status: z.string() })) .mutation(async ({ ctx, input }) => { + const signal = await ctx.db.signal.findUnique({ where: { id: input.signalId } }); + if (!signal) { + throw new TRPCError({ code: "NOT_FOUND", message: "Signal not found" }); + } + // TODO: Add authorization check - verify ctx.user is allowed to verify this signal + await ctx.db.signal.update({
♻️ Duplicate comments (6)
src/server/api/routers/signal.ts (4)
29-29: Remove or sanitize the console log.Line 29 logs the complete
signalDataobject includingstudentIdandmessage, which may expose PII in server logs.Based on learnings
37-37: Use strict equality.Line 37 uses loose equality (
==) instead of strict equality (===).- if (input.status == "accepted") { + if (input.status === "accepted") {
104-161: Critical: Implement transaction to prevent race conditions.The TODO on line 125 identifies a critical issue: without wrapping the conversation creation and signal update in a transaction, two tutors can simultaneously accept the same signal, creating duplicate conversations and inconsistent state.
34-52: Add signal existence and authorization validation.The mutation updates a signal without verifying it exists or checking if the user is authorized to update it. After the unused query was removed (commit 6e026b1), no validation was added to replace it.
Apply this diff to add proper validation:
updateSignal: protectedProcedure .input(z.object({ signalId: z.string(), studentId: z.string(), status: z.string() })) .mutation(async ({ input, ctx }) => { + const signal = await ctx.db.signal.findUnique({ where: { id: input.signalId } }); + if (!signal) { + throw new TRPCError({ code: "NOT_FOUND", message: "Signal not found" }); + } + // Add authorization check if needed - verify ctx.user is allowed to update this signal + if (input.status === "accepted") {src/app/dashboard/signal/page.tsx (2)
28-36: Add fallback for out-of-range urgency values.Line 31 assumes
signal.urgencyis always 1-5. If the API returns an unexpected value,configwill beundefined, causing a runtime error when accessingconfig.labelor other properties.Apply this diff:
function SignalCard({ signal }: { signal: SignalData }) { const router = useRouter(); const acceptSignal = api.signal.acceptSignal.useMutation(); - const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig]; + const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig] ?? urgencyConfig[3]; const timeAgo = new Date(signal.createdAt);
200-240: Add fallback for urgency config lookup.Line 202 has the same issue as line 31: if
signal.urgencyis out of range,configwill be undefined.Apply this diff:
{mySignals.map((signal: SignalData) => { - const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig]; + const config = urgencyConfig[signal.urgency as keyof typeof urgencyConfig] ?? urgencyConfig[3]; const timeAgo = new Date(signal.createdAt);
🧹 Nitpick comments (1)
src/server/api/routers/signal.ts (1)
79-86: LGTM!The query correctly filters signals by the authenticated user's ID, ensuring users only see their own signals.
Consider adding pagination if users might accumulate many signals:
getMySignals: protectedProcedure .input(z.object({ limit: z.number().min(1).max(100).default(50).optional() }).optional()) .query(async ({ ctx, input }) => { const signals = await ctx.db.signal.findMany({ where: { studentId: ctx.user.id }, orderBy: { createdAt: "desc" }, take: input?.limit ?? 50, }); return signals; }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json(1 hunks)src/app/dashboard/signal/_components/signal-form.tsx(1 hunks)src/app/dashboard/signal/page.tsx(1 hunks)src/server/api/routers/signal.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/dashboard/signal/_components/signal-form.tsx
- package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-25T06:09:14.372Z
Learnt from: AJ-505
PR: Tobshub/tutorlink#28
File: src/app/dashboard/signal/page.tsx:138-173
Timestamp: 2025-10-25T06:09:14.372Z
Learning: `bg-linear-to-r`, `bg-linear-to-br`, and other `bg-linear-to-*` variants are valid Tailwind CSS utility classes for applying linear gradients, alongside the `bg-gradient-to-*` utilities.
Applied to files:
src/app/dashboard/signal/page.tsx
🧬 Code graph analysis (2)
src/server/api/routers/signal.ts (2)
src/server/api/trpc.ts (3)
createTRPCRouter(78-78)protectedProcedure(104-104)publicProcedure(87-87)src/server/wsBridge.ts (2)
broadcastToTutors(6-14)notifyStudent(16-22)
src/app/dashboard/signal/page.tsx (2)
src/trpc/react.tsx (1)
api(25-25)src/app/dashboard/signal/_components/signal-form.tsx (1)
SignalForm(35-177)
🔇 Additional comments (5)
src/server/api/routers/signal.ts (3)
67-77: LGTM!The query correctly filters pending signals and relies on the frontend's role-based enabled flag. The ordering and pagination are appropriate.
88-102: Excellent validation and authorization!This mutation demonstrates proper defensive programming with comprehensive checks for existence, ownership, and state validation before performing the deletion.
162-162: LGTM!Simple health check endpoint is correctly implemented as a public procedure.
src/app/dashboard/signal/page.tsx (2)
103-280: Well-structured UI with proper loading and empty states.The component correctly implements:
- Role-based view switching for tutors vs students
- Tab navigation for students (Create/Manage)
- Loading states for async queries
- Empty states with helpful messaging
- Proper mutation handling with toasts
86-101: Frontend implementation is correct; server-side dependency is accurately documented.The role-based query gating (lines 89-92) is correctly implemented. The
isTutorflag properly controls query execution through theenabledparameter. The issue identified—hardcoded"STUDENT"return insrc/server/api/routers/signal.tsline 13—is confirmed and accurately represents a blocker, but it's a server-side concern, not a frontend code problem.
| // Returns the current viewer's role for conditional UI rendering | ||
| getViewerRole: protectedProcedure.query(async () => { | ||
| // Clerk manages auth, so default to STUDENT role | ||
| // Tutors will be explicitly set in DB when they complete tutor onboarding | ||
| return "STUDENT"; | ||
| }), |
There was a problem hiding this comment.
Fetch the actual user role from the database.
This endpoint returns the hardcoded string "STUDENT" instead of querying the database for the user's actual role. This breaks the entire tutor view on the frontend (line 90 in page.tsx uses this to determine isTutor), meaning tutors will never see active signals.
Apply this diff to fetch the role from the database:
- getViewerRole: protectedProcedure.query(async () => {
- // Clerk manages auth, so default to STUDENT role
- // Tutors will be explicitly set in DB when they complete tutor onboarding
- return "STUDENT";
+ getViewerRole: protectedProcedure.query(async ({ ctx }) => {
+ const user = await ctx.db.user.findUnique({
+ where: { id: ctx.user.id },
+ select: { role: true },
+ });
+ return user?.role ?? "STUDENT";
}),📝 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.
| // Returns the current viewer's role for conditional UI rendering | |
| getViewerRole: protectedProcedure.query(async () => { | |
| // Clerk manages auth, so default to STUDENT role | |
| // Tutors will be explicitly set in DB when they complete tutor onboarding | |
| return "STUDENT"; | |
| }), | |
| // Returns the current viewer's role for conditional UI rendering | |
| getViewerRole: protectedProcedure.query(async ({ ctx }) => { | |
| const user = await ctx.db.user.findUnique({ | |
| where: { id: ctx.user.id }, | |
| select: { role: true }, | |
| }); | |
| return user?.role ?? "STUDENT"; | |
| }), |
🤖 Prompt for AI Agents
In src/server/api/routers/signal.ts around lines 9 to 14, the getViewerRole
resolver currently returns the hardcoded string "STUDENT"; replace this with a
DB lookup using the authenticated user's id from the procedure context (e.g.,
ctx.user.id or ctx.session.userId) to fetch the user record and return its role.
If the user id is missing or the DB lookup returns no user, fall back to
"STUDENT". Ensure the query is awaited, handle errors or nulls safely, and
return only the role string.
Summary by CodeRabbit
New Features
UI/UX Updates