Merge branch 'main' of github.com:maybe-finance/maybe into zachgoll/plaid-sync-domain-improvements

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}"
 

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

app/jobs/fetch_security_info_job.rb

@@ -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

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

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
 

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

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
 

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

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
 

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
 

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

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|

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

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

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