mirror of
https://github.com/maybe-finance/maybe.git
synced 2025-07-19 05:09:38 +02:00
Remove missing prices issue (#1390)
This commit is contained in:
parent
7d8028b505
commit
bf695972e4
8 changed files with 11 additions and 114 deletions
|
@ -68,8 +68,6 @@ class Account::Holding::Syncer
|
|||
|
||||
price = get_cached_price(ticker, date) || trade_price
|
||||
|
||||
account.observe_missing_price(ticker:, date:) unless price
|
||||
|
||||
account.holdings.build \
|
||||
date: date,
|
||||
security_id: holding[:security_id],
|
||||
|
|
|
@ -33,12 +33,6 @@ module Issuable
|
|||
)
|
||||
end
|
||||
|
||||
def observe_missing_price(ticker:, date:)
|
||||
issue = issues.find_or_create_by(type: Issue::PricesMissing.name, resolved_at: nil)
|
||||
issue.append_missing_price(ticker, date)
|
||||
issue.save!
|
||||
end
|
||||
|
||||
def highest_priority_issue
|
||||
issues.active.ordered.first
|
||||
end
|
||||
|
|
|
@ -1,33 +0,0 @@
|
|||
class Issue::PricesMissing < Issue
|
||||
store_accessor :data, :missing_prices
|
||||
|
||||
after_initialize :initialize_missing_prices
|
||||
|
||||
validates :missing_prices, presence: true, allow_blank: true
|
||||
|
||||
def append_missing_price(ticker, date)
|
||||
missing_prices[ticker] ||= []
|
||||
missing_prices[ticker] << date unless missing_prices[ticker].include?(date.to_s)
|
||||
end
|
||||
|
||||
def stale?
|
||||
stale = true
|
||||
|
||||
missing_prices.each do |ticker, dates|
|
||||
next unless issuable.owns_ticker?(ticker)
|
||||
|
||||
oldest_date = dates.min.to_date
|
||||
expected_price_count = (oldest_date..Date.current).count
|
||||
prices = Security::Price.find_prices(ticker: ticker, start_date: oldest_date)
|
||||
stale = false if prices.count < expected_price_count
|
||||
end
|
||||
|
||||
stale
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def initialize_missing_prices
|
||||
self.missing_prices ||= {}
|
||||
end
|
||||
end
|
9
db/migrate/20241029184115_remove_prices_missing_issue.rb
Normal file
9
db/migrate/20241029184115_remove_prices_missing_issue.rb
Normal file
|
@ -0,0 +1,9 @@
|
|||
class RemovePricesMissingIssue < ActiveRecord::Migration[7.2]
|
||||
def up
|
||||
execute "DELETE FROM issues WHERE type = 'Issue::PricesMissing'"
|
||||
end
|
||||
|
||||
def down
|
||||
# Cannot restore deleted issues since we don't have the original data
|
||||
end
|
||||
end
|
4
db/schema.rb
generated
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: 2024_10_25_182612) do
|
||||
ActiveRecord::Schema[7.2].define(version: 2024_10_29_184115) do
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "pgcrypto"
|
||||
enable_extension "plpgsql"
|
||||
|
@ -119,7 +119,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_10_25_182612) do
|
|||
t.boolean "is_active", default: true, null: false
|
||||
t.date "last_sync_date"
|
||||
t.uuid "institution_id"
|
||||
t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true
|
||||
t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true
|
||||
t.uuid "import_id"
|
||||
t.string "mode"
|
||||
t.index ["accountable_id", "accountable_type"], name: "index_accounts_on_accountable_id_and_accountable_type"
|
||||
|
|
|
@ -85,8 +85,6 @@ class Account::Holding::SyncerTest < ActiveSupport::TestCase
|
|||
.once
|
||||
.returns(fetched_prices)
|
||||
|
||||
@account.expects(:observe_missing_price).with(ticker: "AMZN", date: Date.current).once
|
||||
|
||||
run_sync_for(@account)
|
||||
|
||||
assert_holdings(expected)
|
||||
|
|
|
@ -89,12 +89,4 @@ class AccountTest < ActiveSupport::TestCase
|
|||
assert_equal 10, account.holding_qty(security, date: 1.day.ago.to_date)
|
||||
assert_equal 0, account.holding_qty(security, date: 2.days.ago.to_date)
|
||||
end
|
||||
|
||||
test "can observe missing price" do
|
||||
account = accounts(:investment)
|
||||
|
||||
assert_difference -> { account.issues.count } do
|
||||
account.observe_missing_price(ticker: "AAPL", date: Date.current)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,61 +0,0 @@
|
|||
require "test_helper"
|
||||
|
||||
class Issue::PricesMissingTest < ActiveSupport::TestCase
|
||||
setup do
|
||||
@issue = Issue::PricesMissing.new
|
||||
@account = Account.new(id: "123abc")
|
||||
@issue.issuable = @account
|
||||
end
|
||||
|
||||
test "stale? returns false when no prices are found" do
|
||||
@issue.append_missing_price("AAPL", (Date.current - 5.days).to_s)
|
||||
|
||||
Security::Price.expects(:find_prices).returns([])
|
||||
@account.expects(:owns_ticker?).with("AAPL").returns(true)
|
||||
|
||||
assert_not @issue.stale?
|
||||
end
|
||||
|
||||
test "stale? returns true when all expected prices are found" do
|
||||
start_date = Date.current - 5.days
|
||||
@issue.append_missing_price("AAPL", start_date.to_s)
|
||||
|
||||
expected_prices = (start_date..Date.current).map { |date| { date: date } }
|
||||
Security::Price.expects(:find_prices).returns(expected_prices)
|
||||
@account.expects(:owns_ticker?).with("AAPL").returns(true)
|
||||
|
||||
assert @issue.stale?
|
||||
end
|
||||
|
||||
test "stale? returns false when some prices are missing" do
|
||||
start_date = Date.current - 5.days
|
||||
@issue.append_missing_price("AAPL", start_date.to_s)
|
||||
|
||||
incomplete_prices = (start_date..Date.current - 2.days).map { |date| { date: date } }
|
||||
Security::Price.expects(:find_prices).returns(incomplete_prices)
|
||||
@account.expects(:owns_ticker?).with("AAPL").returns(true)
|
||||
|
||||
assert_not @issue.stale?
|
||||
end
|
||||
|
||||
test "stale? returns true when the account doesn't own the ticker" do
|
||||
@issue.append_missing_price("AAPL", (Date.current - 5.days).to_s)
|
||||
|
||||
@account.expects(:owns_ticker?).with("AAPL").returns(false)
|
||||
|
||||
assert @issue.stale?
|
||||
end
|
||||
|
||||
test "stale? handles multiple tickers correctly" do
|
||||
@issue.append_missing_price("AAPL", (Date.current - 5.days).to_s)
|
||||
@issue.append_missing_price("GOOGL", (Date.current - 3.days).to_s)
|
||||
|
||||
@account.expects(:owns_ticker?).with("AAPL").returns(true)
|
||||
@account.expects(:owns_ticker?).with("GOOGL").returns(true)
|
||||
|
||||
Security::Price.expects(:find_prices).with(ticker: "AAPL", start_date: Date.current - 5.days).returns([])
|
||||
Security::Price.expects(:find_prices).with(ticker: "GOOGL", start_date: Date.current - 3.days).returns([])
|
||||
|
||||
assert_not @issue.stale?
|
||||
end
|
||||
end
|
Loading…
Add table
Add a link
Reference in a new issue