Distribution/docs/refactor_docs/invoice_collection.md

@@ -0,0 +1,224 @@
+# [Refactor] Invoice Collection — Cash/Cheque/Credit Collection After Finalize
+
+**Status**: Proposed (design only — no code changes yet)
+**Depends On**: [order_invoice_lifecycle.md](./order_invoice_lifecycle.md) — this document only applies
+once an `Invoice` reaches `Finalized`; it does not touch Pending/Draft.
+**Refactors**: `Models/Payment.cs`, `Models/Enums.cs` (`PaymentMethod`, `PaymentStatus`),
+`Sale.PaymentStatus`, and the payment recording flow in
+[06_sale_ride.md](../06_sale_ride.md) (`POST /api/sales/{saleId}/payments`). Also resolves
+[19_finance_accounting.md](../19_finance_accounting.md)'s open question of whether field and
+warehouse invoices share one `Payment` shape, and [order_invoice_lifecycle.md](./order_invoice_lifecycle.md)'s
+Impact item 2 / Open Question 4 ("how does `Payment` attach once `Invoice` exists?").
+
+---
+
+## Goal
+
+Once an `Invoice` is `Finalized` (locked amount — see `order_invoice_lifecycle.md`), someone still has
+to physically collect that money from the shop and get it recorded. Today `Payment` attaches to
+`Sale` and only distinguishes `Cash`/`Credit` (`Models/Payment.cs`, `Models/Enums.cs:28`). This
+document re-points `Payment` at `Invoice` (the real billing document, not the Sale), adds `Cheque` as
+a method, and — the core problem this document solves — accounts for the fact that the money doesn't
+always arrive in one clean, connected transaction:
+
+- **Real-time**: whoever is at the shop (the same rep, a dedicated collector, or anyone authorized —
+  no role restriction) enters the collection on the spot as it happens, one method per entry.
+- **Day-end / offline**: when there's no real-time connectivity or no live tracking at the point of
+  collection, office staff enter the same details after the fact, at day-end, from whatever
+  paper/verbal record the field brought back.
+
+Both paths produce the exact same `Payment` row shape — `EntryMode` only records *how* it was
+captured, it does not change what's captured or how it's totaled.
+
+---
+
+## Background / Context
+
+- `order_invoice_lifecycle.md` deliberately left `Invoice.TotalAmount` as "the amount the shop
+  actually received and paid" once `Finalized`, but designed no collection/payment mechanism against
+  it — it only flagged the gap (Impact item 2, Open Question 4).
+- `19_finance_accounting.md` independently sketched a `Payment` (`InvoiceId`, `Amount`, `Method`,
+  `CollectedAt`) and asked whether it should unify with the existing `Sale`-attached `Payment` from
+  `06_sale_ride.md`. This document answers that: yes, one `Payment` table, moved to key off
+  `Invoice`, not `Sale`.
+- Any *reduction* in the amount owed (returns, damage, short-delivery, discounts) is **not** a new
+  concept introduced here — it is the same deduction mechanism already described at Finalize time in
+  `order_invoice_lifecycle.md`, reusing the `CreditNote` shape from
+  [18_returns_claims.md](../18_returns_claims.md). By the time a `Payment` is recorded, `Invoice.TotalAmount`
+  already reflects every reduction — collection only ever records money actually received against
+  that already-net amount. This document does not add a second reduction path.
+- A single `Payment` uses exactly one `Method` — a shop paying part cash and part cheque on the same
+  visit is recorded as two `Payment` rows, not one row with a split. Multiple `Payment` rows against
+  the same `Invoice` over time are expected and normal (e.g. partial cash today, the remainder by
+  cheque next week, or the balance written off to credit) — collection is not a single lump action.
+
+---
+
+## Data Model Changes
+
+### `Payment` — re-pointed from `Sale` to `Invoice`
+
+```
+Payment
+├── Id (Guid)                  — PK, from Template
+├── FK_InvoiceId (Guid)        — required. WAS FK_SaleId — see Migration Notes for the cutover.
+│                                 Invoice replaces Sale as the thing being paid down, per
+│                                 order_invoice_lifecycle.md.
+├── FK_StoreId (Guid)          — unchanged, still denormalized for reporting/scope filtering
+├── Amount (decimal)           — unchanged
+├── Method (PaymentMethod)     — Cash, Cheque, Credit (Cheque is new — see below)
+├── EntryMode (EntryMode)      — NEW: RealTime, DayEndManual (see below)
+├── CollectedAt (DateTime)     — unchanged: when the money actually changed hands at the shop, not
+│                                 when the row was saved (see EntryMode below for why these differ)
+├── CollectedBy (Guid)         — unchanged: UserId of whoever collected it. No role constraint — can
+│                                 be the rep, a dedicated collector, or office staff entering on
+│                                 someone else's behalf at day-end.
+├── RecordedBy (Guid?)         — NEW, nullable. Only set when EntryMode = DayEndManual and the person
+│                                 keying it in is not the same person who physically collected it
+│                                 (CollectedBy). Null under RealTime, where the collector is the one
+│                                 entering it.
+├── ChequeNumber (string?)     — NEW, only meaningful when Method = Cheque
+├── ChequeDate (DateTime?)     — NEW, only meaningful when Method = Cheque (the date on the cheque,
+│                                 not CollectedAt)
+├── IsActive, CreatedAt        — from Template
+```
+
+### `PaymentMethod` — add `Cheque`
+
+```csharp
+public enum PaymentMethod { Cash, Cheque, Credit }
+```
+
+`Credit` records that the shop was not charged now — the amount is carried as store debt (see
+`Store.OutstandingBalance`, `19_finance_accounting.md`) rather than settled; no money moves.
+
+### New Enum — `EntryMode`
+
+```csharp
+public enum EntryMode { RealTime, DayEndManual }
+```
+
+Purely a provenance flag on how the row was captured — it does not affect totaling, `PaymentStatus`
+computation, or any business rule below. It exists so a day-end reconciliation view can distinguish
+"confirmed live at the shop" from "keyed in later from a paper/verbal record," which matters for
+audit and for spotting collectors who never enter anything in real time.
+
+### `Invoice.PaymentStatus` (new field on `Invoice`, from `order_invoice_lifecycle.md`)
+
+```
+Invoice
+├── ... (existing fields from order_invoice_lifecycle.md: Status, ConfirmedAt, FinalizedAt, ...)
+├── PaymentStatus (PaymentStatus)  — NEW: Pending, PartiallyPaid, Paid — mirrors the existing
+│                                     Sale.PaymentStatus pattern from 06_sale_ride.md, recomputed on
+│                                     every Payment insert (see Business Rules)
+```
+
+This is a deliberate second axis alongside `Invoice.Status` (`Pending/Draft/Finalized`, the document
+lifecycle) — `PaymentStatus` only starts being meaningful once `Status = Finalized`, and answers
+`order_invoice_lifecycle.md`'s Open Question 3 (whether the two axes should merge): they stay
+separate. `PaymentStatus` is the same axis `19_finance_accounting.md` proposed baking into
+`Invoice.Status` (`Issued/PartiallyPaid/Paid`) — this document keeps it as its own field instead of
+folding it into `Invoice.Status`, so `Invoice.Status` stays purely about the PO→Invoice document
+pipeline.
+
+`Sale.PaymentStatus` (`06_sale_ride.md`) is superseded by `Invoice.PaymentStatus` for any Sale that
+has a linked Order/Invoice — see Impact section.
+
+---
+
+## Business Rules
+
+1. A `Payment` can only be recorded against an `Invoice` whose `Status = Finalized` — matches
+   `order_invoice_lifecycle.md`'s framing that Finalize is "the record used for day-end calculations."
+   Attempting to record a `Payment` against a Pending/Draft invoice is rejected (409).
+2. Multiple `Payment` rows are allowed per `Invoice` — collection is not required to happen in a
+   single action. Each row has exactly one `Method`; a mixed cash+cheque collection on one visit is
+   two rows with the same `CollectedAt`/visit context, not one split row.
+3. On every `Payment` insert, sum all `Payment.Amount` for the `Invoice` and recompute
+   `Invoice.PaymentStatus` in the same transaction — identical logic to the existing
+   `Sale.PaymentStatus` computation in `06_sale_ride.md`:
+   - `sum == 0` → `Pending`
+   - `sum < Invoice.TotalAmount` → `PartiallyPaid`
+   - `sum >= Invoice.TotalAmount` → `Paid`
+4. Overpayment is allowed (`sum > Invoice.TotalAmount`), same as the existing Sale-level rule —
+   `PaymentStatus` stays `Paid`. No refund/credit-back mechanism is designed here.
+5. `Method = Credit` still inserts a `Payment` row (so it's part of the same total and audit trail)
+   but represents no money movement — it exists so the collector/office can record "this portion
+   isn't being collected now, it's going on the shop's account" in the same flow as an actual cash or
+   cheque collection, rather than needing a separate mechanism.
+6. `ChequeNumber`/`ChequeDate` are required when `Method = Cheque`, ignored (nullable, left unset) for
+   `Cash`/`Credit`.
+7. `EntryMode` does not gate or delay anything — a `RealTime` and a `DayEndManual` `Payment` are
+   processed identically once submitted; the only difference is which field is populated
+   (`RecordedBy` vs. not) and what the UI shows for provenance.
+8. `RecordedBy` is only set when `EntryMode = DayEndManual` and the person entering the row is not
+   `CollectedBy` (e.g. office staff transcribing a rep's paper collection sheet at day-end). If the
+   same person both collected and enters it, `RecordedBy` is left null even under `DayEndManual`.
+9. No reduction/deduction is recorded on `Payment` itself — any amount not being collected in full is
+   either already netted into `Invoice.TotalAmount` via the Finalize-time deduction (returns/damage/
+   discount, per `order_invoice_lifecycle.md`) or is recorded as `Method = Credit` (rule 5) for the
+   uncollected remainder. There is no third path.
+
+---
+
+## Impact on Dependent Features
+
+1. **`06_sale_ride.md`'s `POST /api/sales/{saleId}/payments`** — this endpoint and
+   `Sale.PaymentStatus` become the field-sale-specific legacy path. Once a Sale has a linked
+   Order/Invoice (per `order_invoice_lifecycle.md`), payments should go through
+   `POST /api/invoices/{id}/payments` instead and `Sale.PaymentStatus` stops being authoritative.
+   Whether `Sale.PaymentStatus` is removed outright or kept in sync (mirrored from
+   `Invoice.PaymentStatus`) for backward compatibility is not decided here — see Open Questions.
+2. **`Store.OutstandingBalance`** (`12_store_management.md`, referenced in `19_finance_accounting.md`)
+   — needs to account for `Method = Credit` payments (rule 5) increasing the balance, same as it
+   already should for any unpaid `Invoice.TotalAmount` minus `PaymentStatus`-tracked collections. Not
+   redesigned here, just flagged as touched.
+3. **`19_finance_accounting.md`'s GL posting** (`postToGeneralLedger`) should trigger off `Payment`
+   insert events the same way it already anticipates triggering off invoice/payment/credit-note
+   events — no new trigger type, just confirms `Payment` (now Invoice-keyed) is the event source.
+4. **Existing `Payment` rows** (keyed by `FK_SaleId` today) need a migration path once `Order`/
+   `Invoice` exist for the sales they belong to — see Migration Notes.
+
+---
+
+## Open Questions
+
+1. **Does `Sale.PaymentStatus` get removed, or mirrored?** — see Impact item 1. Not decided here;
+   removing it outright is cleaner but touches whatever UI/reporting still reads it directly.
+2. **Multi-visit collection reminders/scheduling** — this document only covers *recording* a
+   collection, not scheduling or reminding a collector to go back for the remainder of a
+   `PartiallyPaid` invoice. Out of scope here.
+3. **Cheque bounce / dishonor handling** — a `Cheque` `Payment` is recorded as collected the moment
+   it's received; there's no designed path for reversing a `Payment` (and its effect on
+   `Invoice.PaymentStatus`) if the cheque later bounces. Flagged as a gap, not solved.
+4. **Does `EntryMode = DayEndManual` need a batch/reconciliation view** (e.g. "these 12 collections
+   were keyed in for yesterday's rides") rather than being entered one `Payment` at a time through the
+   same endpoint as real-time? Not designed here — this document only specifies the data shape and
+   business rules, not the day-end UI workflow.
+5. **Should a `Payment` be linkable to a specific `Ride`/visit** (beyond `CollectedAt`/`CollectedBy`),
+   so a day-end reconciliation can group collections by the ride that generated them? Not modeled
+   here — `Payment` only links to `Invoice` and `Store`.
+
+---
+
+## Migration Notes (for when this is implemented)
+
+Not part of this docs-only pass, but captured for the eventual implementation step:
+
+- Add `Cheque` to `PaymentMethod` (`Models/Enums.cs:28`); add `EntryMode` enum (`RealTime`,
+  `DayEndManual`).
+- Add `FK_InvoiceId`, `EntryMode`, `RecordedBy`, `ChequeNumber`, `ChequeDate` to `Models/Payment.cs`.
+- `FK_SaleId` on `Payment` is superseded by `FK_InvoiceId` — do not drop it immediately; keep it
+  nullable through a transition period so existing field-sale payments (which predate any
+  Order/Invoice) aren't orphaned, and backfill `FK_InvoiceId` where a Sale's Order has a linked
+  Invoice. Full removal of `FK_SaleId` is a follow-up once backfill is verified complete.
+- Add `PaymentStatus` to `Models/Invoice.cs` (from `order_invoice_lifecycle.md`'s new `Invoice`
+  entity), defaulting to `Pending`.
+- New `POST /api/invoices/{id}/payments` endpoint (`Features/Payment/`, extending the existing
+  feature rather than duplicating it) — validates `Invoice.Status == Finalized`, inserts `Payment`,
+  recomputes `Invoice.PaymentStatus` in the same transaction as the existing
+  `POST /api/sales/{saleId}/payments` logic in `06_sale_ride.md`.
+- `GET /api/invoices/{id}/payments` — list collections against an invoice (needed for both the
+  real-time collector view and the day-end reconciliation view).
+- Resolve Open Question 1 (Sale.PaymentStatus fate) before removing or repointing the existing
+  `POST /api/sales/{saleId}/payments` endpoint.