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
This commit is contained in:
parent
d5d262e9fd
commit
6caadad1db
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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 () {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue