re-designing UI for PM'

This commit is contained in:
Khalim Conn-Kowlessar 2026-05-06 14:23:54 +00:00
parent a754f9a7a3
commit bf85787936
4 changed files with 146 additions and 108 deletions

View file

@ -133,7 +133,6 @@ export async function POST(
return NextResponse.json({
ok: true,
hubspotSync: result.hubspotSync,
autoPopulatedProposed: result.autoPopulatedProposed,
hubspotError: result.hubspotError,
});
} catch (err) {

View file

@ -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();
});

View file

@ -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<number>`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,
};
}

View file

@ -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") {