mirror of
https://github.com/we-promise/sure.git
synced 2026-04-20 12:34:12 +00:00
Record dividends and interest as Trades in investment accounts (#1311)
* Record dividends and interest as Trades in investment accounts
All investment income (dividends and interest) is now modeled as a
Trade with qty: 0 and price: 0, keeping security_id NOT NULL on trades
intact. Dividends require a security; interest falls back to a
per-account synthetic cash security (kind: "cash", offline: true) when
none is selected, matching how brokerages handle uninvested cash
internally.
- Add `kind` column to securities ("standard" | "cash") with DB check
constraint; `Security.cash_for(account)` lazily finds or creates the
synthetic cash security; `scope :standard` excludes synthetic
securities from user-facing pickers
- Trade::CreateForm: new `dividend` type (security required); `interest`
now creates a Trade instead of a Transaction
- Trade form: Dividend and Interest in the type dropdown with a security
combobox (required for dividend, optional for interest)
- transactions table: untouched
* UI fixes
* HealthChecker — both scopes now chain .standard to exclude cash securities from provider health checks.
DB query moved to model — Account#traded_standard_securities in app/models/account.rb, view uses account.traded_standard_securities.
DRY income creation — create_income_trade(sec:, label:, name:) extracted as shared private method; create_dividend_income and create_interest_income delegate to it.
show.html.erb blocks merged — single unless trade.qty.zero? block covers qty/price/fee fields.
Test extended — assert_response :unprocessable_entity added after the assert_no_difference block.
* Hide cash account ticker from no-security trade detail
* Fix CodeRabbit review issues from PR #1311
- Remove duplicate YAML keys in translation files (de, es, fr)
- Add error handling for security resolution in create_dividend_income
- Extract income trade check to reduce duplication in header template
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* Include holdings in dividend/interest security picker
The security picker for dividend/interest trades should include all securities
in holdings, not just those with trade history. This fixes the issue where
accounts with imported holdings (e.g., SimpleFIN) but no trades would have an
empty picker and be unable to record dividends.
Uses UNION to combine securities from both trades and holdings.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* scope picker to holdings only (a trade creates a holding anyway)
---------
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -105,9 +105,16 @@ class TradesController < ApplicationController
|
||||
end
|
||||
|
||||
def update_entry_params
|
||||
return entry_params unless entry_params[:entryable_attributes].present?
|
||||
|
||||
update_params = entry_params
|
||||
|
||||
# Income trades (Dividend/Interest) store amounts as negative (inflow convention).
|
||||
# The form displays the absolute value, so we re-negate before saving.
|
||||
if %w[Dividend Interest].include?(@entry.trade&.investment_activity_label) && update_params[:amount].present?
|
||||
update_params = update_params.merge(amount: -update_params[:amount].to_d.abs)
|
||||
end
|
||||
|
||||
return update_params unless update_params[:entryable_attributes].present?
|
||||
|
||||
update_params = update_params.merge(entryable_type: "Trade")
|
||||
|
||||
qty = update_params[:entryable_attributes][:qty]
|
||||
|
||||
@@ -362,6 +362,13 @@ class Account < ApplicationRecord
|
||||
false
|
||||
end
|
||||
|
||||
def traded_standard_securities
|
||||
Security.where(id: holdings.select(:security_id))
|
||||
.standard
|
||||
.distinct
|
||||
.order(:ticker)
|
||||
end
|
||||
|
||||
# The balance type determines which "component" of balance is being tracked.
|
||||
# This is primarily used for balance related calculations and updates.
|
||||
#
|
||||
|
||||
@@ -6,6 +6,8 @@ class Security < ApplicationRecord
|
||||
# Data stored in config/exchanges.yml
|
||||
EXCHANGES = YAML.safe_load_file(Rails.root.join("config", "exchanges.yml")).freeze
|
||||
|
||||
KINDS = %w[standard cash].freeze
|
||||
|
||||
before_validation :upcase_symbols
|
||||
before_save :generate_logo_url_from_brandfetch, if: :should_generate_logo?
|
||||
|
||||
@@ -14,8 +16,24 @@ class Security < ApplicationRecord
|
||||
|
||||
validates :ticker, presence: true
|
||||
validates :ticker, uniqueness: { scope: :exchange_operating_mic, case_sensitive: false }
|
||||
validates :kind, inclusion: { in: KINDS }
|
||||
|
||||
scope :online, -> { where(offline: false) }
|
||||
scope :standard, -> { where(kind: "standard") }
|
||||
|
||||
# Lazily finds or creates a synthetic cash security for an account.
|
||||
# Used as fallback when creating an interest Trade without a user-selected security.
|
||||
def self.cash_for(account)
|
||||
ticker = "CASH-#{account.id}".upcase
|
||||
find_or_create_by!(ticker: ticker, kind: "cash") do |s|
|
||||
s.name = "Cash"
|
||||
s.offline = true
|
||||
end
|
||||
end
|
||||
|
||||
def cash?
|
||||
kind == "cash"
|
||||
end
|
||||
|
||||
# Returns user-friendly exchange name for a MIC code
|
||||
def self.exchange_name_for(mic)
|
||||
@@ -73,6 +91,7 @@ class Security < ApplicationRecord
|
||||
end
|
||||
|
||||
def should_generate_logo?
|
||||
return false if cash?
|
||||
url = brandfetch_icon_url
|
||||
return false unless url.present?
|
||||
|
||||
|
||||
@@ -30,15 +30,15 @@ class Security::HealthChecker
|
||||
private
|
||||
# If a security has never had a health check, we prioritize it, regardless of batch size
|
||||
def never_checked_scope
|
||||
Security.where(last_health_check_at: nil)
|
||||
Security.standard.where(last_health_check_at: nil)
|
||||
end
|
||||
|
||||
# Any securities not checked for 30 days are due
|
||||
# We only process the batch size, which means some "due" securities will not be checked today
|
||||
# This is by design, to prevent all securities from coming due at the same time
|
||||
def due_for_check_scope
|
||||
Security.where(last_health_check_at: ..HEALTH_CHECK_INTERVAL.ago)
|
||||
.order(last_health_check_at: :asc)
|
||||
Security.standard.where(last_health_check_at: ..HEALTH_CHECK_INTERVAL.ago)
|
||||
.order(last_health_check_at: :asc)
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -31,7 +31,7 @@ class Trade < ApplicationRecord
|
||||
end
|
||||
|
||||
def unrealized_gain_loss
|
||||
return nil if qty.negative?
|
||||
return nil unless qty.positive?
|
||||
current_price = security.current_price
|
||||
return nil if current_price.nil?
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@ class Trade::CreateForm
|
||||
case type
|
||||
when "buy", "sell"
|
||||
create_trade
|
||||
when "dividend"
|
||||
create_dividend_income
|
||||
when "interest"
|
||||
create_interest_income
|
||||
when "deposit", "withdrawal"
|
||||
@@ -28,6 +30,10 @@ class Trade::CreateForm
|
||||
).resolve
|
||||
end
|
||||
|
||||
def ticker_present?
|
||||
ticker.present? || manual_ticker.present?
|
||||
end
|
||||
|
||||
def create_trade
|
||||
signed_qty = type == "sell" ? -qty.to_d : qty.to_d
|
||||
signed_amount = signed_qty * price.to_d + fee.to_d
|
||||
@@ -55,15 +61,47 @@ class Trade::CreateForm
|
||||
trade_entry
|
||||
end
|
||||
|
||||
def create_interest_income
|
||||
signed_amount = amount.to_d * -1
|
||||
# Dividends are always a Trade. Security is required.
|
||||
def create_dividend_income
|
||||
unless ticker_present?
|
||||
entry = account.entries.build(entryable: Trade.new)
|
||||
entry.errors.add(:base, I18n.t("trades.form.dividend_requires_security"))
|
||||
return entry
|
||||
end
|
||||
|
||||
begin
|
||||
sec = security
|
||||
create_income_trade(sec: sec, label: "Dividend", name: "Dividend: #{sec.ticker}")
|
||||
rescue => e
|
||||
Rails.logger.warn("Dividend security resolution failed: #{e.class} - #{e.message}")
|
||||
entry = account.entries.build(entryable: Trade.new)
|
||||
entry.errors.add(:base, I18n.t("trades.form.dividend_requires_security"))
|
||||
entry
|
||||
end
|
||||
end
|
||||
|
||||
# Interest in an investment account is always a Trade.
|
||||
# Falls back to a synthetic cash security when none is selected.
|
||||
def create_interest_income
|
||||
sec = ticker_present? ? security : Security.cash_for(account)
|
||||
name = sec.cash? ? "Interest" : "Interest: #{sec.ticker}"
|
||||
create_income_trade(sec: sec, label: "Interest", name: name)
|
||||
end
|
||||
|
||||
def create_income_trade(sec:, label:, name:)
|
||||
entry = account.entries.build(
|
||||
name: "Interest payment",
|
||||
name: name,
|
||||
date: date,
|
||||
amount: signed_amount,
|
||||
amount: amount.to_d * -1,
|
||||
currency: currency,
|
||||
entryable: Transaction.new
|
||||
entryable: Trade.new(
|
||||
qty: 0,
|
||||
price: 0,
|
||||
fee: 0,
|
||||
currency: currency,
|
||||
security: sec,
|
||||
investment_activity_label: label
|
||||
)
|
||||
)
|
||||
|
||||
if entry.save
|
||||
|
||||
@@ -35,6 +35,7 @@
|
||||
activity_labels = entryable.is_a?(Trade) ? Trade::ACTIVITY_LABELS : Transaction::ACTIVITY_LABELS
|
||||
entryable_type = entryable.is_a?(Trade) ? "Trade" : "Transaction"
|
||||
convert_url = entryable.is_a?(Transaction) ? convert_to_trade_transaction_path(entryable) : nil
|
||||
income_trade = entryable.is_a?(Trade) && %w[Dividend Interest].include?(label)
|
||||
%>
|
||||
|
||||
<div class="relative"
|
||||
@@ -48,7 +49,7 @@
|
||||
<% end %>>
|
||||
|
||||
<button type="button"
|
||||
class="inline-flex items-center gap-1 text-xs font-medium rounded-full px-2.5 py-1 cursor-pointer hover:opacity-80 transition-opacity"
|
||||
class="inline-flex items-center gap-1 text-xs font-medium rounded-full px-2.5 py-1 <%= income_trade ? "cursor-default" : "cursor-pointer hover:opacity-80" %> transition-opacity"
|
||||
style="<% if has_label %>
|
||||
background-color: color-mix(in oklab, <%= color %> 15%, transparent);
|
||||
border: 1px solid color-mix(in oklab, <%= color %> 25%, transparent);
|
||||
@@ -58,8 +59,10 @@
|
||||
border: 1px solid var(--color-border-secondary);
|
||||
color: var(--color-text-secondary);
|
||||
<% end %>"
|
||||
<% unless income_trade %>
|
||||
data-action="click->activity-label-quick-edit#toggle"
|
||||
data-activity-label-quick-edit-target="badge"
|
||||
<% end %>
|
||||
title="<%= has_label ? t("transactions.transaction.activity_type_tooltip") : t("transactions.show.activity_type") %>">
|
||||
<% if has_label %>
|
||||
<%= label %>
|
||||
@@ -67,29 +70,33 @@
|
||||
<%= icon "tag", size: "xs", color: "current" %>
|
||||
<span><%= t("transactions.show.activity_type") %></span>
|
||||
<% end %>
|
||||
<%= icon "chevron-down", size: "xs", color: "current" %>
|
||||
<% unless income_trade %>
|
||||
<%= icon "chevron-down", size: "xs", color: "current" %>
|
||||
<% end %>
|
||||
</button>
|
||||
|
||||
<!-- Dropdown menu -->
|
||||
<div class="hidden absolute right-0 z-50 mt-1 w-44 rounded-lg border border-primary bg-container shadow-lg"
|
||||
data-activity-label-quick-edit-target="dropdown">
|
||||
<div class="py-1 max-h-64 overflow-y-auto">
|
||||
<% if has_label %>
|
||||
<button type="button"
|
||||
class="w-full text-left px-3 py-1.5 text-sm text-secondary hover:bg-surface-inset transition-colors border-b border-primary"
|
||||
data-action="click->activity-label-quick-edit#select"
|
||||
data-label="">
|
||||
<span class="italic"><%= t("transactions.form.none") %></span>
|
||||
</button>
|
||||
<% end %>
|
||||
<% activity_labels.each do |activity_label| %>
|
||||
<button type="button"
|
||||
class="w-full text-left px-3 py-1.5 text-sm text-primary hover:bg-surface-inset transition-colors <%= activity_label == label ? "font-semibold bg-surface-inset" : "" %>"
|
||||
data-action="click->activity-label-quick-edit#select"
|
||||
data-label="<%= activity_label %>">
|
||||
<%= activity_label %>
|
||||
</button>
|
||||
<% end %>
|
||||
<% unless income_trade %>
|
||||
<!-- Dropdown menu -->
|
||||
<div class="hidden absolute right-0 z-50 mt-1 w-44 rounded-lg border border-primary bg-container shadow-lg"
|
||||
data-activity-label-quick-edit-target="dropdown">
|
||||
<div class="py-1 max-h-64 overflow-y-auto">
|
||||
<% if has_label %>
|
||||
<button type="button"
|
||||
class="w-full text-left px-3 py-1.5 text-sm text-secondary hover:bg-surface-inset transition-colors border-b border-primary"
|
||||
data-action="click->activity-label-quick-edit#select"
|
||||
data-label="">
|
||||
<span class="italic"><%= t("transactions.form.none") %></span>
|
||||
</button>
|
||||
<% end %>
|
||||
<% activity_labels.each do |activity_label| %>
|
||||
<button type="button"
|
||||
class="w-full text-left px-3 py-1.5 text-sm text-primary hover:bg-surface-inset transition-colors <%= activity_label == label ? "font-semibold bg-surface-inset" : "" %>"
|
||||
data-action="click->activity-label-quick-edit#select"
|
||||
data-label="<%= activity_label %>">
|
||||
<%= activity_label %>
|
||||
</button>
|
||||
<% end %>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
@@ -10,11 +10,12 @@
|
||||
|
||||
<div class="space-y-2">
|
||||
<%= form.select :type, [
|
||||
["Buy", "buy"],
|
||||
["Sell", "sell"],
|
||||
["Deposit", "deposit"],
|
||||
["Withdrawal", "withdrawal"],
|
||||
["Interest", "interest"]
|
||||
[t(".type_buy"), "buy"],
|
||||
[t(".type_sell"), "sell"],
|
||||
[t(".type_deposit"), "deposit"],
|
||||
[t(".type_withdrawal"), "withdrawal"],
|
||||
[t(".type_dividend"), "dividend"],
|
||||
[t(".type_interest"), "interest"]
|
||||
],
|
||||
{ label: t(".type"), selected: type },
|
||||
{ data: {
|
||||
@@ -34,10 +35,19 @@
|
||||
required: true %>
|
||||
</div>
|
||||
<% else %>
|
||||
<%= form.text_field :manual_ticker, label: "Ticker symbol", placeholder: "AAPL", required: true %>
|
||||
<%= form.text_field :manual_ticker, label: t(".holding"), placeholder: t(".ticker_placeholder"), required: true %>
|
||||
<% end %>
|
||||
<% end %>
|
||||
|
||||
<% if %w[dividend interest].include?(type) %>
|
||||
<% account_securities = account ? account.traded_standard_securities : [] %>
|
||||
<% security_options = account_securities.map { |s| [ s.name.presence || s.ticker, s.exchange_operating_mic.present? ? "#{s.ticker}|#{s.exchange_operating_mic}" : s.ticker ] } %>
|
||||
<% select_options = { label: type == "dividend" ? t(".holding") : t(".holding_optional") } %>
|
||||
<% select_options[:include_blank] = true if type == "interest" %>
|
||||
<% select_options[:required] = true if type == "dividend" %>
|
||||
<%= form.select :ticker, security_options, select_options %>
|
||||
<% end %>
|
||||
|
||||
<%= form.date_field :date, label: true, value: model.date || Date.current, required: true %>
|
||||
|
||||
<% unless %w[buy sell].include?(type) %>
|
||||
|
||||
@@ -2,14 +2,20 @@
|
||||
|
||||
<div id="<%= dom_id(entry, :header) %>">
|
||||
<%= tag.header class: "mb-4 space-y-1" do %>
|
||||
<% label = entry.trade.investment_activity_label %>
|
||||
<% income_trade = %w[Dividend Interest].include?(label) %>
|
||||
<span class="text-secondary text-sm">
|
||||
<%= entry.amount.positive? ? t(".buy") : t(".sell") %>
|
||||
<% if income_trade %>
|
||||
<%= t(".#{label.downcase}") %>
|
||||
<% else %>
|
||||
<%= entry.amount.positive? ? t(".buy") : t(".sell") %>
|
||||
<% end %>
|
||||
</span>
|
||||
|
||||
<div class="flex items-center gap-4">
|
||||
<h3 class="font-medium">
|
||||
<span class="text-2xl text-primary privacy-sensitive">
|
||||
<%= format_money entry.amount_money %>
|
||||
<%= format_money(income_trade ? entry.amount_money.abs : entry.amount_money) %>
|
||||
</span>
|
||||
|
||||
<span class="text-lg text-secondary">
|
||||
@@ -30,15 +36,16 @@
|
||||
<% end %>
|
||||
|
||||
<% trade = entry.trade %>
|
||||
|
||||
|
||||
<% unless trade.security.cash? %>
|
||||
<div class="mb-2">
|
||||
<%= render DS::Disclosure.new(title: t(".overview"), open: true) do %>
|
||||
<div class="pb-4">
|
||||
<dl class="space-y-3 px-3 py-2">
|
||||
<div class="flex items-center justify-between text-sm">
|
||||
<dt class="text-secondary"><%= t(".symbol_label") %></dt>
|
||||
<dd class="text-primary"><%= trade.security.ticker %></dd>
|
||||
</div>
|
||||
<dl class="space-y-3 px-3 py-2">
|
||||
<div class="flex items-center justify-between text-sm">
|
||||
<dt class="text-secondary"><%= t(".symbol_label") %></dt>
|
||||
<dd class="text-primary"><%= trade.security.ticker %></dd>
|
||||
</div>
|
||||
|
||||
<% if trade.qty.positive? %>
|
||||
<div class="flex items-center justify-between text-sm">
|
||||
@@ -52,7 +59,7 @@
|
||||
</div>
|
||||
<% end %>
|
||||
|
||||
<% if trade.security.current_price.present? %>
|
||||
<% if trade.qty.positive? && trade.security.current_price.present? %>
|
||||
<div class="flex items-center justify-between text-sm">
|
||||
<dt class="text-secondary"><%= t(".current_market_price_label") %></dt>
|
||||
<dd class="text-primary privacy-sensitive"><%= format_money trade.security.current_price %></dd>
|
||||
@@ -71,4 +78,5 @@
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
@@ -19,37 +19,48 @@
|
||||
max: Date.current,
|
||||
disabled: @entry.linked?,
|
||||
"data-auto-submit-form-target": "auto" %>
|
||||
<div class="flex items-center gap-2">
|
||||
<%= f.select :nature,
|
||||
[[t(".buy"), "outflow"], [t(".sell"), "inflow"]],
|
||||
{ container_class: "w-1/3", label: t(".type_label"), selected: @entry.amount.positive? ? "outflow" : "inflow" },
|
||||
{ data: { "auto-submit-form-target": "auto" }, disabled: @entry.linked? } %>
|
||||
<%= f.fields_for :entryable do |ef| %>
|
||||
<%= ef.number_field :qty,
|
||||
label: t(".quantity_label"),
|
||||
step: "any",
|
||||
value: trade.qty.abs,
|
||||
"data-auto-submit-form-target": "auto",
|
||||
disabled: @entry.linked? %>
|
||||
<% end %>
|
||||
</div>
|
||||
<%= f.fields_for :entryable do |ef| %>
|
||||
<%= ef.money_field :price,
|
||||
label: t(".cost_per_share_label"),
|
||||
disable_currency: true,
|
||||
<% if trade.qty.zero? %>
|
||||
<%= f.money_field :amount,
|
||||
label: t(".amount_label"),
|
||||
value: @entry.amount_money.abs,
|
||||
auto_submit: true,
|
||||
min: 0,
|
||||
step: "any",
|
||||
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? %>
|
||||
<% unless trade.qty.zero? %>
|
||||
<div class="flex items-center gap-2">
|
||||
<%= f.select :nature,
|
||||
[[t(".buy"), "outflow"], [t(".sell"), "inflow"]],
|
||||
{ container_class: "w-1/3", label: t(".type_label"), selected: @entry.amount.positive? ? "outflow" : "inflow" },
|
||||
{ data: { "auto-submit-form-target": "auto" }, disabled: @entry.linked? } %>
|
||||
<%= f.fields_for :entryable do |ef| %>
|
||||
<%= ef.number_field :qty,
|
||||
label: t(".quantity_label"),
|
||||
step: "any",
|
||||
value: trade.qty.abs,
|
||||
"data-auto-submit-form-target": "auto",
|
||||
disabled: @entry.linked? %>
|
||||
<% end %>
|
||||
</div>
|
||||
<%= f.fields_for :entryable do |ef| %>
|
||||
<%= ef.money_field :price,
|
||||
label: t(".cost_per_share_label"),
|
||||
disable_currency: true,
|
||||
auto_submit: true,
|
||||
min: 0,
|
||||
step: "any",
|
||||
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 %>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user