fix payments

2026-07-08 23:30:16 -04:00
parent 6e8da3cc2c
commit ed11cccecc
15 changed files with 959 additions and 132 deletions
@@ -0,0 +1,492 @@
+# Payment Data Repair Plan
+
+## Scope
+
+This plan fixes the payment table, invoice/payment display logic, and manual payment write flow.
+
+Reviewed inputs:
+
+- `invoices(2).sql`
+- `payments(1).sql`
+- `PaymentController(2).php`
+- `PaymentTransactionController(1).php`
+
+## Current findings
+
+| Check | Result |
+|---|---:|
+| Invoice rows | 126 |
+| Payment rows | 321 |
+| Invoice `paid_amount` vs `SUM(payments.paid_amount)` mismatches | 0 |
+| Payment rows where `payments.total_amount` differs from linked invoice total | 106 |
+| Invoices affected by stale `payments.total_amount` | 66 |
+| Payment rows with negative `payments.balance` | 5 |
+| Payment rows whose `semester` / `school_year` differs from linked invoice | 26 |
+| Invoice IDs with no payment rows | 46, 76, 77, 108, 112, 122 |
+
+Main conclusion: payment rows are not missing. The system is reading and storing payment context inconsistently. The payment table is being treated as both a transaction ledger and a cached invoice snapshot, which is why the UI looks wrong.
+
+---
+
+## Priority 1 — Protect production before repair
+
+- [ ] Take a database backup.
+- [ ] Export `invoices`, `payments`, `discount_usages`, `refunds`, `balance_transfers`, and any `payment_transactions` table.
+- [ ] Run all reconciliation SQL in read-only mode first.
+- [ ] Apply data repair only after confirming the result count of each query.
+
+Recommended backup command:
+
+```bash
+mysqldump -u <user> -p <database> invoices payments discount_usages refunds balance_transfers payment_transactions > payment_repair_backup.sql
+```
+
+---
+
+## Priority 2 — Add missing schema support
+
+The PHP controller writes `installment_seq`, but the uploaded `payments` schema only has `number_of_installments`.
+
+Add a dedicated installment sequence column:
+
+```sql
+ALTER TABLE payments
+  ADD COLUMN installment_seq INT NULL AFTER number_of_installments;
+
+UPDATE payments
+SET installment_seq = number_of_installments
+WHERE installment_seq IS NULL;
+```
+
+Change `payment_date` from `DATE` to `DATETIME` so multiple same-day payments can be sorted correctly:
+
+```sql
+ALTER TABLE payments
+  MODIFY payment_date DATETIME NOT NULL;
+```
+
+Add indexes:
+
+```sql
+CREATE INDEX idx_payments_invoice_id ON payments(invoice_id);
+CREATE INDEX idx_payments_parent_year ON payments(parent_id, school_year, semester);
+CREATE UNIQUE INDEX uq_payments_transaction_id ON payments(transaction_id);
+CREATE INDEX idx_invoices_parent_year ON invoices(parent_id, school_year, semester);
+```
+
+---
+
+## Priority 3 — Repair existing payment school year and semester
+
+Payments should inherit their term from the linked invoice. The current data has payments marked `Spring` while their invoice is `Fall`.
+
+Preview first:
+
+```sql
+SELECT
+  p.id AS payment_id,
+  p.invoice_id,
+  p.parent_id,
+  p.school_year AS payment_school_year,
+  p.semester AS payment_semester,
+  i.school_year AS invoice_school_year,
+  i.semester AS invoice_semester
+FROM payments p
+JOIN invoices i ON i.id = p.invoice_id
+WHERE p.school_year <> i.school_year
+   OR p.semester <> i.semester
+ORDER BY p.id;
+```
+
+Repair:
+
+```sql
+UPDATE payments p
+JOIN invoices i ON i.id = p.invoice_id
+SET
+  p.school_year = i.school_year,
+  p.semester = i.semester
+WHERE
+  p.school_year <> i.school_year
+  OR p.semester <> i.semester;
+```
+
+---
+
+## Priority 4 — Stop trusting stale copied fields
+
+Do not use these fields as authoritative in the UI:
+
+- `payments.total_amount`
+- `payments.balance`
+- `payments.status` as invoice status
+
+Use:
+
+- `payments.paid_amount` as the actual transaction amount.
+- `invoices.total_amount` as the invoice total.
+- `InvoiceLedgerService` for current invoice balance.
+- Invoice status from recalculation, not from the latest payment row.
+
+The payment history table should display:
+
+```text
+invoice_number
+payment_date
+paid_amount
+payment_method
+check_number
+transaction_id
+installment_seq
+balance_after_payment
+invoice_total
+invoice_current_balance
+invoice_status
+school_year
+semester
+```
+
+---
+
+## Priority 5 — Fix `manualPayUpdate()` term assignment
+
+Current bug: the controller locks the invoice and reads the invoice school year, but then passes `$this->schoolYear` and `$this->semester` into `processPayment()`. That stores payment rows under the active config term instead of the invoice term.
+
+Change the invoice lock query:
+
+```php
+$row = $this->db->query(
+    'SELECT id, parent_id, total_amount, school_year, semester FROM invoices WHERE id = ? FOR UPDATE',
+    [$invoiceId]
+)->getRowArray();
+
+$parentId    = (int) ($row['parent_id'] ?? 0);
+$invYear     = (string) ($row['school_year'] ?? $this->schoolYear);
+$invSemester = (string) ($row['semester'] ?? $this->semester);
+```
+
+Then pass the invoice term into `processPayment()`:
+
+```php
+$ok = $this->processPayment(
+    $invoiceId,
+    $amount,
+    $paymentMethod,
+    $checkFile,
+    $transactionId,
+    $paymentDate,
+    $invYear,
+    $invSemester,
+    $checkNumber,
+    $installmentSeq,
+    (array) $this->invoiceModel->find($invoiceId),
+    $currentBalance
+);
+```
+
+Do not use the current configuration term when recording a payment for an existing invoice. The invoice owns the term.
+
+---
+
+## Priority 6 — Replace weak parent payment queries
+
+Current parent payment lookup is too broad and does not include enough invoice context.
+
+Replace simple payment-only queries with invoice-joined queries.
+
+Example:
+
+```php
+$selectedYear = $this->request->getGet('school_year') ?? $this->schoolYear;
+$selectedSemester = $this->request->getGet('semester') ?? null;
+
+$paymentsQuery = $this->paymentModel
+    ->select([
+        'payments.id',
+        'payments.invoice_id',
+        'invoices.invoice_number',
+        'payments.transaction_id',
+        'payments.paid_amount',
+        'payments.payment_method',
+        'payments.check_number',
+        'payments.check_file',
+        'payments.payment_date',
+        'payments.installment_seq',
+        'payments.number_of_installments',
+        'payments.balance AS balance_after_payment',
+        'payments.status AS payment_status',
+        'invoices.total_amount AS invoice_total',
+        'invoices.balance AS invoice_current_balance',
+        'invoices.status AS invoice_status',
+        'invoices.school_year',
+        'invoices.semester',
+    ])
+    ->join('invoices', 'invoices.id = payments.invoice_id', 'inner')
+    ->where('payments.parent_id', $parentId)
+    ->where('invoices.school_year', $selectedYear);
+
+if ($selectedSemester !== null && $selectedSemester !== '') {
+    $paymentsQuery->where('invoices.semester', $selectedSemester);
+}
+
+$payments = $paymentsQuery
+    ->orderBy('payments.payment_date', 'DESC')
+    ->orderBy('payments.id', 'DESC')
+    ->paginate(10);
+```
+
+Apply this to:
+
+- `getByParent()`
+- `manualPaySearch()`
+- Any dashboard or parent profile payment history endpoint
+- Any admin finance page reading payment history
+
+---
+
+## Priority 7 — Normalize payment status
+
+Payment status and invoice status must not mean the same thing.
+
+Payment statuses should be transaction-level:
+
+```text
+Recorded
+Voided
+Refunded
+Failed
+Reversed
+Chargeback
+```
+
+Invoice statuses should be computed:
+
+```text
+Unpaid
+Partially Paid
+Paid
+Overpaid
+Cancelled
+```
+
+After the UI stops depending on `payments.status` as invoice status, normalize existing successful payment rows:
+
+```sql
+UPDATE payments
+SET status = 'Recorded'
+WHERE LOWER(status) IN ('paid', 'partially paid', 'payment recorded');
+```
+
+Update `FinancialStatus` to include transaction statuses separately from invoice statuses.
+
+---
+
+## Priority 8 — Decide what to do with `payment_transactions`
+
+`PaymentTransactionController` writes to a separate transaction model, but manual payments are recorded directly into `payments`.
+
+Pick one architecture:
+
+### Preferred
+
+Use `payments` as the single payment transaction ledger.
+
+Then:
+
+- [ ] Retire payment transaction pages/routes if unused.
+- [ ] Do not build UI reports from `payment_transactions`.
+- [ ] Keep gateway attempts in a separate `payment_attempts` or `online_payment_transactions` table only if needed.
+
+### Alternative
+
+If `payment_transactions` must stay:
+
+- [ ] Create a transaction row every time a payment row is inserted.
+- [ ] Do it inside the same DB transaction.
+- [ ] Never show `payment_transactions` without joining back to `payments`.
+
+Preferred option is cleaner. Two ledgers become two lies unless aggressively synchronized.
+
+---
+
+## Priority 9 — Recalculate invoice ledger
+
+After schema and controller fixes, recalculate every invoice through `InvoiceLedgerService`.
+
+Required behavior:
+
+- [ ] Sum successful payments from `payments`.
+- [ ] Subtract discounts from `discount_usages`.
+- [ ] Apply refunds/credits consistently.
+- [ ] Include additional/event charges if they belong in invoice total.
+- [ ] Update cached `invoices.paid_amount`, `invoices.balance`, and `invoices.status`.
+- [ ] Never calculate invoice balance directly inside controllers.
+
+Suggested maintenance command:
+
+```php
+foreach ($invoiceModel->findAll() as $invoice) {
+    $invoiceLedgerService->recalculateInvoice((int) $invoice['id']);
+}
+```
+
+Run this from a CLI command, not from a browser request.
+
+---
+
+## Priority 10 — Reconciliation SQL
+
+Run these checks after repair.
+
+### Stale payment totals
+
+```sql
+SELECT
+  p.id AS payment_id,
+  p.invoice_id,
+  p.parent_id,
+  p.total_amount AS payment_total_amount,
+  i.total_amount AS invoice_total_amount,
+  p.paid_amount,
+  p.balance,
+  p.payment_date
+FROM payments p
+JOIN invoices i ON i.id = p.invoice_id
+WHERE p.total_amount <> i.total_amount
+ORDER BY p.invoice_id, p.id;
+```
+
+Expected after UI fix: rows may still exist historically, but the UI must not rely on `payments.total_amount`.
+
+### Term mismatch
+
+```sql
+SELECT
+  p.id AS payment_id,
+  p.invoice_id,
+  p.parent_id,
+  p.school_year AS payment_school_year,
+  p.semester AS payment_semester,
+  i.school_year AS invoice_school_year,
+  i.semester AS invoice_semester
+FROM payments p
+JOIN invoices i ON i.id = p.invoice_id
+WHERE p.school_year <> i.school_year
+   OR p.semester <> i.semester
+ORDER BY p.id;
+```
+
+Expected: zero rows.
+
+### Negative payment balance snapshots
+
+```sql
+SELECT
+  id,
+  parent_id,
+  invoice_id,
+  total_amount,
+  paid_amount,
+  balance,
+  status,
+  payment_date
+FROM payments
+WHERE balance < 0
+ORDER BY invoice_id, id;
+```
+
+Expected: zero rows after balance snapshot repair, or ignored if `payments.balance` is deprecated.
+
+### Invoices with no payment rows
+
+```sql
+SELECT
+  i.id,
+  i.parent_id,
+  i.invoice_number,
+  i.total_amount,
+  i.paid_amount,
+  i.balance,
+  i.has_discount,
+  i.status
+FROM invoices i
+LEFT JOIN payments p ON p.invoice_id = i.id
+WHERE p.id IS NULL
+ORDER BY i.id;
+```
+
+Expected: only invoices fully covered by discounts, credits, cancellations, or legitimate zero-payment states.
+
+### Invoice paid amount consistency
+
+```sql
+SELECT
+  i.id AS invoice_id,
+  i.parent_id,
+  i.invoice_number,
+  i.paid_amount AS invoice_paid_amount,
+  COALESCE(SUM(p.paid_amount), 0) AS payment_sum
+FROM invoices i
+LEFT JOIN payments p ON p.invoice_id = i.id
+GROUP BY i.id, i.parent_id, i.invoice_number, i.paid_amount
+HAVING ROUND(i.paid_amount, 2) <> ROUND(payment_sum, 2)
+ORDER BY i.id;
+```
+
+Expected: zero rows, unless invoice paid amount intentionally includes imported legacy payments that are not in `payments`.
+
+---
+
+## Priority 11 — Tests to add
+
+### Unit tests
+
+- [ ] Payment creation uses invoice `school_year` and `semester`, not config term.
+- [ ] Payment amount cannot exceed current ledger balance.
+- [ ] Card payment must equal full current balance.
+- [ ] Editing a payment recalculates invoice balance.
+- [ ] `installment_seq` increments only successful payments.
+- [ ] Voided/refunded/failed payments are excluded from balance.
+- [ ] `payments.total_amount` is never used as current invoice total in DTOs.
+
+### Integration tests
+
+- [ ] Parent payment history returns payments joined to invoice number.
+- [ ] Parent payment history filters by invoice school year.
+- [ ] Parent payment history filters by invoice semester.
+- [ ] Admin finance table shows transaction amount and current invoice balance correctly.
+- [ ] Manual payment insert, invoice recalculation, and notification event are atomic.
+
+### Data repair tests
+
+- [ ] Term mismatch query returns zero rows after repair.
+- [ ] Negative payment balance query returns zero rows or deprecated field is hidden.
+- [ ] Invoice paid amount consistency query returns zero rows.
+- [ ] Invoices with no payments are manually classified as discount-only, cancelled, imported, or broken.
+
+---
+
+## Acceptance criteria
+
+The fix is complete only when:
+
+- [ ] Existing payments are visible in parent and admin payment history.
+- [ ] Payment history shows transaction amount, not stale invoice total.
+- [ ] Payment rows inherit school year and semester from invoice.
+- [ ] No payment row has mismatched invoice term.
+- [ ] No new payment insert depends on global config term for existing invoices.
+- [ ] `installment_seq` exists and is populated.
+- [ ] Invoice balances are recalculated by `InvoiceLedgerService`.
+- [ ] `payment_transactions` is either retired or explicitly separated as gateway-attempt history.
+- [ ] Reconciliation SQL has been run and saved as evidence.
+
+---
+
+## Do not do this
+
+- Do not manually edit invoice balances row by row.
+- Do not use `payments.total_amount` as invoice total.
+- Do not use `payments.status` as invoice status.
+- Do not filter payment history only by `payments.school_year` if invoice term exists.
+- Do not keep two independent payment ledgers unless one is clearly marked as gateway attempts.
+- Do not trust the latest payment row balance as the current invoice balance.