diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 935ab67..3db3773 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,8 +120,11 @@ jobs: - name: Generate image tag id: version run: | + # Always include short SHA so each build is immutable and cache-from can never + # cross-contaminate between commits. For PRs the format is pr-N-sha7; for main + # it is YYYY.MM.DD-sha7. if [ "${{ github.event_name }}" = "pull_request" ]; then - TAG="pr-${{ github.event.pull_request.number }}" + TAG="pr-${{ github.event.pull_request.number }}-${GITHUB_SHA::7}" else TAG="$(date -u +%Y.%m.%d)-${GITHUB_SHA::7}" fi @@ -207,7 +210,7 @@ jobs: - name: Deploy to groombook-dev env: - TAG: pr-${{ github.event.pull_request.number }} + TAG: pr-${{ github.event.pull_request.number }}-${{ github.sha::7}} PR_NUM: ${{ github.event.pull_request.number }} run: | echo "Deploying images tagged $TAG to groombook-dev..." diff --git a/.gitignore b/.gitignore index b826df6..0b4a94f 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,10 @@ dist/ *.log .turbo/ coverage/ + +# Vite resolves .js before .tsx — don't accidentally commit transpiled output +apps/web/src/**/*.js +!apps/web/src/lib/*.js +!apps/web/src/portal/mockData.js +!apps/web/src/portal/sections/PetForm.js +!apps/web/src/test/setup.js diff --git a/apps/api/src/routes/staff.ts b/apps/api/src/routes/staff.ts index 0aa6b70..977a176 100644 --- a/apps/api/src/routes/staff.ts +++ b/apps/api/src/routes/staff.ts @@ -15,7 +15,38 @@ const createStaffSchema = z.object({ active: z.boolean().default(true), }); -const updateStaffSchema = createStaffSchema.partial().omit({ email: true }); +const updateStaffSchema = z.object({ + name: z.string().min(1).max(200).optional(), + role: z.enum(["groomer", "receptionist", "manager"]).optional(), + active: z.boolean().optional(), + isSuperUser: z.boolean().optional(), + oidcSub: z.string().optional(), +}); + +staffRouter.get("/me", async (c) => { + try { + const staffRow = c.get("staff"); + if (!staffRow) { + return c.json({ error: "Staff record not found in context" }, 500); + } + // Explicitly pick serializable fields to avoid BigInt/Date/undefined serialization issues + return c.json({ + id: staffRow.id, + name: staffRow.name, + email: staffRow.email, + role: staffRow.role, + active: staffRow.active, + isSuperUser: staffRow.isSuperUser, + userId: staffRow.userId, + oidcSub: staffRow.oidcSub, + createdAt: staffRow.createdAt, + updatedAt: staffRow.updatedAt, + }); + } catch (err) { + console.error("[/api/staff/me] error:", err, "staffRow:", c.get("staff")); + return c.json({ error: "Internal error", detail: String(err) }, 500); + } +}); staffRouter.get("/", async (c) => { const db = getDb(); @@ -45,11 +76,83 @@ staffRouter.post("/", zValidator("json", createStaffSchema), async (c) => { staffRouter.patch("/:id", zValidator("json", updateStaffSchema), async (c) => { const db = getDb(); + const currentStaff = c.get("staff"); const body = c.req.valid("json"); + const targetId = c.req.param("id"); + + // Only super users can change isSuperUser + if (body.isSuperUser !== undefined && !currentStaff.isSuperUser) { + return c.json({ error: "Forbidden: super user privileges required to modify super user status" }, 403); + } + + // Before revoking or deactivating the last super user, serialize access with a + // transaction + FOR UPDATE to prevent a race where two concurrent requests both + // pass the count check and leave zero super users. + const needsSuperUserGuard = body.isSuperUser === false || body.active === false; + if (needsSuperUserGuard) { + const [guardError, row] = await db.transaction(async (tx) => { + // Lock the target row so no other request can modify it concurrently + const [target] = await tx + .select({ isSuperUser: staff.isSuperUser }) + .from(staff) + .where(eq(staff.id, targetId)) + .limit(1) + .for("update"); + + if (!target) return ["Not found", null as (typeof staff.$inferSelect | null)]; + + // Only enforce guard if the target is actually a super user + const isRevokingSuperUser = body.isSuperUser === false && target.isSuperUser; + const isDeactivatingSuperUser = body.active === false && target.isSuperUser; + if (!isRevokingSuperUser && !isDeactivatingSuperUser) { + const [updated] = await tx + .update(staff) + .set({ ...body, updatedAt: new Date() }) + .where(eq(staff.id, targetId)) + .returning(); + return [null, updated]; + } + + // Count active super users (excluding target — it will be changed) + const superUserCount = await tx + .select({ id: staff.id }) + .from(staff) + .where(and(eq(staff.isSuperUser, true), eq(staff.active, true), ne(staff.id, targetId))) + .limit(2); + + if (superUserCount.length < 1) { + return [ + body.isSuperUser === false + ? "Cannot revoke the last super user. Assign another super user first." + : "Cannot deactivate the last super user. Assign another super user first.", + null, + ]; + } + + // Perform the update (outside the count query but still in the transaction) + await tx + .update(staff) + .set({ ...body, updatedAt: new Date() }) + .where(eq(staff.id, targetId)); + + // Re-select to get the post-update state (avoids FOR UPDATE + RETURNING issues in some DB drivers) + const [updated] = await tx + .select() + .from(staff) + .where(eq(staff.id, targetId)) + .limit(1); + return [null, updated]; + }); + + if (guardError) return c.json({ error: guardError }, guardError === "Not found" ? 404 : 400); + if (!row) return c.json({ error: "Not found" }, 404); + return c.json(row); + } + const [row] = await db .update(staff) .set({ ...body, updatedAt: new Date() }) - .where(eq(staff.id, c.req.param("id"))) + .where(eq(staff.id, targetId)) .returning(); if (!row) return c.json({ error: "Not found" }, 404); return c.json(row); @@ -81,11 +184,34 @@ staffRouter.delete("/:id", async (c) => { ); } - const [row] = await db - .delete(staff) - .where(eq(staff.id, id)) - .returning(); - if (!row) return c.json({ error: "Not found" }, 404); + // Prevent deleting the last super user — use transaction to avoid race + const [guardError] = await db.transaction(async (tx) => { + const [targetStaff] = await tx + .select({ isSuperUser: staff.isSuperUser }) + .from(staff) + .where(eq(staff.id, id)) + .limit(1) + .for("update"); + + if (!targetStaff) return ["Not found", null]; + + if (targetStaff.isSuperUser) { + const superUserCount = await tx + .select({ id: staff.id }) + .from(staff) + .where(and(eq(staff.isSuperUser, true), eq(staff.active, true), ne(staff.id, id))) + .limit(2); + if (superUserCount.length < 1) { + return ["Cannot delete the last super user. Assign another super user first.", null]; + } + } + + const [row] = await tx.delete(staff).where(eq(staff.id, id)).returning(); + return [null, row]; + }); + + if (guardError === "Not found") return c.json({ error: "Not found" }, 404); + if (guardError) return c.json({ error: guardError }, 400); return c.json({ ok: true }); }); diff --git a/apps/web/src/pages/Staff.tsx b/apps/web/src/pages/Staff.tsx index aac23a7..ae08a93 100644 --- a/apps/web/src/pages/Staff.tsx +++ b/apps/web/src/pages/Staff.tsx @@ -7,10 +7,11 @@ interface StaffForm { role: "groomer" | "receptionist" | "manager"; } -const EMPTY_FORM: StaffForm = { name: "", email: "", role: "groomer" }; +const EMPTY_FORM: StaffForm = { name: "", email: "", role: "groomer" }; // GRO-206 rebuild trigger export function StaffPage() { const [staff, setStaff] = useState([]); + const [currentUser, setCurrentUser] = useState(null); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [editing, setEditing] = useState(null); @@ -18,6 +19,7 @@ export function StaffPage() { const [form, setForm] = useState(EMPTY_FORM); const [formError, setFormError] = useState(null); const [saving, setSaving] = useState(false); + const [toggleError, setToggleError] = useState(null); async function load() { const r = await fetch("/api/staff?includeInactive=true"); @@ -25,8 +27,15 @@ export function StaffPage() { setStaff((await r.json()) as Staff[]); } + async function loadCurrentUser() { + const r = await fetch("/api/staff/me"); + if (r.ok) { + setCurrentUser((await r.json()) as Staff); + } + } + useEffect(() => { - load() + Promise.all([load(), loadCurrentUser()]) .catch((e: unknown) => setError(e instanceof Error ? e.message : "Unknown error")) .finally(() => setLoading(false)); }, []); @@ -71,6 +80,26 @@ export function StaffPage() { await load(); } + async function toggleSuperUser(s: Staff) { + setToggleError(null); + const res = await fetch(`/api/staff/${s.id}`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ isSuperUser: !s.isSuperUser }), + }); + if (!res.ok) { + const err = (await res.json()) as { error?: string }; + setToggleError(err.error ?? `HTTP ${res.status}`); + return; + } + await load(); + await loadCurrentUser(); + } + + const isCurrentSuperUser = currentUser?.isSuperUser ?? false; + const superUserCount = staff.filter((s) => s.isSuperUser && s.active).length; + const isLastSuperUser = (s: Staff) => s.isSuperUser && superUserCount === 1 && s.active; + if (loading) return

Loading…

; if (error) return

Error: {error}

; @@ -90,7 +119,7 @@ export function StaffPage() { - {["Name", "Email", "Role", "Status", ""].map((h) => ( + {["Name", "Email", "Role", ...(isCurrentSuperUser ? ["Super User"] : []), "Status", ""].map((h) => ( ))} @@ -101,12 +130,31 @@ export function StaffPage() { + {isCurrentSuperUser && ( + + )} @@ -117,6 +165,10 @@ export function StaffPage() { )} + {toggleError && ( +

{toggleError}

+ )} + {showForm && (
{h}
{s.name} {s.email} {s.role} + {s.isSuperUser ? ( + Super User + ) : ( + + )} + {s.active ? "Active" : "Inactive"} + {isCurrentSuperUser && s.id !== currentUser?.id && ( + + )}