feat: per-month "set aside" toggle for deferred transactions #235
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
nidde/parenting-tool!235
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "copilot/discuss-deferred-payments-approach"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Deferred transactions had no way to acknowledge that the monthly accrual had been acted on (e.g. moved to savings). Adds a lightweight, purely informational toggle — no effect on ledger totals, balance, or any calculated figures.
Description
Backend
deferred_set_asidestable:(user_id, finance_template_id, year, month, set_aside_at)with unique constraint on the composite keyDeferredSetAsideController::toggle()— creates or deletes the record; returns{ is_set_aside, set_aside_at }POST /api/deferred/{financeTemplate}/{year}/{month}/toggle-set-aside(auth:sanctum)DeferredTransactionDTO now includesis_set_aside/set_aside_atintoArray()TransactionAggregatorServicebulk-loads set-aside records per period before constructing DTOs (avoids N+1)Frontend
TransactionRow.vue: replaces the static info tooltip on deferred rows with amdi-piggy-banktoggle button — green + full opacity when set aside, muted + 60% opacity when notCombinedMonthlyAccordion→CombinedLedger, wherehandleToggleSetAsidecalls the API and applies an optimistic store update viaupdateDeferredSetAside()route('api.deferred.toggle-set-aside', …)(Ziggy) rather than a hardcoded pathNo financial impact —
LedgerService,BalanceService, and all derived totals (deferred_budgeted,deferred_due,deferred_out) are unchanged. Set-aside is a tracking marker only.Note on PR diff scope: This branch was cut from
main(which includes PRs #198–#227) whiledevis currently at#197. TheBalanceService,LedgerService, and store changes visible in the diff are carry-along history from those upstream PRs — they are not part of the set-aside feature. The set-aside feature itself is contained in exactly 2 commits (3fd317cand7b00c86). If merging only the set-aside feature todevis desired, cherry-picking those 2 commits is the cleanest path.Type of Change
Testing
5 feature tests in
DeferredSetAsideControllerTest: mark, unmark, 403 cross-user, 401 unauthenticated, toggle idempotence (multiple round-trips produce exactly 1 DB row).Code Quality
CI/CD Checks
Additional Notes
Set-aside status is independent per
(template, year, month)tuple — marking March does not affect April. If a future requirement emerges to reflect set-aside amounts in balance calculations (e.g. subtract from disposable income), that would require targeted changes toBalanceServiceandLedgerService.Comment
@coderabbitai helpto get the list of available commands and usage tips.@copilot
Seems there's some work done already and there's a conflict. Will this interfere with your changes? Looks like the balance service might be affected
Pull request overview
Adds a per-month “set aside” marker for deferred transactions so users can track whether they’ve moved the monthly accrual (e.g., to savings) without affecting any financial totals.
Changes:
deferred_set_asidespersistence + toggle API endpoint (auth-protected) and wires DTO enrichment via bulk load in the aggregator.Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
is_set_aside/set_aside_at.deferred_set_asidestable with uniqueness constraint.is_set_aside/set_aside_atto DTO serialization.@ -0,0 +17,4 @@** Marks the monthly portion of a deferred expense as set aside (or unmarks it).*/public function toggle(Request $request, FinanceTemplate $financeTemplate, int $year, int $month): JsonResponseThe endpoint accepts arbitrary
year/monthroute params without validation. Sincemonthis persisted asunsignedTinyIntegerand the feature is defined as per-calendar-month, invalid values (e.g. 0, 13, negative, far-future years) can be stored. Add request validation (FormRequest or Validator) to enforcemonthin 1..12 and a sensibleyearrange (and return a 422 on invalid input).@ -0,0 +48,4 @@'year' => $year,'month' => $month,'set_aside_at' => now(),]);The toggle implementation is vulnerable to a race condition: two concurrent requests can both observe no existing row and attempt
create(), causing a unique-constraint violation and a 500 response. Consider making the operation resilient by using an atomic pattern (e.g., delete-then-create with a try/catch for duplicate key, orfirstOrCreate/updateOrCreatecombined with consistent return semantics) so concurrent/double-click requests remain safe.@ -654,0 +654,4 @@async function handleToggleSetAside({ transaction, year, month }) {// Extract template ID from the deferred transaction string ID: "deferred_{templateId}_{date}"const parts = String(transaction.id).split('_');const templateId = parts[1];Parsing
templateIdfrom the synthetic transactionidcouples the frontend to a specific string format. A more robust approach is to include an explicitfinance_template_id(or similar) field on the deferred transaction DTO and use that directly, avoiding fragile string parsing if the ID format changes.The comment says this is an 'optimistic local toggle', but
localIsSetAsidenever flips on click—it's only updated from props. Either (a) actually togglelocalIsSetAsideimmediately on click (and reconcile on API failure), or (b) adjust the comment/naming to reflect that it mirrors store state rather than being optimistic.This is an icon-only toggle button; relying on
title/tooltip alone is not sufficient for assistive tech. Add an explicitaria-label(and ideally:aria-pressed=\"localIsSetAside\"since it’s a toggle) so screen readers can understand the control and its state.This update scans every month’s transaction list to find a single ID. Since the toggle handler already has
year/month, consider passing a month key (or maintaining an ID→location index) so the update is O(monthTransactions) (or O(1)) instead of O(allVisibleTransactions).@ -83,2 +83,4 @@is_deferred_payment?: boolean;next_due_date?: string;is_set_aside?: boolean;set_aside_at?: string;set_aside_atcan benull(backend returns null when unmarked), but the type isstringwhen present. With strict null checks this is a type mismatch and can lead to incorrect handling. Update the type toset_aside_at?: string | null(or make it non-optional but nullable if it is always included for deferred entries).@ -0,0 +113,4 @@->assertUnauthorized();});it('does not allow the same month to be marked twice creating duplicate records', function () {This test name doesn’t match the behavior being exercised (it toggles mark→unmark→mark, rather than attempting to mark twice). Rename it to reflect what it verifies (e.g., that repeated toggles don’t leave duplicate rows / at most one row exists per key).
The set-aside feature itself does not touch
BalanceService,LedgerService, or any balance/ledger totals — those are purely a tracking marker.The
BalanceService/LedgerServicechanges you're seeing in the PR diff came from pre-existing main-branch work (#199, #200, #224, #227) that were already merged before this branch was created. Our branch was cut frommainwhiledevis still at#197, so the PR diff contains ~50 upstream commits of carry-along history. Our 2 set-aside commits only add:deferred_set_asidestable, model, controller, and routeis_set_aside/set_aside_atfields on theDeferredTransactionDTOTransactionAggregatorService(no financial logic change)TransactionRow.vue+ store action inledgerStoreNone of that changes any calculated amounts. If the plan is to merge only the set-aside feature to
dev(without the carry-along history), the cleanest path would be to cherry-pick the 2 commits (3fd317cand7b00c86) ontodevdirectly.@copilot resolve the merge conflicts in this pull request
@copilot
In Connection.php line 826:
SQLSTATE[42000]: Syntax error or access violation: 1059 Identifier name 'de
ferred_set_asides_user_id_finance_template_id_year_month_unique' is too lon
g (Connection: mysql, SQL: alter table
deferred_set_asidesadd uniquede ferred_set_asides_user_id_finance_template_id_year_month_unique(user_id,finance_template_id,year,month))In Connection.php line 570:
SQLSTATE[42000]: Syntax error or access violation: 1059 Identifier name 'de
ferred_set_asides_user_id_finance_template_id_year_month_unique' is too lon
g