From 9312d24f9347ea7b07ef3955b7596f495afed512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Thu, 16 Jul 2026 08:38:02 +0200 Subject: [PATCH] fix(balances): propagate retroactive transaction changes to later balances (#682) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem When a transaction is created (or deleted) with the **update balance** option on a **past date**, only that day's balance snapshot was adjusted. Balances are stored as sparse per-date snapshots read with carry-forward semantics (`BalanceLookup::getBalanceAt` returns the most recent snapshot on/before a date), so later snapshots — most importantly *today's current balance* — kept their stale value. Reproduction: yesterday = 10, today = 10. Add a −5 expense dated yesterday with "update balance" on. Before this fix, yesterday became 5 but today stayed 10. Expected: both become 5. ## Fix `ManualBalanceAdjuster` now shifts the transaction's own day **and every later snapshot** by the same delta, on both create and delete: - `applyCreatedTransaction` seeds a snapshot on the transaction's date from the carried-forward balance (when none exists yet), then increments that date and all later snapshots by `+amount`. - `reverseDeletedTransaction` now applies the exact inverse from the transaction's own date forward (previously it only ever adjusted *today*, which left the past day stale and would corrupt a create→delete round-trip of a back-dated transaction). The shared forward-shift is extracted into `shiftBalancesFrom(delta)`, used by both paths. ## Also in this PR (small UI fix) The balance history modal (`BalancesModal`) rendered each row's date/balance text top-aligned while the action icons were centered, because the base `TableCell` defaults to `align-top`. Center the data-row cells so every column lines up. Scoped to this modal — the shared `TableCell` is untouched. ## Notes / intended semantics - **Later user-set balances shift too.** Snapshots are treated as points on one running balance, so backfilling/removing a past transaction moves today's balance as well. That is exactly the reported expectation ("today should also change"). We deliberately do **not** try to distinguish "anchor" snapshots (a value the user reconciled by hand) from transaction-derived ones — there is no such distinction in the schema, and adding one is a separate product decision. - **Pre-existing, out of scope:** editing a transaction's amount/date never adjusts balances (`amount`/`transaction_date` aren't editable via `UpdateTransactionRequest`), and the create vs. delete "update balance" toggles are independent — creating with the box off but deleting with it on can over-correct. Neither is introduced here. ## Tests - New: creating a past-dated transaction updates that date **and** every later balance (the exact reported bug). - Rewrote the delete test to assert the reverse propagates from the transaction's date forward (the old test encoded the now-removed "always write today" behavior). - Full `TransactionTest` suite green (49 passed). ## QA Verified end-to-end in the running app on a manual account (yesterday = 10.00, today = 10.00): created a −5.00 expense dated yesterday with "update balance" on. The balance history modal shows **both** records — yesterday and today — drop from 10.00 to 5.00, and the DB confirms both snapshots = 500. ## Demo https://github.com/user-attachments/assets/18aa043d-37aa-4073-990c-a269c982a818 --- app/Services/ManualBalanceAdjuster.php | 81 ++++++++++--------- .../js/components/accounts/balances-modal.tsx | 5 +- tests/Feature/TransactionTest.php | 47 +++++++++-- 3 files changed, 87 insertions(+), 46 deletions(-) diff --git a/app/Services/ManualBalanceAdjuster.php b/app/Services/ManualBalanceAdjuster.php index 758ae109..88156f23 100644 --- a/app/Services/ManualBalanceAdjuster.php +++ b/app/Services/ManualBalanceAdjuster.php @@ -2,18 +2,17 @@ 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 { @@ -23,31 +22,20 @@ class ManualBalanceAdjuster return; } - $today = Carbon::now()->toDateString(); - - $currentBalance = $account->balances() - ->where('balance_date', '<=', $today) - ->orderByDesc('balance_date') - ->value('balance') ?? 0; - - AccountBalance::updateOrCreate( - [ - 'account_id' => $account->id, - 'balance_date' => $today, - ], - [ - 'balance' => $currentBalance - $transaction->amount, - ], + $this->shiftBalancesFrom( + $account, + $transaction->transaction_date->toDateString(), + -$transaction->amount, ); } /** - * Apply a newly created transaction to its manual account's balance. + * Apply a newly created transaction to its manual account's balances. * - * 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. + * 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 { @@ -59,19 +47,36 @@ class ManualBalanceAdjuster $transactionDate = $transaction->transaction_date->toDateString(); - $baseBalance = $account->balances() - ->where('balance_date', '<=', $transactionDate) + $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; - - AccountBalance::updateOrCreate( - [ - 'account_id' => $account->id, - 'balance_date' => $transactionDate, - ], - [ - 'balance' => $baseBalance + $transaction->amount, - ], - ); } } 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/tests/Feature/TransactionTest.php b/tests/Feature/TransactionTest.php index 25618126..b9a92b25 100644 --- a/tests/Feature/TransactionTest.php +++ b/tests/Feature/TransactionTest.php @@ -290,19 +290,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) @@ -311,8 +310,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, ]); }); @@ -421,6 +425,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]);