Handle missing weekend stock prices in sync process (#1242)

* Don't append missing prices if already known

* Add failing test

* Handle weekend stock prices

* Fix tests and gapfill logic

app/models/account/holding/syncer.rb

@@ -48,7 +48,9 @@ class Account::Holding::Syncer
                              end
 
                              ticker_start_dates.each do |ticker, date|
-                               prices[ticker] = Security::Price.find_prices(ticker: ticker, start_date: date, end_date: Date.current)
+                               fetched_prices = Security::Price.find_prices(ticker: ticker, start_date: date, end_date: Date.current)
+                               gapfilled_prices = Gapfiller.new(fetched_prices, start_date: date, end_date: Date.current, cache: false).run
+                               prices[ticker] = gapfilled_prices
                              end
 
                              prices

app/models/gapfiller.rb

@@ -0,0 +1,44 @@
+class Gapfiller
+  attr_reader :series
+
+  def initialize(series, start_date:, end_date:, cache:)
+    @series = series
+    @date_range = start_date..end_date
+    @cache = cache
+  end
+
+  def run
+    gapfilled_records = []
+
+    date_range.each do |date|
+      record = series.find { |r| r.date == date }
+
+      if should_gapfill?(date, record)
+        prev_record = gapfilled_records.find { |r| r.date == date - 1.day }
+
+        if prev_record
+          new_record = create_gapfilled_record(prev_record, date)
+          gapfilled_records << new_record
+        end
+      else
+        gapfilled_records << record if record
+      end
+    end
+
+    gapfilled_records
+  end
+
+  private
+    attr_reader :date_range, :cache
+
+    def should_gapfill?(date, record)
+      date.on_weekend? && record.nil?
+    end
+
+    def create_gapfilled_record(prev_record, date)
+      new_record = prev_record.class.new(prev_record.attributes.except("id", "created_at", "updated_at"))
+      new_record.date = date
+      new_record.save! if cache
+      new_record
+    end
+end

app/models/issue/prices_missing.rb

@@ -3,11 +3,11 @@ class Issue::PricesMissing < Issue
 
   after_initialize :initialize_missing_prices
 
-  validates :missing_prices, presence: true
+  validates :missing_prices, presence: true, allow_blank: true
 
   def append_missing_price(ticker, date)
     missing_prices[ticker] ||= []
-    missing_prices[ticker] << date
+    missing_prices[ticker] << date unless missing_prices[ticker].include?(date.to_s)
   end
 
   def stale?

app/models/security/price.rb

@@ -27,7 +27,6 @@ class Security::Price < ApplicationRecord
   end
 
   private
-
     def upcase_ticker
       self.ticker = ticker.upcase
     end

test/models/account/holding/syncer_test.rb

@@ -77,12 +77,13 @@ class Account::Holding::SyncerTest < ActiveSupport::TestCase
       { ticker: "AMZN", qty: 10, price: nil, amount: nil, date: Date.current }
     ]
 
+    fetched_prices = [ Security::Price.new(ticker: "AMZN", date: 1.day.ago.to_date, price: 215) ]
+
+    Gapfiller.any_instance.expects(:run).returns(fetched_prices)
     Security::Price.expects(:find_prices)
                    .with(start_date: 2.days.ago.to_date, end_date: Date.current, ticker: "AMZN")
                    .once
-                   .returns([
-                              Security::Price.new(ticker: "AMZN", date: 1.day.ago.to_date, price: 215)
-                            ])
+                   .returns(fetched_prices)
 
     @account.expects(:observe_missing_price).with(ticker: "AMZN", date: Date.current).once
 
@@ -91,6 +92,33 @@ class Account::Holding::SyncerTest < ActiveSupport::TestCase
     assert_holdings(expected)
   end
 
+  # It is common for data providers to not provide prices for weekends, so we need to carry the last observation forward
+  test "uses locf gapfilling when price is missing" do
+    friday = Date.new(2024, 9, 27) # A known Friday
+    saturday = friday + 1.day # weekend
+    sunday = saturday + 1.day # weekend
+    monday = sunday + 1.day # known Monday
+
+    # Prices should be gapfilled like this: 210, 210, 210, 220
+    tm = create_security("TM", prices: [
+      { date: friday, price: 210 },
+      { date: monday, price: 220 }
+    ])
+
+    create_trade(tm, account: @account, qty: 10, date: friday)
+
+    expected = [
+      { ticker: "TM", qty: 10, price: 210, amount: 10 * 210, date: friday },
+      { ticker: "TM", qty: 10, price: 210, amount: 10 * 210, date: saturday },
+      { ticker: "TM", qty: 10, price: 210, amount: 10 * 210, date: sunday },
+      { ticker: "TM", qty: 10, price: 220, amount: 10 * 220, date: monday }
+    ]
+
+    run_sync_for(@account)
+
+    assert_holdings(expected)
+  end
+
   private
 
     def assert_holdings(expected_holdings)