Full Stack - Completed appointment booking system#10
Open
DIVYADHARSHINI-08 wants to merge 1 commit intobotcode-com:mainfrom
Open
Full Stack - Completed appointment booking system#10DIVYADHARSHINI-08 wants to merge 1 commit intobotcode-com:mainfrom
DIVYADHARSHINI-08 wants to merge 1 commit intobotcode-com:mainfrom
Conversation
![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 administrative and doctor dashboards, login pages, and API routes for booking and cancelling appointments, supported by a new set of business rules and tests. While the addition of testing infrastructure is positive, several critical security and logic issues were identified. The admin authentication flow is highly insecure, utilizing plain-text password comparisons and client-side local storage checks. Furthermore, the booking API route is fundamentally misaligned with the frontend implementation, the cancellation route lacks proper authorization, and the doctor dashboard introduces a performance bottleneck via an N+1 query pattern.
Comment on lines
+9
to
+104
| export async function POST(req: NextRequest) { | ||
| try { | ||
| const body = await req.json(); | ||
| const { appointmentId, action } = body; | ||
|
|
||
| if (!appointmentId || !action) { | ||
| return NextResponse.json( | ||
| { error: "Missing appointmentId or action" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| // ✅ STEP 1: Get logged-in user | ||
| const { | ||
| data: { user }, | ||
| error: userError, | ||
| } = await supabase.auth.getUser(); | ||
|
|
||
| if (userError || !user) { | ||
| return NextResponse.json( | ||
| { error: "Unauthorized" }, | ||
| { status: 401 } | ||
| ); | ||
| } | ||
|
|
||
| // ✅ STEP 2: Check if doctor | ||
| const { data: doctor } = await supabase | ||
| .from("doctors") | ||
| .select("id") | ||
| .eq("id", user.id) | ||
| .single(); | ||
|
|
||
| const isDoctor = !!doctor; | ||
|
|
||
| // ❌ Only doctor can mark "done" | ||
| if (action === "done" && !isDoctor) { | ||
| return NextResponse.json( | ||
| { error: "Only doctor can mark appointment as done" }, | ||
| { status: 403 } | ||
| ); | ||
| } | ||
|
|
||
| // ✅ STEP 3: Get appointment | ||
| const { data: appt } = await supabase | ||
| .from("appointments") | ||
| .select("*") | ||
| .eq("id", appointmentId) | ||
| .single(); | ||
|
|
||
| if (!appt) { | ||
| return NextResponse.json( | ||
| { error: "Appointment not found" }, | ||
| { status: 404 } | ||
| ); | ||
| } | ||
|
|
||
| // ❌ Patient can only cancel their own appointment | ||
| if (!isDoctor && appt.patient_id !== user.id) { | ||
| return NextResponse.json( | ||
| { error: "Not allowed" }, | ||
| { status: 403 } | ||
| ); | ||
| } | ||
|
|
||
| // ✅ STEP 4: Update status | ||
| const newStatus = action === "done" ? "done" : "cancelled"; | ||
|
|
||
| const { error: updateError } = await supabase | ||
| .from("appointments") | ||
| .update({ status: newStatus }) | ||
| .eq("id", appointmentId); | ||
|
|
||
| if (updateError) { | ||
| return NextResponse.json( | ||
| { error: updateError.message }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
|
|
||
| // ✅ STEP 5: Free slot if cancelled | ||
| if (newStatus === "cancelled") { | ||
| await supabase | ||
| .from("slots") | ||
| .update({ is_booked: false }) | ||
| .eq("id", appt.slot_id); | ||
| } | ||
|
|
||
| return NextResponse.json({ success: true }); | ||
|
|
||
| } catch (err: any) { | ||
| return NextResponse.json( | ||
| { error: err.message }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The implementation of this route is fundamentally incorrect. It contains logic for updating appointment status (marking as 'done' or 'cancelled') rather than booking a new appointment. Additionally, it expects appointmentId and action in the request body, which contradicts the client-side implementation in app/patient/dashboard/page.tsx that sends slot_id and patient_id. This mismatch will cause the booking process to fail entirely.
Comment on lines
+22
to
+27
| const { data, error } = await supabase | ||
| .from("system_admins") | ||
| .select("*") | ||
| .eq("email", email) | ||
| .eq("password", password) | ||
| .single(); |
There was a problem hiding this comment.
Comparing passwords in plain text within a database query is a severe security vulnerability. Passwords should never be stored or compared in plain text. This approach exposes administrative credentials to anyone with access to the database or the query logs. You should use a secure authentication service like Supabase Auth to handle user credentials and session management.
Comment on lines
+10
to
+84
| export async function POST(req: NextRequest) { | ||
| try { | ||
| const { appointmentId, action } = await req.json(); | ||
|
|
||
| if (!appointmentId || !action) { | ||
| return NextResponse.json( | ||
| { error: "Missing appointmentId or action" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| // 1️⃣ Get appointment | ||
| const { data: appt, error: apptError } = await supabase | ||
| .from("appointments") | ||
| .select("*") | ||
| .eq("id", appointmentId) | ||
| .single(); | ||
|
|
||
| if (apptError || !appt) { | ||
| return NextResponse.json( | ||
| { error: "Appointment not found" }, | ||
| { status: 404 } | ||
| ); | ||
| } | ||
|
|
||
| // =============================== | ||
| // 🔥 ADD THIS BLOCK HERE | ||
| // =============================== | ||
|
|
||
| // Get slot time | ||
| const { data: slot } = await supabase | ||
| .from("slots") | ||
| .select("start_time") | ||
| .eq("id", appt.slot_id) | ||
| .single(); | ||
|
|
||
| // Apply rule ONLY for cancel (not for "done") | ||
| if (action === "cancel") { | ||
| const canCancel = canCancelWithTime(slot.start_time); | ||
|
|
||
| if (!canCancel) { | ||
| return NextResponse.json( | ||
| { error: "Cannot cancel within 1 hour of appointment" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // =============================== | ||
| // 🔥 END OF NEW BLOCK | ||
| // =============================== | ||
|
|
||
| // 2️⃣ Update status | ||
| const newStatus = action === "done" ? "done" : "cancelled"; | ||
|
|
||
| await supabase | ||
| .from("appointments") | ||
| .update({ status: newStatus }) | ||
| .eq("id", appointmentId); | ||
|
|
||
| // 3️⃣ Free slot | ||
| await supabase | ||
| .from("slots") | ||
| .update({ is_booked: false }) | ||
| .eq("id", appt.slot_id); | ||
|
|
||
| return NextResponse.json({ success: true }); | ||
|
|
||
| } catch (err: any) { | ||
| return NextResponse.json( | ||
| { error: err.message }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
This endpoint has two major issues:\n1. Missing Authorization: It does not verify if the requester is the patient who owns the appointment or the assigned doctor. Any authenticated user could potentially cancel any appointment by providing its ID.\n2. Incorrect Business Logic: The 1-hour cancellation restriction is applied to all users. However, the business rules specify that 'Doctors can cancel anytime.' You must verify the user's role and bypass the time check for doctors.
Comment on lines
+24
to
+28
| const isAdmin = localStorage.getItem("admin"); | ||
| if (!isAdmin) { | ||
| router.push("/admin/login"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The admin check relies on a value in localStorage, which is insecure as it can be easily manipulated by users in the browser console. This allows unauthorized access to the admin dashboard. Authentication should be verified on the server side or via a secure session token managed by an authentication provider.
Comment on lines
76
to
+104
| const { data: apptData } = await supabase | ||
| .from("appointments") | ||
| .select("id, status, created_at, patients(name), slots(start_time, end_time)") | ||
| .eq("doctor_id", user.id) | ||
| .order("created_at", { ascending: false }); | ||
| .from("appointments") | ||
| .select(` | ||
| id, | ||
| status, | ||
| created_at, | ||
| patient_id, | ||
| slot_id, | ||
| slot:slots(start_time, end_time) | ||
| `) | ||
| .eq("doctor_id", user.id) | ||
| .order("created_at", { ascending: false }); | ||
|
|
||
| const enrichedAppointments = await Promise.all( | ||
| (apptData ?? []).map(async (appt: any) => { | ||
| const { data: patient } = await supabase | ||
| .from("patients") | ||
| .select("name") | ||
| .eq("id", appt.patient_id) | ||
| .single(); | ||
|
|
||
| return { | ||
| ...appt, | ||
| patient: { name: patient?.name || "Unknown" }, | ||
| }; | ||
| }) | ||
| ); | ||
|
|
||
| setAppointments((apptData as Appointment[]) ?? []); | ||
| setAppointments(enrichedAppointments); |
There was a problem hiding this comment.
This implementation introduces an N+1 query problem by fetching patient details individually for each appointment inside a Promise.all loop. This will lead to significant performance issues as the number of appointments grows. You should use Supabase's relational querying to fetch the patient name in the initial query.
References
- Avoid N+1 query patterns by using joins or batching requests to improve performance.
WalkthroughThis PR introduces admin authentication with login and dashboard pages, implements appointment booking and cancellation API endpoints with Supabase persistence and authorization checks, refactors appointment data models across dashboards, establishes appointment validation rules, and configures Jest testing infrastructure with unit and integration test coverage. Changes
Sequence DiagramssequenceDiagram
participant Patient as Patient UI
participant API as /api/appointments/book
participant Supabase as Supabase
participant DB as Database
Patient->>API: POST { slot_id, patient_id }
API->>Supabase: getUser() - fetch auth info
Supabase-->>API: user object
API->>Supabase: fetch appointment by id
DB-->>Supabase: appointment record
Supabase-->>API: appointment data
API->>API: Validate authorization (role check)
API->>Supabase: Update appointment status
DB-->>Supabase: ✓ Update confirmed
Supabase-->>API: { success: true }
API-->>Patient: 200 Response
sequenceDiagram
participant Patient as Patient UI
participant API as /api/appointments/cancel
participant Rules as appointmentRules
participant Supabase as Supabase
participant DB as Database
Patient->>API: POST { appointmentId, action: "cancel" }
API->>Supabase: Fetch appointment by id
DB-->>Supabase: appointment + slot_id
Supabase-->>API: appointment record
API->>Supabase: Fetch slot start_time
DB-->>Supabase: slot.start_time
Supabase-->>API: start_time value
API->>Rules: canCancelWithTime(start_time)
Rules-->>API: boolean (1-hour check result)
alt Not allowed
API-->>Patient: 400 Cancellation denied
else Allowed
API->>Supabase: Update appointment status to "cancelled"
DB-->>Supabase: ✓ Appointment updated
API->>Supabase: Update slot is_booked = false
DB-->>Supabase: ✓ Slot freed
Supabase-->>API: { success: true }
API-->>Patient: 200 Response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
![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/login/page.tsx`:
- Around line 21-27: The current client-side admin check is insecure: the code
in app/admin/login/page.tsx uses
supabase.from("system_admins").select(...).eq("email", email).eq("password",
password).single() to compare plaintext passwords in the browser; move this
logic to a server-side endpoint (API route or Next.js server action) and stop
querying plaintext passwords from "system_admins". Instead, store only hashed
passwords and perform verification server-side using a secure hash compare (or
migrate to Supabase Auth and validate via its JWT/claims), then return a session
token or signed JWT to the client; update references to the client-side call
(the supabase .from(...).select(...) usage) to instead call the new server
endpoint (or use Supabase Auth) so authentication and password verification
happen only on the server.
In `@app/api/appointments/book/route.ts`:
- Around line 12-19: The route currently only accepts appointmentId/action and
returns 400 for the booking payload sent from the front end (slot_id and
patient_id); update the handler in route.ts to support both flows by branching:
if body contains slot_id and patient_id, run the "create booking" logic (create
new appointment using slot_id and patient_id), else if body contains
appointmentId and action, run the existing status update logic; validate each
branch's required fields separately and only return 400 when neither set of
required fields is present, ensuring you check for the exact front-end keys
slot_id and patient_id and preserve the appointmentId/action update behavior.
- Around line 74-75: The current assignment to newStatus (const newStatus =
action === "done" ? "done" : "cancelled") treats any unknown action as
"cancelled"; validate action against an explicit allowlist (e.g., allowed =
{"done","cancelled"} or include any other valid tokens) before mapping it to
newStatus, and reject/return a 4xx error for invalid input instead of silently
treating it as "cancelled"; update the handler that reads action and the
newStatus assignment to first check allowed.has(action) and only then set
newStatus = action (or map known aliases), otherwise respond with a clear
validation error.
In `@app/api/appointments/cancel/route.ts`:
- Around line 40-49: The supabase slot lookup result is not being checked before
using slot.start_time; update the code after the supabase .single() call to
guard for missing data or an error (inspect the response's data/error fields),
and when action === "cancel" return a deterministic HTTP error (4xx for not
found / 5xx for DB error) instead of dereferencing slot.start_time; ensure
canCancelWithTime(slot.start_time) is only called when slot exists and no DB
error occurred.
- Around line 12-19: The handler currently reads appointmentId and action
directly from the request body; enforce server-side actor auth and a strict
action allowlist before performing any mutation: derive the caller identity and
role from your server-trusted auth (e.g., server session / JWT verification used
elsewhere in the app) and check that the actor is authorized to change this
appointment (owner or admin) before calling the status update logic, and
validate action against an allowlist of only "cancel" or "done" (reject other
values with a 400/403); update the code paths that perform status updates (the
place that uses the service-role client / appointment status update call) to
only run after both the identity/role check and action validation pass.
- Around line 65-74: The update results are ignored causing false success and
possible appointment/slot divergence; modify the flow in route handler to check
the result/error of the appointments update (the .from("appointments").update
call that sets { status: newStatus } using appointmentId) and return a failure
if it fails, then perform the slots update (.from("slots").update where id ==
appt.slot_id) and check its result; if the slots update fails, attempt to roll
back the appointment by updating it back to the previous status (use appt.status
or a saved previousStatus) and return an error if rollback fails, or use a DB
transaction/RPC instead to make both writes atomic.
In `@app/doctor/dashboard/page.tsx`:
- Around line 76-102: The code does N+1 queries by fetching patients inside the
enrichedAppointments map; instead modify the initial supabase query (the
supabase.from("appointments").select(...)) to include the related patient in the
select (e.g., add patient:patients(name) to the select list) so each appointment
returns its patient data in one query, then remove the Promise.all map that
queries patients per-appointment and instead build enrichedAppointments directly
from apptData (using the returned patient field and existing slot:slots(...)
field); update any references to patient.name to use the embedded patient
payload and handle missing patient with a fallback like "Unknown".
In `@app/patient/dashboard/page.tsx`:
- Around line 108-111: Remove the debug console.log statements that print
patient-linked identifiers (the console.log that outputs { slot_id: slotId,
patient_id: user.id } and the similar log at the second occurrence). In page.tsx
delete those console.log calls (or replace them with a non-identifying debug
flag/log that does not include slotId or user.id), ensuring no patient IDs are
written to browser logs; locate the calls by searching for
console.log("Sending:" and the object with slot_id/patient_id.
- Around line 94-135: Wrap the body of handleBook in a try/catch/finally: move
the async fetch and res.json() into try, catch any thrown error to call
setActionMsg with a useful message (and console.error the error), and ensure
setBookingSlotId(null) runs in finally so the UI button is never left disabled;
keep the existing success branch that calls setAvailableSlots and setActionMsg
when res.ok, but in the catch also clear any partial state (e.g., reset
bookingSlotId) and avoid swallowing the error.
In `@app/patient/login/page.tsx`:
- Line 41: Remove the unused localStorage write: delete the call to
localStorage.setItem("patient_id", authData.user.id) in the login flow
(page.tsx) because the app relies on Supabase auth state (getUser) instead;
search for any other reads of "patient_id" and remove or refactor them if found,
and ensure no runtime code depends on that key (e.g., in authentication helpers
or components that reference patient_id).
In `@tests/integration.test.ts`:
- Around line 10-14: Tests share mutable globals (testSlotId, appointmentId,
etc.) causing cross-test coupling; move the setup that creates testDoctor,
testPatient, testSlot and books appointment into a beforeAll hook (and store
results into the same variables) or alternatively combine
create+cancel/verification into a single test so no other test depends on
appointmentId; update or add an afterAll to clean up created entities. Locate
references to testSlotId, appointmentId, testPatientId, testDoctorId in the test
file and refactor initialization into beforeAll/afterAll or local test-scoped
variables to ensure each test is deterministic and independent.
- Around line 16-99: The tests directly manipulate the DB; update the "Booking
Flow - patient books available slot" and "Cancellation Flow - valid cancel
updates status" tests to exercise the actual endpoints (/api/appointments/book
and /api/appointments/cancel) instead of using supabase .from(...).insert/update
for booking/canceling: send HTTP requests to POST/PUT the booking and
cancellation endpoints (including required auth headers/token and request body
shape the routes expect), assert the endpoint responses (status and body) and
only then verify DB state via supabase selects (e.g., check appointments.status
and slots.is_booked). Ensure you use the same test identifiers (testSlotId,
testDoctorId, testPatientId, appointmentId) so existing verification steps can
remain but are triggered by the API calls rather than direct DB writes.
In `@tsconfig.json`:
- Line 3: The tsconfig currently restricts global types to Jest only which
removes Node globals like process/env used in production (e.g., lib/supabase.ts
and app/api/appointments/*.ts); update the "types" array in tsconfig.json to
include "node" alongside "jest" (or alternatively move "jest" into a
test-specific tsconfig) so `@types/node` is available globally and type-checking
for process.env and other Node globals succeeds.
🪄 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: 0ceb9ceb-f857-4216-a507-0695da5a9030
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
app/admin/dashboard/page.tsxapp/admin/login/page.tsxapp/api/appointments/book/route.tsapp/api/appointments/cancel/route.tsapp/doctor/dashboard/page.tsxapp/patient/dashboard/page.tsxapp/patient/login/page.tsxjest.config.jslib/appointmentRules.tspackage.jsontests/appointmentRules.test.tstests/integration.test.tstsconfig.json
Comment on lines
+21
to
+27
| // ✅ Check admin in DB | ||
| const { data, error } = await supabase | ||
| .from("system_admins") | ||
| .select("*") | ||
| .eq("email", email) | ||
| .eq("password", password) | ||
| .single(); |
There was a problem hiding this comment.
Move admin credential verification off the client immediately.
Line 22–27 performs admin auth in the browser and compares plaintext passwords from DB. This exposes authentication logic and requires insecure password storage.
🔧 Suggested direction
- const { data, error } = await supabase
- .from("system_admins")
- .select("*")
- .eq("email", email)
- .eq("password", password)
- .single();
-
- if (error || !data) {
+ const res = await fetch("/api/admin/login", {
+ method: "POST",
+ headers: { "Content-Type": "application/json" },
+ body: JSON.stringify({ email, password }),
+ });
+ const data = await res.json();
+ if (!res.ok) {
setError("Invalid admin credentials");
setLoading(false);
return;
}Based on learnings: In the Next.js admin area, do not authenticate by comparing passwords in plaintext and use a secure, verifiable mechanism (e.g., signed JWT or Supabase Auth claims).
🤖 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 21 - 27, The current client-side admin
check is insecure: the code in app/admin/login/page.tsx uses
supabase.from("system_admins").select(...).eq("email", email).eq("password",
password).single() to compare plaintext passwords in the browser; move this
logic to a server-side endpoint (API route or Next.js server action) and stop
querying plaintext passwords from "system_admins". Instead, store only hashed
passwords and perform verification server-side using a secure hash compare (or
migrate to Supabase Auth and validate via its JWT/claims), then return a session
token or signed JWT to the client; update references to the client-side call
(the supabase .from(...).select(...) usage) to instead call the new server
endpoint (or use Supabase Auth) so authentication and password verification
happen only on the server.
Comment on lines
+12
to
+19
| const { appointmentId, action } = body; | ||
|
|
||
| if (!appointmentId || !action) { | ||
| return NextResponse.json( | ||
| { error: "Missing appointmentId or action" }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
/book endpoint currently performs status updates, not booking.
This route requires appointmentId/action and updates existing appointments, while booking flow sends slot_id/patient_id from app/patient/dashboard/page.tsx (Lines 118-121). As implemented, booking requests fail at Line 14 with 400.
Also applies to: 73-96
🤖 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 12 - 19, The route currently
only accepts appointmentId/action and returns 400 for the booking payload sent
from the front end (slot_id and patient_id); update the handler in route.ts to
support both flows by branching: if body contains slot_id and patient_id, run
the "create booking" logic (create new appointment using slot_id and
patient_id), else if body contains appointmentId and action, run the existing
status update logic; validate each branch's required fields separately and only
return 400 when neither set of required fields is present, ensuring you check
for the exact front-end keys slot_id and patient_id and preserve the
appointmentId/action update behavior.
Comment on lines
+74
to
+75
| const newStatus = action === "done" ? "done" : "cancelled"; | ||
|
|
There was a problem hiding this comment.
Validate action against an explicit allowlist before mutation.
Line 74 maps any value other than "done" to "cancelled", so invalid input can cancel appointments unintentionally.
Proposed fix
- const newStatus = action === "done" ? "done" : "cancelled";
+ if (action !== "done" && action !== "cancel") {
+ 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/book/route.ts` around lines 74 - 75, The current
assignment to newStatus (const newStatus = action === "done" ? "done" :
"cancelled") treats any unknown action as "cancelled"; validate action against
an explicit allowlist (e.g., allowed = {"done","cancelled"} or include any other
valid tokens) before mapping it to newStatus, and reject/return a 4xx error for
invalid input instead of silently treating it as "cancelled"; update the handler
that reads action and the newStatus assignment to first check
allowed.has(action) and only then set newStatus = action (or map known aliases),
otherwise respond with a clear validation error.
Comment on lines
+12
to
+19
| const { appointmentId, action } = await req.json(); | ||
|
|
||
| if (!appointmentId || !action) { | ||
| return NextResponse.json( | ||
| { error: "Missing appointmentId or action" }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Enforce actor authorization and strict action allowlist before mutation.
Line 12 accepts arbitrary action, and this route performs status updates with a service-role client without verifying who is calling. Any caller with an appointment ID can potentially mark records done/cancelled. Derive actor identity/role from trusted server auth and reject unsupported actions.
Based on learnings: In production, role must come from trusted server-side session/JWT and action should be validated against an allowlist ("cancel" | "done"), with forbidden mutations rejected.
Also applies to: 63-64
🤖 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 12 - 19, The handler
currently reads appointmentId and action directly from the request body; enforce
server-side actor auth and a strict action allowlist before performing any
mutation: derive the caller identity and role from your server-trusted auth
(e.g., server session / JWT verification used elsewhere in the app) and check
that the actor is authorized to change this appointment (owner or admin) before
calling the status update logic, and validate action against an allowlist of
only "cancel" or "done" (reject other values with a 400/403); update the code
paths that perform status updates (the place that uses the service-role client /
appointment status update call) to only run after both the identity/role check
and action validation pass.
Comment on lines
+40
to
+49
| const { data: slot } = await supabase | ||
| .from("slots") | ||
| .select("start_time") | ||
| .eq("id", appt.slot_id) | ||
| .single(); | ||
|
|
||
| // Apply rule ONLY for cancel (not for "done") | ||
| if (action === "cancel") { | ||
| const canCancel = canCancelWithTime(slot.start_time); | ||
|
|
There was a problem hiding this comment.
Guard slot lookup failures before using slot.start_time.
Line 48 dereferences slot.start_time without checking whether Line 40–44 returned data/error. A missing/failed slot lookup currently becomes a 500 path instead of a deterministic 4xx/5xx response.
🤖 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 40 - 49, The supabase slot
lookup result is not being checked before using slot.start_time; update the code
after the supabase .single() call to guard for missing data or an error (inspect
the response's data/error fields), and when action === "cancel" return a
deterministic HTTP error (4xx for not found / 5xx for DB error) instead of
dereferencing slot.start_time; ensure canCancelWithTime(slot.start_time) is only
called when slot exists and no DB error occurred.
Comment on lines
+108
to
+111
| console.log("Sending:", { | ||
| slot_id: slotId, | ||
| patient_id: user.id, | ||
| }); |
There was a problem hiding this comment.
Remove debug logs that print patient-linked identifiers.
Logging request/response payloads here exposes internal IDs in browser logs and should be avoided outside local debugging.
Also applies to: 125-125
🤖 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 108 - 111, Remove the debug
console.log statements that print patient-linked identifiers (the console.log
that outputs { slot_id: slotId, patient_id: user.id } and the similar log at the
second occurrence). In page.tsx delete those console.log calls (or replace them
with a non-identifying debug flag/log that does not include slotId or user.id),
ensuring no patient IDs are written to browser logs; locate the calls by
searching for console.log("Sending:" and the object with slot_id/patient_id.
| return; | ||
| } | ||
|
|
||
| localStorage.setItem("patient_id", authData.user.id); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused patient_id localStorage write.
Line 41 persists patient_id, but current patient session flow relies on Supabase auth state (getUser) and does not consume this value. Keeping it adds unnecessary mutable client state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/patient/login/page.tsx` at line 41, Remove the unused localStorage write:
delete the call to localStorage.setItem("patient_id", authData.user.id) in the
login flow (page.tsx) because the app relies on Supabase auth state (getUser)
instead; search for any other reads of "patient_id" and remove or refactor them
if found, and ensure no runtime code depends on that key (e.g., in
authentication helpers or components that reference patient_id).
Comment on lines
+10
to
+14
| let testSlotId: string; | ||
| let testPatientId: string; | ||
| let testDoctorId: string; | ||
| let appointmentId: string; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid cross-test state coupling via shared globals.
appointmentId/testSlotId from the first test are required by the second test. Move setup into beforeAll (or create+cancel within one test) for deterministic behavior.
Also applies to: 76-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration.test.ts` around lines 10 - 14, Tests share mutable globals
(testSlotId, appointmentId, etc.) causing cross-test coupling; move the setup
that creates testDoctor, testPatient, testSlot and books appointment into a
beforeAll hook (and store results into the same variables) or alternatively
combine create+cancel/verification into a single test so no other test depends
on appointmentId; update or add an afterAll to clean up created entities. Locate
references to testSlotId, appointmentId, testPatientId, testDoctorId in the test
file and refactor initialization into beforeAll/afterAll or local test-scoped
variables to ensure each test is deterministic and independent.
Comment on lines
+16
to
+99
| test("Booking Flow - patient books available slot", async () => { | ||
| // 1️⃣ Get a free slot | ||
| const { data: slot } = await supabase | ||
| .from("slots") | ||
| .select("*") | ||
| .eq("is_booked", false) | ||
| .limit(1) | ||
| .single(); | ||
|
|
||
| expect(slot).toBeTruthy(); | ||
|
|
||
| testSlotId = slot.id; | ||
| testDoctorId = slot.doctor_id; | ||
|
|
||
| // 2️⃣ Get a patient | ||
| const { data: patient } = await supabase | ||
| .from("patients") | ||
| .select("*") | ||
| .limit(1) | ||
| .single(); | ||
|
|
||
| expect(patient).toBeTruthy(); | ||
|
|
||
| testPatientId = patient.id; | ||
|
|
||
| // 3️⃣ Create appointment | ||
| const { data: appt, error } = await supabase | ||
| .from("appointments") | ||
| .insert({ | ||
| patient_id: testPatientId, | ||
| doctor_id: testDoctorId, | ||
| slot_id: testSlotId, | ||
| status: "active", | ||
| }) | ||
| .select() | ||
| .single(); | ||
|
|
||
| expect(error).toBeNull(); | ||
| expect(appt).toBeTruthy(); | ||
|
|
||
| appointmentId = appt.id; | ||
|
|
||
| // 4️⃣ Mark slot booked | ||
| await supabase | ||
| .from("slots") | ||
| .update({ is_booked: true }) | ||
| .eq("id", testSlotId); | ||
|
|
||
| // 5️⃣ Verify in DB | ||
| const { data: check } = await supabase | ||
| .from("appointments") | ||
| .select("*") | ||
| .eq("id", appointmentId) | ||
| .single(); | ||
|
|
||
| expect(check).toBeTruthy(); | ||
| }); | ||
|
|
||
|
|
||
| // 🔹 CANCELLATION FLOW | ||
| test("Cancellation Flow - valid cancel updates status", async () => { | ||
| // 1️⃣ Cancel appointment | ||
| const { error } = await supabase | ||
| .from("appointments") | ||
| .update({ status: "cancelled" }) | ||
| .eq("id", appointmentId); | ||
|
|
||
| expect(error).toBeNull(); | ||
|
|
||
| // 2️⃣ Verify status | ||
| const { data: updated } = await supabase | ||
| .from("appointments") | ||
| .select("status") | ||
| .eq("id", appointmentId) | ||
| .single(); | ||
|
|
||
| expect(updated.status).toBe("cancelled"); | ||
|
|
||
| // 3️⃣ Free slot | ||
| await supabase | ||
| .from("slots") | ||
| .update({ is_booked: false }) | ||
| .eq("id", testSlotId); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
These are DB workflow tests, not API integration tests.
Current tests bypass /api/appointments/book and /api/appointments/cancel, so they do not validate endpoint request contracts, auth checks, or route-level business-rule enforcement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration.test.ts` around lines 16 - 99, The tests directly
manipulate the DB; update the "Booking Flow - patient books available slot" and
"Cancellation Flow - valid cancel updates status" tests to exercise the actual
endpoints (/api/appointments/book and /api/appointments/cancel) instead of using
supabase .from(...).insert/update for booking/canceling: send HTTP requests to
POST/PUT the booking and cancellation endpoints (including required auth
headers/token and request body shape the routes expect), assert the endpoint
responses (status and body) and only then verify DB state via supabase selects
(e.g., check appointments.status and slots.is_booked). Ensure you use the same
test identifiers (testSlotId, testDoctorId, testPatientId, appointmentId) so
existing verification steps can remain but are triggered by the API calls rather
than direct DB writes.
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "types": ["jest"], | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current global types config and whether Node globals are used in TS files.
jq -r '.compilerOptions.types' tsconfig.json
rg -nP '\bprocess\.env\b|\bNodeJS\b|\bBuffer\b' --type ts --type tsxRepository: botcode-com/task
Length of output: 119
🏁 Script executed:
# Search for Node globals without problematic type filters
rg '\b(process\.(env|cwd|argv)|Buffer|__dirname|__filename|NodeJS)\b' --type ts -n
# Also check for `@types/node` in dependencies
if [ -f package.json ]; then
echo "=== Checking package.json for `@types/node` ==="
jq '.dependencies, .devDependencies' package.json 2>/dev/null | grep -A5 -B5 "@types/node" || echo "No `@types/node` found"
fi
# Check the full tsconfig.json to understand the setup
echo "=== Full tsconfig.json ==="
cat tsconfig.jsonRepository: botcode-com/task
Length of output: 1578
types: ["jest"] removes Node global typings needed by production code.
Line 3 restricts global types to Jest only. However, your codebase uses process.env in production files (lib/supabase.ts, app/api/appointments/*.ts). Even though @types/node is installed, these types won't be globally available with the current config, causing type-checking failures. Add "node" to the types array or move Jest types to a test-specific tsconfig.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.json` at line 3, The tsconfig currently restricts global types to
Jest only which removes Node globals like process/env used in production (e.g.,
lib/supabase.ts and app/api/appointments/*.ts); update the "types" array in
tsconfig.json to include "node" alongside "jest" (or alternatively move "jest"
into a test-specific tsconfig) so `@types/node` is available globally and
type-checking for process.env and other Node globals succeeds.
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.
Appointment Booking System
Overview
This is a full-stack appointment booking system built using Next.js and Supabase.
It supports patients, doctors, and system administrators with role-based workflows and validation rules.
Features
Patient
Doctor
Admin
API Routes
1. POST
/api/appointments/bookBooks an appointment
Validates:
2. POST
/api/appointments/cancelCancels or marks appointment as done
Validates:
Business Rules
A slot cannot be booked if already booked
A patient cannot:
Doctors can cancel anytime
Testing
Unit Tests
Integration Tests
Run tests using:
Tech Stack
Environment Variables
Create
.envfile:Project Structure
Status
Result
Successfully developed a full-stack assignment demonstrating backend logic, API design, and testing.
Summary by CodeRabbit
New Features
Tests
Chores