mirror of
https://github.com/we-promise/sure.git
synced 2026-09-09 08:34:26 +00:00
* fix(transfers): prevent duplicate creation on double-submit TransfersController#create -> Transfer::Creator had no protection against a repeated form submission - a double-click, a browser retry, or two near-simultaneous requests could each create a separate, identical transfer (and its 2-4 underlying Entry/Transaction rows). Adds a per-form idempotency key, the same approach already used for TransactionsController#create: a UUID hidden field generated fresh on page load, tagging the outflow/inflow (and fee, with a distinguishing suffix since a fee leg shares its account with its primary leg) entries via the existing entries(account_id, source, external_id) partial unique index. A pre-check handles the sequential double-submit case; rescue ActiveRecord::RecordNotUnique is the authoritative backstop for genuine concurrent requests - the whole Transfer.transaction block rolls back cleanly on conflict, so there's no risk of a half-created transfer. A same-day duplicate transfer can be legitimate (unlike a duplicate valuation, see #3339/PR #3340), so this uses the same per-submission token approach as #3334/PR #3338 rather than a natural-key DB constraint. Fixes #3341. Co-Authored-By: Claude <noreply@anthropic.com> * fix(transfers): store the idempotency key in its own column, isolate the retry with a savepoint Same two review findings as PR #3338 (transactions) and #3340 (valuations), applied here since this branch shares the same mechanism: - Reusing external_id/source for the web-form idempotency token made every leg of a manually-created transfer satisfy Entry#linked?, incorrectly making it look provider-synced. Uses the same dedicated entries.idempotency_key column added in db/migrate/20260902180400_add_idempotency_key_to_entries.rb (cherry-picked identically from PR #3338 - this branch depends on that migration; please merge #3338 first, or merge this after it lands so the duplicate migration file is a no-op). - Transfer::Creator now wraps the actual save in Transfer.transaction(requires_new: true) so a RecordNotUnique only rolls back to a savepoint rather than aborting any transaction the caller might already be in, keeping the rescue's retry lookup usable (mirrors the fix already applied to Account::ReconciliationManager in PR #3340). Added a regression test asserting neither leg of a transfer created via this path is linked? or has external_id/source set. Co-Authored-By: Claude <noreply@anthropic.com> * fix(transfers): distinct idempotency key per leg, rebuild invalid index on retry Two more review findings: - CodeRabbit: the form doesn't prevent selecting the same account as both source and destination. The outflow and inflow legs shared the bare idempotency key, so on that same-account path they'd collide with each other under the same account-scoped unique index (as would both fee legs, which shared a single "-fee" suffix). Every leg now gets a distinct, role-specific suffix (outflow stays bare - that's what find_existing_transfer looks up by - inflow/source_fee/ destination_fee each get their own). - Codex (same finding already fixed once for entries.idempotency_key's sibling migration, recurring here since this branch carries an identical copy): index_exists? alone doesn't distinguish a valid index from an INVALID one left behind by an interrupted CREATE INDEX CONCURRENTLY, so a retry after a failed build would short-circuit and record the migration as applied while the constraint was still missing. Now checks pg_index.indisvalid directly before deciding to skip. Co-Authored-By: Claude <noreply@anthropic.com> * fix(transfers): clear idempotency key on destroy so a retry doesn't 500 Codex flagged that Transfer#destroy! (used by reject!) preserves the outflow/inflow entries but not the Transfer join row - a retried create request with the same idempotency_key would find no Transfer via find_existing_transfer, attempt another insert, hit the stale entry's unique key, and re-raise RecordNotUnique instead of finding a match. Clear the key on the surviving entries when a transfer is destroyed. Also adds a regression test for the CodeRabbit-flagged per-leg key collision concern (already fixed by role-specific suffixes in the prior commit) to lock in that fee legs never share a key with their primary leg. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(transfers): verify idempotency key matches the request, fix stale doc comment jjmata review on #3342: - find_existing_transfer matched on idempotency_key + source_account only, so a stale key from a cached form could silently return a different, older transfer instead of creating the one actually requested. Now verifies destination account, date, and amount before treating a key match as the same request; a genuine mismatch surfaces as a new StaleIdempotencyKeyError (422 + message) instead of a false success or a raw 500. - Removed a comment claiming parity with a TransactionsController#new_transaction_idempotency_key method that doesn't exist in the codebase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(transfers): preserve from_account_id on error, match fees/exchange rate in idempotency check coderabbitai review on #3342: - All three create rescue blocks (exchange rate unavailable, invalid date, stale idempotency key) failed to set @from_account_id, so the re-rendered form lost the user's selected source account. - matches_request? only compared accounts/date/outflow amount, so a retry with the same key but a different exchange_rate or fee would be reported as success while silently keeping the old inflow amount and fee entries. Now recomputes the request's effective inflow amount and compares derived fee totals too; a mismatch raises StaleIdempotencyKeyError like other stale-key mismatches instead of silently returning the old transfer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Signed-off-by: Juan José Mata <juanjo.mata@gmail.com> Co-authored-by: Gerald <248542187+gfr-free@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
704 lines
23 KiB
Ruby
704 lines
23 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 "resubmitting the same idempotency key does not create a duplicate transfer" do
|
|
idempotency_key = SecureRandom.uuid
|
|
params = {
|
|
transfer: {
|
|
from_account_id: accounts(:depository).id,
|
|
to_account_id: accounts(:credit_card).id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
name: "Test Transfer",
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
|
|
assert_difference "Transfer.count", 1 do
|
|
assert_difference "Entry.count", 2 do
|
|
post transfers_url, params: params
|
|
end
|
|
end
|
|
first_transfer = Transfer.order(:created_at).last
|
|
|
|
# Simulates a double-click or a browser retry: same form, same
|
|
# idempotency key, submitted again after the first request already
|
|
# completed and committed.
|
|
assert_no_difference [ "Transfer.count", "Entry.count" ] do
|
|
post transfers_url, params: params
|
|
end
|
|
assert_redirected_to transactions_path
|
|
end
|
|
|
|
test "the idempotency key does not mark either leg as provider-linked" do
|
|
# Regression test: the key must not be stored in external_id/source
|
|
# (Entry#linked? = external_id.present?), or an ordinary manual transfer
|
|
# would incorrectly look provider-synced.
|
|
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",
|
|
source_fee_amount: 3,
|
|
idempotency_key: SecureRandom.uuid
|
|
}
|
|
}
|
|
|
|
transfer = Transfer.order(:created_at).last
|
|
[ transfer.outflow_transaction.entry, transfer.inflow_transaction.entry, transfer.fee_transactions.first.entry ].each do |entry|
|
|
assert_not entry.linked?
|
|
assert_nil entry.external_id
|
|
assert_nil entry.source
|
|
end
|
|
end
|
|
|
|
test "each leg gets a distinct idempotency key so fee legs don't collide with their primary leg" do
|
|
# Regression test: the outflow/source-fee share source_account, and the
|
|
# inflow/destination-fee share destination_account. The unique index is
|
|
# scoped per account_id, so without role-specific suffixes a fee leg
|
|
# would collide with its primary leg under the same idempotency key.
|
|
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",
|
|
source_fee_amount: 2,
|
|
destination_fee_amount: 3,
|
|
idempotency_key: SecureRandom.uuid
|
|
}
|
|
}
|
|
|
|
transfer = Transfer.order(:created_at).last
|
|
keys = [
|
|
transfer.outflow_transaction.entry.idempotency_key,
|
|
transfer.inflow_transaction.entry.idempotency_key,
|
|
transfer.fee_transactions.map { |t| t.entry.idempotency_key }
|
|
].flatten
|
|
|
|
assert_equal keys.uniq.length, keys.length
|
|
assert keys.all?(&:present?)
|
|
end
|
|
|
|
test "handles a genuine concurrent double-submit without raising or duplicating" do
|
|
idempotency_key = SecureRandom.uuid
|
|
from_account = accounts(:depository)
|
|
to_account = accounts(:credit_card)
|
|
|
|
# Simulates the race: another request with the same idempotency key wins
|
|
# and commits its transfer in the window between our pre-check (which
|
|
# therefore still sees nothing, hence the first `nil`) and our own
|
|
# Transfer#save! (which then hits the real partial unique index on
|
|
# entries(account_id, idempotency_key) and raises RecordNotUnique,
|
|
# exactly like the DB would under real concurrent requests). The rescue
|
|
# then re-runs the same lookup, this time finding the winner.
|
|
winning_transfer = Transfer::Creator.new(
|
|
family: users(:family_admin).family,
|
|
source_account_id: from_account.id,
|
|
destination_account_id: to_account.id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
idempotency_key: idempotency_key
|
|
).create
|
|
Transfer::Creator.any_instance.stubs(:find_existing_transfer).returns(nil, winning_transfer)
|
|
Transfer.any_instance.stubs(:save!).raises(ActiveRecord::RecordNotUnique.new("duplicate key value violates unique constraint"))
|
|
|
|
assert_no_difference [ "Transfer.count", "Entry.count" ] do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: from_account.id,
|
|
to_account_id: to_account.id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
name: "Test Transfer",
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
end
|
|
|
|
assert_redirected_to transactions_path
|
|
end
|
|
|
|
test "a RecordNotUnique with no matching transfer is not silently swallowed" do
|
|
idempotency_key = SecureRandom.uuid
|
|
|
|
# Defensive-branch coverage: if the unique index ever rejects an insert
|
|
# for a reason other than "another request with this exact idempotency
|
|
# key already won," we must not pretend it succeeded - it should
|
|
# surface as a stale-form error instead of a raw DB exception.
|
|
Transfer::Creator.any_instance.stubs(:find_existing_transfer).returns(nil)
|
|
Transfer.any_instance.stubs(:save!).raises(ActiveRecord::RecordNotUnique.new("duplicate key value violates unique constraint"))
|
|
|
|
assert_no_difference [ "Transfer.count", "Entry.count" ] 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",
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
end
|
|
|
|
assert_response :unprocessable_entity
|
|
end
|
|
|
|
test "reusing a key for a genuinely different request does not silently return the old transfer" do
|
|
idempotency_key = SecureRandom.uuid
|
|
from_account = accounts(:depository)
|
|
to_account = accounts(:credit_card)
|
|
other_destination = accounts(:other_liability)
|
|
|
|
assert_difference "Transfer.count", 1 do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: from_account.id,
|
|
to_account_id: to_account.id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
name: "Test Transfer",
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
end
|
|
|
|
# Simulates a stale hidden field (cached page / reopened dialog) still
|
|
# carrying the first request's key while the user changed the
|
|
# destination account before resubmitting.
|
|
assert_no_difference [ "Transfer.count", "Entry.count" ] do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: from_account.id,
|
|
to_account_id: other_destination.id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
name: "Test Transfer",
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
end
|
|
|
|
assert_response :unprocessable_entity
|
|
end
|
|
|
|
test "reusing a key with a different fee does not silently return the old transfer" do
|
|
idempotency_key = SecureRandom.uuid
|
|
from_account = accounts(:depository)
|
|
to_account = accounts(:credit_card)
|
|
|
|
assert_difference "Transfer.count", 1 do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: from_account.id,
|
|
to_account_id: to_account.id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
name: "Test Transfer",
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
end
|
|
|
|
# Same accounts/date/amount/key as the first request, but a fee was
|
|
# added before resubmitting - the persisted effect differs, so this
|
|
# must not be reported as a successful retry of the fee-less transfer.
|
|
assert_no_difference [ "Transfer.count", "Entry.count" ] do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: from_account.id,
|
|
to_account_id: to_account.id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
name: "Test Transfer",
|
|
source_fee_amount: 5,
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
end
|
|
|
|
assert_response :unprocessable_entity
|
|
end
|
|
|
|
test "reusing a key with a different exchange rate does not silently return the old transfer" do
|
|
idempotency_key = SecureRandom.uuid
|
|
usd_account = accounts(:depository)
|
|
eur_account = users(:family_admin).family.accounts.create!(
|
|
name: "EUR Account",
|
|
balance: 1000,
|
|
currency: "EUR",
|
|
accountable: Depository.new
|
|
)
|
|
|
|
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.9,
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
end
|
|
|
|
# Same accounts/date/amount/key, but a different exchange rate - the
|
|
# persisted inflow amount would differ, so this must not be reported as
|
|
# a successful retry of the first, differently-converted transfer.
|
|
assert_no_difference [ "Transfer.count", "Entry.count" ] 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.5,
|
|
idempotency_key: idempotency_key
|
|
}
|
|
}
|
|
end
|
|
|
|
assert_response :unprocessable_entity
|
|
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 "can create transfer with source fee" 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,
|
|
source_fee_amount: 3
|
|
}
|
|
}
|
|
end
|
|
|
|
transfer = Transfer.order(created_at: :desc).first
|
|
assert_equal 100, transfer.amount
|
|
assert_equal 3, transfer.derived_source_fee_amount
|
|
assert_equal 0, transfer.derived_destination_fee_amount
|
|
# Outflow should be principal only (no fee baked in)
|
|
assert_equal 100, transfer.outflow_transaction.entry.amount
|
|
# Inflow should be -(converted_principal)
|
|
assert_equal(-100, transfer.inflow_transaction.entry.amount)
|
|
# Fee transaction should be created
|
|
assert_equal 1, transfer.fee_transactions.count
|
|
fee_tx = transfer.fee_transactions.first
|
|
assert_equal "standard", fee_tx.kind
|
|
assert_equal 3, fee_tx.entry.amount
|
|
assert_equal accounts(:depository).id, fee_tx.entry.account_id
|
|
assert transfer.has_source_fee?
|
|
assert_not transfer.has_destination_fee?
|
|
end
|
|
|
|
test "can create transfer with destination fee" 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,
|
|
destination_fee_amount: 3
|
|
}
|
|
}
|
|
end
|
|
|
|
transfer = Transfer.order(created_at: :desc).first
|
|
assert_equal 100, transfer.amount
|
|
assert_equal 0, transfer.derived_source_fee_amount
|
|
assert_equal 3, transfer.derived_destination_fee_amount
|
|
# Outflow should be principal only
|
|
assert_equal 100, transfer.outflow_transaction.entry.amount
|
|
# Inflow should be -(converted_principal)
|
|
assert_equal(-100, transfer.inflow_transaction.entry.amount)
|
|
# Fee transaction should be created
|
|
assert_equal 1, transfer.fee_transactions.count
|
|
fee_tx = transfer.fee_transactions.first
|
|
assert_equal "standard", fee_tx.kind
|
|
assert_equal 3, fee_tx.entry.amount
|
|
assert_equal accounts(:credit_card).id, fee_tx.entry.account_id
|
|
assert_not transfer.has_source_fee?
|
|
assert transfer.has_destination_fee?
|
|
end
|
|
|
|
test "can create transfer with both source and destination fees" 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,
|
|
source_fee_amount: 2,
|
|
destination_fee_amount: 3
|
|
}
|
|
}
|
|
end
|
|
|
|
transfer = Transfer.order(created_at: :desc).first
|
|
assert_equal 100, transfer.amount
|
|
assert_equal 2, transfer.derived_source_fee_amount
|
|
assert_equal 3, transfer.derived_destination_fee_amount
|
|
# Outflow should be principal only
|
|
assert_equal 100, transfer.outflow_transaction.entry.amount
|
|
# Inflow should be -(converted_principal)
|
|
assert_equal(-100, transfer.inflow_transaction.entry.amount)
|
|
# Two fee transactions should be created
|
|
assert_equal 2, transfer.fee_transactions.count
|
|
source_fee_tx = transfer.fee_transactions.find { |t| t.entry.account_id == accounts(:depository).id }
|
|
dest_fee_tx = transfer.fee_transactions.find { |t| t.entry.account_id == accounts(:credit_card).id }
|
|
assert_equal 2, source_fee_tx.entry.amount
|
|
assert_equal 3, dest_fee_tx.entry.amount
|
|
assert transfer.has_fees?
|
|
end
|
|
|
|
test "derived fee methods reflect fee transaction entry edits" do
|
|
post transfers_url, params: {
|
|
transfer: {
|
|
from_account_id: accounts(:depository).id,
|
|
to_account_id: accounts(:credit_card).id,
|
|
date: Date.current,
|
|
amount: 100,
|
|
source_fee_amount: 3
|
|
}
|
|
}
|
|
|
|
transfer = Transfer.order(created_at: :desc).first
|
|
assert_equal 3, transfer.derived_source_fee_amount
|
|
|
|
# Simulate an independent edit of the fee transaction entry
|
|
fee_tx = transfer.fee_transactions.first
|
|
fee_tx.entry.update!(amount: 5)
|
|
|
|
# Derived fee should reflect the updated entry
|
|
transfer.reload
|
|
assert_equal 5, transfer.derived_source_fee_amount
|
|
assert transfer.has_source_fee?
|
|
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 create transfer with tags on both sides" do
|
|
tag = tags(:one)
|
|
|
|
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,
|
|
tag_ids: [ tag.id ]
|
|
}
|
|
}
|
|
end
|
|
|
|
transfer = Transfer.order(:created_at).last
|
|
assert_equal [ tag.id ], transfer.outflow_transaction.tag_ids
|
|
assert_equal [ tag.id ], transfer.inflow_transaction.tag_ids
|
|
end
|
|
|
|
test "can update transfer tags on both sides" do
|
|
transfer = transfers(:one)
|
|
tag = tags(:one)
|
|
|
|
patch tags_transfer_url(transfer), params: { tag_ids: [ tag.id ] }, as: :json
|
|
|
|
assert_response :success
|
|
assert_equal [ tag.id ], transfer.outflow_transaction.reload.tag_ids
|
|
assert_equal [ tag.id ], transfer.inflow_transaction.reload.tag_ids
|
|
assert_equal [ tag.id ], JSON.parse(response.body)["tag_ids"]
|
|
end
|
|
|
|
test "update transfer tags ignores tags from other families" do
|
|
transfer = transfers(:one)
|
|
family_tag = tags(:one)
|
|
other_family = Family.create!(name: "Other Family", currency: "USD")
|
|
other_tag = other_family.tags.create!(name: "Foreign")
|
|
|
|
patch tags_transfer_url(transfer), params: {
|
|
tag_ids: [ family_tag.id, other_tag.id ]
|
|
}, as: :json
|
|
|
|
assert_response :success
|
|
assert_equal [ family_tag.id ], transfer.outflow_transaction.reload.tag_ids
|
|
assert_equal [ family_tag.id ], transfer.inflow_transaction.reload.tag_ids
|
|
end
|
|
|
|
test "can clear transfer tags" do
|
|
transfer = transfers(:one)
|
|
tag = tags(:one)
|
|
transfer.outflow_transaction.update!(tag_ids: [ tag.id ])
|
|
transfer.inflow_transaction.update!(tag_ids: [ tag.id ])
|
|
|
|
patch tags_transfer_url(transfer), params: { tag_ids: [] }, as: :json
|
|
|
|
assert_response :success
|
|
assert_empty transfer.outflow_transaction.reload.tag_ids
|
|
assert_empty transfer.inflow_transaction.reload.tag_ids
|
|
end
|
|
|
|
test "update tags requires annotate permission on both transfer sides" do
|
|
# family_member: full_control on depository (outflow), read_only on credit_card (inflow)
|
|
sign_in users(:family_member)
|
|
transfer = transfers(:one)
|
|
tag = tags(:one)
|
|
original_outflow_tags = transfer.outflow_transaction.tag_ids
|
|
original_inflow_tags = transfer.inflow_transaction.tag_ids
|
|
|
|
patch tags_transfer_url(transfer), params: { tag_ids: [ tag.id ] }, as: :json
|
|
|
|
assert_response :forbidden
|
|
assert_equal I18n.t("accounts.not_authorized"), JSON.parse(response.body)["error"]
|
|
assert_equal original_outflow_tags, transfer.outflow_transaction.reload.tag_ids
|
|
assert_equal original_inflow_tags, transfer.inflow_transaction.reload.tag_ids
|
|
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
|