diff --git a/app/controllers/concerns/auto_sync.rb b/app/controllers/concerns/auto_sync.rb index e6ced672..15cdc557 100644 --- a/app/controllers/concerns/auto_sync.rb +++ b/app/controllers/concerns/auto_sync.rb @@ -13,6 +13,7 @@ module AutoSync def family_needs_auto_sync? return false unless Current.family&.accounts&.active&.any? return false if (Current.family.last_sync_created_at&.to_date || 1.day.ago) >= Date.current + return false unless Current.family.auto_sync_on_login Rails.logger.info "Auto-syncing family #{Current.family.id}, last sync was #{Current.family.last_sync_created_at}" diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index c1b51c23..20e5f9c4 100644 --- a/app/controllers/imports_controller.rb +++ b/app/controllers/imports_controller.rb @@ -5,6 +5,8 @@ class ImportsController < ApplicationController @import.publish_later redirect_to import_path(@import), notice: "Your import has started in the background." + rescue Import::MaxRowCountExceededError + redirect_back_or_to import_path(@import), alert: "Your import exceeds the maximum row count of #{@import.max_row_count}." end def index diff --git a/app/jobs/fetch_security_info_job.rb b/app/jobs/fetch_security_info_job.rb deleted file mode 100644 index e789222f..00000000 --- a/app/jobs/fetch_security_info_job.rb +++ /dev/null @@ -1,21 +0,0 @@ -class FetchSecurityInfoJob < ApplicationJob - queue_as :low_priority - - def perform(security_id) - return unless Security.provider.present? - - security = Security.find(security_id) - - params = { - ticker: security.ticker - } - params[:mic_code] = security.exchange_mic if security.exchange_mic.present? - params[:operating_mic] = security.exchange_operating_mic if security.exchange_operating_mic.present? - - security_info_response = Security.provider.fetch_security_info(**params) - - security.update( - name: security_info_response.info.dig("name") - ) - end -end diff --git a/app/models/account_import.rb b/app/models/account_import.rb index 96fdfd47..aa4c6dfe 100644 --- a/app/models/account_import.rb +++ b/app/models/account_import.rb @@ -54,4 +54,8 @@ class AccountImport < Import CSV.parse(template, headers: true) end + + def max_row_count + 50 + end end diff --git a/app/models/concerns/syncable.rb b/app/models/concerns/syncable.rb index 9b5e09e4..6b0ba684 100644 --- a/app/models/concerns/syncable.rb +++ b/app/models/concerns/syncable.rb @@ -11,17 +11,18 @@ module Syncable def sync_later(parent_sync: nil, window_start_date: nil, window_end_date: nil) Sync.transaction do - # Expire any previous in-flight syncs for this record that exceeded the - # global staleness window. - syncs.stale_candidates.find_each(&:mark_stale!) + # Since we're scheduling a new sync, mark old syncs for this syncable as stale + self.syncs.incomplete.find_each(&:mark_stale!) - new_sync = syncs.create!( + new_sync = self.syncs.create!( parent: parent_sync, window_start_date: window_start_date, window_end_date: window_end_date ) SyncJob.perform_later(new_sync) + + new_sync end end diff --git a/app/models/import.rb b/app/models/import.rb index e96d1fc1..b0a02ea0 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -1,4 +1,6 @@ class Import < ApplicationRecord + MaxRowCountExceededError = Class.new(StandardError) + TYPES = %w[TransactionImport TradeImport AccountImport MintImport].freeze SIGNAGE_CONVENTIONS = %w[inflows_positive inflows_negative] SEPARATORS = [ [ "Comma (,)", "," ], [ "Semicolon (;)", ";" ] ].freeze @@ -52,6 +54,7 @@ class Import < ApplicationRecord end def publish_later + raise MaxRowCountExceededError if row_count_exceeded? raise "Import is not publishable" unless publishable? update! status: :importing @@ -60,6 +63,8 @@ class Import < ApplicationRecord end def publish + raise MaxRowCountExceededError if row_count_exceeded? + import! family.sync_later @@ -220,7 +225,15 @@ class Import < ApplicationRecord ) end + def max_row_count + 10000 + end + private + def row_count_exceeded? + rows.count > max_row_count + end + def import! # no-op, subclasses can implement for customization of algorithm end diff --git a/app/models/provider/synth.rb b/app/models/provider/synth.rb index fee3a236..8d76fc72 100644 --- a/app/models/provider/synth.rb +++ b/app/models/provider/synth.rb @@ -71,9 +71,11 @@ class Provider::Synth < Provider rate = rate.dig("rates", to) if date.nil? || rate.nil? - message = "#{self.class.name} returned invalid rate data for pair from: #{from} to: #{to} on: #{date}. Rate data: #{rate.inspect}" - Rails.logger.warn(message) - Sentry.capture_exception(InvalidExchangeRateError.new(message), level: :warning) + Rails.logger.warn("#{self.class.name} returned invalid rate data for pair from: #{from} to: #{to} on: #{date}. Rate data: #{rate.inspect}") + Sentry.capture_exception(InvalidExchangeRateError.new("#{self.class.name} returned invalid rate data"), level: :warning) do |scope| + scope.set_context("rate", { from: from, to: to, date: date }) + end + next end @@ -162,9 +164,11 @@ class Provider::Synth < Provider price = price.dig("close") || price.dig("open") if date.nil? || price.nil? - message = "#{self.class.name} returned invalid price data for security #{symbol} on: #{date}. Price data: #{price.inspect}" - Rails.logger.warn(message) - Sentry.capture_exception(InvalidSecurityPriceError.new(message), level: :warning) + Rails.logger.warn("#{self.class.name} returned invalid price data for security #{symbol} on: #{date}. Price data: #{price.inspect}") + Sentry.capture_exception(InvalidSecurityPriceError.new("#{self.class.name} returned invalid security price data"), level: :warning) do |scope| + scope.set_context("security", { symbol: symbol, date: date }) + end + next end diff --git a/app/models/security/price/importer.rb b/app/models/security/price/importer.rb index 4e6bee2f..6143f22a 100644 --- a/app/models/security/price/importer.rb +++ b/app/models/security/price/importer.rb @@ -26,9 +26,16 @@ class Security::Price::Importer prev_price_value = start_price_value unless prev_price_value.present? - error = MissingStartPriceError.new("Could not find a start price for #{security.ticker} on or before #{start_date}") - Rails.logger.error(error.message) - Sentry.capture_exception(error) + Rails.logger.error("Could not find a start price for #{security.ticker} on or before #{start_date}") + + Sentry.capture_exception(MissingStartPriceError.new("Could not determine start price for ticker")) do |scope| + scope.set_tags(security_id: security.id) + scope.set_context("security", { + id: security.id, + start_date: start_date + }) + end + return 0 end @@ -75,9 +82,12 @@ class Security::Price::Importer if response.success? response.data.index_by(&:date) else - msg = "#{security_provider.class.name} could not fetch prices for #{security.ticker} between #{provider_fetch_start_date} and #{end_date}. Provider error: #{response.error.message}" - Rails.logger.warn(msg) - Sentry.capture_exception(MissingSecurityPriceError.new(msg), level: :warning) + Rails.logger.warn("#{security_provider.class.name} could not fetch prices for #{security.ticker} between #{provider_fetch_start_date} and #{end_date}. Provider error: #{response.error.message}") + Sentry.capture_exception(MissingSecurityPriceError.new("Could not fetch prices for ticker"), level: :warning) do |scope| + scope.set_tags(security_id: security.id) + scope.set_context("security", { id: security.id, start_date: start_date, end_date: end_date }) + end + {} end end diff --git a/app/models/security/provided.rb b/app/models/security/provided.rb index 2fdcc607..7927d6e6 100644 --- a/app/models/security/provided.rb +++ b/app/models/security/provided.rb @@ -1,6 +1,8 @@ module Security::Provided extend ActiveSupport::Concern + SecurityInfoMissingError = Class.new(StandardError) + class_methods do def provider registry = Provider::Registry.for_concept(:securities) @@ -70,9 +72,11 @@ module Security::Provided logo_url: response.data.logo_url, ) else - err = StandardError.new("Failed to fetch security info for #{ticker} from #{provider.class.name}: #{response.error.message}") - Rails.logger.warn(err.message) - Sentry.capture_exception(err, level: :warning) + Rails.logger.warn("Failed to fetch security info for #{ticker} from #{provider.class.name}: #{response.error.message}") + Sentry.capture_exception(SecurityInfoMissingError.new("Failed to get security info"), level: :warning) do |scope| + scope.set_tags(security_id: self.id) + scope.set_context("security", { id: self.id, provider_error: response.error.message }) + end end end diff --git a/app/models/sync.rb b/app/models/sync.rb index 456a4b01..aa40b313 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -1,7 +1,7 @@ class Sync < ApplicationRecord - # We run a cron that marks any syncs that have been running > 2 hours as "stale" + # We run a cron that marks any syncs that have not been resolved in 24 hours as "stale" # Syncs often become stale when new code is deployed and the worker restarts - STALE_AFTER = 2.hours + STALE_AFTER = 24.hours # The max time that a sync will show in the UI (after 5 minutes) VISIBLE_FOR = 5.minutes @@ -19,9 +19,6 @@ class Sync < ApplicationRecord scope :incomplete, -> { where("syncs.status IN (?)", %w[pending syncing]) } scope :visible, -> { incomplete.where("syncs.created_at > ?", VISIBLE_FOR.ago) } - # In-flight records that have exceeded the allowed runtime - scope :stale_candidates, -> { incomplete.where("syncs.created_at < ?", STALE_AFTER.ago) } - validate :window_valid # Sync state machine @@ -54,7 +51,7 @@ class Sync < ApplicationRecord class << self def clean - stale_candidates.find_each(&:mark_stale!) + incomplete.where("syncs.created_at < ?", STALE_AFTER.ago).find_each(&:mark_stale!) end end diff --git a/db/migrate/20250518181619_add_auto_sync_preference_to_family.rb b/db/migrate/20250518181619_add_auto_sync_preference_to_family.rb new file mode 100644 index 00000000..80e1cd9f --- /dev/null +++ b/db/migrate/20250518181619_add_auto_sync_preference_to_family.rb @@ -0,0 +1,5 @@ +class AddAutoSyncPreferenceToFamily < ActiveRecord::Migration[7.2] + def change + add_column :families, :auto_sync_on_login, :boolean, default: true, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 9a7aa93f..7c4a65ce 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -227,6 +227,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_18_133020) do t.string "timezone" t.boolean "data_enrichment_enabled", default: false t.boolean "early_access", default: false + t.boolean "auto_sync_on_login", default: true, null: false end create_table "holdings", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| diff --git a/test/controllers/concerns/auto_sync_test.rb b/test/controllers/concerns/auto_sync_test.rb index a388456b..462850f8 100644 --- a/test/controllers/concerns/auto_sync_test.rb +++ b/test/controllers/concerns/auto_sync_test.rb @@ -38,4 +38,12 @@ class AutoSyncTest < ActionDispatch::IntegrationTest get root_path end end + + test "does not auto-sync if preference is disabled" do + @family.update!(auto_sync_on_login: false) + + assert_no_difference "Sync.count" do + get root_path + end + end end diff --git a/test/interfaces/syncable_interface_test.rb b/test/interfaces/syncable_interface_test.rb index a9f1fa7d..95f6789d 100644 --- a/test/interfaces/syncable_interface_test.rb +++ b/test/interfaces/syncable_interface_test.rb @@ -17,4 +17,12 @@ module SyncableInterfaceTest @syncable.class.any_instance.expects(:perform_sync).with(mock_sync).once @syncable.perform_sync(mock_sync) end + + test "any prior syncs for the same syncable entity are marked stale when new sync is requested" do + stale_sync = @syncable.sync_later + new_sync = @syncable.sync_later + + assert_equal "stale", stale_sync.reload.status + assert_equal "pending", new_sync.reload.status + end end diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index 12e410c6..09266bb5 100644 --- a/test/models/sync_test.rb +++ b/test/models/sync_test.rb @@ -172,15 +172,15 @@ class SyncTest < ActiveSupport::TestCase stale_pending = Sync.create!( syncable: accounts(:depository), status: :pending, - created_at: 3.hours.ago + created_at: 25.hours.ago ) stale_syncing = Sync.create!( syncable: accounts(:depository), status: :syncing, - created_at: 3.hours.ago, - pending_at: 3.hours.ago, - syncing_at: 2.hours.ago + created_at: 25.hours.ago, + pending_at: 24.hours.ago, + syncing_at: 23.hours.ago ) Sync.clean