From efdb34e7104146eeafa0d7564f1465000e98ee4a Mon Sep 17 00:00:00 2001 From: Thomas Joise Date: Fri, 17 Jul 2026 01:27:44 +0530 Subject: [PATCH] =?UTF-8?q?feat(clients):=20account-owner=20plumbing=20?= =?UTF-8?q?=E2=80=94=20client.owner=5Fid,=20audited=20owner=20route,=20rol?= =?UTF-8?q?e=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- apps/hq-web/src/api.ts | 13 +- apps/hq-web/src/pages/ClientDetail.tsx | 43 ++++++- apps/hq/src/api.ts | 27 +++- apps/hq/src/auth.ts | 17 +++ apps/hq/src/db.ts | 5 + apps/hq/src/repos-clients.ts | 27 +++- apps/hq/test/client-owner.test.ts | 170 +++++++++++++++++++++++++ 7 files changed, 296 insertions(+), 6 deletions(-) create mode 100644 apps/hq/test/client-owner.test.ts diff --git a/apps/hq-web/src/api.ts b/apps/hq-web/src/api.ts index 59aedcd..e33fd93 100644 --- a/apps/hq-web/src/api.ts +++ b/apps/hq-web/src/api.ts @@ -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): Promise => apiFetch<{ client: Client }>('/clients', { method: 'POST', body: JSON.stringify(body) }).then((r) => r.client) export const patchClient = (id: string, body: Record): Promise => 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 => + apiFetch<{ client: Client }>(`/clients/${id}/owner`, { method: 'PATCH', body: JSON.stringify({ ownerId }) }).then((r) => r.client) export const getModules = (): Promise => apiFetch<{ modules: Module[] }>('/modules').then((r) => r.modules) diff --git a/apps/hq-web/src/pages/ClientDetail.tsx b/apps/hq-web/src/pages/ClientDetail.tsx index 6af08f6..c40b5b2 100644 --- a/apps/hq-web/src/pages/ClientDetail.tsx +++ b/apps/hq-web/src/pages/ClientDetail.tsx @@ -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() const c = client.data @@ -67,6 +70,17 @@ export function ClientDetail() { {c.contacts.map((ct, i) => ( {[ct.name, ct.email, ct.phone].filter((x) => x !== undefined && x !== '').join(' · ')} ))} + {canRoute && employees.data !== undefined && ( + { + setActionErr(undefined) + setClientOwner(id, ownerId) + .then(client.reload).catch((err: Error) => setActionErr(err.message)) + }} + /> + )} {actionErr !== undefined && {actionErr}} @@ -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 ( + + ) +} + /** 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 diff --git a/apps/hq/src/api.ts b/apps/hq/src/api.ts index d98d509..7667286 100644 --- a/apps/hq/src/api.ts +++ b/apps/hq/src/api.ts @@ -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) => { diff --git a/apps/hq/src/auth.ts b/apps/hq/src/auth.ts index daf5b95..5a7317e 100644 --- a/apps/hq/src/auth.ts +++ b/apps/hq/src/auth.ts @@ -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 +} diff --git a/apps/hq/src/db.ts b/apps/hq/src/db.ts index 2bbef72..8d3785a 100644 --- a/apps/hq/src/db.ts +++ b/apps/hq/src/db.ts @@ -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) } diff --git a/apps/hq/src/repos-clients.ts b/apps/hq/src/repos-clients.ts index d843afd..f5f63ed 100644 --- a/apps/hq/src/repos-clients.ts +++ b/apps/hq/src/repos-clients.ts @@ -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 + })() +} diff --git a/apps/hq/test/client-owner.test.ts b/apps/hq/test/client-owner.test.ts new file mode 100644 index 0000000..0ad1399 --- /dev/null +++ b/apps/hq/test/client-owner.test.ts @@ -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) + }) +})