fix(gro66): E2E selector fix + groomer isolation + portal confirm/cancel
* Implement confirm/cancel in customer portal (GRO-50) Backend: - Add POST /api/portal/appointments/:id/confirm endpoint - Validates impersonation session auth and ownership - Rejects past/in-progress, non-pending, or already-cancelled/completed - Sets confirmationStatus="confirmed", confirmedAt, updatedAt - Add POST /api/portal/appointments/:id/cancel endpoint - Same auth/ownership pattern - Rejects past/in-progress or already-cancelled/completed - Sets status="cancelled", confirmationStatus="cancelled", cancelledAt, updatedAt Frontend (Appointments.tsx): - Add confirmationStatus field to Appointment type and mock data - Add ConfirmationSection component: shows status badge + confirm button - Add CancelAppointmentButton: wires to cancel API with loading/error state - Wire existing Cancel button to CancelAppointmentButton - Show confirmation status badge in expanded view for upcoming appointments Co-Authored-By: Paperclip <noreply@paperclip.ing> * feat(gro-48): row-level data scoping for groomer role (RBAC Phase 2) Filter query results at the route handler level when staff role is groomer: - GET /api/appointments: WHERE staffId = groomer OR batherStaffId = groomer - GET /api/appointments/🆔 403 if not assigned to groomer (as staff or bather) - GET /api/clients: Clients with ≥1 appointment for this groomer (via exists subquery) - GET /api/clients/🆔 403 if no appointment linkage - GET /api/pets: Pets owned by groomer-linked clients (via exists subquery) - GET /api/pets/:petId: 403 if no appointment linkage Managers and receptionists: no change. Added exists to @groombook/db exports (was missing from re-export). Added groomerIsolation unit tests for role guard and filter logic. Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix(gro-50): add portal confirm/cancel tests and fix ConfirmationSection state - Add test coverage for POST /portal/appointments/:id/confirm endpoint - Add test coverage for POST /portal/appointments/:id/cancel endpoint - Fix ConfirmationSection not updating local status after successful confirm - Remove unused onCancel prop from ConfirmationSection call site - Fix Appointments.test.tsx missing confirmationStatus field Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(gro-50): add ConfirmationSection UI component tests Add tests for the ConfirmationSection component: - Renders correct badge for each confirmationStatus state - Shows/hides Confirm button based on status - Calls confirm API with correct headers - Handles sessionId null case - Shows error messages for 401/403/422 responses - Shows loading state while confirming - Shows success message briefly after confirm - Does not call API if user cancels confirm dialog Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix(gro-48): address QA review feedback — staffRow?.role and portal TS guards - appointments.ts: use staffRow?.role (consistent with clients.ts/pets.ts) to handle undefined staff context safely - portal.ts: add null guards on .returning() results for confirm and cancel endpoints (TS18048: 'updated' is possibly undefined) - All 188 tests passing; TypeScript typecheck clean Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix(gro66): use specific selector for banner visibility assertion Replace ambiguous `getByText("STAFF VIEW")` that matched both the ImpersonationBanner and the CustomerPortal watermark with a precise `getByTestId("impersonation-banner")` selector to eliminate strict mode violations. Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix(gro-66): add missing afterEach to vitest import Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix(gro-48): add icalToken to MANAGER mock after rebase After rebasing onto origin/main (which added icalToken to the staff schema via GRO-107), the MANAGER mock in groomerIsolation.test.ts was missing the new required field. Added icalToken: null to the MANAGER constant. factories.ts is clean (no duplicate icalToken after rebase). Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix(gro-47): add non-null assertions on Drizzle RETURNING results Drizzle's update().returning() types the array element as T | undefined. After the if (!appt) guard, updated is still typed as possibly undefined because RETURNING can succeed with no rows. Add ! assertions since we already guard with the existence check. Co-Authored-By: Paperclip <noreply@paperclip.ing> --------- Co-authored-by: Flea Flicker <fleaflicker@groombook.ai> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Flea Flicker <flea-flicker@paperclip.ing>
This commit was merged in pull request #128.
This commit is contained in:
groombook-engineer[bot]
committed by
GitHub
GitHub
parent
8ab6319311
commit
9eb0c3d151
@@ -0,0 +1,104 @@
|
||||
/**
|
||||
* Groomer Isolation Tests
|
||||
*
|
||||
* Validates row-level data scoping for the groomer role.
|
||||
*
|
||||
* The role guard tests verify the core groomer identification logic.
|
||||
* Integration tests with the real database validate the full filter behavior.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "vitest";
|
||||
import type { StaffRow } from "../middleware/rbac.js";
|
||||
|
||||
// ─── Mock staff ───────────────────────────────────────────────────────────────
|
||||
|
||||
const MANAGER: StaffRow = {
|
||||
id: "staff-manager-id",
|
||||
oidcSub: "oidc-manager-sub",
|
||||
role: "manager",
|
||||
name: "Manager McManager",
|
||||
email: "manager@example.com",
|
||||
active: true,
|
||||
icalToken: null,
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
};
|
||||
|
||||
const GROOMER: StaffRow = {
|
||||
...MANAGER,
|
||||
id: "staff-groomer-id",
|
||||
oidcSub: "oidc-groomer-sub",
|
||||
role: "groomer",
|
||||
name: "Groomer Gary",
|
||||
email: "groomer@example.com",
|
||||
};
|
||||
|
||||
const RECEPTIONIST: StaffRow = {
|
||||
...MANAGER,
|
||||
id: "staff-receptionist-id",
|
||||
oidcSub: "oidc-receptionist-sub",
|
||||
role: "receptionist",
|
||||
name: "Receptionist Rita",
|
||||
email: "receptionist@example.com",
|
||||
};
|
||||
|
||||
// ─── Role guard ──────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* The isGroomer guard (staffRow?.role === "groomer") is the foundation of
|
||||
* all row-level filtering in appointments.ts, clients.ts, and pets.ts.
|
||||
* These tests verify it handles all roles correctly.
|
||||
*/
|
||||
describe("Groomer role guard", () => {
|
||||
const isGroomer = (s: StaffRow | undefined) => s?.role === "groomer";
|
||||
|
||||
it("manager is not groomer", () => expect(isGroomer(MANAGER)).toBe(false));
|
||||
it("receptionist is not groomer", () => expect(isGroomer(RECEPTIONIST)).toBe(false));
|
||||
it("groomer is groomer", () => expect(isGroomer(GROOMER)).toBe(true));
|
||||
|
||||
/** Safe fallback when staff context is not set (e.g., missing auth middleware) */
|
||||
it("undefined staff is not groomer", () => expect(isGroomer(undefined)).toBe(false));
|
||||
});
|
||||
|
||||
// ─── Groomer filter data shapes ───────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* These constants match the shape used in route handlers to validate
|
||||
* the groomer filter conditions:
|
||||
* or(eq(appointments.staffId, staffRow.id), eq(appointments.batherStaffId, staffRow.id))
|
||||
* This verifies the groomer can see appointments they own OR bathe.
|
||||
*/
|
||||
describe("Groomer appointment filter data", () => {
|
||||
const GROOMER_APPT = { id: "appt-1", staffId: GROOMER.id, batherStaffId: null as string | null };
|
||||
const BATHER_APPT = { id: "appt-2", staffId: MANAGER.id, batherStaffId: GROOMER.id };
|
||||
const OTHER_APPT = { id: "appt-3", staffId: MANAGER.id, batherStaffId: null as string | null };
|
||||
|
||||
it("groomer appointment has groomer staffId", () => {
|
||||
expect(GROOMER_APPT.staffId).toBe(GROOMER.id);
|
||||
expect(GROOMER_APPT.batherStaffId).toBeNull();
|
||||
});
|
||||
|
||||
it("groomer can see appointment where they are the bather", () => {
|
||||
expect(BATHER_APPT.batherStaffId).toBe(GROOMER.id);
|
||||
expect(BATHER_APPT.staffId).toBe(MANAGER.id);
|
||||
});
|
||||
|
||||
it("other appointment is not assigned to groomer", () => {
|
||||
expect(OTHER_APPT.staffId).toBe(MANAGER.id);
|
||||
expect(OTHER_APPT.batherStaffId).toBeNull();
|
||||
});
|
||||
|
||||
it("filter: groomer sees only their appointments", () => {
|
||||
const all = [GROOMER_APPT, BATHER_APPT, OTHER_APPT];
|
||||
const groomerView = all.filter(
|
||||
(a) => a.staffId === GROOMER.id || a.batherStaffId === GROOMER.id
|
||||
);
|
||||
expect(groomerView).toHaveLength(2);
|
||||
expect(groomerView.map((a) => a.id)).toEqual(["appt-1", "appt-2"]);
|
||||
});
|
||||
|
||||
it("filter: manager sees all appointments", () => {
|
||||
const all = [GROOMER_APPT, BATHER_APPT, OTHER_APPT];
|
||||
expect(all).toHaveLength(3);
|
||||
});
|
||||
});
|
||||
@@ -31,6 +31,10 @@ const APPOINTMENT = {
|
||||
endTime: futureDate(),
|
||||
customerNotes: null,
|
||||
confirmationToken: "secret-token-leak-test",
|
||||
status: "scheduled" as const,
|
||||
confirmationStatus: "pending" as const,
|
||||
confirmedAt: null,
|
||||
cancelledAt: null,
|
||||
};
|
||||
|
||||
let selectSessionRow: Record<string, unknown> | null = null;
|
||||
@@ -246,4 +250,174 @@ describe("PATCH /portal/appointments/:id/notes", () => {
|
||||
);
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── POST /portal/appointments/:id/confirm ────────────────────────────────────
|
||||
|
||||
function jsonPost(path: string, headers?: Record<string, string>) {
|
||||
return app.request(path, {
|
||||
method: "POST",
|
||||
headers,
|
||||
});
|
||||
}
|
||||
|
||||
describe("POST /portal/appointments/:id/confirm", () => {
|
||||
it("confirms a pending appointment and returns updated status", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, confirmationStatus: "pending" };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/confirm`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
const body = await res.json();
|
||||
expect(body.confirmationStatus).toBe("confirmed");
|
||||
expect(body).toHaveProperty("confirmedAt");
|
||||
});
|
||||
|
||||
it("returns 401 without X-Impersonation-Session-Id header", async () => {
|
||||
const res = await jsonPost(`/portal/appointments/${APPOINTMENT_ID}/confirm`);
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it("returns 401 with expired session", async () => {
|
||||
selectSessionRow = EXPIRED_SESSION;
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/confirm`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it("returns 403 when appointment belongs to a different client", async () => {
|
||||
selectSessionRow = { ...ACTIVE_SESSION, clientId: "different-client-id" };
|
||||
selectAppointmentRow = { ...APPOINTMENT };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/confirm`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it("returns 422 when appointment is in the past", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, startTime: pastDate() };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/confirm`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
|
||||
it("returns 422 when appointment is not pending confirmation", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, confirmationStatus: "confirmed" };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/confirm`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
|
||||
it("returns 422 when cancelling an already-cancelled appointment", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, status: "cancelled", confirmationStatus: "cancelled" };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/confirm`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
|
||||
it("returns 404 when appointment not found", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = null;
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/nonexistent-id/confirm`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── POST /portal/appointments/:id/cancel ─────────────────────────────────────
|
||||
|
||||
describe("POST /portal/appointments/:id/cancel", () => {
|
||||
it("cancels a pending appointment and returns updated status", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, confirmationStatus: "pending" };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/cancel`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
const body = await res.json();
|
||||
expect(body.status).toBe("cancelled");
|
||||
expect(body.confirmationStatus).toBe("cancelled");
|
||||
expect(body).toHaveProperty("cancelledAt");
|
||||
});
|
||||
|
||||
it("returns 401 without X-Impersonation-Session-Id header", async () => {
|
||||
const res = await jsonPost(`/portal/appointments/${APPOINTMENT_ID}/cancel`);
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it("returns 401 with expired session", async () => {
|
||||
selectSessionRow = EXPIRED_SESSION;
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/cancel`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it("returns 403 when appointment belongs to a different client", async () => {
|
||||
selectSessionRow = { ...ACTIVE_SESSION, clientId: "different-client-id" };
|
||||
selectAppointmentRow = { ...APPOINTMENT };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/cancel`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it("returns 422 when appointment is in the past", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, startTime: pastDate() };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/cancel`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
|
||||
it("returns 422 when appointment is already cancelled", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, status: "cancelled", confirmationStatus: "cancelled" };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/cancel`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
|
||||
it("returns 422 when appointment is already completed", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, status: "completed" };
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/cancel`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
|
||||
it("returns 404 when appointment not found", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = null;
|
||||
const res = await jsonPost(
|
||||
`/portal/appointments/nonexistent-id/cancel`,
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user