feat(budgets): count shared accounts at the owner's percentage (#786)
## Why #750 made a shared account count only your share of every transaction — on the dashboard and on the cashflow screen. Budgets were left out and shipped as a known gap: `budget_transactions.amount` is a snapshot written when a transaction is assigned, so a 50% joint account still spent **100%** of every expense against its budget. The same category could read €400 on the dashboard and €800 in Budgets. This closes that gap. ## What A €100 expense on an account you own 50% of now counts €50 towards your budgets. Because every budget reader funnels through `BudgetPeriod::spentAmount()` — a sum of those snapshots — weighing the snapshot covers the budget cards, the detail page, the spending chart, carry-over, the limit alert emails and the MCP tools in one move. - **Assignment writes the owner's share.** Both paths (the per-transaction listener and the historical backfill) go through one `recordSnapshot()`. - **Changing an account's share rewrites its history.** A single SQL `UPDATE` re-weighs every budget row of that account, in every period, past ones included. - **A migration re-weighs the rows written before this**, so existing shared accounts are correct on deploy instead of on the next edit. ## How The share is computed at the same two choke points #750 established: - **PHP** — `Transaction::ownerShareOf()` (extracted from `ConvertsTransactionCurrency`, which was doing the same null dance inline) feeds `BudgetTransactionService::recordSnapshot()`. - **SQL** — `BudgetTransactionService::reweighAccountSnapshots()` reuses `Transaction::OWNED_AMOUNT_SQL`, so the rounding matches what PHP would have written. A test pins both to the same answer on an uneven share. The re-weigh hangs off `Account::booted()` rather than `AccountController`, so a seeder, an artisan command or a future MCP write tool cannot silently skip it. It also clears the period's `close_to_limit_notified` / `over_limit_notified` flags, the same way a refund that drops a budget back under its limit does — otherwise a budget that fell out of "over limit" would stay claimed and never alert on the next real crossing. The owning account is eager loaded **withTrashed** everywhere the snapshot is written, because `OWNED_AMOUNT_SQL` joins `accounts` without the soft-delete scope; without it a transaction whose account was deleted would snapshot at 100% in PHP and at the real share in SQL. ## Deliberate boundaries - **Transaction rows still show the real bank amount** — #750's rule. The budget detail page is the one screen where a weighted total sits directly above its own itemised list, so it now says so in a line under the chart. The alert email had the same mismatch inside one message and now quotes your share. - **`carried_over_amount` is not re-derived.** It is a second snapshot taken when a period closes. `remainingAmount()` deliberately ignores it and the UI only types the field; it surfaces solely through MCP. Left alone rather than adding a second re-derivation path. - The migration's `down()` restores the full transaction amount, which is what those rows held — it cannot know a share an account no longer has. ## Testing `tests/Feature/SharedAccountOwnershipTest.php` gains the budget cases: assignment, the historical backfill, the re-weigh through the settings screen, PHP/SQL rounding parity on 33% of 3333, and the alert flags being cleared. `tests/Feature/WeighBudgetTransactionsMigrationTest.php` covers the backfill, including that it is idempotent and that it keeps refunds negative (the fix from `2026_02_24_193117`). Fixing that test needed the `BudgetTransaction` factory, which had been pointing at a `BudgetPeriodAllocation` model that no longer exists. Manual QA on real local data — budget "Miami Flight", account "Daily" set to 50%: | | Before | At 50% | Back at 100% | |---|---|---|---| | Miami Flight spent | €3,229.40 | **€1,955.79** | €3,229.40 | | Yearly Padel spent | €1,514.91 | €785.49 | €1,514.91 | €1,955.79 is €3,229.40 − €1,273.61, exactly half of the €2,547.24 that account had in the budget. The round trip lands back on the original figure to the cent, so the re-weigh is idempotent on real data too. ## Demo <!-- PLACEHOLDER: drag the QA video here --> https://github.com/user-attachments/assets/1fa5b98a-2a11-4cad-b927-496b434d3295
This commit is contained in:
Víctor Falcón
GitHub
parent
8af9b74fd3
commit
f4c21147f1
|
|
@ -56,7 +56,7 @@ class ReassignLabeledBudgetTransactions extends Command
|
|||
|
||||
// Silently: these are historical assignments, so a budget-limit email
|
||||
// would announce a threshold the user crossed weeks ago.
|
||||
$query->with('labels')->chunkById(200, function (Collection $transactions) use ($service): void {
|
||||
$query->with('labels', 'account')->chunkById(200, function (Collection $transactions) use ($service): void {
|
||||
foreach ($transactions as $transaction) {
|
||||
$service->assignTransaction($transaction, notify: false);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -497,7 +497,7 @@ class ResetDemoAccountCommand extends Command
|
|||
|
||||
private function assignTransactionsToBudgets(User $user): void
|
||||
{
|
||||
$transactions = $user->transactions()->get();
|
||||
$transactions = $user->transactions()->with('account')->get();
|
||||
$assignedCount = 0;
|
||||
$budgetAssignments = [];
|
||||
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ trait ConvertsTransactionCurrency
|
|||
$transaction->transaction_date->toDateString(),
|
||||
);
|
||||
|
||||
return $transaction->account?->shareOfAmount($converted) ?? $converted;
|
||||
return $transaction->ownerShareOf($converted);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -83,8 +83,11 @@ class BudgetNotificationEmail extends Mailable implements ShouldQueue
|
|||
'allocatedFormatted' => Money::format($allocated, $currency),
|
||||
'spentFormatted' => Money::format($spent, $currency),
|
||||
'remainingFormatted' => Money::format(abs($remaining), $currency),
|
||||
// The owner's share, not the full charge: every other figure in
|
||||
// this email is what the budget counted, so a shared account
|
||||
// would otherwise show €80 spent against a €40 rise.
|
||||
'transactionAmountFormatted' => $this->transaction
|
||||
? Money::format(abs((int) $this->transaction->amount), $currency)
|
||||
? Money::format(abs($this->transaction->ownerShareOf((int) $this->transaction->amount)), $currency)
|
||||
: null,
|
||||
// Aligns with the service: at exactly the limit we are "over".
|
||||
'isOverLimit' => $hasLimit && $spent >= $allocated,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ namespace App\Models;
|
|||
|
||||
use App\Enums\AccountType;
|
||||
use App\Models\Concerns\BelongsToSpace;
|
||||
use App\Services\BudgetTransactionService;
|
||||
use Database\Factories\AccountFactory;
|
||||
use Illuminate\Database\Eloquent\Casts\Attribute;
|
||||
use Illuminate\Database\Eloquent\Concerns\HasUuids;
|
||||
|
|
@ -59,6 +60,21 @@ class Account extends Model
|
|||
'linked_loan_account_id',
|
||||
];
|
||||
|
||||
/**
|
||||
* Budget amounts are snapshots taken when a transaction is assigned, so a
|
||||
* new ownership share only reaches the budgets that already counted this
|
||||
* account if something rewrites them. Hooked on the model rather than on
|
||||
* the settings controller so every write path is covered.
|
||||
*/
|
||||
protected static function booted(): void
|
||||
{
|
||||
static::updated(function (Account $account): void {
|
||||
if ($account->wasChanged('ownership_percentage')) {
|
||||
app(BudgetTransactionService::class)->reweighAccountSnapshots($account);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
protected function casts(): array
|
||||
{
|
||||
return [
|
||||
|
|
|
|||
|
|
@ -248,6 +248,16 @@ class Transaction extends Model
|
|||
return $query->whereNull('category_id')->whereNull('description_iv');
|
||||
}
|
||||
|
||||
/**
|
||||
* The owner's share of an amount held by this transaction's account, for
|
||||
* the row-by-row PHP paths. Falls back to the full amount when the account
|
||||
* is not loaded, so a partial select never silently zeroes the figure.
|
||||
*/
|
||||
public function ownerShareOf(int $amount): int
|
||||
{
|
||||
return $this->account?->shareOfAmount($amount) ?? $amount;
|
||||
}
|
||||
|
||||
/**
|
||||
* A transaction amount reduced to the owner's share of its account, for
|
||||
* SQL-side aggregates. Rounds per row, matching {@see Account::shareOfAmount()}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
namespace App\Services;
|
||||
|
||||
use App\Enums\CategoryType;
|
||||
use App\Models\Account;
|
||||
use App\Models\Budget;
|
||||
use App\Models\BudgetPeriod;
|
||||
use App\Models\BudgetTransaction;
|
||||
|
|
@ -30,8 +31,12 @@ class BudgetTransactionService
|
|||
return;
|
||||
}
|
||||
|
||||
// Ensure labels are available for matching (safe if already loaded).
|
||||
// Ensure labels are available for matching, and the account for the
|
||||
// ownership share (both safe if already loaded). The account is loaded
|
||||
// with trashed ones too, so this agrees with the SQL re-weigh, which
|
||||
// ignores the soft-delete scope as well.
|
||||
$transaction->loadMissing('labels');
|
||||
$transaction->loadMissing(['account' => fn ($query) => $query->withTrashed()]);
|
||||
|
||||
$matchingPeriodIds = $this->trackedPeriodIds($transaction, $userId);
|
||||
|
||||
|
|
@ -65,15 +70,7 @@ class BudgetTransactionService
|
|||
->delete();
|
||||
|
||||
foreach ($matchingPeriodIds as $periodId) {
|
||||
$budgetTransaction = BudgetTransaction::updateOrCreate(
|
||||
[
|
||||
'transaction_id' => $transaction->id,
|
||||
'budget_period_id' => $periodId,
|
||||
],
|
||||
[
|
||||
'amount' => -$transaction->amount,
|
||||
],
|
||||
);
|
||||
$budgetTransaction = $this->recordSnapshot($transaction, $periodId);
|
||||
|
||||
if ($budgetTransaction->wasRecentlyCreated) {
|
||||
$createdPeriodIds[] = $periodId;
|
||||
|
|
@ -118,6 +115,10 @@ class BudgetTransactionService
|
|||
$query = Transaction::query()
|
||||
->where('user_id', $budget->user_id)
|
||||
->whereBetween('transaction_date', [$period->start_date, $period->end_date])
|
||||
// The owning account weighs every snapshot; eager loaded so a
|
||||
// 500-row chunk does not turn into 500 account lookups, and with
|
||||
// trashed ones so it agrees with the SQL re-weigh.
|
||||
->with(['account' => fn ($query) => $query->withTrashed()])
|
||||
->withoutTrashed();
|
||||
|
||||
if ($budget->is_catch_all) {
|
||||
|
|
@ -143,17 +144,7 @@ class BudgetTransactionService
|
|||
// Process in chunks to prevent memory issues
|
||||
$query->chunk(500, function ($transactions) use ($period, &$assignedCount) {
|
||||
foreach ($transactions as $transaction) {
|
||||
$budgetTransaction = BudgetTransaction::updateOrCreate(
|
||||
[
|
||||
'transaction_id' => $transaction->id,
|
||||
'budget_period_id' => $period->id,
|
||||
],
|
||||
[
|
||||
'amount' => -$transaction->amount,
|
||||
],
|
||||
);
|
||||
|
||||
if ($budgetTransaction->wasRecentlyCreated) {
|
||||
if ($this->recordSnapshot($transaction, $period->id)->wasRecentlyCreated) {
|
||||
$assignedCount++;
|
||||
}
|
||||
}
|
||||
|
|
@ -162,6 +153,59 @@ class BudgetTransactionService
|
|||
return $assignedCount;
|
||||
}
|
||||
|
||||
/**
|
||||
* Record what a transaction contributes to a budget period: the owner's
|
||||
* share of it, flipped so an expense counts as positive spending.
|
||||
*
|
||||
* The amount is a snapshot taken here and never revisited, so a later
|
||||
* change to the account's share has to go through
|
||||
* {@see self::reweighAccountSnapshots()}.
|
||||
*/
|
||||
private function recordSnapshot(Transaction $transaction, string $budgetPeriodId): BudgetTransaction
|
||||
{
|
||||
return BudgetTransaction::updateOrCreate(
|
||||
[
|
||||
'transaction_id' => $transaction->id,
|
||||
'budget_period_id' => $budgetPeriodId,
|
||||
],
|
||||
[
|
||||
'amount' => -$transaction->ownerShareOf($transaction->amount),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-snapshot every budget row of an account after its ownership share
|
||||
* changed, in SQL so it stays one query no matter how much history the
|
||||
* account has. Uses {@see Transaction::OWNED_AMOUNT_SQL} so the rounding
|
||||
* matches what {@see self::recordSnapshot()} would have written.
|
||||
*
|
||||
* @return int the number of rows re-weighed
|
||||
*/
|
||||
public function reweighAccountSnapshots(Account $account): int
|
||||
{
|
||||
$reweighed = DB::table('budget_transactions')
|
||||
->join('transactions', 'transactions.id', '=', 'budget_transactions.transaction_id')
|
||||
->join('accounts', 'accounts.id', '=', 'transactions.account_id')
|
||||
->where('accounts.id', $account->id)
|
||||
->update(['budget_transactions.amount' => DB::raw('-('.Transaction::OWNED_AMOUNT_SQL.')')]);
|
||||
|
||||
// Every affected period now holds a different total, so the limit alerts
|
||||
// it already sent describe a state that no longer exists. Clearing the
|
||||
// flags lets the next crossing notify again, the same way a refund that
|
||||
// drops a budget back under its limit does.
|
||||
if ($reweighed > 0) {
|
||||
BudgetPeriod::query()
|
||||
->whereHas(
|
||||
'budgetTransactions.transaction',
|
||||
fn (Builder $query) => $query->where('account_id', $account->id),
|
||||
)
|
||||
->update(['close_to_limit_notified' => false, 'over_limit_notified' => false]);
|
||||
}
|
||||
|
||||
return $reweighed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Narrow a transaction query to what a catch-all budget absorbs: expenses
|
||||
* whose category and labels are not already tracked by another budget.
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
namespace Database\Factories;
|
||||
|
||||
use App\Models\BudgetPeriodAllocation;
|
||||
use App\Models\BudgetPeriod;
|
||||
use App\Models\BudgetTransaction;
|
||||
use App\Models\Transaction;
|
||||
use Illuminate\Database\Eloquent\Factories\Factory;
|
||||
|
|
@ -21,7 +21,7 @@ class BudgetTransactionFactory extends Factory
|
|||
{
|
||||
return [
|
||||
'transaction_id' => Transaction::factory(),
|
||||
'budget_period_allocation_id' => BudgetPeriodAllocation::factory(),
|
||||
'budget_period_id' => BudgetPeriod::factory(),
|
||||
'amount' => fake()->numberBetween(1000, 50000),
|
||||
];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,43 @@
|
|||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
/**
|
||||
* Budget amounts are snapshots written when a transaction is assigned, so
|
||||
* every row created before the assignment weighed the account's ownership
|
||||
* share still counts a shared account at 100%. Rewrite them once here.
|
||||
*
|
||||
* The expression matches `Transaction::OWNED_AMOUNT_SQL`; kept inline so
|
||||
* this stays a self-contained one-shot fix.
|
||||
*/
|
||||
public function up(): void
|
||||
{
|
||||
DB::statement('
|
||||
UPDATE budget_transactions
|
||||
JOIN transactions ON transactions.id = budget_transactions.transaction_id
|
||||
JOIN accounts ON accounts.id = transactions.account_id
|
||||
SET budget_transactions.amount = -round(transactions.amount * accounts.ownership_percentage / 100)
|
||||
WHERE accounts.ownership_percentage < 100
|
||||
');
|
||||
}
|
||||
|
||||
/**
|
||||
* Back to the full transaction amount, which is what these rows held. An
|
||||
* account moved back to 100% after `up()` ran is left alone: its rows are
|
||||
* already at face value and there is no record of the share they were
|
||||
* written with.
|
||||
*/
|
||||
public function down(): void
|
||||
{
|
||||
DB::statement('
|
||||
UPDATE budget_transactions
|
||||
JOIN transactions ON transactions.id = budget_transactions.transaction_id
|
||||
JOIN accounts ON accounts.id = transactions.account_id
|
||||
SET budget_transactions.amount = -transactions.amount
|
||||
WHERE accounts.ownership_percentage < 100
|
||||
');
|
||||
}
|
||||
};
|
||||
|
|
@ -1,7 +1,8 @@
|
|||
{
|
||||
"Apply it to the balance too, so only my share counts towards net worth": "Aplicarlo también al saldo, para que solo mi parte cuente en el patrimonio neto",
|
||||
"Enter the full amount held in the account. You own :percentage% of it, and that is the share shown everywhere else.": "Introduce el importe total que hay en la cuenta. Tú tienes el :percentage % y esa es la parte que se muestra en el resto de pantallas.",
|
||||
"For accounts you share with someone else. Income and expenses only count towards your figures by this percentage.": "Para cuentas que compartes con otra persona. Los ingresos y gastos solo cuentan en tus cifras en ese porcentaje.",
|
||||
"Each row shows the full amount charged by the bank. The totals above only count your share of the accounts you share with someone else.": "Cada línea muestra el importe completo que cobró el banco. Los totales de arriba solo cuentan tu parte de las cuentas que compartes con otra persona.",
|
||||
"For accounts you share with someone else. Income and expenses only count towards your figures by this percentage. Changing it also rewrites what this account has already spent in your budgets, past periods included.": "Para cuentas que compartes con otra persona. Los ingresos y gastos solo cuentan en tus cifras en ese porcentaje. Cambiarlo también reescribe lo que esta cuenta ya ha gastado en tus presupuestos, incluidos los periodos pasados.",
|
||||
"My share of this account (%)": "Mi parte de esta cuenta (%)",
|
||||
"Set this up on a computer": "Configúralo desde un ordenador",
|
||||
"Signing in and approving works fine in a desktop browser, but usually breaks in a phone's in-app browser. Once it's connected, you can chat with Whisper Money from Claude or ChatGPT on your phone as usual.": "El inicio de sesión y la aprobación funcionan bien en un navegador de escritorio, pero suelen fallar en el navegador interno del móvil. Una vez conectado, puedes chatear con Whisper Money desde Claude o ChatGPT en el móvil con normalidad.",
|
||||
|
|
|
|||
|
|
@ -376,7 +376,7 @@ export function EditAccountDialog({
|
|||
className="text-xs text-muted-foreground"
|
||||
>
|
||||
{__(
|
||||
'For accounts you share with someone else. Income and expenses only count towards your figures by this percentage.',
|
||||
'For accounts you share with someone else. Income and expenses only count towards your figures by this percentage. Changing it also rewrites what this account has already spent in your budgets, past periods included.',
|
||||
)}
|
||||
</p>
|
||||
<InputError
|
||||
|
|
|
|||
|
|
@ -91,6 +91,17 @@ export default function BudgetShow({
|
|||
);
|
||||
}, [currentPeriod]);
|
||||
|
||||
// The rows below show what the bank charged while the totals above count
|
||||
// only the user's share, so say so when the two can differ.
|
||||
const hasSharedAccountTransactions = useMemo(
|
||||
() =>
|
||||
periodTransactions.some(
|
||||
(transaction) =>
|
||||
(transaction.account?.ownership_percentage ?? 100) < 100,
|
||||
),
|
||||
[periodTransactions],
|
||||
);
|
||||
|
||||
return (
|
||||
<AppSidebarLayout
|
||||
breadcrumbs={breadcrumbs}
|
||||
|
|
@ -204,16 +215,25 @@ export default function BudgetShow({
|
|||
</div>
|
||||
</div>
|
||||
) : (
|
||||
<TransactionList
|
||||
categories={categories}
|
||||
accounts={accounts}
|
||||
banks={banks}
|
||||
labels={labels}
|
||||
transactions={periodTransactions}
|
||||
pageSize={10}
|
||||
showActionsMenu={false}
|
||||
maxHeight={600}
|
||||
/>
|
||||
<>
|
||||
{hasSharedAccountTransactions && (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{__(
|
||||
'Each row shows the full amount charged by the bank. The totals above only count your share of the accounts you share with someone else.',
|
||||
)}
|
||||
</p>
|
||||
)}
|
||||
<TransactionList
|
||||
categories={categories}
|
||||
accounts={accounts}
|
||||
banks={banks}
|
||||
labels={labels}
|
||||
transactions={periodTransactions}
|
||||
pageSize={10}
|
||||
showActionsMenu={false}
|
||||
maxHeight={600}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -5,9 +5,12 @@ use App\Enums\CategoryType;
|
|||
use App\Models\Account;
|
||||
use App\Models\AccountBalance;
|
||||
use App\Models\Bank;
|
||||
use App\Models\Budget;
|
||||
use App\Models\BudgetPeriod;
|
||||
use App\Models\Category;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
use App\Services\BudgetTransactionService;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
|
||||
beforeEach(function () {
|
||||
|
|
@ -255,6 +258,122 @@ test('updating an account persists its ownership settings', function () {
|
|||
->and($account->fresh()->ownership_applies_to_balance)->toBeTrue();
|
||||
});
|
||||
|
||||
/**
|
||||
* A budget tracking one expense category over the current month, plus a shared
|
||||
* account holding a single expense in that category. The transaction is not
|
||||
* assigned yet, so each test decides which assignment path writes the snapshot.
|
||||
*
|
||||
* @return array{account: Account, period: BudgetPeriod, transaction: Transaction}
|
||||
*/
|
||||
function sharedAccountExpenseAndBudget(User $user, int $percentage, int $amount = -80000): array
|
||||
{
|
||||
$account = Account::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'type' => AccountType::Checking,
|
||||
'currency_code' => 'USD',
|
||||
'ownership_percentage' => $percentage,
|
||||
]);
|
||||
|
||||
$category = Category::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'type' => CategoryType::Expense,
|
||||
]);
|
||||
|
||||
$budget = Budget::factory()->forCategories($category)->create([
|
||||
'user_id' => $user->id,
|
||||
]);
|
||||
|
||||
$period = BudgetPeriod::factory()->create([
|
||||
'budget_id' => $budget->id,
|
||||
'start_date' => now()->startOfMonth(),
|
||||
'end_date' => now()->endOfMonth(),
|
||||
'allocated_amount' => 100000,
|
||||
]);
|
||||
|
||||
$transaction = Transaction::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'account_id' => $account->id,
|
||||
'category_id' => $category->id,
|
||||
'amount' => $amount,
|
||||
'currency_code' => 'USD',
|
||||
'transaction_date' => now(),
|
||||
]);
|
||||
|
||||
return ['account' => $account, 'period' => $period, 'transaction' => $transaction];
|
||||
}
|
||||
|
||||
test('a budget counts only the owner share of a transaction on a shared account', function () {
|
||||
['period' => $period, 'transaction' => $transaction] = sharedAccountExpenseAndBudget($this->user, 50);
|
||||
|
||||
app(BudgetTransactionService::class)->assignTransaction($transaction);
|
||||
|
||||
expect($period->spentAmount())->toBe(40000)
|
||||
->and($period->remainingAmount())->toBe(60000);
|
||||
});
|
||||
|
||||
test('a fully owned account still counts in full towards a budget', function () {
|
||||
['period' => $period, 'transaction' => $transaction] = sharedAccountExpenseAndBudget($this->user, 100);
|
||||
|
||||
app(BudgetTransactionService::class)->assignTransaction($transaction);
|
||||
|
||||
expect($period->spentAmount())->toBe(80000);
|
||||
});
|
||||
|
||||
test('the historical backfill counts only the owner share of a shared account', function () {
|
||||
['period' => $period] = sharedAccountExpenseAndBudget($this->user, 50);
|
||||
|
||||
app(BudgetTransactionService::class)->assignHistoricalTransactionsToPeriod($period);
|
||||
|
||||
expect($period->spentAmount())->toBe(40000);
|
||||
});
|
||||
|
||||
test('changing an account share re-weighs the budget amounts already recorded', function () {
|
||||
['account' => $account, 'period' => $period, 'transaction' => $transaction] = sharedAccountExpenseAndBudget($this->user, 100);
|
||||
|
||||
app(BudgetTransactionService::class)->assignTransaction($transaction);
|
||||
expect($period->spentAmount())->toBe(80000);
|
||||
|
||||
$this->patch(route('accounts.update', $account), [
|
||||
'name' => $account->name,
|
||||
'type' => AccountType::Checking->value,
|
||||
'currency_code' => 'USD',
|
||||
'ownership_percentage' => 50,
|
||||
])->assertRedirect();
|
||||
|
||||
expect($period->spentAmount())->toBe(40000);
|
||||
});
|
||||
|
||||
/**
|
||||
* Budget amounts are written in PHP on assignment and rewritten in SQL when the
|
||||
* share changes. An amount that does not divide evenly is the only thing that
|
||||
* catches the two rounding rules drifting apart.
|
||||
*/
|
||||
test('the PHP and SQL budget paths round an uneven share identically', function () {
|
||||
['account' => $account, 'period' => $period, 'transaction' => $transaction] = sharedAccountExpenseAndBudget($this->user, 33, amount: -3333);
|
||||
|
||||
app(BudgetTransactionService::class)->assignTransaction($transaction);
|
||||
expect($period->spentAmount())->toBe(1100);
|
||||
|
||||
// Round-trip through 100% and back so the SQL path recomputes the same 33%.
|
||||
$account->update(['ownership_percentage' => 100]);
|
||||
expect($period->spentAmount())->toBe(3333);
|
||||
|
||||
$account->update(['ownership_percentage' => 33]);
|
||||
expect($period->spentAmount())->toBe(1100);
|
||||
});
|
||||
|
||||
test('a re-weigh lets a budget notify again on the next crossing', function () {
|
||||
['account' => $account, 'period' => $period, 'transaction' => $transaction] = sharedAccountExpenseAndBudget($this->user, 100, amount: -120000);
|
||||
|
||||
app(BudgetTransactionService::class)->assignTransaction($transaction);
|
||||
$period->update(['over_limit_notified' => true, 'close_to_limit_notified' => true]);
|
||||
|
||||
$account->update(['ownership_percentage' => 50]);
|
||||
|
||||
expect($period->fresh()->over_limit_notified)->toBeFalse()
|
||||
->and($period->fresh()->close_to_limit_notified)->toBeFalse();
|
||||
});
|
||||
|
||||
test('the ownership percentage must stay between 1 and 100', function () {
|
||||
$bank = Bank::factory()->create();
|
||||
$account = Account::factory()->create([
|
||||
|
|
|
|||
|
|
@ -0,0 +1,62 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Account;
|
||||
use App\Models\BudgetPeriod;
|
||||
use App\Models\BudgetTransaction;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
|
||||
function runWeighBudgetTransactionsMigration(): void
|
||||
{
|
||||
$migration = require database_path('migrations/2026_08_12_100000_weigh_budget_transactions_by_account_ownership.php');
|
||||
$migration->up();
|
||||
}
|
||||
|
||||
/**
|
||||
* A budget row holding the full transaction amount, the way every row written
|
||||
* before the ownership weighting looks.
|
||||
*/
|
||||
function unweighedBudgetRow(User $user, int $percentage, int $amount): BudgetTransaction
|
||||
{
|
||||
$account = Account::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'ownership_percentage' => $percentage,
|
||||
]);
|
||||
|
||||
$transaction = Transaction::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'account_id' => $account->id,
|
||||
'amount' => $amount,
|
||||
]);
|
||||
|
||||
return BudgetTransaction::factory()->create([
|
||||
'transaction_id' => $transaction->id,
|
||||
'budget_period_id' => BudgetPeriod::factory()->create()->id,
|
||||
'amount' => -$amount,
|
||||
]);
|
||||
}
|
||||
|
||||
it('re-weighs only the rows of shared accounts', function () {
|
||||
$user = User::factory()->create();
|
||||
|
||||
$shared = unweighedBudgetRow($user, 50, -80000);
|
||||
$refundOnShared = unweighedBudgetRow($user, 50, 20000);
|
||||
$fullyOwned = unweighedBudgetRow($user, 100, -80000);
|
||||
|
||||
runWeighBudgetTransactionsMigration();
|
||||
|
||||
expect($shared->fresh()->amount)->toBe(40000)
|
||||
// A refund stays negative, keeping the fix from the 2026_02_24 migration.
|
||||
->and($refundOnShared->fresh()->amount)->toBe(-10000)
|
||||
->and($fullyOwned->fresh()->amount)->toBe(80000);
|
||||
});
|
||||
|
||||
it('is idempotent, so a second run does not halve the amounts again', function () {
|
||||
$user = User::factory()->create();
|
||||
$shared = unweighedBudgetRow($user, 50, -80000);
|
||||
|
||||
runWeighBudgetTransactionsMigration();
|
||||
runWeighBudgetTransactionsMigration();
|
||||
|
||||
expect($shared->fresh()->amount)->toBe(40000);
|
||||
});
|
||||
Loading…
Reference in New Issue