fix(GRO-2180): normalize portal appointments API shape so /appointments loads

The /api/portal/appointments endpoint returns ISO startTime/endTime plus nested pet/service/staff objects, but the portal client Appointment type expected flat date/time/petName fields. isUpcoming() read appt.date/appt.time (both undefined), so parseTimeTo24Hour(undefined) threw a TypeError; the useEffect try/catch set the error state and the success-path-only Book New button became unreachable. - Add normalizeAppointment() at the fetch boundary mapping the API shape to the flat Appointment shape (derives display date/time from startTime, duration from the start/end delta), tolerant of the legacy flat shape. - Prefer absolute startTime in isUpcoming(); fall back to date/time. - Harden parseTimeTo24Hour against blank/undefined input (no NaN). - Add Appointment.startTime/endTime to the type. - Tests: normalizeAppointment + isUpcoming(startTime) + parseTimeTo24Hour safety. - Update UAT_PLAYBOOK.md §5.12.2 and new §5.12d regression cases. Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-08 04:18:35 +00:00
3 changed files with 186 additions and 131 deletions
@@ -182,13 +182,9 @@ export const { signIn, signOut, useSession, changePassword } = authClient;
 | # | Scenario | Steps | Expected |
 |---|----------|-------|----------|
 | TC-WEB-5.12.1 | Client-facing view | Log in as client persona | Customer portal UI displayed |
-| TC-WEB-5.12.2 | Appointment list loads | Sign in as `uat-customer@groombook.dev`, open **Appointments** | List of the customer's appointments renders. **No** "Failed to load appointments" error and **no** Retry button. (GRO-2180) |
-| TC-WEB-5.12.3 | Date/time display | Inspect each appointment card | Each card shows a human-readable date and time derived from the API `startTime` (e.g. "Mon, Jun 1, 2026" / "10:00 AM"); no `undefined` or blank date/time. (GRO-2180) |
-| TC-WEB-5.12.4 | Book New reachable | On the loaded Appointments view (non-readonly), look for the **Book New** button | "Book New" button is visible and opens the booking modal. (GRO-2180) |
-| TC-WEB-5.12.5 | Upcoming/Past split | Toggle the **Upcoming** and **Past** tabs | Future appointments appear under Upcoming; completed/cancelled/past appear under Past. (GRO-2180) |
-| TC-WEB-5.12.6 | Confirm appointment | Click confirm on pending appointment | Appointment status updated to confirmed |
-| TC-WEB-5.12.7 | Cancel appointment | Click cancel on appointment | Appointment marked as cancelled |
-| TC-WEB-5.12.8 | Reschedule display | Open **Reschedule** on an upcoming appointment | Summary header shows the current appointment's date and time (from `startTime`); no `undefined`. (GRO-2180) |
+| TC-WEB-5.12.2 | Appointment list | View client portal appointments | List of client's appointments visible — each card shows pet name, service, formatted date/time, and groomer (no "Failed to load appointments" error, no blank screen). "Book New" button is visible and clickable. See 5.12d. |
+| TC-WEB-5.12.3 | Confirm appointment | Click confirm on pending appointment | Appointment status updated to confirmed |
+| TC-WEB-5.12.4 | Cancel appointment | Click cancel on appointment | Appointment marked as cancelled |

 #### 5.12b Dynamic Portal Time Slots (GRO-1793, GRO-2105)

@@ -221,6 +217,26 @@ export const { signIn, signOut, useSession, changePassword } = authClient;
 | TC-WEB-5.12.16 | Badge status from data | Compare badge label to appointment.status field | Badge label matches the API appointment status exactly |
 | TC-WEB-5.12.17 | Unknown status fallback | Render badge with unknown status value | Badge renders with the raw status string as label and fallback CSS class |

+#### 5.12d Appointment API Shape Normalization (GRO-2180)
+
+| # | Scenario | Steps | Expected |
+|---|----------|-------|----------|
+| TC-WEB-5.12.18 | Portal appointments load (regression) | Sign in as `uat-customer@groombook.dev`, open `Appointments` | List renders without the "Failed to load appointments. Please try again." error; "Book New" button is visible and clickable |
+| TC-WEB-5.12.19 | Card fields populated from API | Inspect an appointment card | Pet name, service, formatted date (e.g. "Mon, Jun 1, 2026"), time (e.g. "10:00 AM"), and groomer name render — derived from the API's `startTime`/`endTime`/nested `pet`/`staff` objects |
+| TC-WEB-5.12.20 | Upcoming vs Past split | View both tabs | Future, non-cancelled/non-completed appointments appear under "Upcoming"; past/completed/cancelled under "Past" (classification uses absolute `startTime`) |
+| TC-WEB-5.12.21 | Reschedule from card | Expand an upcoming appointment, click Reschedule, pick a date | `GET /api/book/availability?serviceId=<appt.serviceId>&date=<picked>` fires with a non-empty `serviceId` (sourced from the API's nested `service.id`) |
+
+> **GRO-2180 regression note:** `/api/portal/appointments` returns ISO
+> `startTime`/`endTime` and nested `pet`/`service`/`staff` objects, but the portal
+> client `Appointment` type expected flat `date`/`time`/`petName` fields.
+> `isUpcoming()` read `appt.date`/`appt.time` (both `undefined`), so
+> `parseTimeTo24Hour(undefined)` threw `TypeError`, the `useEffect` `try/catch`
+> set the error state, and the "Book New" button (only rendered in the success
+> path) became unreachable. The fix normalizes the API response into the flat
+> `Appointment` shape at the fetch boundary (`normalizeAppointment`), prefers the
+> absolute `startTime` in `isUpcoming`, and hardens `parseTimeTo24Hour` against
+> blank/undefined input.
+
 ### 5.13 Reports UI

 | # | Scenario | Steps | Expected |
@@ -1,6 +1,6 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { render, screen, fireEvent, waitFor } from "@testing-library/react";
-import { parseTimeTo24Hour, isUpcoming, getAppointmentStart, formatAppointmentDate, formatAppointmentTime, CustomerNotesSection, ConfirmationSection, StatusBadge } from "../portal/sections/Appointments.tsx";
+import { parseTimeTo24Hour, isUpcoming, normalizeAppointment, CustomerNotesSection, ConfirmationSection, StatusBadge } from "../portal/sections/Appointments.tsx";

 const UPCOMING_APPT = {
  id: "appt-1",
@@ -29,26 +29,6 @@ const PAST_APPT = {
  status: "completed" as const,
 };

-// GRO-2180: the /api/portal/appointments contract returns ISO startTime/endTime
-// and no date/time fields. These fixtures mirror that shape exactly.
-const API_UPCOMING_APPT = {
-  id: "appt-api-1",
-  petId: "pet-1",
-  serviceId: "service-1",
-  groomerId: null,
-  startTime: "2099-01-01T10:00:00.000Z",
-  endTime: "2099-01-01T10:45:00.000Z",
-  status: "confirmed" as const,
-};
-
-const API_PAST_APPT = {
-  ...API_UPCOMING_APPT,
-  id: "appt-api-2",
-  startTime: "2020-01-01T10:00:00.000Z",
-  endTime: "2020-01-01T10:45:00.000Z",
-  status: "completed" as const,
-};
-
 describe("parseTimeTo24Hour", () => {
  it("converts AM times correctly", () => {
    expect(parseTimeTo24Hour("9:00 AM")).toBe("09:00:00");
@@ -63,14 +43,85 @@ describe("parseTimeTo24Hour", () => {
    expect(parseTimeTo24Hour("12:00 PM")).toBe("12:00:00");
  });

-  // GRO-2180 regression: must not throw on undefined/empty input.
-  it("returns a safe default for missing input", () => {
+  it("does not throw on undefined/null/empty input (GRO-2180)", () => {
    expect(() => parseTimeTo24Hour(undefined)).not.toThrow();
+    expect(() => parseTimeTo24Hour(null)).not.toThrow();
    expect(parseTimeTo24Hour(undefined)).toBe("00:00:00");
    expect(parseTimeTo24Hour("")).toBe("00:00:00");
  });
 });

+// GRO-2180: `/api/portal/appointments` returns ISO `startTime`/`endTime` + nested
+// pet/service/staff objects, not the flat date/time/petName shape the UI renders.
+describe("normalizeAppointment (API startTime shape — GRO-2180)", () => {
+  const RAW_API_APPT = {
+    id: "a0000001-0000-0000-0000-000000000001",
+    startTime: "2026-06-01T10:00:00.000Z",
+    endTime: "2026-06-01T10:45:00.000Z",
+    status: "completed" as const,
+    confirmationStatus: "confirmed" as const,
+    customerNotes: "Please be gentle",
+    notes: null,
+    pet: { id: "c0000001-0000-0000-0000-000000000001", name: "UAT Pup Alpha", photo: null },
+    service: { id: "b0000001-0000-0000-0000-000000000001", name: "Full Groom" },
+    staff: { id: "00000000-0000-0000-0000-000000000004", name: "UAT Staff Groomer" },
+  };
+
+  it("maps nested pet/service/staff and ISO startTime without throwing", () => {
+    const appt = normalizeAppointment(RAW_API_APPT);
+    expect(appt.id).toBe("a0000001-0000-0000-0000-000000000001");
+    expect(appt.petId).toBe("c0000001-0000-0000-0000-000000000001");
+    expect(appt.serviceId).toBe("b0000001-0000-0000-0000-000000000001");
+    expect(appt.groomerId).toBe("00000000-0000-0000-0000-000000000004");
+    expect(appt.petName).toBe("UAT Pup Alpha");
+    expect(appt.serviceName).toBe("Full Groom");
+    expect(appt.groomerName).toBe("UAT Staff Groomer");
+    expect(appt.startTime).toBe("2026-06-01T10:00:00.000Z");
+    expect(appt.customerNotes).toBe("Please be gentle");
+  });
+
+  it("derives duration in minutes from start/end delta", () => {
+    expect(normalizeAppointment(RAW_API_APPT).duration).toBe(45);
+  });
+
+  it("produces a date/time pair that does not crash isUpcoming or formatDate", () => {
+    const appt = normalizeAppointment(RAW_API_APPT);
+    expect(typeof appt.date).toBe("string");
+    expect(typeof appt.time).toBe("string");
+    expect(() => isUpcoming(appt)).not.toThrow();
+  });
+
+  it("classifies a past completed appointment as not upcoming", () => {
+    expect(isUpcoming(normalizeAppointment(RAW_API_APPT))).toBe(false);
+  });
+
+  it("classifies a future scheduled appointment as upcoming via startTime", () => {
+    const future = normalizeAppointment({
+      ...RAW_API_APPT,
+      startTime: "2099-01-01T10:00:00.000Z",
+      endTime: "2099-01-01T11:00:00.000Z",
+      status: "confirmed",
+    });
+    expect(isUpcoming(future)).toBe(true);
+  });
+
+  it("tolerates null nested objects without throwing", () => {
+    const appt = normalizeAppointment({
+      id: "a2",
+      startTime: "2099-01-01T10:00:00.000Z",
+      endTime: "2099-01-01T11:00:00.000Z",
+      status: "scheduled",
+      pet: null,
+      service: null,
+      staff: null,
+    });
+    expect(appt.petId).toBe("");
+    expect(appt.serviceId).toBe("");
+    expect(appt.groomerId).toBeNull();
+    expect(appt.petName).toBeUndefined();
+  });
+});
+
 describe("isUpcoming", () => {
  it("returns true for future confirmed appointments", () => {
    expect(isUpcoming(UPCOMING_APPT)).toBe(true);
@@ -87,63 +138,6 @@ describe("isUpcoming", () => {
  it("returns false for completed appointments", () => {
    expect(isUpcoming({ ...UPCOMING_APPT, status: "completed" })).toBe(false);
  });
-
-  // GRO-2180 regression: the API contract uses ISO startTime with no date/time.
-  // Previously isUpcoming threw a TypeError on this shape, breaking the page.
-  it("does not throw on the API startTime/endTime shape", () => {
-    expect(() => isUpcoming(API_UPCOMING_APPT)).not.toThrow();
-    expect(() => isUpcoming(API_PAST_APPT)).not.toThrow();
-  });
-
-  it("returns true for future appointments using startTime", () => {
-    expect(isUpcoming(API_UPCOMING_APPT)).toBe(true);
-  });
-
-  it("returns false for past appointments using startTime", () => {
-    expect(isUpcoming(API_PAST_APPT)).toBe(false);
-  });
-
-  it("returns false (not throw) when neither startTime nor date is present", () => {
-    const { startTime, endTime, ...noDate } = API_UPCOMING_APPT;
-    void startTime;
-    void endTime;
-    expect(() => isUpcoming(noDate)).not.toThrow();
-    expect(isUpcoming(noDate)).toBe(false);
-  });
-});
-
-describe("getAppointmentStart / display helpers (GRO-2180)", () => {
-  it("resolves the start instant from ISO startTime", () => {
-    const start = getAppointmentStart(API_UPCOMING_APPT);
-    expect(start).not.toBeNull();
-    expect(start?.toISOString()).toBe("2099-01-01T10:00:00.000Z");
-  });
-
-  it("falls back to legacy date + time when startTime is absent", () => {
-    const start = getAppointmentStart(UPCOMING_APPT);
-    expect(start).not.toBeNull();
-  });
-
-  it("returns null when there is no usable date", () => {
-    const { startTime, endTime, ...noDate } = API_UPCOMING_APPT;
-    void startTime;
-    void endTime;
-    expect(getAppointmentStart(noDate)).toBeNull();
-  });
-
-  it("formats date/time without throwing on the API shape", () => {
-    expect(() => formatAppointmentDate(API_UPCOMING_APPT)).not.toThrow();
-    expect(() => formatAppointmentTime(API_UPCOMING_APPT)).not.toThrow();
-    expect(formatAppointmentDate(API_UPCOMING_APPT)).not.toBe("");
-    expect(formatAppointmentTime(API_UPCOMING_APPT)).not.toBe("");
-  });
-
-  it("returns empty display strings when there is no usable date", () => {
-    const { startTime, endTime, ...noDate } = API_UPCOMING_APPT;
-    void startTime;
-    void endTime;
-    expect(formatAppointmentDate(noDate)).toBe("");
-  });
 });

 describe("CustomerNotesSection", () => {
@@ -36,14 +36,12 @@ export interface Appointment {
  petId: string;
  serviceId: string;
  groomerId: string | null;
-  // The /api/portal/appointments contract returns ISO `startTime`/`endTime`.
-  // `date`/`time` are the legacy display shape, still produced locally by some
-  // flows (e.g. test fixtures), so both shapes are optional and code reads
-  // `startTime` first, falling back to `date` + `time`.
+  // Absolute ISO instants as returned by `/api/portal/appointments`. `date`/`time`
+  // below are the locally-formatted display strings derived from `startTime`.
  startTime?: string;
  endTime?: string;
-  date?: string;
-  time?: string;
+  date: string;
+  time: string;
  status: 'scheduled' | 'confirmed' | 'pending' | 'waitlisted' | 'completed' | 'cancelled' | 'no-show';
  petName?: string;
  serviceName?: string;
@@ -98,54 +96,100 @@ export function formatDate(dateStr: string): string {
 }

 export function parseTimeTo24Hour(time: string | null | undefined): string {
-  if (!time) return '00:00:00';
-  const parts = time.split(' ');
+  const parts = (time ?? '').split(' ');
  const hoursMinutes = parts[0] ?? '';
  const period = parts[1] ?? '';
  const [hoursStr, minutesStr] = hoursMinutes.split(':');
-  const hours = parseInt(hoursStr ?? '0', 10);
-  const minutes = parseInt(minutesStr ?? '0', 10);
+  // `|| '0'` (not `?? '0'`) so empty strings from blank/undefined input
+  // fall back to 0 rather than parsing to NaN.
+  const hours = parseInt(hoursStr || '0', 10);
+  const minutes = parseInt(minutesStr || '0', 10);
  let hours24 = hours;
  if (period === 'PM' && hours !== 12) hours24 += 12;
  if (period === 'AM' && hours === 12) hours24 = 0;
  return `${hours24.toString().padStart(2, '0')}:${minutes.toString().padStart(2, '0')}:00`;
 }

-/**
- * Resolve an appointment's start instant from either the API contract shape
- * (ISO `startTime`) or the legacy `date` + `time` shape. Returns null when no
- * usable date is present or the value is unparseable, so callers never throw.
- */
-export function getAppointmentStart(appt: Appointment): Date | null {
-  const raw = appt.startTime
-    ? appt.startTime
-    : appt.date
-      ? `${appt.date}T${parseTimeTo24Hour(appt.time)}`
-      : null;
-  if (!raw) return null;
-  const parsed = new Date(raw);
-  return isNaN(parsed.getTime()) ? null : parsed;
-}
-
 export function isUpcoming(appt: Appointment): boolean {
-  const start = getAppointmentStart(appt);
-  if (!start) return false;
-  return start > new Date() && appt.status !== 'cancelled' && appt.status !== 'completed';
+  const now = new Date();
+  // Prefer the absolute ISO `startTime` from the API; fall back to the
+  // locally-formatted date/time pair for already-normalized/legacy shapes.
+  const apptDate = appt.startTime
+    ? new Date(appt.startTime)
+    : new Date(`${appt.date}T${parseTimeTo24Hour(appt.time)}`);
+  return apptDate > now && appt.status !== 'cancelled' && appt.status !== 'completed';
 }

-/** Display date string, preferring the ISO `startTime` contract shape. */
-export function formatAppointmentDate(appt: Appointment): string {
-  const start = getAppointmentStart(appt);
-  return start ? formatDate(start.toISOString()) : '';
+// ─── API → UI shape normalization ────────────────────────────────────────────
+// `/api/portal/appointments` returns ISO `startTime`/`endTime` plus nested
+// pet/service/staff objects, NOT the flat `date`/`time`/`petName` shape the
+// portal UI renders. Every field below is optional so the legacy flat shape
+// (used by tests/fixtures) is tolerated unchanged.
+export interface RawApiAppointment {
+  id: string;
+  startTime?: string | null;
+  endTime?: string | null;
+  status: Appointment['status'];
+  confirmationStatus?: Appointment['confirmationStatus'];
+  customerNotes?: string | null;
+  notes?: string | null;
+  pet?: { id?: string | null; name?: string | null; photo?: string | null } | null;
+  service?: { id?: string | null; name?: string | null } | null;
+  staff?: { id?: string | null; name?: string | null } | null;
+  // Legacy / already-flat fields
+  petId?: string;
+  serviceId?: string;
+  groomerId?: string | null;
+  date?: string;
+  time?: string;
+  petName?: string;
+  serviceName?: string;
+  groomerName?: string;
+  duration?: number;
+  price?: number;
+  addOns?: string[];
 }

-/** Display time string, preferring the ISO `startTime` contract shape. */
-export function formatAppointmentTime(appt: Appointment): string {
-  const start = getAppointmentStart(appt);
-  if (start) {
-    return start.toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit' });
-  }
-  return appt.time ?? '';
+function toLocalDateString(d: Date): string {
+  const year = d.getFullYear();
+  const month = String(d.getMonth() + 1).padStart(2, '0');
+  const day = String(d.getDate()).padStart(2, '0');
+  return `${year}-${month}-${day}`;
+}
+
+function toLocalTimeString(d: Date): string {
+  return d.toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit', hour12: true });
+}
+
+// Maps a raw API appointment into the flat `Appointment` shape the portal
+// renders. Derives display `date`/`time` from the absolute `startTime` and
+// `duration` from the start/end delta. Tolerates the legacy flat shape.
+export function normalizeAppointment(raw: RawApiAppointment): Appointment {
+  const start = raw.startTime ? new Date(raw.startTime) : null;
+  const end = raw.endTime ? new Date(raw.endTime) : null;
+  const derivedDuration =
+    start && end ? Math.round((end.getTime() - start.getTime()) / 60000) : undefined;
+
+  return {
+    id: raw.id,
+    petId: raw.pet?.id ?? raw.petId ?? '',
+    serviceId: raw.service?.id ?? raw.serviceId ?? '',
+    groomerId: raw.staff?.id ?? raw.groomerId ?? null,
+    startTime: raw.startTime ?? undefined,
+    endTime: raw.endTime ?? undefined,
+    date: start ? toLocalDateString(start) : raw.date ?? '',
+    time: start ? toLocalTimeString(start) : raw.time ?? '',
+    status: raw.status,
+    petName: raw.pet?.name ?? raw.petName,
+    serviceName: raw.service?.name ?? raw.serviceName,
+    groomerName: raw.staff?.name ?? raw.groomerName,
+    duration: raw.duration ?? derivedDuration,
+    price: raw.price,
+    notes: raw.notes ?? undefined,
+    customerNotes: raw.customerNotes ?? undefined,
+    addOns: raw.addOns,
+    confirmationStatus: raw.confirmationStatus,
+  };
 }

 const STATUS_COLORS: Record<string, string> = {
@@ -211,7 +255,8 @@ export const AppointmentsSection: React.FC<AppointmentsSectionProps> = ({ sessio

        if (response.ok) {
          const data = await response.json();
-          const fetchedAppointments: Appointment[] = data.appointments || data || [];
+          const rawAppointments: RawApiAppointment[] = data.appointments || data || [];
+          const fetchedAppointments: Appointment[] = rawAppointments.map(normalizeAppointment);

          const upcoming = fetchedAppointments.filter((appt) => isUpcoming(appt));
          const past = fetchedAppointments.filter((appt) => !isUpcoming(appt));
@@ -376,11 +421,11 @@ function AppointmentCard({
          <div className="flex items-center gap-3 text-xs text-stone-500 mt-0.5">
            <span className="flex items-center gap-1">
              <Calendar size={12} />
-              {formatAppointmentDate(appt)}
+              {formatDate(appt.date)}
            </span>
            <span className="flex items-center gap-1">
              <Clock size={12} />
-              {formatAppointmentTime(appt)}
+              {appt.time}
            </span>
            <span>with {appt.groomerName || 'First Available'}</span>
          </div>
@@ -748,7 +793,7 @@ export function RescheduleFlow({
                  {appt.petName || 'Pet'} — {appt.serviceName || 'Service'}
                </p>
                <p className="text-stone-500 mt-0.5">
-                  {formatAppointmentDate(appt)} at {formatAppointmentTime(appt)} with{' '}
+                  {formatDate(appt.date)} at {appt.time} with{' '}
                  {appt.groomerName || 'First Available'}
                </p>
              </div>