diff --git a/app/Services/BudgetPeriodService.php b/app/Services/BudgetPeriodService.php index 589a5cb6..8caf03c2 100644 --- a/app/Services/BudgetPeriodService.php +++ b/app/Services/BudgetPeriodService.php @@ -40,7 +40,7 @@ class BudgetPeriodService if ($budget->rollover_type->value === 'carry_over') { $totalSpent = $period->budgetTransactions()->sum('amount'); - $remaining = $period->allocated_amount - abs($totalSpent); + $remaining = $period->allocated_amount - $totalSpent; if ($remaining > 0) { $carriedOverAmount = $remaining; diff --git a/app/Services/BudgetTransactionService.php b/app/Services/BudgetTransactionService.php index cc117e1b..2d9d2214 100644 --- a/app/Services/BudgetTransactionService.php +++ b/app/Services/BudgetTransactionService.php @@ -62,7 +62,7 @@ class BudgetTransactionService BudgetTransaction::create([ 'transaction_id' => $transaction->id, 'budget_period_id' => $period->id, - 'amount' => abs($transaction->amount), + 'amount' => -$transaction->amount, ]); } } @@ -126,7 +126,7 @@ class BudgetTransactionService BudgetTransaction::create([ 'transaction_id' => $transaction->id, 'budget_period_id' => $period->id, - 'amount' => abs($transaction->amount), + 'amount' => -$transaction->amount, ]); $assignedCount++; diff --git a/database/migrations/2026_02_24_193117_fix_budget_transaction_refund_amounts.php b/database/migrations/2026_02_24_193117_fix_budget_transaction_refund_amounts.php new file mode 100644 index 00000000..8dda3039 --- /dev/null +++ b/database/migrations/2026_02_24_193117_fix_budget_transaction_refund_amounts.php @@ -0,0 +1,34 @@ + 0 + ) + '); + } + + /** + * Revert: make all budget_transaction amounts positive again (original behavior). + */ + public function down(): void + { + DB::statement(' + UPDATE budget_transactions + SET amount = ABS(amount) + '); + } +}; diff --git a/tests/Feature/BudgetTransactionServiceTest.php b/tests/Feature/BudgetTransactionServiceTest.php index d69dc9eb..28dbd791 100644 --- a/tests/Feature/BudgetTransactionServiceTest.php +++ b/tests/Feature/BudgetTransactionServiceTest.php @@ -240,14 +240,14 @@ test('assignHistoricalTransactionsToPeriod works with transactions having multip expect($count)->toBe(1); }); -test('assignHistoricalTransactionsToPeriod stores absolute value of amount', function () { +test('assignHistoricalTransactionsToPeriod stores negated transaction amount for expenses', function () { $category = Category::factory()->create(['user_id' => $this->user->id]); - $transaction = Transaction::factory()->create([ + Transaction::factory()->create([ 'user_id' => $this->user->id, 'category_id' => $category->id, 'transaction_date' => now()->subDays(5), - 'amount' => -5000, // Negative amount + 'amount' => -5000, // Expense (negative) ]); $budget = Budget::factory()->create([ @@ -265,7 +265,101 @@ test('assignHistoricalTransactionsToPeriod stores absolute value of amount', fun $budgetTransaction = $period->budgetTransactions()->first(); - expect($budgetTransaction->amount)->toBe(5000); // Should be positive + expect($budgetTransaction->amount)->toBe(5000); // -(-5000) = 5000, adds to spending +}); + +test('assignHistoricalTransactionsToPeriod stores refund as negative amount', function () { + $category = Category::factory()->create(['user_id' => $this->user->id]); + + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $category->id, + 'transaction_date' => now()->subDays(5), + 'amount' => 1000, // Refund (positive) + ]); + + $budget = Budget::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $category->id, + ]); + + $period = BudgetPeriod::factory()->create([ + 'budget_id' => $budget->id, + 'start_date' => now()->subDays(30), + 'end_date' => now()->addDays(30), + ]); + + $this->service->assignHistoricalTransactionsToPeriod($period); + + $budgetTransaction = $period->budgetTransactions()->first(); + + expect($budgetTransaction->amount)->toBe(-1000); // -(+1000) = -1000, reduces spending +}); + +test('budget spending correctly reflects mix of expenses and refunds', function () { + $category = Category::factory()->create(['user_id' => $this->user->id]); + + // $50 expense + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $category->id, + 'transaction_date' => now()->subDays(5), + 'amount' => -5000, + ]); + + // $10 refund + Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $category->id, + 'transaction_date' => now()->subDays(3), + 'amount' => 1000, + ]); + + $budget = Budget::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $category->id, + ]); + + $period = BudgetPeriod::factory()->create([ + 'budget_id' => $budget->id, + 'start_date' => now()->subDays(30), + 'end_date' => now()->addDays(30), + 'allocated_amount' => 10000, + ]); + + $this->service->assignHistoricalTransactionsToPeriod($period); + + // Net spending should be $40 (5000 - 1000 = 4000) + $totalSpent = (int) $period->budgetTransactions()->sum('amount'); + expect($totalSpent)->toBe(4000); +}); + +test('assignTransaction stores refund as negative budget transaction amount', function () { + $category = Category::factory()->create(['user_id' => $this->user->id]); + + $budget = Budget::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $category->id, + ]); + + $period = BudgetPeriod::factory()->create([ + 'budget_id' => $budget->id, + 'start_date' => now()->subDays(30), + 'end_date' => now()->addDays(30), + ]); + + // Create a refund transaction + $refund = Transaction::factory()->create([ + 'user_id' => $this->user->id, + 'category_id' => $category->id, + 'transaction_date' => now()->subDays(2), + 'amount' => 2000, // positive = refund + ]); + + $this->service->assignTransaction($refund); + + $budgetTransaction = $period->budgetTransactions()->first(); + expect($budgetTransaction->amount)->toBe(-2000); }); test('assignHistoricalTransactionsToPeriod only assigns to correct user', function () {