fix(api): resolve /me 500 and revoke not persisting (GRO-206) #161
@@ -120,8 +120,11 @@ jobs:
|
|||||||
- name: Generate image tag
|
- name: Generate image tag
|
||||||
id: version
|
id: version
|
||||||
run: |
|
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
|
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
|
else
|
||||||
TAG="$(date -u +%Y.%m.%d)-${GITHUB_SHA::7}"
|
TAG="$(date -u +%Y.%m.%d)-${GITHUB_SHA::7}"
|
||||||
fi
|
fi
|
||||||
@@ -207,7 +210,7 @@ jobs:
|
|||||||
|
|
||||||
- name: Deploy to groombook-dev
|
- name: Deploy to groombook-dev
|
||||||
env:
|
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 }}
|
PR_NUM: ${{ github.event.pull_request.number }}
|
||||||
run: |
|
run: |
|
||||||
echo "Deploying images tagged $TAG to groombook-dev..."
|
echo "Deploying images tagged $TAG to groombook-dev..."
|
||||||
|
|||||||
@@ -7,3 +7,10 @@ dist/
|
|||||||
*.log
|
*.log
|
||||||
.turbo/
|
.turbo/
|
||||||
coverage/
|
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
|
||||||
|
|||||||
@@ -15,7 +15,38 @@ const createStaffSchema = z.object({
|
|||||||
active: z.boolean().default(true),
|
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) => {
|
staffRouter.get("/", async (c) => {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
@@ -45,11 +76,83 @@ staffRouter.post("/", zValidator("json", createStaffSchema), async (c) => {
|
|||||||
|
|
||||||
staffRouter.patch("/:id", zValidator("json", updateStaffSchema), async (c) => {
|
staffRouter.patch("/:id", zValidator("json", updateStaffSchema), async (c) => {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
|
const currentStaff = c.get("staff");
|
||||||
const body = c.req.valid("json");
|
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
|
const [row] = await db
|
||||||
.update(staff)
|
.update(staff)
|
||||||
.set({ ...body, updatedAt: new Date() })
|
.set({ ...body, updatedAt: new Date() })
|
||||||
.where(eq(staff.id, c.req.param("id")))
|
.where(eq(staff.id, targetId))
|
||||||
.returning();
|
.returning();
|
||||||
if (!row) return c.json({ error: "Not found" }, 404);
|
if (!row) return c.json({ error: "Not found" }, 404);
|
||||||
return c.json(row);
|
return c.json(row);
|
||||||
@@ -81,11 +184,34 @@ staffRouter.delete("/:id", async (c) => {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
const [row] = await db
|
// Prevent deleting the last super user — use transaction to avoid race
|
||||||
.delete(staff)
|
const [guardError] = await db.transaction(async (tx) => {
|
||||||
.where(eq(staff.id, id))
|
const [targetStaff] = await tx
|
||||||
.returning();
|
.select({ isSuperUser: staff.isSuperUser })
|
||||||
if (!row) return c.json({ error: "Not found" }, 404);
|
.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 });
|
return c.json({ ok: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -7,10 +7,11 @@ interface StaffForm {
|
|||||||
role: "groomer" | "receptionist" | "manager";
|
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() {
|
export function StaffPage() {
|
||||||
const [staff, setStaff] = useState<Staff[]>([]);
|
const [staff, setStaff] = useState<Staff[]>([]);
|
||||||
|
const [currentUser, setCurrentUser] = useState<Staff | null>(null);
|
||||||
const [loading, setLoading] = useState(true);
|
const [loading, setLoading] = useState(true);
|
||||||
const [error, setError] = useState<string | null>(null);
|
const [error, setError] = useState<string | null>(null);
|
||||||
const [editing, setEditing] = useState<Staff | null>(null);
|
const [editing, setEditing] = useState<Staff | null>(null);
|
||||||
@@ -18,6 +19,7 @@ export function StaffPage() {
|
|||||||
const [form, setForm] = useState<StaffForm>(EMPTY_FORM);
|
const [form, setForm] = useState<StaffForm>(EMPTY_FORM);
|
||||||
const [formError, setFormError] = useState<string | null>(null);
|
const [formError, setFormError] = useState<string | null>(null);
|
||||||
const [saving, setSaving] = useState(false);
|
const [saving, setSaving] = useState(false);
|
||||||
|
const [toggleError, setToggleError] = useState<string | null>(null);
|
||||||
|
|
||||||
async function load() {
|
async function load() {
|
||||||
const r = await fetch("/api/staff?includeInactive=true");
|
const r = await fetch("/api/staff?includeInactive=true");
|
||||||
@@ -25,8 +27,15 @@ export function StaffPage() {
|
|||||||
setStaff((await r.json()) as Staff[]);
|
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(() => {
|
useEffect(() => {
|
||||||
load()
|
Promise.all([load(), loadCurrentUser()])
|
||||||
.catch((e: unknown) => setError(e instanceof Error ? e.message : "Unknown error"))
|
.catch((e: unknown) => setError(e instanceof Error ? e.message : "Unknown error"))
|
||||||
.finally(() => setLoading(false));
|
.finally(() => setLoading(false));
|
||||||
}, []);
|
}, []);
|
||||||
@@ -71,6 +80,26 @@ export function StaffPage() {
|
|||||||
await load();
|
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 <p style={{ padding: "1rem" }}>Loading…</p>;
|
if (loading) return <p style={{ padding: "1rem" }}>Loading…</p>;
|
||||||
if (error) return <p style={{ padding: "1rem", color: "red" }}>Error: {error}</p>;
|
if (error) return <p style={{ padding: "1rem", color: "red" }}>Error: {error}</p>;
|
||||||
|
|
||||||
@@ -90,7 +119,7 @@ export function StaffPage() {
|
|||||||
<table style={{ width: "100%", borderCollapse: "collapse", fontSize: 14 }}>
|
<table style={{ width: "100%", borderCollapse: "collapse", fontSize: 14 }}>
|
||||||
<thead>
|
<thead>
|
||||||
<tr style={{ background: "#f8fafc" }}>
|
<tr style={{ background: "#f8fafc" }}>
|
||||||
{["Name", "Email", "Role", "Status", ""].map((h) => (
|
{["Name", "Email", "Role", ...(isCurrentSuperUser ? ["Super User"] : []), "Status", ""].map((h) => (
|
||||||
<th key={h} style={{ textAlign: "left", padding: "0.55rem 0.75rem", borderBottom: "1px solid #e5e7eb", fontSize: 11, fontWeight: 600, color: "#6b7280", textTransform: "uppercase", letterSpacing: "0.04em" }}>{h}</th>
|
<th key={h} style={{ textAlign: "left", padding: "0.55rem 0.75rem", borderBottom: "1px solid #e5e7eb", fontSize: 11, fontWeight: 600, color: "#6b7280", textTransform: "uppercase", letterSpacing: "0.04em" }}>{h}</th>
|
||||||
))}
|
))}
|
||||||
</tr>
|
</tr>
|
||||||
@@ -101,12 +130,31 @@ export function StaffPage() {
|
|||||||
<td style={tdStyle}>{s.name}</td>
|
<td style={tdStyle}>{s.name}</td>
|
||||||
<td style={tdStyle}>{s.email}</td>
|
<td style={tdStyle}>{s.email}</td>
|
||||||
<td style={tdStyle}><span style={{ textTransform: "capitalize" }}>{s.role}</span></td>
|
<td style={tdStyle}><span style={{ textTransform: "capitalize" }}>{s.role}</span></td>
|
||||||
|
{isCurrentSuperUser && (
|
||||||
|
<td style={tdStyle}>
|
||||||
|
{s.isSuperUser ? (
|
||||||
|
<span style={{ padding: "2px 8px", borderRadius: 12, fontSize: 11, fontWeight: 600, background: "#ede9fe", color: "#5b21b6" }}>Super User</span>
|
||||||
|
) : (
|
||||||
|
<span style={{ padding: "2px 8px", borderRadius: 12, fontSize: 11, fontWeight: 600, background: "#f3f4f6", color: "#6b7280" }}>—</span>
|
||||||
|
)}
|
||||||
|
</td>
|
||||||
|
)}
|
||||||
<td style={tdStyle}>
|
<td style={tdStyle}>
|
||||||
<span style={{ padding: "2px 8px", borderRadius: 12, fontSize: 11, fontWeight: 600, background: s.active ? "#d1fae5" : "#f3f4f6", color: s.active ? "#065f46" : "#6b7280" }}>
|
<span style={{ padding: "2px 8px", borderRadius: 12, fontSize: 11, fontWeight: 600, background: s.active ? "#d1fae5" : "#f3f4f6", color: s.active ? "#065f46" : "#6b7280" }}>
|
||||||
{s.active ? "Active" : "Inactive"}
|
{s.active ? "Active" : "Inactive"}
|
||||||
</span>
|
</span>
|
||||||
</td>
|
</td>
|
||||||
<td style={{ ...tdStyle, whiteSpace: "nowrap" }}>
|
<td style={{ ...tdStyle, whiteSpace: "nowrap" }}>
|
||||||
|
{isCurrentSuperUser && s.id !== currentUser?.id && (
|
||||||
|
<button
|
||||||
|
onClick={() => toggleSuperUser(s)}
|
||||||
|
disabled={isLastSuperUser(s)}
|
||||||
|
title={isLastSuperUser(s) ? "Cannot revoke the last super user" : undefined}
|
||||||
|
style={{ ...btnStyle, marginRight: "0.4rem", ...(isLastSuperUser(s) ? { opacity: 0.5, cursor: "not-allowed" } : {}) }}
|
||||||
|
>
|
||||||
|
{s.isSuperUser ? "Revoke" : "Grant"}
|
||||||
|
</button>
|
||||||
|
)}
|
||||||
<button onClick={() => openEdit(s)} style={{ ...btnStyle, marginRight: "0.4rem" }}>Edit</button>
|
<button onClick={() => openEdit(s)} style={{ ...btnStyle, marginRight: "0.4rem" }}>Edit</button>
|
||||||
<button onClick={() => toggleActive(s)} style={btnStyle}>{s.active ? "Deactivate" : "Activate"}</button>
|
<button onClick={() => toggleActive(s)} style={btnStyle}>{s.active ? "Deactivate" : "Activate"}</button>
|
||||||
</td>
|
</td>
|
||||||
@@ -117,6 +165,10 @@ export function StaffPage() {
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
{toggleError && (
|
||||||
|
<p style={{ color: "red", margin: "0.75rem 0 0", fontSize: 14 }}>{toggleError}</p>
|
||||||
|
)}
|
||||||
|
|
||||||
{showForm && (
|
{showForm && (
|
||||||
<div
|
<div
|
||||||
style={{ position: "fixed", inset: 0, background: "rgba(0,0,0,0.45)", display: "flex", alignItems: "center", justifyContent: "center", zIndex: 100 }}
|
style={{ position: "fixed", inset: 0, background: "rgba(0,0,0,0.45)", display: "flex", alignItems: "center", justifyContent: "center", zIndex: 100 }}
|
||||||
|
|||||||
@@ -72,6 +72,7 @@ export interface Staff {
|
|||||||
name: string;
|
name: string;
|
||||||
email: string;
|
email: string;
|
||||||
role: "groomer" | "receptionist" | "manager";
|
role: "groomer" | "receptionist" | "manager";
|
||||||
|
isSuperUser: boolean;
|
||||||
active: boolean;
|
active: boolean;
|
||||||
createdAt: string;
|
createdAt: string;
|
||||||
updatedAt: string;
|
updatedAt: string;
|
||||||
|
|||||||
Reference in New Issue
Block a user