fix(milestones): fresh projects false-flagged as stalled, fallback modules stuck at quoted, bulk ops skipped status/link logic (FIX D, E, F)

FIX D: stalledProjects treated a no-tick project's missing done_on as the
sentinel '0000-00-00', always older than any cutoff, so every
freshly-assigned project immediately showed up on the "Onboarding stalled"
tile. It's now measured against client_module.created_at (stamped by
assignModule); when that's also unknown (predates the column, or an import
path that doesn't stamp it) the project is excluded rather than assumed
ancient.

FIX E: STEP_STATUS only mapped the new template's keys, but TAIL_FALLBACK
(any module with no seeded/owner-configured tail -- WHATSAPP, ATM, AMC, ...)
still uses the legacy 'installed' key, so ticking "Installation done" on
those modules left status at 'quoted' forever. Added installed->installed
to STEP_STATUS (checked: no legacy payment key needs a mapping, since
payment never drove status even in the new vocabulary).

FIX F: bulkSetMilestone did a bare UPDATE, so a bulk go_live never advanced
client_module.status and a bulk untick never cleared a stale
payment_id/document_id link. Each id now routes through setMilestone itself
(same status-advance + link-clearing path a single tick uses), inside the
existing transaction -- nested transaction() calls become savepoints on
both engines, so a project without the milestone key is caught and skipped
without losing "bulk ops only touch projects that have the milestone".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
main
Thomas Joise 24 hours ago
parent e8baa1111e
commit 9457b0f145

@ -35,6 +35,13 @@ const STATUS_RANK: Record<string, number> = {
}
const STEP_STATUS: Record<string, string> = {
quotation_approved: 'ordered', installation: 'installed', training: 'trained', go_live: 'live',
// Pre-ship audit FIX E: TAIL_FALLBACK (used by any module with no seeded/owner-configured
// tail — e.g. WHATSAPP, ATM, AMC) still uses the legacy 'installed' key, not the new
// template vocabulary's 'installation'. Map it to the same target so those modules still
// auto-advance status (and stamp installed_on, below) instead of getting stuck at 'quoted'.
// TAIL_FALLBACK has no training-equivalent step and no payment key ever drove status, so
// no other legacy key needs a mapping here.
installed: 'installed',
}
/**
@ -330,6 +337,11 @@ export async function addProjectMilestone(
* Bulk-tick one milestone across many projects at once (D26) for onboarding cleanup
* (e.g. mark the already-live services 'installed' + 'go-live'). One transaction; a
* 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.
* 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.
*/
export async function bulkSetMilestone(
db: DB, userId: string, clientModuleIds: string[], key: string, done: boolean, doneOn?: string,
@ -342,11 +354,17 @@ export async function bulkSetMilestone(
return db.transaction(async () => {
let updated = 0
for (const id of clientModuleIds) {
const r = await db.run(
`UPDATE project_milestone SET done=?, done_on=? WHERE client_module_id=? AND key=?`,
done ? 1 : 0, stamp, id, key,
)
updated += r.changes
try {
await setMilestone(db, userId, id, key, done, doneOn)
updated++
} catch (err) {
// Bulk ops only ever touch projects that actually carry this milestone key — same
// as before this fix, just expressed via setMilestone's own "not found" error
// instead of a bare UPDATE quietly affecting 0 rows. Anything else (a genuine,
// unexpected failure) must still surface, not be swallowed.
if (err instanceof Error && err.message === 'Milestone not found on this project') continue
throw err
}
}
await writeAudit(db, userId, 'bulk_set_milestone', 'project_milestone', '*', undefined,
{ key, done, doneOn: stamp, projects: clientModuleIds.length, updated })
@ -400,15 +418,20 @@ export async function projectsPendingMilestone(db: DB, key: string): Promise<Pen
* Active projects with at least one pending step whose progress last moved more than
* `olderThanDays` ago (measured against `today`, passed in never `new Date()` here, so this
* stays testable). "Last moved" is the most recent `done_on` across the project's ticked
* steps; a project with nothing ticked yet counts as parked since creation (no lower bound).
* steps; a project with nothing ticked yet is measured from `client_module.created_at`
* instead (FIX D treating a NULL last-moved date as an infinitely old one made every
* freshly-assigned project show up as stalled immediately). When even that isn't known (a
* project created before the created_at column existed, or carried over by an import that
* doesn't stamp it) there is no way to prove it's actually old, so it is excluded rather than
* guessed at the intent is "parked past N days", not "we don't know, so assume forever".
* Non-aggregated columns are wrapped in MIN() grouping by `pm.client_module_id` alone isn't
* enough for Postgres to infer they're single-valued per group (D15 portability).
*/
export async function stalledProjects(db: DB, olderThanDays: number, today: string): Promise<PendingProject[]> {
const rows = await db.all<PendingProject & { last_done: string | null }>(
const rows = await db.all<PendingProject & { last_done: string | null; created_at: string | null }>(
`SELECT pm.client_module_id AS "clientModuleId", MIN(cm.client_id) AS "clientId",
MIN(c.name) AS "clientName", MIN(m.code) AS "moduleCode", MIN(m.name) AS "moduleName",
MAX(pm.done_on) AS last_done
MAX(pm.done_on) AS last_done, MIN(cm.created_at) AS created_at
FROM project_milestone pm
JOIN client_module cm ON cm.id = pm.client_module_id
JOIN client c ON c.id = cm.client_id
@ -419,6 +442,10 @@ export async function stalledProjects(db: DB, olderThanDays: number, today: stri
)
const cutoff = new Date(Date.parse(today) - olderThanDays * 86_400_000).toISOString().slice(0, 10)
return rows
.filter((r) => (r.last_done ?? '0000-00-00') < cutoff)
.map(({ last_done: _last_done, ...rest }) => rest)
.filter((r) => {
if (r.last_done !== null) return r.last_done < cutoff
if (r.created_at !== null) return r.created_at.slice(0, 10) < cutoff
return false
})
.map(({ last_done: _last_done, created_at: _created_at, ...rest }) => rest)
}

@ -491,9 +491,12 @@ export async function assignModule(db: DB, userId: string, input: AssignModuleIn
}
const id = uuidv7()
await db.run(
`INSERT INTO client_module (id, client_id, module_id, status, kind, edition)
VALUES (?, ?, ?, ?, ?, ?)`,
// created_at (FIX D — stalled-onboarding false positives): real assignment time, so a
// brand-new project with zero ticks is measured from here, not treated as ancient.
`INSERT INTO client_module (id, client_id, module_id, status, kind, edition, created_at)
VALUES (?, ?, ?, ?, ?, ?, ?)`,
id, input.clientId, input.moduleId, status, input.kind, input.edition ?? 'standard',
new Date().toISOString(),
)
await ensureProjectMilestones(db, id) // D22: seed the onboarding checklist for the new project
const cm = (await getClientModule(db, id))!

@ -3,10 +3,10 @@ import { describe, it, expect } from 'vitest'
import { openDb } from '../src/db'
import { seedIfEmpty } from '../src/seed'
import { createClient, bulkUpdateClients, getClient } from '../src/repos-clients'
import { createModule, setPrice, assignModule } from '../src/repos-modules'
import { createModule, setPrice, assignModule, getClientModule } from '../src/repos-modules'
import { createDraft, issueDocument, createCreditNote, listDocuments, getDocument } from '../src/repos-documents'
import { gstSummary } from '../src/repos-reports'
import { listProjectMilestones, bulkSetMilestone } from '../src/repos-milestones'
import { listProjectMilestones, bulkSetMilestone, setMilestone } from '../src/repos-milestones'
async function world() {
const db = openDb(':memory:'); await seedIfEmpty(db) // company state 32, GST18
@ -81,4 +81,48 @@ describe('bulk operations (D26)', () => {
const ms = await listProjectMilestones(db, p.id)
expect(ms.find((x) => x.key === 'installed')).toMatchObject({ done: true, doneOn: '2026-07-10' })
})
// Pre-ship audit FIX F: bulkSetMilestone used a bare UPDATE, so it never drove
// client_module.status (a bulk go_live left status at 'quoted') and never cleared a stale
// payment/document link on untick. It now routes through setMilestone — the same path a
// single tick uses.
it('bulk-tick drives client_module.status the same as a single tick (FIX F)', async () => {
const { db, c, m } = await world()
const p = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' })
const out = await bulkSetMilestone(db, 'u1', [p.id], 'go_live', true, '2026-07-10')
expect(out.updated).toBe(1)
const cm = await getClientModule(db, p.id)
expect(cm!.status).toBe('live')
})
it('bulk-untick clears a stale payment/document link, same as a single untick (FIX F)', async () => {
const { db, c, m } = await world()
const p = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' })
const paymentId = 'pay-bulk-1'
await db.run(
`INSERT INTO payment (id, client_id, received_on, mode, amount_paise, created_by, created_at)
VALUES (?, ?, '2026-07-01', 'bank', 500000, 'u1', '2026-07-01T00:00:00.000Z')`,
paymentId, c.id,
)
// Single-tick with a linked payment (as the UI does)...
await setMilestone(db, 'u1', p.id, 'advance_paid', true, '2026-07-01', paymentId)
let ms = await listProjectMilestones(db, p.id)
expect(ms.find((x) => x.key === 'advance_paid')!.paymentId).toBe(paymentId)
// ...then a bulk untick must clear that link, not leave it stale.
const out = await bulkSetMilestone(db, 'u1', [p.id], 'advance_paid', false)
expect(out.updated).toBe(1)
ms = await listProjectMilestones(db, p.id)
const advancePaid = ms.find((x) => x.key === 'advance_paid')!
expect(advancePaid.done).toBe(false)
expect(advancePaid.doneOn).toBeNull()
expect(advancePaid.paymentId).toBeNull()
})
it('bulk ops only touch projects that actually have the milestone key, others are silently skipped', async () => {
const { db, c, m } = await world()
const p = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' })
const out = await bulkSetMilestone(db, 'u1', [p.id, 'not-a-real-client-module-id'], 'go_live', true, '2026-07-10')
expect(out.updated).toBe(1) // only the real one counted
})
})

@ -109,3 +109,26 @@ describe('milestone-driven installed_on / trained_on stamping (FIX 5)', () => {
expect(JSON.parse(stamped!.after_json!)).toMatchObject({ status: 'installed', installedOn: '2026-03-25' })
})
})
// 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
// 'installed' key and has no training step. So ticking "Installation done" on one of those
// modules left status at 'quoted' and installed_on NULL forever (the UI's date writers were
// removed with nothing to replace them there). STEP_STATUS now maps the legacy key too.
describe('fallback-template (TAIL_FALLBACK) modules auto-advance too (FIX E)', () => {
it("ticking the legacy 'installed' step (no seeded per-module template) advances status to installed and stamps installed_on", async () => {
const db = openDb(':memory:')
await seedIfEmpty(db)
const c = await createClient(db, 'u1', { name: 'Kothavara SCB', stateCode: '32' })
// A module code with no seeded per-module tail (seed.ts only carries SMS/RTGS/MOBILEAPP
// overrides) and no global tail setting -> resolves to the code TAIL_FALLBACK, whose
// install step is keyed 'installed', not 'installation'.
const m = await createModule(db, 'u1', { code: 'ATM', name: 'ATM Service' })
const cm = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' })
await setMilestone(db, 'u1', cm.id, 'installed', true, '2026-03-25')
const after = await getClientModule(db, cm.id)
expect(after!.status).toBe('installed')
expect(after!.installedOn).toBe('2026-03-25')
})
})

@ -54,4 +54,49 @@ describe('stalledProjects (D22 — dashboard/MIS tile)', () => {
// 'live' status (from ticking go_live) also excludes it, which the loop above proves.
expect((await db.get<{ status: string }>(`SELECT status FROM client_module WHERE id=?`, done.id))!.status).toBe('live')
})
// Pre-ship audit FIX D: a project with ZERO ticks used to be measured against the sentinel
// '0000-00-00', which is always older than any cutoff — so a project assigned moments ago
// immediately showed up as "stalled" on the dashboard tile. It should instead be measured
// from client_module.created_at (stamped by assignModule), and excluded entirely when that
// isn't known (never guessed to be old).
describe('a no-tick project is measured from its creation date, not treated as infinitely old (FIX D)', () => {
it('a project assigned "today" with zero ticks is NOT stalled', async () => {
const { db, c, m } = await world()
const cm = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' })
// Deterministic: pin created_at to the test's fictional "today" rather than relying on
// assignModule's real wall-clock stamp matching whatever the test's fixed date happens
// to be — this test is about the QUERY logic, not the real-time write path.
await db.run(`UPDATE client_module SET created_at=? WHERE id=?`, '2026-07-19T09:00:00.000Z', cm.id)
const stalled = await stalledProjects(db, 30, '2026-07-19')
expect(stalled.map((p) => p.clientModuleId)).not.toContain(cm.id)
})
it('a project created long ago with zero ticks IS stalled', async () => {
const { db, c, m } = await world()
const cm = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' })
await db.run(`UPDATE client_module SET created_at=? WHERE id=?`, '2025-01-01T09:00:00.000Z', cm.id)
const stalled = await stalledProjects(db, 30, '2026-07-19')
expect(stalled.map((p) => p.clientModuleId)).toContain(cm.id)
})
it('a no-tick project with an UNKNOWN creation date (created_at NULL) is excluded, not assumed ancient', async () => {
const { db, c, m } = await world()
const cm = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' })
// Simulates a client_module that predates the created_at column (or an import that
// doesn't stamp it) — created_at is genuinely unknown.
await db.run(`UPDATE client_module SET created_at=NULL WHERE id=?`, cm.id)
const stalled = await stalledProjects(db, 0, '2026-07-19') // even a 0-day cutoff must not flag it
expect(stalled.map((p) => p.clientModuleId)).not.toContain(cm.id)
})
it('assignModule itself stamps a real (non-null) created_at', async () => {
const { db, c, m } = await world()
const cm = await assignModule(db, 'u1', { clientId: c.id, moduleId: m.id, kind: 'yearly' })
const row = await db.get<{ created_at: string | null }>(
`SELECT created_at FROM client_module WHERE id=?`, cm.id,
)
expect(row!.created_at).not.toBeNull()
})
})
})

Loading…
Cancel
Save