Implemented appointment booking and cancellation #5
Open
Akshaya-SK wants to merge 5 commits intobotcode-com:mainfrom
Open
Implemented appointment booking and cancellation #5Akshaya-SK wants to merge 5 commits intobotcode-com:mainfrom
Akshaya-SK wants to merge 5 commits intobotcode-com:mainfrom
Conversation
WalkthroughAdds admin login/dashboard pages, implements appointment booking and cancellation APIs with server-side Supabase auth checks and business rules, refactors schema to remove slot-level booking flags and add appointment-level constraints and RLS policies, updates doctor/patient dashboards to use bearer tokens, and adds Jest tests and validation helpers. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant Browser
participant NextJS_Page
participant Supabase_Auth
participant Supabase_DB
Admin->>Browser: Open /admin/login
Browser->>NextJS_Page: Load login page
Admin->>Browser: Submit credentials
Browser->>NextJS_Page: signInWithPassword -> Supabase_Auth
NextJS_Page->>Supabase_Auth: signInWithPassword()
Supabase_Auth-->>NextJS_Page: session/token
NextJS_Page->>Supabase_DB: Query `system_admins` by user id
Supabase_DB-->>NextJS_Page: admin record?
alt admin exists
NextJS_Page->>Browser: Redirect to /admin/dashboard
Browser->>NextJS_Page: Load dashboard (client)
NextJS_Page->>Supabase_DB: Concurrent fetch: appointments, doctors, patients, slots
Supabase_DB-->>NextJS_Page: data with relations
NextJS_Page->>Browser: Render metrics & tables
else not admin
NextJS_Page->>Supabase_Auth: signOut()
NextJS_Page->>Browser: Show error / redirect
end
sequenceDiagram
actor Patient
participant Browser
participant NextJS_API
participant Supabase_Auth
participant Supabase_DB
Patient->>Browser: Click "Book" on slot
Browser->>Browser: Get session token
Browser->>NextJS_API: POST /api/appointments/book (Authorization: Bearer token, body: slotId)
NextJS_API->>Supabase_Auth: auth.getUser(bearer)
Supabase_Auth-->>NextJS_API: user id
NextJS_API->>Supabase_DB: Check existing appointments for slot (status IN active,done)
Supabase_DB-->>NextJS_API: conflict?
alt slot free
NextJS_API->>Supabase_DB: Check active appointment for same patient+doctor
Supabase_DB-->>NextJS_API: duplicate?
alt no duplicate
NextJS_API->>Supabase_DB: INSERT appointment (status: active)
Supabase_DB-->>NextJS_API: success -> 201
NextJS_API->>Browser: success
else duplicate
NextJS_API->>Browser: 409 Conflict
end
else slot occupied
NextJS_API->>Browser: 409 Conflict
end
sequenceDiagram
actor User
participant Browser
participant NextJS_API
participant Supabase_Auth
participant Supabase_DB
User->>Browser: Click "Cancel/Mark done"
Browser->>Browser: Get session token
Browser->>NextJS_API: POST /api/appointments/cancel (Authorization: Bearer token, appointmentId, action)
NextJS_API->>Supabase_Auth: auth.getUser(bearer)
Supabase_Auth-->>NextJS_API: user id
NextJS_API->>Supabase_DB: Fetch appointment + slot.start_time
Supabase_DB-->>NextJS_API: appointment data
alt user is doctor or patient
alt appointment.status == active
alt action == done and user is patient
NextJS_API->>Browser: 403/400 (forbidden)
else if patient within 1 hour and action == cancelled
NextJS_API->>Browser: 409 (too late)
else
NextJS_API->>Supabase_DB: UPDATE appointment.status -> action
Supabase_DB-->>NextJS_API: success
NextJS_API->>Browser: 200 OK
end
else
NextJS_API->>Browser: 409 Conflict (invalid state)
end
else
NextJS_API->>Browser: 403 Forbidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an administrative dashboard and login system, updates the doctor and patient dashboards, and implements backend API routes for booking and cancelling appointments. It also includes a comprehensive testing suite and database schema updates with Row Level Security (RLS) policies. Key feedback includes fixing a Supabase query bug in the booking route, adding a database-level unique index to prevent slot double-booking, restricting overly broad RLS policies on appointments to protect patient privacy, and optimizing slot availability filtering by moving it from the client to the database.
| .select("id") | ||
| .eq("slot_id", slotId) | ||
| .eq("status", ["active", "done"]) | ||
| .maybeSingle(); |
There was a problem hiding this comment.
The .eq() filter in the Supabase client does not support arrays for 'IN' logic. This query will fail to find existing appointments correctly because it will look for a status that literally matches the string representation of the array. Use the .in() method instead.
Suggested change
| .maybeSingle(); | |
| .in("status", ["active", "done"]) |
Comment on lines
+38
to
+40
| CREATE UNIQUE INDEX IF NOT EXISTS unique_active_patient_doctor | ||
| ON appointments (patient_id, doctor_id) | ||
| WHERE status = 'active'; |
There was a problem hiding this comment.
The current unique index prevents a patient from booking the same doctor twice, but it does not prevent 'double booking' where two different patients book the same slot. Since the API check is non-atomic, a race condition exists. You should add a partial unique index on slot_id for active and done appointments to ensure data integrity at the database level.
CREATE UNIQUE INDEX IF NOT EXISTS unique_active_slot
ON appointments (slot_id)
WHERE status IN ('active', 'done');
CREATE UNIQUE INDEX IF NOT EXISTS unique_active_patient_doctor
ON appointments (patient_id, doctor_id)
WHERE status = 'active';
Comment on lines
+72
to
+73
| CREATE POLICY "authenticated_read_appointments" ON appointments | ||
| FOR SELECT USING (auth.role() = 'authenticated'); |
There was a problem hiding this comment.
This policy allows any authenticated user to read all appointment records, which exposes sensitive patient data (names, doctor choices, etc.) to other patients. This is a significant privacy violation. Access should be restricted to the specific patient, the assigned doctor, or an admin. To check slot availability without exposing appointment details, consider using a more restrictive approach or a dedicated database view.
Comment on lines
+58
to
+76
| const { data: bookedAppointments } = await supabase | ||
| .from("appointments") | ||
| .select("slot_id") | ||
| .in("status", ["active","done"]); | ||
|
|
||
| setPatient(patientData); | ||
| const bookedSlotIds = (bookedAppointments ?? []).map((a) => a.slot_id); | ||
|
|
||
| const { data: slotsData } = await supabase | ||
| // Get all slots, then filter out booked ones on the client | ||
| const now = new Date().toISOString(); | ||
| const { data: allSlots } = await supabase | ||
| .from("slots") | ||
| .select("id, start_time, end_time, doctors(id, name, specialty)") | ||
| .eq("is_booked", false) | ||
| .gte("start_time", now) | ||
| .order("start_time"); | ||
|
|
||
| setAvailableSlots((slotsData as AvailableSlot[]) ?? []); | ||
| const available = (allSlots ?? []).filter( | ||
| (s) => !bookedSlotIds.includes(s.id) | ||
| ); | ||
| setAvailableSlots(available as AvailableSlot[]); |
There was a problem hiding this comment.
Fetching all booked appointments and all future slots to perform filtering on the client side is highly inefficient and will lead to performance degradation as the database grows. This logic should be offloaded to the database using a join or a subquery to fetch only available slots directly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/admin/dashboard/page.tsx`:
- Around line 224-226: The appointment counting currently compares names
(appointments.filter(a => a.doctors?.name === doc.name) and similarly for
patients) which breaks when names duplicate; instead update the appointments
query to include doctor_id and patient_id fields and change the filters to
compare IDs (e.g., a.doctor_id === doc.id and a.patient_id === pat.id) so use
the appointment record's doctor_id/patient_id for matching rather than
a.doctors?.name or a.patients?.name; apply the same fix to the other occurrence
around the 250–252 area.
- Around line 272-294: The slot-to-appointment matching is using
a.slots?.start_time === slot.start_time which can incorrectly match different
slots with the same start_time; instead ensure the Appointment objects include
slot_id (update the appointments query and Appointment type to select slot_id)
and change the matching in the slots.map callback to use appointments.find(a =>
a.slot_id === slot.id) (update any references to a.slots?.start_time
accordingly) so each slot is matched by its unique slot.id.
In `@app/admin/login/page.tsx`:
- Around line 29-40: The system_admins query result is not checking for errors;
update the supabase call in app/admin/login/page.tsx to destructure and check
the error (e.g., const { data: admin, error } = await
supabase.from("system_admins").select("id").eq("id",
authData.user.id).single()), and if error is present handle it separately (call
supabase.auth.signOut(), setError(error.message or a concise message),
setLoading(false) and return) before treating a null admin as "no admin
account"; keep the existing handling for the case where error is null but admin
is falsy.
In `@app/api/appointments/book/route.ts`:
- Around line 31-40: The duplicate-active-appointment check in route.ts
duplicates existing logic; replace the manual supabaseAdmin query and local
duplicate handling with the shared validation helper canBookWithDoctor from
lib/validation.ts: call canBookWithDoctor(user.id, doctorId) (or the helper's
exact signature) and use its result/throw to decide failure, removing the manual
.from("appointments") .select(...).maybeSingle() block and the duplicate
variable, and return the same 409 NextResponse when the helper indicates booking
is not allowed.
- Around line 22-29: The availability check uses .eq("status",
["active","done"]) which doesn't match multiple values; update the query in the
supabaseAdmin .from("appointments").select("id").eq("slot_id", slotId)... chain
to use .in("status", ["active","done"]) instead of .eq for status so rows with
status "active" or "done" are correctly detected (leave maybeSingle() and the
subsequent NextResponse.json({ error: "Slot is already booked" }, { status: 409
}) handling unchanged).
In `@app/api/appointments/cancel/route.ts`:
- Around line 36-41: Replace the inlined 1-hour check with the existing
canCancelAppointment helper: import canCancelAppointment from lib/validation.ts,
call canCancelAppointment(appointment) inside the isPatient && action ===
"cancel" branch, and if it returns false return the same NextResponse.json({
error: "Cannot cancel within 1 hour of appointment" }, { status: 409 }); remove
the duplicated new Date(...) logic so the route uses the shared validation
function.
- Around line 21-27: The Supabase appointment fetch currently only destructures
data and treats a missing appointment as a 404, which hides real query errors;
update the query to also capture the error (e.g., destructure error alongside
data from supabaseAdmin.from("appointments").select(...).eq(...).single()), then
if error is present return or throw a 500/appropriate response (and log the
error) and only return 404 when no error and no appointment is found; keep the
rest of the flow (variables appointment, appointmentId, slots selection)
unchanged.
In `@app/doctor/dashboard/page.tsx`:
- Around line 74-76: The isSlotBooked function currently only treats
appointments with status === "active" as occupying a slot, causing "done"
appointments to show as available; update isSlotBooked (and any related checks
using appointments.some) to consider both "active" and "done" as occupying
statuses (e.g., check a.status === "active" || a.status === "done") so the
doctor dashboard matches the booking API and patient dashboard behavior.
In `@app/patient/dashboard/page.tsx`:
- Around line 160-168: The styling logic for the action message currently
inspects actionMsg text (in the JSX that renders actionMsg) and can misclassify
errors that include success-like substrings; add an explicit boolean state
(e.g., actionSuccess) that you set based on the HTTP response (res.ok) in the
handlers that set actionMsg, then change the render condition to use
actionSuccess to choose the green vs red classes instead of parsing actionMsg;
update all places that call setActionMsg to also setActionSuccess so rendering
reflects the real success/error state.
In `@schema.sql`:
- Around line 37-40: Add a DB-level partial unique index to prevent multiple
patients from booking the same slot: create a unique index named
unique_active_slot on the appointments table over slot_id with a WHERE clause
limiting it to statuses 'active' and 'done' (so it enforces uniqueness for
occupied/finished slots); add this index in the schema/migration alongside the
existing unique_active_patient_doctor index and run the migration to apply it in
production.
- Around line 95-109: The SQL policy definitions
(admin_full_access_appointments, admin_read_doctors, admin_read_patients,
admin_read_slots) use inline WHERE clauses inside the EXISTS subqueries;
reformat each EXISTS (SELECT 1 FROM system_admins WHERE id = auth.uid()) so the
WHERE keyword is on its own new line (i.e., break before WHERE) to match the
project's SQL style conventions; leave admin_read_self as-is since it doesn't
use that pattern.
In `@seed.mjs`:
- Around line 67-82: The insert into supabase.from("slots") (the const { data:
slots, error: slotsError } = await
supabase.from("slots").insert([...]).select()) is using wrong column names
start_time and end_time; update each inserted object to use the schema-correct
column names start and end (keep doctor_id and the same timestamp values and
d1/d2/d3 references) so the insert matches the slots table definition.
In `@tests/integration.test.ts`:
- Around line 8-19: The getAvailableSlot helper currently queries takenSlots
from the appointments table without scoping to the doctor, which can wrongly
treat another doctor's booked slots as taken; update the appointments query used
to build takenIds (the takenSlots fetch in getAvailableSlot) to filter by
doctor_id (e.g., add an .eq("doctor_id", doctorId) predicate) so only
appointments for the specified doctor are considered when computing available
slots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50afa122-2b4b-431a-9c0e-d4052559645a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
app/admin/dashboard/page.tsxapp/admin/login/page.tsxapp/api/appointments/book/route.tsapp/api/appointments/cancel/route.tsapp/doctor/dashboard/page.tsxapp/page.tsxapp/patient/dashboard/page.tsxjest.config.jsjest.setup.jslib/validation.tspackage.jsonschema.sqlseed.mjstests/integration.test.tstests/unit.test.ts
Comment on lines
+224
to
+226
| <td className="px-4 py-3"> | ||
| {appointments.filter(a => a.doctors?.name === doc.name).length} | ||
| </td> |
There was a problem hiding this comment.
Name-based appointment counting is fragile—use IDs instead.
Filtering by a.doctors?.name === doc.name and a.patients?.name === pat.name may produce incorrect counts if names are duplicated. Include doctor_id/patient_id in the appointments query and match by ID.
Also applies to: 250-252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/admin/dashboard/page.tsx` around lines 224 - 226, The appointment
counting currently compares names (appointments.filter(a => a.doctors?.name ===
doc.name) and similarly for patients) which breaks when names duplicate; instead
update the appointments query to include doctor_id and patient_id fields and
change the filters to compare IDs (e.g., a.doctor_id === doc.id and a.patient_id
=== pat.id) so use the appointment record's doctor_id/patient_id for matching
rather than a.doctors?.name or a.patients?.name; apply the same fix to the other
occurrence around the 250–252 area.
Comment on lines
+272
to
+294
| {slots.map((slot) => { | ||
| const appt = appointments.find(a => a.slots?.start_time === slot.start_time); | ||
| const status = appt ? appt.status : "available"; | ||
| return ( | ||
| <tr key={slot.id}> | ||
| <td className="px-4 py-3">{slot.doctors?.name}</td> | ||
| <td className="px-4 py-3"> | ||
| {formatDateTime(slot.start_time)} —{" "} | ||
| {new Date(slot.end_time).toLocaleTimeString("en-IN", { timeStyle: "short" })} | ||
| </td> | ||
| <td className="px-4 py-3"> | ||
| <span className={`rounded-full px-2 py-0.5 text-xs font-medium ${ | ||
| status === "active" ? "bg-blue-100 text-blue-700" | ||
| : status === "done" ? "bg-green-100 text-green-700" | ||
| : status === "cancelled" ? "bg-gray-100 text-gray-600" | ||
| : "bg-green-50 text-green-600" | ||
| }`}> | ||
| {status === "available" ? "Available" : status} | ||
| </span> | ||
| </td> | ||
| </tr> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
Incorrect slot-to-appointment matching by start_time instead of slot_id.
Matching appointments to slots using a.slots?.start_time === slot.start_time is unreliable—multiple slots across different doctors could share the same start time. Use slot_id for accurate matching.
🐛 Proposed fix
First, update the appointments query to include slot_id:
supabase.from("appointments")
- .select("id, status, created_at, patients(name, email), doctors(name, specialty), slots(start_time, end_time)")
+ .select("id, status, created_at, slot_id, patients(name, email), doctors(name, specialty), slots(start_time, end_time)")Update the Appointment type:
type Appointment = {
id: string;
status: "active" | "done" | "cancelled";
created_at: string;
+ slot_id: string;
patients: { name: string; email: string };
// ...
};Then fix the matching logic:
-const appt = appointments.find(a => a.slots?.start_time === slot.start_time);
+const appt = appointments.find(a => a.slot_id === slot.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/admin/dashboard/page.tsx` around lines 272 - 294, The slot-to-appointment
matching is using a.slots?.start_time === slot.start_time which can incorrectly
match different slots with the same start_time; instead ensure the Appointment
objects include slot_id (update the appointments query and Appointment type to
select slot_id) and change the matching in the slots.map callback to use
appointments.find(a => a.slot_id === slot.id) (update any references to
a.slots?.start_time accordingly) so each slot is matched by its unique slot.id.
Comment on lines
+29
to
+40
| const { data: admin } = await supabase | ||
| .from("system_admins") | ||
| .select("id") | ||
| .eq("id", authData.user.id) | ||
| .single(); | ||
|
|
||
| if (!admin) { | ||
| await supabase.auth.signOut(); | ||
| setError("No admin account found for this email."); | ||
| setLoading(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Handle potential error from the system_admins query.
The query to check if the user is in system_admins ignores potential errors. If the query fails (e.g., network issue, RLS policy block), admin will be null and the user will be signed out with a misleading "No admin account found" error.
🛡️ Proposed fix to handle query errors
- const { data: admin } = await supabase
+ const { data: admin, error: adminError } = await supabase
.from("system_admins")
.select("id")
.eq("id", authData.user.id)
.single();
+ if (adminError && adminError.code !== "PGRST116") {
+ // PGRST116 = no rows returned (not an admin)
+ await supabase.auth.signOut();
+ setError("Failed to verify admin status. Please try again.");
+ setLoading(false);
+ return;
+ }
+
if (!admin) {📝 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.
Suggested change
| const { data: admin } = await supabase | |
| .from("system_admins") | |
| .select("id") | |
| .eq("id", authData.user.id) | |
| .single(); | |
| if (!admin) { | |
| await supabase.auth.signOut(); | |
| setError("No admin account found for this email."); | |
| setLoading(false); | |
| return; | |
| } | |
| const { data: admin, error: adminError } = await supabase | |
| .from("system_admins") | |
| .select("id") | |
| .eq("id", authData.user.id) | |
| .single(); | |
| if (adminError && adminError.code !== "PGRST116") { | |
| // PGRST116 = no rows returned (not an admin) | |
| await supabase.auth.signOut(); | |
| setError("Failed to verify admin status. Please try again."); | |
| setLoading(false); | |
| return; | |
| } | |
| if (!admin) { | |
| await supabase.auth.signOut(); | |
| setError("No admin account found for this email."); | |
| setLoading(false); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/admin/login/page.tsx` around lines 29 - 40, The system_admins query
result is not checking for errors; update the supabase call in
app/admin/login/page.tsx to destructure and check the error (e.g., const { data:
admin, error } = await supabase.from("system_admins").select("id").eq("id",
authData.user.id).single()), and if error is present handle it separately (call
supabase.auth.signOut(), setError(error.message or a concise message),
setLoading(false) and return) before treating a null admin as "no admin
account"; keep the existing handling for the case where error is null but admin
is falsy.
Comment on lines
+31
to
+40
| // Check patient doesn't already have active appointment with this doctor | ||
| const { data: duplicate } = await supabaseAdmin | ||
| .from("appointments") | ||
| .select("id") | ||
| .eq("patient_id", user.id) | ||
| .eq("doctor_id", doctorId) | ||
| .eq("status", "active") | ||
| .maybeSingle(); | ||
|
|
||
| if (duplicate) return NextResponse.json({ error: "You already have an active appointment with this doctor" }, { status: 409 }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using the existing canBookWithDoctor validation helper.
The duplicate appointment check duplicates logic already implemented in lib/validation.ts:canBookWithDoctor(). Reusing that helper would improve maintainability and ensure consistent validation across the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/appointments/book/route.ts` around lines 31 - 40, The
duplicate-active-appointment check in route.ts duplicates existing logic;
replace the manual supabaseAdmin query and local duplicate handling with the
shared validation helper canBookWithDoctor from lib/validation.ts: call
canBookWithDoctor(user.id, doctorId) (or the helper's exact signature) and use
its result/throw to decide failure, removing the manual .from("appointments")
.select(...).maybeSingle() block and the duplicate variable, and return the same
409 NextResponse when the helper indicates booking is not allowed.
Comment on lines
160
to
168
| {actionMsg && ( | ||
| <p className="mb-4 rounded-lg bg-green-50 px-4 py-2 text-sm text-green-700"> | ||
| <p className={`mb-4 rounded-lg px-4 py-2 text-sm ${ | ||
| actionMsg.includes("success") || actionMsg.includes("booked") || actionMsg.includes("cancelled") | ||
| ? "bg-green-50 text-green-700" | ||
| : "bg-red-50 text-red-700" | ||
| }`}> | ||
| {actionMsg} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
Message styling may incorrectly show green for error messages containing certain keywords.
The condition checks for "cancelled" substring, but an error message like "Cannot cancel - already cancelled" would be styled green when it should be red. Consider checking res.ok status instead of parsing message content.
💡 Alternative approach
Track success/error state explicitly rather than parsing message content:
+ const [isSuccess, setIsSuccess] = useState(false);
// In handleBook:
+ setIsSuccess(res.ok);
// In render:
- actionMsg.includes("success") || actionMsg.includes("booked") || actionMsg.includes("cancelled")
+ isSuccess🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/patient/dashboard/page.tsx` around lines 160 - 168, The styling logic for
the action message currently inspects actionMsg text (in the JSX that renders
actionMsg) and can misclassify errors that include success-like substrings; add
an explicit boolean state (e.g., actionSuccess) that you set based on the HTTP
response (res.ok) in the handlers that set actionMsg, then change the render
condition to use actionSuccess to choose the green vs red classes instead of
parsing actionMsg; update all places that call setActionMsg to also
setActionSuccess so rendering reflects the real success/error state.
Comment on lines
67
to
82
| const { data: slots, error: slotsError } = await supabase.from("slots").insert([ | ||
| { doctor_id: d1, start_time: "2026-04-28 09:00:00+00", end_time: "2026-04-28 10:00:00+00", is_booked: true }, | ||
| { doctor_id: d1, start_time: "2026-04-29 10:00:00+00", end_time: "2026-04-29 11:00:00+00", is_booked: false }, | ||
| { doctor_id: d1, start_time: "2026-05-04 09:00:00+00", end_time: "2026-05-04 10:00:00+00", is_booked: false }, | ||
| { doctor_id: d1, start_time: "2026-05-06 11:00:00+00", end_time: "2026-05-06 12:00:00+00", is_booked: false }, | ||
| { doctor_id: d2, start_time: "2026-04-28 11:00:00+00", end_time: "2026-04-28 12:00:00+00", is_booked: false }, | ||
| { doctor_id: d2, start_time: "2026-04-30 09:00:00+00", end_time: "2026-04-30 10:00:00+00", is_booked: false }, | ||
| { doctor_id: d2, start_time: "2026-05-05 10:00:00+00", end_time: "2026-05-05 11:00:00+00", is_booked: false }, | ||
| { doctor_id: d2, start_time: "2026-05-07 09:00:00+00", end_time: "2026-05-07 10:00:00+00", is_booked: false }, | ||
| { doctor_id: d3, start_time: "2026-04-29 09:00:00+00", end_time: "2026-04-29 10:00:00+00", is_booked: false }, | ||
| { doctor_id: d3, start_time: "2026-04-30 11:00:00+00", end_time: "2026-04-30 12:00:00+00", is_booked: false }, | ||
| { doctor_id: d3, start_time: "2026-05-04 11:00:00+00", end_time: "2026-05-04 12:00:00+00", is_booked: false }, | ||
| { doctor_id: d3, start_time: "2026-05-07 10:00:00+00", end_time: "2026-05-07 11:00:00+00", is_booked: false }, | ||
| { doctor_id: d1, start_time: "2026-04-28 09:00:00+00", end_time: "2026-04-28 10:00:00+00" }, | ||
| { doctor_id: d1, start_time: "2026-04-29 10:00:00+00", end_time: "2026-04-29 11:00:00+00" }, | ||
| { doctor_id: d1, start_time: "2026-05-04 09:00:00+00", end_time: "2026-05-04 10:00:00+00" }, | ||
| { doctor_id: d1, start_time: "2026-05-06 11:00:00+00", end_time: "2026-05-06 12:00:00+00" }, | ||
|
|
||
| { doctor_id: d2, start_time: "2026-04-28 11:00:00+00", end_time: "2026-04-28 12:00:00+00" }, | ||
| { doctor_id: d2, start_time: "2026-04-30 09:00:00+00", end_time: "2026-04-30 10:00:00+00" }, | ||
| { doctor_id: d2, start_time: "2026-05-05 10:00:00+00", end_time: "2026-05-05 11:00:00+00" }, | ||
| { doctor_id: d2, start_time: "2026-05-07 09:00:00+00", end_time: "2026-05-07 10:00:00+00" }, | ||
|
|
||
| { doctor_id: d3, start_time: "2026-04-29 09:00:00+00", end_time: "2026-04-29 10:00:00+00" }, | ||
| { doctor_id: d3, start_time: "2026-04-30 11:00:00+00", end_time: "2026-04-30 12:00:00+00" }, | ||
| { doctor_id: d3, start_time: "2026-05-04 11:00:00+00", end_time: "2026-05-04 12:00:00+00" }, | ||
| { doctor_id: d3, start_time: "2026-05-07 10:00:00+00", end_time: "2026-05-07 11:00:00+00" }, | ||
| ]).select(); |
There was a problem hiding this comment.
Column names do not match the schema definition.
The seed script uses start_time and end_time column names, but according to the schema (schema.sql), the slots table defines columns as start and end. This mismatch will cause the insert to fail with a column-not-found error.
🐛 Proposed fix to use correct column names
const { data: slots, error: slotsError } = await supabase.from("slots").insert([
- { doctor_id: d1, start_time: "2026-04-28 09:00:00+00", end_time: "2026-04-28 10:00:00+00" },
- { doctor_id: d1, start_time: "2026-04-29 10:00:00+00", end_time: "2026-04-29 11:00:00+00" },
- { doctor_id: d1, start_time: "2026-05-04 09:00:00+00", end_time: "2026-05-04 10:00:00+00" },
- { doctor_id: d1, start_time: "2026-05-06 11:00:00+00", end_time: "2026-05-06 12:00:00+00" },
+ { doctor_id: d1, start: "2026-04-28 09:00:00+00", end: "2026-04-28 10:00:00+00" },
+ { doctor_id: d1, start: "2026-04-29 10:00:00+00", end: "2026-04-29 11:00:00+00" },
+ { doctor_id: d1, start: "2026-05-04 09:00:00+00", end: "2026-05-04 10:00:00+00" },
+ { doctor_id: d1, start: "2026-05-06 11:00:00+00", end: "2026-05-06 12:00:00+00" },
- { doctor_id: d2, start_time: "2026-04-28 11:00:00+00", end_time: "2026-04-28 12:00:00+00" },
- { doctor_id: d2, start_time: "2026-04-30 09:00:00+00", end_time: "2026-04-30 10:00:00+00" },
- { doctor_id: d2, start_time: "2026-05-05 10:00:00+00", end_time: "2026-05-05 11:00:00+00" },
- { doctor_id: d2, start_time: "2026-05-07 09:00:00+00", end_time: "2026-05-07 10:00:00+00" },
+ { doctor_id: d2, start: "2026-04-28 11:00:00+00", end: "2026-04-28 12:00:00+00" },
+ { doctor_id: d2, start: "2026-04-30 09:00:00+00", end: "2026-04-30 10:00:00+00" },
+ { doctor_id: d2, start: "2026-05-05 10:00:00+00", end: "2026-05-05 11:00:00+00" },
+ { doctor_id: d2, start: "2026-05-07 09:00:00+00", end: "2026-05-07 10:00:00+00" },
- { doctor_id: d3, start_time: "2026-04-29 09:00:00+00", end_time: "2026-04-29 10:00:00+00" },
- { doctor_id: d3, start_time: "2026-04-30 11:00:00+00", end_time: "2026-04-30 12:00:00+00" },
- { doctor_id: d3, start_time: "2026-05-04 11:00:00+00", end_time: "2026-05-04 12:00:00+00" },
- { doctor_id: d3, start_time: "2026-05-07 10:00:00+00", end_time: "2026-05-07 11:00:00+00" },
+ { doctor_id: d3, start: "2026-04-29 09:00:00+00", end: "2026-04-29 10:00:00+00" },
+ { doctor_id: d3, start: "2026-04-30 11:00:00+00", end: "2026-04-30 12:00:00+00" },
+ { doctor_id: d3, start: "2026-05-04 11:00:00+00", end: "2026-05-04 12:00:00+00" },
+ { doctor_id: d3, start: "2026-05-07 10:00:00+00", end: "2026-05-07 11:00:00+00" },
]).select();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@seed.mjs` around lines 67 - 82, The insert into supabase.from("slots") (the
const { data: slots, error: slotsError } = await
supabase.from("slots").insert([...]).select()) is using wrong column names
start_time and end_time; update each inserted object to use the schema-correct
column names start and end (keep doctor_id and the same timestamp values and
d1/d2/d3 references) so the insert matches the slots table definition.
Comment on lines
+8
to
+19
| async function getAvailableSlot(doctorId: string) { | ||
| const { data: allSlots } = await supabase | ||
| .from("slots").select("id").eq("doctor_id", doctorId); | ||
|
|
||
| const { data: takenSlots } = await supabase | ||
| .from("appointments").select("slot_id") | ||
| .in("status", ["active", "done"]); | ||
|
|
||
| const takenIds = new Set((takenSlots ?? []).map(a => a.slot_id)); | ||
| const available = (allSlots ?? []).find(s => !takenIds.has(s.id)); | ||
| return available?.id ?? null; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider filtering appointments by doctor_id in getAvailableSlot.
The takenSlots query fetches all appointments with status active or done across all doctors, not just the specified doctor. While this won't cause false positives (a slot won't be incorrectly marked as available), it may unnecessarily mark slots as taken when they belong to a different doctor, potentially causing the test to fail if all of one doctor's slots happen to share IDs with other doctors' booked slots.
♻️ Proposed refinement to filter by doctor_id
async function getAvailableSlot(doctorId: string) {
const { data: allSlots } = await supabase
.from("slots").select("id").eq("doctor_id", doctorId);
const { data: takenSlots } = await supabase
.from("appointments").select("slot_id")
- .in("status", ["active", "done"]);
+ .in("status", ["active", "done"])
+ .in("slot_id", (allSlots ?? []).map(s => s.id));
const takenIds = new Set((takenSlots ?? []).map(a => a.slot_id));
const available = (allSlots ?? []).find(s => !takenIds.has(s.id));
return available?.id ?? null;
}📝 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.
Suggested change
| async function getAvailableSlot(doctorId: string) { | |
| const { data: allSlots } = await supabase | |
| .from("slots").select("id").eq("doctor_id", doctorId); | |
| const { data: takenSlots } = await supabase | |
| .from("appointments").select("slot_id") | |
| .in("status", ["active", "done"]); | |
| const takenIds = new Set((takenSlots ?? []).map(a => a.slot_id)); | |
| const available = (allSlots ?? []).find(s => !takenIds.has(s.id)); | |
| return available?.id ?? null; | |
| } | |
| async function getAvailableSlot(doctorId: string) { | |
| const { data: allSlots } = await supabase | |
| .from("slots").select("id").eq("doctor_id", doctorId); | |
| const { data: takenSlots } = await supabase | |
| .from("appointments").select("slot_id") | |
| .in("status", ["active", "done"]) | |
| .in("slot_id", (allSlots ?? []).map(s => s.id)); | |
| const takenIds = new Set((takenSlots ?? []).map(a => a.slot_id)); | |
| const available = (allSlots ?? []).find(s => !takenIds.has(s.id)); | |
| return available?.id ?? null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration.test.ts` around lines 8 - 19, The getAvailableSlot helper
currently queries takenSlots from the appointments table without scoping to the
doctor, which can wrongly treat another doctor's booked slots as taken; update
the appointments query used to build takenIds (the takenSlots fetch in
getAvailableSlot) to filter by doctor_id (e.g., add an .eq("doctor_id",
doctorId) predicate) so only appointments for the specified doctor are
considered when computing available slots.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
app/patient/dashboard/page.tsx (1)
173-178:⚠️ Potential issue | 🟡 MinorUse request outcome, not message text, to choose the alert color.
"Slot already booked"and similar conflict messages still matchincludes("booked"), so they render as success. This was already flagged and is still present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/patient/dashboard/page.tsx` around lines 173 - 178, The alert color is being derived from the text in actionMsg which causes conflict messages (e.g., "Slot already booked") to appear as success; change the component to use an explicit outcome/status boolean or enum instead of parsing actionMsg: introduce or use a status variable (e.g., actionStatus, actionOutcome, isSuccess) that your request handler sets on success/error, then replace the class conditional that currently checks actionMsg.includes(...) with a direct check of that status (e.g., actionStatus === "success" or isSuccess === true) and leave actionMsg solely for the displayed message; update any related handler that sets actionMsg to also set the corresponding actionStatus.app/admin/dashboard/page.tsx (1)
7-14:⚠️ Potential issue | 🔴 CriticalSelect and type the foreign keys before the tabs render against them.
Line 65 never selects
doctor_id,patient_id, orslot_id, andAppointmenton Lines 7-14 does not declare them. That leaves the doctor counts and slot status lookup wrong at runtime, and the patient tab currently breaks on Line 251 becauseappointments.filter(a.patient === pat.id)is not a valid predicate.🐛 Proposed fix
type Appointment = { id: string; status: "active" | "done" | "cancelled"; created_at: string; + doctor_id: string; + patient_id: string; + slot_id: string; patients: { name: string; email: string }; doctors: { name: string; specialty: string }; slots: { start_time: string; end_time: string }; }; @@ const [apptRes, doctorRes, patientRes, slotRes] = await Promise.all([ supabase.from("appointments") - .select("id, status, created_at, patients(name, email), doctors(name, specialty), slots(start_time, end_time)") + .select("id, status, created_at, doctor_id, patient_id, slot_id, patients(name, email), doctors(name, specialty), slots(start_time, end_time)") .order("created_at", { ascending: false }), @@ <td className="px-4 py-3"> - {appointments.filter(a => a.doctor_id === doc.id).length} + {appointments.filter((a) => a.doctor_id === doc.id).length} </td> @@ <td className="px-4 py-3"> - {appointments.filter(a.patient === pat.id).length} + {appointments.filter((a) => a.patient_id === pat.id).length} </td> @@ {slots.map((slot) => { - const appt = appointments.find(a => a.slot_id === slot.id); + const appt = appointments.find((a) => a.slot_id === slot.id); const status = appt ? appt.status : "available";Quick verification: inspect the selected appointment shape and the downstream usages. Expected result: the query and type both include
doctor_id,patient_id, andslot_id, and everyfilter(...)receives a callback.#!/bin/bash rg -n 'type Appointment|select\("id, status, created_at|doctor_id|patient_id|slot_id|filter\(a\.patient' app/admin/dashboard/page.tsxAlso applies to: 64-66, 224-225, 250-251, 272-274
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/admin/dashboard/page.tsx` around lines 7 - 14, The Appointment type and the DB selection are missing foreign keys (doctor_id, patient_id, slot_id) and some filters use an invalid predicate; update the Appointment type to include doctor_id, patient_id, and slot_id, update the select(...) that currently selects "id, status, created_at" to also select "doctor_id, patient_id, slot_id", and fix all filters to pass a callback (e.g., change appointments.filter(a.patient === pat.id) to appointments.filter(a => a.patient_id === pat.id)); also update any downstream lookups that reference a.patient/a.doctor/a.slot to use a.patient_id/a.doctor_id/a.slot_id accordingly (symbols: Appointment, select(...), appointments.filter, and any slot/doctor lookup code that uses a.slot or a.doctor).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/appointments/book/route.ts`:
- Around line 31-38: The duplicate-check queries the appointments table using
slot.doctor_id before slot is fetched, causing a TDZ/runtime error and the
subsequent insert uses the wrong property name; move the slot fetch (the code
that assigns slot from supabaseAdmin / .from("slots") / .select(...)) to run
before the duplicate check, then update the insert into "appointments" to use
slot.doctor_id (snake_case) instead of slot.doctorId so the correct doctor_id
value is stored; ensure any references to slot in the duplicate-check and insert
use the fetched slot variable and its snake_case fields.
In `@app/api/appointments/cancel/route.ts`:
- Around line 32-45: The current Supabase query uses .single() which causes
missing appointments to surface as an error (preventing the 404 path); change
the query chain on supabaseAdmin.from("appointments").select(...).eq("id",
appointmentId).single() to use .maybeSingle() so data will be null instead of an
error when no row matches, then keep the existing null check of appointment
(AppointmentWithSlot | null) to return the 404; ensure existing error handling
still returns 500 only for real query errors (i.e., when error is set and data
is undefined).
- Around line 26-31: The action allowlist only accepts "done" and "cancelled"
but dashboards send "cancel", and newStatus is declared twice causing a compile
error; update validation to accept "cancel" (or normalize "cancel" to
"cancelled") when reading const { appointmentId, action } = await req.json(),
set a single normalized newStatus variable (e.g., map "cancel" -> "cancelled")
immediately after validation, and remove the later redeclaration of newStatus so
there's only one declaration in the scope; ensure NextResponse.json error
branches remain unchanged and that the patient-specific checks that depend on
newStatus use the normalized variable.
In `@app/patient/dashboard/page.tsx`:
- Around line 57-63: The client-side supabase query in page.tsx that builds
bookedSlotIds (the bookedAppointments query and the bookedSlotIds mapping) is
blocked by RLS and only returns the current user's appointments; move this
availability calculation behind a trusted server boundary by adding a server RPC
or API endpoint that returns booked or free future slot IDs (e.g., create a
Postgres view/RPC like get_booked_future_slots or a server route
/api/slots/availability that queries appointments without user-level RLS using
the service_role key), then replace the client-side supabase
.from("appointments").select(...) logic in page.tsx (the
bookedAppointments/bookedSlotIds usage and the similar block at lines 68-81)
with a fetch to that endpoint and consume the returned free/booked slot IDs.
Ensure the server RPC or endpoint only exposes the minimal data (slot IDs and
future timestamp) and enforces any necessary filters (future slots, status in
["active","done"]).
In `@schema.sql`:
- Around line 97-107: The current row-level policies
appointments_insert_patient, appointments_update_patient, and
appointments_update_doctor on the appointments table allow authenticated clients
to bypass API-level business rules; remove or tighten these policies so direct
client INSERT/UPDATE is disallowed and enforce business logic via RPCs or
stricter policies. Specifically, delete or revoke open INSERT/UPDATE policies
for patients and doctors, create a single granular policy that only allows
selects for authenticated users, and implement Postgres functions/RPCs (e.g.,
book_appointment(), cancel_appointment(), mark_appointment_done()) that perform
the future-slot, 1-hour cancellation, and status-transition checks server-side,
then grant execute on those RPCs instead of row write privileges. Ensure
appointments_insert_patient, appointments_update_patient, and
appointments_update_doctor are replaced or disabled and that only the RPCs (and
any minimal read-only policies) are permitted for client sessions.
---
Duplicate comments:
In `@app/admin/dashboard/page.tsx`:
- Around line 7-14: The Appointment type and the DB selection are missing
foreign keys (doctor_id, patient_id, slot_id) and some filters use an invalid
predicate; update the Appointment type to include doctor_id, patient_id, and
slot_id, update the select(...) that currently selects "id, status, created_at"
to also select "doctor_id, patient_id, slot_id", and fix all filters to pass a
callback (e.g., change appointments.filter(a.patient === pat.id) to
appointments.filter(a => a.patient_id === pat.id)); also update any downstream
lookups that reference a.patient/a.doctor/a.slot to use
a.patient_id/a.doctor_id/a.slot_id accordingly (symbols: Appointment,
select(...), appointments.filter, and any slot/doctor lookup code that uses
a.slot or a.doctor).
In `@app/patient/dashboard/page.tsx`:
- Around line 173-178: The alert color is being derived from the text in
actionMsg which causes conflict messages (e.g., "Slot already booked") to appear
as success; change the component to use an explicit outcome/status boolean or
enum instead of parsing actionMsg: introduce or use a status variable (e.g.,
actionStatus, actionOutcome, isSuccess) that your request handler sets on
success/error, then replace the class conditional that currently checks
actionMsg.includes(...) with a direct check of that status (e.g., actionStatus
=== "success" or isSuccess === true) and leave actionMsg solely for the
displayed message; update any related handler that sets actionMsg to also set
the corresponding actionStatus.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d0a0ee5c-0191-48e8-8e06-3f40f17ce5db
📒 Files selected for processing (5)
app/admin/dashboard/page.tsxapp/api/appointments/book/route.tsapp/api/appointments/cancel/route.tsapp/patient/dashboard/page.tsxschema.sql
Comment on lines
+31
to
+38
| // Check patient doesn't already have active appointment with this doctor | ||
| const { data: duplicate } = await supabaseAdmin | ||
| .from("appointments") | ||
| .select("id") | ||
| .eq("patient_id", user.id) | ||
| .eq("doctor_id", slot.doctor_id) | ||
| .eq("status", "active") | ||
| .maybeSingle(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '20,70p' app/api/appointments/book/route.tsRepository: botcode-com/task
Length of output: 1825
Reorder the slot lookup before the duplicate check, and fix the property-name mismatch in the insert.
Line 36 references slot.doctor_id before slot is declared on Line 44 — this violates JavaScript's temporal dead zone and would cause a runtime error. Additionally, Line 61 inserts slot.doctorId (camelCase), but the database returns doctor_id (snake_case), resulting in doctor_id: undefined being written to the appointments table.
Move the slot fetch (lines 44–48) before the duplicate check (lines 31–38) and use slot.doctor_id instead of slot.doctorId in the insert statement.
Fix
+ const { data: slot } = await supabaseAdmin
+ .from("slots")
+ .select("start_time, doctor_id")
+ .eq("id", slotId)
+ .single();
+
+ if (!slot) {
+ return NextResponse.json({ error: "Slot not found" }, { status: 404 });
+ }
+
// Check slot exists and is not already booked
const { data: existingAppt } = await supabaseAdmin
.from("appointments")
.select("id")
.eq("slot_id", slotId)
.in("status", ["active", "done"])
.maybeSingle();
if (existingAppt) return NextResponse.json({ error: "Slot is already booked" }, { status: 409 });
// Check patient doesn't already have active appointment with this doctor
const { data: duplicate } = await supabaseAdmin
.from("appointments")
.select("id")
.eq("patient_id", user.id)
.eq("doctor_id", slot.doctor_id)
.eq("status", "active")
.maybeSingle();
if (duplicate) return NextResponse.json({ error: "You already have an active appointment with this doctor" }, { status: 409 });
const now = new Date().toISOString();
- const { data: slot } = await supabaseAdmin
- .from("slots")
- .select("start_time, doctor_id")
- .eq("id", slotId)
- .single();
-
- if (!slot) {
- return NextResponse.json({ error: "Slot not found" }, { status: 404 });
- }
-
if (new Date(slot.start_time) < new Date()) {
return NextResponse.json({ error: "Cannot book past slots" }, { status: 400 });
}
// Book it
const { error: insertError } = await supabaseAdmin
.from("appointments")
- .insert({ patient_id: user.id, doctor_id: slot.doctorId, slot_id: slotId, status: "active" });
+ .insert({ patient_id: user.id, doctor_id: slot.doctor_id, slot_id: slotId, status: "active" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/appointments/book/route.ts` around lines 31 - 38, The duplicate-check
queries the appointments table using slot.doctor_id before slot is fetched,
causing a TDZ/runtime error and the subsequent insert uses the wrong property
name; move the slot fetch (the code that assigns slot from supabaseAdmin /
.from("slots") / .select(...)) to run before the duplicate check, then update
the insert into "appointments" to use slot.doctor_id (snake_case) instead of
slot.doctorId so the correct doctor_id value is stored; ensure any references to
slot in the duplicate-check and insert use the fetched slot variable and its
snake_case fields.
Comment on lines
+26
to
+31
| const { appointmentId, action } = await req.json(); | ||
| if (!appointmentId || !action) return NextResponse.json({ error: "Missing fields" }, { status: 400 }); | ||
| if (!["done", "cancelled"].includes(action)) return NextResponse.json({ error: "Invalid action" }, { status: 400 }); | ||
|
|
||
| const newStatus = action; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'action: "cancel"|const newStatus|includes\(action\)' app/patient/dashboard/page.tsx app/doctor/dashboard/page.tsx app/api/appointments/cancel/route.tsRepository: botcode-com/task
Length of output: 495
🏁 Script executed:
rg -n 'action: "' app/doctor/dashboard/page.tsxRepository: botcode-com/task
Length of output: 145
🏁 Script executed:
rg -n 'handleAction\(' app/doctor/dashboard/page.tsx -A 2Repository: botcode-com/task
Length of output: 729
Accept "cancel" in the action allowlist and normalize it once.
Both dashboards send action: "cancel", but Line 28 only accepts "done" and "cancelled". This causes every cancel request to fail with 400 and leaves the patient-specific checks on Lines 60-67 dead code. Additionally, newStatus is declared twice in the same scope (Lines 30 and 77), causing a compile error.
Fix
const { appointmentId, action } = await req.json();
if (!appointmentId || !action) return NextResponse.json({ error: "Missing fields" }, { status: 400 });
- if (!["done", "cancelled"].includes(action)) return NextResponse.json({ error: "Invalid action" }, { status: 400 });
-
- const newStatus = action;
+ if (!["done", "cancel"].includes(action)) return NextResponse.json({ error: "Invalid action" }, { status: 400 });
+ const newStatus = action === "done" ? "done" : "cancelled";Then remove the redeclaration at Line 77 (the second const newStatus = assignment).
📝 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.
Suggested change
| const { appointmentId, action } = await req.json(); | |
| if (!appointmentId || !action) return NextResponse.json({ error: "Missing fields" }, { status: 400 }); | |
| if (!["done", "cancelled"].includes(action)) return NextResponse.json({ error: "Invalid action" }, { status: 400 }); | |
| const newStatus = action; | |
| const { appointmentId, action } = await req.json(); | |
| if (!appointmentId || !action) return NextResponse.json({ error: "Missing fields" }, { status: 400 }); | |
| if (!["done", "cancel"].includes(action)) return NextResponse.json({ error: "Invalid action" }, { status: 400 }); | |
| const newStatus = action === "done" ? "done" : "cancelled"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/appointments/cancel/route.ts` around lines 26 - 31, The action
allowlist only accepts "done" and "cancelled" but dashboards send "cancel", and
newStatus is declared twice causing a compile error; update validation to accept
"cancel" (or normalize "cancel" to "cancelled") when reading const {
appointmentId, action } = await req.json(), set a single normalized newStatus
variable (e.g., map "cancel" -> "cancelled") immediately after validation, and
remove the later redeclaration of newStatus so there's only one declaration in
the scope; ensure NextResponse.json error branches remain unchanged and that the
patient-specific checks that depend on newStatus use the normalized variable.
Comment on lines
+32
to
+45
| const { data, error } = await supabaseAdmin | ||
| .from("appointments") | ||
| .select("id, status, patient_id, doctor_id, slot_id, slots(start_time)") | ||
| .eq("id", appointmentId) | ||
| .single(); | ||
|
|
||
| if (error) { | ||
| return NextResponse.json({ error: "Failed to fetch appointment" }, { status: 500 }); | ||
| } | ||
|
|
||
| const appointment = data as AppointmentWithSlot | null; | ||
|
|
||
| if (!appointment) return NextResponse.json({ error: "Appointment not found" }, { status: 404 }); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @supabase/supabase-jsv2, what does.single()return when zero rows match, and when should.maybeSingle() be used instead?
💡 Result:
In @supabase/supabase-js v2, .single returns an error (typically status 406 with message "JSON object requested, multiple (or no) rows returned") when zero rows match the query. It expects exactly one row and throws an error for zero or multiple rows. Use .maybeSingle instead when the query might return zero or one row (e.g., lookups where the record may not exist). It returns data: null and error: null for zero rows, or the single object for one row, but errors if more than one row is found.
Citations:
- 1: https://supabase.com/docs/reference/javascript/maybesingle
- 2: https://supabase.com/docs/reference/javascript/single
- 3: https://www.hemantasundaray.com/blog/supabase-single-maybesingle#:~:text=single() when your logic,expect zero or one row).
.single() makes the 404 branch unreachable.
When appointmentId doesn't exist, .single() returns an error (Supabase returns status 406 when zero rows match), which is caught and returns a 500 response. The 404 check on line 44 never executes for missing records.
Use .maybeSingle() instead. It returns data: null with no error when zero rows match, allowing the downstream null check to return 404 correctly.
Suggested fix
const { data, error } = await supabaseAdmin
.from("appointments")
.select("id, status, patient_id, doctor_id, slot_id, slots(start_time)")
.eq("id", appointmentId)
- .single();
+ .maybeSingle();
if (error) {
return NextResponse.json({ error: "Failed to fetch appointment" }, { status: 500 });
}
const appointment = data as AppointmentWithSlot | null;
if (!appointment) return NextResponse.json({ error: "Appointment not found" }, { status: 404 });📝 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.
Suggested change
| const { data, error } = await supabaseAdmin | |
| .from("appointments") | |
| .select("id, status, patient_id, doctor_id, slot_id, slots(start_time)") | |
| .eq("id", appointmentId) | |
| .single(); | |
| if (error) { | |
| return NextResponse.json({ error: "Failed to fetch appointment" }, { status: 500 }); | |
| } | |
| const appointment = data as AppointmentWithSlot | null; | |
| if (!appointment) return NextResponse.json({ error: "Appointment not found" }, { status: 404 }); | |
| const { data, error } = await supabaseAdmin | |
| .from("appointments") | |
| .select("id, status, patient_id, doctor_id, slot_id, slots(start_time)") | |
| .eq("id", appointmentId) | |
| .maybeSingle(); | |
| if (error) { | |
| return NextResponse.json({ error: "Failed to fetch appointment" }, { status: 500 }); | |
| } | |
| const appointment = data as AppointmentWithSlot | null; | |
| if (!appointment) return NextResponse.json({ error: "Appointment not found" }, { status: 404 }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/appointments/cancel/route.ts` around lines 32 - 45, The current
Supabase query uses .single() which causes missing appointments to surface as an
error (preventing the 404 path); change the query chain on
supabaseAdmin.from("appointments").select(...).eq("id", appointmentId).single()
to use .maybeSingle() so data will be null instead of an error when no row
matches, then keep the existing null check of appointment (AppointmentWithSlot |
null) to return the 404; ensure existing error handling still returns 500 only
for real query errors (i.e., when error is set and data is undefined).
Comment on lines
+57
to
+63
| // Get all booked slot IDs from active appointments | ||
| const { data: bookedAppointments } = await supabase | ||
| .from("appointments") | ||
| .select("slot_id") | ||
| .in("status", ["active","done"]); | ||
|
|
||
| setPatient(patientData); | ||
| const bookedSlotIds = (bookedAppointments ?? []).map((a) => a.slot_id); |
There was a problem hiding this comment.
This client-side availability query cannot see other patients’ bookings.
With the current RLS policies in schema.sql Lines 85-91, this browser query only returns appointments where patient_id = auth.uid(). bookedSlotIds therefore misses slots booked by other patients, so the UI advertises unavailable slots as bookable and only fails later when /api/appointments/book returns 409.
The availability calculation needs to move behind a trusted server boundary, or to a view/RPC that exposes only free future slots.
Also applies to: 68-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/patient/dashboard/page.tsx` around lines 57 - 63, The client-side
supabase query in page.tsx that builds bookedSlotIds (the bookedAppointments
query and the bookedSlotIds mapping) is blocked by RLS and only returns the
current user's appointments; move this availability calculation behind a trusted
server boundary by adding a server RPC or API endpoint that returns booked or
free future slot IDs (e.g., create a Postgres view/RPC like
get_booked_future_slots or a server route /api/slots/availability that queries
appointments without user-level RLS using the service_role key), then replace
the client-side supabase .from("appointments").select(...) logic in page.tsx
(the bookedAppointments/bookedSlotIds usage and the similar block at lines
68-81) with a fetch to that endpoint and consume the returned free/booked slot
IDs. Ensure the server RPC or endpoint only exposes the minimal data (slot IDs
and future timestamp) and enforces any necessary filters (future slots, status
in ["active","done"]).
Comment on lines
+97
to
+107
| CREATE POLICY "appointments_insert_patient" | ||
| ON appointments FOR INSERT | ||
| WITH CHECK (auth.uid() = patient_id); | ||
|
|
||
| CREATE POLICY "appointments_update_patient" | ||
| ON appointments FOR UPDATE | ||
| USING (auth.uid() = patient_id); | ||
|
|
||
| CREATE POLICY "appointments_update_doctor" | ||
| ON appointments FOR UPDATE | ||
| USING (auth.uid() = doctor_id); |
There was a problem hiding this comment.
These write policies let authenticated clients bypass the API rules.
Because the browser already has a usable Supabase session, these policies allow direct insert/update calls against appointments without going through /api/appointments/book or /api/appointments/cancel. A patient can skip the future-slot and 1-hour cancellation checks, and can mark their own appointment as done directly because the database policy only checks row ownership.
If the route handlers are meant to be the enforcement point, direct table writes should be removed or replaced with much tighter RPC/policy logic that encodes the same business rules.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@schema.sql` around lines 97 - 107, The current row-level policies
appointments_insert_patient, appointments_update_patient, and
appointments_update_doctor on the appointments table allow authenticated clients
to bypass API-level business rules; remove or tighten these policies so direct
client INSERT/UPDATE is disallowed and enforce business logic via RPCs or
stricter policies. Specifically, delete or revoke open INSERT/UPDATE policies
for patients and doctors, create a single granular policy that only allows
selects for authenticated users, and implement Postgres functions/RPCs (e.g.,
book_appointment(), cancel_appointment(), mark_appointment_done()) that perform
the future-slot, 1-hour cancellation, and status-transition checks server-side,
then grant execute on those RPCs instead of row write privileges. Ensure
appointments_insert_patient, appointments_update_patient, and
appointments_update_doctor are replaced or disabled and that only the RPCs (and
any minimal read-only policies) are permitted for client sessions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR completes the backend functionality for the appointment booking system built on Next.js and Supabase.
What's implemented:
POST /api/appointments/book : patients can book available slots with full validation (no double booking, no duplicate doctor appointments)
POST /api/appointments/cancel : patients can cancel their appointments (with 1-hour restriction), doctors can cancel or mark appointments as done anytime
Admin login page and dashboard displaying all appointments, doctors, patients, and slots with status metrics
Updated schema with improved constraints : partial unique index to prevent duplicate active appointments per doctor, cancelled slots are rebookable
4 unit tests covering all business rules
2 integration tests covering the full booking and cancellation flow, all passing
Test results: 10/10 tests passing
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores