Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Add validation to ensure transfer inflow is after outflow #1604

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/controllers/account/transactions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ def mark_transfers
.mark_transfers!

redirect_back_or_to transactions_url, notice: t(".success")
rescue ActiveRecord::RecordInvalid => e
redirect_back_or_to transactions_url, alert: e.message
end

def unmark_transfers
Expand Down
2 changes: 1 addition & 1 deletion app/models/account/entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def mark_transfers!
update_all marked_as_transfer: true

# Attempt to "auto match" and save a transfer if 2 transactions selected
Account::Transfer.new(entries: all).save if all.count == 2
Account::Transfer.new(entries: all).save! if all.count == 2
end

def bulk_update!(bulk_update_params)
Expand Down
10 changes: 9 additions & 1 deletion app/models/account/transfer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class Account::Transfer < ApplicationRecord
has_many :entries, dependent: :destroy

validate :net_zero_flows, if: :single_currency_transfer?
validate :transaction_count, :from_different_accounts, :all_transactions_marked
validate :transaction_count, :from_different_accounts, :inflow_after_outflow, :all_transactions_marked

def date
outflow_transaction&.date
Expand Down Expand Up @@ -99,6 +99,14 @@ def from_different_accounts
errors.add :entries, :must_be_from_different_accounts if accounts.size < entries.size
end

def inflow_after_outflow
return unless inflow_transaction && outflow_transaction

if inflow_transaction.date < outflow_transaction.date
errors.add :entries, :inflow_must_be_after_outflow
end
end

def net_zero_flows
unless entries.sum(&:amount).zero?
errors.add :entries, :must_have_an_inflow_and_outflow_that_net_to_zero
Expand Down
1 change: 1 addition & 0 deletions config/locales/models/account/transfer/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ en:
must_have_an_inflow_and_outflow_that_net_to_zero: must have an inflow
and outflow that net to zero
must_have_exactly_2_entries: must have exactly 2 entries
inflow_must_be_after_outflow: inflow must be after outflow
32 changes: 32 additions & 0 deletions test/system/transfers_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,38 @@ class TransfersTest < ApplicationSystemTestCase
end
end

test "cannot create transfer where inflow date is before outflow date" do
outflow = accounts(:depository).entries.create!(
name: "Outflow from checking account",
date: Date.current,
amount: 100,
currency: "USD",
entryable: Account::Transaction.new
)

inflow = accounts(:credit_card).entries.create!(
name: "Inflow to cc account",
date: 1.day.ago.to_date, # Date before outflow
amount: -100,
currency: "USD",
entryable: Account::Transaction.new
)

visit transactions_url

transaction_entry_checkbox(inflow).check
transaction_entry_checkbox(outflow).check

bulk_transfer_action_button.click
click_on "Mark as transfers"

assert_text "Validation failed: Entries inflow must be after outflow"

# Verify the entries weren't linked as a transfer
assert_nil outflow.reload.transfer_id
assert_nil inflow.reload.transfer_id
end

test "can mark a single transaction as a transfer" do
txn = @user.family.entries.account_transactions.reverse_chronological.first

Expand Down