From 4a1b5bc7250707472b6ed9cf801b6b454edecd34 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 2 May 2026 12:19:32 +0000
Subject: [PATCH] fix(billing): address code review feedback

- Use DECIMAL instead of FLOAT for monetary columns in ALTER TABLE
- Simplify bind_param type string from concatenated to single literal
- Validate payment_status against ENUM values before CSS class injection
- Add provisioning failure logging when panel bootstrap fails
- Add comment explaining total_due/amount legacy fallback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: iaretechnician <2749183+iaretechnician@users.noreply.github.com>
---
 modules/billing/admin_invoices.php            | 8 ++++++--
 modules/billing/api/capture_order.php         | 5 +++++
 modules/billing/classes/BillingRepository.php | 2 +-
 modules/billing/module.php                    | 6 +++---
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/modules/billing/admin_invoices.php b/modules/billing/admin_invoices.php
index c4babc78..87cdf778 100644
--- a/modules/billing/admin_invoices.php
+++ b/modules/billing/admin_invoices.php
@@ -43,7 +43,11 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'], $_POST['inv
         $msgType = 'error';
     } elseif ($action === 'mark_paid') {
         $gateway       = GatewayFactory::make('manual');
-        $captureResult = $gateway->handleCallback(['amount' => $invRow['total_due'] ?? $invRow['amount'] ?? 0, 'currency' => $invRow['currency'] ?? 'USD']);
+        $captureResult = $gateway->handleCallback([
+            // total_due is the new schema field; amount is the legacy column during migration
+            'amount'   => $invRow['total_due'] ?? $invRow['amount'] ?? 0,
+            'currency' => $invRow['currency'] ?? 'USD',
+        ]);
         $captureResult['payment_method'] = 'manual';
         $homeId  = intval($invRow['home_id'] ?? 0);
         $result  = $svc->processPaymentSuccess($captureResult, $invId, intval($invRow['user_id']), $homeId, $invRow);
@@ -131,7 +135,7 @@ if (isset($_GET['type'])) $msgType = $_GET['type'];
         <td><?= h($inv['players'] ?? '—') ?></td>
         <td style="font-size:11px"><?= h(substr($inv['period_start'] ?? '', 0, 10)) ?> – <?= h(substr($inv['period_end'] ?? '', 0, 10)) ?></td>
         <td><?= h(number_format((float)($inv['total_due'] ?? $inv['amount'] ?? 0), 2)) ?></td>
-        <td><span class="status-badge status-<?= h($inv['payment_status'] ?? 'unpaid') ?>"><?= h($inv['payment_status'] ?? 'unpaid') ?></span></td>
+        <td><span class="status-badge status-<?= h(in_array($inv['payment_status'] ?? '', ['unpaid','paid','cancelled','refunded']) ? $inv['payment_status'] : 'unpaid') ?>"><?= h($inv['payment_status'] ?? 'unpaid') ?></span></td>
         <td><?= h($inv['payment_method'] ?? '—') ?></td>
         <td style="font-size:11px;max-width:120px;overflow:hidden"><?= h($inv['payment_txid'] ?? '—') ?></td>
         <td>
diff --git a/modules/billing/api/capture_order.php b/modules/billing/api/capture_order.php
index cea38cd8..61628245 100644
--- a/modules/billing/api/capture_order.php
+++ b/modules/billing/api/capture_order.php
@@ -153,6 +153,11 @@ if (!empty($newOrderIds)) {
         $GLOBALS['settings'] = $panelCtx['settings'];
         require_once __DIR__ . '/../create_servers.php';
         $autoProvision = billing_invoke_provision(['order_ids' => $newOrderIds, 'user_id' => $userId, 'is_admin' => true]);
+        if (($autoProvision['failed_count'] ?? 0) > 0) {
+            cap_log('AUTO_PROVISION_PARTIAL_FAILURE', $autoProvision);
+        }
+    } else {
+        cap_log('AUTO_PROVISION_SKIPPED', 'panel bootstrap failed — orders require manual provisioning: ' . implode(',', $newOrderIds));
     }
 }
 
diff --git a/modules/billing/classes/BillingRepository.php b/modules/billing/classes/BillingRepository.php
index c059d5c1..f969f3db 100644
--- a/modules/billing/classes/BillingRepository.php
+++ b/modules/billing/classes/BillingRepository.php
@@ -109,7 +109,7 @@ class BillingRepository
         if (!$stmt) return 0;
         $rawJson = is_array($data['raw_response']) ? json_encode($data['raw_response']) : (string)($data['raw_response'] ?? '');
         $stmt->bind_param(
-            'iiissdss' . 's',
+            'iiissdsss',
             $data['invoice_id'],
             $data['user_id'],
             $data['home_id'],
diff --git a/modules/billing/module.php b/modules/billing/module.php
index a9e38c7e..c2aa5f80 100644
--- a/modules/billing/module.php
+++ b/modules/billing/module.php
@@ -130,12 +130,12 @@ $install_queries[1] = array(
     "ALTER TABLE `".OGP_DB_PREFIX."billing_invoices`
         ADD COLUMN IF NOT EXISTS `home_id` INT(11) NOT NULL DEFAULT 0 AFTER `service_id`,
         ADD COLUMN IF NOT EXISTS `rate_type` ENUM('daily','monthly','yearly') NOT NULL DEFAULT 'monthly' AFTER `invoice_duration`,
-        ADD COLUMN IF NOT EXISTS `rate_per_player` FLOAT(15,4) NOT NULL DEFAULT 0 AFTER `rate_type`,
+        ADD COLUMN IF NOT EXISTS `rate_per_player` DECIMAL(15,4) NOT NULL DEFAULT 0 AFTER `rate_type`,
         ADD COLUMN IF NOT EXISTS `players` INT(11) NOT NULL DEFAULT 0 AFTER `rate_per_player`,
         ADD COLUMN IF NOT EXISTS `period_start` DATETIME NULL AFTER `players`,
         ADD COLUMN IF NOT EXISTS `period_end` DATETIME NULL AFTER `period_start`,
-        ADD COLUMN IF NOT EXISTS `subtotal` FLOAT(15,2) NOT NULL DEFAULT 0 AFTER `period_end`,
-        ADD COLUMN IF NOT EXISTS `total_due` FLOAT(15,2) NOT NULL DEFAULT 0 AFTER `subtotal`,
+        ADD COLUMN IF NOT EXISTS `subtotal` DECIMAL(15,2) NOT NULL DEFAULT 0 AFTER `period_end`,
+        ADD COLUMN IF NOT EXISTS `total_due` DECIMAL(15,2) NOT NULL DEFAULT 0 AFTER `subtotal`,
         ADD COLUMN IF NOT EXISTS `payment_status` ENUM('unpaid','paid','cancelled','refunded') NOT NULL DEFAULT 'unpaid' AFTER `total_due`",
 
     // Payment transaction log — immutable audit trail