diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 895dcfa2..48556f09 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -1,4 +1,6 @@ class TransfersController < ApplicationController + include StreamExtensions + before_action :set_transfer, only: %i[destroy show update] def new @@ -10,25 +12,19 @@ class TransfersController < ApplicationController end def create - from_account = Current.family.accounts.find(transfer_params[:from_account_id]) - to_account = Current.family.accounts.find(transfer_params[:to_account_id]) - - @transfer = Transfer.from_accounts( - from_account: from_account, - to_account: to_account, + @transfer = Transfer::Creator.new( + family: Current.family, + source_account_id: transfer_params[:from_account_id], + destination_account_id: transfer_params[:to_account_id], date: transfer_params[:date], amount: transfer_params[:amount].to_d - ) - - if @transfer.save - @transfer.sync_account_later - - flash[:notice] = t(".success") + ).create + if @transfer.persisted? + success_message = "Transfer created" respond_to do |format| - format.html { redirect_back_or_to transactions_path } - redirect_target_url = request.referer || transactions_path - format.turbo_stream { render turbo_stream: turbo_stream.action(:redirect, redirect_target_url) } + format.html { redirect_back_or_to transactions_path, notice: success_message } + format.turbo_stream { stream_redirect_back_or_to transactions_path, notice: success_message } end else render :new, status: :unprocessable_entity diff --git a/app/models/trade/creator.rb b/app/models/trade/creator.rb new file mode 100644 index 00000000..224f3576 --- /dev/null +++ b/app/models/trade/creator.rb @@ -0,0 +1,12 @@ +class Trade::Creator + def initialize(attrs) + @attrs = attrs + end + + def create + # TODO + end + + private + attr_reader :attrs +end \ No newline at end of file diff --git a/app/models/transaction.rb b/app/models/transaction.rb index c2e20148..0967aac1 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -10,10 +10,11 @@ class Transaction < ApplicationRecord accepts_nested_attributes_for :taggings, allow_destroy: true enum :kind, { - standard: "standard", - transfer: "transfer", - loan_payment: "loan_payment", - one_time: "one_time" + standard: "standard", # A regular transaction, included in budget analytics + transfer: "transfer", # Movement of funds, excluded from budget analytics + payment: "payment", # A CC or Other payment, excluded from budget analytics (CC payments offset the sum of expense transactions) + loan_payment: "loan_payment", # A payment to a Loan account, treated as an expense in budgets + one_time: "one_time" # A one-time expense/income, excluded from budget analytics } class << self diff --git a/app/models/transfer.rb b/app/models/transfer.rb index 20804ddd..7177a0dd 100644 --- a/app/models/transfer.rb +++ b/app/models/transfer.rb @@ -22,8 +22,17 @@ class Transfer < ApplicationRecord Money.new(amount.abs, from_account.currency) end + outflow_kind = if to_account&.accountable_type == "Loan" + "loan_payment" + elsif to_account&.liability? + "payment" + else + "transfer" + end + new( inflow_transaction: Transaction.new( + kind: "transfer", entry: to_account.entries.build( amount: converted_amount.amount.abs * -1, currency: converted_amount.currency.iso_code, @@ -32,6 +41,7 @@ class Transfer < ApplicationRecord ) ), outflow_transaction: Transaction.new( + kind: outflow_kind, entry: from_account.entries.build( amount: amount.abs, currency: from_account.currency, @@ -89,6 +99,24 @@ class Transfer < ApplicationRecord to_account&.liability? end + def loan_payment? + outflow_transaction&.kind == "loan_payment" + end + + def liability_payment? + outflow_transaction&.kind == "payment" + end + + def regular_transfer? + outflow_transaction&.kind == "transfer" + end + + def transfer_type + return "loan_payment" if loan_payment? + return "liability_payment" if liability_payment? + "transfer" + end + def categorizable? to_account&.accountable_type == "Loan" end diff --git a/app/models/transfer/creator.rb b/app/models/transfer/creator.rb new file mode 100644 index 00000000..b713bafa --- /dev/null +++ b/app/models/transfer/creator.rb @@ -0,0 +1,85 @@ +class Transfer::Creator + def initialize(family:, source_account_id:, destination_account_id:, date:, amount:) + @family = family + @source_account = family.accounts.find(source_account_id) # early throw if not found + @destination_account = family.accounts.find(destination_account_id) # early throw if not found + @date = date + @amount = amount + end + + def create + transfer = Transfer.new( + inflow_transaction: inflow_transaction, + outflow_transaction: outflow_transaction, + status: "confirmed" + ) + + if transfer.save + source_account.sync_later + destination_account.sync_later + end + + transfer + end + + private + attr_reader :family, :source_account, :destination_account, :date, :amount + + def outflow_transaction + name = "#{name_prefix} to #{destination_account.name}" + + Transaction.new( + kind: outflow_transaction_kind, + entry: source_account.entries.build( + amount: amount.abs, + currency: source_account.currency, + date: date, + name: name, + ) + ) + end + + def inflow_transaction + name = "#{name_prefix} from #{source_account.name}" + + Transaction.new( + kind: "transfer", + entry: destination_account.entries.build( + amount: inflow_converted_money.amount.abs * -1, + currency: destination_account.currency, + date: date, + name: name, + ) + ) + end + + # If destination account has different currency, its transaction should show up as converted + # Future improvement: instead of a 1:1 conversion fallback, add a UI/UX flow for missing rates + def inflow_converted_money + Money.new(amount.abs, source_account.currency) + .exchange_to( + destination_account.currency, + date: date, + fallback_rate: 1.0 + ) + end + + # The "expense" side of a transfer is treated different in analytics based on where it goes. + def outflow_transaction_kind + if destination_account.loan? + "loan_payment" + elsif destination_account.liability? + "payment" + else + "transfer" + end + end + + def name_prefix + if destination_account.liability? + "Payment" + else + "Transfer" + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 9c10112c..f4d6b2a9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_06_10_181219) do +ActiveRecord::Schema[7.2].define(version: 2025_06_16_183654) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -648,7 +648,9 @@ ActiveRecord::Schema[7.2].define(version: 2025_06_10_181219) do t.uuid "category_id" t.uuid "merchant_id" t.jsonb "locked_attributes", default: {} + t.string "kind", default: "standard", null: false t.index ["category_id"], name: "index_transactions_on_category_id" + t.index ["kind"], name: "index_transactions_on_kind" t.index ["merchant_id"], name: "index_transactions_on_merchant_id" end diff --git a/test/fixtures/transactions.yml b/test/fixtures/transactions.yml index 426d7d58..7420c310 100644 --- a/test/fixtures/transactions.yml +++ b/test/fixtures/transactions.yml @@ -2,5 +2,7 @@ one: category: food_and_drink merchant: amazon -transfer_out: { } -transfer_in: { } \ No newline at end of file +transfer_out: + kind: payment +transfer_in: + kind: transfer \ No newline at end of file diff --git a/test/models/transfer/creator_test.rb b/test/models/transfer/creator_test.rb new file mode 100644 index 00000000..6328a1b4 --- /dev/null +++ b/test/models/transfer/creator_test.rb @@ -0,0 +1,166 @@ +require "test_helper" + +class Transfer::CreatorTest < ActiveSupport::TestCase + setup do + @family = families(:dylan_family) + @source_account = accounts(:depository) + @destination_account = accounts(:investment) + @date = Date.current + @amount = 100 + end + + test "creates basic transfer" do + creator = Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: @destination_account.id, + date: @date, + amount: @amount + ) + + transfer = creator.create + + assert transfer.persisted? + assert_equal "confirmed", transfer.status + assert transfer.regular_transfer? + assert_equal "transfer", transfer.transfer_type + + # Verify outflow transaction (from source account) + outflow = transfer.outflow_transaction + assert_equal "transfer", outflow.kind + assert_equal @amount, outflow.entry.amount + assert_equal @source_account.currency, outflow.entry.currency + assert_equal "Transfer to #{@destination_account.name}", outflow.entry.name + + # Verify inflow transaction (to destination account) + inflow = transfer.inflow_transaction + assert_equal "transfer", inflow.kind + assert_equal(@amount * -1, inflow.entry.amount) + assert_equal @destination_account.currency, inflow.entry.currency + assert_equal "Transfer from #{@source_account.name}", inflow.entry.name + end + + test "creates multi-currency transfer" do + # Use crypto account which has USD currency but different from source + crypto_account = accounts(:crypto) + + creator = Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: crypto_account.id, + date: @date, + amount: @amount + ) + + transfer = creator.create + + assert transfer.persisted? + assert transfer.regular_transfer? + assert_equal "transfer", transfer.transfer_type + + # Verify outflow transaction + outflow = transfer.outflow_transaction + assert_equal "transfer", outflow.kind + assert_equal "Transfer to #{crypto_account.name}", outflow.entry.name + + # Verify inflow transaction with currency handling + inflow = transfer.inflow_transaction + assert_equal "transfer", inflow.kind + assert_equal "Transfer from #{@source_account.name}", inflow.entry.name + assert_equal crypto_account.currency, inflow.entry.currency + end + + test "creates loan payment" do + loan_account = accounts(:loan) + + creator = Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: loan_account.id, + date: @date, + amount: @amount + ) + + transfer = creator.create + + assert transfer.persisted? + assert transfer.loan_payment? + assert_equal "loan_payment", transfer.transfer_type + + # Verify outflow transaction is marked as loan payment + outflow = transfer.outflow_transaction + assert_equal "loan_payment", outflow.kind + assert_equal "Payment to #{loan_account.name}", outflow.entry.name + + # Verify inflow transaction + inflow = transfer.inflow_transaction + assert_equal "transfer", inflow.kind + assert_equal "Payment from #{@source_account.name}", inflow.entry.name + end + + test "creates credit card payment" do + credit_card_account = accounts(:credit_card) + + creator = Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: credit_card_account.id, + date: @date, + amount: @amount + ) + + transfer = creator.create + + assert transfer.persisted? + assert transfer.liability_payment? + assert_equal "liability_payment", transfer.transfer_type + + # Verify outflow transaction is marked as payment for liability + outflow = transfer.outflow_transaction + assert_equal "payment", outflow.kind + assert_equal "Payment to #{credit_card_account.name}", outflow.entry.name + + # Verify inflow transaction + inflow = transfer.inflow_transaction + assert_equal "transfer", inflow.kind + assert_equal "Payment from #{@source_account.name}", inflow.entry.name + end + + test "raises error when source account ID is invalid" do + assert_raises(ActiveRecord::RecordNotFound) do + Transfer::Creator.new( + family: @family, + source_account_id: 99999, + destination_account_id: @destination_account.id, + date: @date, + amount: @amount + ) + end + end + + test "raises error when destination account ID is invalid" do + assert_raises(ActiveRecord::RecordNotFound) do + Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: 99999, + date: @date, + amount: @amount + ) + end + end + + test "raises error when source account belongs to different family" do + other_family = families(:empty) + + assert_raises(ActiveRecord::RecordNotFound) do + Transfer::Creator.new( + family: other_family, + source_account_id: @source_account.id, + destination_account_id: @destination_account.id, + date: @date, + amount: @amount + ) + end + end +end diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index 4b3bcae8..8b292c8e 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -203,6 +203,7 @@ class TransactionsTest < ApplicationSystemTestCase inflow_entry = create_transaction("inflow", 1.day.ago.to_date, -500, account: investment_account) @user.family.auto_match_transfers! visit transactions_url + within "#entry-group-" + Date.current.to_s + "-totals" do assert_text "-$100.00" # transaction eleven from setup end