diff --git a/apps/hq/src/repos-milestones.ts b/apps/hq/src/repos-milestones.ts index 9e4b04c..f2dc600 100644 --- a/apps/hq/src/repos-milestones.ts +++ b/apps/hq/src/repos-milestones.ts @@ -45,13 +45,25 @@ const STEP_STATUS: Record = { } /** - * Also replaces the removed UI date-writers (Client Detail redesign dropped `RowDate` / - * the summary-table date columns without a replacement): the first time status crosses - * into 'installed' / 'trained', stamp client_module.installed_on / trained_on from the - * triggering milestone's own done_on — never overwriting a date already on file (e.g. one - * set by the APEX importer or a direct edit). + * The two things ticking a milestone does to its parent `client_module`, applied in the + * caller's transaction and audited together: + * + * 1. **Status**, advanced to the step's target — never downgraded. + * 2. **installed_on / trained_on**, stamped from the triggering milestone's own done_on. + * This also replaces the removed UI date-writers (the Client Detail redesign dropped + * `RowDate` / the summary-table date columns without a replacement), so a tick is now the + * ONLY way those columns get filled. + * + * The two are INDEPENDENT (bug fix): the date used to be stamped only as a side effect of a + * status upgrade, so on the ~300 APEX-imported projects — already at `live`, the furthest + * status — ticking Installation or Training upgraded nothing and therefore recorded nothing, + * leaving the Module Directory's "Installed" column blank forever with no way to fill it. + * Ticking the step now stamps the date whether or not the coarse status moves. Still + * write-once: a date already on file (from the APEX importer or an earlier tick) is never + * overwritten. If neither the status nor a date would change, nothing is written and no audit + * row is added — a re-tick stays a no-op. */ -async function advanceStatusForStep( +async function applyStepEffects( db: DB, userId: string, clientModuleId: string, key: string, doneOn: string | null, ): Promise { const target = STEP_STATUS[key] @@ -59,11 +71,14 @@ async function advanceStatusForStep( const cm = await db.get<{ status: string; installed_on: string | null; trained_on: string | null }>( `SELECT status, installed_on, trained_on FROM client_module WHERE id=?`, clientModuleId) if (cm === undefined) return - if ((STATUS_RANK[cm.status] ?? 0) >= (STATUS_RANK[target] ?? 0)) return // never downgrade - const sets = ['status=?'] - const args: unknown[] = [target] - const before: Record = { status: cm.status } - const after: Record = { status: target } + const sets: string[] = [] + const args: unknown[] = [] + const before: Record = {} + const after: Record = {} + if ((STATUS_RANK[cm.status] ?? 0) < (STATUS_RANK[target] ?? 0)) { // never downgrade + sets.push('status=?'); args.push(target) + before['status'] = cm.status; after['status'] = target + } if (target === 'installed' && cm.installed_on === null && doneOn !== null) { sets.push('installed_on=?'); args.push(doneOn) before['installedOn'] = null; after['installedOn'] = doneOn @@ -72,6 +87,7 @@ async function advanceStatusForStep( sets.push('trained_on=?'); args.push(doneOn) before['trainedOn'] = null; after['trainedOn'] = doneOn } + if (sets.length === 0) return args.push(clientModuleId) await db.run(`UPDATE client_module SET ${sets.join(', ')} WHERE id=?`, ...args) await writeAudit(db, userId, 'update', 'client_module', clientModuleId, before, after) @@ -266,6 +282,13 @@ export async function listProjectMilestones(db: DB, clientModuleId: string): Pro * and its `received_on` mirrors into `done_on` unless an explicit `doneOn` is also given. * A quotation-step tick (`quotation_sent`) can likewise pass `documentId` to link the * quotation / proforma that was sent — it must belong to the same client too. Audited. + * + * Link lifecycle (bug fix): re-ticking an ALREADY-DONE step without resending its ids keeps + * the links already on file. They used to be written as `done ? linked : null`, so any tick + * that didn't carry the ids wiped them — most visibly `bulkSetMilestone`, which never passes + * them, so one bulk re-tick silently cut every project's step loose from its payment / + * document. Passing an explicit empty string still clears a link (that is the "unlink" the + * picker sends), and unticking still clears both, as before. */ export async function setMilestone( db: DB, userId: string, clientModuleId: string, key: string, done: boolean, @@ -282,7 +305,9 @@ export async function setMilestone( throw new Error('doneOn must be YYYY-MM-DD') } let stamp = done ? (doneOn ?? new Date().toISOString().slice(0, 10)) : null - let linkedPayment: string | null = null + // Nothing supplied for an already-done step means "leave the link alone", not "clear it". + const wasDone = before.done === 1 + let linkedPayment: string | null = paymentId === undefined && wasDone ? before.payment_id : null if (done && paymentId !== undefined && paymentId !== '') { const pay = await db.get<{ received_on: string }>( `SELECT received_on FROM payment WHERE id=? AND client_id=(SELECT client_id FROM client_module WHERE id=?)`, @@ -292,7 +317,7 @@ export async function setMilestone( linkedPayment = paymentId if (doneOn === undefined) stamp = pay.received_on // link date wins unless caller forced one } - let linkedDocument: string | null = null + let linkedDocument: string | null = documentId === undefined && wasDone ? before.document_id : null if (done && documentId !== undefined && documentId !== '') { const doc = await db.get<{ id: string }>( `SELECT id FROM document WHERE id=? AND client_id=(SELECT client_id FROM client_module WHERE id=?)`, @@ -301,14 +326,17 @@ export async function setMilestone( if (doc === undefined) throw new Error('Document not found for this client') linkedDocument = documentId } + // Unticking always clears both links, whatever was resolved above. + const finalPayment = done ? linkedPayment : null + const finalDocument = done ? linkedDocument : null await db.run( `UPDATE project_milestone SET done=?, done_on=?, payment_id=?, document_id=? WHERE client_module_id=? AND key=?`, - done ? 1 : 0, stamp, done ? linkedPayment : null, done ? linkedDocument : null, clientModuleId, key, + done ? 1 : 0, stamp, finalPayment, finalDocument, clientModuleId, key, ) await writeAudit(db, userId, 'set_milestone', 'client_module', clientModuleId, undefined, - { key, done, doneOn: stamp, paymentId: linkedPayment, documentId: linkedDocument }) - if (done) await advanceStatusForStep(db, userId, clientModuleId, key, stamp) + { key, done, doneOn: stamp, paymentId: finalPayment, documentId: finalDocument }) + if (done) await applyStepEffects(db, userId, clientModuleId, key, stamp) return listProjectMilestones(db, clientModuleId) }) } @@ -339,9 +367,13 @@ export async function addProjectMilestone( * single summary audit row. Only touches projects that actually have the milestone. * * FIX F: routes each project through `setMilestone` — the SAME status-advance and - * payment/document link-clearing logic a single tick uses — instead of a bare UPDATE. + * payment/document link logic a single tick uses — instead of a bare UPDATE. * Before this, a bulk `go_live` left `client_module.status` at 'quoted' (never advanced) * and a bulk untick left a stale `payment_id`/`document_id` link on the row. + * + * A bulk call never carries per-project ids, so it relies on `setMilestone` PRESERVING the + * links already on an already-done step (see there) — otherwise re-ticking a step in bulk + * would wipe every payment/document link that had been set one at a time. */ export async function bulkSetMilestone( db: DB, userId: string, clientModuleIds: string[], key: string, done: boolean, doneOn?: string, diff --git a/apps/hq/test/milestone-link-preservation.test.ts b/apps/hq/test/milestone-link-preservation.test.ts new file mode 100644 index 0000000..bf509a7 --- /dev/null +++ b/apps/hq/test/milestone-link-preservation.test.ts @@ -0,0 +1,113 @@ +// apps/hq/test/milestone-link-preservation.test.ts — re-ticking an already-done step must not +// wipe the payment / document link it already carries. +// +// The bug: setMilestone wrote `payment_id`/`document_id` as `done ? linked : null`, so any tick +// that didn't resend the ids cleared them. bulkSetMilestone routes through setMilestone and +// never passes ids at all (there is one key for many projects), so a single bulk re-tick — the +// onboarding-cleanup tool — silently cut every project's step loose from the payment or +// quotation it had been linked to one at a time. Unticking must still clear both. +import { describe, it, expect } from 'vitest' +import { openDb } from '../src/db' +import { seedIfEmpty } from '../src/seed' +import { createClient } from '../src/repos-clients' +import { createModule, setPrice, assignModule } from '../src/repos-modules' +import { createDraft } from '../src/repos-documents' +import { recordPayment } from '../src/repos-payments' +import { bulkSetMilestone, listProjectMilestones, setMilestone } from '../src/repos-milestones' + +async function world() { + const db = openDb(':memory:') + await seedIfEmpty(db) + const c = await createClient(db, 'u1', { name: 'Kothavara SCB', stateCode: '32' }) + const m = await createModule(db, 'u1', { code: 'SMS', name: 'Bulk SMS' }) + await setPrice(db, 'u1', { moduleId: m.id, kind: 'yearly', pricePaise: 10_000_00, effectiveFrom: '2020-01-01' }) + const cm = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' }) + const step = async (key: string) => (await listProjectMilestones(db, cm.id)).find((s) => s.key === key)! + return { db, clientId: c.id, moduleId: m.id, cmId: cm.id, step } +} + +describe('bulk re-tick preserves the payment link (5a)', () => { + it('tick with a payment link → bulk re-tick keeps it → bulk untick clears it', async () => { + const { db, clientId, cmId, step } = await world() + const pay = await recordPayment(db, 'u1', { + clientId, receivedOn: '2026-05-01', mode: 'bank', amountPaise: 1000_00, + }) + await setMilestone(db, 'u1', cmId, 'payment_received', true, undefined, pay.payment.id) + expect((await step('payment_received')).paymentId).toBe(pay.payment.id) + + // The bulk tool carries no ids — the stored link must survive. + const res = await bulkSetMilestone(db, 'u1', [cmId], 'payment_received', true, '2026-06-01') + expect(res.updated).toBe(1) + const after = await step('payment_received') + expect(after.done).toBe(true) + expect(after.paymentId).toBe(pay.payment.id) + + // Unticking still cuts the link loose. + await bulkSetMilestone(db, 'u1', [cmId], 'payment_received', false) + const cleared = await step('payment_received') + expect(cleared.done).toBe(false) + expect(cleared.doneOn).toBeNull() + expect(cleared.paymentId).toBeNull() + }) + + it('a single re-tick that sends no ids keeps the link too', async () => { + const { db, clientId, cmId, step } = await world() + const pay = await recordPayment(db, 'u1', { + clientId, receivedOn: '2026-05-01', mode: 'bank', amountPaise: 1000_00, + }) + await setMilestone(db, 'u1', cmId, 'payment_received', true, undefined, pay.payment.id) + await setMilestone(db, 'u1', cmId, 'payment_received', true, '2026-06-01') + const s = await step('payment_received') + expect(s.doneOn).toBe('2026-06-01') // the date the caller did send still moves + expect(s.paymentId).toBe(pay.payment.id) + }) + + it('an explicit empty id is still an unlink, not a preserve', async () => { + const { db, clientId, cmId, step } = await world() + const pay = await recordPayment(db, 'u1', { + clientId, receivedOn: '2026-05-01', mode: 'bank', amountPaise: 1000_00, + }) + await setMilestone(db, 'u1', cmId, 'payment_received', true, undefined, pay.payment.id) + await setMilestone(db, 'u1', cmId, 'payment_received', true, undefined, '') + expect((await step('payment_received')).paymentId).toBeNull() + }) + + it('the preserved link is what the audit row reports', async () => { + const { db, clientId, cmId } = await world() + const pay = await recordPayment(db, 'u1', { + clientId, receivedOn: '2026-05-01', mode: 'bank', amountPaise: 1000_00, + }) + await setMilestone(db, 'u1', cmId, 'payment_received', true, undefined, pay.payment.id) + await bulkSetMilestone(db, 'u1', [cmId], 'payment_received', true, '2026-06-01') + const audits = await db.all<{ after_json: string | null }>( + `SELECT after_json FROM audit_log WHERE action='set_milestone' ORDER BY id`, + ) + expect(JSON.parse(audits.at(-1)!.after_json!)).toMatchObject({ paymentId: pay.payment.id }) + }) +}) + +describe('bulk re-tick preserves the document link (5a)', () => { + it('tick with a quotation link → bulk re-tick keeps it → untick clears it', async () => { + const { db, clientId, moduleId, cmId, step } = await world() + const doc = await createDraft(db, 'u1', { + docType: 'QUOTATION', clientId, lines: [{ moduleId, qty: 1, kind: 'yearly' }], + }) + await setMilestone(db, 'u1', cmId, 'quotation_sent', true, '2026-05-01', undefined, doc.id) + expect((await step('quotation_sent')).documentId).toBe(doc.id) + + await bulkSetMilestone(db, 'u1', [cmId], 'quotation_sent', true, '2026-06-01') + expect((await step('quotation_sent')).documentId).toBe(doc.id) + + await bulkSetMilestone(db, 'u1', [cmId], 'quotation_sent', false) + expect((await step('quotation_sent')).documentId).toBeNull() + }) + + it('a step that was never done still starts with no link (nothing to preserve)', async () => { + const { db, cmId, step } = await world() + await bulkSetMilestone(db, 'u1', [cmId], 'quotation_sent', true, '2026-06-01') + const s = await step('quotation_sent') + expect(s.done).toBe(true) + expect(s.documentId).toBeNull() + expect(s.paymentId).toBeNull() + }) +}) diff --git a/apps/hq/test/milestone-status.test.ts b/apps/hq/test/milestone-status.test.ts index 2ff2621..285f3ce 100644 --- a/apps/hq/test/milestone-status.test.ts +++ b/apps/hq/test/milestone-status.test.ts @@ -110,6 +110,63 @@ describe('milestone-driven installed_on / trained_on stamping (FIX 5)', () => { }) }) +// The date-stamp used to ride on the status UPGRADE, so a project already at the furthest +// status could never record one. That is the state of all ~300 APEX-imported projects: they +// come in at 'live', ticking Installation/Training upgrades nothing, and — with the UI's own +// date writers removed — installed_on/trained_on became permanently unfillable, leaving the +// Module Directory's "Installed" column blank forever. Ticking now stamps regardless. +describe('date stamping is independent of the status upgrade (APEX-imported projects at live)', () => { + it("ticking 'installation' on a project already at live records installed_on and leaves the status alone", async () => { + const { db, cmId } = await world() + await updateClientModule(db, 'u1', cmId, { status: 'live' }) + await setMilestone(db, 'u1', cmId, 'installation', true, '2026-03-25') + const cm = await getClientModule(db, cmId) + expect(cm!.installedOn).toBe('2026-03-25') + expect(cm!.status).toBe('live') // unchanged — no downgrade, no spurious upgrade + }) + + it("ticking 'training' on a project already at live records trained_on and leaves the status alone", async () => { + const { db, cmId } = await world() + await updateClientModule(db, 'u1', cmId, { status: 'live' }) + await setMilestone(db, 'u1', cmId, 'training', true, '2026-05-02') + const cm = await getClientModule(db, cmId) + expect(cm!.trainedOn).toBe('2026-05-02') + expect(cm!.status).toBe('live') + }) + + it('still never overwrites a date already on file, even with no status move', async () => { + const { db, cmId } = await world() + await updateClientModule(db, 'u1', cmId, { status: 'live', installedOn: '2025-01-01' }) + await setMilestone(db, 'u1', cmId, 'installation', true, '2026-03-25') + expect((await getClientModule(db, cmId))!.installedOn).toBe('2025-01-01') + }) + + it('the audit payload records the date-only change (no status key, since status did not move)', async () => { + const { db, cmId } = await world() + await updateClientModule(db, 'u1', cmId, { status: 'live' }) + await setMilestone(db, 'u1', cmId, 'installation', true, '2026-03-25') + const audits = await db.all<{ after_json: string | null }>( + `SELECT after_json FROM audit_log WHERE entity='client_module' AND entity_id=? AND action='update' ORDER BY id`, + cmId, + ) + // The last 'update' row is the tick's own (the earlier one is the status seed above). + const after = JSON.parse(audits.at(-1)!.after_json!) as Record + expect(after['installedOn']).toBe('2026-03-25') + expect(after['status']).toBeUndefined() + }) + + it('a re-tick that changes nothing writes no client_module audit row at all', async () => { + const { db, cmId } = await world() + await updateClientModule(db, 'u1', cmId, { status: 'live', installedOn: '2025-01-01' }) + const countBefore = (await db.all( + `SELECT id FROM audit_log WHERE entity='client_module' AND entity_id=? AND action='update'`, cmId)).length + await setMilestone(db, 'u1', cmId, 'installation', true, '2026-03-25') + const countAfter = (await db.all( + `SELECT id FROM audit_log WHERE entity='client_module' AND entity_id=? AND action='update'`, cmId)).length + expect(countAfter).toBe(countBefore) + }) +}) + // Pre-ship audit FIX E: STEP_STATUS only had keys from the NEW template vocabulary // (installation/training/go_live/quotation_approved), but TAIL_FALLBACK — used by any module // with no seeded/owner-configured tail (e.g. WHATSAPP, ATM, AMC) — still uses the legacy