Add exchange and currency fields to trade imports #1822
@@ -29,6 +29,7 @@ class Import::ConfigurationsController < ApplicationController
|
||||
:account_col_label,
|
||||
|
|
||||
:qty_col_label,
|
||||
:ticker_col_label,
|
||||
:exchange_operating_mic_col_label,
|
||||
:price_col_label,
|
||||
:entity_type_col_label,
|
||||
:notes_col_label,
|
||||
|
||||
@@ -20,6 +20,7 @@ module ImportsHelper
|
||||
notes: "Notes",
|
||||
qty: "Quantity",
|
||||
ticker: "Ticker",
|
||||
exchange: "Exchange",
|
||||
price: "Price",
|
||||
entity_type: "Type"
|
||||
}[key]
|
||||
|
||||
@@ -111,6 +111,7 @@ class Import < ApplicationRecord
|
||||
date: row[date_col_label].to_s,
|
||||
qty: sanitize_number(row[qty_col_label]).to_s,
|
||||
ticker: row[ticker_col_label].to_s,
|
||||
exchange_operating_mic: row[exchange_operating_mic_col_label].to_s,
|
||||
price: sanitize_number(row[price_col_label]).to_s,
|
||||
amount: sanitize_number(row[amount_col_label]).to_s,
|
||||
currency: (row[currency_col_label] || default_currency).to_s,
|
||||
|
||||
@@ -128,11 +128,12 @@ class Provider::Synth
|
||||
raw_response: error
|
||||
end
|
||||
|
||||
def search_securities(query:, dataset: "limited", country_code:)
|
||||
def search_securities(query:, dataset: "limited", country_code: nil, exchange_operating_mic: nil)
|
||||
response = client.get("#{base_url}/tickers/search") do |req|
|
||||
req.params["name"] = query
|
||||
req.params["dataset"] = dataset
|
||||
req.params["country_code"] = country_code
|
||||
req.params["country_code"] = country_code if country_code.present?
|
||||
req.params["exchange_operating_mic"] = exchange_operating_mic if exchange_operating_mic.present?
|
||||
req.params["limit"] = 25
|
||||
end
|
||||
|
||||
|
||||
@@ -13,7 +13,8 @@ class Security < ApplicationRecord
|
||||
security_prices_provider.search_securities(
|
||||
query: query[:search],
|
||||
dataset: "limited",
|
||||
country_code: query[:country]
|
||||
country_code: query[:country],
|
||||
exchange_operating_mic: query[:exchange_operating_mic]
|
||||
).securities.map { |attrs| new(**attrs) }
|
||||
end
|
||||
end
|
||||
|
||||
@@ -5,14 +5,24 @@ class TradeImport < Import
|
||||
|
||||
rows.each do |row|
|
||||
account = mappings.accounts.mappable_for(row.account)
|
||||
security = Security.find_or_create_by(ticker: row.ticker)
|
||||
|
||||
# Try to find or create security with ticker only
|
||||
security = find_or_create_security(
|
||||
ticker: row.ticker,
|
||||
exchange_operating_mic: row.exchange_operating_mic
|
||||
)
|
||||
|
||||
entry = account.entries.build \
|
||||
date: row.date_iso,
|
||||
amount: row.signed_amount,
|
||||
name: row.name,
|
||||
currency: row.currency,
|
||||
entryable: Account::Trade.new(security: security, qty: row.qty, currency: row.currency, price: row.price),
|
||||
currency: row.currency.presence || account.currency,
|
||||
entryable: Account::Trade.new(
|
||||
security: security,
|
||||
qty: row.qty,
|
||||
currency: row.currency.presence || account.currency,
|
||||
price: row.price
|
||||
),
|
||||
import: self
|
||||
|
||||
entry.save!
|
||||
@@ -29,7 +39,7 @@ class TradeImport < Import
|
||||
end
|
||||
|
||||
def column_keys
|
||||
%i[date ticker qty price currency account name]
|
||||
%i[date ticker exchange_operating_mic currency qty price account name]
|
||||
end
|
||||
|
||||
def dry_run
|
||||
@@ -41,12 +51,63 @@ class TradeImport < Import
|
||||
|
||||
|
We'll want to remove the We'll want to remove the `exchange_operating_mic` from this so that our `Security` model properly identifies it as "missing prices":
https://github.com/maybe-finance/maybe/blob/79e1a2c0ffd5191fe6c040da68a311e1c1e6beb4/app/models/security.rb#L38-L40
We need to explicitly set We need to explicitly set `exchange_operating_mic: nil` here as I had in my snippet below to prevent invalid exchange mics from being persisted to the DB.
The issue with adding in this logic is in the case where the user has provided a valid ticker + exchange mic in their CSV that doesn't exist in our current DB. For example, let's say they provide I think given that scenario, we should remove this and go with something like the example code I provided: https://github.com/maybe-finance/maybe/pull/1822/files#r1963740617 ```rb
# If no exact match and no exchange_operating_mic was provided, try to find any security with the same ticker
if exchange_operating_mic.nil?
internal_security = Security.where(ticker:).first
if internal_security.present?
internal_security.update!(exchange_operating_mic: nil)
return internal_security
end
end
# If we couldn't find the security locally and the provider isn't available, create with provided info
return Security.create!(ticker: ticker, exchange_operating_mic: exchange_operating_mic) unless Security.security_prices_provider.present?
```
The issue with adding in this logic is in the case where the user has provided a valid ticker + exchange mic in their CSV that doesn't exist in our current DB. For example, let's say they provide `AAPL` + `XNAS` (valid in Synth) and it is not currently in our DB, but there _is_ a `Security` in our DB of `[`AAPL`, `nil`]`. This logic will pick that existing security up even though what we want to do is go fetch from Synth to find the new security.
I think given that scenario, we should remove this and go with something like the example code I provided: https://github.com/maybe-finance/maybe/pull/1822/files#r1963740617
|
||||
def csv_template
|
||||
template = <<-CSV
|
||||
date*,ticker*,qty*,price*,currency,account,name
|
||||
05/15/2024,AAPL,10,150.00,USD,Trading Account,Apple Inc. Purchase
|
||||
05/16/2024,GOOGL,-5,2500.00,USD,Investment Account,Alphabet Inc. Sale
|
||||
05/17/2024,TSLA,2,700.50,USD,Retirement Account,Tesla Inc. Purchase
|
||||
date*,ticker*,exchange_operating_mic,currency,qty*,price*,account,name
|
||||
05/15/2024,AAPL,XNAS,USD,10,150.00,Trading Account,Apple Inc. Purchase
|
||||
05/16/2024,GOOGL,XNAS,USD,-5,2500.00,Investment Account,Alphabet Inc. Sale
|
||||
05/17/2024,TSLA,XNAS,USD,2,700.50,Retirement Account,Tesla Inc. Purchase
|
||||
CSV
|
||||
|
||||
CSV.parse(template, headers: true)
|
||||
end
|
||||
|
||||
private
|
||||
def find_or_create_security(ticker:, exchange_operating_mic:)
|
||||
# Normalize empty string to nil for consistency
|
||||
exchange_operating_mic = nil if exchange_operating_mic.blank?
|
||||
|
||||
# First try to find an exact match in our DB, or if no exchange_operating_mic is provided, find by ticker only
|
||||
internal_security = if exchange_operating_mic.present?
|
||||
Security.find_by(ticker:, exchange_operating_mic:)
|
||||
else
|
||||
Security.find_by(ticker:)
|
||||
end
|
||||
|
||||
return internal_security if internal_security.present?
|
||||
|
||||
# If security prices provider isn't properly configured or available, create with nil exchange_operating_mic
|
||||
provider = Security.security_prices_provider
|
||||
unless provider.present? && provider.respond_to?(:search_securities)
|
||||
return Security.find_or_create_by!(ticker: ticker, exchange_operating_mic: nil)
|
||||
end
|
||||
|
||||
# Cache provider responses so that when we're looping through rows and importing,
|
||||
# we only hit our provider for the unique combinations of ticker / exchange_operating_mic
|
||||
cache_key = [ ticker, exchange_operating_mic ]
|
||||
@provider_securities_cache ||= {}
|
||||
|
||||
provider_security = @provider_securities_cache[cache_key] ||= begin
|
||||
response = provider.search_securities(
|
||||
query: ticker,
|
||||
exchange_operating_mic: exchange_operating_mic
|
||||
)
|
||||
|
||||
if !response || !response.success? || !response.securities || response.securities.empty?
|
||||
nil
|
||||
else
|
||||
response.securities.first
|
||||
end
|
||||
rescue => e
|
||||
nil
|
||||
end
|
||||
|
||||
return Security.find_or_create_by!(ticker: ticker, exchange_operating_mic: nil) if provider_security.nil?
|
||||
|
||||
Security.find_or_create_by!(ticker: provider_security[:ticker], exchange_operating_mic: provider_security[:exchange_operating_mic]) do |security|
|
||||
security.name = provider_security[:name]
|
||||
security.country_code = provider_security[:country_code]
|
||||
security.logo_url = provider_security[:logo_url]
|
||||
security.exchange_acronym = provider_security[:exchange_acronym]
|
||||
security.exchange_mic = provider_security[:exchange_mic]
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,25 +1,37 @@
|
||||
<%# locals: (import:) %>
|
||||
|
||||
<%= styled_form_with model: @import, url: import_configuration_path(@import), scope: :import, method: :patch, class: "space-y-2" do |form| %>
|
||||
<div class="flex items-center gap-2">
|
||||
<%= form.select :date_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Date" }, required: true %>
|
||||
<%= form.select :date_format, Family::DATE_FORMATS, { label: t(".date_format_label")}, label: true, required: true %>
|
||||
</div>
|
||||
<div class="space-y-4">
|
||||
<div class="flex items-center gap-2">
|
||||
<%= form.select :date_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Date" }, required: true %>
|
||||
<%= form.select :date_format, Family::DATE_FORMATS, { label: t(".date_format_label")}, label: true, required: true %>
|
||||
</div>
|
||||
|
||||
<div class="flex items-center gap-2">
|
||||
<%= form.select :qty_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Quantity" } %>
|
||||
<%= form.select :signage_convention, [["Buys are positive qty", "inflows_positive"], ["Buys are negative qty", "inflows_negative"]], label: true %>
|
||||
</div>
|
||||
<div class="flex items-center gap-2">
|
||||
<%= form.select :qty_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Quantity" } %>
|
||||
<%= form.select :signage_convention, [["Buys are positive qty", "inflows_positive"], ["Buys are negative qty", "inflows_negative"]], label: true %>
|
||||
</div>
|
||||
|
||||
<div class="flex items-center gap-2">
|
||||
<%= form.select :currency_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Currency" } %>
|
||||
<%= form.select :number_format, Import::NUMBER_FORMATS.keys, { label: "Format", prompt: "Select format" }, required: true %>
|
||||
</div>
|
||||
<div class="flex items-center gap-2">
|
||||
<%= form.select :currency_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Currency" } %>
|
||||
<%= form.select :number_format, Import::NUMBER_FORMATS.keys, { label: "Format", prompt: "Select format" }, required: true %>
|
||||
</div>
|
||||
|
||||
<%= form.select :ticker_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Ticker" } %>
|
||||
<%= form.select :price_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Price" } %>
|
||||
<%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" } %>
|
||||
<%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" } %>
|
||||
<%= form.select :ticker_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Ticker" } %>
|
||||
<%= form.select :exchange_operating_mic_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Exchange Operating MIC" } %>
|
||||
<%= form.select :price_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Price" } %>
|
||||
<%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" } %>
|
||||
<%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" } %>
|
||||
|
||||
<% if Security.security_prices_provider.nil? %>
|
||||
<div class="alert alert-warning">
|
||||
<p>
|
||||
<strong>Note:</strong> The Synth provider is not configured. Exchange validation is disabled.
|
||||
Securities will be created without exchange validation, and price history will not be available.
|
||||
</p>
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<%= form.submit "Apply configuration", class: "w-full btn btn--primary", disabled: import.complete? %>
|
||||
<% end %>
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
class UpdateImportsForOperatingMicV2 < ActiveRecord::Migration[7.2]
|
||||
def change
|
||||
add_column :import_rows, :exchange_operating_mic, :string
|
||||
add_column :imports, :exchange_operating_mic_col_label, :string
|
||||
end
|
||||
end
|
||||
|
With rails we should always prefer a With rails we should always prefer a `change` migration as it's much easier to read and reason about. Here's what this migration should be:
```rb
def change
add_column :import_rows, :exchange_operating_mic, :string
add_column :imports, :exchange_operating_mic_col_label, :string
end
```
|
||||
4
db/schema.rb
generated
@@ -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_02_12_213301) do
|
||||
ActiveRecord::Schema[7.2].define(version: 2025_02_20_153958) do
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "pgcrypto"
|
||||
enable_extension "plpgsql"
|
||||
@@ -385,6 +385,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_12_213301) do
|
||||
t.text "notes"
|
||||
t.datetime "created_at", null: false
|
||||
t.datetime "updated_at", null: false
|
||||
t.string "exchange_operating_mic"
|
||||
t.index ["import_id"], name: "index_import_rows_on_import_id"
|
||||
end
|
||||
|
||||
@@ -415,6 +416,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_12_213301) do
|
||||
t.string "signage_convention", default: "inflows_positive"
|
||||
t.string "error"
|
||||
t.string "number_format"
|
||||
t.string "exchange_operating_mic_col_label"
|
||||
t.index ["family_id"], name: "index_imports_on_family_id"
|
||||
end
|
||||
|
||||
|
||||
5
test/fixtures/imports.yml
vendored
@@ -2,3 +2,8 @@ transaction:
|
||||
family: dylan_family
|
||||
type: TransactionImport
|
||||
status: pending
|
||||
|
||||
trade:
|
||||
family: dylan_family
|
||||
type: TradeImport
|
||||
status: pending
|
||||
|
||||
85
test/models/trade_import_test.rb
Normal file
@@ -0,0 +1,85 @@
|
||||
require "test_helper"
|
||||
require "ostruct"
|
||||
|
||||
class TradeImportTest < ActiveSupport::TestCase
|
||||
include ActiveJob::TestHelper, ImportInterfaceTest
|
||||
|
||||
setup do
|
||||
@subject = @import = imports(:trade)
|
||||
end
|
||||
|
||||
test "imports trades and accounts" do
|
||||
# Create an existing AAPL security with no exchange_operating_mic
|
||||
aapl = Security.create!(ticker: "AAPL", exchange_operating_mic: nil)
|
||||
|
||||
provider = mock
|
||||
|
||||
# We should only hit the provider for GOOGL since AAPL already exists
|
||||
provider.expects(:search_securities).with(
|
||||
query: "GOOGL",
|
||||
exchange_operating_mic: "XNAS"
|
||||
).returns(
|
||||
OpenStruct.new(
|
||||
securities: [
|
||||
{
|
||||
ticker: "GOOGL",
|
||||
name: "Google Inc.",
|
||||
country_code: "US",
|
||||
exchange_mic: "XNGS",
|
||||
exchange_operating_mic: "XNAS",
|
||||
exchange_acronym: "NGS"
|
||||
}
|
||||
],
|
||||
success?: true,
|
||||
raw_response: nil
|
||||
)
|
||||
).once
|
||||
|
||||
Security.stubs(:security_prices_provider).returns(provider)
|
||||
|
||||
import = <<~CSV
|
||||
date,ticker,qty,price,currency,account,name,exchange_operating_mic
|
||||
01/01/2024,AAPL,10,150.00,USD,TestAccount1,Apple Purchase,
|
||||
01/02/2024,GOOGL,5,2500.00,USD,TestAccount1,Google Purchase,XNAS
|
||||
CSV
|
||||
|
||||
@import.update!(
|
||||
raw_file_str: import,
|
||||
date_col_label: "date",
|
||||
ticker_col_label: "ticker",
|
||||
qty_col_label: "qty",
|
||||
price_col_label: "price",
|
||||
exchange_operating_mic_col_label: "exchange_operating_mic",
|
||||
date_format: "%m/%d/%Y",
|
||||
signage_convention: "inflows_positive"
|
||||
)
|
||||
|
||||
@import.generate_rows_from_csv
|
||||
|
||||
@import.mappings.create! key: "TestAccount1", create_when_empty: true, type: "Import::AccountMapping"
|
||||
|
||||
@import.reload
|
||||
|
||||
assert_difference [
|
||||
-> { Account::Entry.count },
|
||||
-> { Account::Trade.count }
|
||||
], 2 do
|
||||
assert_difference [
|
||||
-> { Security.count },
|
||||
-> { Account.count }
|
||||
], 1 do
|
||||
@import.publish
|
||||
end
|
||||
end
|
||||
|
||||
assert_equal "complete", @import.status
|
||||
|
||||
# Verify the securities were created/updated correctly
|
||||
aapl.reload
|
||||
assert_nil aapl.exchange_operating_mic
|
||||
|
||||
googl = Security.find_by(ticker: "GOOGL")
|
||||
assert_equal "XNAS", googl.exchange_operating_mic
|
||||
assert_equal "XNGS", googl.exchange_mic
|
||||
end
|
||||
end
|
||||
I think this needs updating