diff --git a/app/Http/Controllers/TransactionController.php b/app/Http/Controllers/TransactionController.php index a2fc312b..da086fc8 100644 --- a/app/Http/Controllers/TransactionController.php +++ b/app/Http/Controllers/TransactionController.php @@ -264,15 +264,15 @@ class TransactionController extends Controller $transaction->save(); } - // Move the manual account balance to match an edited amount/date/account. - // ponytail: like create/delete, this trusts the opt-in flag and nudges a - // single dated snapshot — it does not cascade to later snapshots and keeps - // no record of whether creation adjusted the balance. So it is exact for - // the common case (recent transaction, flag used consistently) and can - // drift otherwise; upgrade to a transaction-derived balance if that drift - // ever matters. Connected accounts are skipped inside the adjuster. + // Move the manual account balance to match an edited amount/date/account: + // strip the pre-edit contribution (exactly as a deletion would) and apply + // the new one (exactly as a creation would), both cascading forward. + // ponytail: like create/delete, this trusts the opt-in flag and keeps no + // record of whether creation adjusted the balance, so mixing the flag + // across create and edit can drift; a transaction-derived balance would + // remove that trust. Connected accounts are skipped inside the adjuster. if ($request->boolean('update_balance') && $transaction->wasChanged(['amount', 'transaction_date', 'account_id'])) { - $balanceAdjuster->reverseCreatedTransaction($originalSnapshot); + $balanceAdjuster->reverseDeletedTransaction($originalSnapshot); $balanceAdjuster->applyCreatedTransaction($transaction->load('account')); } diff --git a/app/Services/ManualBalanceAdjuster.php b/app/Services/ManualBalanceAdjuster.php index 24e02091..88156f23 100644 --- a/app/Services/ManualBalanceAdjuster.php +++ b/app/Services/ManualBalanceAdjuster.php @@ -2,52 +2,19 @@ namespace App\Services; -use App\Models\AccountBalance; +use App\Models\Account; use App\Models\Transaction; -use Illuminate\Support\Carbon; class ManualBalanceAdjuster { /** - * Reverse a deleted transaction's effect on its manual account's current balance. + * Reverse a deleted transaction's effect on its manual account's balances. * - * Adjusts today's balance by the inverse of the transaction amount: an expense - * (negative amount) increases the balance, income (positive amount) decreases it. - * Connected accounts are skipped because their balances come from bank sync. + * Subtracts the transaction amount from its own day and every later + * snapshot, mirroring the forward shift applied on creation. Connected + * accounts are skipped because their balances come from bank sync. */ public function reverseDeletedTransaction(Transaction $transaction): void - { - $this->adjust($transaction, Carbon::now()->toDateString(), -$transaction->amount); - } - - /** - * Apply a newly created transaction to its manual account's balance. - * - * Adjusts the balance on the transaction's own date. The base is that day's - * balance if one exists, otherwise the closest earlier balance, otherwise - * zero (the first transaction on the account). Connected accounts are - * skipped because their balances come from bank sync. - */ - public function applyCreatedTransaction(Transaction $transaction): void - { - $this->adjust($transaction, $transaction->transaction_date->toDateString(), $transaction->amount); - } - - /** - * Reverse a transaction's effect on its manual account's balance on the - * transaction's own date. Pair with applyCreatedTransaction to move the - * balance when an existing manual transaction is edited. - */ - public function reverseCreatedTransaction(Transaction $transaction): void - { - $this->adjust($transaction, $transaction->transaction_date->toDateString(), -$transaction->amount); - } - - /** - * Nudge the manual account's stored balance on a given date by a delta. - * Connected accounts are skipped because their balances come from bank sync. - */ - private function adjust(Transaction $transaction, string $balanceDate, int $delta): void { $account = $transaction->account; @@ -55,19 +22,61 @@ class ManualBalanceAdjuster return; } - $baseBalance = $account->balances() - ->where('balance_date', '<=', $balanceDate) - ->orderByDesc('balance_date') - ->value('balance') ?? 0; - - AccountBalance::updateOrCreate( - [ - 'account_id' => $account->id, - 'balance_date' => $balanceDate, - ], - [ - 'balance' => $baseBalance + $delta, - ], + $this->shiftBalancesFrom( + $account, + $transaction->transaction_date->toDateString(), + -$transaction->amount, ); } + + /** + * Apply a newly created transaction to its manual account's balances. + * + * Seeds a snapshot on the transaction's own date (from the carried-forward + * balance when none exists yet), then shifts that day and every later + * snapshot by the transaction amount. Connected accounts are skipped + * because their balances come from bank sync. + */ + public function applyCreatedTransaction(Transaction $transaction): void + { + $account = $transaction->account; + + if ($account === null || $account->isConnected()) { + return; + } + + $transactionDate = $transaction->transaction_date->toDateString(); + + $account->balances()->firstOrCreate( + ['balance_date' => $transactionDate], + ['balance' => $this->carriedForwardBalance($account, $transactionDate)], + ); + + $this->shiftBalancesFrom($account, $transactionDate, $transaction->amount); + } + + /** + * Shift every balance snapshot on or after the given date by the delta. + * + * Balances carry forward, so a retroactive change must move the + * transaction's own day and every later snapshot (such as today's current + * balance) by the same amount to keep the running balance consistent. + */ + private function shiftBalancesFrom(Account $account, string $fromDate, int $delta): void + { + $account->balances() + ->where('balance_date', '>=', $fromDate) + ->increment('balance', $delta); + } + + /** + * The most recent balance strictly before the given date, or 0 if none. + */ + private function carriedForwardBalance(Account $account, string $date): int + { + return $account->balances() + ->where('balance_date', '<', $date) + ->orderByDesc('balance_date') + ->value('balance') ?? 0; + } } diff --git a/resources/js/components/accounts/balances-modal.tsx b/resources/js/components/accounts/balances-modal.tsx index f3367887..bde25549 100644 --- a/resources/js/components/accounts/balances-modal.tsx +++ b/resources/js/components/accounts/balances-modal.tsx @@ -312,7 +312,10 @@ export function BalancesModal({ ) : ( balances.map((balance) => ( - + {formatDate( balance.balance_date, diff --git a/resources/js/components/transactions/edit-transaction-dialog.test.tsx b/resources/js/components/transactions/edit-transaction-dialog.test.tsx index d4923b0e..c287bb16 100644 --- a/resources/js/components/transactions/edit-transaction-dialog.test.tsx +++ b/resources/js/components/transactions/edit-transaction-dialog.test.tsx @@ -269,7 +269,9 @@ describe('EditTransactionDialog', () => { expect( screen.getByPlaceholderText('Transaction description'), ).toBeInTheDocument(); - expect(document.querySelector('input[type="date"]')).toBeInTheDocument(); + expect( + document.querySelector('input[type="date"]'), + ).toBeInTheDocument(); }); it('keeps amount and description read-only for an imported transaction', () => { diff --git a/tests/Feature/TransactionTest.php b/tests/Feature/TransactionTest.php index 62e3efe3..4d72b429 100644 --- a/tests/Feature/TransactionTest.php +++ b/tests/Feature/TransactionTest.php @@ -350,19 +350,18 @@ test('deleting a manual account income decreases the current balance when reques ]); }); -test('deleting a transaction creates a current balance from the latest known balance', function () { +test('deleting a past-dated transaction reverses it on that date and every later balance', function () { $user = User::factory()->onboarded()->create(); $account = Account::factory()->create(['user_id' => $user->id]); - $account->balances()->create([ - 'balance_date' => now()->subDays(5)->toDateString(), - 'balance' => 50000, - ]); + $account->balances()->create(['balance_date' => '2025-11-10', 'balance' => 5000]); + $account->balances()->create(['balance_date' => '2025-11-11', 'balance' => 5000]); $transaction = Transaction::factory()->create([ 'user_id' => $user->id, 'account_id' => $account->id, 'amount' => -1500, + 'transaction_date' => '2025-11-10', ]); actingAs($user) @@ -371,8 +370,13 @@ test('deleting a transaction creates a current balance from the latest known bal $this->assertDatabaseHas('account_balances', [ 'account_id' => $account->id, - 'balance_date' => now()->toDateString(), - 'balance' => 51500, + 'balance_date' => '2025-11-10', + 'balance' => 6500, + ]); + $this->assertDatabaseHas('account_balances', [ + 'account_id' => $account->id, + 'balance_date' => '2025-11-11', + 'balance' => 6500, ]); }); @@ -481,6 +485,35 @@ test('creating a transaction creates a balance on its date from the closest earl ]); }); +test('creating a past-dated transaction updates that date and every later balance', function () { + $user = User::factory()->onboarded()->create(); + $account = Account::factory()->create(['user_id' => $user->id]); + + $account->balances()->create(['balance_date' => '2025-11-10', 'balance' => 1000]); + $account->balances()->create(['balance_date' => '2025-11-11', 'balance' => 1000]); + + actingAs($user)->postJson(route('transactions.store'), [ + 'account_id' => $account->id, + 'description' => 'encrypted_description', + 'transaction_date' => '2025-11-10', + 'amount' => -500, + 'currency_code' => 'USD', + 'source' => 'manually_created', + 'update_balance' => true, + ])->assertCreated(); + + $this->assertDatabaseHas('account_balances', [ + 'account_id' => $account->id, + 'balance_date' => '2025-11-10', + 'balance' => 500, + ]); + $this->assertDatabaseHas('account_balances', [ + 'account_id' => $account->id, + 'balance_date' => '2025-11-11', + 'balance' => 500, + ]); +}); + test('creating the first transaction on an account creates a balance equal to its amount', function () { $user = User::factory()->onboarded()->create(); $account = Account::factory()->create(['user_id' => $user->id]);