From 94c26af92dbb5f45adf6cda108b9353ea2905593 Mon Sep 17 00:00:00 2001 From: aswin Date: Mon, 29 Jun 2026 13:24:54 +0530 Subject: [PATCH] Fix silent deletes: 204 handling, batch-delete guard, mutation feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a batch did nothing visible — no refresh, no message. Root causes: - api.js called res.json() on every OK response, including 204 No Content (DELETE), throwing "Unexpected end of JSON input". The thrown error skipped the list reload, so the item stayed on screen. Return null on 204. - delete_batch did a raw db.delete + commit; Batch has no cascade and the FKs have no ondelete, so deleting a batch with students/sessions/payments raised a Postgres FK error -> HTTP 500. Now block with a clean 409 naming the blockers (active enrollments / sessions / payments), and sweep the dead soft-deleted join rows left by unenroll before deleting. - No mutation button gave feedback. Wrap delete/remove/unenroll/deactivate/ disable handlers in try/catch with success/error toasts (the toast system already existed and was used elsewhere). Verified end-to-end via Docker + Playwright: block path -> 409 + error toast + card stays; success path -> 204 + success toast + card removed; create flows unregressed (8/8). Co-Authored-By: Claude Opus 4.8 --- backend/app/routers/batches.py | 24 ++++++++++++++++++++++- frontend/src/api.js | 1 + frontend/src/pages/Batches.jsx | 29 ++++++++++++++++++++++------ frontend/src/pages/StudentDetail.jsx | 22 ++++++++++++++++----- frontend/src/pages/Tutors.jsx | 11 +++++++++-- frontend/src/pages/Users.jsx | 11 +++++++++-- 6 files changed, 82 insertions(+), 16 deletions(-) diff --git a/backend/app/routers/batches.py b/backend/app/routers/batches.py index a501211..4853ead 100644 --- a/backend/app/routers/batches.py +++ b/backend/app/routers/batches.py @@ -5,7 +5,7 @@ from ..database import get_db from ..deps import require_staff -from ..models import Batch, BatchEnrollment, Student +from ..models import Batch, BatchEnrollment, Payment, Session, Student from ..schemas import BatchCreate, BatchOut, NameRef, StudentOut router = APIRouter(prefix="/batches", tags=["batches"]) @@ -70,6 +70,28 @@ def update_batch( @router.delete("/{batch_id}", status_code=204) def delete_batch(batch_id: int, db: Session = Depends(get_db), _=Depends(require_staff)): batch = _get(db, batch_id) + # Block deletes that would orphan real data (FK has no cascade, so the commit + # would otherwise 500). Only *active* enrollments count as "in use" — unenroll + # is a soft delete, so removed students leave an inactive row behind that we + # clean up below rather than block on. + blockers = [] + n = db.query(BatchEnrollment).filter_by(batch_id=batch_id, is_active=True).count() + if n: + blockers.append(f"{n} enrolled student(s)") + n = db.query(Session).filter_by(batch_id=batch_id).count() + if n: + blockers.append(f"{n} session(s)") + n = db.query(Payment).filter_by(batch_id=batch_id).count() + if n: + blockers.append(f"{n} payment(s)") + if blockers: + raise HTTPException( + status_code=409, + detail="Can't delete: batch still has " + ", ".join(blockers) + ". Remove them first.", + ) + # Drop the dead join rows left by unenroll — they only hold the FK and have no + # meaning once the batch is gone. + db.query(BatchEnrollment).filter_by(batch_id=batch_id).delete() db.delete(batch) db.commit() diff --git a/frontend/src/api.js b/frontend/src/api.js index 1978ca9..5fd26d5 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -32,6 +32,7 @@ async function request(path, { method = "GET", body, form } = {}) { const detail = await res.json().catch(() => ({})); throw new Error(detail.detail || `Request failed (${res.status})`); } + if (res.status === 204) return null; // No Content (e.g. DELETE) — nothing to parse const ct = res.headers.get("content-type") || ""; return ct.includes("application/json") ? res.json() : res.text(); } diff --git a/frontend/src/pages/Batches.jsx b/frontend/src/pages/Batches.jsx index d7384a8..edc11a5 100644 --- a/frontend/src/pages/Batches.jsx +++ b/frontend/src/pages/Batches.jsx @@ -1,26 +1,43 @@ import { useState } from "react"; import { api } from "../api"; import { Page, Card, EmptyState, Stagger, inr, useApi } from "../ui"; +import { useToast } from "../components/Toast"; function BatchCard({ batch, allStudents, onEdit, onChanged }) { // Roster comes from the /batches list response (no per-card fetch); onChanged // reloads the list, which refreshes this batch's students. const roster = batch.students || []; + const toast = useToast(); async function add(e) { const sid = Number(e.target.value); if (!sid) return; - await api.post("/students/enroll", { student_id: sid, batch_id: batch.id }); - onChanged(); + try { + await api.post("/students/enroll", { student_id: sid, batch_id: batch.id }); + toast.success("Student added."); + onChanged(); + } catch (err) { + toast.error(err.message); + } } async function remove(sid) { - await api.post("/students/unenroll", { student_id: sid, batch_id: batch.id }); - onChanged(); + try { + await api.post("/students/unenroll", { student_id: sid, batch_id: batch.id }); + toast.success("Student removed."); + onChanged(); + } catch (err) { + toast.error(err.message); + } } async function del() { if (!confirm(`Delete batch "${batch.name}"?`)) return; - await api.del(`/batches/${batch.id}`); - onChanged(); + try { + await api.del(`/batches/${batch.id}`); + toast.success("Batch deleted."); + onChanged(); + } catch (err) { + toast.error(err.message); + } } const enrolledIds = new Set(roster.map((s) => s.id)); diff --git a/frontend/src/pages/StudentDetail.jsx b/frontend/src/pages/StudentDetail.jsx index 1ab6a8e..f817e6e 100644 --- a/frontend/src/pages/StudentDetail.jsx +++ b/frontend/src/pages/StudentDetail.jsx @@ -2,6 +2,7 @@ import { useState } from "react"; import { Link, useParams } from "react-router-dom"; import { api } from "../api"; import { Page, Table, Loading, useApi } from "../ui"; +import { useToast } from "../components/Toast"; const MARK = { present: { icon: "✓", word: "", cls: "text-sage" }, @@ -87,6 +88,7 @@ export default function StudentDetail() { const calendar = useApi(() => api.get(`/students/${id}/attendance-calendar`), [id]); const [batchId, setBatchId] = useState(""); const [form, setForm] = useState(null); // edit student details + const toast = useToast(); async function save(e) { e.preventDefault(); @@ -103,13 +105,23 @@ export default function StudentDetail() { async function enroll() { if (!batchId) return; - await api.post("/students/enroll", { student_id: Number(id), batch_id: Number(batchId) }); - setBatchId(""); - enrolled.reload(); + try { + await api.post("/students/enroll", { student_id: Number(id), batch_id: Number(batchId) }); + setBatchId(""); + toast.success("Enrolled."); + enrolled.reload(); + } catch (err) { + toast.error(err.message); + } } async function unenroll(bid) { - await api.post("/students/unenroll", { student_id: Number(id), batch_id: bid }); - enrolled.reload(); + try { + await api.post("/students/unenroll", { student_id: Number(id), batch_id: bid }); + toast.success("Unenrolled."); + enrolled.reload(); + } catch (err) { + toast.error(err.message); + } } if (!student.data) return {student.error || "Loading…"}; diff --git a/frontend/src/pages/Tutors.jsx b/frontend/src/pages/Tutors.jsx index 82e8856..3b7dcb8 100644 --- a/frontend/src/pages/Tutors.jsx +++ b/frontend/src/pages/Tutors.jsx @@ -1,10 +1,12 @@ import { useState } from "react"; import { api } from "../api"; import { Page, EntityCard, EmptyState, Stagger, inr, useApi } from "../ui"; +import { useToast } from "../components/Toast"; export default function Tutors() { const list = useApi(() => api.get("/tutors")); const [form, setForm] = useState(null); + const toast = useToast(); async function save(e) { e.preventDefault(); @@ -22,8 +24,13 @@ export default function Tutors() { } async function deactivate(t) { - await api.post(`/tutors/${t.id}/deactivate`); - list.reload(); + try { + await api.post(`/tutors/${t.id}/deactivate`); + toast.success("Tutor deactivated."); + list.reload(); + } catch (err) { + toast.error(err.message); + } } return ( diff --git a/frontend/src/pages/Users.jsx b/frontend/src/pages/Users.jsx index 57a330a..ae53dbb 100644 --- a/frontend/src/pages/Users.jsx +++ b/frontend/src/pages/Users.jsx @@ -1,12 +1,14 @@ import { useState } from "react"; import { api } from "../api"; import { Page, Table, useApi } from "../ui"; +import { useToast } from "../components/Toast"; export default function Users() { const list = useApi(() => api.get("/users")); const students = useApi(() => api.get("/students")); const tutors = useApi(() => api.get("/tutors")); const [form, setForm] = useState(null); + const toast = useToast(); async function create(e) { e.preventDefault(); @@ -22,8 +24,13 @@ export default function Users() { } async function toggle(u) { - await api.post(`/users/${u.id}/${u.is_active ? "disable" : "enable"}`); - list.reload(); + try { + await api.post(`/users/${u.id}/${u.is_active ? "disable" : "enable"}`); + toast.success(u.is_active ? "User disabled." : "User enabled."); + list.reload(); + } catch (err) { + toast.error(err.message); + } } return (