From d8e058d7c6112d23fa9d823cfcf9861f779edbab Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Thu, 8 May 2025 14:31:43 -0400
Subject: [PATCH] Handle case sensitive values when creating securities

---
 app/models/plaid_investment_sync.rb |  4 +--
 app/models/security.rb              |  6 ++---
 app/models/trade_import.rb          |  6 ++---
 test/models/security_test.rb        | 41 +++++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 8 deletions(-)
 create mode 100644 test/models/security_test.rb

diff --git a/app/models/plaid_investment_sync.rb b/app/models/plaid_investment_sync.rb
index 34f0d7e4..cc0d56a6 100644
--- a/app/models/plaid_investment_sync.rb
+++ b/app/models/plaid_investment_sync.rb
@@ -106,8 +106,8 @@ class PlaidInvestmentSync
 
       # Find any matching security
       security = Security.find_or_create_by!(
-        ticker: plaid_security.ticker_symbol,
-        exchange_operating_mic: operating_mic
+        ticker: plaid_security.ticker_symbol&.upcase,
+        exchange_operating_mic: operating_mic&.upcase
       )
 
       [ security, plaid_security ]
diff --git a/app/models/security.rb b/app/models/security.rb
index 0adabd8a..6115aa4c 100644
--- a/app/models/security.rb
+++ b/app/models/security.rb
@@ -1,7 +1,7 @@
 class Security < ApplicationRecord
   include Provided
 
-  before_save :upcase_ticker
+  before_validation :upcase_symbols
 
   has_many :trades, dependent: :nullify, class_name: "Trade"
   has_many :prices, dependent: :destroy
@@ -29,8 +29,8 @@ class Security < ApplicationRecord
   end
 
   private
-
-    def upcase_ticker
+    def upcase_symbols
       self.ticker = ticker.upcase
+      self.exchange_operating_mic = exchange_operating_mic.upcase if exchange_operating_mic.present?
     end
 end
diff --git a/app/models/trade_import.rb b/app/models/trade_import.rb
index 04a6a4cf..e7a57f64 100644
--- a/app/models/trade_import.rb
+++ b/app/models/trade_import.rb
@@ -90,7 +90,7 @@ class TradeImport < Import
       return internal_security if internal_security.present?
 
       # If security prices provider isn't properly configured or available, create with nil exchange_operating_mic
-      return Security.find_or_create_by!(ticker: ticker, exchange_operating_mic: nil) unless Security.provider.present?
+      return Security.find_or_create_by!(ticker: ticker&.upcase, exchange_operating_mic: nil) unless Security.provider.present?
 
       # 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
@@ -104,9 +104,9 @@ class TradeImport < Import
         ).first
       end
 
-      return Security.find_or_create_by!(ticker: ticker, exchange_operating_mic: nil) if provider_security.nil?
+      return Security.find_or_create_by!(ticker: ticker&.upcase, 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.find_or_create_by!(ticker: provider_security[:ticker]&.upcase, exchange_operating_mic: provider_security[:exchange_operating_mic]&.upcase) do |security|
         security.name = provider_security[:name]
         security.country_code = provider_security[:country_code]
         security.logo_url = provider_security[:logo_url]
diff --git a/test/models/security_test.rb b/test/models/security_test.rb
new file mode 100644
index 00000000..0e6ef3c8
--- /dev/null
+++ b/test/models/security_test.rb
@@ -0,0 +1,41 @@
+require "test_helper"
+
+class SecurityTest < ActiveSupport::TestCase
+  # Below has 3 example scenarios:
+  # 1. Original ticker
+  # 2. Duplicate ticker on a different exchange (different market price)
+  # 3. "Offline" version of the same ticker (for users not connected to a provider)
+  test "can have duplicate tickers if exchange is different" do
+    original = Security.create!(ticker: "TEST", exchange_operating_mic: "XNAS")
+    duplicate = Security.create!(ticker: "TEST", exchange_operating_mic: "CBOE")
+    offline = Security.create!(ticker: "TEST", exchange_operating_mic: nil)
+
+    assert original.valid?
+    assert duplicate.valid?
+    assert offline.valid?
+  end
+
+  test "cannot have duplicate tickers if exchange is the same" do
+    original = Security.create!(ticker: "TEST", exchange_operating_mic: "XNAS")
+    duplicate = Security.new(ticker: "TEST", exchange_operating_mic: "XNAS")
+
+    assert_not duplicate.valid?
+    assert_equal [ "has already been taken" ], duplicate.errors[:ticker]
+  end
+
+  test "cannot have duplicate tickers if exchange is nil" do
+    original = Security.create!(ticker: "TEST", exchange_operating_mic: nil)
+    duplicate = Security.new(ticker: "TEST", exchange_operating_mic: nil)
+
+    assert_not duplicate.valid?
+    assert_equal [ "has already been taken" ], duplicate.errors[:ticker]
+  end
+
+  test "casing is ignored when checking for duplicates" do
+    original = Security.create!(ticker: "TEST", exchange_operating_mic: "XNAS")
+    duplicate = Security.new(ticker: "tEst", exchange_operating_mic: "xNaS")
+
+    assert_not duplicate.valid?
+    assert_equal [ "has already been taken" ], duplicate.errors[:ticker]
+  end
+end