fix(cashflow): only count sign-matching transactions in Sankey category breakdown (#232)

## 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).
This commit is contained in:
Víctor Falcón 2026-03-17 09:29:36 +00:00 committed by GitHub
parent 6525c31fff
commit 9e2a9cadfe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 140 additions and 4 deletions

View File

@ -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);

View File

@ -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,