From a873582191574b4a4192b26ec3d48687aaa1bea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Thu, 16 Jul 2026 08:59:10 +0200 Subject: [PATCH] feat(transactions): allow editing all fields of manual transactions (#683) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What & why Until now only the **category, notes, labels and (for manual transactions) the description** could be changed after a transaction was created — the **amount and date were immutable and the account was create-only**. Users creating transactions by hand had no way to fix a wrong amount or date. This lets **manually created transactions edit every field at any time after creation**: account, date, description and amount, on top of category/labels/notes. **Imported / bank-synced transactions keep amount, date, account, currency and description locked** to their source data. ## Changes **Backend** - `UpdateTransactionRequest` now validates `amount`, `transaction_date`, `account_id` and `currency_code` **only when the transaction's `source` is `manually_created`**. For imported transactions those keys are not validated, so `validated()` drops them and they can't be changed even via a crafted request. - `TransactionController::update()` moves the manual account balance to match an edited amount/date/account, **opt-in via the same `update_balance` flag used by create/delete**. It snapshots the pre-edit state, and only rebalances when one of amount/date/account actually changed. Connected accounts are skipped inside the adjuster. - `ManualBalanceAdjuster` was refactored to a shared private `adjust()` primitive (removing duplication between the existing create/delete paths) and gains `reverseCreatedTransaction()` so an edit can reverse the old contribution and apply the new one. **Frontend** - `edit-transaction-dialog.tsx` unifies the create/edit editability decision behind a single `canEditAllFields` flag and renders the account, date and amount inputs (plus the "update account balance" checkbox) when editing a manual transaction. - `transaction-sync.ts` `update()` gains an `{ updateBalance }` option, mirroring `create()`. ## Known limitation (by design) Like create/delete, the balance update trusts the opt-in flag and nudges a **single dated snapshot** — it doesn't cascade to later snapshots and keeps no record of whether creation adjusted the balance. So it's exact for the common case (recent transaction, flag used consistently) and can drift otherwise. This matches the app's existing snapshot-based balance model; a transaction-derived balance would be a separate, larger change. Documented with a `ponytail:` comment at the call site. ## Tests - **Feature (`tests/Feature/TransactionTest.php`)**: manual transaction edits amount/date/account/currency; imported transaction cannot; balance moves by the delta on amount change; no change when not requested; moving between accounts reverses the old and credits the new; currency-only edit doesn't rebalance; connected accounts never change. All green locally (55 passed). - **Component (`edit-transaction-dialog.test.tsx`)**: manual transaction shows editable amount/date/description; imported keeps them read-only. ## QA Real browser QA (Playwright) against the running app with the `demo@whisper.money` data: - Opened a manual transaction → edited date, description and amount, kept "update account balance" checked, saved. Verified in the DB: `amount -4200 → -7550`, `date 2026-07-10 → 2026-07-12`, description updated, and the account balance snapshots moved accordingly. - Opened an imported transaction → amount and description render read-only (locked). ## Demo https://github.com/user-attachments/assets/1c8790e8-31f5-4283-b260-353650ad007c --- .../Controllers/TransactionController.php | 18 +- .../Requests/UpdateTransactionRequest.php | 18 +- app/Models/Transaction.php | 1 + lang/es.json | 1 + .../edit-transaction-dialog.test.tsx | 89 ++++++++ .../transactions/edit-transaction-dialog.tsx | 98 ++++++--- resources/js/services/transaction-sync.ts | 2 + tests/Feature/TransactionTest.php | 208 ++++++++++++++++++ 8 files changed, 399 insertions(+), 36 deletions(-) diff --git a/app/Http/Controllers/TransactionController.php b/app/Http/Controllers/TransactionController.php index 3df66dec..da086fc8 100644 --- a/app/Http/Controllers/TransactionController.php +++ b/app/Http/Controllers/TransactionController.php @@ -213,7 +213,7 @@ class TransactionController extends Controller ], 201); } - public function update(UpdateTransactionRequest $request, Transaction $transaction): JsonResponse + public function update(UpdateTransactionRequest $request, Transaction $transaction, ManualBalanceAdjuster $balanceAdjuster): JsonResponse { $this->authorize('update', $transaction); @@ -238,6 +238,10 @@ class TransactionController extends Controller } } + // Snapshot the pre-edit account/date/amount before filling, so a manual + // account balance can be moved off the old values if the edit changes them. + $originalSnapshot = clone $transaction; + // Update attributes directly without firing events yet if (! empty($data)) { $transaction->fill($data); @@ -260,6 +264,18 @@ class TransactionController extends Controller $transaction->save(); } + // 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->reverseDeletedTransaction($originalSnapshot); + $balanceAdjuster->applyCreatedTransaction($transaction->load('account')); + } + return response()->json([ 'data' => $transaction->fresh()->load('labels'), 'learned_rule' => $learnedRule === null ? null : [ diff --git a/app/Http/Requests/UpdateTransactionRequest.php b/app/Http/Requests/UpdateTransactionRequest.php index 1fca7a75..d77dde52 100644 --- a/app/Http/Requests/UpdateTransactionRequest.php +++ b/app/Http/Requests/UpdateTransactionRequest.php @@ -2,7 +2,9 @@ namespace App\Http\Requests; +use App\Enums\TransactionSource; use App\Http\Requests\Concerns\ValidatesUserOwnedResources; +use App\Models\Transaction; use Illuminate\Foundation\Http\FormRequest; class UpdateTransactionRequest extends FormRequest @@ -16,7 +18,7 @@ class UpdateTransactionRequest extends FormRequest public function rules(): array { - return [ + $rules = [ 'category_id' => ['nullable', $this->userOwned('categories')], 'description' => ['sometimes', 'string'], 'description_iv' => ['nullable', 'string', 'size:16'], @@ -27,6 +29,20 @@ class UpdateTransactionRequest extends FormRequest 'label_ids' => ['nullable', 'array'], 'label_ids.*' => ['required', 'string', 'uuid', $this->userOwned('labels')], ]; + + // Manually created transactions can edit every field after creation. + // Imported ones keep amount, date, account and currency locked to the + // source data, so those keys are only validated (and thus persisted) + // for manual transactions. + $transaction = $this->route('transaction'); + if ($transaction instanceof Transaction && $transaction->source === TransactionSource::ManuallyCreated) { + $rules['account_id'] = ['sometimes', $this->userOwned('accounts')]; + $rules['transaction_date'] = ['sometimes', 'date']; + $rules['amount'] = ['sometimes', 'integer']; + $rules['currency_code'] = ['sometimes', 'string', 'size:3']; + } + + return $rules; } public function messages(): array diff --git a/app/Models/Transaction.php b/app/Models/Transaction.php index fb9dccdd..84d3b169 100644 --- a/app/Models/Transaction.php +++ b/app/Models/Transaction.php @@ -26,6 +26,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; /** * @property Carbon $transaction_date * @property int|float $total_amount + * @property TransactionSource $source * @property ?CategorySource $category_source * @property ?float $ai_confidence * @property ?string $categorized_by_rule_id diff --git a/lang/es.json b/lang/es.json index 66138f8e..8facad96 100644 --- a/lang/es.json +++ b/lang/es.json @@ -1745,6 +1745,7 @@ "Update the owed amount record.": "Actualiza el registro de monto adeudado.", "Update the rule to automatically categorize transactions and add labels.": "Actualiza la regla para categorizar automáticamente transacciones y agregar etiquetas.", "Update the rule to automatically categorize transactions.": "Actualiza la regla para categorizar transacciones automáticamente.", + "Update this transaction.": "Actualiza esta transacción.", "Update your account's appearance settings": "Actualiza la configuración de apariencia de tu cuenta", "Update your budget settings. To change the allocated amount or tracking, use the budget page directly.": "Actualiza la configuración de tu presupuesto. Para cambiar el valor asignado o el seguimiento, usa la página de presupuesto directamente.", "Update your budget settings. To change the allocated\\n amount or tracking, use the budget page directly.": "Actualiza la configuración de tu presupuesto. Para cambiar el monto asignado o el seguimiento, usa la página de presupuesto directamente.", diff --git a/resources/js/components/transactions/edit-transaction-dialog.test.tsx b/resources/js/components/transactions/edit-transaction-dialog.test.tsx index d4814261..c287bb16 100644 --- a/resources/js/components/transactions/edit-transaction-dialog.test.tsx +++ b/resources/js/components/transactions/edit-transaction-dialog.test.tsx @@ -229,6 +229,95 @@ describe('EditTransactionDialog', () => { expect(screen.getByRole('checkbox')).toBeChecked(); }); + it('lets you edit every field of a manually created transaction', () => { + render( + , + ); + + // Amount, date and description render as editable inputs, not read-only text. + expect(screen.getByPlaceholderText('25.00')).toBeInTheDocument(); + expect( + screen.getByPlaceholderText('Transaction description'), + ).toBeInTheDocument(); + expect( + document.querySelector('input[type="date"]'), + ).toBeInTheDocument(); + }); + + it('keeps amount and description read-only for an imported transaction', () => { + render( + , + ); + + expect(screen.queryByPlaceholderText('25.00')).not.toBeInTheDocument(); + expect( + screen.queryByPlaceholderText('Transaction description'), + ).not.toBeInTheDocument(); + expect( + document.querySelector('input[type="date"]'), + ).not.toBeInTheDocument(); + }); + it('hides "update account balance" for a connected account', () => { const connectedAccount = { ...checkingAccount, diff --git a/resources/js/components/transactions/edit-transaction-dialog.tsx b/resources/js/components/transactions/edit-transaction-dialog.tsx index 20194352..db8a673a 100644 --- a/resources/js/components/transactions/edit-transaction-dialog.tsx +++ b/resources/js/components/transactions/edit-transaction-dialog.tsx @@ -105,6 +105,11 @@ export function EditTransactionDialog({ return true; }); + // Manually created transactions can edit every field (account, date, amount, + // description) both on creation and afterwards. Imported ones keep those locked. + const canEditAllFields = + mode === 'create' || transaction?.source === 'manually_created'; + useEffect(() => { if (mode === 'edit' && transaction) { setTransactionDate(transaction.transaction_date); @@ -135,7 +140,7 @@ export function EditTransactionDialog({ }, [mode, transaction, open, accounts, initialAccountId]); useEffect(() => { - if (!open || mode !== 'create') return; + if (!open || !canEditAllFields) return; async function decryptAccountNames() { const keyString = getStoredKey(); @@ -185,7 +190,7 @@ export function EditTransactionDialog({ } decryptAccountNames(); - }, [open, mode, accounts]); + }, [open, canEditAllFields, accounts]); async function checkAndApplyAutomationRules() { if (mode !== 'create' || automationRules.length === 0) { @@ -273,7 +278,7 @@ export function EditTransactionDialog({ async function handleSubmit(e: React.FormEvent) { e.preventDefault(); - if (mode === 'create') { + if (canEditAllFields) { if (!description.trim()) { toast.error(__('Description is required')); return; @@ -290,14 +295,6 @@ export function EditTransactionDialog({ toast.error(__('Date is required')); return; } - } else if ( - mode === 'edit' && - transaction?.source === 'manually_created' - ) { - if (!description.trim()) { - toast.error(__('Description is required')); - return; - } } setIsSubmitting(true); @@ -423,6 +420,10 @@ export function EditTransactionDialog({ description?: string; description_iv?: string | null; label_ids?: string[]; + amount?: number; + transaction_date?: string; + account_id?: string; + currency_code?: string; } = { category_id: selectedCategoryId, notes: encryptedNotes, @@ -433,18 +434,34 @@ export function EditTransactionDialog({ let finalDecryptedDescription = transaction.decryptedDescription; - if ( - transaction.source === 'manually_created' && - trimmedDescription - ) { + const editedAccount = accounts.find( + (acc) => acc.id === accountId, + ); + const editedCurrencyCode = + editedAccount?.currency_code ?? transaction.currency_code; + + if (canEditAllFields) { updateData.description = trimmedDescription; updateData.description_iv = null; finalDecryptedDescription = trimmedDescription; + updateData.amount = amount; + updateData.transaction_date = transactionDate; + updateData.account_id = accountId; + updateData.currency_code = editedCurrencyCode; } const result = await transactionSyncService.update( transaction.id, updateData, + { + // Gate on the transaction being manual, not on the target + // account: the backend adjuster skips connected accounts + // per-account, so this still reverses the old manual + // account when the edit moves it onto a connected one. + updateBalance: canEditAllFields + ? updateAccountBalance + : false, + }, ); const updatedRecord = await transactionSyncService.getById( @@ -476,6 +493,20 @@ export function EditTransactionDialog({ labels: selectedLabels, updated_at: updatedRecord?.updated_at ?? transaction.updated_at, + ...(canEditAllFields + ? { + amount, + transaction_date: transactionDate, + account_id: accountId, + currency_code: editedCurrencyCode, + account: editedAccount ?? transaction.account, + bank: editedAccount?.bank?.id + ? banks.find( + (b) => b.id === editedAccount.bank?.id, + ) + : transaction.bank, + } + : {}), }; toast.success(__('Transaction updated successfully')); @@ -549,15 +580,17 @@ export function EditTransactionDialog({ {mode === 'create' ? __('Create a new transaction.') - : __( - 'Update the category and notes for this transaction.', - )} + : canEditAllFields + ? __('Update this transaction.') + : __( + 'Update the category and notes for this transaction.', + )}
- {mode === 'create' && ( + {canEditAllFields && (
{__('Account')} @@ -597,14 +630,14 @@ export function EditTransactionDialog({ {__('Date')} - {mode === 'create' ? ( + {canEditAllFields ? ( {__('Description')} - {mode === 'create' || - (mode === 'edit' && - transaction?.source === 'manually_created') ? ( + {canEditAllFields ? (