refactor: consolidate duplicated financial calculations (#643)
## Summary Wave 2 structural refactor: the same financial math was copy-pasted across the dashboard and the analytics API endpoints, so it could silently diverge between screens — a real risk in a finance app. This consolidates the duplicated calculations into single sources of truth, kills a net-worth N+1, and aligns the PHP/TS rule engines and the `TransactionSource` enum. Every change is **behavior-preserving**; the numeric outputs of every endpoint are unchanged and are locked down with characterization/parity tests. Builds on merged #640 (Wave 1); no file overlap. No dependencies changed. ## Changes (per commit) - **Consolidate savings-rate math into `CashflowSummaryService`** — `savings_rate` and `net` were byte-identical inline in `DashboardController` and `Api/CashflowAnalyticsController`. Extracted to `CashflowSummaryService::summarize(income, expense)`; both controllers now spread its result (same keys, same values). - **Move income/expense-side classification onto the `Transaction` model** — the income/expense side test was reimplemented in three places (`Api/TransactionAnalysisController`, `Api/CashflowAnalyticsController`, `DashboardController`). Now `Transaction::isIncomeSide()` / `isExpenseSide()`. - **Extract duplicated `getCategorySpending` into `CategorySpendingService`** — the tree-rollup expense-spending query was duplicated verbatim between `DashboardController` and `Api/DashboardAnalyticsController`. Moved to `CategorySpendingService::forPeriod()` (drill-parent parameterized). - **Batch net-worth balance lookups to kill the per-account N+1** — `Api/DashboardAnalyticsController::calculateNetWorthAt` ran one `AccountBalance` query per account per compared period. Now uses the existing `BalanceLookup::forAccounts()` (fixed 3 queries via carry-forward seed + in-range records), reproducing the exact "latest balance <= date, else 0" semantics. - **Align server rule normalization with the client and lock it with parity fixtures** — `AutomationRuleService::normalizeRuleJson` protected only `['description','notes']` while `rule-engine.ts` also protected `creditor_name`/`debtor_name`. Aligned to the superset (structurally a no-op since those var names are already lowercase, so no matching change) and added shared PHP+TS parity fixtures so the two engines can never drift unnoticed. - **Add missing `TransactionSource` cases to the TS type** — `transaction.ts` was missing `enablebanking`/`wise`; now mirrors `App\Enums\TransactionSource`. - **Guard `sumTransactions` against unsupported category types** (reviewer fix) — replaced the income/expense ternary that silently treated any non-Income type as expense with a `match` that throws on Savings/Investment/Transfer. - **Document category eager-load expectation on `Transaction` side methods** (reviewer fix) — doc-only note to prevent a future N+1. ## Test plan New tests: - `tests/Unit/Services/CashflowSummaryServiceTest.php` — net/savings-rate/rounding/div-by-zero. - `tests/Feature/TransactionSideClassificationTest.php` — income/expense side across signs, uncategorized, and transfer/savings/investment = neither side. - `tests/Feature/DashboardAnalyticsTest.php` — new net-worth test asserts both the values (600000 / 540000) and a flat balance-query count (<= 3) regardless of account count. - `tests/Feature/RuleEngineParityTest.php` + `resources/js/lib/rule-engine-parity.test.ts` + `tests/Fixtures/rule-engine-parity.json` — one shared fixture set driving both the PHP and TS rule engines. Results (targeted, local): - `--filter=Cashflow` (exclude Browser): 57/57 passed - `--filter=DashboardAnalytics`: 41/41 passed - `--filter=AutomationRule`: 60/60 passed - `--filter=TransactionSideClassification|CashflowSummaryService|RuleEngineParity`: 21/21 passed - `bun run test rule-engine` (vitest): 13/13 passed - `vendor/bin/pint --test`: pass; `bun run lint`: 0 errors; `bun run format:check`: clean - `bun run types`: 157 errors (unchanged pre-existing baseline), 0 in touched files Note: the 6 `Cashflow*` Browser tests fail locally only on "Vite manifest not found" (no build present); they are environmental, not logic, and pass in CI. ## Reviewer findings Two read-only reviewers (architecture/quality and product/behavior) reviewed the diff. **Addressed** - Both flagged that `sumTransactions` silently treated any non-Income type as expense — added a throwing `match` guard. - Eager-load expectation documented on the `Transaction` side methods. **Verified identical** (behavior reviewer): income/expense/net/savings_rate across all three endpoints; net worth for both compared dates including no-record / all-records-after-range / same-date edge cases; category spending (uncategorized excluded, soft-deleted categories excluded, rollup/drill preserved); rule-engine normalization output; multi-currency conversion. **Deferred (documented)** - The income/expense **summation** itself is still computed three ways with differing uncategorized-transaction handling (dashboard `whereExists` + sign vs analytics `join` excluding uncategorized vs in-memory `isIncomeSide`). Unifying it would change numbers, so it is out of scope for this behavior-preserving PR — worth a dedicated follow-up. - `savings_rate` keeps its `int|float` union (int `0` when income is 0). Intentionally preserved to keep JSON output byte-identical. - `BalanceLookup`'s `empty()` guard never short-circuits a `Collection`, so a zero-account user runs 3 empty (harmless) queries. Left untouched — it lives in a shared, unchanged service and only affects a no-account edge case. Do not merge before Wave 1 (#640) is in main.
This commit is contained in:
parent
6ff7edf193
commit
26875bbfff
|
|
@ -8,6 +8,7 @@ use App\Http\Controllers\Api\Concerns\ConvertsTransactionCurrency;
|
|||
use App\Http\Controllers\Controller;
|
||||
use App\Models\Category;
|
||||
use App\Models\Transaction;
|
||||
use App\Services\CashflowSummaryService;
|
||||
use App\Services\CategoryTree;
|
||||
use App\Services\ExchangeRateService;
|
||||
use App\Services\PeriodComparator;
|
||||
|
|
@ -15,6 +16,7 @@ use Carbon\Carbon;
|
|||
use Illuminate\Http\JsonResponse;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Support\Collection;
|
||||
use InvalidArgumentException;
|
||||
|
||||
class CashflowAnalyticsController extends Controller
|
||||
{
|
||||
|
|
@ -208,14 +210,8 @@ class CashflowAnalyticsController extends Controller
|
|||
$savings = $this->sumOutflowTransactions($transactions, $userCurrency, CategoryType::Savings);
|
||||
$investments = $this->sumOutflowTransactions($transactions, $userCurrency, CategoryType::Investment);
|
||||
|
||||
$net = $income - $expense;
|
||||
$savingsRate = $income > 0 ? round((($income - $expense) / $income) * 100, 1) : 0;
|
||||
|
||||
return [
|
||||
'income' => $income,
|
||||
'expense' => $expense,
|
||||
'net' => $net,
|
||||
'savings_rate' => $savingsRate,
|
||||
...CashflowSummaryService::summarize($income, $expense),
|
||||
'savings' => $savings,
|
||||
'investments' => $investments,
|
||||
];
|
||||
|
|
@ -223,15 +219,14 @@ class CashflowAnalyticsController extends Controller
|
|||
|
||||
private function sumTransactions(Collection $transactions, string $userCurrency, CategoryType $type): int
|
||||
{
|
||||
return $transactions
|
||||
->filter(function (Transaction $transaction) use ($type): bool {
|
||||
if ($transaction->categoryType() === $type) {
|
||||
return true;
|
||||
}
|
||||
$onSide = match ($type) {
|
||||
CategoryType::Income => fn (Transaction $transaction): bool => $transaction->isIncomeSide(),
|
||||
CategoryType::Expense => fn (Transaction $transaction): bool => $transaction->isExpenseSide(),
|
||||
default => throw new InvalidArgumentException("sumTransactions only supports Income and Expense, got {$type->value}."),
|
||||
};
|
||||
|
||||
return $transaction->category_id === null
|
||||
&& $this->matchesSign($transaction->amount, $type === CategoryType::Income ? '>' : '<');
|
||||
})
|
||||
return $transactions
|
||||
->filter($onSide)
|
||||
->sum(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $userCurrency));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -6,19 +6,16 @@ use App\Enums\AccountType;
|
|||
use App\Enums\CategoryType;
|
||||
use App\Http\Controllers\Controller;
|
||||
use App\Models\Account;
|
||||
use App\Models\AccountBalance;
|
||||
use App\Models\Category;
|
||||
use App\Models\Transaction;
|
||||
use App\Services\AccountMetricsService;
|
||||
use App\Services\BalanceLookup;
|
||||
use App\Services\CategoryTree;
|
||||
use App\Services\CategorySpendingService;
|
||||
use App\Services\ExchangeRateService;
|
||||
use App\Services\LoanAmortizationService;
|
||||
use App\Services\PeriodComparator;
|
||||
use Carbon\Carbon;
|
||||
use Illuminate\Database\Eloquent\Collection;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Support\Collection;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
class DashboardAnalyticsController extends Controller
|
||||
{
|
||||
|
|
@ -26,7 +23,7 @@ class DashboardAnalyticsController extends Controller
|
|||
private ExchangeRateService $exchangeRateService,
|
||||
private AccountMetricsService $accountMetricsService,
|
||||
private LoanAmortizationService $loanAmortizationService,
|
||||
private CategoryTree $tree,
|
||||
private CategorySpendingService $categorySpendingService,
|
||||
) {}
|
||||
|
||||
public function netWorth(Request $request)
|
||||
|
|
@ -41,9 +38,21 @@ class DashboardAnalyticsController extends Controller
|
|||
|
||||
$userCurrency = $request->user()->currency_code;
|
||||
|
||||
$accounts = Account::query()
|
||||
->where('user_id', $request->user()->id)
|
||||
->get();
|
||||
|
||||
// Load every account's balance history for the compared range in a
|
||||
// fixed number of queries instead of one balance lookup per account.
|
||||
$lookup = BalanceLookup::forAccounts(
|
||||
$accounts->pluck('id'),
|
||||
$previousPeriod->to,
|
||||
$period->to,
|
||||
);
|
||||
|
||||
return response()->json([
|
||||
'current' => $this->calculateNetWorthAt($period->to, $userCurrency),
|
||||
'previous' => $this->calculateNetWorthAt($previousPeriod->to, $userCurrency),
|
||||
'current' => $this->calculateNetWorthAt($accounts, $lookup, $period->to, $userCurrency),
|
||||
'previous' => $this->calculateNetWorthAt($accounts, $lookup, $previousPeriod->to, $userCurrency),
|
||||
'currency_code' => $userCurrency,
|
||||
]);
|
||||
}
|
||||
|
|
@ -439,8 +448,8 @@ class DashboardAnalyticsController extends Controller
|
|||
$previousPeriod = $period->previous();
|
||||
$drillParentId = $validated['parent'] ?? null;
|
||||
|
||||
$currentSpending = $this->getCategorySpending($request->user()->id, $period->from, $period->to, $drillParentId);
|
||||
$previousSpending = $this->getCategorySpending($request->user()->id, $previousPeriod->from, $previousPeriod->to, $drillParentId);
|
||||
$currentSpending = $this->categorySpendingService->forPeriod($request->user()->id, $period->from, $period->to, $drillParentId);
|
||||
$previousSpending = $this->categorySpendingService->forPeriod($request->user()->id, $previousPeriod->from, $previousPeriod->to, $drillParentId);
|
||||
|
||||
$totalAmount = $currentSpending->sum('amount');
|
||||
|
||||
|
|
@ -466,50 +475,14 @@ class DashboardAnalyticsController extends Controller
|
|||
}
|
||||
|
||||
/**
|
||||
* Expense spending rolled up the category tree.
|
||||
*
|
||||
* Without a drill target, child amounts fold into their top-level ancestor
|
||||
* so only parents are listed. With one, the parent's children become the
|
||||
* rows (plus a direct node for transactions sitting on the parent itself).
|
||||
* @param Collection<int, Account> $accounts
|
||||
*/
|
||||
private function getCategorySpending(string $userId, Carbon $from, Carbon $to, ?string $drillParentId = null): Collection
|
||||
private function calculateNetWorthAt(Collection $accounts, BalanceLookup $lookup, Carbon $date, string $userCurrency): int
|
||||
{
|
||||
$perCategory = Transaction::query()
|
||||
->where('transactions.user_id', $userId)
|
||||
->whereBetween('transactions.transaction_date', [$from, $to])
|
||||
->join('categories', function ($join) {
|
||||
$join->on('transactions.category_id', '=', 'categories.id')
|
||||
->where('categories.type', '=', CategoryType::Expense)
|
||||
->whereNull('categories.deleted_at');
|
||||
})
|
||||
->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount'))
|
||||
->groupBy('transactions.category_id')
|
||||
->get()
|
||||
->map(fn ($item): array => [
|
||||
'category_id' => $item->category_id,
|
||||
'category' => null,
|
||||
'amount' => (int) -$item->total_amount,
|
||||
])
|
||||
->values()
|
||||
->all();
|
||||
|
||||
return collect($this->tree->rollUp($perCategory, $userId, $drillParentId))
|
||||
->filter(fn (array $item): bool => $item['amount'] > 0)
|
||||
->values();
|
||||
}
|
||||
|
||||
private function calculateNetWorthAt(Carbon $date, string $userCurrency): int
|
||||
{
|
||||
$accounts = Account::where('user_id', request()->user()->id)->get();
|
||||
|
||||
$total = 0;
|
||||
|
||||
foreach ($accounts as $account) {
|
||||
$balance = AccountBalance::query()
|
||||
->where('account_id', $account->id)
|
||||
->where('balance_date', '<=', $date->toDateString())
|
||||
->orderBy('balance_date', 'desc')
|
||||
->value('balance') ?? 0;
|
||||
$balance = $lookup->getBalanceAt($account->id, $date);
|
||||
|
||||
$convertedBalance = $this->exchangeRateService->convert(
|
||||
$account->currency_code,
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@
|
|||
|
||||
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;
|
||||
|
|
@ -95,9 +94,9 @@ class TransactionAnalysisController extends Controller
|
|||
$expense = 0;
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
if ($this->isIncomeSide($transaction)) {
|
||||
if ($transaction->isIncomeSide()) {
|
||||
$income += $this->convertTransactionAmount($transaction, $currency);
|
||||
} elseif ($this->isExpenseSide($transaction)) {
|
||||
} elseif ($transaction->isExpenseSide()) {
|
||||
$expense += $this->convertTransactionAmount($transaction, $currency);
|
||||
}
|
||||
}
|
||||
|
|
@ -128,7 +127,7 @@ class TransactionAnalysisController extends Controller
|
|||
private function categoryBreakdown(Collection $transactions, string $currency, string $userId): Collection
|
||||
{
|
||||
$expenses = $transactions->filter(
|
||||
fn (Transaction $transaction): bool => $this->isExpenseSide($transaction),
|
||||
fn (Transaction $transaction): bool => $transaction->isExpenseSide(),
|
||||
);
|
||||
|
||||
$grouped = $expenses
|
||||
|
|
@ -187,7 +186,7 @@ class TransactionAnalysisController extends Controller
|
|||
$totals = [];
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
if (! $this->isExpenseSide($transaction)) {
|
||||
if (! $transaction->isExpenseSide()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -215,7 +214,7 @@ class TransactionAnalysisController extends Controller
|
|||
$totals = [];
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
if (! $this->isExpenseSide($transaction)) {
|
||||
if (! $transaction->isExpenseSide()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -246,7 +245,7 @@ class TransactionAnalysisController extends Controller
|
|||
$totals = [];
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
if (! $this->isExpenseSide($transaction)) {
|
||||
if (! $transaction->isExpenseSide()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -279,7 +278,7 @@ class TransactionAnalysisController extends Controller
|
|||
private function largestExpenses(Collection $transactions, string $currency): array
|
||||
{
|
||||
return $transactions
|
||||
->filter(fn (Transaction $transaction): bool => $this->isExpenseSide($transaction) && $transaction->amount < 0)
|
||||
->filter(fn (Transaction $transaction): bool => $transaction->isExpenseSide() && $transaction->amount < 0)
|
||||
->sortBy(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $currency))
|
||||
->take(self::LARGEST_EXPENSES_LIMIT)
|
||||
->map(fn (Transaction $transaction): array => [
|
||||
|
|
@ -332,9 +331,9 @@ class TransactionAnalysisController extends Controller
|
|||
$key = $transaction->transaction_date->format($keyFormat);
|
||||
$buckets[$key] ??= ['income' => 0, 'expense' => 0];
|
||||
|
||||
if ($this->isIncomeSide($transaction)) {
|
||||
if ($transaction->isIncomeSide()) {
|
||||
$buckets[$key]['income'] += $this->convertTransactionAmount($transaction, $currency);
|
||||
} elseif ($this->isExpenseSide($transaction)) {
|
||||
} elseif ($transaction->isExpenseSide()) {
|
||||
$buckets[$key]['expense'] -= $this->convertTransactionAmount($transaction, $currency);
|
||||
}
|
||||
}
|
||||
|
|
@ -377,27 +376,4 @@ class TransactionAnalysisController extends Controller
|
|||
|
||||
return (int) $dates->min()->diffInDays($dates->max()) + 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a transaction belongs to the expense side: anything booked to an
|
||||
* expense category (a refund there nets back out), plus uncategorized
|
||||
* outflows. Transfers, savings and investments are internal movements, so
|
||||
* they sit on neither side — matching the cashflow screen.
|
||||
*/
|
||||
private function isExpenseSide(Transaction $transaction): bool
|
||||
{
|
||||
return $transaction->categoryType() === CategoryType::Expense
|
||||
|| ($transaction->category_id === null && $transaction->amount < 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a transaction belongs to the income side: anything booked to an
|
||||
* income category (a reversal there nets back out), plus uncategorized
|
||||
* inflows. Internal movements are excluded for the same reason as expenses.
|
||||
*/
|
||||
private function isIncomeSide(Transaction $transaction): bool
|
||||
{
|
||||
return $transaction->categoryType() === CategoryType::Income
|
||||
|| ($transaction->category_id === null && $transaction->amount > 0);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,11 +6,11 @@ use App\Enums\CategoryType;
|
|||
use App\Models\Account;
|
||||
use App\Models\Transaction;
|
||||
use App\Services\AccountMetricsService;
|
||||
use App\Services\CategoryTree;
|
||||
use App\Services\CashflowSummaryService;
|
||||
use App\Services\CategorySpendingService;
|
||||
use App\Services\PeriodComparator;
|
||||
use Carbon\Carbon;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Support\Collection;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Inertia\Inertia;
|
||||
use Inertia\Response;
|
||||
|
|
@ -19,7 +19,7 @@ class DashboardController extends Controller
|
|||
{
|
||||
public function __construct(
|
||||
private AccountMetricsService $accountMetricsService,
|
||||
private CategoryTree $tree,
|
||||
private CategorySpendingService $categorySpendingService,
|
||||
) {}
|
||||
|
||||
public function __invoke(Request $request): Response
|
||||
|
|
@ -59,8 +59,8 @@ class DashboardController extends Controller
|
|||
$period = new PeriodComparator($from, $to);
|
||||
$previousPeriod = $period->previous();
|
||||
|
||||
$currentSpending = $this->getCategorySpending($user->id, $period->from, $period->to);
|
||||
$previousSpending = $this->getCategorySpending($user->id, $previousPeriod->from, $previousPeriod->to);
|
||||
$currentSpending = $this->categorySpendingService->forPeriod($user->id, $period->from, $period->to);
|
||||
$previousSpending = $this->categorySpendingService->forPeriod($user->id, $previousPeriod->from, $previousPeriod->to);
|
||||
|
||||
$totalAmount = $currentSpending->sum('amount');
|
||||
|
||||
|
|
@ -100,50 +100,12 @@ class DashboardController extends Controller
|
|||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Spending per top-level category: child category amounts roll up into
|
||||
* their root ancestor so the dashboard only lists parents.
|
||||
*/
|
||||
private function getCategorySpending(string $userId, Carbon $from, Carbon $to): Collection
|
||||
{
|
||||
$perCategory = Transaction::query()
|
||||
->where('transactions.user_id', $userId)
|
||||
->whereBetween('transactions.transaction_date', [$from, $to])
|
||||
->join('categories', function ($join) {
|
||||
$join->on('transactions.category_id', '=', 'categories.id')
|
||||
->where('categories.type', '=', CategoryType::Expense)
|
||||
->whereNull('categories.deleted_at');
|
||||
})
|
||||
->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount'))
|
||||
->groupBy('transactions.category_id')
|
||||
->get()
|
||||
->map(fn ($item): array => [
|
||||
'category_id' => $item->category_id,
|
||||
'category' => null,
|
||||
'amount' => (int) -$item->total_amount,
|
||||
])
|
||||
->values()
|
||||
->all();
|
||||
|
||||
return collect($this->tree->rollUp($perCategory, $userId, null))
|
||||
->filter(fn (array $item): bool => $item['amount'] > 0)
|
||||
->values();
|
||||
}
|
||||
|
||||
private function calculateCashflowSummary(string $userId, Carbon $from, Carbon $to): array
|
||||
{
|
||||
$income = max(0, $this->getTransactionSum($userId, $from, $to, CategoryType::Income));
|
||||
$expense = max(0, -$this->getTransactionSum($userId, $from, $to, CategoryType::Expense));
|
||||
|
||||
$net = $income - $expense;
|
||||
$savingsRate = $income > 0 ? round((($income - $expense) / $income) * 100, 1) : 0;
|
||||
|
||||
return [
|
||||
'income' => $income,
|
||||
'expense' => $expense,
|
||||
'net' => $net,
|
||||
'savings_rate' => $savingsRate,
|
||||
];
|
||||
return CashflowSummaryService::summarize($income, $expense);
|
||||
}
|
||||
|
||||
private function getTransactionSum(string $userId, Carbon $from, Carbon $to, CategoryType $type): int
|
||||
|
|
|
|||
|
|
@ -133,6 +133,34 @@ class Transaction extends Model
|
|||
return is_string($type) ? CategoryType::tryFrom($type) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this transaction sits on the income side of a cashflow split:
|
||||
* booked to an income category (a reversal there nets back out) or an
|
||||
* uncategorized inflow. Internal movements (transfer, savings, investment)
|
||||
* belong to neither side.
|
||||
*
|
||||
* Reads categoryType(), so callers should eager-load the category relation
|
||||
* when classifying a collection to avoid an N+1.
|
||||
*/
|
||||
public function isIncomeSide(): bool
|
||||
{
|
||||
return $this->categoryType() === CategoryType::Income
|
||||
|| ($this->category_id === null && $this->amount > 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this transaction sits on the expense side: booked to an expense
|
||||
* category (a refund there nets back out) or an uncategorized outflow.
|
||||
*
|
||||
* Reads categoryType(), so callers should eager-load the category relation
|
||||
* when classifying a collection to avoid an N+1.
|
||||
*/
|
||||
public function isExpenseSide(): bool
|
||||
{
|
||||
return $this->categoryType() === CategoryType::Expense
|
||||
|| ($this->category_id === null && $this->amount < 0);
|
||||
}
|
||||
|
||||
/** @return BelongsTo<AutomationRule, $this> */
|
||||
public function categorizedByRule(): BelongsTo
|
||||
{
|
||||
|
|
|
|||
|
|
@ -373,7 +373,7 @@ class AutomationRuleService
|
|||
return mb_strtolower($item);
|
||||
}
|
||||
|
||||
if (is_array($item) && isset($item['var']) && in_array($item['var'], ['description', 'notes'])) {
|
||||
if (is_array($item) && isset($item['var']) && in_array($item['var'], ['description', 'notes', 'creditor_name', 'debtor_name'], true)) {
|
||||
return $item;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,24 @@
|
|||
<?php
|
||||
|
||||
namespace App\Services;
|
||||
|
||||
class CashflowSummaryService
|
||||
{
|
||||
/**
|
||||
* Derive the shared cashflow summary from already-clamped income and
|
||||
* expense totals (both non-negative, in minor units). Kept in one place so
|
||||
* the net and savings-rate math can never drift between the dashboard and
|
||||
* the cashflow analytics endpoints.
|
||||
*
|
||||
* @return array{income: int, expense: int, net: int, savings_rate: float|int}
|
||||
*/
|
||||
public static function summarize(int $income, int $expense): array
|
||||
{
|
||||
return [
|
||||
'income' => $income,
|
||||
'expense' => $expense,
|
||||
'net' => $income - $expense,
|
||||
'savings_rate' => $income > 0 ? round((($income - $expense) / $income) * 100, 1) : 0,
|
||||
];
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,48 @@
|
|||
<?php
|
||||
|
||||
namespace App\Services;
|
||||
|
||||
use App\Enums\CategoryType;
|
||||
use App\Models\Transaction;
|
||||
use Carbon\Carbon;
|
||||
use Illuminate\Support\Collection;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
class CategorySpendingService
|
||||
{
|
||||
public function __construct(private CategoryTree $tree) {}
|
||||
|
||||
/**
|
||||
* Expense spending rolled up the category tree.
|
||||
*
|
||||
* Without a drill target, child category amounts fold into their root
|
||||
* ancestor so only parents are listed. With one, the parent's children
|
||||
* become the rows (plus a direct node for transactions sitting on the
|
||||
* parent itself). Soft-deleted categories are excluded.
|
||||
*/
|
||||
public function forPeriod(string $userId, Carbon $from, Carbon $to, ?string $drillParentId = null): Collection
|
||||
{
|
||||
$perCategory = Transaction::query()
|
||||
->where('transactions.user_id', $userId)
|
||||
->whereBetween('transactions.transaction_date', [$from, $to])
|
||||
->join('categories', function ($join) {
|
||||
$join->on('transactions.category_id', '=', 'categories.id')
|
||||
->where('categories.type', '=', CategoryType::Expense)
|
||||
->whereNull('categories.deleted_at');
|
||||
})
|
||||
->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount'))
|
||||
->groupBy('transactions.category_id')
|
||||
->get()
|
||||
->map(fn ($item): array => [
|
||||
'category_id' => $item->category_id,
|
||||
'category' => null,
|
||||
'amount' => (int) -$item->total_amount,
|
||||
])
|
||||
->values()
|
||||
->all();
|
||||
|
||||
return collect($this->tree->rollUp($perCategory, $userId, $drillParentId))
|
||||
->filter(fn (array $item): bool => $item['amount'] > 0)
|
||||
->values();
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,80 @@
|
|||
import {
|
||||
evaluateRulesForNewTransaction,
|
||||
type NewTransactionData,
|
||||
} from '@/lib/rule-engine';
|
||||
import type { AutomationRule } from '@/types/automation-rule';
|
||||
import type { UUID } from '@/types/uuid';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
// The engine logs verbose evaluation traces through consoleDebug, which reads
|
||||
// localStorage; stub it so the parity cases exercise pure rule evaluation.
|
||||
vi.mock('@/lib/debug', () => ({ consoleDebug: () => {} }));
|
||||
|
||||
interface ParityFixture {
|
||||
name: string;
|
||||
rule: Record<string, unknown>;
|
||||
transaction: {
|
||||
description: string;
|
||||
amount: number;
|
||||
creditor_name: string | null;
|
||||
debtor_name: string | null;
|
||||
notes: string | null;
|
||||
};
|
||||
expected: boolean;
|
||||
}
|
||||
|
||||
// The same fixtures drive the PHP RuleEngineParityTest, so both engines must
|
||||
// agree on every case here — locking client and server rule evaluation together.
|
||||
const fixtures = JSON.parse(
|
||||
readFileSync(
|
||||
resolve(process.cwd(), 'tests/Fixtures/rule-engine-parity.json'),
|
||||
'utf-8',
|
||||
),
|
||||
) as ParityFixture[];
|
||||
|
||||
function makeRule(rulesJson: Record<string, unknown>): AutomationRule {
|
||||
return {
|
||||
id: 'rule-1' as UUID,
|
||||
user_id: 'user-1' as UUID,
|
||||
title: 'Parity rule',
|
||||
priority: 1,
|
||||
origin: 'user',
|
||||
rules_json: rulesJson,
|
||||
action_category_id: 'cat-1' as UUID,
|
||||
action_note: null,
|
||||
action_note_iv: null,
|
||||
labels: [],
|
||||
created_at: '2026-01-01T00:00:00.000Z',
|
||||
updated_at: '2026-01-01T00:00:00.000Z',
|
||||
deleted_at: null,
|
||||
};
|
||||
}
|
||||
|
||||
describe('rule engine PHP/TS parity', () => {
|
||||
it.each(fixtures)('$name', async (fixture) => {
|
||||
const transactionData: NewTransactionData = {
|
||||
description: fixture.transaction.description,
|
||||
// Fixture amounts are already in dollars, the unit the client engine
|
||||
// compares directly for a new transaction.
|
||||
amount: fixture.transaction.amount,
|
||||
transaction_date: '2026-01-01',
|
||||
account_id: 'acc-1' as UUID,
|
||||
notes: fixture.transaction.notes ?? undefined,
|
||||
creditor_name: fixture.transaction.creditor_name,
|
||||
debtor_name: fixture.transaction.debtor_name,
|
||||
};
|
||||
|
||||
const result = await evaluateRulesForNewTransaction(
|
||||
transactionData,
|
||||
[makeRule(fixture.rule)],
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
null,
|
||||
);
|
||||
|
||||
expect(result !== null).toBe(fixture.expected);
|
||||
});
|
||||
});
|
||||
|
|
@ -3,7 +3,12 @@ import { type Category } from './category';
|
|||
import { type Label } from './label';
|
||||
import { UUID } from './uuid';
|
||||
|
||||
export type TransactionSource = 'manually_created' | 'imported';
|
||||
// Mirrors App\Enums\TransactionSource (PHP). Keep both in sync.
|
||||
export type TransactionSource =
|
||||
| 'manually_created'
|
||||
| 'imported'
|
||||
| 'enablebanking'
|
||||
| 'wise';
|
||||
|
||||
export type CategorySource = 'manual' | 'rule' | 'ai' | 'bank';
|
||||
|
||||
|
|
|
|||
|
|
@ -71,6 +71,52 @@ test('net worth calculates assets minus liabilities', function () {
|
|||
]);
|
||||
});
|
||||
|
||||
test('net worth queries account balances a fixed number of times regardless of account count', function () {
|
||||
// Six accounts, each with a current and a prior-period balance. The old
|
||||
// implementation ran one balance query per account per compared period,
|
||||
// so this would scale with the account count; BalanceLookup keeps it flat.
|
||||
for ($i = 0; $i < 6; $i++) {
|
||||
$account = Account::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'type' => AccountType::Checking,
|
||||
'currency_code' => 'USD',
|
||||
]);
|
||||
|
||||
AccountBalance::factory()->create([
|
||||
'account_id' => $account->id,
|
||||
'balance_date' => now(),
|
||||
'balance' => 100000,
|
||||
]);
|
||||
AccountBalance::factory()->create([
|
||||
'account_id' => $account->id,
|
||||
'balance_date' => now()->subDays(30),
|
||||
'balance' => 90000,
|
||||
]);
|
||||
}
|
||||
|
||||
$balanceQueries = 0;
|
||||
DB::listen(function ($query) use (&$balanceQueries): void {
|
||||
if (str_starts_with(strtolower($query->sql), 'select') && str_contains($query->sql, 'account_balances')) {
|
||||
$balanceQueries++;
|
||||
}
|
||||
});
|
||||
|
||||
$response = $this->getJson('/api/dashboard/net-worth?'.http_build_query([
|
||||
'from' => now()->subDays(29)->toDateString(),
|
||||
'to' => now()->toDateString(),
|
||||
]));
|
||||
|
||||
$response->assertOk()
|
||||
->assertJson([
|
||||
'current' => 600000, // 6 x 100000
|
||||
'previous' => 540000, // 6 x 90000
|
||||
'currency_code' => 'USD',
|
||||
]);
|
||||
|
||||
// BalanceLookup issues at most three balance queries for the whole request.
|
||||
expect($balanceQueries)->toBeLessThanOrEqual(3);
|
||||
});
|
||||
|
||||
test('net worth response includes currency_code', function () {
|
||||
$response = $this->getJson('/api/dashboard/net-worth?'.http_build_query([
|
||||
'from' => now()->subDays(29)->toDateString(),
|
||||
|
|
|
|||
|
|
@ -0,0 +1,59 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Account;
|
||||
use App\Models\AutomationRule;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
use App\Services\AutomationRuleService;
|
||||
|
||||
beforeEach(function () {
|
||||
$this->user = User::factory()->create();
|
||||
$this->account = Account::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'encrypted' => false,
|
||||
]);
|
||||
$this->service = app(AutomationRuleService::class);
|
||||
});
|
||||
|
||||
/**
|
||||
* The same fixtures drive the TS rule-engine-parity.test.ts, so both engines
|
||||
* must agree on every case here — locking client and server evaluation together.
|
||||
*
|
||||
* @return array<string, array{0: array<string, mixed>}>
|
||||
*/
|
||||
function ruleEngineParityFixtures(): array
|
||||
{
|
||||
$fixtures = json_decode(
|
||||
file_get_contents(__DIR__.'/../Fixtures/rule-engine-parity.json'),
|
||||
true,
|
||||
512,
|
||||
JSON_THROW_ON_ERROR,
|
||||
);
|
||||
|
||||
return collect($fixtures)
|
||||
->mapWithKeys(fn (array $fixture): array => [$fixture['name'] => [$fixture]])
|
||||
->all();
|
||||
}
|
||||
|
||||
it('evaluates the shared parity fixtures the same way the client engine does', function (array $fixture) {
|
||||
$rule = AutomationRule::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'priority' => 1,
|
||||
'rules_json' => $fixture['rule'],
|
||||
'action_category_id' => null,
|
||||
]);
|
||||
|
||||
$transaction = Transaction::factory()->enableBanking()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'account_id' => $this->account->id,
|
||||
'description' => $fixture['transaction']['description'],
|
||||
'creditor_name' => $fixture['transaction']['creditor_name'],
|
||||
'debtor_name' => $fixture['transaction']['debtor_name'],
|
||||
'notes' => $fixture['transaction']['notes'],
|
||||
// Fixture amounts are in dollars, the unit the rule engine compares
|
||||
// after dividing the stored minor units by 100.
|
||||
'amount' => (int) round($fixture['transaction']['amount'] * 100),
|
||||
]);
|
||||
|
||||
expect($this->service->ruleMatches($rule, $transaction))->toBe($fixture['expected']);
|
||||
})->with(ruleEngineParityFixtures());
|
||||
|
|
@ -0,0 +1,64 @@
|
|||
<?php
|
||||
|
||||
use App\Enums\CategoryType;
|
||||
use App\Models\Account;
|
||||
use App\Models\Category;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
|
||||
beforeEach(function () {
|
||||
$this->user = User::factory()->create();
|
||||
$this->account = Account::factory()->create(['user_id' => $this->user->id]);
|
||||
});
|
||||
|
||||
function makeSideTransaction(User $user, Account $account, ?Category $category, int $amount): Transaction
|
||||
{
|
||||
return Transaction::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'account_id' => $account->id,
|
||||
'category_id' => $category?->id,
|
||||
'amount' => $amount,
|
||||
])->load('category');
|
||||
}
|
||||
|
||||
it('classifies an income-category transaction as income side regardless of sign', function () {
|
||||
$income = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Income]);
|
||||
|
||||
$positive = makeSideTransaction($this->user, $this->account, $income, 10000);
|
||||
$reversal = makeSideTransaction($this->user, $this->account, $income, -2000);
|
||||
|
||||
expect($positive->isIncomeSide())->toBeTrue()
|
||||
->and($positive->isExpenseSide())->toBeFalse()
|
||||
->and($reversal->isIncomeSide())->toBeTrue();
|
||||
});
|
||||
|
||||
it('classifies an expense-category transaction as expense side regardless of sign', function () {
|
||||
$expense = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Expense]);
|
||||
|
||||
$spend = makeSideTransaction($this->user, $this->account, $expense, -5000);
|
||||
$refund = makeSideTransaction($this->user, $this->account, $expense, 1500);
|
||||
|
||||
expect($spend->isExpenseSide())->toBeTrue()
|
||||
->and($spend->isIncomeSide())->toBeFalse()
|
||||
->and($refund->isExpenseSide())->toBeTrue();
|
||||
});
|
||||
|
||||
it('classifies uncategorized inflows as income and outflows as expense', function () {
|
||||
$inflow = makeSideTransaction($this->user, $this->account, null, 7000);
|
||||
$outflow = makeSideTransaction($this->user, $this->account, null, -3000);
|
||||
|
||||
expect($inflow->isIncomeSide())->toBeTrue()
|
||||
->and($inflow->isExpenseSide())->toBeFalse()
|
||||
->and($outflow->isExpenseSide())->toBeTrue()
|
||||
->and($outflow->isIncomeSide())->toBeFalse();
|
||||
});
|
||||
|
||||
it('treats transfer, savings and investment categories as neither side', function () {
|
||||
foreach ([CategoryType::Transfer, CategoryType::Savings, CategoryType::Investment] as $type) {
|
||||
$category = Category::factory()->create(['user_id' => $this->user->id, 'type' => $type]);
|
||||
$transaction = makeSideTransaction($this->user, $this->account, $category, -5000);
|
||||
|
||||
expect($transaction->isIncomeSide())->toBeFalse()
|
||||
->and($transaction->isExpenseSide())->toBeFalse();
|
||||
}
|
||||
});
|
||||
|
|
@ -0,0 +1,80 @@
|
|||
[
|
||||
{
|
||||
"name": "in operator matches description",
|
||||
"rule": { "in": ["grocery", { "var": "description" }] },
|
||||
"transaction": { "description": "Grocery Store Purchase", "amount": -50, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "in operator does not match unrelated description",
|
||||
"rule": { "in": ["grocery", { "var": "description" }] },
|
||||
"transaction": { "description": "Coffee Shop Downtown", "amount": -50, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": false
|
||||
},
|
||||
{
|
||||
"name": "in operator matches creditor_name",
|
||||
"rule": { "in": ["amazon", { "var": "creditor_name" }] },
|
||||
"transaction": { "description": "Card payment", "amount": -50, "creditor_name": "Amazon EU", "debtor_name": null, "notes": null },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "in operator does not match unrelated creditor_name",
|
||||
"rule": { "in": ["ikea", { "var": "creditor_name" }] },
|
||||
"transaction": { "description": "Card payment", "amount": -50, "creditor_name": "Amazon EU", "debtor_name": null, "notes": null },
|
||||
"expected": false
|
||||
},
|
||||
{
|
||||
"name": "in operator matches debtor_name",
|
||||
"rule": { "in": ["payroll", { "var": "debtor_name" }] },
|
||||
"transaction": { "description": "Incoming transfer", "amount": 5000, "creditor_name": null, "debtor_name": "Payroll GmbH", "notes": null },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "equals operator matches exact lowercased description",
|
||||
"rule": { "==": [{ "var": "description" }, "salary payment"] },
|
||||
"transaction": { "description": "Salary Payment", "amount": 1000, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "rule value is matched case-insensitively",
|
||||
"rule": { "in": ["GROCERY", { "var": "description" }] },
|
||||
"transaction": { "description": "grocery store", "amount": -30, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "description whitespace is normalized before matching",
|
||||
"rule": { "in": ["grocery store", { "var": "description" }] },
|
||||
"transaction": { "description": " Grocery Store Purchase ", "amount": -30, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "in operator matches notes",
|
||||
"rule": { "in": ["reimburse", { "var": "notes" }] },
|
||||
"transaction": { "description": "Bank transfer", "amount": 200, "creditor_name": null, "debtor_name": null, "notes": "Please Reimburse Me" },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "greater than amount comparison uses dollars",
|
||||
"rule": { ">": [{ "var": "amount" }, 50] },
|
||||
"transaction": { "description": "Salary", "amount": 100, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "greater than amount comparison fails below threshold",
|
||||
"rule": { ">": [{ "var": "amount" }, 200] },
|
||||
"transaction": { "description": "Small purchase", "amount": 50, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": false
|
||||
},
|
||||
{
|
||||
"name": "compound and rule matches when both conditions hold",
|
||||
"rule": { "and": [{ "in": ["grocery", { "var": "description" }] }, { "<": [{ "var": "amount" }, 0] }] },
|
||||
"transaction": { "description": "Grocery Store", "amount": -50, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": true
|
||||
},
|
||||
{
|
||||
"name": "compound and rule fails when one condition fails",
|
||||
"rule": { "and": [{ "in": ["grocery", { "var": "description" }] }, { ">": [{ "var": "amount" }, 0] }] },
|
||||
"transaction": { "description": "Grocery Store", "amount": -50, "creditor_name": null, "debtor_name": null, "notes": null },
|
||||
"expected": false
|
||||
}
|
||||
]
|
||||
|
|
@ -0,0 +1,33 @@
|
|||
<?php
|
||||
|
||||
use App\Services\CashflowSummaryService;
|
||||
|
||||
it('derives net and savings rate from income and expense', function () {
|
||||
expect(CashflowSummaryService::summarize(100000, 40000))->toBe([
|
||||
'income' => 100000,
|
||||
'expense' => 40000,
|
||||
'net' => 60000,
|
||||
'savings_rate' => 60.0,
|
||||
]);
|
||||
});
|
||||
|
||||
it('returns a negative net when expense exceeds income', function () {
|
||||
expect(CashflowSummaryService::summarize(0, 10000))->toBe([
|
||||
'income' => 0,
|
||||
'expense' => 10000,
|
||||
'net' => -10000,
|
||||
'savings_rate' => 0,
|
||||
]);
|
||||
});
|
||||
|
||||
it('avoids division by zero when income is zero', function () {
|
||||
$summary = CashflowSummaryService::summarize(0, 0);
|
||||
|
||||
expect($summary['savings_rate'])->toBe(0)
|
||||
->and($summary['net'])->toBe(0);
|
||||
});
|
||||
|
||||
it('rounds the savings rate to one decimal place', function () {
|
||||
// (100000 - 33333) / 100000 * 100 = 66.667 -> 66.7
|
||||
expect(CashflowSummaryService::summarize(100000, 33333)['savings_rate'])->toBe(66.7);
|
||||
});
|
||||
Loading…
Reference in New Issue