feat(clients): account-owner plumbing — client.owner_id, audited owner route, role gate

Phase 3 of the quote-to-close funnel spec: client.owner_id added additively
(SCHEMA + PRAGMA-guarded migrate) so a bare lead is routable to an employee;
setClientOwner repo fn (validates active employee, audits set_owner in-txn);
PATCH /clients/:id/owner gated inline to owner/manager; shared ownerScope role
gate (staff forced to self, widening ignored) for later phases; ClientDetail
gains an owner/manager-only Account owner dropdown fed by GET /employees.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat/client-detail-redesign
Thomas Joise 5 days ago
parent ffba5c3d3e
commit efdb34e710

@ -12,11 +12,17 @@ export function displayName(): string {
return localStorage.getItem(NAME_KEY) ?? ''
}
/** 'owner' | 'staff' — Modules page renders read-only for staff. */
/** 'owner' | 'manager' | 'staff' — pages gate write actions on this. */
export function role(): string {
return localStorage.getItem(ROLE_KEY) ?? 'staff'
}
/** True for the managerial roles (owner|manager); mirrors the server-side gate. */
export function isManagerial(): boolean {
const r = role()
return r === 'owner' || r === 'manager'
}
export function clearSession(): void {
localStorage.removeItem(TOKEN_KEY)
localStorage.removeItem(NAME_KEY)
@ -72,6 +78,8 @@ export const CLIENT_STATUSES: ClientStatus[] = ['lead', 'active', 'dormant', 'lo
export interface Client {
id: string; code: string; name: string; gstin?: string; stateCode: string
address: string; contacts: ClientContact[]; status: ClientStatus; notes: string
/** Account/enquiry owner (→ employee id); absent = unassigned. */
ownerId?: string
}
export type Kind = 'one_time' | 'monthly' | 'yearly' | 'usage'
@ -163,6 +171,9 @@ export const createClient = (body: Record<string, unknown>): Promise<Client> =>
apiFetch<{ client: Client }>('/clients', { method: 'POST', body: JSON.stringify(body) }).then((r) => r.client)
export const patchClient = (id: string, body: Record<string, unknown>): Promise<Client> =>
apiFetch<{ client: Client }>(`/clients/${id}`, { method: 'PATCH', body: JSON.stringify(body) }).then((r) => r.client)
/** Assign/clear the account owner — owner/manager only; audited server-side. `null` clears. */
export const setClientOwner = (id: string, ownerId: string | null): Promise<Client> =>
apiFetch<{ client: Client }>(`/clients/${id}/owner`, { method: 'PATCH', body: JSON.stringify({ ownerId }) }).then((r) => r.client)
export const getModules = (): Promise<Module[]> =>
apiFetch<{ modules: Module[] }>('/modules').then((r) => r.modules)

@ -5,13 +5,14 @@ import { Badge, Button, DataTable, EmptyState, Field, Notice, PageHeader, StatCa
import {
assignClientModule, getClient, getClientModules, getLedger, getModules,
patchClient, patchClientModule, recordPayment,
role, getRecurringPlans, createRecurringPlan, deactivateRecurringPlan,
role, isManagerial, getEmployees, setClientOwner,
getRecurringPlans, createRecurringPlan, deactivateRecurringPlan,
getAmc, createAmc, deactivateAmc, generateAmcRenewalInvoice,
getInteractions, getInteractionTypes, createInteraction, updateInteraction,
getClientAwsUsage,
CLIENT_MODULE_STATUSES, CLIENT_STATUSES, KIND_LABEL, PAYMENT_MODES,
type AmcPaidStatus, type ClientModule, type ClientModuleStatus, type ClientStatus,
type InteractionType, type Kind, type Module, type Outcome, type PaymentMode,
type Employee, type InteractionType, type Kind, type Module, type Outcome, type PaymentMode,
} from '../api'
import { CLIENT_TONE, DOC_TONE, useData } from './Clients'
@ -34,7 +35,9 @@ export function ClientDetail() {
const interactions = useData(() => getInteractions(id), [id])
const types = useData(getInteractionTypes, [])
const awsUsage = useData(() => getClientAwsUsage(id), [id])
const employees = useData(() => getEmployees(), [])
const isOwner = role() === 'owner'
const canRoute = isManagerial() // account-owner routing is owner/manager only (server-enforced too)
const [actionErr, setActionErr] = useState<string | undefined>()
const c = client.data
@ -67,6 +70,17 @@ export function ClientDetail() {
{c.contacts.map((ct, i) => (
<Badge key={i}>{[ct.name, ct.email, ct.phone].filter((x) => x !== undefined && x !== '').join(' · ')}</Badge>
))}
{canRoute && employees.data !== undefined && (
<AccountOwnerSelect
ownerId={c.ownerId}
employees={employees.data.employees}
onChange={(ownerId) => {
setActionErr(undefined)
setClientOwner(id, ownerId)
.then(client.reload).catch((err: Error) => setActionErr(err.message))
}}
/>
)}
</Toolbar>
{actionErr !== undefined && <Notice tone="err">{actionErr}</Notice>}
@ -301,6 +315,31 @@ export function ClientDetail() {
)
}
/**
* Owner/manager only (server re-checks): pick the employee who owns this account,
* so a lead with no quote is still routable to someone's queue. Writes client.owner_id
* via the audited PATCH /clients/:id/owner. Inactive employees are hidden from the
* picker, but a current owner who was since deactivated still renders.
*/
function AccountOwnerSelect(props: {
ownerId?: string; employees: Employee[]; onChange: (ownerId: string | null) => void
}) {
return (
<label style={{ display: 'inline-flex', alignItems: 'center', gap: 6 }}>
<span style={{ color: 'var(--muted, #6b7280)', fontSize: 13 }}>Account owner</span>
<select
className="wf" value={props.ownerId ?? ''}
onChange={(e) => props.onChange(e.target.value !== '' ? e.target.value : null)}
>
<option value=""> unassigned </option>
{props.employees
.filter((emp) => emp.active || emp.id === props.ownerId)
.map((emp) => <option key={emp.id} value={emp.id}>{emp.displayName}</option>)}
</select>
</label>
)
}
/** Owner-only: create a recurring plan on one of the client's modules. */
function NewRecurringForm(props: {
clientId: string; options: { id: string; label: string }[]; onDone: () => void

@ -1,9 +1,9 @@
import { Router, type Request, type RequestHandler, type Response } from 'express'
import { fyOf, validateGstin } from '@sims/domain'
import { login, requireAuth, requireOwner } from './auth'
import { isManagerial, login, requireAuth, requireOwner } from './auth'
import type { DB } from './db'
import {
createClient, getClient, listClients, updateClient,
createClient, getClient, listClients, setClientOwner, updateClient,
type Client, type ClientInput, type ClientPatch,
} from './repos-clients'
import {
@ -179,6 +179,29 @@ export function apiRouter(
res.status(400).json({ ok: false, error: err instanceof Error ? err.message : String(err) })
}
})
// Account-owner routing — owner/manager only (inline check per spec: no requireManager
// middleware). Separate from the staff-open generic PATCH above so staff cannot set or
// widen ownership; the repo audits the write as its own 'set_owner' action.
r.patch('/clients/:id/owner', requireAuth, (req, res) => {
const viewer = res.locals['staff'] as { id: string; role: string }
if (!isManagerial(viewer.role)) {
res.status(403).json({ ok: false, error: 'Owner or manager only' }); return
}
const id = String(req.params['id'] ?? '')
if (getClient(db, id) === null) {
res.status(404).json({ ok: false, error: 'Client not found' }); return
}
try {
const raw = (req.body as { ownerId?: unknown }).ownerId
if (raw !== null && typeof raw !== 'string') {
throw new Error('ownerId is required — an employee id, or null to clear')
}
const client = setClientOwner(db, viewer.id, id, raw === null || raw === '' ? null : raw)
res.json({ ok: true, client })
} catch (err) {
res.status(400).json({ ok: false, error: err instanceof Error ? err.message : String(err) })
}
})
// ---------- modules ----------
r.get('/modules', requireAuth, (_req, res) => {

@ -58,3 +58,20 @@ export const requireOwner: RequestHandler = (_req, res, next) => {
}
next()
}
/** True for the managerial roles (owner|manager) allowed to see/act beyond their own rows. */
export const isManagerial = (role: string): boolean => role === 'owner' || role === 'manager'
/**
* The shared server-side role gate for owner-scoped queries. Input comes from
* res.locals.staff never from the request body. Staff are forced to their own
* rows and any widening param is ignored; owner/manager are unfiltered by default
* and may narrow via `requestedOwnerId`. Returns the employee id every owner-scoped
* query must filter by, or undefined for no filter.
*/
export function ownerScope(
viewer: { id: string; role: string }, requestedOwnerId?: string,
): string | undefined {
if (!isManagerial(viewer.role)) return viewer.id
return requestedOwnerId !== undefined && requestedOwnerId !== '' ? requestedOwnerId : undefined
}

@ -19,6 +19,7 @@ CREATE TABLE IF NOT EXISTS client (
gstin TEXT, state_code TEXT NOT NULL DEFAULT '32', address TEXT NOT NULL DEFAULT '',
contacts TEXT NOT NULL DEFAULT '[]', -- JSON [{name,phone,email,role}]
status TEXT NOT NULL DEFAULT 'active' CHECK (status IN ('lead','active','dormant','lost')),
owner_id TEXT, -- account/enquiry owner staff_user.id; NULL = unassigned (routing, D-EMP A4)
notes TEXT NOT NULL DEFAULT '', source TEXT NOT NULL DEFAULT 'hq', created_at TEXT NOT NULL
);
CREATE TABLE IF NOT EXISTS module (
@ -180,6 +181,10 @@ function migrate(db: DB): void {
if (!emailCols.some((c) => c.name === 'bounced')) {
db.exec(`ALTER TABLE email_log ADD COLUMN bounced INTEGER NOT NULL DEFAULT 0`)
}
const clientCols = db.prepare(`PRAGMA table_info(client)`).all() as { name: string }[]
if (!clientCols.some((c) => c.name === 'owner_id')) {
db.exec(`ALTER TABLE client ADD COLUMN owner_id TEXT`)
}
rebuildStaffUserRoleCheck(db)
}

@ -10,11 +10,13 @@ export interface Client {
id: string; code: string; name: string; gstin?: string; stateCode: string
address: string; contacts: ClientContact[]
status: 'lead' | 'active' | 'dormant' | 'lost'; notes: string
/** Account/enquiry owner (→ staff_user.id); absent = unassigned. Routes leads (D-EMP A4). */
ownerId?: string
}
interface ClientRow {
id: string; code: string; name: string; gstin: string | null; state_code: string
address: string; contacts: string; status: string; notes: string
address: string; contacts: string; status: string; owner_id: string | null; notes: string
source: string; created_at: string
}
@ -25,6 +27,7 @@ function toClient(r: ClientRow): Client {
stateCode: r.state_code, address: r.address,
contacts: JSON.parse(r.contacts) as ClientContact[],
status: r.status as Client['status'], notes: r.notes,
...(r.owner_id !== null ? { ownerId: r.owner_id } : {}),
}
}
@ -100,3 +103,25 @@ export function updateClient(db: DB, userId: string, id: string, patch: ClientPa
writeAudit(db, userId, 'update', 'client', id, before, after)
return after
}
/**
* Assign (or clear, with null) the account/enquiry owner. Deliberately NOT part of
* updateClient/ClientPatch: the generic PATCH /clients/:id is open to staff, while
* ownership routing is owner/manager-gated and audited as its own action.
*/
export function setClientOwner(db: DB, userId: string, id: string, ownerId: string | null): Client {
return db.transaction(() => {
const before = getClient(db, id)
if (before === null) throw new Error('Client not found')
if (ownerId !== null) {
const emp = db.prepare(`SELECT active FROM staff_user WHERE id=?`)
.get(ownerId) as { active: number } | undefined
if (emp === undefined) throw new Error('Employee not found')
if (emp.active !== 1) throw new Error('Cannot assign an inactive employee as account owner')
}
db.prepare(`UPDATE client SET owner_id=? WHERE id=?`).run(ownerId, id)
const after = getClient(db, id)!
writeAudit(db, userId, 'set_owner', 'client', id, before, after)
return after
})()
}

@ -0,0 +1,170 @@
// apps/hq/test/client-owner.test.ts — Phase 3: owner plumbing.
// client.owner_id (additive column), setClientOwner (audited), the ownerScope
// role gate (staff forced to self), and PATCH /clients/:id/owner (owner/manager only).
import Database from 'better-sqlite3'
import express from 'express'
import fs from 'node:fs'
import os from 'node:os'
import path from 'node:path'
import { describe, it, expect, afterAll } from 'vitest'
import { openDb, type DB } from '../src/db'
import { createStaff, ownerScope } from '../src/auth'
import { createEmployee, deactivateEmployee } from '../src/repos-employees'
import { createClient, getClient, listClients, setClientOwner } from '../src/repos-clients'
import { listAudit } from '../src/audit'
import { apiRouter } from '../src/api'
function withOwner(): { db: DB; ownerId: string } {
const db = openDb(':memory:')
const { id } = createStaff(db, {
email: 'admin@tecnostac.com', displayName: 'Owner', role: 'owner', password: 'let-me-in-9',
})
return { db, ownerId: id }
}
describe('client.owner_id column', () => {
it('exists on a fresh DB (SCHEMA born correct) and defaults to NULL', () => {
const db = openDb(':memory:')
const cols = (db.prepare(`PRAGMA table_info(client)`).all() as { name: string }[]).map((c) => c.name)
expect(cols).toContain('owner_id')
const c = createClient(db, 'u1', { name: 'Fresh Co', stateCode: '32' })
expect(getClient(db, c.id)!.ownerId).toBeUndefined()
})
it('migrate() adds owner_id to a pre-Phase-3 DB, preserving rows, idempotently', () => {
// Simulate a DB created before this phase: client table without owner_id + one row.
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'hq-owner-'))
const raw = new Database(path.join(dir, 'hq.db'))
raw.exec(`CREATE TABLE client (
id TEXT PRIMARY KEY, code TEXT NOT NULL UNIQUE, name TEXT NOT NULL,
gstin TEXT, state_code TEXT NOT NULL DEFAULT '32', address TEXT NOT NULL DEFAULT '',
contacts TEXT NOT NULL DEFAULT '[]',
status TEXT NOT NULL DEFAULT 'active' CHECK (status IN ('lead','active','dormant','lost')),
notes TEXT NOT NULL DEFAULT '', source TEXT NOT NULL DEFAULT 'hq', created_at TEXT NOT NULL
)`)
raw.prepare(
`INSERT INTO client (id, code, name, created_at) VALUES ('c1', 'CL0001', 'Old Row', '2026-01-01T00:00:00Z')`,
).run()
raw.close()
const db = openDb(dir) // exec SCHEMA (IF NOT EXISTS skips client) + migrate() adds the column
const cols = (db.prepare(`PRAGMA table_info(client)`).all() as { name: string }[]).map((c) => c.name)
expect(cols).toContain('owner_id')
const kept = db.prepare(`SELECT id, name, owner_id FROM client WHERE id='c1'`).get()
expect(kept).toMatchObject({ id: 'c1', name: 'Old Row', owner_id: null })
db.close()
const again = openDb(dir) // idempotent re-run loses nothing
expect(again.prepare(`SELECT COUNT(*) AS n FROM client`).get()).toMatchObject({ n: 1 })
again.close()
fs.rmSync(dir, { recursive: true, force: true })
})
})
describe('setClientOwner', () => {
it('gives a lead a routable owner and writes an audit row in the same transaction', () => {
const { db, ownerId } = withOwner()
const emp = createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
const lead = createClient(db, ownerId, { name: 'Nagari Sah.', stateCode: '32', status: 'lead' })
const after = setClientOwner(db, ownerId, lead.id, emp.id)
expect(after.ownerId).toBe(emp.id)
expect(getClient(db, lead.id)!.ownerId).toBe(emp.id)
expect(listClients(db).find((c) => c.id === lead.id)!.ownerId).toBe(emp.id)
const audit = listAudit(db).find((a) => a.action === 'set_owner' && a.entity === 'client' && a.entity_id === lead.id)
expect(audit).toBeDefined()
expect(JSON.parse(audit!.before_json!)).not.toHaveProperty('ownerId')
expect(JSON.parse(audit!.after_json!)).toMatchObject({ ownerId: emp.id })
})
it('clears the owner with null, audited', () => {
const { db, ownerId } = withOwner()
const emp = createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
const c = createClient(db, ownerId, { name: 'Clearable', stateCode: '32' })
setClientOwner(db, ownerId, c.id, emp.id)
const cleared = setClientOwner(db, ownerId, c.id, null)
expect(cleared.ownerId).toBeUndefined()
const audits = listAudit(db).filter((a) => a.action === 'set_owner' && a.entity_id === c.id)
expect(audits).toHaveLength(2)
})
it('rejects an unknown employee and an inactive employee; unknown client 404s', () => {
const { db, ownerId } = withOwner()
const c = createClient(db, ownerId, { name: 'Guarded', stateCode: '32' })
expect(() => setClientOwner(db, ownerId, c.id, 'nope')).toThrow(/employee/i)
const emp = createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
deactivateEmployee(db, ownerId, emp.id)
expect(() => setClientOwner(db, ownerId, c.id, emp.id)).toThrow(/inactive/i)
expect(() => setClientOwner(db, ownerId, 'missing', null)).toThrow(/client/i)
// failed sets leave no owner and no audit row behind
expect(getClient(db, c.id)!.ownerId).toBeUndefined()
expect(listAudit(db).filter((a) => a.action === 'set_owner')).toHaveLength(0)
})
})
describe('ownerScope role gate', () => {
it('forces staff to self and ignores any widening param', () => {
expect(ownerScope({ id: 'me', role: 'staff' })).toBe('me')
expect(ownerScope({ id: 'me', role: 'staff' }, 'someone-else')).toBe('me')
})
it('owner and manager are unfiltered by default and may narrow', () => {
expect(ownerScope({ id: 'o1', role: 'owner' })).toBeUndefined()
expect(ownerScope({ id: 'm1', role: 'manager' })).toBeUndefined()
expect(ownerScope({ id: 'o1', role: 'owner' }, 'e9')).toBe('e9')
expect(ownerScope({ id: 'm1', role: 'manager' }, 'e9')).toBe('e9')
expect(ownerScope({ id: 'm1', role: 'manager' }, '')).toBeUndefined() // empty ≠ a filter
})
})
describe('PATCH /clients/:id/owner', () => {
const db = openDb(':memory:')
createStaff(db, { email: 'owner@test.in', displayName: 'Owner', role: 'owner', password: 'owner-password' })
createStaff(db, { email: 'manager@test.in', displayName: 'Mgr', role: 'manager', password: 'manager-password' })
createStaff(db, { email: 'staff@test.in', displayName: 'Stf', role: 'staff', password: 'staff-password' })
const staffDbId = (db.prepare(`SELECT id FROM staff_user WHERE email='staff@test.in'`).get() as { id: string }).id
const app = express(); app.use(express.json()); app.locals['db'] = db; app.use('/api', apiRouter(db))
const server = app.listen(0)
const base = `http://localhost:${(server.address() as { port: number }).port}/api`
afterAll(() => server.close())
const tokenOf = async (email: string, password: string) =>
(await (await fetch(`${base}/auth/login`, {
method: 'POST', headers: { 'content-type': 'application/json' },
body: JSON.stringify({ email, password }),
})).json() as { token: string }).token
const patchOwner = async (token: string, clientId: string, body: unknown) => {
const res = await fetch(`${base}/clients/${clientId}/owner`, {
method: 'PATCH',
headers: { 'content-type': 'application/json', authorization: `Bearer ${token}` },
body: JSON.stringify(body),
})
return { status: res.status, json: await res.json() as any }
}
it('owner and manager can set an owner; the write is audited; staff gets 403', async () => {
const owner = await tokenOf('owner@test.in', 'owner-password')
const c = createClient(db, 'seed', { name: 'Routable Lead', stateCode: '32', status: 'lead' })
const set = await patchOwner(owner, c.id, { ownerId: staffDbId })
expect(set.status).toBe(200)
expect(set.json.client.ownerId).toBe(staffDbId)
const audit = listAudit(db).find((a) => a.action === 'set_owner' && a.entity_id === c.id)
expect(audit).toBeDefined()
const manager = await tokenOf('manager@test.in', 'manager-password')
const cleared = await patchOwner(manager, c.id, { ownerId: null })
expect(cleared.status).toBe(200)
expect(cleared.json.client.ownerId).toBeUndefined()
const staff = await tokenOf('staff@test.in', 'staff-password')
const denied = await patchOwner(staff, c.id, { ownerId: staffDbId })
expect(denied.status).toBe(403)
expect(getClient(db, c.id)!.ownerId).toBeUndefined() // staff write did not land
})
it('404s an unknown client; 400s a bad employee id or missing ownerId field', async () => {
const owner = await tokenOf('owner@test.in', 'owner-password')
expect((await patchOwner(owner, 'missing', { ownerId: staffDbId })).status).toBe(404)
const c = createClient(db, 'seed', { name: 'Bad Input Co', stateCode: '32' })
expect((await patchOwner(owner, c.id, { ownerId: 'not-an-employee' })).status).toBe(400)
expect((await patchOwner(owner, c.id, {})).status).toBe(400)
})
})
Loading…
Cancel
Save