Files
portal-api/docs/order-invoice-payment-fix-todo.md
2026-09-09 16:56:22 +07:00

13 KiB

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)

  • 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.

  • 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.

  • 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)

  • 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.

  • 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.

  • 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.

  • 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).

  • 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)

  • 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óa app/Http/Services/Invoice/InvoiceService.php — dùng cột không tồn tại (order_num, date_paid).
  • 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óa app/Http/Controllers/Api/Service/PaymentController.php — không được route tới (route sống là OrderControllerRefactor::payOrder).
  • Xóa tests/Unit/ProcessOrderPaymentJobTest.php — đã verify chạy FAIL 4/4 (Class "Database\Factories\OrderFactory" not found), test cho code chết.
  • 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ó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

  • test_renewal_invoice_uses_original_order_id_not_new_order → RenewServiceTest::test_renew_reuses_original_order_instead_of_creating_a_new_one
  • test_second_renewal_for_same_service_still_uses_original_order_id → ServiceExpiryTest::test_expiry_command_creates_renewal_invoice_on_the_original_order
  • 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
  • test_concurrent_pay_requests_do_not_double_deduct_balance → InvoicePaymentServiceTest::test_paying_an_already_paid_invoice_does_not_double_deduct_balance
  • test_duplicate_manual_deposit_approval_does_not_double_credit → AdminTransactionApproveTest::test_approving_the_same_manual_deposit_twice_credits_the_customer_only_once
  • test_cancelled_order_does_not_trigger_renewal_invoice_generation → ServiceExpiryTest::test_cancelled_service_does_not_get_a_renewal_invoice
  • 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)

  • 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.