feat: customer-facing appointment notes (GRO-106) (#109)
* feat: add customer-facing appointment notes (GRO-106) - Migration 0014: add customer_notes column to appointments - Schema update: add customerNotes field to appointments table - Factory update: include customerNotes in buildAppointment - Portal route: PATCH /api/portal/appointments/:id/notes - Ownership validation via impersonation session - Future-only validation (no edits after start) - 500 character limit - Register portal router in index.ts Co-Authored-By: Paperclip <noreply@paperclip.ing> * Fix confirmationToken leak and add unit tests for portal notes endpoint - Return only id, customerNotes, updatedAt instead of full appointment row - Add comprehensive unit tests covering auth, ownership, time-gating, and validation - Fix: confirmationToken no longer returned to portal session Co-Authored-By: Paperclip <noreply@paperclip.ing> * feat: add customer notes UI to portal and staff views (GRO-178) - Add customerNotes field to Appointment type - Add read-only customer notes display in staff appointment detail modal - Add customer notes textarea with save, char counter (500 max), and disabled state - Wire up PATCH /api/portal/appointments/:id/notes in portal UI - Update mockData with customerNotes field Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix: address QA review feedback - null check and portal route auth - Add null check after db.update().returning() in portal notes endpoint - Move portal router registration before auth middleware so clients can access it - Remove unused ENDED_SESSION variable from test file Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix(portal): address QA review - isUpcoming time parsing and session header - Fixed parseTimeTo24Hour to handle 12-hour AM/PM format correctly - Added X-Impersonation-Session-Id header to CustomerNotesSection fetch - Added comprehensive tests for CustomerNotesSection and time parsing - Fixed TypeScript strict null checks for parseTimeTo24Hour Fixes QA review issues: - isUpcoming() now correctly parses 12-hour time format - CustomerNotesSection sends session ID header for auth - Added unit tests for new UI component Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix: thread sessionId as prop instead of sessionStorage CustomerNotesSection was reading sessionStorage for the impersonation session ID, but CustomerPortal stores it in React state. Pass sessionId as a prop through AppointmentsSection and AppointmentCard instead. Also update tests to pass sessionId prop and add test for null sessionId case. Co-Authored-By: Paperclip <noreply@paperclip.ing> --------- Co-authored-by: Scrubs McBarkley <scrubs@groombook.app> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: groombook-cto[bot] <269737991+groombook-cto[bot]@users.noreply.github.com>
This commit was merged in pull request #109.
This commit is contained in:
committed by
GitHub
parent
553fa435ed
commit
d0b4baf5aa
@@ -0,0 +1,249 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { Hono } from "hono";
|
||||
|
||||
const CLIENT_ID = "550e8400-e29b-41d4-a716-446655440001";
|
||||
const APPOINTMENT_ID = "660e8400-e29b-41d4-a716-446655440002";
|
||||
const SESSION_ID = "770e8400-e29b-41d4-a716-446655440003";
|
||||
|
||||
const futureDate = () => new Date(Date.now() + 30 * 60 * 1000);
|
||||
const pastDate = () => new Date(Date.now() - 5 * 60 * 1000);
|
||||
|
||||
const ACTIVE_SESSION = {
|
||||
id: SESSION_ID,
|
||||
clientId: CLIENT_ID,
|
||||
status: "active" as const,
|
||||
expiresAt: futureDate(),
|
||||
createdAt: new Date(),
|
||||
};
|
||||
|
||||
const EXPIRED_SESSION = {
|
||||
id: SESSION_ID,
|
||||
clientId: CLIENT_ID,
|
||||
status: "active" as const,
|
||||
expiresAt: pastDate(),
|
||||
createdAt: new Date(),
|
||||
};
|
||||
|
||||
const APPOINTMENT = {
|
||||
id: APPOINTMENT_ID,
|
||||
clientId: CLIENT_ID,
|
||||
startTime: futureDate(),
|
||||
endTime: futureDate(),
|
||||
customerNotes: null,
|
||||
confirmationToken: "secret-token-leak-test",
|
||||
};
|
||||
|
||||
let selectSessionRow: Record<string, unknown> | null = null;
|
||||
let selectAppointmentRow: Record<string, unknown> | null = null;
|
||||
let updatedValues: Record<string, unknown>[] = [];
|
||||
|
||||
function resetMock() {
|
||||
selectSessionRow = null;
|
||||
selectAppointmentRow = null;
|
||||
updatedValues = [];
|
||||
}
|
||||
|
||||
vi.mock("@groombook/db", () => {
|
||||
function makeChainable(data: unknown[]): unknown {
|
||||
const arr = [...data];
|
||||
const chain = new Proxy(arr, {
|
||||
get(target, prop) {
|
||||
if (prop === "where" || prop === "orderBy" || prop === "limit") {
|
||||
return () => chain;
|
||||
}
|
||||
// @ts-expect-error proxy
|
||||
return target[prop];
|
||||
},
|
||||
});
|
||||
return chain;
|
||||
}
|
||||
|
||||
const impersonationSessions = new Proxy(
|
||||
{ _name: "impersonationSessions" },
|
||||
{ get: (t, p) => (p === "_name" ? "impersonationSessions" : { table: "impersonationSessions", column: p }) }
|
||||
);
|
||||
|
||||
const appointments = new Proxy(
|
||||
{ _name: "appointments" },
|
||||
{ get: (t, p) => (p === "_name" ? "appointments" : { table: "appointments", column: p }) }
|
||||
);
|
||||
|
||||
return {
|
||||
getDb: () => ({
|
||||
select: () => ({
|
||||
from: (table: { _name: string }) => {
|
||||
if (table._name === "impersonationSessions") {
|
||||
return makeChainable(selectSessionRow ? [selectSessionRow] : []);
|
||||
}
|
||||
if (table._name === "appointments") {
|
||||
return makeChainable(selectAppointmentRow ? [selectAppointmentRow] : []);
|
||||
}
|
||||
return makeChainable([]);
|
||||
},
|
||||
}),
|
||||
update: () => ({
|
||||
set: (vals: Record<string, unknown>) => ({
|
||||
where: () => ({
|
||||
returning: () => {
|
||||
if (selectAppointmentRow) {
|
||||
const updated = { ...selectAppointmentRow, ...vals };
|
||||
updatedValues.push(vals);
|
||||
return [updated];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
impersonationSessions,
|
||||
appointments,
|
||||
eq: vi.fn(),
|
||||
and: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
const { portalRouter } = await import("../routes/portal.js");
|
||||
|
||||
const app = new Hono();
|
||||
app.route("/portal", portalRouter);
|
||||
|
||||
function jsonPatch(path: string, body: unknown, headers?: Record<string, string>) {
|
||||
return app.request(path, {
|
||||
method: "PATCH",
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
...headers,
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => resetMock());
|
||||
|
||||
describe("PATCH /portal/appointments/:id/notes", () => {
|
||||
it("returns updated appointment with safe fields only", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT };
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: "Please be gentle with Fido" },
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
const body = await res.json();
|
||||
expect(body).toHaveProperty("id");
|
||||
expect(body).toHaveProperty("customerNotes", "Please be gentle with Fido");
|
||||
expect(body).toHaveProperty("updatedAt");
|
||||
expect(body).not.toHaveProperty("confirmationToken");
|
||||
expect(body).not.toHaveProperty("clientId");
|
||||
});
|
||||
|
||||
it("returns 401 without X-Impersonation-Session-Id header", async () => {
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: "Test note" }
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
const body = await res.json();
|
||||
expect(body.error).toBe("Unauthorized");
|
||||
});
|
||||
|
||||
it("returns 401 with expired session", async () => {
|
||||
selectSessionRow = EXPIRED_SESSION;
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: "Test note" },
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
const body = await res.json();
|
||||
expect(body.error).toBe("Unauthorized");
|
||||
});
|
||||
|
||||
it("returns 401 with ended session", async () => {
|
||||
selectSessionRow = null;
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: "Test note" },
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
const body = await res.json();
|
||||
expect(body.error).toBe("Unauthorized");
|
||||
});
|
||||
|
||||
it("returns 403 when appointment belongs to different client", async () => {
|
||||
selectSessionRow = { ...ACTIVE_SESSION, clientId: "different-client-id" };
|
||||
selectAppointmentRow = { ...APPOINTMENT };
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: "Test note" },
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(403);
|
||||
const body = await res.json();
|
||||
expect(body.error).toBe("Forbidden");
|
||||
});
|
||||
|
||||
it("returns 422 for past appointment", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, startTime: pastDate() };
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: "Test note" },
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/past|in-progress|cannot edit/i);
|
||||
});
|
||||
|
||||
it("returns 422 when appointment is in progress", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT, startTime: new Date(Date.now() - 2 * 60 * 1000) };
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: "Test note" },
|
||||
{ "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 jsonPatch(
|
||||
`/portal/appointments/nonexistent-id/notes`,
|
||||
{ customerNotes: "Test note" },
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it("accepts notes at exactly 500 characters", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT };
|
||||
const longNote = "a".repeat(500);
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: longNote },
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
const body = await res.json();
|
||||
expect(body.customerNotes).toBe(longNote);
|
||||
});
|
||||
|
||||
it("rejects notes exceeding 500 characters", async () => {
|
||||
selectSessionRow = ACTIVE_SESSION;
|
||||
selectAppointmentRow = { ...APPOINTMENT };
|
||||
const longNote = "a".repeat(501);
|
||||
const res = await jsonPatch(
|
||||
`/portal/appointments/${APPOINTMENT_ID}/notes`,
|
||||
{ customerNotes: longNote },
|
||||
{ "X-Impersonation-Session-Id": SESSION_ID }
|
||||
);
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
});
|
||||
@@ -6,6 +6,7 @@ import { clientsRouter } from "./routes/clients.js";
|
||||
import { petsRouter } from "./routes/pets.js";
|
||||
import { servicesRouter } from "./routes/services.js";
|
||||
import { appointmentsRouter } from "./routes/appointments.js";
|
||||
import { portalRouter } from "./routes/portal.js";
|
||||
import { staffRouter } from "./routes/staff.js";
|
||||
import { invoicesRouter } from "./routes/invoices.js";
|
||||
import { bookRouter } from "./routes/book.js";
|
||||
@@ -56,6 +57,9 @@ app.get("/api/branding", async (c) => {
|
||||
});
|
||||
});
|
||||
|
||||
// Portal routes — no staff auth required, uses impersonation session for client auth
|
||||
app.route("/api/portal", portalRouter);
|
||||
|
||||
// Protected API routes
|
||||
const api = app.basePath("/api");
|
||||
api.use("*", authMiddleware);
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
import { Hono } from "hono";
|
||||
import { zValidator } from "@hono/zod-validator";
|
||||
import { z } from "zod";
|
||||
import { and, eq, getDb, appointments, impersonationSessions } from "@groombook/db";
|
||||
import type { AppEnv } from "../middleware/rbac.js";
|
||||
|
||||
export const portalRouter = new Hono<AppEnv>();
|
||||
|
||||
const customerNotesSchema = z.object({
|
||||
customerNotes: z.string().max(500),
|
||||
});
|
||||
|
||||
portalRouter.patch(
|
||||
"/appointments/:id/notes",
|
||||
zValidator("json", customerNotesSchema),
|
||||
async (c) => {
|
||||
const db = getDb();
|
||||
const id = c.req.param("id");
|
||||
const body = c.req.valid("json");
|
||||
|
||||
const sessionId = c.req.header("X-Impersonation-Session-Id");
|
||||
if (!sessionId) {
|
||||
return c.json({ error: "Unauthorized" }, 401);
|
||||
}
|
||||
|
||||
const [session] = await db
|
||||
.select()
|
||||
.from(impersonationSessions)
|
||||
.where(
|
||||
and(
|
||||
eq(impersonationSessions.id, sessionId),
|
||||
eq(impersonationSessions.status, "active")
|
||||
)
|
||||
)
|
||||
.limit(1);
|
||||
|
||||
if (!session || session.expiresAt <= new Date()) {
|
||||
return c.json({ error: "Unauthorized" }, 401);
|
||||
}
|
||||
|
||||
const authClientId = session.clientId;
|
||||
|
||||
const [appt] = await db
|
||||
.select()
|
||||
.from(appointments)
|
||||
.where(eq(appointments.id, id))
|
||||
.limit(1);
|
||||
|
||||
if (!appt) {
|
||||
return c.json({ error: "Not found" }, 404);
|
||||
}
|
||||
|
||||
if (appt.clientId !== authClientId) {
|
||||
return c.json({ error: "Forbidden" }, 403);
|
||||
}
|
||||
|
||||
if (appt.startTime <= new Date()) {
|
||||
return c.json({ error: "Cannot edit notes for past or in-progress appointments" }, 422);
|
||||
}
|
||||
|
||||
const [updated] = await db
|
||||
.update(appointments)
|
||||
.set({ customerNotes: body.customerNotes, updatedAt: new Date() })
|
||||
.where(eq(appointments.id, id))
|
||||
.returning();
|
||||
|
||||
if (!updated) {
|
||||
return c.json({ error: "Not found" }, 404);
|
||||
}
|
||||
|
||||
return c.json({
|
||||
id: updated.id,
|
||||
customerNotes: updated.customerNotes,
|
||||
updatedAt: updated.updatedAt,
|
||||
});
|
||||
}
|
||||
);
|
||||
Reference in New Issue
Block a user