From cc7d67550030f0716e646fbf43e42322659a8104 Mon Sep 17 00:00:00 2001 From: Serge L Date: Sat, 28 Mar 2026 14:03:16 -0400 Subject: [PATCH] Add transaction fee support to trades (#1248) Add an optional fee field (decimal, precision: 19, scale: 4) to trades. Fee is included in the total amount calculation (qty * price + fee) for both create and update flows. The fee field appears on both the create and edit forms, defaults to 0, and auto-submits like other trade fields. Co-authored-by: Claude Opus 4.6 --- app/controllers/trades_controller.rb | 8 +- app/models/trade.rb | 1 + app/models/trade/create_form.rb | 5 +- app/views/trades/_form.html.erb | 1 + app/views/trades/show.html.erb | 10 ++- config/locales/views/trades/en.yml | 2 + .../20260322120702_add_fee_to_trades.rb | 5 ++ db/schema.rb | 1 + test/controllers/trades_controller_test.rb | 83 +++++++++++++++++++ test/models/trade_test.rb | 13 +++ 10 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20260322120702_add_fee_to_trades.rb diff --git a/app/controllers/trades_controller.rb b/app/controllers/trades_controller.rb index c8b76af02..a8f0940c7 100644 --- a/app/controllers/trades_controller.rb +++ b/app/controllers/trades_controller.rb @@ -94,13 +94,13 @@ class TradesController < ApplicationController def entry_params params.require(:entry).permit( :name, :date, :amount, :currency, :excluded, :notes, :nature, - entryable_attributes: [ :id, :qty, :price, :investment_activity_label ] + entryable_attributes: [ :id, :qty, :price, :fee, :investment_activity_label ] ) end def create_params params.require(:model).permit( - :date, :amount, :currency, :qty, :price, :ticker, :manual_ticker, :type, :transfer_account_id + :date, :amount, :currency, :qty, :price, :fee, :ticker, :manual_ticker, :type, :transfer_account_id ) end @@ -112,13 +112,15 @@ class TradesController < ApplicationController qty = update_params[:entryable_attributes][:qty] price = update_params[:entryable_attributes][:price] + fee = update_params[:entryable_attributes][:fee] nature = update_params[:nature] if qty.present? && price.present? is_sell = nature == "inflow" qty = is_sell ? -qty.to_d.abs : qty.to_d.abs + fee_val = fee.present? ? fee.to_d : (@entry.trade&.fee || 0) update_params[:entryable_attributes][:qty] = qty - update_params[:amount] = qty * price.to_d + update_params[:amount] = qty * price.to_d + fee_val # Sync investment_activity_label with Buy/Sell type if not explicitly set to something else # Check both the submitted param and the existing record's label diff --git a/app/models/trade.rb b/app/models/trade.rb index 10159d201..fd0168b5b 100644 --- a/app/models/trade.rb +++ b/app/models/trade.rb @@ -2,6 +2,7 @@ class Trade < ApplicationRecord include Entryable, Monetizable monetize :price + monetize :fee belongs_to :security belongs_to :category, optional: true diff --git a/app/models/trade/create_form.rb b/app/models/trade/create_form.rb index d822131c5..239e109d0 100644 --- a/app/models/trade/create_form.rb +++ b/app/models/trade/create_form.rb @@ -2,7 +2,7 @@ class Trade::CreateForm include ActiveModel::Model attr_accessor :account, :date, :amount, :currency, :qty, - :price, :ticker, :manual_ticker, :type, :transfer_account_id + :price, :fee, :ticker, :manual_ticker, :type, :transfer_account_id # Either creates a trade, transaction, or transfer based on type # Returns the model, regardless of success or failure @@ -30,7 +30,7 @@ class Trade::CreateForm def create_trade signed_qty = type == "sell" ? -qty.to_d : qty.to_d - signed_amount = signed_qty * price.to_d + signed_amount = signed_qty * price.to_d + fee.to_d trade_entry = account.entries.new( name: Trade.build_name(type, qty, security.ticker), @@ -40,6 +40,7 @@ class Trade::CreateForm entryable: Trade.new( qty: signed_qty, price: price, + fee: fee.to_d, currency: currency, security: security, investment_activity_label: type.capitalize # "buy" → "Buy", "sell" → "Sell" diff --git a/app/views/trades/_form.html.erb b/app/views/trades/_form.html.erb index 78c1b1b0b..2ea361b6e 100644 --- a/app/views/trades/_form.html.erb +++ b/app/views/trades/_form.html.erb @@ -51,6 +51,7 @@ <% if %w[buy sell].include?(type) %> <%= form.number_field :qty, label: t(".qty"), placeholder: "10", min: 0.000000000000000001, step: "any", required: true %> <%= form.money_field :price, label: t(".price"), step: "any", precision: 10, required: true %> + <%= form.money_field :fee, label: t(".fee"), step: "any", min: 0 %> <% end %> diff --git a/app/views/trades/show.html.erb b/app/views/trades/show.html.erb index 31f49fe27..b056b06ce 100644 --- a/app/views/trades/show.html.erb +++ b/app/views/trades/show.html.erb @@ -40,7 +40,15 @@ auto_submit: true, min: 0, step: "any", - precision: 10, + disabled: @entry.linked? %> + <% end %> + <%= f.fields_for :entryable do |ef| %> + <%= ef.money_field :fee, + label: t(".fee_label"), + disable_currency: true, + auto_submit: true, + min: 0, + step: "any", disabled: @entry.linked? %> <% end %> <% end %> diff --git a/config/locales/views/trades/en.yml b/config/locales/views/trades/en.yml index 606975c19..9659b44f8 100644 --- a/config/locales/views/trades/en.yml +++ b/config/locales/views/trades/en.yml @@ -5,6 +5,7 @@ en: account: Transfer account (optional) account_prompt: Search account amount: Amount + fee: Transaction fee holding: Ticker symbol price: Price per share qty: Quantity @@ -29,6 +30,7 @@ en: cost_per_share_label: Cost per Share date_label: Date delete: Delete + fee_label: Transaction fee delete_subtitle: This action cannot be undone delete_title: Delete Trade details: Details diff --git a/db/migrate/20260322120702_add_fee_to_trades.rb b/db/migrate/20260322120702_add_fee_to_trades.rb new file mode 100644 index 000000000..09b9c91a2 --- /dev/null +++ b/db/migrate/20260322120702_add_fee_to_trades.rb @@ -0,0 +1,5 @@ +class AddFeeToTrades < ActiveRecord::Migration[7.2] + def change + add_column :trades, :fee, :decimal, precision: 19, scale: 4, default: 0, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 388dab435..7350b7845 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1424,6 +1424,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_26_112218) do t.string "currency" t.jsonb "locked_attributes", default: {} t.string "investment_activity_label" + t.decimal "fee", precision: 19, scale: 4, default: "0.0", null: false t.index ["investment_activity_label"], name: "index_trades_on_investment_activity_label" t.index ["security_id"], name: "index_trades_on_security_id" end diff --git a/test/controllers/trades_controller_test.rb b/test/controllers/trades_controller_test.rb index 21e34249a..ea6127f8a 100644 --- a/test/controllers/trades_controller_test.rb +++ b/test/controllers/trades_controller_test.rb @@ -111,6 +111,89 @@ class TradesControllerTest < ActionDispatch::IntegrationTest assert_redirected_to @entry.account end + test "creates trade buy entry with fee" do + assert_difference [ "Entry.count", "Trade.count" ], 1 do + post trades_url(account_id: @entry.account_id), params: { + model: { + type: "buy", + date: Date.current, + ticker: "NVDA (NASDAQ)", + qty: 10, + price: 20, + fee: 9.95, + currency: "USD" + } + } + end + + created_entry = Entry.order(created_at: :desc).first + + assert_in_delta 209.95, created_entry.amount.to_f, 0.001 + assert_in_delta 9.95, created_entry.trade.fee.to_f, 0.001 + assert_redirected_to account_url(created_entry.account) + end + + test "creates trade sell entry with fee" do + assert_difference [ "Entry.count", "Trade.count" ], 1 do + post trades_url(account_id: @entry.account_id), params: { + model: { + type: "sell", + date: Date.current, + ticker: "AAPL (NYSE)", + qty: 10, + price: 20, + fee: 9.95, + currency: "USD" + } + } + end + + created_entry = Entry.order(created_at: :desc).first + + # sell: signed_amount = -10 * 20 + 9.95 = -190.05 + assert_in_delta(-190.05, created_entry.amount.to_f, 0.001) + assert_in_delta 9.95, created_entry.trade.fee.to_f, 0.001 + assert_redirected_to account_url(created_entry.account) + end + + test "creates trade buy entry without fee defaults to zero" do + post trades_url(account_id: @entry.account_id), params: { + model: { + type: "buy", + date: Date.current, + ticker: "NVDA (NASDAQ)", + qty: 10, + price: 20, + currency: "USD" + } + } + + created_entry = Entry.order(created_at: :desc).first + + assert_in_delta 200, created_entry.amount.to_f, 0.001 + assert_equal 0, created_entry.trade.fee.to_f + end + + test "update includes fee in amount" do + patch trade_url(@entry), params: { + entry: { + currency: "USD", + nature: "outflow", + entryable_attributes: { + id: @entry.entryable_id, + qty: 10, + price: 20, + fee: 9.95 + } + } + } + + @entry.reload + + assert_in_delta 209.95, @entry.amount.to_f, 0.001 + assert_in_delta 9.95, @entry.trade.fee.to_f, 0.001 + end + test "creates trade buy entry" do assert_difference [ "Entry.count", "Trade.count", "Security.count" ], 1 do post trades_url(account_id: @entry.account_id), params: { diff --git a/test/models/trade_test.rb b/test/models/trade_test.rb index ab8ba56ba..bcf09b192 100644 --- a/test/models/trade_test.rb +++ b/test/models/trade_test.rb @@ -39,6 +39,19 @@ class TradeTest < ActiveSupport::TestCase assert_equal precise_price, trade.price end + test "fee defaults to 0" do + security = Security.create!(ticker: "FEETEST", exchange_operating_mic: "XNAS") + trade = Trade.create!( + security: security, + price: 100, + qty: 10, + currency: "USD", + investment_activity_label: "Buy" + ) + + assert_equal 0, trade.fee + end + test "price is rounded to 10 decimal places" do security = Security.create!(ticker: "TEST", exchange_operating_mic: "XNAS")