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.
This commit is contained in:
parent
6727a9c64a
commit
fad189e23b
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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]);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue