From 6490729fcc1aef19c1c68a8188691b118a550d71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Wed, 15 Jul 2026 09:47:53 +0200 Subject: [PATCH] refactor: extract duplicated money formatter into App\Support\Money (#680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What The same currency-symbol money formatter — a `match` on `strtolower($currency)` returning a symbol, concatenated with `number_format($cents / 100, 2)` — was copy-pasted across **five** files: - `app/Console/Commands/SendExperimentFunnelReportCommand.php` (`money(int $cents, …)`) - `app/Listeners/PostStripeEventToDiscord.php` (`money(int $amount, …)`) - `app/Console/Commands/SyncStripePricesCommand.php` (`formatAmount(int $amountInCents, …)`) - `app/Console/Commands/StripeSubscriptionStatsCommand.php` (`format(float $amount, …)`) - `app/Console/Commands/SendDailyStatsReportCommand.php` (`money(float $amount, …)`) This extracts it into a single `App\Support\Money::format(int $cents, string $currency): string` helper and removes the duplicated logic from every site. ## Zero behavior change The five copies were **not** verbatim — they diverged on three axes: unit (cents vs. major units), symbol coverage, and default casing. The shared helper is the **union** of the existing maps (`eur`/`gbp`/`usd`/`jpy`/`brl`, otherwise uppercased code + trailing space), and reproduces every call site's output for the currencies each actually receives: - **Cents-based sites** (experiment funnel, Discord listener, price sync) call the helper directly. - The experiment funnel only ever passes the Cashier currency (uppercased `EUR` from the collector), so its narrower map / raw-case default was a dead branch. - The price sync command only formats `config('cashier.currency')` (`eur`), so it never hit `usd`/`brl` — adding those symbols is a no-op here. - **Major-unit sites** (`StripeSubscriptionStatsCommand`, `SendDailyStatsReportCommand`) hold MRR as floats, so their private method now converts to cents (`(int) round($amount * 100)`) and delegates; call sites are untouched. `number_format` of the round-tripped value is identical to the original. ## Tests - New `tests/Unit/Support/MoneyTest.php` covering `eur`, `gbp`, `usd`, `jpy`, `brl`, an unknown currency (`chf` → `CHF 3.99`), case-insensitivity, thousands separator, and zero. - The existing command feature tests (which assert exact rendered strings like `€10.00`, `€19.99`, `€9.99 / month → €3.99 / month`) still pass unchanged. `pint`, `phpstan`, and the affected feature + unit tests are green locally (42 passing). --- .../Commands/SendDailyStatsReportCommand.php | 16 +++++------ .../SendExperimentFunnelReportCommand.php | 25 +++++------------ .../StripeSubscriptionStatsCommand.php | 16 +++++------ .../Commands/SyncStripePricesCommand.php | 20 ++++---------- app/Listeners/PostStripeEventToDiscord.php | 19 +++---------- app/Support/Money.php | 27 +++++++++++++++++++ tests/Unit/Support/MoneyTest.php | 17 ++++++++++++ 7 files changed, 71 insertions(+), 69 deletions(-) create mode 100644 app/Support/Money.php create mode 100644 tests/Unit/Support/MoneyTest.php diff --git a/app/Console/Commands/SendDailyStatsReportCommand.php b/app/Console/Commands/SendDailyStatsReportCommand.php index 68fc4965..64b2e545 100644 --- a/app/Console/Commands/SendDailyStatsReportCommand.php +++ b/app/Console/Commands/SendDailyStatsReportCommand.php @@ -6,6 +6,7 @@ use App\Console\Commands\Concerns\RendersReportToConsole; use App\Models\User; use App\Services\Discord\DiscordWebhook; use App\Services\Stripe\SubscriptionStatsCollector; +use App\Support\Money; use Illuminate\Console\Command; use Illuminate\Support\Carbon; use Stripe\Exception\ApiErrorException; @@ -115,17 +116,12 @@ class SendDailyStatsReportCommand extends Command ]; } + /** + * MRR figures are held in major units (e.g. euros), so convert to cents for + * the shared formatter. + */ private function money(float $amount, string $currency): string { - $symbol = match (strtolower($currency)) { - 'eur' => '€', - 'gbp' => '£', - 'usd' => '$', - 'jpy' => '¥', - 'brl' => 'R$', - default => strtoupper($currency).' ', - }; - - return $symbol.number_format($amount, 2); + return Money::format((int) round($amount * 100), $currency); } } diff --git a/app/Console/Commands/SendExperimentFunnelReportCommand.php b/app/Console/Commands/SendExperimentFunnelReportCommand.php index cd2caaa7..56f56b2f 100644 --- a/app/Console/Commands/SendExperimentFunnelReportCommand.php +++ b/app/Console/Commands/SendExperimentFunnelReportCommand.php @@ -7,6 +7,7 @@ use App\Services\Discord\DiscordWebhook; use App\Services\Stats\BinomialProportion; use App\Services\Stats\ExperimentFunnelCollector; use App\Services\Stats\ProportionSignificance; +use App\Support\Money; use Carbon\CarbonImmutable; use Illuminate\Console\Command; @@ -93,11 +94,11 @@ class SendExperimentFunnelReportCommand extends Command $row['assignedMature'], $row['convertedMature'], $mature ? ((int) round($row['conversionRate'] * 100)).'%' : 'pend', - $showMoney && $row['arpuCents'] !== null ? $this->money($row['arpuCents'], $currency) : '—', - $showMoney ? $this->money($row['mrrCents'], $currency) : '—', - $mature ? $this->money($row['costCents'], $currency) : '—', - $mature ? $this->money($row['wastedCostCents'], $currency) : '—', - $showMoney ? $this->money($row['contributionMarginCents'], $currency) : '—', + $showMoney && $row['arpuCents'] !== null ? Money::format($row['arpuCents'], $currency) : '—', + $showMoney ? Money::format($row['mrrCents'], $currency) : '—', + $mature ? Money::format($row['costCents'], $currency) : '—', + $mature ? Money::format($row['wastedCostCents'], $currency) : '—', + $showMoney ? Money::format($row['contributionMarginCents'], $currency) : '—', ); } @@ -171,18 +172,6 @@ class SendExperimentFunnelReportCommand extends Command return number_format($rate * 100, 1).'%'; } - private function money(int $cents, string $currency): string - { - $symbol = match (strtolower($currency)) { - 'eur' => '€', - 'gbp' => '£', - 'usd' => '$', - default => $currency.' ', - }; - - return $symbol.number_format($cents / 100, 2); - } - /** * @param array{startedAt: ?CarbonImmutable, currency: string, revenueAvailable: bool, costPerConnectionCents: int, variants: array>} $report * @return array @@ -208,7 +197,7 @@ class SendExperimentFunnelReportCommand extends Command 'name' => 'Legend', 'value' => sprintf( 'Assg = signups · Actd = activated (connected a bank or enabled AI = cost triggered) · Card = completed checkout (card on file) · MatU = matured assigned (cohort old enough to score for this variant) · Conv = matured users who ever converted (were charged, net of refund) — time-invariant, so it does not shrink as an older cohort has longer to churn · Conv%% = Conv ÷ MatU (always ≤100%%, comparable across variants) · ARPU = MRR ÷ MatU (revenue per matured user) · MRR = monthly run-rate of *currently* paying subs (yearly ÷ 12); Conv above MRR is churn · Cost = est. connection cost of MatU (%s/connection) · Burn = connection cost of matured users who never earned net revenue (connected a bank but never paid, or paid then refunded) · CM = MRR − Cost · `pend`/`—` = no matured data yet.', - $this->money($report['costPerConnectionCents'], $report['currency']), + Money::format($report['costPerConnectionCents'], $report['currency']), ), 'inline' => false, ], diff --git a/app/Console/Commands/StripeSubscriptionStatsCommand.php b/app/Console/Commands/StripeSubscriptionStatsCommand.php index aa840f14..7ebc4e4f 100644 --- a/app/Console/Commands/StripeSubscriptionStatsCommand.php +++ b/app/Console/Commands/StripeSubscriptionStatsCommand.php @@ -3,6 +3,7 @@ namespace App\Console\Commands; use App\Services\Stripe\SubscriptionStatsCollector; +use App\Support\Money; use Illuminate\Console\Command; use Stripe\Exception\ApiErrorException; @@ -77,17 +78,12 @@ class StripeSubscriptionStatsCommand extends Command $this->newLine(); } + /** + * MRR figures are held in major units (e.g. euros), so convert to cents for + * the shared formatter. + */ private function format(float $amount, string $currency): string { - $symbol = match (strtolower($currency)) { - 'eur' => '€', - 'gbp' => '£', - 'usd' => '$', - 'jpy' => '¥', - 'brl' => 'R$', - default => strtoupper($currency).' ', - }; - - return $symbol.number_format($amount, 2); + return Money::format((int) round($amount * 100), $currency); } } diff --git a/app/Console/Commands/SyncStripePricesCommand.php b/app/Console/Commands/SyncStripePricesCommand.php index 6a3c7cad..ce34f821 100644 --- a/app/Console/Commands/SyncStripePricesCommand.php +++ b/app/Console/Commands/SyncStripePricesCommand.php @@ -2,6 +2,7 @@ namespace App\Console\Commands; +use App\Support\Money; use Illuminate\Console\Command; use Laravel\Cashier\Cashier; use Stripe\Exception\ApiErrorException; @@ -50,6 +51,7 @@ class SyncStripePricesCommand extends Command $amountInCents = (int) round($plan['price'] * 100); $billingPeriod = $plan['billing_period'] ?? null; $productId = config('subscriptions.products.pro'); + $formattedAmount = Money::format($amountInCents, $currency); $this->line(" {$plan['name']}"); @@ -68,7 +70,7 @@ class SyncStripePricesCommand extends Command if (! $dryRun) { $price = $this->createPrice($productId, $amountInCents, $currency, $billingPeriod, $lookupKey, transferLookupKey: true); - $this->line(" ✓ Transferred to {$price->id} ({$this->formatAmount($amountInCents, $currency)}/{$billingPeriod})"); + $this->line(" ✓ Transferred to {$price->id} ({$formattedAmount}/{$billingPeriod})"); } else { $this->line(" [dry-run] Would create new price and transfer lookup key '{$lookupKey}'"); } @@ -79,9 +81,9 @@ class SyncStripePricesCommand extends Command if (! $dryRun) { $price = $this->createPrice($productId, $amountInCents, $currency, $billingPeriod, $lookupKey, transferLookupKey: false); - $this->line(" ✓ Created {$price->id} ({$this->formatAmount($amountInCents, $currency)}/{$billingPeriod})"); + $this->line(" ✓ Created {$price->id} ({$formattedAmount}/{$billingPeriod})"); } else { - $this->line(" [dry-run] Would create price '{$lookupKey}' at {$this->formatAmount($amountInCents, $currency)}/{$billingPeriod}"); + $this->line(" [dry-run] Would create price '{$lookupKey}' at {$formattedAmount}/{$billingPeriod}"); } $created++; @@ -148,16 +150,4 @@ class SyncStripePricesCommand extends Command return $currencyMatches && $amountMatches && $intervalMatches; } - - private function formatAmount(int $amountInCents, string $currency): string - { - $symbol = match (strtolower($currency)) { - 'eur' => '€', - 'gbp' => '£', - 'jpy' => '¥', - default => strtoupper($currency).' ', - }; - - return $symbol.number_format($amountInCents / 100, 2); - } } diff --git a/app/Listeners/PostStripeEventToDiscord.php b/app/Listeners/PostStripeEventToDiscord.php index a8b60fbd..6f8ab88e 100644 --- a/app/Listeners/PostStripeEventToDiscord.php +++ b/app/Listeners/PostStripeEventToDiscord.php @@ -4,6 +4,7 @@ namespace App\Listeners; use App\Services\Discord\DiscordWebhook; use App\Services\Stripe\StripeCustomerResolver; +use App\Support\Money; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Support\Facades\Cache; use Laravel\Cashier\Events\WebhookReceived; @@ -171,7 +172,7 @@ class PostStripeEventToDiscord implements ShouldQueue $email = $this->stringOrNull($object['customer_email'] ?? null); $fields = [ - $this->field('Amount', $this->money((int) $meta['amount'], (string) ($object['currency'] ?? 'usd')), true), + $this->field('Amount', Money::format((int) $meta['amount'], (string) ($object['currency'] ?? 'usd')), true), $this->field('Customer', $email ?? $this->customers->label($this->stringOrNull($object['customer'] ?? null)), true), ]; @@ -237,7 +238,7 @@ class PostStripeEventToDiscord implements ShouldQueue return null; } - $label = $this->money((int) $amount, (string) ($price['currency'] ?? 'usd')); + $label = Money::format((int) $amount, (string) ($price['currency'] ?? 'usd')); $recurring = is_array($price['recurring'] ?? null) ? $price['recurring'] : $price; if (! isset($recurring['interval'])) { @@ -260,20 +261,6 @@ class PostStripeEventToDiscord implements ShouldQueue return is_string($reason) ? str_replace('_', ' ', $reason) : null; } - private function money(int $amount, string $currency): string - { - $symbol = match (strtolower($currency)) { - 'eur' => '€', - 'gbp' => '£', - 'usd' => '$', - 'jpy' => '¥', - 'brl' => 'R$', - default => strtoupper($currency).' ', - }; - - return $symbol.number_format($amount / 100, 2); - } - private function timestamp(mixed $value): ?string { if (! is_int($value) && ! (is_string($value) && ctype_digit($value))) { diff --git a/app/Support/Money.php b/app/Support/Money.php new file mode 100644 index 00000000..4c9de8a0 --- /dev/null +++ b/app/Support/Money.php @@ -0,0 +1,27 @@ + '€', + 'gbp' => '£', + 'usd' => '$', + 'jpy' => '¥', + 'brl' => 'R$', + default => strtoupper($currency).' ', + }; + + return $symbol.number_format($cents / 100, 2); + } +} diff --git a/tests/Unit/Support/MoneyTest.php b/tests/Unit/Support/MoneyTest.php new file mode 100644 index 00000000..d5317610 --- /dev/null +++ b/tests/Unit/Support/MoneyTest.php @@ -0,0 +1,17 @@ +toBe($expected); +})->with([ + 'eur' => [399, 'eur', '€3.99'], + 'gbp' => [399, 'gbp', '£3.99'], + 'usd' => [399, 'usd', '$3.99'], + 'jpy' => [100000, 'jpy', '¥1,000.00'], + 'brl' => [399, 'brl', 'R$3.99'], + 'unknown currency falls back to uppercased code' => [399, 'chf', 'CHF 3.99'], + 'currency match is case-insensitive' => [399, 'EUR', '€3.99'], + 'thousands separator' => [123456, 'eur', '€1,234.56'], + 'zero' => [0, 'eur', '€0.00'], +]);