* fix: disable "Mark as Recurring" button when a manual recurring transaction already exists
Previously the button was always clickable and only failed after a POST,
showing "A manual recurring transaction already exists for this pattern".
Extract the lookup into Transaction#existing_manual_recurring_transaction
(reused by the controller guard) so the view can disable the button ahead
of time and show the reason inline.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: address CodeRabbit review feedback on PR #3103
Move the existing_manual_recurring lookup out of the show view and into
the controller so rendering no longer runs an Active Record query
in-template, and strengthen the "no match" model test with near-match
recurring transactions that individually differ by account, merchant,
amount, currency, and manual flag.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: move mark-recurring presentation state fully into controller, fix stale state on failed update
Address CodeRabbit follow-up on PR #3103:
- Compute the mark-recurring button's subtitle text/class, href, disabled
state, title, and class entirely in TransactionsController (via a shared
assign_mark_recurring_state helper) instead of deriving them with
ternaries in the view.
- Populate that state before TransactionsController#update re-renders
:show on a failed entry update, so the button doesn't incorrectly appear
enabled when a matching manual recurring transaction exists.
- Add a controller test covering the failed-update render path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: use blank name instead of blank date to trigger validation failure in mark-recurring test
The CI test_unit run flagged a real bug in the test itself: TransactionsController#entry_params
strips blank :date/:amount before update, so date: "" never reached model validation and the
update succeeded (302) instead of failing (422) as the test expected. Use a blank :name instead,
which isn't stripped, and add DOM assertions (disabled button, no mark_as_recurring form action)
per CodeRabbit's follow-up review.
Verified against a live NAS Rails console reproduction (bypassing the test stack's broken
fixtures) that the failed-update render now correctly shows the button as disabled with the
"already exists" message and no action link.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: use transaction id instead of entry id in mark-recurring route assertion
mark_as_recurring is a member route on the transactions resource keyed by the
Transaction's id, not the Entry's id (Entry uses delegated_type, so Entry and
its Transaction entryable have distinct ids). The prior assertion built the
path from `entry`, which could produce a different URL than the one actually
rendered, so the "no href" check could pass even if the button leaked a link.
Use entry.entryable so the assertion matches the real route.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: refresh mark-recurring state on turbo_stream update, avoid unconditional query, keep DS::Button href
Addresses jjmata's review on PR #3103:
- Extract the "Mark as Recurring" block into a dom_id-wrapped partial and
replace it in the successful update turbo_stream response, so inline
edits that change whether the transaction matches an existing manual
recurring transaction are reflected immediately instead of only on the
next full page render.
- Skip the existing_manual_recurring_transaction lookup entirely when the
block won't be rendered (no edit permission, or split-child entry),
avoiding an unconditional extra query on every transaction show/failed
update.
- Keep href present on the DS::Button and only toggle disabled, matching
the established pattern elsewhere in the app, instead of nulling href
(which flips the component to a bare <button> and leaks a stray
method="post" attribute).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Gerald <248542187+gfr-free@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>