refactor(analytics): de-duplicate currency conversion and category-type helpers

The analytics controllers each carried byte-identical copies of
convertTransactionAmount()/preloadExchangeRates() (three copies) and
categoryType() (two copies), which let the analysis and cashflow screens
silently drift apart. Extract the currency helpers into a shared
ConvertsTransactionCurrency trait (following the existing OpenBanking/Concerns
pattern) and move categoryType() onto the Transaction model. No behavior
change.
This commit is contained in:
Víctor Falcón 2026-06-29 20:48:44 +02:00
parent fad189e23b
commit 1c18166901
5 changed files with 77 additions and 108 deletions

View File

@ -4,6 +4,7 @@ namespace App\Http\Controllers\Api;
use App\Enums\CategoryCashflowDirection;
use App\Enums\CategoryType;
use App\Http\Controllers\Api\Concerns\ConvertsTransactionCurrency;
use App\Http\Controllers\Controller;
use App\Models\Category;
use App\Models\Transaction;
@ -17,6 +18,8 @@ use Illuminate\Support\Collection;
class CashflowAnalyticsController extends Controller
{
use ConvertsTransactionCurrency;
private const MAX_TREND_MONTHS = 24;
public function __construct(
@ -222,7 +225,7 @@ class CashflowAnalyticsController extends Controller
{
return $transactions
->filter(function (Transaction $transaction) use ($type): bool {
if ($this->categoryType($transaction) === $type) {
if ($transaction->categoryType() === $type) {
return true;
}
@ -235,7 +238,7 @@ class CashflowAnalyticsController extends Controller
private function sumOutflowTransactions(Collection $transactions, string $userCurrency, CategoryType $type): int
{
return abs($transactions
->filter(fn (Transaction $transaction): bool => $this->categoryType($transaction) === $type
->filter(fn (Transaction $transaction): bool => $transaction->categoryType() === $type
&& $transaction->amount < 0)
->sum(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $userCurrency)));
}
@ -254,7 +257,7 @@ class CashflowAnalyticsController extends Controller
$regularCategories = $transactions
->filter(function (Transaction $transaction) use ($type): bool {
$categoryType = $this->categoryType($transaction);
$categoryType = $transaction->categoryType();
return $transaction->category_id !== null
&& ($categoryType === $type
@ -282,7 +285,7 @@ class CashflowAnalyticsController extends Controller
$transferCategories = $transactions
->filter(function (Transaction $transaction) use ($isIncome): bool {
return $transaction->category_id !== null
&& $this->categoryType($transaction) === CategoryType::Transfer
&& $transaction->categoryType() === CategoryType::Transfer
&& $this->categoryCashflowDirection($transaction) === ($isIncome
? CategoryCashflowDirection::Inflow
: CategoryCashflowDirection::Outflow);
@ -359,7 +362,7 @@ class CashflowAnalyticsController extends Controller
foreach ($categorized as $categoryTransactions) {
$firstTransaction = $categoryTransactions->first();
$type = $this->categoryType($firstTransaction);
$type = $firstTransaction->categoryType();
if (! in_array($type, [CategoryType::Income, CategoryType::Expense], true)) {
continue;
@ -406,7 +409,7 @@ class CashflowAnalyticsController extends Controller
$this->preloadExchangeRates($transactions, $userCurrency);
$categorized = $transactions
->filter(fn (Transaction $transaction): bool => $this->categoryType($transaction) === $type)
->filter(fn (Transaction $transaction): bool => $transaction->categoryType() === $type)
->groupBy('category_id')
->map(function (Collection $transactions) use ($userCurrency): array {
$totalAmount = $transactions->sum(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $userCurrency));
@ -461,42 +464,6 @@ class CashflowAnalyticsController extends Controller
});
}
private function convertTransactionAmount(Transaction $transaction, string $userCurrency): int
{
return $this->exchangeRateService->convert(
$transaction->currency_code ?: $transaction->account?->currency_code ?: $userCurrency,
$userCurrency,
$transaction->amount,
$transaction->transaction_date->toDateString(),
);
}
private function preloadExchangeRates(Collection $transactions, string $userCurrency): void
{
$dates = $transactions
->filter(fn (Transaction $transaction): bool => strcasecmp($transaction->currency_code ?: $transaction->account?->currency_code ?: $userCurrency, $userCurrency) !== 0)
->map(fn (Transaction $transaction): string => $transaction->transaction_date->toDateString())
->unique()
->values();
if ($dates->isEmpty()) {
return;
}
$this->exchangeRateService->preloadRates($userCurrency, $dates);
}
private function categoryType(Transaction $transaction): ?CategoryType
{
$type = $transaction->category?->getAttribute('type');
if ($type instanceof CategoryType) {
return $type;
}
return is_string($type) ? CategoryType::tryFrom($type) : null;
}
private function categoryCashflowDirection(Transaction $transaction): ?CategoryCashflowDirection
{
$direction = $transaction->category?->getAttribute('cashflow_direction');

View File

@ -3,6 +3,7 @@
namespace App\Http\Controllers\Api;
use App\Enums\CategoryType;
use App\Http\Controllers\Api\Concerns\ConvertsTransactionCurrency;
use App\Http\Controllers\Controller;
use App\Models\Category;
use App\Models\Transaction;
@ -14,6 +15,8 @@ use Illuminate\Support\Collection;
class CategoryMonthlyBreakdownController extends Controller
{
use ConvertsTransactionCurrency;
/**
* The rolling window shown on the chart, in months (including the current).
*/
@ -276,32 +279,4 @@ class CategoryMonthlyBreakdownController extends Controller
return $months;
}
private function convertTransactionAmount(Transaction $transaction, string $currency): int
{
return $this->exchangeRateService->convert(
$transaction->currency_code ?: $transaction->account?->currency_code ?: $currency,
$currency,
$transaction->amount,
$transaction->transaction_date->toDateString(),
);
}
/**
* @param Collection<int, Transaction> $transactions
*/
private function preloadExchangeRates(Collection $transactions, string $currency): void
{
$dates = $transactions
->filter(fn (Transaction $transaction): bool => strcasecmp($transaction->currency_code ?: $transaction->account?->currency_code ?: $currency, $currency) !== 0)
->map(fn (Transaction $transaction): string => $transaction->transaction_date->toDateString())
->unique()
->values();
if ($dates->isEmpty()) {
return;
}
$this->exchangeRateService->preloadRates($currency, $dates);
}
}

View File

@ -0,0 +1,43 @@
<?php
namespace App\Http\Controllers\Api\Concerns;
use App\Models\Transaction;
use App\Services\ExchangeRateService;
use Illuminate\Support\Collection;
/**
* Shared currency conversion for the analytics controllers. Each consumer
* injects an {@see ExchangeRateService} as `$exchangeRateService`, then reads
* transaction amounts in the user's currency through these helpers.
*/
trait ConvertsTransactionCurrency
{
protected function convertTransactionAmount(Transaction $transaction, string $currency): int
{
return $this->exchangeRateService->convert(
$transaction->currency_code ?: $transaction->account?->currency_code ?: $currency,
$currency,
$transaction->amount,
$transaction->transaction_date->toDateString(),
);
}
/**
* @param Collection<int, Transaction> $transactions
*/
protected function preloadExchangeRates(Collection $transactions, string $currency): void
{
$dates = $transactions
->filter(fn (Transaction $transaction): bool => strcasecmp($transaction->currency_code ?: $transaction->account?->currency_code ?: $currency, $currency) !== 0)
->map(fn (Transaction $transaction): string => $transaction->transaction_date->toDateString())
->unique()
->values();
if ($dates->isEmpty()) {
return;
}
$this->exchangeRateService->preloadRates($currency, $dates);
}
}

View File

@ -3,6 +3,7 @@
namespace App\Http\Controllers\Api;
use App\Enums\CategoryType;
use App\Http\Controllers\Api\Concerns\ConvertsTransactionCurrency;
use App\Http\Controllers\Controller;
use App\Http\Requests\IndexTransactionRequest;
use App\Models\Label;
@ -15,6 +16,8 @@ use Illuminate\Support\Collection;
class TransactionAnalysisController extends Controller
{
use ConvertsTransactionCurrency;
/**
* A daily breakdown is used while the filtered set spans this many days or
* fewer; beyond that the chart switches to monthly buckets.
@ -374,7 +377,7 @@ class TransactionAnalysisController extends Controller
private function isExpense(Transaction $transaction): bool
{
return $transaction->amount < 0
&& in_array($this->categoryType($transaction), [CategoryType::Expense, null], true);
&& in_array($transaction->categoryType(), [CategoryType::Expense, null], true);
}
/**
@ -385,42 +388,6 @@ class TransactionAnalysisController extends Controller
private function isIncome(Transaction $transaction): bool
{
return $transaction->amount > 0
&& in_array($this->categoryType($transaction), [CategoryType::Income, null], true);
}
private function categoryType(Transaction $transaction): ?CategoryType
{
$type = $transaction->category?->getAttribute('type');
if ($type instanceof CategoryType) {
return $type;
}
return is_string($type) ? CategoryType::tryFrom($type) : null;
}
private function convertTransactionAmount(Transaction $transaction, string $currency): int
{
return $this->exchangeRateService->convert(
$transaction->currency_code ?: $transaction->account?->currency_code ?: $currency,
$currency,
$transaction->amount,
$transaction->transaction_date->toDateString(),
);
}
private function preloadExchangeRates(Collection $transactions, string $currency): void
{
$dates = $transactions
->filter(fn (Transaction $transaction): bool => strcasecmp($transaction->currency_code ?: $transaction->account?->currency_code ?: $currency, $currency) !== 0)
->map(fn (Transaction $transaction): string => $transaction->transaction_date->toDateString())
->unique()
->values();
if ($dates->isEmpty()) {
return;
}
$this->exchangeRateService->preloadRates($currency, $dates);
&& in_array($transaction->categoryType(), [CategoryType::Income, null], true);
}
}

View File

@ -3,6 +3,7 @@
namespace App\Models;
use App\Enums\CategorySource;
use App\Enums\CategoryType;
use App\Enums\RuleOrigin;
use App\Enums\TransactionSource;
use App\Events\TransactionCreated;
@ -116,6 +117,22 @@ class Transaction extends Model
return $this->belongsTo(Category::class);
}
/**
* The type of the assigned category, resilient to phantom categories that
* are force-filled with a raw string type (e.g. the synthetic
* "uncategorized" rows the analytics controllers build).
*/
public function categoryType(): ?CategoryType
{
$type = $this->category?->getAttribute('type');
if ($type instanceof CategoryType) {
return $type;
}
return is_string($type) ? CategoryType::tryFrom($type) : null;
}
/** @return BelongsTo<AutomationRule, $this> */
public function categorizedByRule(): BelongsTo
{