From e472c95a11775b13ffeebbf3e61d2b0103e5f30b Mon Sep 17 00:00:00 2001 From: johnnyq Date: Wed, 4 Nov 2026 14:38:32 -0500 Subject: [PATCH] Record Stripe fees every 15 minutes instead of once a night - a new Payment Provider Fees cron job runs the fee recorder that Nightly Tasks used to, Nightly Tasks no longer does, the recorder holds a lock so the job and Record fees now can't both record the same fee, and the payment provider screens no longer say fees are recorded each night --- .../payment_provider_edit.php | 2 +- admin/payment_providers.php | 6 +-- admin/post/payment_provider.php | 6 ++- cron/nightly_tasks.php | 14 +---- cron/payment_provider_fees.php | 54 +++++++++++++++++++ functions/payment_provider.php | 41 ++++++++++---- includes/cron_jobs.php | 8 +++ 7 files changed, 104 insertions(+), 27 deletions(-) create mode 100644 cron/payment_provider_fees.php diff --git a/admin/modals/payment_provider/payment_provider_edit.php b/admin/modals/payment_provider/payment_provider_edit.php index dc3e08d7b..888500d1a 100644 --- a/admin/modals/payment_provider/payment_provider_edit.php +++ b/admin/modals/payment_provider/payment_provider_edit.php @@ -170,7 +170,7 @@ ob_start(); data-provider-fees-switch> -
Each night, its fee on each card payment is recorded as an expense from the account above.
+
Its fee on each card payment is recorded as an expense from the account above, a few minutes after Stripe settles the payment.
diff --git a/admin/payment_providers.php b/admin/payment_providers.php index 226dd7b18..f93a31bc9 100644 --- a/admin/payment_providers.php +++ b/admin/payment_providers.php @@ -8,7 +8,7 @@ * and how many are still to record), its saved cards and the autopay on * them, and its card payments in the last 30 days. Check connection asks * the provider whose account the key is; Record fees now does what the - * nightly run does. Add and Edit are one window + * Payment Provider Fees job does every few minutes. Add and Edit are one window * (modals/payment_provider/payment_provider_edit.php); the rest are * admin/post/payment_provider.php. */ @@ -155,7 +155,7 @@ $csrf = $_SESSION['csrf_token']; to record + title=" Cron should have had them' : 'Recorded a few minutes after Stripe settles them' ?>"> to record Not recorded @@ -187,7 +187,7 @@ $csrf = $_SESSION['csrf_token']; Check connection - + Record fees now diff --git a/admin/post/payment_provider.php b/admin/post/payment_provider.php index f2eea40cf..9be19fc13 100644 --- a/admin/post/payment_provider.php +++ b/admin/post/payment_provider.php @@ -165,7 +165,7 @@ if (isset($_GET['check_payment_provider_connection'])) { } -// Record fees now - what the nightly run does, for the payments whose fee is still to record +// Record fees now - what the Payment Provider Fees job does every few minutes, for the payments whose fee is still to record if (isset($_GET['record_payment_provider_fees'])) { validateCSRFToken(); @@ -182,6 +182,10 @@ if (isset($_GET['record_payment_provider_fees'])) { } $done = paymentProviderRecordFees($row); + if ($done['busy']) { + flashAlert('' . escapeHtml($name) . '\'s fees are being recorded right now by the scheduled run - look again in a minute', 'info'); + redirect(); + } $said = []; if ($done['recorded']) { $said[] = 'Recorded its fee on ' . $done['recorded'] . ' payment' . ($done['recorded'] === 1 ? '' : 's') . ' - ' . paymentProviderMoneyText($done['amount']); diff --git a/cron/nightly_tasks.php b/cron/nightly_tasks.php index 89577c689..c5607c3f6 100644 --- a/cron/nightly_tasks.php +++ b/cron/nightly_tasks.php @@ -758,19 +758,7 @@ while ($row = mysqli_fetch_assoc($sql_recurring_payments)) { } } -/* - * Stripe fee reconciliation - the fee on each recent card payment that has - * none recorded yet, as Stripe charged it, once it has settled - * (paymentProviderRecordFees(), functions/payment_provider.php - the same - * thing Record fees now on Admin > Payment Providers does). One card payment - * over several invoices has one fee, taken once. A payment can complete - * before Stripe settles it; that one is picked up on a later run. - */ -$stripe_provider = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT payment_provider_name, payment_provider_private_key, payment_provider_account, - payment_provider_expense_vendor, payment_provider_expense_category FROM payment_providers WHERE payment_provider_name = 'Stripe' LIMIT 1")); -if ($stripe_provider) { - paymentProviderRecordFees($stripe_provider); -} +// Stripe fees are recorded by their own job every few minutes - cron/payment_provider_fees.php /* * Recurring expenses - each running one due today, or missed by up to diff --git a/cron/payment_provider_fees.php b/cron/payment_provider_fees.php new file mode 100644 index 000000000..adac6dd74 --- /dev/null +++ b/cron/payment_provider_fees.php @@ -0,0 +1,54 @@ + Payment Providers does. A card payment usually settles within + * seconds, so running this every few minutes has its fee on the books shortly after the + * payment instead of the next morning. One that hasn't settled yet waits for a later run. + * + * Safe on an interval: a payment whose fee is recorded is never looked at again, and the + * recorder holds a lock so this and Record fees now can't both record the same fee. With + * nothing waiting, a run is one query and no call to Stripe. + */ + +// Set working directory to the directory this cron script lives at. +chdir(dirname(__FILE__)); + +// Ensure we're running from command line +if (php_sapi_name() !== 'cli') { + die("This script must be run from the command line.\n"); +} + +// Prevent overlapping runs of this script +$cron_lock_script = __FILE__; +require_once "includes/cron_lock.php"; + +require_once "../config.php"; + +// Set Timezone +require_once "../includes/inc_set_timezone.php"; +require_once "../functions.php"; + +$payment_provider_fees_settings = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT config_enable_cron FROM settings WHERE company_id = 1")); + +if (intval($payment_provider_fees_settings['config_enable_cron'] ?? 0) == 0) { + cronJobStop("Cron: is not enabled\n"); +} + +$payment_provider_fees_provider = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT payment_provider_name, payment_provider_private_key, payment_provider_account, + payment_provider_expense_vendor, payment_provider_expense_category FROM payment_providers WHERE payment_provider_name = 'Stripe' LIMIT 1")); + +if (!$payment_provider_fees_provider) { + cronJobStop("No Stripe payment provider\n"); +} + +$payment_provider_fees_done = paymentProviderRecordFees($payment_provider_fees_provider); + +if ($payment_provider_fees_done['busy']) { + echo "Fees are being recorded already - left to that run\n"; +} else { + echo $payment_provider_fees_done['recorded'] . " fee(s) recorded, " . $payment_provider_fees_done['waiting'] . " waiting for Stripe to settle, " . $payment_provider_fees_done['failed'] . " failed\n"; +} diff --git a/functions/payment_provider.php b/functions/payment_provider.php index 8c3c82f5b..1f4f652a2 100644 --- a/functions/payment_provider.php +++ b/functions/payment_provider.php @@ -6,8 +6,8 @@ * The services clients pay invoices by card through - Stripe, for now. Each * has its keys (live or test, as the keys say), whether it is on, the account * its payments go into, the invoices it is too small for (the card limit), and - * whether its fees are recorded as expenses - each night, by - * cron/nightly_tasks.php. Paying through one is functions/billing_pay.php. + * whether its fees are recorded as expenses - every few minutes, by + * cron/payment_provider_fees.php. Paying through one is functions/billing_pay.php. * * A provider is saved only once it is right: its keys look like that * provider's, come from the same mode, and - when they are new or changed - @@ -411,16 +411,19 @@ function paymentProviderUnrecordedSql() { /* * Record the provider's fee on its recent card payments that have none yet, - * from what it actually charged - each night (cron/nightly_tasks.php), or - * now from Payment Providers. A payment it hasn't settled yet waits for the + * from what it actually charged - every few minutes (cron/payment_provider_fees.php), + * or now from Payment Providers. A payment it hasn't settled yet waits for the * next run. Only while its fees are recorded, under a vendor and a category. * ['recorded' => how many, 'amount' => their fees by currency, 'waiting' => * how many not settled yet, 'failed' => how many it wouldn't say, 'error' => - * the last thing it said]. + * the last thing it said, 'busy' => another run had it in hand, so nothing was done]. + * + * One run at a time: "has no fee yet" is read before the fee is written, so two runs + * at once - the job, and someone pressing Record fees now - would each record it. */ function paymentProviderRecordFees(array $provider, $limit = 50) { global $mysqli; - $result = ['recorded' => 0, 'amount' => [], 'waiting' => 0, 'failed' => 0, 'error' => '']; + $result = ['recorded' => 0, 'amount' => [], 'waiting' => 0, 'failed' => 0, 'error' => '', 'busy' => false]; $private_key = (string) ($provider['payment_provider_private_key'] ?? ''); $vendor_id = intval($provider['payment_provider_expense_vendor'] ?? 0); $category_id = intval($provider['payment_provider_expense_category'] ?? 0); @@ -429,6 +432,26 @@ function paymentProviderRecordFees(array $provider, $limit = 50) { return $result; } + $lock = mysqli_fetch_row(mysqli_query($mysqli, "SELECT GET_LOCK('itflow_payment_provider_fees', 0)")); + if (intval($lock[0] ?? 0) !== 1) { + $result['busy'] = true; + return $result; + } + try { + return paymentProviderRecordFeesLocked($provider, $limit, $result); + } finally { + mysqli_query($mysqli, "SELECT RELEASE_LOCK('itflow_payment_provider_fees')"); + } +} + +/* paymentProviderRecordFees(), with its lock held. */ +function paymentProviderRecordFeesLocked(array $provider, $limit, array $result) { + global $mysqli; + $private_key = (string) $provider['payment_provider_private_key']; + $vendor_id = intval($provider['payment_provider_expense_vendor']); + $category_id = intval($provider['payment_provider_expense_category']); + $account_id = intval($provider['payment_provider_account'] ?? 0); + $sql = mysqli_query($mysqli, "SELECT payment_reference, MIN(payment_date) AS payment_date, SUM(payment_amount) AS payment_amount, GROUP_CONCAT(CONCAT(invoice_prefix, invoice_number) ORDER BY invoice_number SEPARATOR ', ') AS invoice_numbers, MIN(invoice_client_id) AS invoice_client_id @@ -477,8 +500,8 @@ function paymentProviderRecordFees(array $provider, $limit = 50) { * What has gone through a provider in the last 30 days - 'payments' (card * payments, a split one once), 'paid' and 'fees' (by currency), and, while * its fees are recorded, 'waiting' (payments with no fee recorded yet) and - * 'stale' (of those, the ones more than two days old - the nightly run - * should have had them). + * 'stale' (of those, the ones more than two days old - the Payment + * Provider Fees job should have had them). */ function paymentProviderActivity(array $provider) { global $mysqli; @@ -541,7 +564,7 @@ function paymentProviderProblems() { $activity = paymentProviderActivity($provider); if ($activity['stale']) { $problems[] = ['warning', 'fa-receipt', "$name's fees aren't being recorded", - $activity['stale'] . ' card payment' . ($activity['stale'] === 1 ? '' : 's') . ' more than two days old ' . ($activity['stale'] === 1 ? 'has' : 'have') . ' no fee recorded. The nightly run records them - or record them now.', 'payment_providers.php', 'Payment Providers']; + $activity['stale'] . ' card payment' . ($activity['stale'] === 1 ? '' : 's') . ' more than two days old ' . ($activity['stale'] === 1 ? 'has' : 'have') . ' no fee recorded. The Payment Provider Fees job in Maintenance > Cron records them - check it is on, or record them now.', 'payment_providers.php', 'Payment Providers']; } } return $problems; diff --git a/includes/cron_jobs.php b/includes/cron_jobs.php index d1a2b662b..3ec554ede 100644 --- a/includes/cron_jobs.php +++ b/includes/cron_jobs.php @@ -75,6 +75,14 @@ function cronJobRegistry(): array 'schedule' => 'Interval', 'interval_minutes' => 1, ], + [ + 'name' => 'payment_provider_fees', + 'label' => 'Payment Provider Fees', + 'script' => 'payment_provider_fees.php', + 'description' => 'Records what Stripe charged on each card payment as an expense, once Stripe has settled it - while Admin > Payment Providers records its fees.', + 'schedule' => 'Interval', + 'interval_minutes' => 15, + ], [ 'name' => 'domain_refresher', 'label' => 'Domain Refresher',