RomanNum3ral
/
whisper-money
mirror of https://github.com/whisper-money/whisper-money.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(balances): propagate retroactive transaction changes to later balances (#682) 

## 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
This commit is contained in:
Víctor Falcón 2026-07-16 08:38:02 +02:00 committed by GitHub
parent c7719fe5ad
commit 9312d24f93
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 87 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
app/Services/ManualBalanceAdjuster.php
View File

@ -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,
],
);
}
}

5
resources/js/components/accounts/balances-modal.tsx
View File

@ -312,7 +312,10 @@ export function BalancesModal({
</TableRow>
) : (
balances.map((balance) => (
<TableRow key={balance.id}>
<TableRow
key={balance.id}
className="[&>td]:align-middle"
>
<TableCell>
{formatDate(
balance.balance_date,

47
tests/Feature/TransactionTest.php
View File

@ -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]);