101 lines
13 KiB
Markdown
101 lines
13 KiB
Markdown
# Order → Invoice → Payment/Recurring Billing — Fix TODO
|
|
|
|
Nguồn: audit ngày 2026-09-05 (trace thật + chạy test/verify code, không chỉ đọc).
|
|
Thứ tự đề xuất: sửa từ trên xuống, mỗi mục độc lập, có thể merge riêng lẻ.
|
|
|
|
## P0 — Sai cấu trúc nghiệp vụ cốt lõi ✅ DONE (2026-09-05)
|
|
|
|
- [x] **Renewal tạo Order mới thay vì tái dùng Order gốc**
|
|
File: `app/Console/Commands/ServiceExpiry.php` (`createRenewalInvoiceIfMissing()`), `app/Services/Api/Order/OrderQueryService.php::renewService()`
|
|
Đã sửa: bỏ hẳn `Order::create()`/`OrderItem::create()` khi renewal; `Invoice::create(['order_id' => $orderItem->order_id, ...])` trỏ về Order gốc.
|
|
Test: `tests/Feature/Api/Service/RenewServiceTest.php::test_renew_reuses_original_order_instead_of_creating_a_new_one`, `tests/Feature/Console/ServiceExpiryTest.php::test_expiry_command_creates_renewal_invoice_on_the_original_order` — PASS.
|
|
|
|
- [x] **Thiếu khái niệm billing_period trên Invoice**
|
|
File: `database/migrations/2026_09_05_000006_add_billing_period_to_invoices_table.php`
|
|
Đã thêm `billing_period_start`/`billing_period_end` (date, nullable), set ở cả 2 nơi tạo renewal invoice.
|
|
|
|
- [x] **Không có unique constraint chống trùng invoice cùng kỳ billing**
|
|
File: cùng migration trên — `unique(order_id, invoice_type, billing_period_start)` (`invoices_order_type_period_unique`).
|
|
Test: `test_renewal_invoice_has_a_billing_period_and_is_not_duplicated_for_the_same_period`, `test_running_expiry_command_twice_does_not_duplicate_the_renewal_invoice` — PASS (gọi renew 2 lần / chạy cron 2 lần chỉ ra đúng 1 invoice).
|
|
Ghi chú: guard `exists()`-trước-khi-tạo ở tầng app vẫn còn (chưa bắt QueryException dự phòng cho race thật sự đồng thời) — để dành nếu sau này thấy còn lọt duplicate qua race test tải cao.
|
|
|
|
## P1 — Race condition tài chính ✅ DONE (2026-09-05)
|
|
|
|
- [x] **Double-spend balance khi thanh toán đồng thời**
|
|
File: `app/Services/InvoicePaymentService.php::payWithBalance()`
|
|
Đã sửa tại nguồn (không sửa từng caller riêng): lock `Invoice` + `Customer` bằng `lockForUpdate()` bên trong `DB::transaction()`, re-check status/balance sau khi lock, trước khi trừ tiền. Áp dụng cho cả 2 caller (`OrderService::pay()` và `OrderControllerRefactor::payClientInvoiceByNumber()`).
|
|
Test: `tests/Feature/Payment/InvoicePaymentServiceTest.php::test_paying_an_already_paid_invoice_does_not_double_deduct_balance` — gọi `payWithBalance()` 2 lần liên tiếp cho cùng invoice, xác nhận chỉ trừ tiền 1 lần — PASS.
|
|
|
|
- [x] **Duyệt trùng nạp tiền thủ công (admin)**
|
|
File: `app/Http/Controllers/Api/Admin/Transaction/AdminTransactionController.php::approve()` và `reject()`
|
|
Đã sửa: gộp toàn bộ read-check-write vào 1 `DB::transaction()`, lock cả `Transaction` và `Customer` row bằng `lockForUpdate()`, tách phần gửi mail ra ngoài transaction (tránh giữ lock khi I/O).
|
|
Test: `tests/Feature/Payment/AdminTransactionApproveTest.php::test_approving_the_same_manual_deposit_twice_credits_the_customer_only_once` — gọi `approve()` 2 lần cho cùng uuid, xác nhận balance chỉ cộng 1 lần, lần 2 trả 404 — PASS.
|
|
|
|
- [x] **Dedup transaction chỉ ở tầng app, không có unique DB**
|
|
File: `database/migrations/2026_09_05_000007_add_unique_constraint_to_transactions_table.php` — `unique(customer_id, trans, type)`.
|
|
Verify trước khi thêm: query production DB xác nhận 0 nhóm trùng `(customer_id, trans, type)` hiện có, migration chạy sạch.
|
|
Ghi chú: không cần sửa thêm code — `Model::firstOrCreate()` ở Laravel 10 tự bắt `UniqueConstraintViolationException` và fallback về SELECT (xem `vendor/laravel/framework/.../Eloquent/Builder.php::createOrFirst()`), nên `InvoicePaymentService::recordTransaction()` (đã dùng `firstOrCreate`) tự động trở nên race-safe thật sự sau khi có unique index.
|
|
Đã hardening thêm `app/Jobs/Payment/GetTransactionPayment.php` (bank-statement poller): đổi từ pattern `where()->first()` rồi `create()` (race window) sang `firstOrCreate()` + check `wasRecentlyCreated` trước khi cộng tiền, tận dụng cùng cơ chế race-safe trên.
|
|
|
|
## P2 — Nhất quán dữ liệu / logic ✅ DONE (2026-09-05)
|
|
|
|
- [ ] **Giá renewal không nhất quán giữa cron tự động và tự-gia-hạn** (CHƯA SỬA — để dành, xem ghi chú)
|
|
File: `app/Console/Commands/ServiceExpiry.php` (dùng giá đông cứng `$orderItem->price` lúc mua ban đầu) vs `app/Services/Api/Order/OrderQueryService.php::renewService()` (dùng giá catalog hiện tại qua `productRepo->getPricing()`)
|
|
Ghi chú: đây là quyết định nghiệp vụ (cron tự động có nên áp giá mới như khách tự gia hạn hay giữ giá cũ để tránh sốc giá tự động) — cần hỏi lại business trước khi đổi, chưa tự quyết.
|
|
|
|
- [x] **`cycleToMonths()` bị duplicate ở 4 nơi**
|
|
Đã gom về `App\Helpers\OrderHelper::cycleToMonths()` (static), thay thế tại: `InvoiceRepository`, `InvoicePaymentService`, `Api\Order\OrderService`, `Customer\CustomerServiceProvisioningService`. Xóa 4 bản `private function cycleToMonths()` trùng lặp.
|
|
Verify: chạy lại toàn bộ test đã có ở P0/P1 sau refactor — tất cả PASS (không đổi hành vi).
|
|
|
|
- [x] **Tax handling không nhất quán giữa order client-facing và admin**
|
|
Quyết định (theo xác nhận của chủ dự án): giá niêm yết đã bao gồm thuế — không cộng thêm % lên trên ở bất kỳ đâu.
|
|
File: `app/Services/OrderService.php::createOrder()` — bỏ `$orderTotal = $total + $total*10%`, đổi thành `$orderTotal = $total`; `invoice.tax` set về `0` (khớp với `InvoiceRepository::createFromOrder()` phía client-facing vốn đã set `tax=0`).
|
|
Ghi chú: không viết test riêng cho thay đổi 1 dòng arithmetic này — `createOrder()` phụ thuộc sâu vào hệ thống module VM/hosting (dựng test tốn công không tương xứng với rủi ro của 1 phép tính đơn giản).
|
|
|
|
## P3 — Frontend ✅ DONE (2026-09-05)
|
|
|
|
- [x] **Bảng Invoice chính thiếu trạng thái "Quá hạn"/"Đã hoàn tiền"**
|
|
File: `portal-admin/src/features/order-invoice/data/data.tsx` — thêm `4: Quá hạn`, `5: Đã hoàn tiền` vào `invoiceStatuses`.
|
|
Cập nhật màu Tag tương ứng ở 2 nơi tiêu thụ: `invoices-columns.tsx` (`statusColor`), `invoices-table.tsx` (`colors`).
|
|
Verify: `yarn build` — không phát sinh lỗi TS mới trong 3 file đã sửa (các lỗi build hiện có trên nhánh là pre-existing, không liên quan: `firstname/lastname` type mismatch ở `app-sidebar.tsx`/`profile-dropdown.tsx`/`activity-logs-table.tsx`/`users-columns.tsx`, `fmtVND` unused ở `customers-table.tsx`).
|
|
Không làm: hợp nhất hoàn toàn với map riêng ở `customer-invoices-tab.tsx` — 2 map dùng shape và icon-library khác nhau (`@iconify/react` string icon vs `lucide-react` component); hợp nhất đòi hỏi đổi shape ở nhiều nơi, rủi ro cao hơn lợi ích so với việc chỉ vá đúng chỗ thiếu status. Để dành nếu sau này cần refactor UI invoice status rộng hơn.
|
|
|
|
## P4 — Dọn dead code (rủi ro nếu vô tình gọi lại) ✅ DONE (2026-09-05)
|
|
|
|
- [x] Xóa `app/Http/Services/Invoice/InvoiceService.php` — dùng cột không tồn tại (`order_num`, `date_paid`).
|
|
- [x] Xóa `app/Jobs/VirtualMachine/ProcessOrderPaymentJob.php` — dùng cột không tồn tại (`orders.amount`, `order_items.product_type`), không route nào gọi.
|
|
- [x] Xóa `app/Http/Controllers/Api/Service/PaymentController.php` — không được route tới (route sống là `OrderControllerRefactor::payOrder`).
|
|
- [x] Xóa `tests/Unit/ProcessOrderPaymentJobTest.php` — đã verify chạy FAIL 4/4 (`Class "Database\Factories\OrderFactory" not found`), test cho code chết.
|
|
- [x] Xóa `app/Console/Commands/UpdateExpiryDate.php` — xác nhận `order_items.expiry_date` không tồn tại trong bất kỳ migration nào (grep toàn bộ `database/migrations`), lệnh sẽ lỗi SQL nếu chạy, không có trong `Kernel::schedule()`.
|
|
- [x] Xóa 2 import chết còn sót trong `app/Http/Controllers/Api/Service/ServiceController.php` (`use InvoiceService`, `use ProcessOrderPaymentJob`) — import nhưng không dùng ở đâu trong file.
|
|
Verify: `grep` toàn repo xác nhận không còn tham chiếu nào tới 5 file/class đã xóa; `php -l` pass; chạy lại toàn bộ 9 test đã viết ở P0/P1 — vẫn PASS.
|
|
|
|
## P5 — Test cần bổ sung ✅ DONE (2026-09-05) — tất cả 11 test PASS
|
|
|
|
- [x] `test_renewal_invoice_uses_original_order_id_not_new_order` → `RenewServiceTest::test_renew_reuses_original_order_instead_of_creating_a_new_one`
|
|
- [x] `test_second_renewal_for_same_service_still_uses_original_order_id` → `ServiceExpiryTest::test_expiry_command_creates_renewal_invoice_on_the_original_order`
|
|
- [x] `test_concurrent_renewal_requests_create_only_one_invoice_for_same_period` → `RenewServiceTest::test_renewal_invoice_has_a_billing_period_and_is_not_duplicated_for_the_same_period`, `ServiceExpiryTest::test_running_expiry_command_twice_does_not_duplicate_the_renewal_invoice`
|
|
- [x] `test_concurrent_pay_requests_do_not_double_deduct_balance` → `InvoicePaymentServiceTest::test_paying_an_already_paid_invoice_does_not_double_deduct_balance`
|
|
- [x] `test_duplicate_manual_deposit_approval_does_not_double_credit` → `AdminTransactionApproveTest::test_approving_the_same_manual_deposit_twice_credits_the_customer_only_once`
|
|
- [x] `test_cancelled_order_does_not_trigger_renewal_invoice_generation` → `ServiceExpiryTest::test_cancelled_service_does_not_get_a_renewal_invoice`
|
|
- [x] `test_cancelled_invoice_is_not_counted_as_paid` → `InvoicePaymentServiceTest::test_a_cancelled_invoice_cannot_be_paid_and_is_never_counted_as_paid`
|
|
|
|
Tổng: `RenewServiceTest` (5), `ServiceExpiryTest` (4), `InvoicePaymentServiceTest` (2), `AdminTransactionApproveTest` (1), `InvoiceTestRouteRemovedTest` (1, có sẵn) — 13 test, chạy riêng lẻ đều PASS.
|
|
|
|
## P6 — Bug mới phát hiện khi viết test cho case "invoice OVERDUE rồi mới gia hạn tiếp" ✅ DONE (2026-09-05)
|
|
|
|
- [x] **`invoices:check-overdue` crash khi reset order status — do eager-load thiếu cột `customer_id`**
|
|
File: `app/Console/Commands/CheckOverdueInvoices.php` dòng 39; root cause thật ở `app/Observers/OrderObserver.php::updated()` (đăng ký tại `app/Providers/ObServerProvider.php`).
|
|
Logic sai: `Invoice::query()->with('order:id,status')` chỉ eager-load 2 cột của Order. Khi cron cần reset order status (ACTIVE → PENDING vì invoice quá hạn), `$order->update(['status' => ...])` bắn event `updated`, `OrderObserver::updated()` đọc `$order->customer_id` để ghi log — cột này KHÔNG được load nên là `null` → insert vào bảng `log` (`customer_id` NOT NULL) ném `QueryException`, không có try/catch bọc quanh vòng lặp → **toàn bộ cron `invoices:check-overdue` dừng ngay tại invoice đầu tiên cần reset order status**, các invoice quá hạn còn lại trong ngày không được xử lý.
|
|
Phát hiện bằng: viết test tích hợp thật (`ServiceExpiryTest::test_expiry_command_does_not_duplicate_invoice_after_it_became_overdue`, `RenewServiceTest::test_renew_while_existing_invoice_is_overdue_returns_the_same_invoice`) — gọi `invoices:check-overdue` thật (không mock) để mô phỏng đúng flow "invoice tới due_date → OVERDUE" trước khi test hành vi tránh trùng lặp; cả 2 test đều crash với `SQLSTATE[23000]: Column 'customer_id' cannot be null` trước khi fix.
|
|
Cách sửa: đổi `->with('order:id,status')` thành `->with('order:id,status,customer_id')` — 1 dòng, không đổi hành vi nghiệp vụ nào khác.
|
|
Verify: cả 13 test (bao gồm 2 test mới ở trên) PASS sau fix.
|
|
Business impact trước khi sửa: bất kỳ ngày nào có invoice renewal quá hạn của 1 service đang ACTIVE, cron `invoices:check-overdue` sẽ crash ngay từ invoice đầu tiên loại này và bỏ sót toàn bộ các invoice quá hạn phía sau trong cùng lần chạy — nghĩa là tính năng đánh dấu OVERDUE gần như không hoạt động đúng trong thực tế với dữ liệu thật (production DB có invoice + order theo đúng pattern này).
|
|
Ghi chú môi trường: test suite tổng (`--testsuite Feature`) trên nhánh `dev-domain` có ~40 lỗi pre-existing không liên quan (401/403/404 do cấu hình auth/route test), đã xác nhận bằng `git stash` so sánh baseline trước khi có bất kỳ fix nào ở đây — không phải do các thay đổi trong tài liệu này.
|
|
|
|
## Ghi chú (không cần sửa, giữ nguyên)
|
|
|
|
- `CheckOverdueInvoices` (cron `invoices:check-overdue`) đúng nghiệp vụ: chỉ update status, không tạo invoice mới. Không đổi.
|
|
- `InvoicePaymentService::activateOrder()` dùng `firstOrCreate(order_item_id)` chống double-provisioning — đã ổn.
|
|
- Không cần thêm bảng `subscriptions`/`billing_schedules` riêng — `services` + cron `service:expiry` đủ đóng vai trò billing schedule sau khi fix P0.
|