-
Notifications
You must be signed in to change notification settings - Fork 12
Task 2: Appointment Booking System #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import { redirect } from "next/navigation"; | ||
| import { cookies } from "next/headers"; | ||
| import { supabaseAdmin } from "@/lib/supabaseAdmin"; | ||
|
|
||
| function formatDateTime(iso: string) { | ||
| return new Date(iso).toLocaleString("en-IN", { | ||
| dateStyle: "medium", | ||
| timeStyle: "short", | ||
| }); | ||
| } | ||
|
|
||
| export default async function AdminDashboard() { | ||
| const cookieStore = await cookies(); | ||
| const adminToken = cookieStore.get("admin_token"); | ||
|
|
||
| if (!adminToken) { | ||
| redirect("/admin/login"); | ||
| } | ||
|
|
||
| const { data: appointments, error } = await supabaseAdmin | ||
| .from("appointments") | ||
| .select("id, status, created_at, patients(name, email), doctors(name, specialty, email), slots(start_time, end_time)") | ||
| .order("created_at", { ascending: false }); | ||
|
|
||
| if (error) { | ||
| return <div className="p-8 text-red-500">Failed to load appointments.</div>; | ||
| } | ||
|
|
||
| return ( | ||
| <main className="min-h-screen bg-gray-50 p-6"> | ||
| <div className="mx-auto max-w-6xl"> | ||
| <div className="mb-6 flex items-center justify-between"> | ||
| <h1 className="text-2xl font-bold">System Admin Dashboard</h1> | ||
| <a | ||
| href="/" | ||
| className="rounded-lg border px-4 py-2 text-sm text-gray-600 hover:bg-gray-100" | ||
| > | ||
| Go Home | ||
| </a> | ||
| </div> | ||
|
|
||
| <section> | ||
| <h2 className="mb-3 text-lg font-semibold">All Appointments Hub</h2> | ||
| {(!appointments || appointments.length === 0) ? ( | ||
| <p className="text-sm text-gray-500">No appointments in the system.</p> | ||
| ) : ( | ||
| <div className="overflow-hidden rounded-xl border bg-white"> | ||
| <table className="w-full text-sm"> | ||
| <thead className="bg-gray-50 text-left text-gray-600"> | ||
| <tr> | ||
| <th className="px-4 py-3 font-medium">Patient</th> | ||
| <th className="px-4 py-3 font-medium">Doctor</th> | ||
| <th className="px-4 py-3 font-medium">Slot Time</th> | ||
| <th className="px-4 py-3 font-medium">Status</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody className="divide-y"> | ||
| {appointments.map((appt: any) => ( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion. I agree that defining a proper TypeScript interface would improve type safety and maintainability. The |
||
| <tr key={appt.id}> | ||
| <td className="px-4 py-3"> | ||
| <div className="font-medium text-gray-900">{appt.patients?.name || "Unknown"}</div> | ||
| <div className="text-xs text-gray-500">{appt.patients?.email}</div> | ||
| </td> | ||
| <td className="px-4 py-3"> | ||
| <div className="font-medium text-gray-900">{appt.doctors?.name || "Unknown"}</div> | ||
| <div className="text-xs text-gray-500">{appt.doctors?.specialty} β’ {appt.doctors?.email}</div> | ||
| </td> | ||
| <td className="px-4 py-3 text-gray-500"> | ||
| {appt.slots?.start_time ? formatDateTime(appt.slots.start_time) : "N/A"} | ||
| </td> | ||
| <td className="px-4 py-3"> | ||
| <span | ||
| className={`rounded-full px-2 py-0.5 text-xs font-medium ${ | ||
| appt.status === "active" | ||
| ? "bg-blue-100 text-blue-700" | ||
| : appt.status === "done" | ||
| ? "bg-green-100 text-green-700" | ||
| : "bg-gray-100 text-gray-600" | ||
| }`} | ||
| > | ||
| {appt.status} | ||
| </span> | ||
| </td> | ||
| </tr> | ||
| ))} | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| )} | ||
| </section> | ||
| </div> | ||
| </main> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| "use client"; | ||
|
|
||
| import { useState } from "react"; | ||
| import { useRouter } from "next/navigation"; | ||
| import Link from "next/link"; | ||
|
|
||
| export default function AdminLogin() { | ||
| const router = useRouter(); | ||
| const [email, setEmail] = useState(""); | ||
| const [password, setPassword] = useState(""); | ||
| const [error, setError] = useState(""); | ||
| const [loading, setLoading] = useState(false); | ||
|
|
||
| async function handleSubmit(e: React.FormEvent) { | ||
| e.preventDefault(); | ||
| setError(""); | ||
| setLoading(true); | ||
|
|
||
| const res = await fetch("/api/admin/login", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ email, password }), | ||
| }); | ||
|
|
||
| if (!res.ok) { | ||
| const data = await res.json(); | ||
| setError(data.error || "Login failed"); | ||
| setLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| router.push("/admin/dashboard"); | ||
| } | ||
|
afifa17112005 marked this conversation as resolved.
|
||
|
|
||
| return ( | ||
| <main className="flex min-h-screen items-center justify-center bg-gray-50"> | ||
| <div className="w-full max-w-md rounded-xl border bg-white p-8 shadow-sm"> | ||
| <h1 className="mb-6 text-2xl font-bold">Admin Login</h1> | ||
| <form onSubmit={handleSubmit} className="flex flex-col gap-4"> | ||
| <div> | ||
| <label className="mb-1 block text-sm font-medium text-gray-700"> | ||
| </label> | ||
| <input | ||
| type="email" | ||
| value={email} | ||
| onChange={(e) => setEmail(e.target.value)} | ||
| required | ||
| placeholder="admin@test.com" | ||
| className="w-full rounded-lg border px-3 py-2 focus:outline-none focus:ring-2 focus:ring-blue-500" | ||
| /> | ||
| </div> | ||
| <div> | ||
| <label className="mb-1 block text-sm font-medium text-gray-700"> | ||
| Password | ||
| </label> | ||
| <input | ||
| type="password" | ||
| value={password} | ||
| onChange={(e) => setPassword(e.target.value)} | ||
| required | ||
| className="w-full rounded-lg border px-3 py-2 focus:outline-none focus:ring-2 focus:ring-blue-500" | ||
| /> | ||
| </div> | ||
| {error && <p className="text-sm text-red-600">{error}</p>} | ||
| <button | ||
| type="submit" | ||
| disabled={loading} | ||
| className="rounded-lg bg-blue-600 py-2 font-medium text-white hover:bg-blue-700 disabled:opacity-50" | ||
| > | ||
| {loading ? "Logging in..." : "Login"} | ||
| </button> | ||
| </form> | ||
| <p className="mt-4 text-center text-sm text-gray-500"> | ||
| <Link href="/" className="text-blue-600 hover:underline"> | ||
| β Back to home | ||
| </Link> | ||
| </p> | ||
| </div> | ||
| </main> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { supabaseAdmin } from "@/lib/supabaseAdmin"; | ||
|
|
||
| export async function POST(req: NextRequest) { | ||
| try { | ||
| const { email, password } = await req.json(); | ||
|
|
||
| const { data: admin, error } = await supabaseAdmin | ||
| .from("system_admins") | ||
| .select("*") | ||
| .eq("email", email) | ||
| .single(); | ||
|
|
||
| if (error || !admin) { | ||
| return NextResponse.json({ error: "Invalid email or password" }, { status: 401 }); | ||
| } | ||
|
|
||
| if (admin.password !== password) { | ||
| return NextResponse.json({ error: "Invalid email or password" }, { status: 401 }); | ||
| } | ||
|
afifa17112005 marked this conversation as resolved.
Comment on lines
+18
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not authenticate with plaintext password comparison. Line 18 indicates reversible/plaintext credential handling. This is a blocker: store one-way password hashes and verify against hash, not raw text. π€ Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for highlighting this. I agree that comparing plaintext passwords is not secure and is not suitable for production systems. In this implementation, plaintext comparison was used for simplicity and to focus on core application logic. In a production environment, passwords would be securely hashed using a strong algorithm such as bcrypt or Argon2, and comparisons would be performed against the hashed values. This is a valid security improvement and would be addressed in a production-ready version. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Would you like me to open a follow-up GitHub issue to track these production-readiness improvements? π§ Learnings used |
||
|
|
||
| const response = NextResponse.json({ success: true }); | ||
| response.cookies.set("admin_token", admin.id, { | ||
| httpOnly: true, | ||
| path: "/", | ||
| maxAge: 60 * 60 * 24 * 7, // 1 week | ||
| }); | ||
|
afifa17112005 marked this conversation as resolved.
|
||
|
|
||
| return response; | ||
| } catch (error) { | ||
| return NextResponse.json({ error: "Internal server error" }, { status: 500 }); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,105 @@ | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { supabaseAdmin } from "@/lib/supabaseAdmin"; | ||
| import { canPatientBookSlot, canPatientBookDoctor } from "@/lib/appointment-rules"; | ||
|
|
||
| export async function POST(_req: NextRequest) { | ||
| return NextResponse.json({ error: "Not implemented" }, { status: 501 }); | ||
| } | ||
| export async function POST(req: NextRequest) { | ||
| try { | ||
| const body = await req.json(); | ||
|
|
||
| const { patient_id, doctor_id, slot_id } = body; | ||
|
|
||
| console.log("RECEIVED:", { patient_id, doctor_id, slot_id }); | ||
|
|
||
| // 1. Validate input | ||
| if (!patient_id || !doctor_id || !slot_id) { | ||
| return NextResponse.json( | ||
| { error: "Missing required fields" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
Comment on lines
+9
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not accept A caller can book on behalf of any patient by sending another π€ Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for highlighting this. I agree that accepting In this implementation, it was kept simple for demonstration purposes. In a production system, the patient identity would be derived from the authenticated session or JWT on the server side, and any client-provided identifiers would be ignored or validated against the session. This is a valid security improvement and would be enforced in a production-ready setup. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
// Production pattern: derive patient identity server-side
import { createServerClient } from "@supabase/ssr";
import { cookies } from "next/headers";
export async function POST(req: NextRequest) {
const cookieStore = cookies();
const supabase = createServerClient(
process.env.NEXT_PUBLIC_SUPABASE_URL!,
process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!,
{ cookies: { get: (name) => cookieStore.get(name)?.value } }
);
const { data: { user }, error } = await supabase.auth.getUser();
if (!user) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}
// Use user.id as the authoritative patient_id β ignore any client-supplied value
const patient_id = user.id;
const { doctor_id, slot_id } = await req.json();
// ... rest of booking logic
}Key production hardening steps to track for later:
βοΈ Learnings added
|
||
|
|
||
| // 2. Fetch slot safely | ||
| const { data: slots, error } = await supabaseAdmin | ||
| .from("slots") | ||
| .select("*"); | ||
|
afifa17112005 marked this conversation as resolved.
|
||
|
|
||
| if (error) { | ||
| console.error(error); | ||
| return NextResponse.json( | ||
| { error: "Error fetching slots" }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
|
|
||
| const slot = slots?.find((s) => s.id === slot_id); | ||
|
|
||
|
Comment on lines
+22
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Booking flow is race-prone and can create double bookings. Current read-then-insert-then-update sequence is non-atomic. Concurrent requests can both pass checks and insert active appointments. Atomically claim slot first ( Atomic claim pattern (recommended direction)- const { data: slots, error } = await supabaseAdmin
- .from("slots")
- .select("*");
- ...
- const slot = slots?.find((s) => s.id === slot_id);
+ const { data: claimedSlot, error: claimError } = await supabaseAdmin
+ .from("slots")
+ .update({ is_booked: true })
+ .eq("id", slot_id)
+ .eq("doctor_id", doctor_id)
+ .eq("is_booked", false)
+ .select("id, doctor_id")
+ .single();
+
+ if (claimError || !claimedSlot) {
+ return NextResponse.json({ error: "Slot already booked or invalid" }, { status: 409 });
+ }
...
- await supabaseAdmin
- .from("slots")
- .update({ is_booked: true })
- .eq("id", slot_id);
+ // Slot already claimed above. If insert fails, release claim.
+ if (insertError) {
+ await supabaseAdmin.from("slots").update({ is_booked: false }).eq("id", slot_id);
+ ...
+ }Also applies to: 65-91 π€ Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for highlighting this. I agree that the current readβthenβinsertβthenβupdate flow is not atomic and can lead to race conditions where multiple users may book the same slot concurrently. In this implementation, the flow was kept simple for clarity. In a production system, this would be handled using an atomic slot-claim approach, such as a conditional update ( Additionally, rollback mechanisms would be applied in case of failures after the slot is claimed. This is a valid improvement and would be addressed in a production-ready implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's also worth noting that your PR description's sequence diagram (Image 1) already illustrates the correct atomic approach β When you do productionize this, the key steps from your own diagram would translate roughly to:
That matches exactly what you described. Happy to generate a code diff when you're ready to implement it! (γ₯οΏ£ Β³οΏ£)γ₯ βοΈ Learnings added
π§ Learnings used |
||
| if (!slot) { | ||
| return NextResponse.json( | ||
| { error: "Slot not found" }, | ||
| { status: 404 } | ||
| ); | ||
| } | ||
|
|
||
| // 3. Check if already booked | ||
| if (!canPatientBookSlot(slot)) { | ||
| return NextResponse.json( | ||
| { error: "Slot already booked" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| // 4. Check existing appointment with same doctor | ||
| const { data: existing } = await supabaseAdmin | ||
| .from("appointments") | ||
| .select("status") | ||
| .eq("patient_id", patient_id) | ||
| .eq("doctor_id", doctor_id); | ||
|
|
||
| if (existing && !canPatientBookDoctor(existing)) { | ||
|
Comment on lines
+52
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle appointment lookup errors before rule evaluation. The π€ Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing this out. I agree that the current implementation does not explicitly handle errors from the appointments query, which could lead to incorrect behavior if the database call fails. In a production setup, proper error handling would be added to check for query failures and return an appropriate 500 response before proceeding with any business rule evaluation. This is a valid improvement to ensure reliability and will be incorporated in a production-ready version. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
// 4. Check existing appointment with same doctor
const { data: existing, error: existingError } = await supabaseAdmin
.from("appointments")
.select("status")
.eq("patient_id", patient_id)
.eq("doctor_id", doctor_id);
if (existingError) {
console.error(existingError);
return NextResponse.json(
{ error: "Error checking existing appointments" },
{ status: 500 }
);
}
if (existing && !canPatientBookDoctor(existing)) {
return NextResponse.json(
{ error: "Already have an active appointment with this doctor" },
{ status: 400 }
);
}This ensures a DB failure on the duplicate-check query surfaces as a π§ Learnings used |
||
| return NextResponse.json( | ||
| { error: "Already have an active appointment with this doctor" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| // 5. Insert appointment | ||
| const { data: insertData, error: insertError } = await supabaseAdmin | ||
| .from("appointments") | ||
| .insert([ | ||
| { | ||
| patient_id, | ||
| doctor_id, | ||
| slot_id, | ||
| status: "active", | ||
| }, | ||
| ]) | ||
| .select("id, status, slots(id, start_time, end_time), doctors(id, name, specialty)") | ||
| .single(); | ||
|
|
||
| if (insertError) { | ||
| console.error(insertError); | ||
| return NextResponse.json( | ||
| { error: insertError.message }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
|
|
||
| // 6. Update slot | ||
| await supabaseAdmin | ||
| .from("slots") | ||
| .update({ is_booked: true }) | ||
| .eq("id", slot_id); | ||
|
afifa17112005 marked this conversation as resolved.
|
||
|
|
||
| return NextResponse.json({ | ||
| message: "Appointment booked successfully", | ||
| appointment: insertData | ||
| }); | ||
|
|
||
| } catch (err: any) { | ||
| console.error("ERROR:", err); | ||
| return NextResponse.json( | ||
| { error: "Internal server error" }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.