From fad189e23b0ff1c00f473360c12e3251ffdda9ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vi=CC=81ctor=20Falco=CC=81n?= Date: Mon, 29 Jun 2026 20:37:00 +0200 Subject: [PATCH] fix(analysis): exclude transfer, savings and investment categories from income and expense The analysis screen classified every transaction by amount sign alone, so outflows and inflows booked to transfer, savings or investment categories leaked into the expense and income totals (and every breakdown). Cashflow already keys off the category type; analysis now does the same: only expense categories (or uncategorized outflows) count as expenses and only income categories (or uncategorized inflows) count as income. --- .../Api/TransactionAnalysisController.php | 75 ++++++++++----- tests/Feature/TransactionAnalysisTest.php | 96 +++++++++++++++++++ 2 files changed, 149 insertions(+), 22 deletions(-) diff --git a/app/Http/Controllers/Api/TransactionAnalysisController.php b/app/Http/Controllers/Api/TransactionAnalysisController.php index 2f46310a..61c20702 100644 --- a/app/Http/Controllers/Api/TransactionAnalysisController.php +++ b/app/Http/Controllers/Api/TransactionAnalysisController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Enums\CategoryType; use App\Http\Controllers\Controller; use App\Http\Requests\IndexTransactionRequest; use App\Models\Label; @@ -91,12 +92,10 @@ class TransactionAnalysisController extends Controller $expense = 0; foreach ($transactions as $transaction) { - $amount = $this->convertTransactionAmount($transaction, $currency); - - if ($amount > 0) { - $income += $amount; - } else { - $expense += abs($amount); + if ($this->isIncome($transaction)) { + $income += $this->convertTransactionAmount($transaction, $currency); + } elseif ($this->isExpense($transaction)) { + $expense += abs($this->convertTransactionAmount($transaction, $currency)); } } @@ -120,7 +119,7 @@ class TransactionAnalysisController extends Controller private function categoryBreakdown(Collection $transactions, string $currency, string $userId): Collection { $expenses = $transactions->filter( - fn (Transaction $transaction): bool => $this->convertTransactionAmount($transaction, $currency) < 0, + fn (Transaction $transaction): bool => $this->isExpense($transaction), ); $grouped = $expenses @@ -178,12 +177,12 @@ class TransactionAnalysisController extends Controller $totals = []; foreach ($transactions as $transaction) { - $amount = $this->convertTransactionAmount($transaction, $currency); - - if ($amount >= 0) { + if (! $this->isExpense($transaction)) { continue; } + $amount = $this->convertTransactionAmount($transaction, $currency); + foreach ($transaction->labels as $label) { $totals[$label->id] ??= ['id' => $label->id, 'name' => $label->name, 'color' => $label->color, 'amount' => 0]; $totals[$label->id]['amount'] += abs($amount); @@ -206,12 +205,12 @@ class TransactionAnalysisController extends Controller $totals = []; foreach ($transactions as $transaction) { - $amount = $this->convertTransactionAmount($transaction, $currency); - - if ($amount >= 0) { + if (! $this->isExpense($transaction)) { continue; } + $amount = $this->convertTransactionAmount($transaction, $currency); + $name = trim((string) $transaction->creditor_name); if ($name === '') { @@ -236,12 +235,12 @@ class TransactionAnalysisController extends Controller $totals = []; foreach ($transactions as $transaction) { - $amount = $this->convertTransactionAmount($transaction, $currency); - - if ($amount >= 0) { + if (! $this->isExpense($transaction)) { continue; } + $amount = $this->convertTransactionAmount($transaction, $currency); + $account = $transaction->account; $totals[$account->id] ??= [ @@ -268,7 +267,7 @@ class TransactionAnalysisController extends Controller private function largestExpenses(Collection $transactions, string $currency): array { return $transactions - ->filter(fn (Transaction $transaction): bool => $this->convertTransactionAmount($transaction, $currency) < 0) + ->filter(fn (Transaction $transaction): bool => $this->isExpense($transaction)) ->sortBy(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $currency)) ->take(self::LARGEST_EXPENSES_LIMIT) ->map(fn (Transaction $transaction): array => [ @@ -319,13 +318,12 @@ class TransactionAnalysisController extends Controller $buckets = []; foreach ($transactions as $transaction) { $key = $transaction->transaction_date->format($keyFormat); - $amount = $this->convertTransactionAmount($transaction, $currency); $buckets[$key] ??= ['income' => 0, 'expense' => 0]; - if ($amount > 0) { - $buckets[$key]['income'] += $amount; - } else { - $buckets[$key]['expense'] += abs($amount); + if ($this->isIncome($transaction)) { + $buckets[$key]['income'] += $this->convertTransactionAmount($transaction, $currency); + } elseif ($this->isExpense($transaction)) { + $buckets[$key]['expense'] += abs($this->convertTransactionAmount($transaction, $currency)); } } @@ -368,6 +366,39 @@ class TransactionAnalysisController extends Controller return (int) $dates->min()->diffInDays($dates->max()) + 1; } + /** + * Spending is an outflow booked to an expense category or to no category + * at all. Transfers, savings and investments are internal movements, so + * they never count as expenses — matching the cashflow screen. + */ + private function isExpense(Transaction $transaction): bool + { + return $transaction->amount < 0 + && in_array($this->categoryType($transaction), [CategoryType::Expense, null], true); + } + + /** + * Income is an inflow booked to an income category or to no category at + * all. Transfers, savings and investments are excluded for the same reason + * as expenses. + */ + 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( diff --git a/tests/Feature/TransactionAnalysisTest.php b/tests/Feature/TransactionAnalysisTest.php index dca09186..37ea8278 100644 --- a/tests/Feature/TransactionAnalysisTest.php +++ b/tests/Feature/TransactionAnalysisTest.php @@ -27,6 +27,7 @@ function makeTransaction(array $attributes = []): Transaction 'user_id' => test()->user->id, 'account_id' => test()->account->id, 'currency_code' => 'USD', + 'category_id' => null, ...$attributes, ]); } @@ -255,3 +256,98 @@ test('account breakdown sums expenses per funding account', function () { expect($response->json('distinct_account_count'))->toBe(2); expect($response->json('by_account.0'))->toMatchArray(['name' => 'Travel card', 'amount' => 6000]); }); + +test('summary excludes transfer category transactions from income and expense', function () { + $transfer = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Transfer]); + + makeTransaction(['amount' => 100000, 'transaction_date' => '2026-01-10']); + makeTransaction(['amount' => -40000, 'transaction_date' => '2026-01-11']); + + // Internal movements between own accounts: must not move income or expense. + makeTransaction(['amount' => 70000, 'category_id' => $transfer->id, 'transaction_date' => '2026-01-12']); + makeTransaction(['amount' => -70000, 'category_id' => $transfer->id, 'transaction_date' => '2026-01-13']); + + $this->getJson('/api/transactions/analysis') + ->assertOk() + ->assertJson([ + 'summary' => [ + 'income' => 100000, + 'expense' => 40000, + 'net' => 60000, + ], + ]); +}); + +test('summary excludes savings and investment outflows from expense, matching cashflow', function () { + $savings = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Savings]); + $investment = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Investment]); + + makeTransaction(['amount' => -40000, 'transaction_date' => '2026-01-10']); + makeTransaction(['amount' => -25000, 'category_id' => $savings->id, 'transaction_date' => '2026-01-11']); + makeTransaction(['amount' => -15000, 'category_id' => $investment->id, 'transaction_date' => '2026-01-12']); + + $this->getJson('/api/transactions/analysis') + ->assertOk() + ->assertJson(['summary' => ['expense' => 40000]]); +}); + +test('category breakdown ignores transfer, savings and investment categories', function () { + $expense = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Expense, 'name' => 'Groceries']); + $transfer = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Transfer, 'name' => 'Move money']); + $savings = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Savings, 'name' => 'Rainy day']); + + makeTransaction(['amount' => -30000, 'category_id' => $expense->id, 'transaction_date' => '2026-01-10']); + makeTransaction(['amount' => -50000, 'category_id' => $transfer->id, 'transaction_date' => '2026-01-11']); + makeTransaction(['amount' => -20000, 'category_id' => $savings->id, 'transaction_date' => '2026-01-12']); + + $response = $this->getJson('/api/transactions/analysis')->assertOk(); + + expect($response->json('distinct_category_count'))->toBe(1); + expect($response->json('by_category.0'))->toMatchArray(['name' => 'Groceries', 'amount' => 30000]); +}); + +test('tag, payee and account breakdowns ignore transfer categories', function () { + $label = Label::factory()->create(['user_id' => $this->user->id]); + $transfer = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Transfer]); + + makeTransaction(['amount' => -10000, 'creditor_name' => 'Shop', 'transaction_date' => '2026-01-10']) + ->labels()->attach($label); + + $tagged = makeTransaction(['amount' => -90000, 'category_id' => $transfer->id, 'creditor_name' => 'My other account', 'transaction_date' => '2026-01-11']); + $tagged->labels()->attach($label); + + $response = $this->getJson('/api/transactions/analysis')->assertOk(); + + expect($response->json('distinct_label_count'))->toBe(1); + expect($response->json('by_tag.0.amount'))->toBe(10000); + expect($response->json('distinct_payee_count'))->toBe(1); + expect($response->json('by_payee.0'))->toMatchArray(['name' => 'Shop', 'amount' => 10000]); + expect($response->json('distinct_account_count'))->toBe(1); + expect($response->json('by_account.0.amount'))->toBe(10000); +}); + +test('largest expenses ignores transfer category outflows', function () { + $transfer = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Transfer]); + + makeTransaction(['amount' => -5000, 'description' => 'Dinner', 'transaction_date' => '2026-01-10']); + makeTransaction(['amount' => -99000, 'category_id' => $transfer->id, 'description' => 'Transfer out', 'transaction_date' => '2026-01-11']); + + $largest = $this->getJson('/api/transactions/analysis')->assertOk()->json('largest_expenses'); + + expect($largest)->toHaveCount(1); + expect($largest[0])->toMatchArray(['description' => 'Dinner', 'amount' => 5000]); +}); + +test('over time buckets ignore transfer categories', function () { + $transfer = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Transfer]); + + makeTransaction(['amount' => 30000, 'transaction_date' => '2026-01-10']); + makeTransaction(['amount' => -10000, 'transaction_date' => '2026-01-10']); + makeTransaction(['amount' => 50000, 'category_id' => $transfer->id, 'transaction_date' => '2026-01-10']); + makeTransaction(['amount' => -50000, 'category_id' => $transfer->id, 'transaction_date' => '2026-01-10']); + + $points = collect($this->getJson('/api/transactions/analysis')->assertOk()->json('over_time.points')); + $day = $points->firstWhere('date', '2026-01-10'); + + expect($day)->toMatchArray(['income' => 30000, 'expense' => 10000]); +});