From 6caadad1dbc267ea5d2bb58f2ce92200146349bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Wed, 27 May 2026 15:16:50 +0200 Subject: [PATCH] fix: net category refunds in cashflow (#441) ## Summary - net mixed-sign category transactions before cashflow analytics display - align dashboard, cashflow trend, breakdown, and sankey behavior with top categories - add feature tests for refund-style expense netting ## Tests - php artisan test --compact tests/Feature/CashflowAnalyticsTest.php tests/Feature/DashboardAnalyticsTest.php - vendor/bin/pint --dirty --format agent --- .../Api/CashflowAnalyticsController.php | 75 +++++++---- .../Api/DashboardAnalyticsController.php | 9 +- app/Http/Controllers/DashboardController.php | 7 +- tests/Feature/CashflowAnalyticsTest.php | 124 +++++++++++------- tests/Feature/DashboardAnalyticsTest.php | 48 +++++++ 5 files changed, 187 insertions(+), 76 deletions(-) diff --git a/app/Http/Controllers/Api/CashflowAnalyticsController.php b/app/Http/Controllers/Api/CashflowAnalyticsController.php index 7e2658bc..350daac7 100644 --- a/app/Http/Controllers/Api/CashflowAnalyticsController.php +++ b/app/Http/Controllers/Api/CashflowAnalyticsController.php @@ -179,8 +179,8 @@ class CashflowAnalyticsController extends Controller private function cashflowSummaryFromTransactions(Collection $transactions, string $userCurrency): array { - $income = $this->sumTransactions($transactions, $userCurrency, CategoryType::Income); - $expense = abs($this->sumTransactions($transactions, $userCurrency, CategoryType::Expense)); + $income = max(0, $this->sumTransactions($transactions, $userCurrency, CategoryType::Income)); + $expense = max(0, -$this->sumTransactions($transactions, $userCurrency, CategoryType::Expense)); $savings = $this->sumOutflowTransactions($transactions, $userCurrency, CategoryType::Savings); $investments = $this->sumOutflowTransactions($transactions, $userCurrency, CategoryType::Investment); @@ -222,6 +222,7 @@ class CashflowAnalyticsController extends Controller private function getSankeyBreakdown(string $userId, string $userCurrency, Carbon $from, Carbon $to, string $operator): Collection { $isIncome = $operator === '>'; + $type = $isIncome ? CategoryType::Income : CategoryType::Expense; $transactions = Transaction::query() ->where('transactions.user_id', $userId) ->whereBetween('transactions.transaction_date', [$from, $to]) @@ -230,13 +231,10 @@ class CashflowAnalyticsController extends Controller $this->preloadExchangeRates($transactions, $userCurrency); - // Non-transfer categories keep the existing sign-based behavior so a - // category with mixed signs can appear on both sides of the Sankey. $regularCategories = $transactions - ->filter(function (Transaction $transaction) use ($operator): bool { + ->filter(function (Transaction $transaction) use ($type): bool { return $transaction->category_id !== null - && $this->categoryType($transaction) !== CategoryType::Transfer - && $this->matchesSign($transaction->amount, $operator); + && $this->categoryType($transaction) === $type; }) ->groupBy('category_id') ->map(function (Collection $transactions) use ($userCurrency): array { @@ -246,8 +244,15 @@ class CashflowAnalyticsController extends Controller 'category_id' => $transactions->first()->category_id, 'category' => $transactions->first()->category, 'amount' => abs($totalAmount), + 'total_amount' => $totalAmount, ]; - }); + }) + ->filter(fn (array $item): bool => $this->categoryNetAmountMatchesSide($item['total_amount'], $type)) + ->map(fn (array $item): array => [ + 'category_id' => $item['category_id'], + 'category' => $item['category'], + 'amount' => $item['amount'], + ]); $transferCategories = $transactions ->filter(function (Transaction $transaction) use ($isIncome): bool { @@ -317,16 +322,37 @@ class CashflowAnalyticsController extends Controller $income = 0; $expense = 0; - foreach ($transactions as $transaction) { + $categorized = $transactions + ->filter(fn (Transaction $transaction): bool => $transaction->category_id !== null) + ->groupBy('category_id'); + + foreach ($categorized as $categoryTransactions) { + $firstTransaction = $categoryTransactions->first(); + $type = $this->categoryType($firstTransaction); + + if (! in_array($type, [CategoryType::Income, CategoryType::Expense], true)) { + continue; + } + + $amount = $categoryTransactions->sum(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $userCurrency)); + + if ($this->categoryNetAmountMatchesSide($amount, $type)) { + if ($type === CategoryType::Income) { + $income += $amount; + } else { + $expense += abs($amount); + } + } + } + + foreach ($transactions->whereNull('category_id') as $transaction) { $amount = $this->convertTransactionAmount($transaction, $userCurrency); - if (($this->categoryType($transaction) === CategoryType::Income && $transaction->amount > 0) - || ($transaction->category_id === null && $transaction->amount > 0)) { + if ($transaction->amount > 0) { $income += $amount; } - if (($this->categoryType($transaction) === CategoryType::Expense && $transaction->amount < 0) - || ($transaction->category_id === null && $transaction->amount < 0)) { + if ($transaction->amount < 0) { $expense += abs($amount); } } @@ -348,15 +374,8 @@ class CashflowAnalyticsController extends Controller $this->preloadExchangeRates($transactions, $userCurrency); - // Get categorized transactions — filter by sign so that outgoing payments - // in an income category (or refunds in an expense category) are excluded. - // This ensures the Sankey shows the actual gross flow for each side, not - // the net which could be misleading when categories contain mixed-sign entries. $categorized = $transactions - ->filter(function (Transaction $transaction) use ($type): bool { - return $this->categoryType($transaction) === $type - && $this->matchesSign($transaction->amount, $type === CategoryType::Income ? '>' : '<'); - }) + ->filter(fn (Transaction $transaction): bool => $this->categoryType($transaction) === $type) ->groupBy('category_id') ->map(function (Collection $transactions) use ($userCurrency): array { $totalAmount = $transactions->sum(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $userCurrency)); @@ -365,8 +384,15 @@ class CashflowAnalyticsController extends Controller 'category_id' => $transactions->first()->category_id, 'category' => $transactions->first()->category, 'amount' => abs($totalAmount), + 'total_amount' => $totalAmount, ]; - }); + }) + ->filter(fn (array $item): bool => $this->categoryNetAmountMatchesSide($item['total_amount'], $type)) + ->map(fn (array $item): array => [ + 'category_id' => $item['category_id'], + 'category' => $item['category'], + 'amount' => $item['amount'], + ]); $uncategorized = $transactions ->filter(function (Transaction $transaction) use ($type): bool { @@ -451,4 +477,9 @@ class CashflowAnalyticsController extends Controller { return $operator === '>' ? $amount > 0 : $amount < 0; } + + private function categoryNetAmountMatchesSide(int $amount, CategoryType $type): bool + { + return $type === CategoryType::Income ? $amount > 0 : $amount < 0; + } } diff --git a/app/Http/Controllers/Api/DashboardAnalyticsController.php b/app/Http/Controllers/Api/DashboardAnalyticsController.php index a7921da8..4295aa88 100644 --- a/app/Http/Controllers/Api/DashboardAnalyticsController.php +++ b/app/Http/Controllers/Api/DashboardAnalyticsController.php @@ -470,11 +470,12 @@ class DashboardAnalyticsController extends Controller ->groupBy('transactions.category_id') ->with('category') ->get() + ->filter(fn ($item): bool => (int) $item->total_amount < 0) ->map(function ($item) { return [ 'category_id' => $item->category_id, 'category' => $item->category, - 'amount' => abs($item->total_amount), + 'amount' => (int) -$item->total_amount, ]; }); } @@ -519,7 +520,7 @@ class DashboardAnalyticsController extends Controller }) ->sum('transactions.amount'); - return abs($spending); + return max(0, -$spending); } private function calculateCashFlow(Carbon $from, Carbon $to): array @@ -545,8 +546,8 @@ class DashboardAnalyticsController extends Controller ->sum('transactions.amount'); return [ - 'income' => $income, - 'expense' => abs($expense), + 'income' => max(0, $income), + 'expense' => max(0, -$expense), ]; } diff --git a/app/Http/Controllers/DashboardController.php b/app/Http/Controllers/DashboardController.php index 4e3de2dd..6bce5ece 100644 --- a/app/Http/Controllers/DashboardController.php +++ b/app/Http/Controllers/DashboardController.php @@ -104,19 +104,20 @@ class DashboardController extends Controller ->groupBy('transactions.category_id') ->with('category') ->get() + ->filter(fn ($item): bool => (int) $item->total_amount < 0) ->map(function ($item) { return [ 'category_id' => $item->category_id, 'category' => $item->category, - 'amount' => abs($item->total_amount), + 'amount' => (int) -$item->total_amount, ]; }); } private function calculateCashflowSummary(string $userId, Carbon $from, Carbon $to): array { - $income = $this->getTransactionSum($userId, $from, $to, CategoryType::Income); - $expense = abs($this->getTransactionSum($userId, $from, $to, CategoryType::Expense)); + $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; diff --git a/tests/Feature/CashflowAnalyticsTest.php b/tests/Feature/CashflowAnalyticsTest.php index c562dd0e..0139ba2b 100644 --- a/tests/Feature/CashflowAnalyticsTest.php +++ b/tests/Feature/CashflowAnalyticsTest.php @@ -490,6 +490,75 @@ test('cashflow trend returns monthly data for specified months', function () { expect($data['data'][0])->toHaveKeys(['month', 'income', 'expense', 'net']); }); +test('cashflow analytics net refunds in expense categories', function () { + $foodDelivery = Category::factory()->create([ + 'user_id' => $this->user->id, + 'type' => CategoryType::Expense, + 'name' => 'Food Delivery', + ]); + + $account = Account::factory()->create(['user_id' => $this->user->id]); + + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'account_id' => $account->id, + 'category_id' => $foodDelivery->id, + 'amount' => -8000, + 'transaction_date' => '2026-05-05', + ]); + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'account_id' => $account->id, + 'category_id' => $foodDelivery->id, + 'amount' => 2000, + 'transaction_date' => '2026-05-06', + ]); + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'account_id' => $account->id, + 'category_id' => $foodDelivery->id, + 'amount' => 2000, + 'transaction_date' => '2026-05-07', + ]); + + $summary = $this->getJson('/api/cashflow/summary?'.http_build_query([ + 'from' => '2026-05-01', + 'to' => '2026-05-31', + ])); + $trend = $this->getJson('/api/cashflow/trend?'.http_build_query([ + 'from' => '2026-05-01', + 'to' => '2026-05-31', + ])); + $breakdown = $this->getJson('/api/cashflow/breakdown?'.http_build_query([ + 'from' => '2026-05-01', + 'to' => '2026-05-31', + 'type' => 'expense', + ])); + $sankey = $this->getJson('/api/cashflow/sankey?'.http_build_query([ + 'from' => '2026-05-01', + 'to' => '2026-05-31', + ])); + + $summary->assertOk() + ->assertJsonPath('current.expense', 4000) + ->assertJsonPath('current.net', -4000); + + $trend->assertOk() + ->assertJsonPath('data.0.expense', 4000) + ->assertJsonPath('data.0.net', -4000); + + $breakdown->assertOk() + ->assertJsonPath('total', 4000) + ->assertJsonPath('data.0.category.name', 'Food Delivery') + ->assertJsonPath('data.0.amount', 4000); + + $sankey->assertOk() + ->assertJsonPath('total_income', 0) + ->assertJsonPath('total_expense', 4000) + ->assertJsonPath('expense_categories.0.category.name', 'Food Delivery') + ->assertJsonPath('expense_categories.0.amount', 4000); +}); + test('cashflow trend does not include tracked transfers', function () { $incomeCategory = Category::factory()->create([ 'user_id' => $this->user->id, @@ -873,10 +942,7 @@ test('sankey includes unknown income and expense categories', function () { expect($data['expense_categories'][0]['amount'])->toBe(50000); }); -test('sankey income category with mixed positive and negative transactions shows only positive amounts', function () { - // Reproduces a real-world scenario where an "Income from Rents" category contains - // both income receipts and property-related expense payments. The Sankey should - // show only the actual money received (positive flows), not abs(net). +test('sankey income category nets mixed positive and negative transactions', function () { $incomeFromRents = Category::factory()->create([ 'user_id' => $this->user->id, 'type' => CategoryType::Income, @@ -885,49 +951,19 @@ test('sankey income category with mixed positive and negative transactions shows $account = Account::factory()->create(['user_id' => $this->user->id]); - // Mar 13: -€124.03 — BIZUM sent (Lavadora) Transaction::factory()->create([ 'user_id' => $this->user->id, 'account_id' => $account->id, 'category_id' => $incomeFromRents->id, - 'amount' => -12403, - 'transaction_date' => '2026-03-13', - ]); - - // Mar 10: +€38.04 — BIZUM received (luz febrero 2026) - Transaction::factory()->create([ - 'user_id' => $this->user->id, - 'account_id' => $account->id, - 'category_id' => $incomeFromRents->id, - 'amount' => 3804, + 'amount' => 30000, 'transaction_date' => '2026-03-10', ]); - - // Mar 9: -€38.04 — Endesa energy payment Transaction::factory()->create([ 'user_id' => $this->user->id, 'account_id' => $account->id, 'category_id' => $incomeFromRents->id, - 'amount' => -3804, - 'transaction_date' => '2026-03-09', - ]); - - // Mar 4: +€41.34 — BIZUM received (agua Febrero 2026) - Transaction::factory()->create([ - 'user_id' => $this->user->id, - 'account_id' => $account->id, - 'category_id' => $incomeFromRents->id, - 'amount' => 4134, - 'transaction_date' => '2026-03-04', - ]); - - // Mar 2: -€105.92 — Comunidad de Propietarios - Transaction::factory()->create([ - 'user_id' => $this->user->id, - 'account_id' => $account->id, - 'category_id' => $incomeFromRents->id, - 'amount' => -10592, - 'transaction_date' => '2026-03-02', + 'amount' => -10000, + 'transaction_date' => '2026-03-13', ]); $response = $this->getJson('/api/cashflow/sankey?'.http_build_query([ @@ -938,18 +974,12 @@ test('sankey income category with mixed positive and negative transactions shows $response->assertOk(); $data = $response->json(); - // Positive flows only → income side: €38.04 + €41.34 = €79.38 (7938 cents) $rentIncome = collect($data['income_categories'])->firstWhere('category.name', 'Income from Rents'); expect($rentIncome)->not->toBeNull(); - expect($rentIncome['amount'])->toBe(7938); - expect($data['total_income'])->toBe(7938); - - // Negative flows → expense side: €124.03 + €38.04 + €105.92 = €267.99 (26799 cents) - // The category appears on BOTH sides of the Sankey. - $rentExpense = collect($data['expense_categories'])->firstWhere('category.name', 'Income from Rents'); - expect($rentExpense)->not->toBeNull(); - expect($rentExpense['amount'])->toBe(26799); - expect($data['total_expense'])->toBe(26799); + expect($rentIncome['amount'])->toBe(20000); + expect($data['total_income'])->toBe(20000); + expect(collect($data['expense_categories'])->pluck('category.name'))->not->toContain('Income from Rents'); + expect($data['total_expense'])->toBe(0); }); test('sankey excludes hidden transfer categories from both sides', function () { diff --git a/tests/Feature/DashboardAnalyticsTest.php b/tests/Feature/DashboardAnalyticsTest.php index 1b57e85d..3bb5fd61 100644 --- a/tests/Feature/DashboardAnalyticsTest.php +++ b/tests/Feature/DashboardAnalyticsTest.php @@ -247,6 +247,54 @@ test('top categories returns highest spending categories', function () { expect($data[1]['amount'])->toBe(3000); }); +test('dashboard analytics net refunds in expense categories', function () { + $foodDelivery = Category::factory()->create([ + 'user_id' => $this->user->id, + 'type' => CategoryType::Expense, + 'name' => 'Food Delivery', + ]); + + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $foodDelivery->id, + 'amount' => -8000, + 'transaction_date' => '2026-05-05', + ]); + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $foodDelivery->id, + 'amount' => 2000, + 'transaction_date' => '2026-05-06', + ]); + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $foodDelivery->id, + 'amount' => 2000, + 'transaction_date' => '2026-05-07', + ]); + + $period = [ + 'from' => '2026-05-01', + 'to' => '2026-05-31', + ]; + + $monthlySpending = $this->getJson('/api/dashboard/monthly-spending?'.http_build_query($period)); + $cashFlow = $this->getJson('/api/dashboard/cash-flow?'.http_build_query($period)); + $topCategories = $this->getJson('/api/dashboard/top-categories?'.http_build_query($period)); + + $monthlySpending->assertOk() + ->assertJsonPath('current', 4000); + + $cashFlow->assertOk() + ->assertJsonPath('current.income', 0) + ->assertJsonPath('current.expense', 4000); + + $topCategories->assertOk() + ->assertJsonPath('0.category.name', 'Food Delivery') + ->assertJsonPath('0.amount', 4000) + ->assertJsonPath('0.total_amount', 4000); +}); + test('top categories excludes soft deleted categories', function () { $activeCategory = Category::factory()->create([ 'user_id' => $this->user->id,