From c7c83890e4f3856c5bc3c6f1c55807b7b86ffc52 Mon Sep 17 00:00:00 2001 From: Nora Date: Mon, 6 Jul 2026 09:56:13 -0600 Subject: [PATCH] fix(time-tracking): use correct field_workers columns in fleet summary The /admin/time-tracking overview page crashed on render with `column "fw.display_name" does not exist`. Cycle 10 unified `time_tracking_workers` + `water_irrigators` into `field_workers`, which uses `name` and `pin_hash` (not the legacy `display_name` / `pin` from `admin_users`). This fix aligns the SQL, row type, and result mapper with the actual schema. End-to-end verified against water.tuxedocorn.com with worker PIN 2229: PIN auth, task picker, clock-in, and clock-out all succeed with no console errors and no failed HTTP requests. Includes a regression test that fails on the buggy SQL (4/8 fail) and passes after the fix (8/8 pass). Covers the column contract, row projection, empty-result + unauthenticated boundaries, and null aggregate coercion. --- src/actions/time-tracking/route-summary.ts | 18 +- .../time-tracking-fleet-route-summary.test.ts | 211 ++++++++++++++++++ 2 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 tests/unit/time-tracking-fleet-route-summary.test.ts diff --git a/src/actions/time-tracking/route-summary.ts b/src/actions/time-tracking/route-summary.ts index a651cfb..14f5b36 100644 --- a/src/actions/time-tracking/route-summary.ts +++ b/src/actions/time-tracking/route-summary.ts @@ -207,10 +207,14 @@ export async function getFleetRouteSummary( end.setUTCDate(end.getUTCDate() + 1); const pool = getPool(); + // Cycle 10 — `field_workers` uses `name` and `pin_hash` (NOT the legacy + // `display_name` / `pin` columns from `admin_users` / the pre-cycle-10 + // `time_tracking_workers`). Referencing the wrong columns crashes the + // /admin/time-tracking overview page at render time. const { rows } = await pool.query<{ id: string; - display_name: string | null; - pin: string; + name: string | null; + pin_hash: string; active: boolean; water_entries: string; water_gallons: string | null; @@ -219,8 +223,8 @@ export async function getFleetRouteSummary( }>( `SELECT fw.id, - fw.display_name, - fw.pin, + fw.name, + fw.pin_hash, fw.active, COALESCE(w.cnt, 0)::text AS water_entries, COALESCE(w.gal, 0)::text AS water_gallons, @@ -252,9 +256,13 @@ export async function getFleetRouteSummary( [brandId, start.toISOString(), end.toISOString()], ); + // `pin_hash` is a scrypt self-describing hash — never display more + // than a short fingerprint as a last-resort fallback for missing + // worker names. The hash is sensitive but its first 6 chars are not + // reversible, which matches the original code's intent. return rows.map((r) => ({ workerId: r.id, - workerName: r.display_name || r.pin.slice(0, 6) || "Worker", + workerName: r.name || r.pin_hash.slice(0, 6) || "Worker", active: r.active, water: { entryCount: Number(r.water_entries ?? 0), diff --git a/tests/unit/time-tracking-fleet-route-summary.test.ts b/tests/unit/time-tracking-fleet-route-summary.test.ts new file mode 100644 index 0000000..cd8e1e2 --- /dev/null +++ b/tests/unit/time-tracking-fleet-route-summary.test.ts @@ -0,0 +1,211 @@ +/** + * Regression tests for `src/actions/time-tracking/route-summary.ts` → + * `getFleetRouteSummary`. + * + * Why this exists: the original implementation referenced + * `fw.display_name` and `fw.pin` in its SQL, but the unified + * `field_workers` table (cycle 10) uses `name` and `pin_hash`. The + * wrong-column reference crashed the `/admin/time-tracking` overview + * page on render with `column "fw.display_name" does not exist`. No + * test caught it because nothing exercised this code path. + * + * These tests mock the bare minimum (admin user + pg pool) and assert: + * 1. The SQL passed to `pool.query` references the correct columns + * (`fw.name`, `fw.pin_hash`) and NOT the legacy names. + * 2. The row mapper projects `r.name` / `r.pin_hash` (not the + * legacy keys). + * 3. Empty result set yields `[]` without crashing. + * 4. Boundary: an unknown date still produces a valid SQL query + * and a typed return shape. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const { mockGetAdminUser, mockQuery } = vi.hoisted(() => { + return { + mockGetAdminUser: vi.fn(), + mockQuery: vi.fn(), + }; +}); + +vi.mock("server-only", () => ({})); + +vi.mock("@/lib/admin-permissions", () => ({ + getAdminUser: mockGetAdminUser, +})); + +vi.mock("@/lib/db", () => ({ + getPool: () => ({ query: mockQuery }), +})); + +// Stub the withBrand call inside `getRouteTimeSummary` so importing the +// module doesn't try to resolve drizzle at test time. +vi.mock("@/db/client", () => ({ + withBrand: vi.fn(async (_brandId: string, fn: (db: unknown) => unknown) => + fn({ select: () => ({ from: () => ({ where: () => ({ limit: () => [] }) }) }) }), + ), +})); + +// Stub the cross-module call into the admin time-tracking settings. +vi.mock("@/actions/time-tracking", () => ({ + getTimeTrackingSettings: vi.fn(async () => null), +})); + +// Imported AFTER all mocks are wired. +import { getFleetRouteSummary } from "@/actions/time-tracking/route-summary"; + +const BRAND_ID = "11111111-2222-3333-4444-555555555555"; +const TODAY = "2026-07-06"; + +beforeEach(() => { + mockGetAdminUser.mockReset(); + mockQuery.mockReset(); + mockGetAdminUser.mockResolvedValue({ id: "admin-1", role: "platform_admin" }); +}); + +describe("getFleetRouteSummary — SQL column contract", () => { + it("references fw.name and fw.pin_hash (NOT the legacy fw.display_name / fw.pin)", async () => { + mockQuery.mockResolvedValueOnce({ rows: [] }); + + await getFleetRouteSummary(BRAND_ID, TODAY); + + expect(mockQuery).toHaveBeenCalledTimes(1); + const [sql] = mockQuery.mock.calls[0]; + + expect(sql).toContain("fw.name"); + expect(sql).toContain("fw.pin_hash"); + // The legacy columns must NOT appear anywhere in the query — + // catching them in the SELECT, JOIN, or ORDER BY is the whole + // point of this test. + expect(sql).not.toMatch(/fw\.display_name/); + expect(sql).not.toMatch(/\bfw\.pin\b/); + }); + + it("queries with the brand id and ISO date bounds (regression: off-by-one date math)", async () => { + mockQuery.mockResolvedValueOnce({ rows: [] }); + + await getFleetRouteSummary(BRAND_ID, TODAY); + + const [, params] = mockQuery.mock.calls[0]; + expect(params).toHaveLength(3); + expect(params[0]).toBe(BRAND_ID); + // Lower bound is the requested date at UTC midnight; upper bound + // is the NEXT day at UTC midnight (24-hour window, half-open). + expect(params[1]).toBe(`${TODAY}T00:00:00.000Z`); + const upper = new Date(params[2] as string); + const lower = new Date(params[1] as string); + expect(upper.getTime() - lower.getTime()).toBe(24 * 60 * 60 * 1000); + }); +}); + +describe("getFleetRouteSummary — row projection", () => { + it("maps the row's `name` field onto workerName and falls back when name is null", async () => { + mockQuery.mockResolvedValueOnce({ + rows: [ + { + id: "worker-1", + name: "Tyler", + pin_hash: "scrypt$abc123$xyz", + active: true, + water_entries: "3", + water_gallons: "120", + worked_minutes: "240", + is_clocked_in: true, + }, + ], + }); + + const rows = await getFleetRouteSummary(BRAND_ID, TODAY); + + expect(rows).toEqual([ + { + workerId: "worker-1", + workerName: "Tyler", + active: true, + water: { entryCount: 3, totalGallons: 120 }, + time: { workedMinutesToday: 240, isClockedIn: true }, + }, + ]); + }); + + it("falls back to a 6-char pin_hash fingerprint when name is null (not the legacy pin column)", async () => { + mockQuery.mockResolvedValueOnce({ + rows: [ + { + id: "worker-2", + name: null, + pin_hash: "scrypt$ABCDEF$rest", + active: false, + water_entries: "0", + water_gallons: null, + worked_minutes: "0", + is_clocked_in: false, + }, + ], + }); + + const rows = await getFleetRouteSummary(BRAND_ID, TODAY); + + expect(rows).toHaveLength(1); + expect(rows[0].workerName).toBe("scrypt"); + expect(rows[0].active).toBe(false); + expect(rows[0].water.totalGallons).toBe(0); + expect(rows[0].time.workedMinutesToday).toBe(0); + expect(rows[0].time.isClockedIn).toBe(false); + }); +}); + +describe("getFleetRouteSummary — boundaries + failure modes", () => { + it("returns [] when there are no active workers in the brand", async () => { + mockQuery.mockResolvedValueOnce({ rows: [] }); + + const rows = await getFleetRouteSummary(BRAND_ID, TODAY); + expect(rows).toEqual([]); + }); + + it("returns [] when the caller is unauthenticated (admin gate)", async () => { + mockGetAdminUser.mockResolvedValueOnce(null); + + const rows = await getFleetRouteSummary(BRAND_ID, TODAY); + expect(rows).toEqual([]); + // Critical: we must not have hit the pool if auth failed. + expect(mockQuery).not.toHaveBeenCalled(); + }); + + it("defaults to today's UTC date when no date argument is supplied", async () => { + mockQuery.mockResolvedValueOnce({ rows: [] }); + + await getFleetRouteSummary(BRAND_ID); + + expect(mockQuery).toHaveBeenCalledTimes(1); + const [, params] = mockQuery.mock.calls[0]; + // The bounds must form a one-day window; lower < upper, and + // lower must be UTC midnight on today's date. + const lower = new Date(params[1] as string); + const upper = new Date(params[2] as string); + expect(lower.getUTCHours()).toBe(0); + expect(upper.getTime() - lower.getTime()).toBe(24 * 60 * 60 * 1000); + }); + + it("coerces null numeric aggregates to 0 (water_gallons / worked_minutes can be null)", async () => { + mockQuery.mockResolvedValueOnce({ + rows: [ + { + id: "worker-3", + name: "Pat", + pin_hash: "scrypt$XXX", + active: true, + water_entries: "0", + water_gallons: null, + worked_minutes: null, + is_clocked_in: false, + }, + ], + }); + + const rows = await getFleetRouteSummary(BRAND_ID, TODAY); + + expect(rows[0].water.totalGallons).toBe(0); + expect(rows[0].water.entryCount).toBe(0); + expect(rows[0].time.workedMinutesToday).toBe(0); + }); +}); \ No newline at end of file