From 9e2a9cadfe0210e0f2a45da8dbcaab1552dc0844 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Tue, 17 Mar 2026 09:29:36 +0000 Subject: [PATCH] fix(cashflow): only count sign-matching transactions in Sankey category breakdown (#232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem When an income category contained both incoming and outgoing transactions (e.g. an \"Income from Rents\" category that also holds property-related expense payments), the Sankey endpoint was summing **all** amounts together and applying `abs()` to the net result. Real example reported: | Date | Description | Amount | |------|-------------|--------| | Mar 13 | BIZUM sent — Lavadora | -€124.03 | | Mar 10 | BIZUM received — luz febrero | +€38.04 | | Mar 9 | Endesa energy payment | -€38.04 | | Mar 4 | BIZUM received — agua Febrero | +€41.34 | | Mar 2 | Comunidad de Propietarios | -€105.92 | - Net: **-€188.61** → `abs()` → **€188.61 shown as income** ❌ - Correct: sum of positive flows only → **€79.38** ✅ ## Fix Added a sign filter to `getCategoryBreakdown()` before the `GROUP BY` aggregation: - Income categories: `WHERE transactions.amount > 0` - Expense categories: `WHERE transactions.amount < 0` This ensures the Sankey shows the **actual gross flow** on each side rather than a misleading abs-of-net. ## Test Added a regression test in `CashflowAnalyticsTest` that reproduces the exact five transactions from the reported scenario and asserts the Sankey returns `7938` cents (€79.38) instead of the buggy `18861` (€188.61). --- .../Api/CashflowAnalyticsController.php | 65 ++++++++++++++- tests/Feature/CashflowAnalyticsTest.php | 79 +++++++++++++++++++ 2 files changed, 140 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Api/CashflowAnalyticsController.php b/app/Http/Controllers/Api/CashflowAnalyticsController.php index 26e848d0..9ea37ec3 100644 --- a/app/Http/Controllers/Api/CashflowAnalyticsController.php +++ b/app/Http/Controllers/Api/CashflowAnalyticsController.php @@ -44,8 +44,10 @@ class CashflowAnalyticsController extends Controller $to = Carbon::parse($validated['to']); $userId = $request->user()->id; - $incomeCategories = $this->getCategoryBreakdown($userId, $from, $to, CategoryType::Income); - $expenseCategories = $this->getCategoryBreakdown($userId, $from, $to, CategoryType::Expense); + // Split by sign, not by category type: a single category can appear on + // both sides when it has both incoming and outgoing transactions. + $incomeCategories = $this->getSankeyBreakdown($userId, $from, $to, '>'); + $expenseCategories = $this->getSankeyBreakdown($userId, $from, $to, '<'); $totalIncome = $incomeCategories->sum('amount'); $totalExpense = $expenseCategories->sum('amount'); @@ -171,12 +173,67 @@ class CashflowAnalyticsController extends Controller ->sum('transactions.amount'); } - private function getCategoryBreakdown(string $userId, Carbon $from, Carbon $to, CategoryType $type) + private function getSankeyBreakdown(string $userId, Carbon $from, Carbon $to, string $operator) { - // Get categorized transactions + $isIncome = $operator === '>'; + + // Group all transactions by category using the amount sign, not the category + // type. This allows one category to appear on both sides of the Sankey when + // it contains transactions of both signs (e.g. an income category that also + // holds property expense payments will show income on the left and the + // outgoing payments on the right). $categorized = Transaction::query() ->where('transactions.user_id', $userId) ->whereBetween('transactions.transaction_date', [$from, $to]) + ->where('transactions.amount', $operator, 0) + ->whereNotNull('transactions.category_id') + ->join('categories', 'transactions.category_id', '=', 'categories.id') + ->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount')) + ->groupBy('transactions.category_id') + ->with('category') + ->get() + ->map(function ($item) { + return [ + 'category_id' => $item->category_id, + 'category' => $item->category, + 'amount' => abs($item->total_amount), + ]; + }); + + $uncategorized = Transaction::query() + ->where('user_id', $userId) + ->whereBetween('transaction_date', [$from, $to]) + ->whereNull('category_id') + ->where('amount', $operator, 0) + ->sum('amount'); + + if ($uncategorized != 0) { + $categorized->push([ + 'category_id' => null, + 'category' => (new Category)->forceFill([ + 'id' => null, + 'name' => $isIncome ? 'Unknown Income' : 'Unknown Expense', + 'type' => $isIncome ? CategoryType::Income : CategoryType::Expense, + 'color' => 'gray', + 'icon' => 'HelpCircle', + ]), + 'amount' => abs($uncategorized), + ]); + } + + return $categorized; + } + + private function getCategoryBreakdown(string $userId, Carbon $from, Carbon $to, CategoryType $type) + { + // 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 = Transaction::query() + ->where('transactions.user_id', $userId) + ->whereBetween('transactions.transaction_date', [$from, $to]) + ->where('transactions.amount', $type === CategoryType::Income ? '>' : '<', 0) ->join('categories', function ($join) use ($type) { $join->on('transactions.category_id', '=', 'categories.id') ->where('categories.type', '=', $type); diff --git a/tests/Feature/CashflowAnalyticsTest.php b/tests/Feature/CashflowAnalyticsTest.php index bc6432bb..4a9520e3 100644 --- a/tests/Feature/CashflowAnalyticsTest.php +++ b/tests/Feature/CashflowAnalyticsTest.php @@ -483,6 +483,85 @@ 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). + $incomeFromRents = Category::factory()->create([ + 'user_id' => $this->user->id, + 'type' => CategoryType::Income, + 'name' => 'Income from Rents', + ]); + + $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, + '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', + ]); + + $response = $this->getJson('/api/cashflow/sankey?'.http_build_query([ + 'from' => '2026-03-01', + 'to' => '2026-03-31', + ])); + + $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); +}); + test('breakdown includes unknown income category', function () { $incomeCategory = Category::factory()->create([ 'user_id' => $this->user->id,