From 32ef6ca15406318d53b917f2d17188bc9a2e1719 Mon Sep 17 00:00:00 2001
From: David Anyatonwu <51977119+onyedikachi-david@users.noreply.github.com>
Date: Mon, 24 Feb 2025 16:00:24 +0100
Subject: [PATCH] Add exchange and currency fields to trade imports (#1822)

* Add exchange and currency fields to trade imports

* Add exchange_operating_mic support for trade imports - Added required columns and updated models

* refactor: remove exchange and currency columns

* fix: consolidate import schema and remove redundant columns

* feat: Enhance trade import with exchange_operating_mic support

* Revert changes to existing migration

* Simplify migration to use change method

* Restore previously deleted migration

* Remove unused import_col_labels method

* Update schema.rb after running migrations

* Update trade_import.rb and fix schema.rb with db:migrate:reset

* fix: improve trade import security creation

---------

Signed-off-by: David Anyatonwu <51977119+onyedikachi-david@users.noreply.github.com>
---
 .../import/configurations_controller.rb       |  1 +
 app/helpers/imports_helper.rb                 |  1 +
 app/models/import.rb                          |  1 +
 app/models/provider/synth.rb                  |  5 +-
 app/models/security.rb                        |  3 +-
 app/models/trade_import.rb                    | 77 +++++++++++++++--
 .../configurations/_trade_import.html.erb     | 44 ++++++----
 ...958_update_imports_for_operating_mic_v2.rb |  6 ++
 db/schema.rb                                  |  4 +-
 test/fixtures/imports.yml                     |  5 ++
 test/models/trade_import_test.rb              | 85 +++++++++++++++++++
 11 files changed, 204 insertions(+), 28 deletions(-)
 create mode 100644 db/migrate/20250220153958_update_imports_for_operating_mic_v2.rb
 create mode 100644 test/models/trade_import_test.rb

diff --git a/app/controllers/import/configurations_controller.rb b/app/controllers/import/configurations_controller.rb
index b1ae9e50..9a060f8f 100644
--- a/app/controllers/import/configurations_controller.rb
+++ b/app/controllers/import/configurations_controller.rb
@@ -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,
diff --git a/app/helpers/imports_helper.rb b/app/helpers/imports_helper.rb
index b7718ecf..67930cc1 100644
--- a/app/helpers/imports_helper.rb
+++ b/app/helpers/imports_helper.rb
@@ -20,6 +20,7 @@ module ImportsHelper
       notes: "Notes",
       qty: "Quantity",
       ticker: "Ticker",
+      exchange: "Exchange",
       price: "Price",
       entity_type: "Type"
     }[key]
diff --git a/app/models/import.rb b/app/models/import.rb
index b28c3c81..9542e187 100644
--- a/app/models/import.rb
+++ b/app/models/import.rb
@@ -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,
diff --git a/app/models/provider/synth.rb b/app/models/provider/synth.rb
index 65deaae5..829a5b7f 100644
--- a/app/models/provider/synth.rb
+++ b/app/models/provider/synth.rb
@@ -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
 
diff --git a/app/models/security.rb b/app/models/security.rb
index 4a2bd1a7..46672fb5 100644
--- a/app/models/security.rb
+++ b/app/models/security.rb
@@ -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
diff --git a/app/models/trade_import.rb b/app/models/trade_import.rb
index 4ca57ea1..ddaad904 100644
--- a/app/models/trade_import.rb
+++ b/app/models/trade_import.rb
@@ -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
 
   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
diff --git a/app/views/import/configurations/_trade_import.html.erb b/app/views/import/configurations/_trade_import.html.erb
index e49d29c3..a8d056b0 100644
--- a/app/views/import/configurations/_trade_import.html.erb
+++ b/app/views/import/configurations/_trade_import.html.erb
@@ -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 %>
diff --git a/db/migrate/20250220153958_update_imports_for_operating_mic_v2.rb b/db/migrate/20250220153958_update_imports_for_operating_mic_v2.rb
new file mode 100644
index 00000000..11f10191
--- /dev/null
+++ b/db/migrate/20250220153958_update_imports_for_operating_mic_v2.rb
@@ -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
diff --git a/db/schema.rb b/db/schema.rb
index c5239fba..ccbe329a 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_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
 
diff --git a/test/fixtures/imports.yml b/test/fixtures/imports.yml
index 97e2cd72..366bb6d9 100644
--- a/test/fixtures/imports.yml
+++ b/test/fixtures/imports.yml
@@ -2,3 +2,8 @@ transaction:
   family: dylan_family
   type: TransactionImport
   status: pending
+
+trade:
+  family: dylan_family
+  type: TradeImport
+  status: pending
diff --git a/test/models/trade_import_test.rb b/test/models/trade_import_test.rb
new file mode 100644
index 00000000..2900034e
--- /dev/null
+++ b/test/models/trade_import_test.rb
@@ -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