From fccf4130c8e0f290f6e5f46098e34c837f0354ee Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 6 May 2026 18:00:06 +0000 Subject: [PATCH] added instruct measures approval --- .../instructed-measures/route.ts | 31 +-- src/app/lib/instructMeasure.test.ts | 180 +++++++++++++++ src/app/lib/instructMeasure.ts | 218 ++++++++++++++++++ .../live/PropertyDetailDrawer.tsx | 195 +++++++++++----- .../your-projects/live/[dealId]/DealPage.tsx | 1 + 5 files changed, 555 insertions(+), 70 deletions(-) diff --git a/src/app/api/portfolio/[portfolioId]/instructed-measures/route.ts b/src/app/api/portfolio/[portfolioId]/instructed-measures/route.ts index ff7a74d..533f228 100644 --- a/src/app/api/portfolio/[portfolioId]/instructed-measures/route.ts +++ b/src/app/api/portfolio/[portfolioId]/instructed-measures/route.ts @@ -10,12 +10,14 @@ import { portfolioUsers, } from "@/app/db/schema/portfolio"; import { user } from "@/app/db/schema/users"; -import { instructMeasure } from "@/app/lib/instructMeasure"; +import { instructMeasures } from "@/app/lib/instructMeasure"; import { MEASURE_NAMES } from "@/app/lib/measureDocumentRequirements"; const postSchema = z.object({ dealId: z.string().min(1, "dealId is required"), - measureName: z.string().min(1, "measureName is required"), + measureNames: z + .array(z.string().min(1)) + .min(1, "measureNames must not be empty"), }); /** @@ -27,10 +29,10 @@ const postSchema = z.object({ * pushes back to HubSpot. See `instructMeasure` for the full contract. * * Body: - * { dealId: string, measureName: string } + * { dealId: string, measureNames: string[] } * * Response: - * 200 { ok: true, hubspotSync: "ok" | "failed", autoPopulatedProposed: boolean, hubspotError? } + * 200 { ok: true, hubspotSync: "ok" | "failed", hubspotError? } * 400 { ok: false, error } * 401 / 403 / 404 on auth/role/user errors. */ @@ -60,15 +62,16 @@ export async function POST( ); } - const { dealId, measureName } = parsed.data; + const { dealId, measureNames } = parsed.data; - // Validate against the canonical catalogue up-front so the route returns - // a clean 400 rather than relying on the service-level check. - if (!(MEASURE_NAMES as ReadonlyArray).includes(measureName)) { - return NextResponse.json( - { error: `Unknown measure: ${measureName}` }, - { status: 400 }, - ); + // Validate all names against the canonical catalogue up-front. + for (const name of measureNames) { + if (!(MEASURE_NAMES as ReadonlyArray).includes(name)) { + return NextResponse.json( + { error: `Unknown measure: ${name}` }, + { status: 400 }, + ); + } } const userRow = await db @@ -119,9 +122,9 @@ export async function POST( } try { - const result = await instructMeasure({ + const result = await instructMeasures({ dealId, - measureName, + measureNames, userId: userRow[0].id, }); diff --git a/src/app/lib/instructMeasure.test.ts b/src/app/lib/instructMeasure.test.ts index 7f8ae71..a9bf993 100644 --- a/src/app/lib/instructMeasure.test.ts +++ b/src/app/lib/instructMeasure.test.ts @@ -11,10 +11,13 @@ import { PROPOSED_MEASURES_PROP, APPROVED_MEASURES_PROP, instructMeasure, + instructMeasures, } from "./instructMeasure"; import type { InstructTxOutcome, + InstructMeasuresTxOutcome, RunInstructTx, + RunInstructMeasuresTx, ReadInstructedMeasureNames, StampPushedAt, SyncMeasuresField, @@ -307,3 +310,180 @@ describe("instructMeasure — HubSpot push failure leaves DB committed", () => { expect(deps.stampPushedAt).not.toHaveBeenCalled(); }); }); + +// --------------------------------------------------------------------------- +// instructMeasures (plural) — batch variant +// --------------------------------------------------------------------------- + +function makeBatchDeps(overrides?: { + txOutcome?: Partial; + txError?: Error; + instructedAfter?: string[]; + syncResults?: Array<{ ok: true } | { ok: false; error: string }>; + stampError?: Error; +}) { + const txOutcome: InstructMeasuresTxOutcome = { + instructedRowIds: [1n, 2n], + existingProposedMeasures: [], + allApprovedMeasureNames: [], + ...overrides?.txOutcome, + }; + const runInstructMeasuresTx: RunInstructMeasuresTx = vi.fn(async () => { + if (overrides?.txError) throw overrides.txError; + return txOutcome; + }); + const readInstructedMeasureNames: ReadInstructedMeasureNames = vi.fn( + async () => overrides?.instructedAfter ?? ["ASHP", "Solar PV"], + ); + const syncQueue: Array<{ ok: true } | { ok: false; error: string }> = + overrides?.syncResults ?? [{ ok: true }, { ok: true }, { ok: true }]; + const syncMeasuresField: SyncMeasuresField = vi.fn(async () => { + return syncQueue.shift() ?? ({ ok: true } as const); + }); + const stampPushedAt: StampPushedAt = vi.fn(async () => { + if (overrides?.stampError) throw overrides.stampError; + }); + return { + runInstructMeasuresTx, + readInstructedMeasureNames, + syncMeasuresField, + stampPushedAt, + }; +} + +describe("instructMeasures — input validation", () => { + it("rejects when measureNames is empty", async () => { + const deps = makeBatchDeps(); + const result = await instructMeasures({ + dealId: "deal-1", + measureNames: [], + userId: 1n, + deps, + }); + expect(result).toEqual({ ok: false, error: "measureNames must not be empty" }); + expect(deps.runInstructMeasuresTx).not.toHaveBeenCalled(); + expect(deps.syncMeasuresField).not.toHaveBeenCalled(); + }); + + it("rejects when any measureName is unknown", async () => { + const deps = makeBatchDeps(); + const result = await instructMeasures({ + dealId: "deal-1", + measureNames: ["ASHP", "Not a real measure"], + userId: 1n, + deps, + }); + expect(result).toEqual({ ok: false, error: "Unknown measure: Not a real measure" }); + expect(deps.runInstructMeasuresTx).not.toHaveBeenCalled(); + expect(deps.syncMeasuresField).not.toHaveBeenCalled(); + }); +}); + +describe("instructMeasures — happy path", () => { + it("commits single tx, pushes instructed + proposed + approved, stamps all rowIds", async () => { + const deps = makeBatchDeps({ + instructedAfter: ["ASHP", "Solar PV"], + txOutcome: { + instructedRowIds: [10n, 11n], + existingProposedMeasures: [], + allApprovedMeasureNames: ["ASHP", "Solar PV"], + }, + }); + const result = await instructMeasures({ + dealId: "deal-42", + measureNames: ["ASHP", "Solar PV"], + userId: 7n, + deps, + }); + expect(result).toMatchObject({ ok: true, instructedRowIds: [10n, 11n], hubspotSync: "ok" }); + expect(deps.runInstructMeasuresTx).toHaveBeenCalledOnce(); + expect(deps.runInstructMeasuresTx).toHaveBeenCalledWith({ + dealId: "deal-42", + measureNames: ["ASHP", "Solar PV"], + userId: 7n, + notes: null, + }); + expect(deps.syncMeasuresField).toHaveBeenCalledTimes(3); + expect(deps.syncMeasuresField).toHaveBeenNthCalledWith(1, { + hubspotDealId: "deal-42", + propName: INSTRUCTED_MEASURES_PROP, + measureNames: ["ASHP", "Solar PV"], + }); + expect(deps.syncMeasuresField).toHaveBeenNthCalledWith(2, { + hubspotDealId: "deal-42", + propName: PROPOSED_MEASURES_PROP, + measureNames: ["ASHP", "Solar PV"], + }); + expect(deps.syncMeasuresField).toHaveBeenNthCalledWith(3, { + hubspotDealId: "deal-42", + propName: APPROVED_MEASURES_PROP, + measureNames: ["ASHP", "Solar PV"], + }); + expect(deps.stampPushedAt).toHaveBeenCalledTimes(2); + expect(deps.stampPushedAt).toHaveBeenCalledWith(10n); + expect(deps.stampPushedAt).toHaveBeenCalledWith(11n); + }); + + it("merges all new measures into existing proposed (deduped)", async () => { + const deps = makeBatchDeps({ + instructedAfter: ["ASHP", "EWI", "Solar PV"], + txOutcome: { + instructedRowIds: [20n, 21n], + existingProposedMeasures: ["ASHP", "Loft insulation"], + allApprovedMeasureNames: ["ASHP", "EWI", "Solar PV"], + }, + }); + await instructMeasures({ + dealId: "deal-merge", + measureNames: ["EWI", "Solar PV"], + userId: 3n, + deps, + }); + expect(deps.syncMeasuresField).toHaveBeenNthCalledWith(2, { + hubspotDealId: "deal-merge", + propName: PROPOSED_MEASURES_PROP, + measureNames: ["ASHP", "Loft insulation", "EWI", "Solar PV"], + }); + }); +}); + +describe("instructMeasures — DB transaction failure", () => { + it("returns error and skips HubSpot when tx throws", async () => { + const deps = makeBatchDeps({ txError: new Error("batch insert failed") }); + const result = await instructMeasures({ + dealId: "deal-x", + measureNames: ["ASHP", "EWI"], + userId: 1n, + deps, + }); + expect(result).toEqual({ ok: false, error: "batch insert failed" }); + expect(deps.syncMeasuresField).not.toHaveBeenCalled(); + expect(deps.stampPushedAt).not.toHaveBeenCalled(); + }); +}); + +describe("instructMeasures — HubSpot push failure leaves DB committed", () => { + it("returns ok=true with hubspotSync=failed when sync fails, does NOT stamp", async () => { + const deps = makeBatchDeps({ + instructedAfter: ["ASHP", "EWI"], + txOutcome: { + instructedRowIds: [30n, 31n], + existingProposedMeasures: [], + allApprovedMeasureNames: ["ASHP", "EWI"], + }, + syncResults: [{ ok: false, error: "hubspot 500" }, { ok: true }, { ok: true }], + }); + const result = await instructMeasures({ + dealId: "deal-h", + measureNames: ["ASHP", "EWI"], + userId: 1n, + deps, + }); + expect(result).toMatchObject({ + ok: true, + hubspotSync: "failed", + hubspotError: "hubspot 500", + }); + expect(deps.stampPushedAt).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/lib/instructMeasure.ts b/src/app/lib/instructMeasure.ts index a7421ca..3e66e5e 100644 --- a/src/app/lib/instructMeasure.ts +++ b/src/app/lib/instructMeasure.ts @@ -196,6 +196,224 @@ const defaultStampPushedAt: StampPushedAt = async (rowId) => { .where(eq(userDefinedDealMeasures.id, rowId)); }; +// --------------------------------------------------------------------------- +// Batch (plural) types +// --------------------------------------------------------------------------- + +export interface InstructMeasuresTxOutcome { + instructedRowIds: bigint[]; + existingProposedMeasures: string[]; + allApprovedMeasureNames: string[]; +} + +export type RunInstructMeasuresTx = (params: { + dealId: string; + measureNames: MeasureName[]; + userId: bigint; + notes: string | null; +}) => Promise; + +export type InstructMeasuresResult = + | { + ok: true; + instructedRowIds: bigint[]; + hubspotSync: "ok" | "failed"; + hubspotError?: string; + } + | { ok: false; error: string }; + +export interface InstructMeasuresInput { + dealId: string; + measureNames: string[]; + userId: bigint; + notes?: string; + deps?: { + runInstructMeasuresTx?: RunInstructMeasuresTx; + readInstructedMeasureNames?: ReadInstructedMeasureNames; + syncMeasuresField?: SyncMeasuresField; + stampPushedAt?: StampPushedAt; + }; +} + +const defaultRunInstructMeasuresTx: RunInstructMeasuresTx = async ({ + dealId, + measureNames, + userId, + notes, +}) => { + return await db.transaction(async (tx) => { + const instructedRowIds: bigint[] = []; + + for (const measureName of measureNames) { + const inserted = await tx + .insert(userDefinedDealMeasures) + .values({ + hubspotDealId: dealId, + measureName, + source: "instructed", + createdByUserId: userId, + notes, + }) + .returning({ id: userDefinedDealMeasures.id }); + const rowId = inserted[0]?.id; + if (rowId === undefined || rowId === null) { + throw new Error("Failed to insert user_defined_deal_measures row"); + } + instructedRowIds.push(rowId); + + await tx + .insert(dealMeasureApprovals) + .values({ + hubspotDealId: dealId, + measureName, + isApproved: true, + approvedBy: userId, + }) + .onConflictDoUpdate({ + target: [ + dealMeasureApprovals.hubspotDealId, + dealMeasureApprovals.measureName, + ], + set: { + isApproved: true, + approvedBy: userId, + approvedAt: new Date(), + }, + }); + + await tx.insert(dealMeasureApprovalEvents).values({ + hubspotDealId: dealId, + measureName, + action: "approved", + actedBy: userId, + }); + } + + const dealRows = await tx + .select({ proposedMeasures: hubspotDealData.proposedMeasures }) + .from(hubspotDealData) + .where(eq(hubspotDealData.dealId, dealId)) + .limit(1); + const existingProposedMeasures = parseMeasures(dealRows[0]?.proposedMeasures ?? null); + + const approvedRows = await tx + .select({ measureName: dealMeasureApprovals.measureName }) + .from(dealMeasureApprovals) + .where( + and( + eq(dealMeasureApprovals.hubspotDealId, dealId), + eq(dealMeasureApprovals.isApproved, true), + ), + ); + const allApprovedMeasureNames = approvedRows.map((r) => r.measureName); + + return { instructedRowIds, existingProposedMeasures, allApprovedMeasureNames }; + }); +}; + +export async function instructMeasures( + input: InstructMeasuresInput, +): Promise { + if (input.measureNames.length === 0) { + return { ok: false, error: "measureNames must not be empty" }; + } + + const validatedNames: MeasureName[] = []; + for (const name of input.measureNames) { + const trimmed = name.trim(); + if (!isMeasureName(trimmed)) { + return { ok: false, error: `Unknown measure: ${trimmed}` }; + } + validatedNames.push(trimmed); + } + + const runInstructMeasuresTx = + input.deps?.runInstructMeasuresTx ?? defaultRunInstructMeasuresTx; + const readInstructed = + input.deps?.readInstructedMeasureNames ?? defaultReadInstructedMeasureNames; + const syncMeasuresField = + input.deps?.syncMeasuresField ?? defaultSyncMeasuresField; + const stampPushedAt = input.deps?.stampPushedAt ?? defaultStampPushedAt; + + let txResult: InstructMeasuresTxOutcome; + try { + txResult = await runInstructMeasuresTx({ + dealId: input.dealId, + measureNames: validatedNames, + userId: input.userId, + notes: input.notes ?? null, + }); + } catch (err) { + const message = + err instanceof Error ? err.message : "Failed to instruct measures"; + console.error("[instructMeasures] transaction failed", { + dealId: input.dealId, + measureNames: validatedNames, + error: err, + }); + return { ok: false, error: message }; + } + + const allInstructed = await readInstructed(input.dealId); + + const mergedProposed = Array.from( + new Set([...txResult.existingProposedMeasures, ...validatedNames]), + ); + + const instructedSync = await syncMeasuresField({ + hubspotDealId: input.dealId, + propName: INSTRUCTED_MEASURES_PROP, + measureNames: allInstructed, + }); + + const proposedSync = await syncMeasuresField({ + hubspotDealId: input.dealId, + propName: PROPOSED_MEASURES_PROP, + measureNames: mergedProposed, + }); + + const approvedSync = await syncMeasuresField({ + hubspotDealId: input.dealId, + propName: APPROVED_MEASURES_PROP, + measureNames: txResult.allApprovedMeasureNames, + }); + + const overallOk = instructedSync.ok && proposedSync.ok && approvedSync.ok; + + if (overallOk) { + for (const rowId of txResult.instructedRowIds) { + try { + await stampPushedAt(rowId); + } catch (err) { + console.error("[instructMeasures] failed to stamp pushed_at", { + rowId: String(rowId), + error: err, + }); + } + } + return { + ok: true, + instructedRowIds: txResult.instructedRowIds, + hubspotSync: "ok", + }; + } + + const hubspotError = !instructedSync.ok + ? instructedSync.error + : !proposedSync.ok + ? proposedSync.error + : !approvedSync.ok + ? approvedSync.error + : "HubSpot sync failed"; + + return { + ok: true, + instructedRowIds: txResult.instructedRowIds, + hubspotSync: "failed", + hubspotError, + }; +} + export async function instructMeasure( input: InstructMeasureInput, ): Promise { diff --git a/src/app/portfolio/[slug]/(portfolio)/your-projects/live/PropertyDetailDrawer.tsx b/src/app/portfolio/[slug]/(portfolio)/your-projects/live/PropertyDetailDrawer.tsx index fd9fe62..a3ab982 100644 --- a/src/app/portfolio/[slug]/(portfolio)/your-projects/live/PropertyDetailDrawer.tsx +++ b/src/app/portfolio/[slug]/(portfolio)/your-projects/live/PropertyDetailDrawer.tsx @@ -30,6 +30,7 @@ import { ApprovalConfirmDialog } from "./ApprovalConfirmDialog"; import type { PendingDiff } from "./ApprovalConfirmDialog"; import { MEASURE_NAMES } from "@/app/lib/measureDocumentRequirements"; import { outOfOrderInstructionWarning } from "@/app/lib/softWarnings"; +import { useToast } from "@/app/hooks/use-toast"; // Sections the caller can request focus on. Used by entry-points like the // Measures table row click that should land the user on a specific tab. @@ -1505,12 +1506,13 @@ export function PibiMeasureSelector({ } // ----------------------------------------------------------------------- -// Instruct measure editor — approver-only form to instruct an out-of-band -// measure that the coordinator did not propose (issue #253). +// Instruct measure editor — approver-only form to instruct out-of-band +// measures that the coordinator did not propose (issue #253). // ----------------------------------------------------------------------- interface InstructMeasureEditorProps { dealId: string; portfolioId: string; + proposedMeasures: string[]; /** True when the user has the approver capability on this portfolio. */ canEdit: boolean; /** Soft-warning string from `outOfOrderInstructionWarning`, or null. */ @@ -1520,47 +1522,85 @@ interface InstructMeasureEditorProps { export function InstructMeasureEditor({ dealId, portfolioId, + proposedMeasures, canEdit, outOfOrderWarning, }: InstructMeasureEditorProps) { - const [selected, setSelected] = useState(""); - const [optimisticList, setOptimisticList] = useState([]); + const queryClient = useQueryClient(); + const { toast } = useToast(); + + const { data: pibiData } = useQuery<{ + pibiMeasures: string[]; + approvedMeasures: string[]; + instructedMeasures: string[]; + }>({ + queryKey: ["pibiMeasures", portfolioId, dealId], + queryFn: async () => { + const res = await fetch( + `/api/portfolio/${portfolioId}/pibi-measures?dealId=${encodeURIComponent(dealId)}`, + ); + if (!res.ok) throw new Error("Failed to fetch measures"); + return res.json(); + }, + staleTime: 30_000, + enabled: canEdit, + }); + + const excluded = useMemo(() => { + const set = new Set(proposedMeasures); + for (const m of pibiData?.approvedMeasures ?? []) set.add(m); + for (const m of pibiData?.instructedMeasures ?? []) set.add(m); + return set; + }, [proposedMeasures, pibiData]); + + const eligible = useMemo( + () => MEASURE_NAMES.filter((m) => !excluded.has(m)), + [excluded], + ); + + const [checked, setChecked] = useState>(new Set()); + const [confirmOpen, setConfirmOpen] = useState(false); const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); - // Reset optimistic state when the drawer switches deals. useEffect(() => { - setOptimisticList([]); - setSelected(""); + setChecked(new Set()); setError(null); }, [dealId]); if (!canEdit) return null; - async function handleSubmit() { - if (!selected) return; - const measure = selected; + function toggleMeasure(m: string) { + setChecked((prev) => { + const next = new Set(prev); + if (next.has(m)) next.delete(m); + else next.add(m); + return next; + }); + } + + async function handleConfirm() { + const measureNames = Array.from(checked); + if (measureNames.length === 0) return; setSubmitting(true); setError(null); - // Optimistic append. Reverted on failure. - setOptimisticList((prev) => [...prev, measure]); try { const res = await fetch( `/api/portfolio/${portfolioId}/instructed-measures`, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ dealId, measureName: measure }), + body: JSON.stringify({ dealId, measureNames }), }, ); if (!res.ok) { - setOptimisticList((prev) => prev.filter((m) => m !== measure)); const json = await res.json().catch(() => ({})); setError( typeof json.error === "string" ? json.error - : "Failed to instruct measure", + : "Failed to instruct measures", ); + setConfirmOpen(false); return; } const json = (await res.json()) as { @@ -1568,19 +1608,28 @@ export function InstructMeasureEditor({ hubspotSync: "ok" | "failed"; hubspotError?: string; }; + setConfirmOpen(false); + setChecked(new Set()); + void queryClient.invalidateQueries({ queryKey: ["pibiMeasures", portfolioId, dealId] }); if (json.hubspotSync === "failed") { - setError( - json.hubspotError + toast({ + title: "Measures instructed", + description: json.hubspotError ? `Saved locally — HubSpot sync failed: ${json.hubspotError}` : "Saved locally — HubSpot sync failed", - ); + variant: "destructive", + }); + } else { + toast({ + title: "Measures instructed", + description: `${measureNames.join(", ")} ${measureNames.length === 1 ? "has" : "have"} been instructed.`, + }); } - setSelected(""); } catch (err) { - setOptimisticList((prev) => prev.filter((m) => m !== measure)); setError( - err instanceof Error ? err.message : "Failed to instruct measure", + err instanceof Error ? err.message : "Failed to instruct measures", ); + setConfirmOpen(false); } finally { setSubmitting(false); } @@ -1597,49 +1646,44 @@ export function InstructMeasureEditor({ {outOfOrderWarning} )} - {optimisticList.length > 0 && ( -
- {optimisticList.map((m) => ( - - {m} - - ))} -
- )} -
- ))} - - +
+ )} + + {eligible.length > 0 && ( - + )} {error && (

)} + +

+ + + Instruct measures + +

+ The following measures will be instructed and approved: +

+
    + {Array.from(checked).map((m) => ( +
  • + + {m} +
    • + ))} +
+ + + + + + ); } @@ -2136,6 +2218,7 @@ export default function PropertyDetailDrawer({ diff --git a/src/app/portfolio/[slug]/(portfolio)/your-projects/live/[dealId]/DealPage.tsx b/src/app/portfolio/[slug]/(portfolio)/your-projects/live/[dealId]/DealPage.tsx index cbd680f..534d81c 100644 --- a/src/app/portfolio/[slug]/(portfolio)/your-projects/live/[dealId]/DealPage.tsx +++ b/src/app/portfolio/[slug]/(portfolio)/your-projects/live/[dealId]/DealPage.tsx @@ -551,6 +551,7 @@ export default function DealPage({