docs: add receipt capture, OCR, multi-bank & customer-recycling spec
Forward implementation spec covering the legacy "Editor" receipt-capture workflow plus three net-new requests from the 2026-07-25/26 meeting with Jorge: PDF/OCR auto-capture, multi-bank chequera support, and customer-number recycling. Matching logic and data-model gaps for each were verified against the actual migration scripts and API code, not just the schema comments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,795 @@
|
||||
# Receipt Capture ("Editor") & Related Net-New Features — Implementation Spec
|
||||
|
||||
Source: Jorge Cuadros meeting notes, 2026-07-25/26 (`Utility Management` section)
|
||||
plus a business-logic read-through of `UTILITIES.accdb`'s legacy "Editor"
|
||||
workflow (`docs/LEGACY_DATABASES_OBJECTS.md`, `docs/LEGACY_DATABASES.md`).
|
||||
This is a forward spec for work **not yet built**, not a record of what
|
||||
exists — contrast with `RENEWAL_NOTICES.md`, which documents a legacy
|
||||
workflow already migrated.
|
||||
|
||||
## Why these four features are one spec
|
||||
|
||||
The meeting covered one legacy workflow (receipt capture, the "Editor") plus
|
||||
three requests that have no legacy precedent at all. They're specified
|
||||
together because they compose:
|
||||
|
||||
1. **Receipt capture module** — the direct replacement for the legacy
|
||||
"Editor" screens, extending what's already built in `billing/`.
|
||||
2. **PDF/OCR auto-capture** — a new intake path that feeds *into* module 1
|
||||
(an OCR-confirmed statement becomes a captured receipt, not a separate
|
||||
ledger).
|
||||
3. **Multi-bank chequera** — changes what a captured receipt's check number
|
||||
reconciles against (module 1's check-reconciliation view needs to know
|
||||
*which bank account* a check was drawn on).
|
||||
4. **Customer-number recycling** — changes how a customer is created, which
|
||||
is the first step of capturing a receipt for them (module 1's customer
|
||||
picker needs to respect whatever number a recycled customer was assigned).
|
||||
|
||||
Each section below is independently buildable and independently useful, but
|
||||
1 should land before 2 (2 posts through 1's API), and 4 is fully
|
||||
independent of the other three.
|
||||
|
||||
---
|
||||
|
||||
## 1. Receipt capture module (the "Editor" replacement)
|
||||
|
||||
### What's already built (do not re-build)
|
||||
|
||||
`apps/api/src/billing/` + `apps/web` `MovementForm.tsx` already provide
|
||||
single-movement manual capture: `POST /billing` (`ledger:create`) creates one
|
||||
signed `Transaction` row with customer, domain, amount, currency, date,
|
||||
concept (`typeId`), period, reference, check number, and message; `POST
|
||||
/billing/:id/void` (`ledger:void`) reverses one. This is the direct
|
||||
equivalent of the legacy `DATOS AGUA`/`DATOS LUZ`/`DATOS TEL`/`DATOS CLAVE`
|
||||
per-service capture screens, already generalized into one form — **the
|
||||
per-service screens do not need to be rebuilt separately**; `domain` +
|
||||
`typeId` (from `TypeTransaction`) already carry that distinction, and the
|
||||
capture form can pre-select a concept when opened from a property's service
|
||||
tab.
|
||||
|
||||
What's missing is everything the legacy system did *around* that single
|
||||
capture: batching many receipts against one check, separating out unpaid
|
||||
items, and reconciling a check's total against what was captured against it.
|
||||
|
||||
### 1.1 Outstanding ("NOPAGO") workflow
|
||||
|
||||
`Transaction.outstanding` already exists in the Prisma schema but nothing
|
||||
reads or writes it yet.
|
||||
|
||||
- **`CreateMovementDto`** (`billing/movement.dto.ts`): add `outstanding?:
|
||||
boolean`, default `false`. When `true`, the movement posts normally but is
|
||||
excluded from "settled" balance views — mirrors the legacy `SALDOS ULTIMO
|
||||
0` query, which already `HAVING NOPAGO = 0`s outstanding rows out of the
|
||||
balance.
|
||||
- **`MovementForm.tsx`**: add an "Outstanding (sin fondos)" checkbox, shown
|
||||
only for `domain = UTILITY` charge-direction rows (this is a per-service
|
||||
charge concept, not a credit/payment concept).
|
||||
- **New endpoint** `POST /billing/:id/resolve-outstanding` (`ledger:create` —
|
||||
same tier as capture, since resolving is completing a capture, not
|
||||
reversing one). Body: `{ checkNumber: string, resolvedDate: string }`.
|
||||
Sets `outstanding = false`, `checkNumber`, and updates `transactionDate` to
|
||||
`resolvedDate` — this is the literal legacy behavior ("se actualiza
|
||||
registro con fecha del día y el cheque a pagar y quitas outstanding").
|
||||
Reject (400) if the row is already resolved or voided.
|
||||
- **New list filter**: `GET /billing?outstanding=true` (extend
|
||||
`MovementParams`) — replaces the legacy `EDITA NO PAGO AGUA/LUZ/PHONE`
|
||||
per-service outstanding screens with one filterable view (service already
|
||||
filterable via `typeId`).
|
||||
- **Web**: an "Outstanding" tab or filter chip on `/estado-cuenta`
|
||||
(Movimientos tab), each row showing a "Resolver" action that opens a small
|
||||
form for check number + date.
|
||||
|
||||
### 1.2 Batch capture by check
|
||||
|
||||
Legacy staff key many customers' receipts against one check before cutting
|
||||
it, then verify the captured total matches the check amount
|
||||
(`CAPTURA AGUA`/`CAPTURA LUZ`/etc. feeding into `EDITA CHEQUE COUNT`/
|
||||
`REPORTE POR CHEQUE`). Model this as a **bulk-create, not a new persisted
|
||||
entity** — a check number is already a plain field on `Transaction`; there's
|
||||
no need for a `ReceiptBatch` table when grouping by `checkNumber` already
|
||||
answers every legacy query.
|
||||
|
||||
- **New endpoint** `POST /billing/batch` (`ledger:create`). Body: `{
|
||||
domain, typeId, transactionDate, currency, checkNumber, lines: [{
|
||||
customerId, amount, reference, period, outstanding? }] }` — the
|
||||
check-level fields are shared, only the per-customer fields repeat. Runs
|
||||
as one Prisma `$transaction`, returns the created rows plus `{ total,
|
||||
count }` so the UI can show the running total against the physical check
|
||||
amount as staff add lines, exactly matching the legacy reconciliation
|
||||
practice.
|
||||
- **Web**: a `/estado-cuenta/captura` (or `/estado-cuenta/lote`) page — a
|
||||
service-type + check-number header, then a repeating row (customer picker
|
||||
+ reference + amount + outstanding toggle), a running total, and a single
|
||||
submit. This is the actual "Editor" screen the meeting notes are asking
|
||||
for; it should be reachable from a new nav entry under "Estado de cuenta"
|
||||
or "Servicios."
|
||||
|
||||
### 1.3 Check reconciliation view
|
||||
|
||||
Direct replacement for `EDITA CHEQUE ALF/COUNT/NUM`, `REPORTE POR CHEQUE`,
|
||||
`REPORTE POR CHEQUE PARA ALFA`, and the `REPORTE CHEQUE COUNT` report named
|
||||
explicitly in the meeting notes.
|
||||
|
||||
- **New endpoint** `GET /billing/by-check?checkNumber=...` — all
|
||||
non-voided `Transaction` rows with that `checkNumber`, plus `{ total,
|
||||
count }`. Trivial query, no new indexes needed beyond the existing
|
||||
`checkNumber` column (add a plain index — it's currently unindexed).
|
||||
- **New report entry** in `reports.registry.ts`: `slug: "cheque-count"`,
|
||||
`legacyName: "REPORTE CHEQUE COUNT"`, params `{ checkNumber: text }`,
|
||||
columns customer/reference/period/concept/amount, reusing the by-check
|
||||
query above. This gives it a printable form for free via the existing
|
||||
`/reportes/:slug` machinery — no new page needed.
|
||||
|
||||
### Abilities
|
||||
|
||||
No new abilities required — everything above reuses `ledger:create` /
|
||||
`ledger:void` (already `STAFF` / `MANAGER`). Batch capture and outstanding
|
||||
resolution are both "capturing a receipt," same trust tier as the existing
|
||||
single-movement form.
|
||||
|
||||
---
|
||||
|
||||
## 2. PDF / OCR auto-capture
|
||||
|
||||
### Motivation (from the meeting)
|
||||
|
||||
Each utility company (CFE, water, phone, gas...) sends 300+ individual
|
||||
statements a month, one per customer, currently keyed in by hand through the
|
||||
per-service capture screens — a high-volume, error-prone manual step. The
|
||||
ask: scan/receive the statements as PDF(s), have the system determine
|
||||
customer + amount automatically, and only require staff review rather than
|
||||
full manual entry.
|
||||
|
||||
### Pipeline
|
||||
|
||||
```
|
||||
Upload (1+ PDFs, one service kind per batch)
|
||||
-> StorageService stores raw file(s)
|
||||
-> Split into one document per statement (if a batch PDF bundles multiple)
|
||||
-> OCR extraction (account/meter number, amount, period, due date)
|
||||
-> Auto-match against PropertyService (accountNumber / meterNumber / route)
|
||||
-> Review queue: high-confidence matches pre-filled, no-match/low-confidence flagged
|
||||
-> Staff confirms (bulk-confirm high-confidence rows, hand-correct the rest)
|
||||
-> Confirmed rows post through the SAME batch-capture path as §1.2
|
||||
-> Source PDF page attached as a ServiceDocument on the matched property
|
||||
```
|
||||
|
||||
The last two steps deliberately reuse §1.2's batch-capture endpoint rather
|
||||
than writing `Transaction` rows directly — OCR-sourced and hand-keyed
|
||||
receipts should go through one write path, one validation path, one audit
|
||||
trail.
|
||||
|
||||
### Data model (new)
|
||||
|
||||
```prisma
|
||||
enum StatementBatchStatus {
|
||||
UPLOADED
|
||||
PROCESSING
|
||||
READY_FOR_REVIEW
|
||||
COMPLETED
|
||||
FAILED
|
||||
}
|
||||
|
||||
enum StatementDocumentStatus {
|
||||
PENDING_OCR
|
||||
OCR_FAILED
|
||||
NEEDS_REVIEW // no confident match, or low OCR confidence
|
||||
MATCHED // confident auto-match, awaiting staff confirmation
|
||||
CONFIRMED // staff confirmed, not yet posted
|
||||
POSTED // posted as a Transaction
|
||||
REJECTED // staff rejected (duplicate, unreadable, wrong batch)
|
||||
}
|
||||
|
||||
// `ServiceKind` needs one addition for this feature: `TELEPHONE`. It
|
||||
// doesn't exist today — phone numbers live on `Property.phone1/2/3`, not as
|
||||
// `PropertyService` rows. See "Matching logic" below for why OCR matching
|
||||
// needs it as a real service kind, and the backfill this implies.
|
||||
|
||||
/// One upload session — e.g. "October CFE statements."
|
||||
model StatementBatch {
|
||||
id String @id @default(uuid())
|
||||
serviceKind ServiceKind
|
||||
status StatementBatchStatus @default(UPLOADED)
|
||||
uploadedById String
|
||||
fileCount Int
|
||||
createdAt DateTime @default(now())
|
||||
documents StatementDocument[]
|
||||
|
||||
@@map("statement_batches")
|
||||
}
|
||||
|
||||
/// One statement (one customer, one period) after splitting the batch.
|
||||
model StatementDocument {
|
||||
id String @id @default(uuid())
|
||||
batchId String
|
||||
batch StatementBatch @relation(fields: [batchId], references: [id])
|
||||
storageKey String
|
||||
status StatementDocumentStatus @default(PENDING_OCR)
|
||||
|
||||
// Raw OCR output, kept even after a manual correction so mismatches are
|
||||
// auditable.
|
||||
ocrRawText String? @db.Text
|
||||
ocrConfidence Decimal? @db.Decimal(4, 3)
|
||||
|
||||
// Extracted (and, after review, staff-corrected) fields.
|
||||
extractedAccountRef String? // RPU / phone / water account / zona fed / clave catastral / gas meter — see "Matching logic" for which PropertyService field this maps to per serviceKind
|
||||
extractedAmount Decimal? @db.Decimal(12, 2)
|
||||
extractedPeriod String?
|
||||
extractedDueDate DateTime?
|
||||
|
||||
// Match result.
|
||||
matchedPropertyServiceId String?
|
||||
matchedPropertyService PropertyService? @relation(fields: [matchedPropertyServiceId], references: [id])
|
||||
matchedCustomerId String?
|
||||
matchedCustomer Customer? @relation(fields: [matchedCustomerId], references: [id])
|
||||
|
||||
reviewedById String?
|
||||
reviewedAt DateTime?
|
||||
postedTransactionId String? @unique
|
||||
postedTransaction Transaction? @relation(fields: [postedTransactionId], references: [id])
|
||||
|
||||
createdAt DateTime @default(now())
|
||||
|
||||
@@map("statement_documents")
|
||||
}
|
||||
```
|
||||
|
||||
(`PropertyService`/`Customer`/`Transaction` gain the inverse relations.
|
||||
`ServiceDocument` is reused as-is for the confirmed receipt's permanent
|
||||
attachment — `StatementDocument.storageKey` and the eventual
|
||||
`ServiceDocument.storageKey` may point at the same object, or the confirm
|
||||
step copies it; either is fine, pick whichever is simpler at build time.)
|
||||
|
||||
### Matching logic
|
||||
|
||||
Match `extractedAccountRef` against **one specific `PropertyService` field,
|
||||
chosen by `serviceKind`** — never a fuzzy match across all of
|
||||
`accountNumber`/`meterNumber`/`route` at once, since that's how a water
|
||||
account number could accidentally collide with an unrelated phone number.
|
||||
Per the meeting notes' own field list:
|
||||
|
||||
| Service (meeting note) | `ServiceKind` | Match against | Legacy source (`DATMEX`) | Status |
|
||||
|---|---|---|---|---|
|
||||
| CFE — RPU | `ELECTRIC` | `accountNumber` | `RPU` / `RPU2` / `RPU3` | ✅ populated today (`transform_properties.py`) |
|
||||
| Agua — Número de cuenta | `WATER` | `accountNumber` | `AGUA` | ✅ populated today |
|
||||
| Zona Fed — Número de Zona Federal | `FEDERAL_ZONE` | `accountNumber` | `ZFED` | ✅ populated today |
|
||||
| Tel — Número de teléfono | `TELEPHONE` *(new)* | `accountNumber` | `Property.phone1/2/3` (currently on `Property`, not `PropertyService`) | ⚠️ schema gap — see below |
|
||||
| Impuesto — Clave Catastral | `PROPERTY_TAX` | `accountNumber` | migrated from `PREDIAL`, **not** `CLAVE` | ⚠️ needs verification — see below |
|
||||
| Gas — Número de medidor | `GAS` | `meterNumber` | not populated — folded into free-text `notes` today | ⚠️ data gap — see below |
|
||||
|
||||
Confidence rule of thumb once a field is confirmed populated, tune after
|
||||
seeing real statements:
|
||||
|
||||
- Exact match on the scoped field → `MATCHED`, high confidence, pre-checked
|
||||
for bulk-confirm.
|
||||
- No match, or the OCR confidence itself is low → `NEEDS_REVIEW`.
|
||||
- Multiple candidate matches (shouldn't happen if account numbers are
|
||||
unique, but the legacy data has had duplication issues before — see
|
||||
`docs/LEGACY_DATABASES_OBJECTS.md`'s `DUPLICADOS` report) → `NEEDS_REVIEW`
|
||||
with all candidates surfaced, not an arbitrary pick.
|
||||
|
||||
#### Three gaps this depends on — resolve before building the matcher
|
||||
|
||||
Cross-checking the requested field list against `transform_properties.py`
|
||||
(the script that actually populated today's `property_services` table)
|
||||
surfaced three mismatches. OCR matching is only as good as the field it
|
||||
matches against, so these need to be closed first, not discovered mid-build:
|
||||
|
||||
1. **No `TELEPHONE` service kind exists.** `ServiceKind` today is `WATER |
|
||||
ELECTRIC | GAS | CABLE | PROPERTY_TAX | FEDERAL_ZONE | ALARM | OTHER` —
|
||||
telephone was never unpivoted into `PropertyService` at all; the three
|
||||
phone numbers live directly on `Property.phone1/phone2/phone3` (raw
|
||||
contact fields, not billable-service rows), even though the legacy
|
||||
ledger clearly bills phone as its own `TYPE OF TRX = "TELEPHONE"` (see
|
||||
`EDITA NO PAGO PHONE`, `DATOS TEL`/`CAPTURA TEL` in the prior analysis).
|
||||
**Fix:** add `TELEPHONE` to the `ServiceKind` enum, and backfill one
|
||||
`PropertyService` row per non-null `Property.phone1/2/3` (`kind:
|
||||
TELEPHONE, accountNumber: <the phone number>`) as a one-time migration
|
||||
script companion to this feature — mirrors how `transform_properties.py`
|
||||
already emits multiple `WATER` rows per property for secondary meters.
|
||||
|
||||
2. **`PROPERTY_TAX.accountNumber` holds `PREDIAL`, not `CLAVE`.** The
|
||||
meeting notes name "Clave Catastral" specifically, and the legacy
|
||||
query/report names agree (`CAPTURA CLAVE`, `DATOS CLAVE`, `CLAVES
|
||||
CATASTRALES`, `CATASTRO` — all filter on `DATMEX.CLAVE`). But
|
||||
`transform_properties.py` line ~191 sets `PROPERTY_TAX.accountNumber =
|
||||
DATMEX.PREDIAL`, a *different* column (`DATMEX` has both `CLAVE`
|
||||
VARCHAR and `PREDIAL` DOUBLE). Two live possibilities: either `PREDIAL`
|
||||
is the wrong field and the migration should have used `CLAVE`, or they're
|
||||
two genuinely different numbers (e.g. clave catastral = the cadastral
|
||||
lookup key stamped on the printed bill vs. predial = an internal
|
||||
receipt/folio number) and `PropertyService` needs *both* — only one of
|
||||
which (the clave catastral) is what OCR will actually read off a real
|
||||
predial statement. **Needs a real predial receipt in hand (or Jorge's
|
||||
confirmation) before deciding**; don't wire the matcher to `PREDIAL` on
|
||||
the untested assumption it's the same thing.
|
||||
|
||||
3. **`GAS.meterNumber` is never populated.** `transform_properties.py`
|
||||
only ever sets `notes = DATMEX.GAS` for gas service rows — there's no
|
||||
distinct meter-number column in the legacy `DATMEX` table for gas at
|
||||
all (unlike electric/water, which have `RPU`/`MEDIDOR`). This matches
|
||||
the earlier finding that "Gas – Número de medidor" has no legacy source
|
||||
field. **This can't be backfilled from existing data** — the practical
|
||||
fix is that gas OCR matching starts cold (every gas statement lands in
|
||||
`NEEDS_REVIEW` until a human confirms it once), and *that first
|
||||
confirmation* is what populates `PropertyService.meterNumber` for that
|
||||
property going forward, so subsequent statements for the same meter
|
||||
auto-match. Worth calling out in the review-queue UI ("first time
|
||||
seeing this meter — confirm to enable auto-match next time").
|
||||
|
||||
### OCR provider — open decision, don't build against one prematurely
|
||||
|
||||
CFE (and most MX utility) bills are **fixed-layout, single-language,
|
||||
high-volume forms**, not arbitrary documents — this is closer to
|
||||
"template/anchor text extraction" (regex against OCR'd text for known
|
||||
labels like `RPU`, `No. de Cuenta`, `Total a pagar`) than to a full ML
|
||||
document-understanding problem. Recommend:
|
||||
|
||||
- Define an `OcrProvider` interface (`extract(buffer, hints): Promise<{
|
||||
text: string, fields: ExtractedFields, confidence: number }>`) so the
|
||||
concrete engine is swappable.
|
||||
- Start with a self-hosted OCR (e.g. Tesseract) + hand-written per-company
|
||||
extraction rules (one rule set per `ServiceKind`/provider, since CFE's
|
||||
layout differs from the water company's). Cheap, no per-page cost, and the
|
||||
layouts are stable enough that this is realistic.
|
||||
- Escalate to a managed document-extraction API (AWS Textract, Azure
|
||||
Document Intelligence, Google Document AI) only if the self-hosted
|
||||
accuracy proves too low in practice — all three fit behind the same
|
||||
interface with no schema changes.
|
||||
- **This choice needs Jorge's input on budget/volume before committing** —
|
||||
300+ pages/month/company is enough volume that a per-page-priced API has a
|
||||
real recurring cost.
|
||||
|
||||
### API surface
|
||||
|
||||
- `POST /statements/batches` (`statement:ingest`) — multipart upload, one or
|
||||
more PDFs + `serviceKind`. Creates the batch, kicks off async
|
||||
split+OCR+match (background job, not inline in the request).
|
||||
- `GET /statements/batches` / `GET /statements/batches/:id` — status +
|
||||
document list.
|
||||
- `GET /statements/batches/:id/documents?status=NEEDS_REVIEW` — the review
|
||||
queue.
|
||||
- `PATCH /statements/documents/:id` (`statement:review`) — staff correction
|
||||
of extracted fields or match.
|
||||
- `POST /statements/documents/:id/confirm` (`statement:review`) — single
|
||||
confirm.
|
||||
- `POST /statements/batches/:id/confirm-matched` (`statement:review`) —
|
||||
bulk-confirm every `MATCHED` document in one call.
|
||||
- `POST /statements/documents/:id/reject` (`statement:review`).
|
||||
- Confirming posts through §1.2's batch-capture internals (same service
|
||||
method, not the HTTP endpoint) so it's one transaction per batch of
|
||||
confirms, not N.
|
||||
- **Confirm also backfills the matched field when it was empty** — if
|
||||
`matchedPropertyServiceId` was set by staff (not by an exact auto-match)
|
||||
because the scoped field was blank on that `PropertyService` (the `GAS`
|
||||
case above, and any one-off historical gap in the other kinds), write
|
||||
`extractedAccountRef` into that service's `accountNumber`/`meterNumber`
|
||||
as part of the confirm transaction. This is what makes the "first
|
||||
confirmation teaches the matcher" behavior in gap 3 above actually work,
|
||||
rather than requiring a separate manual data-entry pass.
|
||||
|
||||
### Abilities (new)
|
||||
|
||||
| Ability | Min role | Notes |
|
||||
|---|---|---|
|
||||
| `statement:ingest` | STAFF | upload a batch |
|
||||
| `statement:review` | STAFF | correct/confirm/reject; same tier as `ledger:create` since confirming *is* capturing |
|
||||
|
||||
### Open questions
|
||||
|
||||
- **Clave catastral vs. predial** (gap 2 above) — get a real predial
|
||||
statement or Jorge's confirmation of whether `CLAVE` and `PREDIAL` are the
|
||||
same number before wiring the `PROPERTY_TAX` matcher. Blocks that one
|
||||
service kind, not the whole feature.
|
||||
- **Telephone as a service kind** (gap 1 above) — confirm the backfill
|
||||
approach (one `PropertyService` row per populated `Property.phone1/2/3`)
|
||||
is correct, and whether a property with all three phones populated should
|
||||
really produce three separate billable "services," or whether phone
|
||||
billing is actually 1-per-property regardless of how many numbers are on
|
||||
file (would change the backfill to pick a primary number instead of
|
||||
fanning out to three rows).
|
||||
- Multi-statement PDF splitting: does the source ever arrive as one PDF per
|
||||
customer already (simplifies to "batch = folder of PDFs"), or as one
|
||||
giant PDF per company per month that needs page-range splitting? Changes
|
||||
whether a page-boundary detector is needed at all.
|
||||
- Retention: keep `StatementDocument.storageKey` (and the raw OCR text)
|
||||
indefinitely for audit, or purge after posting since `ServiceDocument`
|
||||
already holds the permanent copy? Recommend keep — cheap, and it's the
|
||||
audit trail for "why did the system think this was customer X."
|
||||
|
||||
---
|
||||
|
||||
## 3. Multi-bank chequera
|
||||
|
||||
### Motivation
|
||||
|
||||
Seguros uses a US bank account; Utilities uses a Mexican bank account. The
|
||||
current `BankTransaction` model (migrated from `SCOTHIA.mdb`) has no bank or
|
||||
currency dimension at all — it's a single implicit account, MXN-only, by
|
||||
design (see `PLAN.md` migration step 7 finding (c)). Need to support more
|
||||
than one register, each with its own bank and currency.
|
||||
|
||||
Confirmed against the actual code, not just the schema comment: this is a
|
||||
real, deliberate, load-bearing assumption, not an oversight to patch around.
|
||||
`migration/transform_bank.py` has no bank/currency column to read in the
|
||||
first place — `DATOS I`/`DATOS E` are `fecha, tipo, num, concepto,
|
||||
ingreso/egreso, operado, notas, cantidad_en_letra`, nothing else. And
|
||||
`bank.service.ts`'s module doc-comment states outright: "SINGLE CURRENCY...
|
||||
`bank_transactions` has none, and every `amountInWords` on the egreso side
|
||||
is spelled out in PESOS. All figures in this module are MXN." Every method
|
||||
in that file — `where()`, `totalsFor()`, `facets()`, `summary()`, `stats()`,
|
||||
`createMovement()` — currently has zero notion of "which account." That's
|
||||
the actual surface area this feature touches, itemized below.
|
||||
|
||||
### Data model changes
|
||||
|
||||
```prisma
|
||||
model Bank {
|
||||
id String @id @default(uuid())
|
||||
name String @unique // e.g. "Scotiabank", "Bank of America"
|
||||
country String? // "MX" | "US" — informational
|
||||
accounts BankAccount[]
|
||||
|
||||
@@map("banks")
|
||||
}
|
||||
|
||||
/// One physical chequera. Currency is fixed per account (real bank
|
||||
/// accounts don't mix currencies) — do NOT add a currency filter to
|
||||
/// BankTransaction itself; it inherits the account's currency.
|
||||
model BankAccount {
|
||||
id String @id @default(uuid())
|
||||
bankId String
|
||||
bank Bank @relation(fields: [bankId], references: [id])
|
||||
label String // "Utilities operating (MXN)", "Seguros operating (USD)"
|
||||
currency Currency
|
||||
businessLine TransactionDomain? // hint only, not enforced — a chequera can pay for more than one line
|
||||
active Boolean @default(true)
|
||||
movements BankTransaction[]
|
||||
|
||||
@@map("bank_accounts")
|
||||
}
|
||||
```
|
||||
|
||||
`BankTransaction` gains:
|
||||
|
||||
```prisma
|
||||
model BankTransaction {
|
||||
// ...existing fields...
|
||||
bankAccountId String
|
||||
bankAccount BankAccount @relation(fields: [bankAccountId], references: [id])
|
||||
}
|
||||
```
|
||||
|
||||
`bankAccountId` should be **required**, not optional — a bank movement
|
||||
without a known account isn't meaningfully reconcilable. This means the
|
||||
migration step below has to run before the column goes non-null.
|
||||
|
||||
### Migration of existing data
|
||||
|
||||
All 22,354 existing `BankTransaction` rows are SCOTHIA data — MXN, single
|
||||
bank. Before making `bankAccountId` required:
|
||||
|
||||
1. Insert one `Bank` row for Scotiabank, one `BankAccount` row under it
|
||||
(`label: "Utilities — Scotiabank (MXN)"`, `currency: MXN`,
|
||||
`businessLine: UTILITY`).
|
||||
2. Backfill every existing `BankTransaction.bankAccountId` to that account's
|
||||
id.
|
||||
3. Add the second account (`"Seguros — <bank TBD> (USD)"`) — **needs the
|
||||
actual US bank name from Jorge**, plus whether historical Seguros bank
|
||||
data exists anywhere to migrate (the current inventory has no Seguros
|
||||
bank register file — only `SCOTHIA.mdb`, which is Utilities' own book,
|
||||
per `PLAN.md`'s source inventory). If no historical USD register exists,
|
||||
this account starts empty and only carries movements captured going
|
||||
forward.
|
||||
|
||||
The existing `@@unique([legacySourceTable, legacyId])` on `BankTransaction`
|
||||
needs no change — every legacy row only ever belongs to the one Scotiabank
|
||||
account being backfilled in step 2, so provenance uniqueness still holds
|
||||
per-row regardless of how many accounts exist afterward.
|
||||
|
||||
### Code touch points (`bank.service.ts`, `bank.controller.ts`)
|
||||
|
||||
This module currently has **no filterable dimension at all** beyond
|
||||
direction/cleared/date — every account-scoping change is additive, not a
|
||||
rewrite, but it touches every read method because two of them
|
||||
(`facets()`, `summary()`) bypass the Prisma query builder entirely and use
|
||||
hand-written `$queryRaw` template SQL:
|
||||
|
||||
- **`where()`** — trivial, add `bankAccountId` to the `AND` array like any
|
||||
other filter (Prisma builder, same pattern as `direction`/`cleared`).
|
||||
- **`totalsFor()`** — takes the already-built `where`, so it inherits the
|
||||
scoping for free once `list()`/`stats()` pass a scoped `where` in.
|
||||
- **`facets()`** — currently `SELECT YEAR(transactionDate)... FROM
|
||||
bank_transactions WHERE voidedAt IS NULL` with no account clause at all;
|
||||
needs `AND bankAccountId = ${accountId}` interpolated into the raw SQL
|
||||
(parameterized, not string-concatenated — this file already uses Prisma's
|
||||
tagged-template `$queryRaw`, which parameterizes automatically as long as
|
||||
the account id is passed as a template value, not spliced into the string
|
||||
by hand).
|
||||
- **`summary()`** — same issue, in *two* raw queries (the yearly rollup and
|
||||
the monthly rollup when a year is selected) — both need the same
|
||||
`AND bankAccountId = ${accountId}` clause. Miss one and the "Resumen"
|
||||
tab's year list and its drill-down would scope to different accounts,
|
||||
which is a worse bug than not scoping at all (looks correct, silently
|
||||
wrong).
|
||||
- **`stats()`** — currently calls `totalsFor({})` (empty filter = every
|
||||
row). Needs `totalsFor({ bankAccountId })`; same for the `count`/`bounds`/
|
||||
`pending`/`transferred` aggregates alongside it.
|
||||
- **`createMovement()` / `voidMovement()`** — `createMovement` needs
|
||||
`bankAccountId` added to the `data` object (from the new required DTO
|
||||
field below); `voidMovement` needs no change — it already operates by row
|
||||
`id`, and a voided row's account never changes.
|
||||
|
||||
### API surface changes
|
||||
|
||||
- `bank.controller.ts`: every route (`list`, `summary`, `stats`, `facets`)
|
||||
gains a required `?bankAccountId=` query param, threaded through to the
|
||||
service methods above. **Required, not optional with an "all accounts"
|
||||
default** — summing MXN and USD registers together would repeat the exact
|
||||
currency-collapsing mistake the billing module's header comment
|
||||
explicitly warns against (912 customers with both-currency ledgers).
|
||||
There is no meaningful "no account selected" state once accounts exist,
|
||||
only "no account selected *yet*" while the UI loads its default.
|
||||
- New `bank/accounts` sub-resource: `GET /bank/accounts` (list, any
|
||||
authenticated user — the account picker needs this before anything else
|
||||
can render), `POST /bank/accounts` / `PATCH /bank/accounts/:id`
|
||||
(`bank:manage-accounts`, MANAGER — creating/editing accounts is rarer and
|
||||
higher-stakes than posting movements).
|
||||
- `CreateBankMovementDto` gains a required `bankAccountId: string`.
|
||||
|
||||
### Web
|
||||
|
||||
- `bank/page.tsx`'s own doc-comment currently states "Single currency
|
||||
(MXN) — the source has no currency column" as a design fact; that
|
||||
comment (and the assumption behind it) needs to be removed/rewritten as
|
||||
part of this change, not just the UI.
|
||||
- `/banco` gains an account selector (tabs or a dropdown) at the top,
|
||||
scoping both the "Movimientos" and "Resumen por periodo" tabs — mirrors
|
||||
how `/estado-cuenta` already scopes by currency without ever summing
|
||||
across it. Every existing call site in `lib/api.ts`
|
||||
(`listBankMovements`, `getBankStats`, `getBankSummary`, `getBankFacets`,
|
||||
`createBankMovement`) needs the new `bankAccountId` parameter threaded
|
||||
through, and `lib/types.ts`'s `CreateBankMovementInput` gains the field.
|
||||
- New `/banco/cuentas` (or a section under `/catalogos`) for managing banks
|
||||
and accounts, gated the same way `/catalogos` already gates lookup
|
||||
management.
|
||||
|
||||
### Abilities (new)
|
||||
|
||||
| Ability | Min role | Notes |
|
||||
|---|---|---|
|
||||
| `bank:manage-accounts` | MANAGER | create/edit `Bank`/`BankAccount` rows |
|
||||
|
||||
### Open questions
|
||||
|
||||
- Confirm the actual US bank name/details for the Seguros account.
|
||||
- Does Seguros have *any* historical bank register data to migrate, or does
|
||||
this start from zero on cutover?
|
||||
- Should `businessLine` on `BankAccount` be enforced (a UTILITY account
|
||||
can't post an INSURANCE movement) or left as a soft hint? Recommend soft —
|
||||
the legacy single account already mixed concerns per `PLAN.md`'s finding
|
||||
that `TABLA RAMODOS` wasn't a clean business-line split.
|
||||
|
||||
---
|
||||
|
||||
## 4. Customer-number recycling
|
||||
|
||||
### Motivation
|
||||
|
||||
The physical folder system is organized by `Customer` number
|
||||
(`DATGRAL.[NUM id]` in the legacy data, currently only preserved as a
|
||||
`CustomerLegacyRef` string, not a first-class field). When a customer
|
||||
cancels, doesn't renew, or goes a year with no activity, staff currently
|
||||
*manually* hunt for such customers, purge their folder, and reuse the
|
||||
number for a new customer. The ask: keep the physical-folder-compatible
|
||||
sequential numbering, but automate the search for reusable numbers and
|
||||
auto-assign the lowest free one at creation — a Claude Code equivalent of
|
||||
"find the first empty spot."
|
||||
|
||||
### Data model changes
|
||||
|
||||
```prisma
|
||||
model Customer {
|
||||
// ...existing fields...
|
||||
customerNumber Int? @unique // the physical-folder number; null = not yet assigned (shouldn't happen post-migration) or released
|
||||
numberReleasedAt DateTime? // non-null once the number has been freed for reuse; customerNumber is cleared at the same time (see below)
|
||||
}
|
||||
|
||||
enum NumberReleaseReason {
|
||||
CANCELLED // explicit non-renewal / service cancellation
|
||||
INACTIVITY // >= 1 year with no ledger activity
|
||||
MANUAL
|
||||
}
|
||||
|
||||
/// Audit trail for a recycled number, surviving the Customer row it came
|
||||
/// from being archived/purged. customerId is nullable so history remains
|
||||
/// readable even if the originating customer is later hard-deleted.
|
||||
model CustomerNumberHistory {
|
||||
id String @id @default(uuid())
|
||||
customerNumber Int
|
||||
customerId String?
|
||||
customer Customer? @relation(fields: [customerId], references: [id])
|
||||
assignedAt DateTime
|
||||
releasedAt DateTime?
|
||||
releaseReason NumberReleaseReason?
|
||||
releasedById String?
|
||||
|
||||
@@index([customerNumber])
|
||||
@@map("customer_number_history")
|
||||
}
|
||||
```
|
||||
|
||||
### Backfill
|
||||
|
||||
At implementation time, backfill `customerNumber` from the existing
|
||||
`CustomerLegacyRef` rows where `sourceSystem = "utilities" AND sourceTable =
|
||||
"DATGRAL"` (the legacy `NUM id` — already migrated, just not promoted to a
|
||||
first-class column). Insurance-only customers (no utilities `DATGRAL` row)
|
||||
won't have a legacy number; decide at build time whether they get one
|
||||
retroactively assigned or stay `null` until they need one (recommend: assign
|
||||
one on demand, the first time anyone needs to give them a physical folder —
|
||||
not retroactively for all 510 insurance-only customers at once).
|
||||
|
||||
**This backfill isn't a straight cast of every `legacyId` to `Int`.**
|
||||
Checked against `migration/transform_customers.py`: for a utilities
|
||||
`DATGRAL` row, `legacyId` is set to `nid or f"rownum_{len(customers)}"` —
|
||||
`nid` is the real `NUM id` only when the source row actually had one;
|
||||
**~140 utilities rows had a blank `NUM id`** (the same "blank name" data
|
||||
quality issue the same script recovers names for) and got a synthetic
|
||||
`rownum_N` placeholder instead, which is not a physical-folder number and
|
||||
must not be cast into `customerNumber`. Insurance-side refs have the same
|
||||
pattern (`insrow_N` placeholders). The backfill query needs an explicit
|
||||
numeric filter (`legacyId REGEXP '^[0-9]+$'`, or equivalent), and every row
|
||||
that fails it is exactly the "insurance-only or blank-`NUM id`" case that
|
||||
falls through to on-demand assignment above, not an error to chase down.
|
||||
|
||||
`customers.service.ts`'s `list()`/`detail()` `select` blocks don't include
|
||||
`customerNumber` today (only `name`, `nameSource`, contact fields, counts)
|
||||
— it needs adding to both, plus to the `/clientes` list-page columns and
|
||||
the customer detail header, since staff read this number constantly for
|
||||
the physical folder. `list()`'s search (`where.OR`) already matches
|
||||
`legacyRefs.some.legacyId.contains` as a fallback for finding someone by
|
||||
their old number; once `customerNumber` is first-class, add a direct
|
||||
`{ customerNumber: Number(query) }` branch when the query parses as an
|
||||
integer, so a numeric search hits the fast indexed column instead of the
|
||||
join.
|
||||
|
||||
### Eligibility detection (the automation)
|
||||
|
||||
This is explicitly framed as **surfacing candidates for staff review, not
|
||||
auto-purging** — the actual release/reuse decision stays a human action,
|
||||
matching how the office works today; only the *search* is automated.
|
||||
|
||||
- **New endpoint** `GET /customers/recycling-candidates` — customers where
|
||||
either:
|
||||
- `CANCELLED`: no active `Policy` (not archived, `policyTo` in the past
|
||||
with no renewal) **and** no active `PropertyService`, or
|
||||
- `INACTIVITY`: `MAX(Transaction.transactionDate)` across all their
|
||||
transactions is more than 1 year ago (or no transactions at all and
|
||||
`customerSince` is more than 1 year ago).
|
||||
|
||||
This can be a plain query (no new job/queue needed — it's a read, not a
|
||||
mutation) run on-demand when staff open a "Clientes para reciclar" screen,
|
||||
the same way `/billing/balances` is computed live rather than
|
||||
materialized.
|
||||
- **New endpoint** `POST /customers/:id/release-number`
|
||||
(`customer:recycle`, MANAGER). Body: `{ reason: NumberReleaseReason }`.
|
||||
- Closes the open `CustomerNumberHistory` row (`releasedAt = now,
|
||||
releaseReason, releasedById`).
|
||||
- Sets `Customer.customerNumber = null`, `numberReleasedAt = now`.
|
||||
- **Archives** the customer (`archivedAt = now`) — does **not** hard-delete
|
||||
or scrub PII by default. See the purge question below.
|
||||
|
||||
### Auto-assignment at creation
|
||||
|
||||
- **`customers.service.ts` create path**: before insert, compute
|
||||
`SELECT MIN(n) candidate FROM (SELECT customerNumber+1 AS n FROM
|
||||
customers) WHERE n NOT IN (SELECT customerNumber FROM customers WHERE
|
||||
customerNumber IS NOT NULL)` — i.e., the lowest positive integer not
|
||||
currently held by any customer (released numbers, being `NULL` again,
|
||||
automatically qualify; no separate "available pool" table needed, which
|
||||
keeps this consistent with "vacancy = not currently claimed" rather than
|
||||
a second source of truth that can drift). Simplify at build time with
|
||||
whatever the DB makes cheapest (a gaps-and-islands query, or maintaining a
|
||||
running `MAX` + a small released-numbers cache — pick based on real
|
||||
customer-count scale, which is ~1,700, trivially small for a live scan).
|
||||
Open a new `CustomerNumberHistory` row (`assignedAt = now`) for the new
|
||||
assignment.
|
||||
- **Concurrency**: `customers.service.ts`'s `create()` today is a single
|
||||
unguarded `prisma.customer.create()` — no transaction, no locking, which
|
||||
is fine for arbitrary fields but not for a "pick the lowest unclaimed
|
||||
integer" computation, where two staff creating a customer at the same
|
||||
moment can both compute the same candidate number before either insert
|
||||
lands. The `customerNumber` unique constraint turns that race into a
|
||||
Prisma unique-violation error rather than silent data corruption, but the
|
||||
create path needs to actually handle it — wrap the compute-and-insert in
|
||||
a `prisma.$transaction` and retry once on a unique-constraint failure
|
||||
(catch `P2002` on `customerNumber`, recompute, re-insert), rather than
|
||||
letting the second staff member's creation just fail.
|
||||
- **Web**: `/clientes/nuevo` (`CustomerForm.tsx`) shows the assigned number
|
||||
as soon as the form loads (read-only, "Número de cliente: 214
|
||||
(reciclado)" if it's a reused slot, so staff know to expect the old
|
||||
physical folder) — server-assigns it on submit, doesn't let staff type an
|
||||
arbitrary one, which is what prevents the collisions manual assignment
|
||||
risks today. `CreateCustomerDto` deliberately gains **no** `customerNumber`
|
||||
field — the whole point is the client can't set it.
|
||||
|
||||
### The purge question — needs Jorge's decision
|
||||
|
||||
The office's paper-world habit is literally "purge their history, info,
|
||||
etc." when recycling a folder. This codebase's established convention is
|
||||
the opposite — **never hard-delete migrated/business data**, only archive
|
||||
(`archivedAt`), specifically so mistakes are reversible and there's always
|
||||
an audit trail (see `Customer.archivedAt`, `Policy.archivedAt`,
|
||||
`Property.archivedAt`, and the `OpsJob`/`ActivityLog` audit models already
|
||||
in the schema).
|
||||
|
||||
Recommend: **archive by default, never hard-delete.** The `customerNumber`
|
||||
release already solves the actual operational need (the number is free to
|
||||
reuse); keeping the old customer's data around under a freed number costs
|
||||
nothing and preserves history for the inevitable case where "definitely
|
||||
cancelled" turns out to be wrong. If Jorge specifically wants literal
|
||||
data purge (e.g. for a data-retention/privacy policy reason, not just
|
||||
paper-world habit), that should be a **separate, explicit, `ADMIN`-only**
|
||||
action (`customer:purge`) taken well after release — not bundled into
|
||||
`release-number` — so the two decisions ("this number is reusable" vs.
|
||||
"permanently destroy this person's records") aren't accidentally coupled.
|
||||
|
||||
### Abilities (new)
|
||||
|
||||
| Ability | Min role | Notes |
|
||||
|---|---|---|
|
||||
| `customer:recycle` | MANAGER | flag a candidate reviewed, release their number, archive the record |
|
||||
| `customer:purge` | ADMIN | **only if** Jorge wants literal PII destruction, kept separate from release |
|
||||
|
||||
### Open questions
|
||||
|
||||
- Confirm the "1 year of no activity" clock: measured from last
|
||||
`Transaction.transactionDate`, or should a customer with an *expired but
|
||||
never-renewed* policy count as cancelled immediately rather than waiting
|
||||
out the year? (Spec above treats these as two independent triggers,
|
||||
`CANCELLED` vs. `INACTIVITY` — confirm that's the right split.)
|
||||
- Does Jorge want true data purge at all, or is archive-and-hide
|
||||
sufficient? (See above — recommend archive-only unless there's a
|
||||
compliance reason for real deletion.)
|
||||
- Should insurance-only customers (no legacy `NUM id`) share the same
|
||||
numbering sequence as utilities customers, or get their own? Recommend
|
||||
one shared sequence — it's one physical-folder system per the notes, not
|
||||
two.
|
||||
|
||||
---
|
||||
|
||||
## Build sequencing
|
||||
|
||||
1. **§1.1 + §1.2 + §1.3 (receipt capture completion)** — smallest, builds
|
||||
directly on existing `billing/` code, no new tables. Ship first; it's
|
||||
also a prerequisite for §2.
|
||||
2. **§4 (customer-number recycling)** — independent of the others,
|
||||
touches `customers/` only. Can be built in parallel with §1.
|
||||
3. **§3 (multi-bank chequera)** — independent of §1/§2, touches `bank/`
|
||||
only. Needs the US bank name from Jorge before the migration step can
|
||||
run; the schema/API work can start before that answer arrives.
|
||||
4. **§2 (PDF/OCR auto-capture)** — largest, depends on §1 being done (it
|
||||
posts through the batch-capture path) and on the OCR-provider decision.
|
||||
Build last, and prototype the extraction accuracy against a handful of
|
||||
real CFE statements before committing to the provider choice.
|
||||
|
||||
## Open questions to take back to Jorge (collected)
|
||||
|
||||
- OCR provider/budget for §2 (self-hosted vs. managed API, given 300+
|
||||
pages/month/company).
|
||||
- Whether source PDFs arrive pre-split per customer or as one bundled file
|
||||
needing page-range detection (§2).
|
||||
- Whether "Clave Catastral" and the already-migrated `PREDIAL`-sourced
|
||||
`PROPERTY_TAX.accountNumber` are the same number — blocks OCR matching
|
||||
for predial statements specifically until confirmed (§2).
|
||||
- Whether phone billing is really one service per phone number on file, or
|
||||
one per property regardless of how many numbers are recorded — decides
|
||||
how the new `TELEPHONE` service kind gets backfilled (§2).
|
||||
- The actual bank name/currency/details for the Seguros USD account, and
|
||||
whether any historical Seguros bank data exists to migrate (§3).
|
||||
- Whether `BankAccount.businessLine` should be enforced or a soft hint
|
||||
(§3).
|
||||
- The exact "1 year inactivity" / "cancelled" recycling triggers (§4).
|
||||
- Whether customer-number recycling should ever include *true* data purge,
|
||||
or archive-and-reuse-the-number is sufficient (§4).
|
||||
Reference in New Issue
Block a user