diff --git a/app/Http/Controllers/Api/CashflowAnalyticsController.php b/app/Http/Controllers/Api/CashflowAnalyticsController.php index 95943cce..cdad16c4 100644 --- a/app/Http/Controllers/Api/CashflowAnalyticsController.php +++ b/app/Http/Controllers/Api/CashflowAnalyticsController.php @@ -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)); } diff --git a/app/Http/Controllers/Api/TransactionAnalysisController.php b/app/Http/Controllers/Api/TransactionAnalysisController.php index 2c471161..ed325060 100644 --- a/app/Http/Controllers/Api/TransactionAnalysisController.php +++ b/app/Http/Controllers/Api/TransactionAnalysisController.php @@ -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); - } } diff --git a/app/Models/Transaction.php b/app/Models/Transaction.php index 2fea5d2a..dadf0757 100644 --- a/app/Models/Transaction.php +++ b/app/Models/Transaction.php @@ -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 */ public function categorizedByRule(): BelongsTo { diff --git a/tests/Feature/TransactionSideClassificationTest.php b/tests/Feature/TransactionSideClassificationTest.php new file mode 100644 index 00000000..c3043ca8 --- /dev/null +++ b/tests/Feature/TransactionSideClassificationTest.php @@ -0,0 +1,64 @@ +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(); + } +});