From bf857879361edf17b306f8864c851bc4fe1c9bad Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 6 May 2026 14:23:54 +0000 Subject: [PATCH] re-designing UI for PM' --- .../instructed-measures/route.ts | 1 - src/app/lib/instructMeasure.test.ts | 171 +++++++++++------- src/app/lib/instructMeasure.ts | 78 ++++---- .../live/PropertyDetailDrawer.tsx | 4 +- 4 files changed, 146 insertions(+), 108 deletions(-) diff --git a/src/app/api/portfolio/[portfolioId]/instructed-measures/route.ts b/src/app/api/portfolio/[portfolioId]/instructed-measures/route.ts index eb5830d..ff7a74d 100644 --- a/src/app/api/portfolio/[portfolioId]/instructed-measures/route.ts +++ b/src/app/api/portfolio/[portfolioId]/instructed-measures/route.ts @@ -133,7 +133,6 @@ export async function POST( return NextResponse.json({ ok: true, hubspotSync: result.hubspotSync, - autoPopulatedProposed: result.autoPopulatedProposed, hubspotError: result.hubspotError, }); } catch (err) { diff --git a/src/app/lib/instructMeasure.test.ts b/src/app/lib/instructMeasure.test.ts index 80089c7..7f8ae71 100644 --- a/src/app/lib/instructMeasure.test.ts +++ b/src/app/lib/instructMeasure.test.ts @@ -2,14 +2,14 @@ * Unit tests for the instruct-measure service. * * These tests exercise the orchestration logic — DB transaction + - * post-commit HubSpot push + auto-populate-proposed branch — by injecting - * lightweight fakes for the DB hooks. The tests never touch the real DB - * or HubSpot client. + * post-commit HubSpot push — by injecting lightweight fakes for the DB hooks. + * The tests never touch the real DB or HubSpot client. */ import { describe, expect, it, vi } from "vitest"; import { INSTRUCTED_MEASURES_PROP, PROPOSED_MEASURES_PROP, + APPROVED_MEASURES_PROP, instructMeasure, } from "./instructMeasure"; import type { @@ -31,8 +31,8 @@ function makeDeps(overrides?: { }) { const txOutcome: InstructTxOutcome = { instructedRowId: 42n, - proposedWasEmpty: false, - hadNoApprovals: false, + existingProposedMeasures: [], + allApprovedMeasureNames: [], ...overrides?.txOutcome, }; const runInstructTx: RunInstructTx = vi.fn(async () => { @@ -43,7 +43,7 @@ function makeDeps(overrides?: { async () => overrides?.instructedAfter ?? ["ASHP"], ); const syncQueue: Array<{ ok: true } | { ok: false; error: string }> = - overrides?.syncResults ?? [{ ok: true }]; + overrides?.syncResults ?? [{ ok: true }, { ok: true }, { ok: true }]; const syncMeasuresField: SyncMeasuresField = vi.fn(async () => { return syncQueue.shift() ?? ({ ok: true } as const); }); @@ -90,13 +90,13 @@ describe("instructMeasure — input validation", () => { }); describe("instructMeasure — happy path", () => { - it("commits the tx, pushes the full instructed list, stamps pushed_at", async () => { + it("commits tx, pushes instructed + proposed + approved, stamps pushed_at", async () => { const deps = makeDeps({ instructedAfter: ["ASHP", "Solar PV"], txOutcome: { instructedRowId: 99n, - proposedWasEmpty: false, - hadNoApprovals: false, + existingProposedMeasures: ["ASHP"], + allApprovedMeasureNames: ["ASHP", "Solar PV"], }, }); const result = await instructMeasure({ @@ -109,7 +109,6 @@ describe("instructMeasure — happy path", () => { ok: true, instructedRowId: 99n, hubspotSync: "ok", - autoPopulatedProposed: false, }); expect(deps.runInstructTx).toHaveBeenCalledWith({ dealId: "deal-42", @@ -117,80 +116,54 @@ describe("instructMeasure — happy path", () => { userId: 7n, notes: null, }); - expect(deps.syncMeasuresField).toHaveBeenCalledTimes(1); - expect(deps.syncMeasuresField).toHaveBeenCalledWith({ + 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).toHaveBeenCalledWith(99n); }); -}); -describe("instructMeasure — auto-populate proposed branch", () => { - it("also pushes proposed_measures when proposed was empty AND no prior approvals", async () => { + it("merges new measure into existing proposed (deduped)", async () => { const deps = makeDeps({ - instructedAfter: ["EWI"], + instructedAfter: ["ASHP", "EWI"], txOutcome: { instructedRowId: 1n, - proposedWasEmpty: true, - hadNoApprovals: true, + existingProposedMeasures: ["ASHP", "Solar PV"], + allApprovedMeasureNames: ["ASHP", "EWI"], }, }); - const result = await instructMeasure({ - dealId: "deal-blank", + await instructMeasure({ + dealId: "deal-merge", measureName: "EWI", userId: 3n, deps, }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.autoPopulatedProposed).toBe(true); - expect(result.hubspotSync).toBe("ok"); - } - expect(deps.syncMeasuresField).toHaveBeenCalledTimes(2); - expect(deps.syncMeasuresField).toHaveBeenNthCalledWith(1, { - hubspotDealId: "deal-blank", - propName: INSTRUCTED_MEASURES_PROP, - measureNames: ["EWI"], - }); expect(deps.syncMeasuresField).toHaveBeenNthCalledWith(2, { - hubspotDealId: "deal-blank", + hubspotDealId: "deal-merge", propName: PROPOSED_MEASURES_PROP, - measureNames: ["EWI"], + measureNames: ["ASHP", "Solar PV", "EWI"], }); }); - it("does NOT auto-populate proposed when proposed was empty but approvals already exist", async () => { + it("adds to proposed even when deal already had proposed measures", async () => { const deps = makeDeps({ instructedAfter: ["EWI"], txOutcome: { instructedRowId: 1n, - proposedWasEmpty: true, - hadNoApprovals: false, // an approval already existed - }, - }); - const result = await instructMeasure({ - dealId: "deal-mid-flow", - measureName: "EWI", - userId: 3n, - deps, - }); - expect(result.ok).toBe(true); - if (result.ok) expect(result.autoPopulatedProposed).toBe(false); - expect(deps.syncMeasuresField).toHaveBeenCalledTimes(1); - expect(deps.syncMeasuresField).toHaveBeenCalledWith( - expect.objectContaining({ propName: INSTRUCTED_MEASURES_PROP }), - ); - }); - - it("does NOT auto-populate proposed when proposed already has measures", async () => { - const deps = makeDeps({ - instructedAfter: ["EWI"], - txOutcome: { - instructedRowId: 1n, - proposedWasEmpty: false, - hadNoApprovals: true, + existingProposedMeasures: ["ASHP", "Solar PV"], + allApprovedMeasureNames: ["EWI"], }, }); const result = await instructMeasure({ @@ -200,8 +173,35 @@ describe("instructMeasure — auto-populate proposed branch", () => { deps, }); expect(result.ok).toBe(true); - if (result.ok) expect(result.autoPopulatedProposed).toBe(false); - expect(deps.syncMeasuresField).toHaveBeenCalledTimes(1); + expect(deps.syncMeasuresField).toHaveBeenCalledTimes(3); + expect(deps.syncMeasuresField).toHaveBeenNthCalledWith(2, + expect.objectContaining({ + propName: PROPOSED_MEASURES_PROP, + measureNames: ["ASHP", "Solar PV", "EWI"], + }), + ); + }); + + it("adds to proposed when deal has no existing proposed measures", async () => { + const deps = makeDeps({ + instructedAfter: ["EWI"], + txOutcome: { + instructedRowId: 1n, + existingProposedMeasures: [], + allApprovedMeasureNames: ["EWI"], + }, + }); + await instructMeasure({ + dealId: "deal-blank", + measureName: "EWI", + userId: 3n, + deps, + }); + expect(deps.syncMeasuresField).toHaveBeenNthCalledWith(2, { + hubspotDealId: "deal-blank", + propName: PROPOSED_MEASURES_PROP, + measureNames: ["EWI"], + }); }); }); @@ -222,15 +222,19 @@ describe("instructMeasure — DB transaction failure", () => { }); describe("instructMeasure — HubSpot push failure leaves DB committed", () => { - it("returns ok=true with hubspotSync=failed and does NOT stamp pushed_at", async () => { + it("returns ok=true with hubspotSync=failed when instructed push fails, does NOT stamp", async () => { const deps = makeDeps({ instructedAfter: ["ASHP"], txOutcome: { instructedRowId: 11n, - proposedWasEmpty: false, - hadNoApprovals: false, + existingProposedMeasures: [], + allApprovedMeasureNames: ["ASHP"], }, - syncResults: [{ ok: false, error: "hubspot 500" }], + syncResults: [ + { ok: false, error: "hubspot 500" }, + { ok: true }, + { ok: true }, + ], }); const result = await instructMeasure({ dealId: "deal-h", @@ -244,21 +248,21 @@ describe("instructMeasure — HubSpot push failure leaves DB committed", () => { hubspotSync: "failed", hubspotError: "hubspot 500", }); - expect(deps.runInstructTx).toHaveBeenCalledTimes(1); expect(deps.stampPushedAt).not.toHaveBeenCalled(); }); - it("treats a failed proposed-measures push as overall failure (still no stamp)", async () => { + it("returns hubspotSync=failed when proposed push fails", async () => { const deps = makeDeps({ instructedAfter: ["EWI"], txOutcome: { instructedRowId: 12n, - proposedWasEmpty: true, - hadNoApprovals: true, + existingProposedMeasures: [], + allApprovedMeasureNames: ["EWI"], }, syncResults: [ { ok: true }, { ok: false, error: "proposed push failed" }, + { ok: true }, ], }); const result = await instructMeasure({ @@ -271,7 +275,34 @@ describe("instructMeasure — HubSpot push failure leaves DB committed", () => { ok: true, hubspotSync: "failed", hubspotError: "proposed push failed", - autoPopulatedProposed: true, + }); + expect(deps.stampPushedAt).not.toHaveBeenCalled(); + }); + + it("returns hubspotSync=failed when approved push fails", async () => { + const deps = makeDeps({ + instructedAfter: ["EWI"], + txOutcome: { + instructedRowId: 13n, + existingProposedMeasures: [], + allApprovedMeasureNames: ["EWI"], + }, + syncResults: [ + { ok: true }, + { ok: true }, + { ok: false, error: "approved push failed" }, + ], + }); + const result = await instructMeasure({ + dealId: "deal-blank", + measureName: "EWI", + userId: 3n, + deps, + }); + expect(result).toMatchObject({ + ok: true, + hubspotSync: "failed", + hubspotError: "approved push failed", }); expect(deps.stampPushedAt).not.toHaveBeenCalled(); }); diff --git a/src/app/lib/instructMeasure.ts b/src/app/lib/instructMeasure.ts index d59bb46..a7421ca 100644 --- a/src/app/lib/instructMeasure.ts +++ b/src/app/lib/instructMeasure.ts @@ -10,18 +10,19 @@ * 1. insert a `user_defined_deal_measures` row with `source = "instructed"` * 2. insert a `deal_measure_approvals` row with `is_approved = true` * 3. insert a corresponding `deal_measure_approval_events` row - * 4. read the deal's current `proposed_measures` + total approval count + * 4. read the deal's current `proposed_measures` list + * 5. read all currently approved measure names for the deal * - * After the transaction commits, push the instructed-measures list to - * HubSpot under `instructed_measures`. If `proposed_measures` was empty - * AND the deal had no prior approvals, ALSO push the new measure as - * `proposed_measures` so the deal has a coherent measures starting point. + * After the transaction commits, push three HubSpot fields: + * - `instructed_measures`: full list of all instructed measures (audit trail) + * - `proposed_measures`: existing list merged with the new measure (always) + * - `approved_measures`: all currently approved measures for the deal * * HubSpot push failures DO NOT roll back the DB. Successful pushes stamp * `pushed_at` on the new local row; failures leave it null so a follow-up * reconcile job can retry. */ -import { and, eq, sql } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import { db } from "@/app/db/db"; import { hubspotDealData } from "@/app/db/schema/crm/hubspot_deal_table"; import { @@ -38,12 +39,15 @@ import { syncMeasuresFieldToHubSpot as defaultSyncMeasuresField } from "@/app/li export const INSTRUCTED_MEASURES_PROP = "instructed_measures"; export const PROPOSED_MEASURES_PROP = "proposed_measures"; +export const APPROVED_MEASURES_PROP = "approved_measures"; /** Snapshot returned by the transactional step of the service. */ export interface InstructTxOutcome { instructedRowId: bigint; - proposedWasEmpty: boolean; - hadNoApprovals: boolean; + /** Existing proposed measures from hubspot_deal_data before this instruction. */ + existingProposedMeasures: string[]; + /** All approved measure names for the deal after this instruction's approval row is upserted. */ + allApprovedMeasureNames: string[]; } export type SyncMeasuresField = typeof defaultSyncMeasuresField; @@ -66,7 +70,6 @@ export type InstructMeasureResult = ok: true; instructedRowId: bigint; hubspotSync: "ok" | "failed"; - autoPopulatedProposed: boolean; hubspotError?: string; } | { ok: false; error: string }; @@ -154,19 +157,20 @@ const defaultRunInstructTx: RunInstructTx = async ({ .from(hubspotDealData) .where(eq(hubspotDealData.dealId, dealId)) .limit(1); - const proposedRaw = dealRows[0]?.proposedMeasures ?? null; - const proposedWasEmpty = parseMeasures(proposedRaw).length === 0; + const existingProposedMeasures = parseMeasures(dealRows[0]?.proposedMeasures ?? null); - const approvalCountRows = await tx - .select({ count: sql`count(*)::int` }) + const approvedRows = await tx + .select({ measureName: dealMeasureApprovals.measureName }) .from(dealMeasureApprovals) - .where(eq(dealMeasureApprovals.hubspotDealId, dealId)); - const approvalCount = Number(approvalCountRows[0]?.count ?? 0); - // We just inserted (or upserted) one approval row in this tx, so any - // count > 1 means the deal already had a prior approval row before us. - const hadNoApprovals = approvalCount <= 1; + .where( + and( + eq(dealMeasureApprovals.hubspotDealId, dealId), + eq(dealMeasureApprovals.isApproved, true), + ), + ); + const allApprovedMeasureNames = approvedRows.map((r) => r.measureName); - return { instructedRowId, proposedWasEmpty, hadNoApprovals }; + return { instructedRowId, existingProposedMeasures, allApprovedMeasureNames }; }); }; @@ -240,25 +244,29 @@ export async function instructMeasure( // --------------------------------------------------------------------- const allInstructed = await readInstructed(input.dealId); + const mergedProposed = Array.from( + new Set([...txResult.existingProposedMeasures, measureName]), + ); + const instructedSync = await syncMeasuresField({ hubspotDealId: input.dealId, propName: INSTRUCTED_MEASURES_PROP, measureNames: allInstructed, }); - let proposedSync: { ok: true } | { ok: false; error: string } | null = null; - const shouldAutoPopulateProposed = - txResult.proposedWasEmpty && txResult.hadNoApprovals; - if (shouldAutoPopulateProposed) { - proposedSync = await syncMeasuresField({ - hubspotDealId: input.dealId, - propName: PROPOSED_MEASURES_PROP, - measureNames: [measureName], - }); - } + const proposedSync = await syncMeasuresField({ + hubspotDealId: input.dealId, + propName: PROPOSED_MEASURES_PROP, + measureNames: mergedProposed, + }); - const overallOk = - instructedSync.ok && (proposedSync === null || proposedSync.ok); + const approvedSync = await syncMeasuresField({ + hubspotDealId: input.dealId, + propName: APPROVED_MEASURES_PROP, + measureNames: txResult.allApprovedMeasureNames, + }); + + const overallOk = instructedSync.ok && proposedSync.ok && approvedSync.ok; if (overallOk) { try { @@ -273,21 +281,21 @@ export async function instructMeasure( ok: true, instructedRowId: txResult.instructedRowId, hubspotSync: "ok", - autoPopulatedProposed: shouldAutoPopulateProposed, }; } const hubspotError = !instructedSync.ok ? instructedSync.error - : proposedSync && !proposedSync.ok + : !proposedSync.ok ? proposedSync.error - : "HubSpot sync failed"; + : !approvedSync.ok + ? approvedSync.error + : "HubSpot sync failed"; return { ok: true, instructedRowId: txResult.instructedRowId, hubspotSync: "failed", - autoPopulatedProposed: shouldAutoPopulateProposed, hubspotError, }; } 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 1d0973d..fd9fe62 100644 --- a/src/app/portfolio/[slug]/(portfolio)/your-projects/live/PropertyDetailDrawer.tsx +++ b/src/app/portfolio/[slug]/(portfolio)/your-projects/live/PropertyDetailDrawer.tsx @@ -1384,7 +1384,7 @@ export function PibiMeasureSelector({ } const json = (await res.json()) as { ok: boolean; - hubspotSync?: "ok" | "failed"; + hubspotSync: "ok" | "failed"; hubspotError?: string; }; if (json.hubspotSync === "failed") { @@ -1565,7 +1565,7 @@ export function InstructMeasureEditor({ } const json = (await res.json()) as { ok: boolean; - hubspotSync?: "ok" | "failed"; + hubspotSync: "ok" | "failed"; hubspotError?: string; }; if (json.hubspotSync === "failed") {