mirror of
https://github.com/we-promise/sure.git
synced 2026-05-24 21:14:56 +00:00
Refs #895, discussion #1224. Adds a "Mark as recurring" entry point on the transfer detail drawer that creates a `RecurringTransaction` carrying both source and destination accounts. The recurring index, settings toggle (`recurring_transactions_disabled`), and projected upcoming feed all light up automatically once the data shape is there. Schema: * `destination_account_id` nullable FK to accounts. `on_delete: :cascade` matches #20251030172500's precedent for accounts FKs. The existing `account_id` FK is widened to cascade in the same migration so Family destruction with a recurring transfer doesn't FK-violate. * Two predicate-partitioned partial unique indexes per shape: non-transfer rows (`destination_account_id IS NULL`, original 5-column shape preserved) and transfer rows (6-column shape including the destination). Postgres treats NULLs as distinct in unique indexes, so widening would have broken non-transfer dedupe. * Two CHECK constraints enforcing transfer invariants in PostgreSQL: `chk_recurring_txns_transfer_requires_source` (destination implies source) and `chk_recurring_txns_transfer_distinct_accounts` (destination cannot equal source). Per CLAUDE.md "Enforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL". * `Account` gains an `inbound_recurring_transfers` inverse so the destroy chain reaches both ends. Controller / behaviour: * `transfers#mark_as_recurring` mirrors `transactions#mark_as_recurring`: i18n flashes (4 new keys: transfer_marked_as_recurring, transfer_already_exists, transfer_creation_failed, transfer_feature_disabled), `respond_to format.html`, `redirect_back_or_to transactions_path`, server-side gate on `recurring_transactions_disabled?`, and rescue both `RecordInvalid` and `RecordNotUnique` for the race window between the dedupe `find_by` and `create_from_transfer`. The `StandardError` rescue now logs the exception (class, message, transfer/family/user ids) before surfacing the generic flash so production failures aren't context-less. * `RecurringTransaction.accessible_by(user)` now requires destination_account_id (when present) to be in the user's accessible set, so a recurring transfer never leaks to a user without access to BOTH endpoints. * Model validation gains a `destination_account.blank?` branch in `transfer_endpoints_consistent` so a dangling `destination_account_id` (referenced row destroyed) surfaces as a normal validation error instead of an FK exception on save. * `Identifier` filter for transfer-kind transactions moved into SQL. UI: * Recurring index table and projected feed render transfer rows with the existing letter-avatar and the row's `name` field ("Transfer to {destination}"). No special pill or icon -- every row in `/recurring_transactions` is recurring by definition. Amount column on transfers uses `text-secondary` (muted-but-live) instead of the income/expense colour, since transfers are zero-net for the family. Out of scope (called out in the PR body): * Auto-creation of future Transfer rows on a schedule (discussion #1224's primary ask). Behaviour change vs the current projection-only model. * Auto-identification of recurring transfer pairs in `Identifier`. * Frequency model richer than `expected_day_of_month`. * `Cleaner` for recurring transfers (issue #1590 tracks this). Tests: * `RecurringTransaction#transfer?` predicate (with / without destination). * `transfer_endpoints_consistent`: rejects same source and destination, rejects dangling destination_account_id, rejects cross-family destination. * `RecurringTransaction.create_from_transfer` happy path; multi-currency variant stores source-side currency. * `projected_entry` exposes source / destination on transfer rows. * `Identifier` skips transfer-kind transactions; creates a pattern from expense halves while ignoring co-resident transfer halves. * Destroying the destination account cascades to inbound recurring transfers (FK + AR association). * Unique partial index still de-duplicates non-transfer rows after the destination_account_id widening. * `transfers#mark_as_recurring` happy path, idempotent on second call, rejected when `recurring_transactions_disabled`. Suite: 3261 / 0 / 0 / 24 on the latest upstream/main. Lint clean. Brakeman clean. Signed-off-by: Guillem Arias Fauste <gariasf@proton.me>
246 lines
7.3 KiB
Ruby
246 lines
7.3 KiB
Ruby
require "test_helper"
|
|
|
|
class TransfersControllerTest < ActionDispatch::IntegrationTest
|
|
setup do
|
|
sign_in users(:family_admin)
|
|
end
|
|
|
|
test "should get new" do
|
|
get new_transfer_url
|
|
assert_response :success
|
|
end
|
|
|
|
test "can create transfers" do
|
|
assert_difference "Transfer.count", 1 do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: accounts(:depository).id,
|
|
to_account_id: accounts(:credit_card).id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
name: "Test Transfer"
|
|
}
|
|
}
|
|
assert_enqueued_with job: SyncJob
|
|
end
|
|
end
|
|
|
|
test "can create transfer with custom exchange rate" do
|
|
usd_account = accounts(:depository)
|
|
eur_account = users(:family_admin).family.accounts.create!(
|
|
name: "EUR Account",
|
|
balance: 1000,
|
|
currency: "EUR",
|
|
accountable: Depository.new
|
|
)
|
|
|
|
assert_equal "USD", usd_account.currency
|
|
assert_equal "EUR", eur_account.currency
|
|
|
|
assert_difference "Transfer.count", 1 do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: usd_account.id,
|
|
to_account_id: eur_account.id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
exchange_rate: 0.92
|
|
}
|
|
}
|
|
end
|
|
|
|
transfer = Transfer.where(
|
|
"outflow_transaction_id IN (?) AND inflow_transaction_id IN (?)",
|
|
usd_account.transactions.pluck(:id),
|
|
eur_account.transactions.pluck(:id)
|
|
).last
|
|
assert_not_nil transfer
|
|
assert_equal "USD", transfer.outflow_transaction.entry.currency
|
|
assert_equal "EUR", transfer.inflow_transaction.entry.currency
|
|
assert_equal 100, transfer.outflow_transaction.entry.amount
|
|
assert_in_delta(-92, transfer.inflow_transaction.entry.amount, 0.01)
|
|
end
|
|
|
|
test "exchange_rate endpoint returns 400 when from currency is missing" do
|
|
get exchange_rate_url, params: {
|
|
to: "USD"
|
|
}
|
|
|
|
assert_response :bad_request
|
|
json_response = JSON.parse(response.body)
|
|
assert_equal "from and to currencies are required", json_response["error"]
|
|
end
|
|
|
|
test "exchange_rate endpoint returns 400 when to currency is missing" do
|
|
get exchange_rate_url, params: {
|
|
from: "EUR"
|
|
}
|
|
|
|
assert_response :bad_request
|
|
json_response = JSON.parse(response.body)
|
|
assert_equal "from and to currencies are required", json_response["error"]
|
|
end
|
|
|
|
test "exchange_rate endpoint returns 400 on invalid date format" do
|
|
get exchange_rate_url, params: {
|
|
from: "EUR",
|
|
to: "USD",
|
|
date: "not-a-date"
|
|
}
|
|
|
|
assert_response :bad_request
|
|
json_response = JSON.parse(response.body)
|
|
assert_equal "Invalid date format", json_response["error"]
|
|
end
|
|
|
|
test "exchange_rate endpoint returns rate for different currencies" do
|
|
ExchangeRate.expects(:find_or_fetch_rate)
|
|
.with(from: "USD", to: "EUR", date: Date.current)
|
|
.returns(OpenStruct.new(rate: 0.92))
|
|
|
|
get exchange_rate_url, params: {
|
|
from: "USD",
|
|
to: "EUR",
|
|
date: Date.current.to_s
|
|
}
|
|
|
|
assert_response :success
|
|
json_response = JSON.parse(response.body)
|
|
assert_equal 0.92, json_response["rate"]
|
|
end
|
|
|
|
test "exchange_rate endpoint returns error when exchange rate unavailable" do
|
|
ExchangeRate.expects(:find_or_fetch_rate)
|
|
.with(from: "USD", to: "EUR", date: Date.current)
|
|
.returns(nil)
|
|
|
|
get exchange_rate_url, params: {
|
|
from: "USD",
|
|
to: "EUR",
|
|
date: Date.current.to_s
|
|
}
|
|
|
|
assert_response :not_found
|
|
json_response = JSON.parse(response.body)
|
|
assert_equal "Exchange rate not found", json_response["error"]
|
|
end
|
|
|
|
test "cannot create transfer when exchange rate unavailable and no custom rate provided" do
|
|
usd_account = accounts(:depository)
|
|
eur_account = users(:family_admin).family.accounts.create!(
|
|
name: "EUR Account",
|
|
balance: 1000,
|
|
currency: "EUR",
|
|
accountable: Depository.new
|
|
)
|
|
|
|
ExchangeRate.stubs(:find_or_fetch_rate).returns(nil)
|
|
|
|
assert_no_difference "Transfer.count" do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: usd_account.id,
|
|
to_account_id: eur_account.id,
|
|
date: Date.current,
|
|
amount: 100
|
|
}
|
|
}
|
|
end
|
|
|
|
assert_response :unprocessable_entity
|
|
end
|
|
|
|
test "exchange_rate endpoint returns same_currency for matching currencies" do
|
|
get exchange_rate_url, params: {
|
|
from: "USD",
|
|
to: "USD"
|
|
}
|
|
|
|
assert_response :success
|
|
json_response = JSON.parse(response.body)
|
|
assert_equal true, json_response["same_currency"]
|
|
assert_equal 1.0, json_response["rate"]
|
|
end
|
|
|
|
test "soft deletes transfer" do
|
|
assert_difference -> { Transfer.count }, -1 do
|
|
delete transfer_url(transfers(:one))
|
|
end
|
|
end
|
|
|
|
test "can add notes to transfer" do
|
|
transfer = transfers(:one)
|
|
assert_nil transfer.notes
|
|
|
|
patch transfer_url(transfer), params: { transfer: { notes: "Test notes" } }
|
|
|
|
assert_redirected_to transactions_url
|
|
assert_equal "Transfer updated", flash[:notice]
|
|
assert_equal "Test notes", transfer.reload.notes
|
|
end
|
|
|
|
test "handles rejection without FrozenError" do
|
|
transfer = transfers(:one)
|
|
|
|
assert_difference "Transfer.count", -1 do
|
|
patch transfer_url(transfer), params: {
|
|
transfer: {
|
|
status: "rejected"
|
|
}
|
|
}
|
|
end
|
|
|
|
assert_redirected_to transactions_url
|
|
assert_equal "Transfer updated", flash[:notice]
|
|
|
|
# Verify the transfer was actually destroyed
|
|
assert_raises(ActiveRecord::RecordNotFound) do
|
|
transfer.reload
|
|
end
|
|
end
|
|
|
|
test "mark_as_recurring creates a recurring transfer" do
|
|
transfer = transfers(:one)
|
|
family = users(:family_admin).family
|
|
family.recurring_transactions.destroy_all
|
|
|
|
assert_difference -> { RecurringTransaction.where(family: family).count }, +1 do
|
|
post mark_as_recurring_transfer_url(transfer)
|
|
end
|
|
|
|
rt = RecurringTransaction.where(family: family).last
|
|
assert rt.transfer?
|
|
assert_equal transfer.outflow_transaction.entry.account, rt.account
|
|
assert_equal transfer.inflow_transaction.entry.account, rt.destination_account
|
|
assert rt.manual?
|
|
assert_equal I18n.t("recurring_transactions.transfer_marked_as_recurring"), flash[:notice]
|
|
assert_redirected_to transactions_path
|
|
end
|
|
|
|
test "mark_as_recurring is idempotent: second call flashes already-exists" do
|
|
transfer = transfers(:one)
|
|
family = users(:family_admin).family
|
|
family.recurring_transactions.destroy_all
|
|
|
|
post mark_as_recurring_transfer_url(transfer)
|
|
assert_equal I18n.t("recurring_transactions.transfer_marked_as_recurring"), flash[:notice]
|
|
|
|
assert_no_difference -> { RecurringTransaction.where(family: family).count } do
|
|
post mark_as_recurring_transfer_url(transfer)
|
|
end
|
|
assert_equal I18n.t("recurring_transactions.transfer_already_exists"), flash[:alert]
|
|
end
|
|
|
|
test "mark_as_recurring is rejected when recurring_transactions_disabled" do
|
|
transfer = transfers(:one)
|
|
family = users(:family_admin).family
|
|
family.update!(recurring_transactions_disabled: true)
|
|
family.recurring_transactions.destroy_all
|
|
|
|
assert_no_difference -> { RecurringTransaction.where(family: family).count } do
|
|
post mark_as_recurring_transfer_url(transfer)
|
|
end
|
|
assert_equal I18n.t("recurring_transactions.transfer_feature_disabled"), flash[:alert]
|
|
end
|
|
end
|