Move income/expense-side classification onto the Transaction model
The same in-memory rule for which side of a cashflow split a transaction belongs to (income category or uncategorized inflow; expense category or uncategorized outflow; transfers/savings/investments on neither) was re-implemented in TransactionAnalysisController (isIncomeSide/isExpenseSide) and inline in CashflowAnalyticsController::sumTransactions. Expose it once as Transaction::isIncomeSide()/isExpenseSide() and have both controllers consume it, so the two analytics surfaces cannot drift on how a refund, reversal or uncategorized transaction is bucketed. Behaviour-preserving.
This commit is contained in:
parent
722c7d1bef
commit
5572a2d75a
|
|
@ -218,15 +218,12 @@ class CashflowAnalyticsController extends Controller
|
|||
|
||||
private function sumTransactions(Collection $transactions, string $userCurrency, CategoryType $type): int
|
||||
{
|
||||
return $transactions
|
||||
->filter(function (Transaction $transaction) use ($type): bool {
|
||||
if ($transaction->categoryType() === $type) {
|
||||
return true;
|
||||
}
|
||||
$onSide = $type === CategoryType::Income
|
||||
? fn (Transaction $transaction): bool => $transaction->isIncomeSide()
|
||||
: fn (Transaction $transaction): bool => $transaction->isExpenseSide();
|
||||
|
||||
return $transaction->category_id === null
|
||||
&& $this->matchesSign($transaction->amount, $type === CategoryType::Income ? '>' : '<');
|
||||
})
|
||||
return $transactions
|
||||
->filter($onSide)
|
||||
->sum(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $userCurrency));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@
|
|||
|
||||
namespace App\Http\Controllers\Api;
|
||||
|
||||
use App\Enums\CategoryType;
|
||||
use App\Http\Controllers\Api\Concerns\ConvertsTransactionCurrency;
|
||||
use App\Http\Controllers\Controller;
|
||||
use App\Http\Requests\IndexTransactionRequest;
|
||||
|
|
@ -95,9 +94,9 @@ class TransactionAnalysisController extends Controller
|
|||
$expense = 0;
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
if ($this->isIncomeSide($transaction)) {
|
||||
if ($transaction->isIncomeSide()) {
|
||||
$income += $this->convertTransactionAmount($transaction, $currency);
|
||||
} elseif ($this->isExpenseSide($transaction)) {
|
||||
} elseif ($transaction->isExpenseSide()) {
|
||||
$expense += $this->convertTransactionAmount($transaction, $currency);
|
||||
}
|
||||
}
|
||||
|
|
@ -128,7 +127,7 @@ class TransactionAnalysisController extends Controller
|
|||
private function categoryBreakdown(Collection $transactions, string $currency, string $userId): Collection
|
||||
{
|
||||
$expenses = $transactions->filter(
|
||||
fn (Transaction $transaction): bool => $this->isExpenseSide($transaction),
|
||||
fn (Transaction $transaction): bool => $transaction->isExpenseSide(),
|
||||
);
|
||||
|
||||
$grouped = $expenses
|
||||
|
|
@ -187,7 +186,7 @@ class TransactionAnalysisController extends Controller
|
|||
$totals = [];
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
if (! $this->isExpenseSide($transaction)) {
|
||||
if (! $transaction->isExpenseSide()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -215,7 +214,7 @@ class TransactionAnalysisController extends Controller
|
|||
$totals = [];
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
if (! $this->isExpenseSide($transaction)) {
|
||||
if (! $transaction->isExpenseSide()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -246,7 +245,7 @@ class TransactionAnalysisController extends Controller
|
|||
$totals = [];
|
||||
|
||||
foreach ($transactions as $transaction) {
|
||||
if (! $this->isExpenseSide($transaction)) {
|
||||
if (! $transaction->isExpenseSide()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -279,7 +278,7 @@ class TransactionAnalysisController extends Controller
|
|||
private function largestExpenses(Collection $transactions, string $currency): array
|
||||
{
|
||||
return $transactions
|
||||
->filter(fn (Transaction $transaction): bool => $this->isExpenseSide($transaction) && $transaction->amount < 0)
|
||||
->filter(fn (Transaction $transaction): bool => $transaction->isExpenseSide() && $transaction->amount < 0)
|
||||
->sortBy(fn (Transaction $transaction): int => $this->convertTransactionAmount($transaction, $currency))
|
||||
->take(self::LARGEST_EXPENSES_LIMIT)
|
||||
->map(fn (Transaction $transaction): array => [
|
||||
|
|
@ -332,9 +331,9 @@ class TransactionAnalysisController extends Controller
|
|||
$key = $transaction->transaction_date->format($keyFormat);
|
||||
$buckets[$key] ??= ['income' => 0, 'expense' => 0];
|
||||
|
||||
if ($this->isIncomeSide($transaction)) {
|
||||
if ($transaction->isIncomeSide()) {
|
||||
$buckets[$key]['income'] += $this->convertTransactionAmount($transaction, $currency);
|
||||
} elseif ($this->isExpenseSide($transaction)) {
|
||||
} elseif ($transaction->isExpenseSide()) {
|
||||
$buckets[$key]['expense'] -= $this->convertTransactionAmount($transaction, $currency);
|
||||
}
|
||||
}
|
||||
|
|
@ -377,27 +376,4 @@ class TransactionAnalysisController extends Controller
|
|||
|
||||
return (int) $dates->min()->diffInDays($dates->max()) + 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a transaction belongs to the expense side: anything booked to an
|
||||
* expense category (a refund there nets back out), plus uncategorized
|
||||
* outflows. Transfers, savings and investments are internal movements, so
|
||||
* they sit on neither side — matching the cashflow screen.
|
||||
*/
|
||||
private function isExpenseSide(Transaction $transaction): bool
|
||||
{
|
||||
return $transaction->categoryType() === CategoryType::Expense
|
||||
|| ($transaction->category_id === null && $transaction->amount < 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a transaction belongs to the income side: anything booked to an
|
||||
* income category (a reversal there nets back out), plus uncategorized
|
||||
* inflows. Internal movements are excluded for the same reason as expenses.
|
||||
*/
|
||||
private function isIncomeSide(Transaction $transaction): bool
|
||||
{
|
||||
return $transaction->categoryType() === CategoryType::Income
|
||||
|| ($transaction->category_id === null && $transaction->amount > 0);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -133,6 +133,28 @@ class Transaction extends Model
|
|||
return is_string($type) ? CategoryType::tryFrom($type) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this transaction sits on the income side of a cashflow split:
|
||||
* booked to an income category (a reversal there nets back out) or an
|
||||
* uncategorized inflow. Internal movements (transfer, savings, investment)
|
||||
* belong to neither side.
|
||||
*/
|
||||
public function isIncomeSide(): bool
|
||||
{
|
||||
return $this->categoryType() === CategoryType::Income
|
||||
|| ($this->category_id === null && $this->amount > 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this transaction sits on the expense side: booked to an expense
|
||||
* category (a refund there nets back out) or an uncategorized outflow.
|
||||
*/
|
||||
public function isExpenseSide(): bool
|
||||
{
|
||||
return $this->categoryType() === CategoryType::Expense
|
||||
|| ($this->category_id === null && $this->amount < 0);
|
||||
}
|
||||
|
||||
/** @return BelongsTo<AutomationRule, $this> */
|
||||
public function categorizedByRule(): BelongsTo
|
||||
{
|
||||
|
|
|
|||
|
|
@ -0,0 +1,64 @@
|
|||
<?php
|
||||
|
||||
use App\Enums\CategoryType;
|
||||
use App\Models\Account;
|
||||
use App\Models\Category;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
|
||||
beforeEach(function () {
|
||||
$this->user = User::factory()->create();
|
||||
$this->account = Account::factory()->create(['user_id' => $this->user->id]);
|
||||
});
|
||||
|
||||
function makeSideTransaction(User $user, Account $account, ?Category $category, int $amount): Transaction
|
||||
{
|
||||
return Transaction::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'account_id' => $account->id,
|
||||
'category_id' => $category?->id,
|
||||
'amount' => $amount,
|
||||
])->load('category');
|
||||
}
|
||||
|
||||
it('classifies an income-category transaction as income side regardless of sign', function () {
|
||||
$income = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Income]);
|
||||
|
||||
$positive = makeSideTransaction($this->user, $this->account, $income, 10000);
|
||||
$reversal = makeSideTransaction($this->user, $this->account, $income, -2000);
|
||||
|
||||
expect($positive->isIncomeSide())->toBeTrue()
|
||||
->and($positive->isExpenseSide())->toBeFalse()
|
||||
->and($reversal->isIncomeSide())->toBeTrue();
|
||||
});
|
||||
|
||||
it('classifies an expense-category transaction as expense side regardless of sign', function () {
|
||||
$expense = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Expense]);
|
||||
|
||||
$spend = makeSideTransaction($this->user, $this->account, $expense, -5000);
|
||||
$refund = makeSideTransaction($this->user, $this->account, $expense, 1500);
|
||||
|
||||
expect($spend->isExpenseSide())->toBeTrue()
|
||||
->and($spend->isIncomeSide())->toBeFalse()
|
||||
->and($refund->isExpenseSide())->toBeTrue();
|
||||
});
|
||||
|
||||
it('classifies uncategorized inflows as income and outflows as expense', function () {
|
||||
$inflow = makeSideTransaction($this->user, $this->account, null, 7000);
|
||||
$outflow = makeSideTransaction($this->user, $this->account, null, -3000);
|
||||
|
||||
expect($inflow->isIncomeSide())->toBeTrue()
|
||||
->and($inflow->isExpenseSide())->toBeFalse()
|
||||
->and($outflow->isExpenseSide())->toBeTrue()
|
||||
->and($outflow->isIncomeSide())->toBeFalse();
|
||||
});
|
||||
|
||||
it('treats transfer, savings and investment categories as neither side', function () {
|
||||
foreach ([CategoryType::Transfer, CategoryType::Savings, CategoryType::Investment] as $type) {
|
||||
$category = Category::factory()->create(['user_id' => $this->user->id, 'type' => $type]);
|
||||
$transaction = makeSideTransaction($this->user, $this->account, $category, -5000);
|
||||
|
||||
expect($transaction->isIncomeSide())->toBeFalse()
|
||||
->and($transaction->isExpenseSide())->toBeFalse();
|
||||
}
|
||||
});
|
||||
Loading…
Reference in New Issue