From 10dd9e061a5dedb0185686f08acfe6248c9a4879 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 15 May 2025 10:19:56 -0400 Subject: [PATCH] Improve account sync performance, handle concurrent market data syncing (#2236) * PlaidConnectable concern * Remove bad abstraction * Put sync implementations in own concerns * Sync strategies * Move sync orchestration to Sync class * Clean up sync class, add state machine * Basic market data sync cron * Fix price sync * Improve sync window column names, add timestamps * 30 day syncs by default * Clean up market data methods * Report high duplicate sync counts to Sentry * Add sync states throughout app * account tab session * Persistent account tab selections * Remove manual sleep * Add migration to clear stale syncs on self hosted apps * Tweak sync states * Sync completion event broadcasts * Fix timezones in tests * Cleanup * More cleanup * Plaid item UI broadcasts for sync * Fix account ID namespace conflict * Sync broadcasters * Smoother account sync refreshes * Remove test sync delay --- Gemfile | 5 + Gemfile.lock | 23 ++ app/assets/tailwind/maybe-design-system.css | 4 +- .../maybe-design-system/background-utils.css | 4 + app/components/tabs_component.html.erb | 1 + app/components/tabs_component.rb | 5 +- app/components/tabs_controller.js | 23 +- app/controllers/accounts_controller.rb | 8 - app/controllers/application_controller.rb | 5 +- app/controllers/concerns/notifiable.rb | 2 - .../concerns/restore_layout_preferences.rb | 24 +++ app/controllers/cookie_sessions_controller.rb | 22 ++ .../current_sessions_controller.rb | 14 ++ app/controllers/plaid_items_controller.rb | 4 +- .../controllers/sidebar_tabs_controller.js | 16 -- app/jobs/sync_market_data_job.rb | 7 + app/models/account.rb | 33 ++- app/models/account/sync_complete_event.rb | 54 +++++ app/models/account/syncer.rb | 22 ++ app/models/balance/base_calculator.rb | 35 ---- app/models/balance/forward_calculator.rb | 35 +++- app/models/balance/reverse_calculator.rb | 35 +++- app/models/balance_sheet.rb | 38 +++- app/models/concerns/accountable.rb | 9 - app/models/concerns/syncable.rb | 30 ++- app/models/entry.rb | 2 +- app/models/family.rb | 66 +----- app/models/family/plaid_connectable.rb | 51 +++++ app/models/family/subscribeable.rb | 9 +- app/models/family/sync_complete_event.rb | 21 ++ app/models/family/syncer.rb | 31 +++ app/models/holding/base_calculator.rb | 62 ------ app/models/holding/forward_calculator.rb | 65 +++++- app/models/holding/portfolio_cache.rb | 3 - app/models/holding/reverse_calculator.rb | 54 ++++- app/models/import.rb | 2 +- app/models/market_data_syncer.rb | 196 ++++++++++++++++++ app/models/plaid_item.rb | 160 +------------- app/models/plaid_item/provided.rb | 30 --- app/models/plaid_item/sync_complete_event.rb | 22 ++ app/models/plaid_item/syncer.rb | 149 +++++++++++++ app/models/provider.rb | 2 - app/models/security/provided.rb | 38 ---- app/models/session.rb | 10 + app/models/subscription.rb | 1 + app/models/sync.rb | 127 +++++++++--- app/views/accounts/_account.html.erb | 10 +- .../accounts/_account_sidebar_tabs.html.erb | 124 ++++++----- .../accounts/_accountable_group.html.erb | 91 ++++---- app/views/accounts/_chart_loader.html.erb | 8 +- app/views/accounts/chart.html.erb | 28 +-- app/views/accounts/index.html.erb | 11 - app/views/accounts/index/_account_groups.erb | 7 +- app/views/accounts/show/_chart.html.erb | 17 +- app/views/accounts/show/_template.html.erb | 6 +- app/views/category/dropdowns/show.html.erb | 14 +- app/views/holdings/_cash.html.erb | 17 +- app/views/holdings/_holding.html.erb | 42 +++- app/views/investments/show.html.erb | 32 +-- app/views/layouts/application.html.erb | 7 +- app/views/layouts/shared/_htmldoc.html.erb | 4 - app/views/pages/dashboard.html.erb | 9 +- .../pages/dashboard/_balance_sheet.html.erb | 88 +++++--- .../pages/dashboard/_net_worth_chart.html.erb | 68 +++--- app/views/rules/_rule.html.erb | 4 +- app/views/rules/confirm.html.erb | 4 +- app/views/rules/edit.html.erb | 2 +- app/views/rules/index.html.erb | 2 +- app/views/settings/_settings_nav.html.erb | 32 +-- .../shared/notifications/_loading.html.erb | 9 - app/views/transactions/index.html.erb | 3 - config/initializers/mini_profiler.rb | 3 +- config/initializers/sidekiq.rb | 5 + config/routes.rb | 7 +- config/schedule.yml | 5 + config/sidekiq.yml | 3 +- .../20250512171654_update_sync_timestamps.rb | 65 ++++++ .../20250514214242_add_metadata_to_session.rb | 5 + db/schema.rb | 12 +- test/controllers/accounts_controller_test.rb | 5 - .../current_sessions_controller_test.rb | 15 ++ .../plaid_items_controller_test.rb | 2 +- test/fixtures/syncs.yml | 6 +- test/interfaces/syncable_interface_test.rb | 14 +- test/jobs/sync_job_test.rb | 2 +- test/models/account/entry_test.rb | 6 +- .../models/balance/forward_calculator_test.rb | 20 +- .../models/balance/reverse_calculator_test.rb | 18 +- test/models/family/syncer_test.rb | 30 +++ test/models/family_test.rb | 16 -- .../models/holding/forward_calculator_test.rb | 26 +-- test/models/holding/portfolio_cache_test.rb | 32 +-- .../models/holding/reverse_calculator_test.rb | 30 +-- test/models/market_data_syncer_test.rb | 71 +++++++ test/models/security/price_test.rb | 19 -- test/models/sync_test.rb | 170 +++++++++++++-- test/test_helper.rb | 1 + 97 files changed, 1837 insertions(+), 949 deletions(-) create mode 100644 app/controllers/concerns/restore_layout_preferences.rb create mode 100644 app/controllers/cookie_sessions_controller.rb create mode 100644 app/controllers/current_sessions_controller.rb delete mode 100644 app/javascript/controllers/sidebar_tabs_controller.js create mode 100644 app/jobs/sync_market_data_job.rb create mode 100644 app/models/account/sync_complete_event.rb create mode 100644 app/models/account/syncer.rb delete mode 100644 app/models/balance/base_calculator.rb create mode 100644 app/models/family/plaid_connectable.rb create mode 100644 app/models/family/sync_complete_event.rb create mode 100644 app/models/family/syncer.rb delete mode 100644 app/models/holding/base_calculator.rb create mode 100644 app/models/market_data_syncer.rb delete mode 100644 app/models/plaid_item/provided.rb create mode 100644 app/models/plaid_item/sync_complete_event.rb create mode 100644 app/models/plaid_item/syncer.rb delete mode 100644 app/views/shared/notifications/_loading.html.erb create mode 100644 config/schedule.yml create mode 100644 db/migrate/20250512171654_update_sync_timestamps.rb create mode 100644 db/migrate/20250514214242_add_metadata_to_session.rb create mode 100644 test/controllers/current_sessions_controller_test.rb create mode 100644 test/models/family/syncer_test.rb create mode 100644 test/models/market_data_syncer_test.rb diff --git a/Gemfile b/Gemfile index 26b29fc9..3ad4f0a8 100644 --- a/Gemfile +++ b/Gemfile @@ -29,6 +29,7 @@ gem "hotwire_combobox" # Background Jobs gem "sidekiq" +gem "sidekiq-cron" # Monitoring gem "vernier" @@ -63,6 +64,10 @@ gem "rotp", "~> 6.3" gem "rqrcode", "~> 3.0" gem "activerecord-import" +# State machines +gem "aasm" +gem "after_commit_everywhere", "~> 1.0" + # AI gem "ruby-openai" diff --git a/Gemfile.lock b/Gemfile.lock index 224f8f9c..a8c86724 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -8,6 +8,8 @@ GIT GEM remote: https://rubygems.org/ specs: + aasm (5.5.0) + concurrent-ruby (~> 1.0) actioncable (7.2.2.1) actionpack (= 7.2.2.1) activesupport (= 7.2.2.1) @@ -83,6 +85,9 @@ GEM tzinfo (~> 2.0, >= 2.0.5) addressable (2.8.7) public_suffix (>= 2.0.2, < 7.0) + after_commit_everywhere (1.6.0) + activerecord (>= 4.2) + activesupport ast (2.4.3) aws-eventstream (1.3.2) aws-partitions (1.1093.0) @@ -139,6 +144,9 @@ GEM bigdecimal rexml crass (1.0.6) + cronex (0.15.0) + tzinfo + unicode (>= 0.4.4.5) css_parser (1.21.1) addressable csv (3.3.4) @@ -160,6 +168,8 @@ GEM rubocop (>= 1) smart_properties erubi (1.13.1) + et-orbi (1.2.11) + tzinfo event_stream_parser (1.0.0) faker (3.5.1) i18n (>= 1.8.11, < 2) @@ -182,6 +192,9 @@ GEM ffi (1.17.2-x86_64-linux-gnu) ffi (1.17.2-x86_64-linux-musl) foreman (0.88.1) + fugit (1.11.1) + et-orbi (~> 1, >= 1.2.11) + raabro (~> 1.4) globalid (1.2.1) activesupport (>= 6.1) hashdiff (1.1.2) @@ -346,6 +359,7 @@ GEM public_suffix (6.0.1) puma (6.6.0) nio4r (~> 2.0) + raabro (1.4.0) racc (1.8.1) rack (3.1.13) rack-mini-profiler (3.3.1) @@ -486,6 +500,11 @@ GEM logger (>= 1.6.2) rack (>= 3.1.0) redis-client (>= 0.23.2) + sidekiq-cron (2.2.0) + cronex (>= 0.13.0) + fugit (~> 1.8, >= 1.11.1) + globalid (>= 1.0.1) + sidekiq (>= 6.5.0) simplecov (0.22.0) docile (~> 1.1) simplecov-html (~> 0.11) @@ -519,6 +538,7 @@ GEM railties (>= 7.1.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) + unicode (0.4.4.5) unicode-display_width (3.1.4) unicode-emoji (~> 4.0, >= 4.0.4) unicode-emoji (4.0.4) @@ -561,7 +581,9 @@ PLATFORMS x86_64-linux-musl DEPENDENCIES + aasm activerecord-import + after_commit_everywhere (~> 1.0) aws-sdk-s3 (~> 1.177.0) bcrypt (~> 3.1) benchmark-ips @@ -612,6 +634,7 @@ DEPENDENCIES sentry-ruby sentry-sidekiq sidekiq + sidekiq-cron simplecov skylight stimulus-rails diff --git a/app/assets/tailwind/maybe-design-system.css b/app/assets/tailwind/maybe-design-system.css index 8bf9c6c8..f9dc6039 100644 --- a/app/assets/tailwind/maybe-design-system.css +++ b/app/assets/tailwind/maybe-design-system.css @@ -240,7 +240,7 @@ 100% { stroke-dashoffset: 0; } - } + } } /* Specific override for strong tags in prose under dark mode */ @@ -429,5 +429,3 @@ } } - - diff --git a/app/assets/tailwind/maybe-design-system/background-utils.css b/app/assets/tailwind/maybe-design-system/background-utils.css index fd5f673f..f7244692 100644 --- a/app/assets/tailwind/maybe-design-system/background-utils.css +++ b/app/assets/tailwind/maybe-design-system/background-utils.css @@ -93,3 +93,7 @@ background-color: var(--color-alpha-black-900); } } + +@utility bg-loader { + @apply bg-surface-inset animate-pulse; +} diff --git a/app/components/tabs_component.html.erb b/app/components/tabs_component.html.erb index 4ec901fa..bfceddad 100644 --- a/app/components/tabs_component.html.erb +++ b/app/components/tabs_component.html.erb @@ -1,6 +1,7 @@ <%= tag.div data: { controller: "tabs", testid: testid, + tabs_session_key_value: session_key, tabs_url_param_key_value: url_param_key, tabs_nav_btn_active_class: active_btn_classes, tabs_nav_btn_inactive_class: inactive_btn_classes diff --git a/app/components/tabs_component.rb b/app/components/tabs_component.rb index 4017b308..747a9420 100644 --- a/app/components/tabs_component.rb +++ b/app/components/tabs_component.rb @@ -27,11 +27,12 @@ class TabsComponent < ViewComponent::Base } } - attr_reader :active_tab, :url_param_key, :variant, :testid + attr_reader :active_tab, :url_param_key, :session_key, :variant, :testid - def initialize(active_tab:, url_param_key: nil, variant: :default, active_btn_classes: "", inactive_btn_classes: "", testid: nil) + def initialize(active_tab:, url_param_key: nil, session_key: nil, variant: :default, active_btn_classes: "", inactive_btn_classes: "", testid: nil) @active_tab = active_tab @url_param_key = url_param_key + @session_key = session_key @variant = variant.to_sym @active_btn_classes = active_btn_classes @inactive_btn_classes = inactive_btn_classes diff --git a/app/components/tabs_controller.js b/app/components/tabs_controller.js index 43a4b192..259765aa 100644 --- a/app/components/tabs_controller.js +++ b/app/components/tabs_controller.js @@ -4,7 +4,7 @@ import { Controller } from "@hotwired/stimulus"; export default class extends Controller { static classes = ["navBtnActive", "navBtnInactive"]; static targets = ["panel", "navBtn"]; - static values = { urlParamKey: String }; + static values = { sessionKey: String, urlParamKey: String }; show(e) { const btn = e.target.closest("button"); @@ -28,11 +28,30 @@ export default class extends Controller { } }); - // Update URL with the selected tab if (this.urlParamKeyValue) { const url = new URL(window.location.href); url.searchParams.set(this.urlParamKeyValue, selectedTabId); window.history.replaceState({}, "", url); } + + // Update URL with the selected tab + if (this.sessionKeyValue) { + this.#updateSessionPreference(selectedTabId); + } } + + #updateSessionPreference(selectedTabId) { + fetch("/current_session", { + method: "PUT", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + "X-CSRF-Token": document.querySelector('[name="csrf-token"]').content, + Accept: "application/json", + }, + body: new URLSearchParams({ + "current_session[tab_key]": this.sessionKeyValue, + "current_session[tab_value]": selectedTabId, + }).toString(), + }); + } } diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index f003ab31..904be2b5 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -26,14 +26,6 @@ class AccountsController < ApplicationController render layout: false end - def sync_all - unless family.syncing? - family.sync_later - end - - redirect_back_or_to accounts_path - end - private def family Current.family diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a54dc088..260579d1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,5 +1,8 @@ class ApplicationController < ActionController::Base - include Onboardable, Localize, AutoSync, Authentication, Invitable, SelfHostable, StoreLocation, Impersonatable, Breadcrumbable, FeatureGuardable, Notifiable + include RestoreLayoutPreferences, Onboardable, Localize, AutoSync, Authentication, Invitable, + SelfHostable, StoreLocation, Impersonatable, Breadcrumbable, + FeatureGuardable, Notifiable + include Pagy::Backend before_action :detect_os diff --git a/app/controllers/concerns/notifiable.rb b/app/controllers/concerns/notifiable.rb index 0d8ea384..b1689f67 100644 --- a/app/controllers/concerns/notifiable.rb +++ b/app/controllers/concerns/notifiable.rb @@ -46,8 +46,6 @@ module Notifiable [ { partial: "shared/notifications/alert", locals: { message: data } } ] when "cta" [ resolve_cta(data) ] - when "loading" - [ { partial: "shared/notifications/loading", locals: { message: data } } ] when "notice" messages = Array(data) messages.map { |message| { partial: "shared/notifications/notice", locals: { message: message } } } diff --git a/app/controllers/concerns/restore_layout_preferences.rb b/app/controllers/concerns/restore_layout_preferences.rb new file mode 100644 index 00000000..284df4cc --- /dev/null +++ b/app/controllers/concerns/restore_layout_preferences.rb @@ -0,0 +1,24 @@ +module RestoreLayoutPreferences + extend ActiveSupport::Concern + + included do + before_action :restore_active_tabs + end + + private + def restore_active_tabs + last_selected_tab = Current.session&.get_preferred_tab("account_sidebar_tab") || "asset" + + @account_group_tab = account_group_tab_param || last_selected_tab + end + + def valid_account_group_tabs + %w[asset liability all] + end + + def account_group_tab_param + param_value = params[:account_sidebar_tab] + return nil unless param_value.in?(valid_account_group_tabs) + param_value + end +end diff --git a/app/controllers/cookie_sessions_controller.rb b/app/controllers/cookie_sessions_controller.rb new file mode 100644 index 00000000..7e76636f --- /dev/null +++ b/app/controllers/cookie_sessions_controller.rb @@ -0,0 +1,22 @@ +class CookieSessionsController < ApplicationController + def update + save_kv_to_session( + cookie_session_params[:tab_key], + cookie_session_params[:tab_value] + ) + + redirect_back_or_to root_path + end + + private + def cookie_session_params + params.require(:cookie_session).permit(:tab_key, :tab_value) + end + + def save_kv_to_session(key, value) + raise "Key must be a string" unless key.is_a?(String) + raise "Value must be a string" unless value.is_a?(String) + + session["custom_#{key}"] = value + end +end diff --git a/app/controllers/current_sessions_controller.rb b/app/controllers/current_sessions_controller.rb new file mode 100644 index 00000000..303b51f0 --- /dev/null +++ b/app/controllers/current_sessions_controller.rb @@ -0,0 +1,14 @@ +class CurrentSessionsController < ApplicationController + def update + if session_params[:tab_key].present? && session_params[:tab_value].present? + Current.session.set_preferred_tab(session_params[:tab_key], session_params[:tab_value]) + end + + head :ok + end + + private + def session_params + params.require(:current_session).permit(:tab_key, :tab_value) + end +end diff --git a/app/controllers/plaid_items_controller.rb b/app/controllers/plaid_items_controller.rb index 37efd5e3..8812cf0f 100644 --- a/app/controllers/plaid_items_controller.rb +++ b/app/controllers/plaid_items_controller.rb @@ -2,8 +2,8 @@ class PlaidItemsController < ApplicationController before_action :set_plaid_item, only: %i[destroy sync] def create - Current.family.plaid_items.create_from_public_token( - plaid_item_params[:public_token], + Current.family.create_plaid_item!( + public_token: plaid_item_params[:public_token], item_name: item_name, region: plaid_item_params[:region] ) diff --git a/app/javascript/controllers/sidebar_tabs_controller.js b/app/javascript/controllers/sidebar_tabs_controller.js deleted file mode 100644 index f88a70f7..00000000 --- a/app/javascript/controllers/sidebar_tabs_controller.js +++ /dev/null @@ -1,16 +0,0 @@ -import { Controller } from "@hotwired/stimulus"; - -// Connects to data-controller="sidebar-tabs" -export default class extends Controller { - static targets = ["account"]; - - select(event) { - this.accountTargets.forEach((account) => { - if (account.contains(event.target)) { - account.classList.add("bg-container"); - } else { - account.classList.remove("bg-container"); - } - }); - } -} diff --git a/app/jobs/sync_market_data_job.rb b/app/jobs/sync_market_data_job.rb new file mode 100644 index 00000000..074cda9f --- /dev/null +++ b/app/jobs/sync_market_data_job.rb @@ -0,0 +1,7 @@ +class SyncMarketDataJob < ApplicationJob + queue_as :scheduled + + def perform + MarketDataSyncer.new.sync_all + end +end diff --git a/app/models/account.rb b/app/models/account.rb index 352335e0..8c74b83e 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -61,6 +61,18 @@ class Account < ApplicationRecord end end + def syncing? + self_syncing = syncs.incomplete.any? + + # Since Plaid Items sync as a "group", if the item is syncing, even if the account + # sync hasn't yet started (i.e. we're still fetching the Plaid data), show it as syncing in UI. + if linked? + plaid_account&.plaid_item&.syncing? || self_syncing + else + self_syncing + end + end + def institution_domain url_string = plaid_account&.plaid_item&.institution_url return nil unless url_string.present? @@ -81,21 +93,6 @@ class Account < ApplicationRecord DestroyJob.perform_later(self) end - def sync_data(sync, start_date: nil) - Rails.logger.info("Processing balances (#{linked? ? 'reverse' : 'forward'})") - sync_balances - end - - def post_sync(sync) - family.remove_syncing_notice! - - accountable.post_sync(sync) - - unless sync.child? - family.auto_match_transfers! - end - end - def current_holdings holdings.where(currency: currency, date: holdings.maximum(:date)).order(amount: :desc) end @@ -172,10 +169,4 @@ class Account < ApplicationRecord def long_subtype_label accountable_class.long_subtype_label_for(subtype) || accountable_class.display_name end - - private - def sync_balances - strategy = linked? ? :reverse : :forward - Balance::Syncer.new(self, strategy: strategy).sync_balances - end end diff --git a/app/models/account/sync_complete_event.rb b/app/models/account/sync_complete_event.rb new file mode 100644 index 00000000..2a64ab80 --- /dev/null +++ b/app/models/account/sync_complete_event.rb @@ -0,0 +1,54 @@ +class Account::SyncCompleteEvent + attr_reader :account + + def initialize(account) + @account = account + end + + def broadcast + # Replace account row in accounts list + account.broadcast_replace_to( + account.family, + target: "account_#{account.id}", + partial: "accounts/account", + locals: { account: account } + ) + + # Replace the groups this account belongs to in the sidebar + account_group_ids.each do |id| + account.broadcast_replace_to( + account.family, + target: id, + partial: "accounts/accountable_group", + locals: { account_group: account_group, open: true } + ) + end + + # If this is a manual, unlinked account (i.e. not part of a Plaid Item), + # trigger the family sync complete broadcast so net worth graph is updated + unless account.linked? + account.family.broadcast_sync_complete + end + + # Refresh entire account page (only applies if currently viewing this account) + account.broadcast_refresh + end + + private + # The sidebar will show the account in both its classification tab and the "all" tab, + # so we need to broadcast to both. + def account_group_ids + id = account_group.id + [ id, "#{account_group.classification}_#{id}" ] + end + + def account_group + family_balance_sheet.account_groups.find do |group| + group.accounts.any? { |a| a.id == account.id } + end + end + + def family_balance_sheet + account.family.balance_sheet + end +end diff --git a/app/models/account/syncer.rb b/app/models/account/syncer.rb new file mode 100644 index 00000000..7e5fabcd --- /dev/null +++ b/app/models/account/syncer.rb @@ -0,0 +1,22 @@ +class Account::Syncer + attr_reader :account + + def initialize(account) + @account = account + end + + def perform_sync(sync) + Rails.logger.info("Processing balances (#{account.linked? ? 'reverse' : 'forward'})") + sync_balances + end + + def perform_post_sync + account.family.auto_match_transfers! + end + + private + def sync_balances + strategy = account.linked? ? :reverse : :forward + Balance::Syncer.new(account, strategy: strategy).sync_balances + end +end diff --git a/app/models/balance/base_calculator.rb b/app/models/balance/base_calculator.rb deleted file mode 100644 index 2d01dfe7..00000000 --- a/app/models/balance/base_calculator.rb +++ /dev/null @@ -1,35 +0,0 @@ -class Balance::BaseCalculator - attr_reader :account - - def initialize(account) - @account = account - end - - def calculate - Rails.logger.tagged(self.class.name) do - calculate_balances - end - end - - private - def sync_cache - @sync_cache ||= Balance::SyncCache.new(account) - end - - def build_balance(date, cash_balance, holdings_value) - Balance.new( - account_id: account.id, - date: date, - balance: holdings_value + cash_balance, - cash_balance: cash_balance, - currency: account.currency - ) - end - - def calculate_next_balance(prior_balance, transactions, direction: :forward) - flows = transactions.sum(&:amount) - negated = direction == :forward ? account.asset? : account.liability? - flows *= -1 if negated - prior_balance + flows - end -end diff --git a/app/models/balance/forward_calculator.rb b/app/models/balance/forward_calculator.rb index d024d2c6..4e6f2d5c 100644 --- a/app/models/balance/forward_calculator.rb +++ b/app/models/balance/forward_calculator.rb @@ -1,4 +1,16 @@ -class Balance::ForwardCalculator < Balance::BaseCalculator +class Balance::ForwardCalculator + attr_reader :account + + def initialize(account) + @account = account + end + + def calculate + Rails.logger.tagged("Balance::ForwardCalculator") do + calculate_balances + end + end + private def calculate_balances current_cash_balance = 0 @@ -25,4 +37,25 @@ class Balance::ForwardCalculator < Balance::BaseCalculator @balances end + + def sync_cache + @sync_cache ||= Balance::SyncCache.new(account) + end + + def build_balance(date, cash_balance, holdings_value) + Balance.new( + account_id: account.id, + date: date, + balance: holdings_value + cash_balance, + cash_balance: cash_balance, + currency: account.currency + ) + end + + def calculate_next_balance(prior_balance, transactions, direction: :forward) + flows = transactions.sum(&:amount) + negated = direction == :forward ? account.asset? : account.liability? + flows *= -1 if negated + prior_balance + flows + end end diff --git a/app/models/balance/reverse_calculator.rb b/app/models/balance/reverse_calculator.rb index 6a2ba70b..52a05608 100644 --- a/app/models/balance/reverse_calculator.rb +++ b/app/models/balance/reverse_calculator.rb @@ -1,4 +1,16 @@ -class Balance::ReverseCalculator < Balance::BaseCalculator +class Balance::ReverseCalculator + attr_reader :account + + def initialize(account) + @account = account + end + + def calculate + Rails.logger.tagged("Balance::ReverseCalculator") do + calculate_balances + end + end + private def calculate_balances current_cash_balance = account.cash_balance @@ -35,4 +47,25 @@ class Balance::ReverseCalculator < Balance::BaseCalculator @balances end + + def sync_cache + @sync_cache ||= Balance::SyncCache.new(account) + end + + def build_balance(date, cash_balance, holdings_value) + Balance.new( + account_id: account.id, + date: date, + balance: holdings_value + cash_balance, + cash_balance: cash_balance, + currency: account.currency + ) + end + + def calculate_next_balance(prior_balance, transactions, direction: :forward) + flows = transactions.sum(&:amount) + negated = direction == :forward ? account.asset? : account.liability? + flows *= -1 if negated + prior_balance + flows + end end diff --git a/app/models/balance_sheet.rb b/app/models/balance_sheet.rb index c289f86f..cc50e3da 100644 --- a/app/models/balance_sheet.rb +++ b/app/models/balance_sheet.rb @@ -22,20 +22,25 @@ class BalanceSheet end def classification_groups + asset_groups = account_groups("asset") + liability_groups = account_groups("liability") + [ ClassificationGroup.new( key: "asset", display_name: "Assets", icon: "plus", total_money: total_assets_money, - account_groups: account_groups("asset") + account_groups: asset_groups, + syncing?: asset_groups.any?(&:syncing?) ), ClassificationGroup.new( key: "liability", display_name: "Debts", icon: "minus", total_money: total_liabilities_money, - account_groups: account_groups("liability") + account_groups: liability_groups, + syncing?: liability_groups.any?(&:syncing?) ) ] end @@ -43,13 +48,17 @@ class BalanceSheet def account_groups(classification = nil) classification_accounts = classification ? totals_query.filter { |t| t.classification == classification } : totals_query classification_total = classification_accounts.sum(&:converted_balance) - account_groups = classification_accounts.group_by(&:accountable_type).transform_keys { |k| Accountable.from_type(k) } + account_groups = classification_accounts.group_by(&:accountable_type) + .transform_keys { |k| Accountable.from_type(k) } - account_groups.map do |accountable, accounts| + groups = account_groups.map do |accountable, accounts| group_total = accounts.sum(&:converted_balance) + key = accountable.model_name.param_key + AccountGroup.new( - key: accountable.model_name.param_key, + id: classification ? "#{classification}_#{key}_group" : "#{key}_group", + key: key, name: accountable.display_name, classification: accountable.classification, total: group_total, @@ -57,6 +66,7 @@ class BalanceSheet weight: classification_total.zero? ? 0 : group_total / classification_total.to_d * 100, missing_rates?: accounts.any? { |a| a.missing_rates? }, color: accountable.color, + syncing?: accounts.any?(&:is_syncing), accounts: accounts.map do |account| account.define_singleton_method(:weight) do classification_total.zero? ? 0 : account.converted_balance / classification_total.to_d * 100 @@ -65,7 +75,13 @@ class BalanceSheet account end.sort_by(&:weight).reverse ) - end.sort_by(&:weight).reverse + end + + groups.sort_by do |group| + manual_order = Accountable::TYPES + type_name = group.key.camelize + manual_order.index(type_name) || Float::INFINITY + end end def net_worth_series(period: Period.last_30_days) @@ -76,9 +92,13 @@ class BalanceSheet family.currency end + def syncing? + classification_groups.any? { |group| group.syncing? } + end + private - ClassificationGroup = Struct.new(:key, :display_name, :icon, :total_money, :account_groups, keyword_init: true) - AccountGroup = Struct.new(:key, :name, :accountable_type, :classification, :total, :total_money, :weight, :accounts, :color, :missing_rates?, keyword_init: true) + ClassificationGroup = Struct.new(:key, :display_name, :icon, :total_money, :account_groups, :syncing?, keyword_init: true) + AccountGroup = Struct.new(:id, :key, :name, :accountable_type, :classification, :total, :total_money, :weight, :accounts, :color, :missing_rates?, :syncing?, keyword_init: true) def active_accounts family.accounts.active.with_attached_logo @@ -87,9 +107,11 @@ class BalanceSheet def totals_query @totals_query ||= active_accounts .joins(ActiveRecord::Base.sanitize_sql_array([ "LEFT JOIN exchange_rates ON exchange_rates.date = CURRENT_DATE AND accounts.currency = exchange_rates.from_currency AND exchange_rates.to_currency = ?", currency ])) + .joins("LEFT JOIN syncs ON syncs.syncable_id = accounts.id AND syncs.syncable_type = 'Account' AND (syncs.status = 'pending' OR syncs.status = 'syncing')") .select( "accounts.*", "SUM(accounts.balance * COALESCE(exchange_rates.rate, 1)) as converted_balance", + "COUNT(syncs.id) > 0 as is_syncing", ActiveRecord::Base.sanitize_sql_array([ "COUNT(CASE WHEN accounts.currency <> ? AND exchange_rates.rate IS NULL THEN 1 END) as missing_rates", currency ]) ) .group(:classification, :accountable_type, :id) diff --git a/app/models/concerns/accountable.rb b/app/models/concerns/accountable.rb index 9d0ecb34..12ee9888 100644 --- a/app/models/concerns/accountable.rb +++ b/app/models/concerns/accountable.rb @@ -68,15 +68,6 @@ module Accountable end end - def post_sync(sync) - broadcast_replace_to( - account, - target: "chart_account_#{account.id}", - partial: "accounts/show/chart", - locals: { account: account } - ) - end - def display_name self.class.display_name end diff --git a/app/models/concerns/syncable.rb b/app/models/concerns/syncable.rb index ce3c30fd..8dfa8e41 100644 --- a/app/models/concerns/syncable.rb +++ b/app/models/concerns/syncable.rb @@ -6,24 +6,24 @@ module Syncable end def syncing? - syncs.where(status: [ :syncing, :pending ]).any? + raise NotImplementedError, "Subclasses must implement the syncing? method" end - def sync_later(start_date: nil, parent_sync: nil) - new_sync = syncs.create!(start_date: start_date, parent: parent_sync) + def sync_later(parent_sync: nil, window_start_date: nil, window_end_date: nil) + new_sync = syncs.create!(parent: parent_sync, window_start_date: window_start_date, window_end_date: window_end_date) SyncJob.perform_later(new_sync) end - def sync(start_date: nil) - syncs.create!(start_date: start_date).perform + def perform_sync(sync) + syncer.perform_sync(sync) end - def sync_data(sync, start_date: nil) - raise NotImplementedError, "Subclasses must implement the `sync_data` method" + def perform_post_sync + syncer.perform_post_sync end - def post_sync(sync) - # no-op, syncable can optionally provide implementation + def broadcast_sync_complete + sync_broadcaster.broadcast end def sync_error @@ -31,7 +31,7 @@ module Syncable end def last_synced_at - latest_sync&.last_ran_at + latest_sync&.completed_at end def last_sync_created_at @@ -40,6 +40,14 @@ module Syncable private def latest_sync - syncs.order(created_at: :desc).first + syncs.ordered.first + end + + def syncer + self.class::Syncer.new(self) + end + + def sync_broadcaster + self.class::SyncCompleteEvent.new(self) end end diff --git a/app/models/entry.rb b/app/models/entry.rb index c07f27cf..36f61c29 100644 --- a/app/models/entry.rb +++ b/app/models/entry.rb @@ -45,7 +45,7 @@ class Entry < ApplicationRecord def sync_account_later sync_start_date = [ date_previously_was, date ].compact.min unless destroyed? - account.sync_later(start_date: sync_start_date) + account.sync_later(window_start_date: sync_start_date) end def entryable_name_short diff --git a/app/models/family.rb b/app/models/family.rb index a3a73eec..0cfb8b89 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -1,5 +1,5 @@ class Family < ApplicationRecord - include Syncable, AutoTransferMatchable, Subscribeable + include PlaidConnectable, Syncable, AutoTransferMatchable, Subscribeable DATE_FORMATS = [ [ "MM-DD-YYYY", "%m-%d-%Y" ], @@ -15,7 +15,6 @@ class Family < ApplicationRecord has_many :users, dependent: :destroy has_many :accounts, dependent: :destroy - has_many :plaid_items, dependent: :destroy has_many :invitations, dependent: :destroy has_many :imports, dependent: :destroy @@ -36,6 +35,15 @@ class Family < ApplicationRecord validates :locale, inclusion: { in: I18n.available_locales.map(&:to_s) } validates :date_format, inclusion: { in: DATE_FORMATS.map(&:last) } + # If any accounts or plaid items are syncing, the family is also syncing, even if a formal "Family Sync" is not running. + def syncing? + Sync.joins("LEFT JOIN plaid_items ON plaid_items.id = syncs.syncable_id AND syncs.syncable_type = 'PlaidItem'") + .joins("LEFT JOIN accounts ON accounts.id = syncs.syncable_id AND syncs.syncable_type = 'Account'") + .where("syncs.syncable_id = ? OR accounts.family_id = ? OR plaid_items.family_id = ?", id, id, id) + .incomplete + .exists? + end + def assigned_merchants merchant_ids = transactions.where.not(merchant_id: nil).pluck(:merchant_id).uniq Merchant.where(id: merchant_ids) @@ -65,64 +73,10 @@ class Family < ApplicationRecord @income_statement ||= IncomeStatement.new(self) end - def sync_data(sync, start_date: nil) - # We don't rely on this value to guard the app, but keep it eventually consistent - sync_trial_status! - - Rails.logger.info("Syncing accounts for family #{id}") - accounts.manual.each do |account| - account.sync_later(start_date: start_date, parent_sync: sync) - end - - Rails.logger.info("Applying rules for family #{id}") - rules.each do |rule| - rule.apply_later - end - end - - def remove_syncing_notice! - broadcast_remove target: "syncing-notice" - end - - def post_sync(sync) - auto_match_transfers! - broadcast_refresh - end - - # If family has any syncs pending/syncing within the last 10 minutes, we show a persistent "syncing" notice. - # Ignore syncs older than 10 minutes as they are considered "stale" - def syncing? - Sync.where( - "(syncable_type = 'Family' AND syncable_id = ?) OR - (syncable_type = 'Account' AND syncable_id IN (SELECT id FROM accounts WHERE family_id = ? AND plaid_account_id IS NULL)) OR - (syncable_type = 'PlaidItem' AND syncable_id IN (SELECT id FROM plaid_items WHERE family_id = ?))", - id, id, id - ).where(status: [ "pending", "syncing" ], created_at: 10.minutes.ago..).exists? - end - def eu? country != "US" && country != "CA" end - def get_link_token(webhooks_url:, redirect_url:, accountable_type: nil, region: :us, access_token: nil) - provider = if region.to_sym == :eu - Provider::Registry.get_provider(:plaid_eu) - else - Provider::Registry.get_provider(:plaid_us) - end - - # early return when no provider - return nil unless provider - - provider.get_link_token( - user_id: id, - webhooks_url: webhooks_url, - redirect_url: redirect_url, - accountable_type: accountable_type, - access_token: access_token - ).link_token - end - def requires_data_provider? # If family has any trades, they need a provider for historical prices return true if trades.any? diff --git a/app/models/family/plaid_connectable.rb b/app/models/family/plaid_connectable.rb new file mode 100644 index 00000000..f2a997c8 --- /dev/null +++ b/app/models/family/plaid_connectable.rb @@ -0,0 +1,51 @@ +module Family::PlaidConnectable + extend ActiveSupport::Concern + + included do + has_many :plaid_items, dependent: :destroy + end + + def create_plaid_item!(public_token:, item_name:, region:) + provider = plaid_provider_for_region(region) + + public_token_response = provider.exchange_public_token(public_token) + + plaid_item = plaid_items.create!( + name: item_name, + plaid_id: public_token_response.item_id, + access_token: public_token_response.access_token, + plaid_region: region + ) + + plaid_item.sync_later + + plaid_item + end + + def get_link_token(webhooks_url:, redirect_url:, accountable_type: nil, region: :us, access_token: nil) + return nil unless plaid_us || plaid_eu + + provider = plaid_provider_for_region(region) + + provider.get_link_token( + user_id: self.id, + webhooks_url: webhooks_url, + redirect_url: redirect_url, + accountable_type: accountable_type, + access_token: access_token + ).link_token + end + + private + def plaid_us + @plaid ||= Provider::Registry.get_provider(:plaid_us) + end + + def plaid_eu + @plaid_eu ||= Provider::Registry.get_provider(:plaid_eu) + end + + def plaid_provider_for_region(region) + region.to_sym == :eu ? plaid_eu : plaid_us + end +end diff --git a/app/models/family/subscribeable.rb b/app/models/family/subscribeable.rb index 4c8624b7..94ee0971 100644 --- a/app/models/family/subscribeable.rb +++ b/app/models/family/subscribeable.rb @@ -72,10 +72,9 @@ module Family::Subscribeable (1 - days_left_in_trial.to_f / Subscription::TRIAL_DAYS) * 100 end - private - def sync_trial_status! - if subscription&.status == "trialing" && days_left_in_trial < 0 - subscription.update!(status: "paused") - end + def sync_trial_status! + if subscription&.status == "trialing" && days_left_in_trial < 0 + subscription.update!(status: "paused") end + end end diff --git a/app/models/family/sync_complete_event.rb b/app/models/family/sync_complete_event.rb new file mode 100644 index 00000000..628841d0 --- /dev/null +++ b/app/models/family/sync_complete_event.rb @@ -0,0 +1,21 @@ +class Family::SyncCompleteEvent + attr_reader :family + + def initialize(family) + @family = family + end + + def broadcast + family.broadcast_replace( + target: "balance-sheet", + partial: "pages/dashboard/balance_sheet", + locals: { balance_sheet: family.balance_sheet } + ) + + family.broadcast_replace( + target: "net-worth-chart", + partial: "pages/dashboard/net_worth_chart", + locals: { balance_sheet: family.balance_sheet, period: Period.last_30_days } + ) + end +end diff --git a/app/models/family/syncer.rb b/app/models/family/syncer.rb new file mode 100644 index 00000000..30ce2ad5 --- /dev/null +++ b/app/models/family/syncer.rb @@ -0,0 +1,31 @@ +class Family::Syncer + attr_reader :family + + def initialize(family) + @family = family + end + + def perform_sync(sync) + # We don't rely on this value to guard the app, but keep it eventually consistent + family.sync_trial_status! + + Rails.logger.info("Applying rules for family #{family.id}") + family.rules.each do |rule| + rule.apply_later + end + + # Schedule child syncs + child_syncables.each do |syncable| + syncable.sync_later(parent_sync: sync, window_start_date: sync.window_start_date, window_end_date: sync.window_end_date) + end + end + + def perform_post_sync + family.auto_match_transfers! + end + + private + def child_syncables + family.plaid_items + family.accounts.manual + end +end diff --git a/app/models/holding/base_calculator.rb b/app/models/holding/base_calculator.rb deleted file mode 100644 index d9b85d03..00000000 --- a/app/models/holding/base_calculator.rb +++ /dev/null @@ -1,62 +0,0 @@ -class Holding::BaseCalculator - attr_reader :account - - def initialize(account) - @account = account - end - - def calculate - Rails.logger.tagged(self.class.name) do - holdings = calculate_holdings - Holding.gapfill(holdings) - end - end - - private - def portfolio_cache - @portfolio_cache ||= Holding::PortfolioCache.new(account) - end - - def empty_portfolio - securities = portfolio_cache.get_securities - securities.each_with_object({}) { |security, hash| hash[security.id] = 0 } - end - - def generate_starting_portfolio - empty_portfolio - end - - def transform_portfolio(previous_portfolio, trade_entries, direction: :forward) - new_quantities = previous_portfolio.dup - - trade_entries.each do |trade_entry| - trade = trade_entry.entryable - security_id = trade.security_id - qty_change = trade.qty - qty_change = qty_change * -1 if direction == :reverse - new_quantities[security_id] = (new_quantities[security_id] || 0) + qty_change - end - - new_quantities - end - - def build_holdings(portfolio, date, price_source: nil) - portfolio.map do |security_id, qty| - price = portfolio_cache.get_price(security_id, date, source: price_source) - - if price.nil? - next - end - - Holding.new( - account_id: account.id, - security_id: security_id, - date: date, - qty: qty, - price: price.price, - currency: price.currency, - amount: qty * price.price - ) - end.compact - end -end diff --git a/app/models/holding/forward_calculator.rb b/app/models/holding/forward_calculator.rb index d2f2e8d7..43f91f7a 100644 --- a/app/models/holding/forward_calculator.rb +++ b/app/models/holding/forward_calculator.rb @@ -1,10 +1,12 @@ -class Holding::ForwardCalculator < Holding::BaseCalculator - private - def portfolio_cache - @portfolio_cache ||= Holding::PortfolioCache.new(account) - end +class Holding::ForwardCalculator + attr_reader :account - def calculate_holdings + def initialize(account) + @account = account + end + + def calculate + Rails.logger.tagged("Holding::ForwardCalculator") do current_portfolio = generate_starting_portfolio next_portfolio = {} holdings = [] @@ -16,6 +18,55 @@ class Holding::ForwardCalculator < Holding::BaseCalculator current_portfolio = next_portfolio end - holdings + Holding.gapfill(holdings) + end + end + + private + def portfolio_cache + @portfolio_cache ||= Holding::PortfolioCache.new(account) + end + + def empty_portfolio + securities = portfolio_cache.get_securities + securities.each_with_object({}) { |security, hash| hash[security.id] = 0 } + end + + def generate_starting_portfolio + empty_portfolio + end + + def transform_portfolio(previous_portfolio, trade_entries, direction: :forward) + new_quantities = previous_portfolio.dup + + trade_entries.each do |trade_entry| + trade = trade_entry.entryable + security_id = trade.security_id + qty_change = trade.qty + qty_change = qty_change * -1 if direction == :reverse + new_quantities[security_id] = (new_quantities[security_id] || 0) + qty_change + end + + new_quantities + end + + def build_holdings(portfolio, date, price_source: nil) + portfolio.map do |security_id, qty| + price = portfolio_cache.get_price(security_id, date, source: price_source) + + if price.nil? + next + end + + Holding.new( + account_id: account.id, + security_id: security_id, + date: date, + qty: qty, + price: price.price, + currency: price.currency, + amount: qty * price.price + ) + end.compact end end diff --git a/app/models/holding/portfolio_cache.rb b/app/models/holding/portfolio_cache.rb index 2d67a1d8..9ffed15b 100644 --- a/app/models/holding/portfolio_cache.rb +++ b/app/models/holding/portfolio_cache.rb @@ -83,9 +83,6 @@ class Holding::PortfolioCache securities.each do |security| Rails.logger.info "Loading security: ID=#{security.id} Ticker=#{security.ticker}" - # Load prices from provider to DB - security.sync_provider_prices(start_date: account.start_date) - # High priority prices from DB (synced from provider) db_prices = security.prices.where(date: account.start_date..Date.current).map do |price| PriceWithPriority.new( diff --git a/app/models/holding/reverse_calculator.rb b/app/models/holding/reverse_calculator.rb index 62e2dc95..f52184d7 100644 --- a/app/models/holding/reverse_calculator.rb +++ b/app/models/holding/reverse_calculator.rb @@ -1,4 +1,17 @@ -class Holding::ReverseCalculator < Holding::BaseCalculator +class Holding::ReverseCalculator + attr_reader :account + + def initialize(account) + @account = account + end + + def calculate + Rails.logger.tagged("Holding::ReverseCalculator") do + holdings = calculate_holdings + Holding.gapfill(holdings) + end + end + private # Reverse calculators will use the existing holdings as a source of security ids and prices # since it is common for a provider to supply "current day" holdings but not all the historical @@ -25,6 +38,11 @@ class Holding::ReverseCalculator < Holding::BaseCalculator holdings end + def empty_portfolio + securities = portfolio_cache.get_securities + securities.each_with_object({}) { |security, hash| hash[security.id] = 0 } + end + # Since this is a reverse sync, we start with today's holdings def generate_starting_portfolio holding_quantities = empty_portfolio @@ -37,4 +55,38 @@ class Holding::ReverseCalculator < Holding::BaseCalculator holding_quantities end + + def transform_portfolio(previous_portfolio, trade_entries, direction: :forward) + new_quantities = previous_portfolio.dup + + trade_entries.each do |trade_entry| + trade = trade_entry.entryable + security_id = trade.security_id + qty_change = trade.qty + qty_change = qty_change * -1 if direction == :reverse + new_quantities[security_id] = (new_quantities[security_id] || 0) + qty_change + end + + new_quantities + end + + def build_holdings(portfolio, date, price_source: nil) + portfolio.map do |security_id, qty| + price = portfolio_cache.get_price(security_id, date, source: price_source) + + if price.nil? + next + end + + Holding.new( + account_id: account.id, + security_id: security_id, + date: date, + qty: qty, + price: price.price, + currency: price.currency, + amount: qty * price.price + ) + end.compact + end end diff --git a/app/models/import.rb b/app/models/import.rb index 3ea68015..e96d1fc1 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -62,7 +62,7 @@ class Import < ApplicationRecord def publish import! - family.sync + family.sync_later update! status: :complete rescue => error diff --git a/app/models/market_data_syncer.rb b/app/models/market_data_syncer.rb new file mode 100644 index 00000000..d634cd45 --- /dev/null +++ b/app/models/market_data_syncer.rb @@ -0,0 +1,196 @@ +class MarketDataSyncer + DEFAULT_HISTORY_DAYS = 30 + RATE_PROVIDER_NAME = :synth + PRICE_PROVIDER_NAME = :synth + + MissingExchangeRateError = Class.new(StandardError) + InvalidExchangeRateDataError = Class.new(StandardError) + MissingSecurityPriceError = Class.new(StandardError) + InvalidSecurityPriceDataError = Class.new(StandardError) + + class << self + def for(family: nil, account: nil) + new(family: family, account: account) + end + end + + # Syncer can optionally be scoped. Otherwise, it syncs all user data + def initialize(family: nil, account: nil) + @family = family + @account = account + end + + def sync_all(full_history: false) + sync_exchange_rates(full_history: full_history) + sync_prices(full_history: full_history) + end + + def sync_exchange_rates(full_history: false) + unless rate_provider + Rails.logger.warn("No rate provider configured for MarketDataSyncer.sync_exchange_rates, skipping sync") + return + end + + # Finds distinct currency pairs + entry_pairs = entries_scope.joins(:account) + .where.not("entries.currency = accounts.currency") + .select("entries.currency as source, accounts.currency as target") + .distinct + + # All accounts in currency not equal to the family currency require exchange rates to show a normalized historical graph + account_pairs = accounts_scope.joins(:family) + .where.not("families.currency = accounts.currency") + .select("accounts.currency as source, families.currency as target") + .distinct + + pairs = (entry_pairs + account_pairs).uniq + + pairs.each do |pair| + sync_exchange_rate(from: pair.source, to: pair.target, full_history: full_history) + end + end + + def sync_prices(full_history: false) + unless price_provider + Rails.logger.warn("No price provider configured for MarketDataSyncer.sync_prices, skipping sync") + nil + end + + securities_scope.each do |security| + sync_security_price(security: security, full_history: full_history) + end + end + + private + attr_reader :family, :account + + def accounts_scope + return Account.where(id: account.id) if account + return family.accounts if family + Account.all + end + + def entries_scope + account&.entries || family&.entries || Entry.all + end + + def securities_scope + if account + account.trades.joins(:security).where.not(securities: { exchange_operating_mic: nil }) + elsif family + family.trades.joins(:security).where.not(securities: { exchange_operating_mic: nil }) + else + Security.where.not(exchange_operating_mic: nil) + end + end + + def sync_security_price(security:, full_history:) + start_date = full_history ? find_oldest_required_price(security: security) : default_start_date + + Rails.logger.info("Syncing security price for: #{security.ticker}, start_date: #{start_date}, end_date: #{end_date}") + + fetched_prices = price_provider.fetch_security_prices( + security, + start_date: start_date, + end_date: end_date + ) + + unless fetched_prices.success? + error = MissingSecurityPriceError.new( + "#{PRICE_PROVIDER_NAME} could not fetch security price for: #{security.ticker} between: #{start_date} and: #{Date.current}. Provider error: #{fetched_prices.error.message}" + ) + + Rails.logger.warn(error.message) + Sentry.capture_exception(error, level: :warning) + + return + end + + prices_for_upsert = fetched_prices.data.map do |price| + if price.security.nil? || price.date.nil? || price.price.nil? || price.currency.nil? + error = InvalidSecurityPriceDataError.new( + "#{PRICE_PROVIDER_NAME} returned invalid price data for security: #{security.ticker} on: #{price.date}. Price data: #{price.inspect}" + ) + + Rails.logger.warn(error.message) + Sentry.capture_exception(error, level: :warning) + + next + end + + { + security_id: price.security.id, + date: price.date, + price: price.price, + currency: price.currency + } + end.compact + + Security::Price.upsert_all( + prices_for_upsert, + unique_by: %i[security_id date currency] + ) + end + + def sync_exchange_rate(from:, to:, full_history:) + start_date = full_history ? find_oldest_required_rate(from_currency: from) : default_start_date + + Rails.logger.info("Syncing exchange rate from: #{from}, to: #{to}, start_date: #{start_date}, end_date: #{end_date}") + + fetched_rates = rate_provider.fetch_exchange_rates( + from: from, + to: to, + start_date: start_date, + end_date: end_date + ) + + unless fetched_rates.success? + message = "#{RATE_PROVIDER_NAME} could not fetch exchange rate pair from: #{from} to: #{to} between: #{start_date} and: #{Date.current}. Provider error: #{fetched_rates.error.message}" + Rails.logger.warn(message) + Sentry.capture_exception(MissingExchangeRateError.new(message)) + return + end + + rates_for_upsert = fetched_rates.data.map do |rate| + if rate.from.nil? || rate.to.nil? || rate.date.nil? || rate.rate.nil? + message = "#{RATE_PROVIDER_NAME} returned invalid rate data for pair from: #{from} to: #{to} on: #{rate.date}. Rate data: #{rate.inspect}" + Rails.logger.warn(message) + Sentry.capture_exception(InvalidExchangeRateDataError.new(message)) + next + end + + { + from_currency: rate.from, + to_currency: rate.to, + date: rate.date, + rate: rate.rate + } + end.compact + + ExchangeRate.upsert_all( + rates_for_upsert, + unique_by: %i[from_currency to_currency date] + ) + end + + def rate_provider + Provider::Registry.for_concept(:exchange_rates).get_provider(RATE_PROVIDER_NAME) + end + + def price_provider + Provider::Registry.for_concept(:securities).get_provider(PRICE_PROVIDER_NAME) + end + + def find_oldest_required_rate(from_currency:) + entries_scope.where(currency: from_currency).minimum(:date) || default_start_date + end + + def default_start_date + DEFAULT_HISTORY_DAYS.days.ago.to_date + end + + # Since we're querying market data from a US-based API, end date should always be today (EST) + def end_date + Date.current.in_time_zone("America/New_York").to_date + end +end diff --git a/app/models/plaid_item.rb b/app/models/plaid_item.rb index 2226f12f..4aae91ca 100644 --- a/app/models/plaid_item.rb +++ b/app/models/plaid_item.rb @@ -1,5 +1,5 @@ class PlaidItem < ApplicationRecord - include Provided, Syncable + include Syncable enum :plaid_region, { us: "us", eu: "eu" } enum :status, { good: "good", requires_update: "requires_update" }, default: :good @@ -22,39 +22,6 @@ class PlaidItem < ApplicationRecord scope :ordered, -> { order(created_at: :desc) } scope :needs_update, -> { where(status: :requires_update) } - class << self - def create_from_public_token(token, item_name:, region:) - response = plaid_provider_for_region(region).exchange_public_token(token) - - new_plaid_item = create!( - name: item_name, - plaid_id: response.item_id, - access_token: response.access_token, - plaid_region: region - ) - - new_plaid_item.sync_later - end - end - - def sync_data(sync, start_date: nil) - begin - Rails.logger.info("Fetching and loading Plaid data") - fetch_and_load_plaid_data(sync) - update!(status: :good) if requires_update? - - # Schedule account syncs - accounts.each do |account| - account.sync_later(start_date: start_date, parent_sync: sync) - end - - Rails.logger.info("Plaid data fetched and loaded") - rescue Plaid::ApiError => e - handle_plaid_error(e) - raise e - end - end - def get_update_link_token(webhooks_url:, redirect_url:) begin family.get_link_token( @@ -76,9 +43,8 @@ class PlaidItem < ApplicationRecord end end - def post_sync(sync) - auto_match_categories! - family.broadcast_refresh + def build_category_alias_matcher(user_categories) + Provider::Plaid::CategoryAliasMatcher.new(user_categories) end def destroy_later @@ -86,6 +52,14 @@ class PlaidItem < ApplicationRecord DestroyJob.perform_later(self) end + def syncing? + Sync.joins("LEFT JOIN accounts a ON a.id = syncs.syncable_id AND syncs.syncable_type = 'Account'") + .joins("LEFT JOIN plaid_accounts pa ON pa.id = a.plaid_account_id") + .where("syncs.syncable_id = ? OR pa.plaid_item_id = ?", id, id) + .incomplete + .exists? + end + def auto_match_categories! if family.categories.none? family.categories.bootstrap! @@ -117,123 +91,11 @@ class PlaidItem < ApplicationRecord end private - def fetch_and_load_plaid_data(sync) - data = {} - - # Log what we're about to fetch - Rails.logger.info "Starting Plaid data fetch (accounts, transactions, investments, liabilities)" - - item = plaid_provider.get_item(access_token).item - update!(available_products: item.available_products, billed_products: item.billed_products) - - # Institution details - if item.institution_id.present? - begin - Rails.logger.info "Fetching Plaid institution details for #{item.institution_id}" - institution = plaid_provider.get_institution(item.institution_id) - update!( - institution_id: item.institution_id, - institution_url: institution.institution.url, - institution_color: institution.institution.primary_color - ) - rescue Plaid::ApiError => e - Rails.logger.warn "Failed to fetch Plaid institution details: #{e.message}" - end - end - - # Accounts - fetched_accounts = plaid_provider.get_item_accounts(self).accounts - data[:accounts] = fetched_accounts || [] - sync.update!(data: data) - Rails.logger.info "Processing Plaid accounts (count: #{fetched_accounts.size})" - - internal_plaid_accounts = fetched_accounts.map do |account| - internal_plaid_account = plaid_accounts.find_or_create_from_plaid_data!(account, family) - internal_plaid_account.sync_account_data!(account) - internal_plaid_account - end - - # Transactions - fetched_transactions = safe_fetch_plaid_data(:get_item_transactions) - data[:transactions] = fetched_transactions || [] - sync.update!(data: data) - - if fetched_transactions - Rails.logger.info "Processing Plaid transactions (added: #{fetched_transactions.added.size}, modified: #{fetched_transactions.modified.size}, removed: #{fetched_transactions.removed.size})" - transaction do - internal_plaid_accounts.each do |internal_plaid_account| - added = fetched_transactions.added.select { |t| t.account_id == internal_plaid_account.plaid_id } - modified = fetched_transactions.modified.select { |t| t.account_id == internal_plaid_account.plaid_id } - removed = fetched_transactions.removed.select { |t| t.account_id == internal_plaid_account.plaid_id } - - internal_plaid_account.sync_transactions!(added:, modified:, removed:) - end - - update!(next_cursor: fetched_transactions.cursor) - end - end - - # Investments - fetched_investments = safe_fetch_plaid_data(:get_item_investments) - data[:investments] = fetched_investments || [] - sync.update!(data: data) - - if fetched_investments - Rails.logger.info "Processing Plaid investments (transactions: #{fetched_investments.transactions.size}, holdings: #{fetched_investments.holdings.size}, securities: #{fetched_investments.securities.size})" - transaction do - internal_plaid_accounts.each do |internal_plaid_account| - transactions = fetched_investments.transactions.select { |t| t.account_id == internal_plaid_account.plaid_id } - holdings = fetched_investments.holdings.select { |h| h.account_id == internal_plaid_account.plaid_id } - securities = fetched_investments.securities - - internal_plaid_account.sync_investments!(transactions:, holdings:, securities:) - end - end - end - - # Liabilities - fetched_liabilities = safe_fetch_plaid_data(:get_item_liabilities) - data[:liabilities] = fetched_liabilities || [] - sync.update!(data: data) - - if fetched_liabilities - Rails.logger.info "Processing Plaid liabilities (credit: #{fetched_liabilities.credit&.size || 0}, mortgage: #{fetched_liabilities.mortgage&.size || 0}, student: #{fetched_liabilities.student&.size || 0})" - transaction do - internal_plaid_accounts.each do |internal_plaid_account| - credit = fetched_liabilities.credit&.find { |l| l.account_id == internal_plaid_account.plaid_id } - mortgage = fetched_liabilities.mortgage&.find { |l| l.account_id == internal_plaid_account.plaid_id } - student = fetched_liabilities.student&.find { |l| l.account_id == internal_plaid_account.plaid_id } - - internal_plaid_account.sync_credit_data!(credit) if credit - internal_plaid_account.sync_mortgage_data!(mortgage) if mortgage - internal_plaid_account.sync_student_loan_data!(student) if student - end - end - end - end - - def safe_fetch_plaid_data(method) - begin - plaid_provider.send(method, self) - rescue Plaid::ApiError => e - Rails.logger.warn("Error fetching #{method} for item #{id}: #{e.message}") - nil - end - end - def remove_plaid_item plaid_provider.remove_item(access_token) rescue StandardError => e Rails.logger.warn("Failed to remove Plaid item #{id}: #{e.message}") end - def handle_plaid_error(error) - error_body = JSON.parse(error.response_body) - - if error_body["error_code"] == "ITEM_LOGIN_REQUIRED" - update!(status: :requires_update) - end - end - class PlaidConnectionLostError < StandardError; end end diff --git a/app/models/plaid_item/provided.rb b/app/models/plaid_item/provided.rb deleted file mode 100644 index f2e8ee8f..00000000 --- a/app/models/plaid_item/provided.rb +++ /dev/null @@ -1,30 +0,0 @@ -module PlaidItem::Provided - extend ActiveSupport::Concern - - class_methods do - def plaid_us_provider - Provider::Registry.get_provider(:plaid_us) - end - - def plaid_eu_provider - Provider::Registry.get_provider(:plaid_eu) - end - - def plaid_provider_for_region(region) - region.to_sym == :eu ? plaid_eu_provider : plaid_us_provider - end - end - - def build_category_alias_matcher(user_categories) - Provider::Plaid::CategoryAliasMatcher.new(user_categories) - end - - private - def eu? - raise "eu? is not implemented for #{self.class.name}" - end - - def plaid_provider - eu? ? self.class.plaid_eu_provider : self.class.plaid_us_provider - end -end diff --git a/app/models/plaid_item/sync_complete_event.rb b/app/models/plaid_item/sync_complete_event.rb new file mode 100644 index 00000000..ca008a0a --- /dev/null +++ b/app/models/plaid_item/sync_complete_event.rb @@ -0,0 +1,22 @@ +class PlaidItem::SyncCompleteEvent + attr_reader :plaid_item + + def initialize(plaid_item) + @plaid_item = plaid_item + end + + def broadcast + plaid_item.accounts.each do |account| + account.broadcast_sync_complete + end + + plaid_item.broadcast_replace_to( + plaid_item.family, + target: "plaid_item_#{plaid_item.id}", + partial: "plaid_items/plaid_item", + locals: { plaid_item: plaid_item } + ) + + plaid_item.family.broadcast_sync_complete + end +end diff --git a/app/models/plaid_item/syncer.rb b/app/models/plaid_item/syncer.rb new file mode 100644 index 00000000..e32b9ed4 --- /dev/null +++ b/app/models/plaid_item/syncer.rb @@ -0,0 +1,149 @@ +class PlaidItem::Syncer + attr_reader :plaid_item + + def initialize(plaid_item) + @plaid_item = plaid_item + end + + def perform_sync(sync) + begin + Rails.logger.info("Fetching and loading Plaid data") + fetch_and_load_plaid_data + plaid_item.update!(status: :good) if plaid_item.requires_update? + + plaid_item.accounts.each do |account| + account.sync_later(parent_sync: sync, window_start_date: sync.window_start_date, window_end_date: sync.window_end_date) + end + + Rails.logger.info("Plaid data fetched and loaded") + rescue Plaid::ApiError => e + handle_plaid_error(e) + raise e + end + end + + def perform_post_sync + plaid_item.auto_match_categories! + end + + private + def plaid + plaid_item.plaid_region == "eu" ? plaid_eu : plaid_us + end + + def plaid_eu + @plaid_eu ||= Provider::Registry.get_provider(:plaid_eu) + end + + def plaid_us + @plaid_us ||= Provider::Registry.get_provider(:plaid_us) + end + + def safe_fetch_plaid_data(method) + begin + plaid.send(method, plaid_item) + rescue Plaid::ApiError => e + Rails.logger.warn("Error fetching #{method} for item #{plaid_item.id}: #{e.message}") + nil + end + end + + def handle_plaid_error(error) + error_body = JSON.parse(error.response_body) + + if error_body["error_code"] == "ITEM_LOGIN_REQUIRED" + plaid_item.update!(status: :requires_update) + end + end + + def fetch_and_load_plaid_data + data = {} + + # Log what we're about to fetch + Rails.logger.info "Starting Plaid data fetch (accounts, transactions, investments, liabilities)" + + item = plaid.get_item(plaid_item.access_token).item + plaid_item.update!(available_products: item.available_products, billed_products: item.billed_products) + + # Institution details + if item.institution_id.present? + begin + Rails.logger.info "Fetching Plaid institution details for #{item.institution_id}" + institution = plaid.get_institution(item.institution_id) + plaid_item.update!( + institution_id: item.institution_id, + institution_url: institution.institution.url, + institution_color: institution.institution.primary_color + ) + rescue Plaid::ApiError => e + Rails.logger.warn "Failed to fetch Plaid institution details: #{e.message}" + end + end + + # Accounts + fetched_accounts = plaid.get_item_accounts(plaid_item).accounts + data[:accounts] = fetched_accounts || [] + Rails.logger.info "Processing Plaid accounts (count: #{fetched_accounts.size})" + + internal_plaid_accounts = fetched_accounts.map do |account| + internal_plaid_account = plaid_item.plaid_accounts.find_or_create_from_plaid_data!(account, plaid_item.family) + internal_plaid_account.sync_account_data!(account) + internal_plaid_account + end + + # Transactions + fetched_transactions = safe_fetch_plaid_data(:get_item_transactions) + data[:transactions] = fetched_transactions || [] + + if fetched_transactions + Rails.logger.info "Processing Plaid transactions (added: #{fetched_transactions.added.size}, modified: #{fetched_transactions.modified.size}, removed: #{fetched_transactions.removed.size})" + PlaidItem.transaction do + internal_plaid_accounts.each do |internal_plaid_account| + added = fetched_transactions.added.select { |t| t.account_id == internal_plaid_account.plaid_id } + modified = fetched_transactions.modified.select { |t| t.account_id == internal_plaid_account.plaid_id } + removed = fetched_transactions.removed.select { |t| t.account_id == internal_plaid_account.plaid_id } + + internal_plaid_account.sync_transactions!(added:, modified:, removed:) + end + + plaid_item.update!(next_cursor: fetched_transactions.cursor) + end + end + + # Investments + fetched_investments = safe_fetch_plaid_data(:get_item_investments) + data[:investments] = fetched_investments || [] + + if fetched_investments + Rails.logger.info "Processing Plaid investments (transactions: #{fetched_investments.transactions.size}, holdings: #{fetched_investments.holdings.size}, securities: #{fetched_investments.securities.size})" + PlaidItem.transaction do + internal_plaid_accounts.each do |internal_plaid_account| + transactions = fetched_investments.transactions.select { |t| t.account_id == internal_plaid_account.plaid_id } + holdings = fetched_investments.holdings.select { |h| h.account_id == internal_plaid_account.plaid_id } + securities = fetched_investments.securities + + internal_plaid_account.sync_investments!(transactions:, holdings:, securities:) + end + end + end + + # Liabilities + fetched_liabilities = safe_fetch_plaid_data(:get_item_liabilities) + data[:liabilities] = fetched_liabilities || [] + + if fetched_liabilities + Rails.logger.info "Processing Plaid liabilities (credit: #{fetched_liabilities.credit&.size || 0}, mortgage: #{fetched_liabilities.mortgage&.size || 0}, student: #{fetched_liabilities.student&.size || 0})" + PlaidItem.transaction do + internal_plaid_accounts.each do |internal_plaid_account| + credit = fetched_liabilities.credit&.find { |l| l.account_id == internal_plaid_account.plaid_id } + mortgage = fetched_liabilities.mortgage&.find { |l| l.account_id == internal_plaid_account.plaid_id } + student = fetched_liabilities.student&.find { |l| l.account_id == internal_plaid_account.plaid_id } + + internal_plaid_account.sync_credit_data!(credit) if credit + internal_plaid_account.sync_mortgage_data!(mortgage) if mortgage + internal_plaid_account.sync_student_loan_data!(student) if student + end + end + end + end +end diff --git a/app/models/provider.rb b/app/models/provider.rb index c90866e7..e9702349 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -36,8 +36,6 @@ class Provider default_error_transformer(error) end - Sentry.capture_exception(transformed_error) - Response.new( success?: false, data: nil, diff --git a/app/models/security/provided.rb b/app/models/security/provided.rb index b342c9e5..3d344f29 100644 --- a/app/models/security/provided.rb +++ b/app/models/security/provided.rb @@ -28,44 +28,6 @@ module Security::Provided end end - def sync_provider_prices(start_date:, end_date: Date.current) - unless has_prices? - Rails.logger.warn("Security id=#{id} ticker=#{ticker} is not known by provider, skipping price sync") - return 0 - end - - unless provider.present? - Rails.logger.warn("No security provider configured, cannot sync prices for id=#{id} ticker=#{ticker}") - return 0 - end - - response = provider.fetch_security_prices(self, start_date: start_date, end_date: end_date) - - unless response.success? - Rails.logger.error("Provider error for sync_provider_prices with id=#{id} ticker=#{ticker}: #{response.error}") - return 0 - end - - fetched_prices = response.data.map do |price| - { - security_id: price.security.id, - date: price.date, - price: price.price, - currency: price.currency - } - end - - valid_prices = fetched_prices.reject do |price| - is_invalid = price[:date].nil? || price[:price].nil? || price[:currency].nil? - if is_invalid - Rails.logger.warn("Invalid price data for security_id=#{id}: Missing required fields in price record: #{price.inspect}") - end - is_invalid - end - - Security::Price.upsert_all(valid_prices, unique_by: %i[security_id date currency]) - end - def find_or_fetch_price(date: Date.current, cache: true) price = prices.find_by(date: date) diff --git a/app/models/session.rb b/app/models/session.rb index ce26938a..66faa0f5 100644 --- a/app/models/session.rb +++ b/app/models/session.rb @@ -9,4 +9,14 @@ class Session < ApplicationRecord self.user_agent = Current.user_agent self.ip_address = Current.ip_address end + + def get_preferred_tab(tab_key) + data.dig("tab_preferences", tab_key) + end + + def set_preferred_tab(tab_key, tab_value) + data["tab_preferences"] ||= {} + data["tab_preferences"][tab_key] = tab_value + save! + end end diff --git a/app/models/subscription.rb b/app/models/subscription.rb index 5f96361e..90b5303d 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -17,6 +17,7 @@ class Subscription < ApplicationRecord validates :stripe_id, presence: true, if: :active? validates :trial_ends_at, presence: true, if: :trialing? + validates :family_id, uniqueness: true class << self def new_trial_ends_at diff --git a/app/models/sync.rb b/app/models/sync.rb index 4d923f12..f783bbc2 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -1,4 +1,6 @@ class Sync < ApplicationRecord + include AASM + Error = Class.new(StandardError) belongs_to :syncable, polymorphic: true @@ -6,12 +8,31 @@ class Sync < ApplicationRecord belongs_to :parent, class_name: "Sync", optional: true has_many :children, class_name: "Sync", foreign_key: :parent_id, dependent: :destroy - enum :status, { pending: "pending", syncing: "syncing", completed: "completed", failed: "failed" } - scope :ordered, -> { order(created_at: :desc) } + scope :incomplete, -> { where(status: [ :pending, :syncing ]) } - def child? - parent_id.present? + validate :window_valid + + # Sync state machine + aasm column: :status, timestamps: true do + state :pending, initial: true + state :syncing + state :completed + state :failed + + after_all_transitions :log_status_change + + event :start, after_commit: :report_warnings do + transitions from: :pending, to: :syncing + end + + event :complete do + transitions from: :syncing, to: :completed + end + + event :fail do + transitions from: :syncing, to: :failed + end end def perform @@ -19,43 +40,83 @@ class Sync < ApplicationRecord start! begin - syncable.sync_data(self, start_date: start_date) - - complete! - Rails.logger.info("Sync completed, starting post-sync") - syncable.post_sync(self) - Rails.logger.info("Post-sync completed") - rescue StandardError => error - fail! error, report_error: true + syncable.perform_sync(self) + rescue => e + fail! + update(error: e.message) + report_error(e) + ensure + finalize_if_all_children_finalized end end end - private - def start! - Rails.logger.info("Starting sync") - update! status: :syncing - end + # Finalizes the current sync AND parent (if it exists) + def finalize_if_all_children_finalized + Sync.transaction do + lock! - def complete! - Rails.logger.info("Sync completed") - update! status: :completed, last_ran_at: Time.current - end + # If this is the "parent" and there are still children running, don't finalize. + return unless all_children_finalized? - def fail!(error, report_error: false) - Rails.logger.error("Sync failed: #{error.message}") - - if report_error - Sentry.capture_exception(error) do |scope| - scope.set_context("sync", { id: id, syncable_type: syncable_type, syncable_id: syncable_id }) - scope.set_tags(sync_id: id) + if syncing? + if has_failed_children? + fail! + else + complete! end end - update!( - status: :failed, - error: error.message, - last_ran_at: Time.current - ) + # If we make it here, the sync is finalized. Run post-sync, regardless of failure/success. + perform_post_sync + end + + # If this sync has a parent, try to finalize it so the child status propagates up the chain. + parent&.finalize_if_all_children_finalized + end + + private + def log_status_change + Rails.logger.info("changing from #{aasm.from_state} to #{aasm.to_state} (event: #{aasm.current_event})") + end + + def has_failed_children? + children.failed.any? + end + + def all_children_finalized? + children.incomplete.empty? + end + + def perform_post_sync + Rails.logger.info("Performing post-sync for #{syncable_type} (#{syncable.id})") + syncable.perform_post_sync + syncable.broadcast_sync_complete + rescue => e + Rails.logger.error("Error performing post-sync for #{syncable_type} (#{syncable.id}): #{e.message}") + report_error(e) + end + + def report_error(error) + Sentry.capture_exception(error) do |scope| + scope.set_tags(sync_id: id) + end + end + + def report_warnings + todays_sync_count = syncable.syncs.where(created_at: Date.current.all_day).count + + if todays_sync_count > 10 + Sentry.capture_exception( + Error.new("#{syncable_type} (#{syncable.id}) has exceeded 10 syncs today (count: #{todays_sync_count})"), + level: :warning + ) + end + end + + def window_valid + if window_start_date && window_end_date && window_start_date > window_end_date + errors.add(:window_end_date, "must be greater than window_start_date") + end end end diff --git a/app/views/accounts/_account.html.erb b/app/views/accounts/_account.html.erb index 07ffd3d5..475f953e 100644 --- a/app/views/accounts/_account.html.erb +++ b/app/views/accounts/_account.html.erb @@ -30,9 +30,13 @@ <% end %>
-

"> - <%= format_money account.balance_money %> -

+ <% if account.syncing? %> +
+ <% else %> +

"> + <%= format_money account.balance_money %> +

+ <% end %> <% unless account.scheduled_for_deletion? %> <%= styled_form_with model: account, data: { turbo_frame: "_top", controller: "auto-submit-form" } do |f| %> diff --git a/app/views/accounts/_account_sidebar_tabs.html.erb b/app/views/accounts/_account_sidebar_tabs.html.erb index 9ca2781c..056e7be1 100644 --- a/app/views/accounts/_account_sidebar_tabs.html.erb +++ b/app/views/accounts/_account_sidebar_tabs.html.erb @@ -1,6 +1,6 @@ -<%# locals: (family:, active_account_group_tab:) %> +<%# locals: (family:, active_tab:, mobile: false) %> -
+
<% if family.missing_data_provider? %>
@@ -21,73 +21,71 @@
<% end %> -
- <%= render TabsComponent.new(active_tab: active_account_group_tab, url_param_key: "account_group_tab", testid: "account-sidebar-tabs") do |tabs| %> - <% tabs.with_nav do |nav| %> - <% nav.with_btn(id: "assets", label: "Assets") %> - <% nav.with_btn(id: "debts", label: "Debts") %> - <% nav.with_btn(id: "all", label: "All") %> - <% end %> + <%= render TabsComponent.new(active_tab: active_tab, session_key: "account_sidebar_tab", testid: "account-sidebar-tabs") do |tabs| %> + <% tabs.with_nav do |nav| %> + <% nav.with_btn(id: "asset", label: "Assets") %> + <% nav.with_btn(id: "liability", label: "Debts") %> + <% nav.with_btn(id: "all", label: "All") %> + <% end %> - <% tabs.with_panel(tab_id: "assets") do %> -
- <%= render LinkComponent.new( - text: "New asset", - variant: "ghost", - href: new_account_path(step: "method_select", classification: "asset"), - icon: "plus", - frame: :modal, - full_width: true, - class: "justify-start" - ) %> - -
- <% family.balance_sheet.account_groups("asset").each do |group| %> - <%= render "accounts/accountable_group", account_group: group %> - <% end %> -
-
- <% end %> - - <% tabs.with_panel(tab_id: "debts") do %> -
- <%= render LinkComponent.new( - text: "New debt", + <% tabs.with_panel(tab_id: "asset") do %> +
+ <%= render LinkComponent.new( + text: "New asset", variant: "ghost", - href: new_account_path(step: "method_select", classification: "liability"), + href: new_account_path(step: "method_select", classification: "asset"), icon: "plus", frame: :modal, - full_width: true, - class: "justify-start" - ) %> - -
- <% family.balance_sheet.account_groups("liability").each do |group| %> - <%= render "accounts/accountable_group", account_group: group %> - <% end %> -
-
- <% end %> - - <% tabs.with_panel(tab_id: "all") do %> -
- <%= render LinkComponent.new( - text: "New account", - variant: "ghost", full_width: true, - href: new_account_path(step: "method_select"), - icon: "plus", - frame: :modal, - class: "justify-start" - ) %> + class: "justify-start" + ) %> -
- <% family.balance_sheet.account_groups.each do |group| %> - <%= render "accounts/accountable_group", account_group: group %> - <% end %> -
+
+ <% family.balance_sheet.account_groups("asset").each do |group| %> + <%= render "accounts/accountable_group", account_group: group, mobile: mobile %> + <% end %>
- <% end %> +
<% end %> -
+ + <% tabs.with_panel(tab_id: "liability") do %> +
+ <%= render LinkComponent.new( + text: "New debt", + variant: "ghost", + href: new_account_path(step: "method_select", classification: "liability"), + icon: "plus", + frame: :modal, + full_width: true, + class: "justify-start" + ) %> + +
+ <% family.balance_sheet.account_groups("liability").each do |group| %> + <%= render "accounts/accountable_group", account_group: group, mobile: mobile %> + <% end %> +
+
+ <% end %> + + <% tabs.with_panel(tab_id: "all") do %> +
+ <%= render LinkComponent.new( + text: "New account", + variant: "ghost", + full_width: true, + href: new_account_path(step: "method_select"), + icon: "plus", + frame: :modal, + class: "justify-start" + ) %> + +
+ <% family.balance_sheet.account_groups.each do |group| %> + <%= render "accounts/accountable_group", account_group: group, mobile: mobile %> + <% end %> +
+
+ <% end %> + <% end %>
diff --git a/app/views/accounts/_accountable_group.html.erb b/app/views/accounts/_accountable_group.html.erb index a1ab2e3a..26633918 100644 --- a/app/views/accounts/_accountable_group.html.erb +++ b/app/views/accounts/_accountable_group.html.erb @@ -1,49 +1,69 @@ -<%# locals: (account_group:) %> +<%# locals: (account_group:, mobile: false, open: nil, **args) %> -<%= render DisclosureComponent.new(title: account_group.name, align: :left, open: account_group.accounts.any? { |account| page_active?(account_path(account)) }) do |disclosure| %> - <% disclosure.with_summary_content do %> -
- <%= tag.p format_money(account_group.total_money), class: "text-sm font-medium text-primary" %> +
"> + <% is_open = open.nil? ? account_group.accounts.any? { |account| page_active?(account_path(account)) } : open %> + <%= render DisclosureComponent.new(title: account_group.name, align: :left, open: is_open) do |disclosure| %> + <% disclosure.with_summary_content do %> +
+ <% if account_group.syncing? %> +
+
+
+
+
+
+ <% else %> + <%= tag.p format_money(account_group.total_money), class: "text-sm font-medium text-primary" %> + <%= turbo_frame_tag "#{account_group.key}_sparkline", src: accountable_sparkline_path(account_group.key), loading: "lazy" do %> +
+
+
+ <% end %> + <% end %> +
+ <% end %> - <%= turbo_frame_tag "#{account_group.key}_sparkline", src: accountable_sparkline_path(account_group.key), loading: "lazy" do %> -
-
-
- <% end %> -
- <% end %> - -
- <% account_group.accounts.each do |account| %> - <%= link_to account_path(account), +
+ <% account_group.accounts.each do |account| %> + <%= link_to account_path(account), class: class_names( "block flex items-center gap-2 px-3 py-2 rounded-lg", page_active?(account_path(account)) ? "bg-container" : "hover:bg-surface-hover" - ), - data: { sidebar_tabs_target: "account", action: "click->sidebar-tabs#select" }, + ), title: account.name do %> - <%= render "accounts/logo", account: account, size: "sm", color: account_group.color %> + <%= render "accounts/logo", account: account, size: "sm", color: account_group.color %> -
- <%= tag.p account.name, class: "text-sm text-primary font-medium mb-0.5 truncate" %> - <%= tag.p account.short_subtype_label, class: "text-sm text-secondary truncate" %> -
+
+ <%= tag.p account.name, class: "text-sm text-primary font-medium mb-0.5 truncate" %> + <%= tag.p account.short_subtype_label, class: "text-sm text-secondary truncate" %> +
-
- <%= tag.p format_money(account.balance_money), class: "text-sm font-medium text-primary whitespace-nowrap" %> + <% if account.syncing? %> +
+
+
+
+
+
+
+
+ <% else %> +
+ <%= tag.p format_money(account.balance_money), class: "text-sm font-medium text-primary whitespace-nowrap" %> - <%= turbo_frame_tag dom_id(account, :sparkline), src: sparkline_account_path(account), loading: "lazy" do %> -
-
+ <%= turbo_frame_tag dom_id(account, :sparkline), src: sparkline_account_path(account), loading: "lazy" do %> +
+
+
+ <% end %>
<% end %> -
+ <% end %> <% end %> - <% end %> -
+
-
- <%= render LinkComponent.new( +
+ <%= render LinkComponent.new( href: new_polymorphic_path(account_group.key, step: "method_select"), text: "New #{account_group.name.downcase.singularize}", icon: "plus", @@ -52,5 +72,6 @@ frame: :modal, class: "justify-start" ) %> -
-<% end %> +
+ <% end %> +
diff --git a/app/views/accounts/_chart_loader.html.erb b/app/views/accounts/_chart_loader.html.erb index f6a9c852..b080329f 100644 --- a/app/views/accounts/_chart_loader.html.erb +++ b/app/views/accounts/_chart_loader.html.erb @@ -1,5 +1,7 @@ -
+
+
-
-

Loading...

+ +
+
diff --git a/app/views/accounts/chart.html.erb b/app/views/accounts/chart.html.erb index 11dcbaac..b181ef20 100644 --- a/app/views/accounts/chart.html.erb +++ b/app/views/accounts/chart.html.erb @@ -2,21 +2,25 @@ <% trend = series.trend %> <%= turbo_frame_tag dom_id(@account, :chart_details) do %> -
- <%= render partial: "shared/trend_change", locals: { trend: trend, comparison_label: @period.comparison_label } %> -
+ <% if @account.syncing? %> + <%= render "accounts/chart_loader" %> + <% else %> +
+ <%= render partial: "shared/trend_change", locals: { trend: trend, comparison_label: @period.comparison_label } %> +
-
- <% if series.any? %> -
+ <% if series.any? %> +
- <% else %> -
-

<%= t(".data_not_available") %>

-
- <% end %> -
+ <% else %> +
+

<%= t(".data_not_available") %>

+
+ <% end %> +
+ <% end %> <% end %> diff --git a/app/views/accounts/index.html.erb b/app/views/accounts/index.html.erb index b4c78332..35e647ea 100644 --- a/app/views/accounts/index.html.erb +++ b/app/views/accounts/index.html.erb @@ -2,17 +2,6 @@

<%= t(".accounts") %>

- <% if Rails.env.development? %> - <%= render ButtonComponent.new( - text: "Sync all", - href: sync_all_accounts_path, - method: :post, - variant: "outline", - disabled: Current.family.syncing?, - icon: "refresh-cw", - ) %> - <% end %> - <%= render LinkComponent.new( text: "New account", href: new_account_path(return_to: accounts_path), diff --git a/app/views/accounts/index/_account_groups.erb b/app/views/accounts/index/_account_groups.erb index 6347339c..51ba4d8b 100644 --- a/app/views/accounts/index/_account_groups.erb +++ b/app/views/accounts/index/_account_groups.erb @@ -6,9 +6,12 @@

<%= Accountable.from_type(group).display_name %>

·

<%= accounts.count %>

-

<%= totals_by_currency(collection: accounts, money_method: :balance_money) %>

+ + <% unless accounts.any?(&:syncing?) %> +

<%= totals_by_currency(collection: accounts, money_method: :balance_money) %>

+ <% end %>
-
+
<% accounts.each do |account| %> <%= render account %> <% end %> diff --git a/app/views/accounts/show/_chart.html.erb b/app/views/accounts/show/_chart.html.erb index 08e67e84..0f63c9f1 100644 --- a/app/views/accounts/show/_chart.html.erb +++ b/app/views/accounts/show/_chart.html.erb @@ -1,17 +1,24 @@ -<%# locals: (account:, title: nil, tooltip: nil, chart_view: nil, **args) %> +<%# locals: (account:, tooltip: nil, chart_view: nil, **args) %> <% period = @period || Period.last_30_days %> <% default_value_title = account.asset? ? t(".balance") : t(".owed") %> -
+
- <%= tag.p title || default_value_title, class: "text-sm font-medium text-secondary" %> - <%= tooltip %> + <%= tag.p account.investment? ? "Total value" : default_value_title, class: "text-sm font-medium text-secondary" %> + + <% if !account.syncing? && account.investment? %> + <%= render "investments/value_tooltip", balance: account.balance_money, holdings: account.balance_money - account.cash_balance_money, cash: account.cash_balance_money %> + <% end %>
- <%= tag.p format_money(account.balance_money), class: "text-primary text-3xl font-medium truncate" %> + <% if account.syncing? %> +
+ <% else %> + <%= tag.p format_money(account.balance_money), class: "text-primary text-3xl font-medium truncate" %> + <% end %>
<%= form_with url: request.path, method: :get, data: { controller: "auto-submit-form" } do |form| %> diff --git a/app/views/accounts/show/_template.html.erb b/app/views/accounts/show/_template.html.erb index cfac9402..efc7ea5a 100644 --- a/app/views/accounts/show/_template.html.erb +++ b/app/views/accounts/show/_template.html.erb @@ -1,8 +1,8 @@ -<%# locals: (account:, header: nil, chart: nil, tabs: nil) %> +<%# locals: (account:, header: nil, chart: nil, chart_view: nil, tabs: nil) %> <%= turbo_stream_from account %> -<%= turbo_frame_tag dom_id(account) do %> +<%= turbo_frame_tag dom_id(account, :container) do %> <%= tag.div class: "space-y-4 pb-32" do %> <% if header.present? %> <%= header %> @@ -13,7 +13,7 @@ <% if chart.present? %> <%= chart %> <% else %> - <%= render "accounts/show/chart", account: account %> + <%= render "accounts/show/chart", account: account, chart_view: chart_view %> <% end %>
diff --git a/app/views/category/dropdowns/show.html.erb b/app/views/category/dropdowns/show.html.erb index 65b5c9be..3b25083b 100644 --- a/app/views/category/dropdowns/show.html.erb +++ b/app/views/category/dropdowns/show.html.erb @@ -2,13 +2,13 @@
- " - autocomplete="nope" - type="search" - class="bg-container placeholder:text-sm placeholder:text-secondary font-normal h-10 relative pl-10 w-full border-none rounded-lg focus:outline-hidden focus:ring-0" - data-list-filter-target="input" - data-action="list-filter#filter" /> + " + autocomplete="nope" + type="search" + class="bg-container placeholder:text-sm placeholder:text-secondary font-normal h-10 relative pl-10 w-full border-none rounded-lg focus:outline-hidden focus:ring-0" + data-list-filter-target="input" + data-action="list-filter#filter"> <%= icon("search", class: "absolute inset-0 ml-2 transform top-1/2 -translate-y-1/2") %>
diff --git a/app/views/holdings/_cash.html.erb b/app/views/holdings/_cash.html.erb index a279d9ab..702f40d0 100644 --- a/app/views/holdings/_cash.html.erb +++ b/app/views/holdings/_cash.html.erb @@ -19,8 +19,13 @@
<% cash_weight = account.balance.zero? ? 0 : account.cash_balance / account.balance * 100 %> - <%= render "shared/progress_circle", progress: cash_weight %> - <%= tag.p number_to_percentage(cash_weight, precision: 1) %> + + <% if account.syncing? %> +
+ <% else %> + <%= render "shared/progress_circle", progress: cash_weight %> + <%= tag.p number_to_percentage(cash_weight, precision: 1) %> + <% end %>
@@ -28,7 +33,13 @@
- <%= tag.p format_money account.cash_balance_money %> + <% if account.syncing? %> +
+
+
+ <% else %> + <%= tag.p format_money account.cash_balance_money %> + <% end %>
diff --git a/app/views/holdings/_holding.html.erb b/app/views/holdings/_holding.html.erb index 5fe0e4c9..c8a2ac59 100644 --- a/app/views/holdings/_holding.html.erb +++ b/app/views/holdings/_holding.html.erb @@ -17,7 +17,9 @@
- <% if holding.weight %> + <% if holding.account.syncing? %> +
+ <% elsif holding.weight %> <%= render "shared/progress_circle", progress: holding.weight %> <%= tag.p number_to_percentage(holding.weight, precision: 1) %> <% else %> @@ -26,21 +28,39 @@
- <%= tag.p format_money holding.avg_cost %> - <%= tag.p t(".per_share"), class: "font-normal text-secondary" %> -
- -
- <% if holding.amount_money %> - <%= tag.p format_money holding.amount_money %> + <% if holding.account.syncing? %> +
+
+
<% else %> - <%= tag.p "--", class: "text-secondary" %> + <%= tag.p format_money holding.avg_cost %> + <%= tag.p t(".per_share"), class: "font-normal text-secondary" %> <% end %> - <%= tag.p t(".shares", qty: number_with_precision(holding.qty, precision: 1)), class: "font-normal text-secondary" %>
- <% if holding.trend %> + <% if holding.account.syncing? %> +
+
+
+
+ <% else %> + <% if holding.amount_money %> + <%= tag.p format_money holding.amount_money %> + <% else %> + <%= tag.p "--", class: "text-secondary" %> + <% end %> + <%= tag.p t(".shares", qty: number_with_precision(holding.qty, precision: 1)), class: "font-normal text-secondary" %> + <% end %> +
+ +
+ <% if holding.account.syncing? %> +
+
+
+
+ <% elsif holding.trend %> <%= tag.p format_money(holding.trend.value), style: "color: #{holding.trend.color};" %> <%= tag.p "(#{number_to_percentage(holding.trend.percent, precision: 1)})", style: "color: #{holding.trend.color};" %> <% else %> diff --git a/app/views/investments/show.html.erb b/app/views/investments/show.html.erb index 7bd7da3b..84be8629 100644 --- a/app/views/investments/show.html.erb +++ b/app/views/investments/show.html.erb @@ -1,25 +1,7 @@ -<%= turbo_stream_from @account %> - -<%= turbo_frame_tag dom_id(@account) do %> - <%= tag.div class: "space-y-4" do %> - <%= render "accounts/show/header", account: @account %> - - <%= render "accounts/show/chart", - account: @account, - title: t(".chart_title"), - chart_view: @chart_view, - tooltip: render( - "investments/value_tooltip", - balance: @account.balance_money, - holdings: @account.balance_money - @account.cash_balance_money, - cash: @account.cash_balance_money - ) %> - -
- <%= render "accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "holdings", contents: render("investments/holdings_tab", account: @account) }, - ] %> -
- <% end %> -<% end %> +<%= render "accounts/show/template", + account: @account, + chart_view: @chart_view, + tabs: render("accounts/show/tabs", account: @account, tabs: [ + { key: "activity", contents: render("accounts/show/activity", account: @account) }, + { key: "holdings", contents: render("investments/holdings_tab", account: @account) }, + ]) %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 53e7f666..e8fc202e 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -26,7 +26,8 @@ <%= render( "accounts/account_sidebar_tabs", family: Current.family, - active_account_group_tab: params[:account_group_tab] || "assets" + active_tab: @account_group_tab, + mobile: true ) %>
@@ -80,8 +81,8 @@ <%= yield :sidebar %> <% else %>
-
- <%= render "accounts/account_sidebar_tabs", family: Current.family, active_account_group_tab: params[:account_group_tab] || "assets" %> +
+ <%= render "accounts/account_sidebar_tabs", family: Current.family, active_tab: @account_group_tab %>
<% if Current.family.trialing? && !self_hosted? %> diff --git a/app/views/layouts/shared/_htmldoc.html.erb b/app/views/layouts/shared/_htmldoc.html.erb index fadcd396..30887f7b 100644 --- a/app/views/layouts/shared/_htmldoc.html.erb +++ b/app/views/layouts/shared/_htmldoc.html.erb @@ -23,10 +23,6 @@ <%= render_flash_notifications %>
- - <% if Current.family&.syncing? %> - <%= render "shared/notifications/loading", id: "syncing-notice", message: "Syncing accounts data..." %> - <% end %>
diff --git a/app/views/pages/dashboard.html.erb b/app/views/pages/dashboard.html.erb index 569caaa2..1f2f347e 100644 --- a/app/views/pages/dashboard.html.erb +++ b/app/views/pages/dashboard.html.erb @@ -25,11 +25,14 @@
<% if Current.family.accounts.any? %> -
- <%= render partial: "pages/dashboard/net_worth_chart", locals: { series: @balance_sheet.net_worth_series(period: @period), period: @period } %> +
+ <%= render partial: "pages/dashboard/net_worth_chart", locals: { + balance_sheet: @balance_sheet, + period: @period + } %>
<% else %> -
+
<%= render "pages/dashboard/no_accounts_graph_placeholder" %>
<% end %> diff --git a/app/views/pages/dashboard/_balance_sheet.html.erb b/app/views/pages/dashboard/_balance_sheet.html.erb index 302ae103..6b4ea525 100644 --- a/app/views/pages/dashboard/_balance_sheet.html.erb +++ b/app/views/pages/dashboard/_balance_sheet.html.erb @@ -1,6 +1,6 @@ -<%# locals: (balance_sheet:) %> +<%# locals: (balance_sheet:, **args) %> -
+
<% balance_sheet.classification_groups.each do |classification_group| %>

@@ -11,26 +11,38 @@ <% if classification_group.account_groups.any? %> · - <%= classification_group.total_money.format(precision: 0) %> + <% if classification_group.syncing? %> +
+
+
+ <% else %> + <%= classification_group.total_money.format(precision: 0) %> + <% end %> <% end %>

<% if classification_group.account_groups.any? %> +
<% classification_group.account_groups.each do |account_group| %>
<% end %>
-
- <% classification_group.account_groups.each do |account_group| %> -
-
-

<%= account_group.name %>

-

<%= number_to_percentage(account_group.weight, precision: 0) %>

-
- <% end %> -
+ + <% if classification_group.syncing? %> +

Calculating latest balance data...

+ <% else %> +
+ <% classification_group.account_groups.each do |account_group| %> +
+
+

<%= account_group.name %>

+

<%= number_to_percentage(account_group.weight, precision: 0) %>

+
+ <% end %> +
+ <% end %>
@@ -56,15 +68,27 @@

<%= account_group.name %>

-
-
- <%= render "pages/dashboard/group_weight", weight: account_group.weight, color: account_group.color %> -
+ <% if account_group.syncing? %> +
+
+
+
-
-

<%= format_money(account_group.total_money) %>

+
+
+
-
+ <% else %> +
+
+ <%= render "pages/dashboard/group_weight", weight: account_group.weight, color: account_group.color %> +
+ +
+

<%= format_money(account_group.total_money) %>

+
+
+ <% end %>
@@ -76,15 +100,27 @@ <%= link_to account.name, account_path(account) %>
-
-
- <%= render "pages/dashboard/group_weight", weight: account.weight, color: account_group.color %> -
+ <% if account.syncing? %> +
+
+
+
-
-

<%= format_money(account.balance_money) %>

+
+
+
-
+ <% else %> +
+
+ <%= render "pages/dashboard/group_weight", weight: account.weight, color: account_group.color %> +
+ +
+

<%= format_money(account.balance_money) %>

+
+
+ <% end %>
<% if idx < account_group.accounts.size - 1 %> diff --git a/app/views/pages/dashboard/_net_worth_chart.html.erb b/app/views/pages/dashboard/_net_worth_chart.html.erb index a8870cac..a6b65852 100644 --- a/app/views/pages/dashboard/_net_worth_chart.html.erb +++ b/app/views/pages/dashboard/_net_worth_chart.html.erb @@ -1,37 +1,55 @@ -<%# locals: (series:, period:) %> +<%# locals: (balance_sheet:, period:, **args) %> -
-
+
+ <% series = balance_sheet.net_worth_series(period: period) %> +
-

<%= t(".title") %>

-

- <%= series.current.format %> -

- <% if series.trend.nil? %> -

<%= t(".data_not_available") %>

- <% else %> - <%= render partial: "shared/trend_change", locals: { trend: series.trend, comparison_label: period.comparison_label } %> - <% end %> -
-
+
+

<%= t(".title") %>

- <%= form_with url: root_path, method: :get, data: { controller: "auto-submit-form" } do |form| %> - <%= form.select :period, + <% if balance_sheet.syncing? %> +
+
+
+
+ <% else %> +

+ <%= series.current.format %> +

+ + <% if series.trend.nil? %> +

<%= t(".data_not_available") %>

+ <% else %> + <%= render partial: "shared/trend_change", locals: { trend: series.trend, comparison_label: period.comparison_label } %> + <% end %> + <% end %> +
+
+ + <%= form_with url: root_path, method: :get, data: { controller: "auto-submit-form" } do |form| %> + <%= form.select :period, Period.as_options, { selected: period.key }, data: { "auto-submit-form-target": "auto" }, class: "bg-container border border-secondary font-medium rounded-lg px-3 py-2 text-sm pr-7 cursor-pointer text-primary focus:outline-hidden focus:ring-0" %> - <% end %> -
+ <% end %> +
-<% if series.any? %> -
+
+
+
+ <% else %> + <% if series.any? %> +
-<% else %> -
-

<%= t(".data_not_available") %>

-
-<% end %> + <% else %> +
+

<%= t(".data_not_available") %>

+
+ <% end %> + <% end %> +
diff --git a/app/views/rules/_rule.html.erb b/app/views/rules/_rule.html.erb index 9307eb80..1dbf9641 100644 --- a/app/views/rules/_rule.html.erb +++ b/app/views/rules/_rule.html.erb @@ -1,5 +1,5 @@ <%# locals: (rule:) %> -
+
">
<% if rule.name.present? %>

<%= rule.name %>

@@ -49,7 +49,7 @@ <% if rule.effective_date.nil? %> All past and future <%= rule.resource_type.pluralize %> <% else %> - <%= rule.resource_type.pluralize %> on or after <%= rule.effective_date.strftime('%b %-d, %Y') %> + <%= rule.resource_type.pluralize %> on or after <%= rule.effective_date.strftime("%b %-d, %Y") %> <% end %>

diff --git a/app/views/rules/confirm.html.erb b/app/views/rules/confirm.html.erb index 28be9496..03311791 100644 --- a/app/views/rules/confirm.html.erb +++ b/app/views/rules/confirm.html.erb @@ -1,5 +1,5 @@ <%= render DialogComponent.new(reload_on_close: true) do |dialog| %> - <% + <% title = if @rule.name.present? "Confirm changes to \"#{@rule.name}\"" else @@ -7,7 +7,7 @@ end %> <% dialog.with_header(title: title) %> - + <% dialog.with_body do %>

You are about to apply this rule to diff --git a/app/views/rules/edit.html.erb b/app/views/rules/edit.html.erb index f73edc90..91dea816 100644 --- a/app/views/rules/edit.html.erb +++ b/app/views/rules/edit.html.erb @@ -1,7 +1,7 @@ <%= link_to "Back to rules", rules_path %> <%= render DialogComponent.new do |dialog| %> - <% + <% title = if @rule.name.present? "Edit #{@rule.resource_type} rule \"#{@rule.name}\"" else diff --git a/app/views/rules/index.html.erb b/app/views/rules/index.html.erb index d549e0f8..c6ca513f 100644 --- a/app/views/rules/index.html.erb +++ b/app/views/rules/index.html.erb @@ -60,7 +60,7 @@

<% @rules.each_with_index do |rule, idx| %> - <%= render "rule", rule: rule%> + <%= render "rule", rule: rule %> <% unless idx == @rules.size - 1 %>
<% end %> diff --git a/app/views/settings/_settings_nav.html.erb b/app/views/settings/_settings_nav.html.erb index 6a80bb87..5aa2c258 100644 --- a/app/views/settings/_settings_nav.html.erb +++ b/app/views/settings/_settings_nav.html.erb @@ -1,31 +1,31 @@ <% nav_sections = [ { - header: t('.general_section_title'), + header: t(".general_section_title"), items: [ - { label: t('.profile_label'), path: settings_profile_path, icon: 'circle-user' }, - { label: t('.preferences_label'), path: settings_preferences_path, icon: 'bolt' }, - { label: t('.security_label'), path: settings_security_path, icon: 'shield-check' }, - { label: t('.self_hosting_label'), path: settings_hosting_path, icon: 'database', if: self_hosted? }, - { label: t('.billing_label'), path: settings_billing_path, icon: 'circle-dollar-sign', if: !self_hosted? }, - { label: t('.accounts_label'), path: accounts_path, icon: 'layers' }, - { label: t('.imports_label'), path: imports_path, icon: 'download' } + { label: t(".profile_label"), path: settings_profile_path, icon: "circle-user" }, + { label: t(".preferences_label"), path: settings_preferences_path, icon: "bolt" }, + { label: t(".security_label"), path: settings_security_path, icon: "shield-check" }, + { label: t(".self_hosting_label"), path: settings_hosting_path, icon: "database", if: self_hosted? }, + { label: t(".billing_label"), path: settings_billing_path, icon: "circle-dollar-sign", if: !self_hosted? }, + { label: t(".accounts_label"), path: accounts_path, icon: "layers" }, + { label: t(".imports_label"), path: imports_path, icon: "download" } ] }, { - header: t('.transactions_section_title'), + header: t(".transactions_section_title"), items: [ - { label: t('.tags_label'), path: tags_path, icon: 'tags' }, - { label: t('.categories_label'), path: categories_path, icon: 'shapes' }, - { label: t('.rules_label'), path: rules_path, icon: 'git-branch' }, - { label: t('.merchants_label'), path: family_merchants_path, icon: 'store' } + { label: t(".tags_label"), path: tags_path, icon: "tags" }, + { label: t(".categories_label"), path: categories_path, icon: "shapes" }, + { label: t(".rules_label"), path: rules_path, icon: "git-branch" }, + { label: t(".merchants_label"), path: family_merchants_path, icon: "store" } ] }, { - header: t('.other_section_title'), + header: t(".other_section_title"), items: [ - { label: t('.whats_new_label'), path: changelog_path, icon: 'box' }, - { label: t('.feedback_label'), path: feedback_path, icon: 'megaphone' } + { label: t(".whats_new_label"), path: changelog_path, icon: "box" }, + { label: t(".feedback_label"), path: feedback_path, icon: "megaphone" } ] } ] diff --git a/app/views/shared/notifications/_loading.html.erb b/app/views/shared/notifications/_loading.html.erb deleted file mode 100644 index 18eabd89..00000000 --- a/app/views/shared/notifications/_loading.html.erb +++ /dev/null @@ -1,9 +0,0 @@ -<%# locals: (message:, id: nil) %> - -<%= tag.div id: id, class: "flex gap-3 rounded-lg bg-container p-4 group w-full md:max-w-80 shadow-border-xs" do %> -
- <%= icon "loader", class: "animate-pulse" %> -
- - <%= tag.p message, class: "text-primary text-sm font-medium" %> -<% end %> diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index 9781672f..52bb9c3d 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -4,9 +4,6 @@
<%= render MenuComponent.new do |menu| %> - <% if Rails.env.development? %> - <% menu.with_item(variant: "button", text: "Dev only: Sync all", href: sync_all_accounts_path, method: :post, icon: "refresh-cw") %> - <% end %> <% menu.with_item(variant: "link", text: "New rule", href: new_rule_path(resource_type: "transaction"), icon: "plus", data: { turbo_frame: :modal }) %> <% menu.with_item(variant: "link", text: "Edit rules", href: rules_path, icon: "git-branch", data: { turbo_frame: :_top }) %> <% menu.with_item(variant: "link", text: "Edit categories", href: categories_path, icon: "shapes", data: { turbo_frame: :_top }) %> diff --git a/config/initializers/mini_profiler.rb b/config/initializers/mini_profiler.rb index d304a50e..6a79f19e 100644 --- a/config/initializers/mini_profiler.rb +++ b/config/initializers/mini_profiler.rb @@ -1,3 +1,4 @@ Rails.application.configure do - Rack::MiniProfiler.config.skip_paths = [ "/design-system" ] + Rack::MiniProfiler.config.skip_paths = [ "/design-system", "/assets", "/cable", "/manifest", "/favicon.ico", "/hotwire-livereload", "/logo-pwa.png" ] + Rack::MiniProfiler.config.max_traces_to_show = 50 end diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 1209a4fa..9040f864 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -7,3 +7,8 @@ Sidekiq::Web.use(Rack::Auth::Basic) do |username, password| ActiveSupport::SecurityUtils.secure_compare(::Digest::SHA256.hexdigest(username), configured_username) && ActiveSupport::SecurityUtils.secure_compare(::Digest::SHA256.hexdigest(password), configured_password) end + +Sidekiq::Cron.configure do |config| + # 10 min "catch-up" window in case worker process is re-deploying when cron tick occurs + config.reschedule_grace_period = 600 +end diff --git a/config/routes.rb b/config/routes.rb index 8384d116..ec9e2cce 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,5 @@ require "sidekiq/web" +require "sidekiq/cron/web" Rails.application.routes.draw do # MFA routes @@ -25,6 +26,8 @@ Rails.application.routes.draw do get "changelog", to: "pages#changelog" get "feedback", to: "pages#feedback" + resource :current_session, only: %i[update] + resource :registration, only: %i[new create] resources :sessions, only: %i[new create destroy] resource :password_reset, only: %i[new create edit update] @@ -104,10 +107,6 @@ Rails.application.routes.draw do end resources :accounts, only: %i[index new], shallow: true do - collection do - post :sync_all - end - member do post :sync get :chart diff --git a/config/schedule.yml b/config/schedule.yml new file mode 100644 index 00000000..561e2327 --- /dev/null +++ b/config/schedule.yml @@ -0,0 +1,5 @@ +sync_market_data: + cron: "0 17 * * 1-5" # 5:00 PM EST (1 hour after market close) + class: "SyncMarketDataJob" + queue: "scheduled" + description: "Syncs market data daily at 5:00 PM EST (1 hour after market close)" diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 20343e8c..4fce6f00 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -1,6 +1,7 @@ concurrency: <%= ENV.fetch("RAILS_MAX_THREADS") { 3 } %> queues: - - [high_priority, 6] + - [scheduled, 10] # For cron-like jobs (e.g. "daily market data sync") + - [high_priority, 4] - [medium_priority, 2] - [low_priority, 1] - [default, 1] diff --git a/db/migrate/20250512171654_update_sync_timestamps.rb b/db/migrate/20250512171654_update_sync_timestamps.rb new file mode 100644 index 00000000..ac0830b6 --- /dev/null +++ b/db/migrate/20250512171654_update_sync_timestamps.rb @@ -0,0 +1,65 @@ +class UpdateSyncTimestamps < ActiveRecord::Migration[7.2] + def change + # Timestamps, managed by aasm + add_column :syncs, :pending_at, :datetime + add_column :syncs, :syncing_at, :datetime + add_column :syncs, :completed_at, :datetime + add_column :syncs, :failed_at, :datetime + + add_column :syncs, :window_start_date, :date + add_column :syncs, :window_end_date, :date + + reversible do |dir| + dir.up do + execute <<-SQL + UPDATE syncs + SET + completed_at = CASE + WHEN status = 'completed' THEN last_ran_at + ELSE NULL + END, + failed_at = CASE + WHEN status = 'failed' THEN last_ran_at + ELSE NULL + END + SQL + + execute <<-SQL + UPDATE syncs + SET window_start_date = start_date + SQL + + # Due to some recent bugs, some self hosters have syncs that are stuck. + # This manually fails those syncs so they stop seeing syncing UI notices. + if Rails.application.config.app_mode.self_hosted? + puts "Self hosted: Fail syncs older than 2 hours" + execute <<-SQL + UPDATE syncs + SET status = 'failed' + WHERE ( + status = 'syncing' AND + created_at < NOW() - INTERVAL '2 hours' + ) + SQL + end + end + + dir.down do + execute <<-SQL + UPDATE syncs + SET + last_ran_at = COALESCE(completed_at, failed_at) + SQL + + execute <<-SQL + UPDATE syncs + SET start_date = window_start_date + SQL + end + end + + remove_column :syncs, :start_date, :date + remove_column :syncs, :last_ran_at, :datetime + remove_column :syncs, :error_backtrace, :text, array: true + end +end diff --git a/db/migrate/20250514214242_add_metadata_to_session.rb b/db/migrate/20250514214242_add_metadata_to_session.rb new file mode 100644 index 00000000..849cdccf --- /dev/null +++ b/db/migrate/20250514214242_add_metadata_to_session.rb @@ -0,0 +1,5 @@ +class AddMetadataToSession < ActiveRecord::Migration[7.2] + def change + add_column :sessions, :data, :jsonb, default: {} + end +end diff --git a/db/schema.rb b/db/schema.rb index 9cfb546c..7f25cb20 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_05_13_122703) do +ActiveRecord::Schema[7.2].define(version: 2025_05_14_214242) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -537,6 +537,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_13_122703) do t.uuid "active_impersonator_session_id" t.datetime "subscribed_at" t.jsonb "prev_transaction_page_params", default: {} + t.jsonb "data", default: {} t.index ["active_impersonator_session_id"], name: "index_sessions_on_active_impersonator_session_id" t.index ["user_id"], name: "index_sessions_on_user_id" end @@ -587,15 +588,18 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_13_122703) do create_table "syncs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.string "syncable_type", null: false t.uuid "syncable_id", null: false - t.datetime "last_ran_at" - t.date "start_date" t.string "status", default: "pending" t.string "error" t.jsonb "data" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.text "error_backtrace", array: true t.uuid "parent_id" + t.datetime "pending_at" + t.datetime "syncing_at" + t.datetime "completed_at" + t.datetime "failed_at" + t.date "window_start_date" + t.date "window_end_date" t.index ["parent_id"], name: "index_syncs_on_parent_id" t.index ["syncable_type", "syncable_id"], name: "index_syncs_on_syncable" end diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb index d85a5ffa..a3d827e8 100644 --- a/test/controllers/accounts_controller_test.rb +++ b/test/controllers/accounts_controller_test.rb @@ -15,9 +15,4 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest post sync_account_path(@account) assert_redirected_to account_path(@account) end - - test "can sync all accounts" do - post sync_all_accounts_path - assert_redirected_to accounts_path - end end diff --git a/test/controllers/current_sessions_controller_test.rb b/test/controllers/current_sessions_controller_test.rb new file mode 100644 index 00000000..9d498fb3 --- /dev/null +++ b/test/controllers/current_sessions_controller_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class CurrentSessionsControllerTest < ActionDispatch::IntegrationTest + setup do + @user = users(:family_admin) + sign_in @user + end + + test "can update the preferred tab for any namespace" do + put current_session_url, params: { current_session: { tab_key: "accounts_sidebar_tab", tab_value: "asset" } } + assert_response :success + session = Session.order(updated_at: :desc).first + assert_equal "asset", session.get_preferred_tab("accounts_sidebar_tab") + end +end diff --git a/test/controllers/plaid_items_controller_test.rb b/test/controllers/plaid_items_controller_test.rb index 7aede85c..4cf8e10a 100644 --- a/test/controllers/plaid_items_controller_test.rb +++ b/test/controllers/plaid_items_controller_test.rb @@ -8,7 +8,7 @@ class PlaidItemsControllerTest < ActionDispatch::IntegrationTest test "create" do @plaid_provider = mock - PlaidItem.expects(:plaid_provider_for_region).with("us").returns(@plaid_provider) + Provider::Registry.expects(:get_provider).with(:plaid_us).returns(@plaid_provider) public_token = "public-sandbox-1234" diff --git a/test/fixtures/syncs.yml b/test/fixtures/syncs.yml index 1b010568..5d954280 100644 --- a/test/fixtures/syncs.yml +++ b/test/fixtures/syncs.yml @@ -1,17 +1,17 @@ account: syncable_type: Account syncable: depository - last_ran_at: <%= Time.now %> status: completed + completed_at: <%= Time.now %> plaid_item: syncable_type: PlaidItem syncable: one - last_ran_at: <%= Time.now %> status: completed + completed_at: <%= Time.now %> family: syncable_type: Family syncable: dylan_family - last_ran_at: <%= Time.now %> status: completed + completed_at: <%= Time.now %> diff --git a/test/interfaces/syncable_interface_test.rb b/test/interfaces/syncable_interface_test.rb index 6613dbcc..a9f1fa7d 100644 --- a/test/interfaces/syncable_interface_test.rb +++ b/test/interfaces/syncable_interface_test.rb @@ -7,18 +7,14 @@ module SyncableInterfaceTest test "can sync later" do assert_difference "@syncable.syncs.count", 1 do assert_enqueued_with job: SyncJob do - @syncable.sync_later + @syncable.sync_later(window_start_date: 2.days.ago.to_date) end end end - test "can sync" do - assert_difference "@syncable.syncs.count", 1 do - @syncable.sync(start_date: 2.days.ago.to_date) - end - end - - test "implements sync_data" do - assert_respond_to @syncable, :sync_data + test "can perform sync" do + mock_sync = mock + @syncable.class.any_instance.expects(:perform_sync).with(mock_sync).once + @syncable.perform_sync(mock_sync) end end diff --git a/test/jobs/sync_job_test.rb b/test/jobs/sync_job_test.rb index b8d34400..b392b3a3 100644 --- a/test/jobs/sync_job_test.rb +++ b/test/jobs/sync_job_test.rb @@ -4,7 +4,7 @@ class SyncJobTest < ActiveJob::TestCase test "sync is performed" do syncable = accounts(:depository) - sync = syncable.syncs.create!(start_date: 2.days.ago.to_date) + sync = syncable.syncs.create!(window_start_date: 2.days.ago.to_date) sync.expects(:perform).once diff --git a/test/models/account/entry_test.rb b/test/models/account/entry_test.rb index 5a07ad6e..edd55e68 100644 --- a/test/models/account/entry_test.rb +++ b/test/models/account/entry_test.rb @@ -30,7 +30,7 @@ class EntryTest < ActiveSupport::TestCase prior_date = @entry.date - 1 @entry.update! date: prior_date - @entry.account.expects(:sync_later).with(start_date: prior_date) + @entry.account.expects(:sync_later).with(window_start_date: prior_date) @entry.sync_account_later end @@ -38,14 +38,14 @@ class EntryTest < ActiveSupport::TestCase prior_date = @entry.date @entry.update! date: @entry.date + 1 - @entry.account.expects(:sync_later).with(start_date: prior_date) + @entry.account.expects(:sync_later).with(window_start_date: prior_date) @entry.sync_account_later end test "triggers sync with correct start date when transaction deleted" do @entry.destroy! - @entry.account.expects(:sync_later).with(start_date: nil) + @entry.account.expects(:sync_later).with(window_start_date: nil) @entry.sync_account_later end diff --git a/test/models/balance/forward_calculator_test.rb b/test/models/balance/forward_calculator_test.rb index 24aecdb5..05215c25 100644 --- a/test/models/balance/forward_calculator_test.rb +++ b/test/models/balance/forward_calculator_test.rb @@ -15,19 +15,19 @@ class Balance::ForwardCalculatorTest < ActiveSupport::TestCase test "balance generation respects user timezone and last generated date is current user date" do # Simulate user in EST timezone - Time.zone = "America/New_York" + Time.use_zone("America/New_York") do + # Set current time to 1am UTC on Jan 5, 2025 + # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for) + travel_to Time.utc(2025, 01, 05, 1, 0, 0) - # Set current time to 1am UTC on Jan 5, 2025 - # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for) - travel_to Time.utc(2025, 01, 05, 1, 0, 0) + # Create a valuation for Jan 3, 2025 + create_valuation(account: @account, date: "2025-01-03", amount: 17000) - # Create a valuation for Jan 3, 2025 - create_valuation(account: @account, date: "2025-01-03", amount: 17000) + expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 17000 ], [ "2025-01-04", 17000 ] ] + calculated = Balance::ForwardCalculator.new(@account).calculate - expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 17000 ], [ "2025-01-04", 17000 ] ] - calculated = Balance::ForwardCalculator.new(@account).calculate - - assert_equal expected, calculated.map { |b| [ b.date.to_s, b.balance ] } + assert_equal expected, calculated.map { |b| [ b.date.to_s, b.balance ] } + end end # When syncing forwards, we don't care about the account balance. We generate everything based on entries, starting from 0. diff --git a/test/models/balance/reverse_calculator_test.rb b/test/models/balance/reverse_calculator_test.rb index 38ede057..6d73aea8 100644 --- a/test/models/balance/reverse_calculator_test.rb +++ b/test/models/balance/reverse_calculator_test.rb @@ -25,18 +25,18 @@ class Balance::ReverseCalculatorTest < ActiveSupport::TestCase test "balance generation respects user timezone and last generated date is current user date" do # Simulate user in EST timezone - Time.zone = "America/New_York" + Time.use_zone("America/New_York") do + # Set current time to 1am UTC on Jan 5, 2025 + # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for) + travel_to Time.utc(2025, 01, 05, 1, 0, 0) - # Set current time to 1am UTC on Jan 5, 2025 - # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for) - travel_to Time.utc(2025, 01, 05, 1, 0, 0) + create_valuation(account: @account, date: "2025-01-03", amount: 17000) - create_valuation(account: @account, date: "2025-01-03", amount: 17000) + expected = [ [ "2025-01-02", 17000 ], [ "2025-01-03", 17000 ], [ "2025-01-04", @account.balance ] ] + calculated = Balance::ReverseCalculator.new(@account).calculate - expected = [ [ "2025-01-02", 17000 ], [ "2025-01-03", 17000 ], [ "2025-01-04", @account.balance ] ] - calculated = Balance::ReverseCalculator.new(@account).calculate - - assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.balance ] } + assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.balance ] } + end end test "valuations sync" do diff --git a/test/models/family/syncer_test.rb b/test/models/family/syncer_test.rb new file mode 100644 index 00000000..7fe01a7e --- /dev/null +++ b/test/models/family/syncer_test.rb @@ -0,0 +1,30 @@ +require "test_helper" + +class Family::SyncerTest < ActiveSupport::TestCase + setup do + @family = families(:dylan_family) + end + + test "syncs plaid items and manual accounts" do + family_sync = syncs(:family) + + manual_accounts_count = @family.accounts.manual.count + items_count = @family.plaid_items.count + + syncer = Family::Syncer.new(@family) + + Account.any_instance + .expects(:sync_later) + .with(parent_sync: family_sync, window_start_date: nil, window_end_date: nil) + .times(manual_accounts_count) + + PlaidItem.any_instance + .expects(:sync_later) + .with(parent_sync: family_sync, window_start_date: nil, window_end_date: nil) + .times(items_count) + + syncer.perform_sync(family_sync) + + assert_equal "completed", family_sync.reload.status + end +end diff --git a/test/models/family_test.rb b/test/models/family_test.rb index 24876a77..0229aa6e 100644 --- a/test/models/family_test.rb +++ b/test/models/family_test.rb @@ -1,25 +1,9 @@ require "test_helper" -require "csv" class FamilyTest < ActiveSupport::TestCase - include EntriesTestHelper include SyncableInterfaceTest def setup - @family = families(:empty) @syncable = families(:dylan_family) end - - test "syncs plaid items and manual accounts" do - family_sync = syncs(:family) - - manual_accounts_count = @syncable.accounts.manual.count - items_count = @syncable.plaid_items.count - - Account.any_instance.expects(:sync_later) - .with(start_date: nil, parent_sync: family_sync) - .times(manual_accounts_count) - - @syncable.sync_data(family_sync, start_date: family_sync.start_date) - end end diff --git a/test/models/holding/forward_calculator_test.rb b/test/models/holding/forward_calculator_test.rb index 7a8aaa53..89e9b28c 100644 --- a/test/models/holding/forward_calculator_test.rb +++ b/test/models/holding/forward_calculator_test.rb @@ -20,22 +20,22 @@ class Holding::ForwardCalculatorTest < ActiveSupport::TestCase test "holding generation respects user timezone and last generated date is current user date" do # Simulate user in EST timezone - Time.zone = "America/New_York" + Time.use_zone("America/New_York") do + # Set current time to 1am UTC on Jan 5, 2025 + # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for) + travel_to Time.utc(2025, 01, 05, 1, 0, 0) - # Set current time to 1am UTC on Jan 5, 2025 - # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for) - travel_to Time.utc(2025, 01, 05, 1, 0, 0) + voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF") + Security::Price.create!(security: voo, date: "2025-01-02", price: 500) + Security::Price.create!(security: voo, date: "2025-01-03", price: 500) + Security::Price.create!(security: voo, date: "2025-01-04", price: 500) + create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account) - voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF") - Security::Price.create!(security: voo, date: "2025-01-02", price: 500) - Security::Price.create!(security: voo, date: "2025-01-03", price: 500) - Security::Price.create!(security: voo, date: "2025-01-04", price: 500) - create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account) + expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ] + calculated = Holding::ForwardCalculator.new(@account).calculate - expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ] - calculated = Holding::ForwardCalculator.new(@account).calculate - - assert_equal expected, calculated.map { |b| [ b.date.to_s, b.amount ] } + assert_equal expected, calculated.map { |b| [ b.date.to_s, b.amount ] } + end end test "forward portfolio calculation" do diff --git a/test/models/holding/portfolio_cache_test.rb b/test/models/holding/portfolio_cache_test.rb index 2518e2b7..4677fc41 100644 --- a/test/models/holding/portfolio_cache_test.rb +++ b/test/models/holding/portfolio_cache_test.rb @@ -28,37 +28,18 @@ class Holding::PortfolioCacheTest < ActiveSupport::TestCase price: db_price ) - expect_provider_prices([], start_date: @account.start_date) - cache = Holding::PortfolioCache.new(@account) assert_equal db_price, cache.get_price(@security.id, Date.current).price end - test "if no price in DB, try fetching from provider" do - Security::Price.delete_all - - provider_price = Security::Price.new( - security: @security, - date: Date.current, - price: 220, - currency: "USD" - ) - - expect_provider_prices([ provider_price ], start_date: @account.start_date) - - cache = Holding::PortfolioCache.new(@account) - assert_equal provider_price.price, cache.get_price(@security.id, Date.current).price - end - - test "if no price from db or provider, try getting the price from trades" do + test "if no price from db, try getting the price from trades" do Security::Price.destroy_all - expect_provider_prices([], start_date: @account.start_date) cache = Holding::PortfolioCache.new(@account) assert_equal @trade.price, cache.get_price(@security.id, @trade.entry.date).price end - test "if no price from db, provider, or trades, search holdings" do + test "if no price from db or trades, search holdings" do Security::Price.delete_all Entry.delete_all @@ -72,16 +53,7 @@ class Holding::PortfolioCacheTest < ActiveSupport::TestCase currency: "USD" ) - expect_provider_prices([], start_date: @account.start_date) - cache = Holding::PortfolioCache.new(@account, use_holdings: true) assert_equal holding.price, cache.get_price(@security.id, holding.date).price end - - private - def expect_provider_prices(prices, start_date:, end_date: Date.current) - @provider.expects(:fetch_security_prices) - .with(@security, start_date: start_date, end_date: end_date) - .returns(provider_success_response(prices)) - end end diff --git a/test/models/holding/reverse_calculator_test.rb b/test/models/holding/reverse_calculator_test.rb index ec9dc204..785d7b94 100644 --- a/test/models/holding/reverse_calculator_test.rb +++ b/test/models/holding/reverse_calculator_test.rb @@ -20,26 +20,26 @@ class Holding::ReverseCalculatorTest < ActiveSupport::TestCase test "holding generation respects user timezone and last generated date is current user date" do # Simulate user in EST timezone - Time.zone = "America/New_York" + Time.use_zone("America/New_York") do + # Set current time to 1am UTC on Jan 5, 2025 + # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for) + travel_to Time.utc(2025, 01, 05, 1, 0, 0) - # Set current time to 1am UTC on Jan 5, 2025 - # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for) - travel_to Time.utc(2025, 01, 05, 1, 0, 0) + voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF") + Security::Price.create!(security: voo, date: "2025-01-02", price: 500) + Security::Price.create!(security: voo, date: "2025-01-03", price: 500) + Security::Price.create!(security: voo, date: "2025-01-04", price: 500) - voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF") - Security::Price.create!(security: voo, date: "2025-01-02", price: 500) - Security::Price.create!(security: voo, date: "2025-01-03", price: 500) - Security::Price.create!(security: voo, date: "2025-01-04", price: 500) + # Today's holdings (provided) + @account.holdings.create!(security: voo, date: "2025-01-04", qty: 10, price: 500, amount: 5000, currency: "USD") - # Today's holdings (provided) - @account.holdings.create!(security: voo, date: "2025-01-04", qty: 10, price: 500, amount: 5000, currency: "USD") + create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account) - create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account) + expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ] + calculated = Holding::ReverseCalculator.new(@account).calculate - expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ] - calculated = Holding::ReverseCalculator.new(@account).calculate - - assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.amount ] } + assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.amount ] } + end end # Should be able to handle this case, although we should not be reverse-syncing an account without provided current day holdings diff --git a/test/models/market_data_syncer_test.rb b/test/models/market_data_syncer_test.rb new file mode 100644 index 00000000..299fb82e --- /dev/null +++ b/test/models/market_data_syncer_test.rb @@ -0,0 +1,71 @@ +require "test_helper" +require "ostruct" + +class MarketDataSyncerTest < ActiveSupport::TestCase + include EntriesTestHelper, ProviderTestHelper + + test "syncs exchange rates with upsert" do + empty_db + + family1 = Family.create!(name: "Family 1", currency: "USD") + account1 = family1.accounts.create!(name: "Account 1", currency: "USD", balance: 100, accountable: Depository.new) + account2 = family1.accounts.create!(name: "Account 2", currency: "CAD", balance: 100, accountable: Depository.new) + + family2 = Family.create!(name: "Family 2", currency: "EUR") + account3 = family2.accounts.create!(name: "Account 3", currency: "EUR", balance: 100, accountable: Depository.new) + account4 = family2.accounts.create!(name: "Account 4", currency: "USD", balance: 100, accountable: Depository.new) + + mock_provider = mock + Provider::Registry.any_instance.expects(:get_provider).with(:synth).returns(mock_provider).at_least_once + + start_date = 1.month.ago.to_date + end_date = Date.current.in_time_zone("America/New_York").to_date + + # Put an existing rate in DB to test upsert + ExchangeRate.create!(from_currency: "CAD", to_currency: "USD", date: start_date, rate: 2.0) + + mock_provider.expects(:fetch_exchange_rates) + .with(from: "CAD", to: "USD", start_date: start_date, end_date: end_date) + .returns(provider_success_response([ OpenStruct.new(from: "CAD", to: "USD", date: start_date, rate: 1.0) ])) + + mock_provider.expects(:fetch_exchange_rates) + .with(from: "USD", to: "EUR", start_date: start_date, end_date: end_date) + .returns(provider_success_response([ OpenStruct.new(from: "USD", to: "EUR", date: start_date, rate: 1.0) ])) + + assert_difference "ExchangeRate.count", 1 do + MarketDataSyncer.new.sync_exchange_rates + end + + assert_equal 1.0, ExchangeRate.where(from_currency: "CAD", to_currency: "USD", date: start_date).first.rate + end + + test "syncs security prices with upsert" do + empty_db + + aapl = Security.create!(ticker: "AAPL", exchange_operating_mic: "XNAS") + + family = Family.create!(name: "Family 1", currency: "USD") + account = family.accounts.create!(name: "Account 1", currency: "USD", balance: 100, accountable: Investment.new) + + mock_provider = mock + Provider::Registry.any_instance.expects(:get_provider).with(:synth).returns(mock_provider).at_least_once + + start_date = 1.month.ago.to_date + end_date = Date.current.in_time_zone("America/New_York").to_date + + mock_provider.expects(:fetch_security_prices) + .with(aapl, start_date: start_date, end_date: end_date) + .returns(provider_success_response([ OpenStruct.new(security: aapl, date: start_date, price: 100, currency: "USD") ])) + + assert_difference "Security::Price.count", 1 do + MarketDataSyncer.new.sync_prices + end + end + + private + def empty_db + Invitation.destroy_all + Family.destroy_all + Security.destroy_all + end +end diff --git a/test/models/security/price_test.rb b/test/models/security/price_test.rb index bd150359..14c022fe 100644 --- a/test/models/security/price_test.rb +++ b/test/models/security/price_test.rb @@ -49,25 +49,6 @@ class Security::PriceTest < ActiveSupport::TestCase assert_not @security.find_or_fetch_price(date: Date.current) end - test "upserts historical prices from provider" do - Security::Price.delete_all - - # Will be overwritten by upsert - Security::Price.create!(security: @security, date: 1.day.ago.to_date, price: 190, currency: "USD") - - expect_provider_prices(security: @security, start_date: 2.days.ago.to_date, end_date: Date.current, prices: [ - Security::Price.new(security: @security, date: Date.current, price: 215, currency: "USD"), - Security::Price.new(security: @security, date: 1.day.ago.to_date, price: 214, currency: "USD"), - Security::Price.new(security: @security, date: 2.days.ago.to_date, price: 213, currency: "USD") - ]) - - @security.sync_provider_prices(start_date: 2.days.ago.to_date) - - assert_equal 215, @security.prices.find_by(date: Date.current).price - assert_equal 214, @security.prices.find_by(date: 1.day.ago.to_date).price - assert_equal 213, @security.prices.find_by(date: 2.days.ago.to_date).price - end - private def expect_provider_price(security:, price:, date:) @provider.expects(:fetch_security_price) diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index 99019146..cbab9ed3 100644 --- a/test/models/sync_test.rb +++ b/test/models/sync_test.rb @@ -1,34 +1,170 @@ require "test_helper" class SyncTest < ActiveSupport::TestCase - setup do - @sync = syncs(:account) - @sync.update(status: "pending") - end + include ActiveJob::TestHelper test "runs successful sync" do - @sync.syncable.expects(:sync_data).with(@sync, start_date: @sync.start_date).once + syncable = accounts(:depository) + sync = Sync.create!(syncable: syncable) - assert_equal "pending", @sync.status + syncable.expects(:perform_sync).with(sync).once - previously_ran_at = @sync.last_ran_at + assert_equal "pending", sync.status - @sync.perform + sync.perform - assert @sync.last_ran_at > previously_ran_at - assert_equal "completed", @sync.status + assert sync.completed_at < Time.now + assert_equal "completed", sync.status end test "handles sync errors" do - @sync.syncable.expects(:sync_data).with(@sync, start_date: @sync.start_date).raises(StandardError.new("test sync error")) + syncable = accounts(:depository) + sync = Sync.create!(syncable: syncable) - assert_equal "pending", @sync.status - previously_ran_at = @sync.last_ran_at + syncable.expects(:perform_sync).with(sync).raises(StandardError.new("test sync error")) - @sync.perform + assert_equal "pending", sync.status - assert @sync.last_ran_at > previously_ran_at - assert_equal "failed", @sync.status - assert_equal "test sync error", @sync.error + sync.perform + + assert sync.failed_at < Time.now + assert_equal "failed", sync.status + assert_equal "test sync error", sync.error + end + + test "can run nested syncs that alert the parent when complete" do + family = families(:dylan_family) + plaid_item = plaid_items(:one) + account = accounts(:connected) + + family_sync = Sync.create!(syncable: family) + plaid_item_sync = Sync.create!(syncable: plaid_item, parent: family_sync) + account_sync = Sync.create!(syncable: account, parent: plaid_item_sync) + + assert_equal "pending", family_sync.status + assert_equal "pending", plaid_item_sync.status + assert_equal "pending", account_sync.status + + family.expects(:perform_sync).with(family_sync).once + + family_sync.perform + + assert_equal "syncing", family_sync.reload.status + + plaid_item.expects(:perform_sync).with(plaid_item_sync).once + + plaid_item_sync.perform + + assert_equal "syncing", family_sync.reload.status + assert_equal "syncing", plaid_item_sync.reload.status + + account.expects(:perform_sync).with(account_sync).once + + # Since these are accessed through `parent`, they won't necessarily be the same + # instance we configured above + Account.any_instance.expects(:perform_post_sync).once + Account.any_instance.expects(:broadcast_sync_complete).once + PlaidItem.any_instance.expects(:perform_post_sync).once + PlaidItem.any_instance.expects(:broadcast_sync_complete).once + Family.any_instance.expects(:perform_post_sync).once + Family.any_instance.expects(:broadcast_sync_complete).once + + account_sync.perform + + assert_equal "completed", plaid_item_sync.reload.status + assert_equal "completed", account_sync.reload.status + assert_equal "completed", family_sync.reload.status + end + + test "failures propagate up the chain" do + family = families(:dylan_family) + plaid_item = plaid_items(:one) + account = accounts(:connected) + + family_sync = Sync.create!(syncable: family) + plaid_item_sync = Sync.create!(syncable: plaid_item, parent: family_sync) + account_sync = Sync.create!(syncable: account, parent: plaid_item_sync) + + assert_equal "pending", family_sync.status + assert_equal "pending", plaid_item_sync.status + assert_equal "pending", account_sync.status + + family.expects(:perform_sync).with(family_sync).once + + family_sync.perform + + assert_equal "syncing", family_sync.reload.status + + plaid_item.expects(:perform_sync).with(plaid_item_sync).once + + plaid_item_sync.perform + + assert_equal "syncing", family_sync.reload.status + assert_equal "syncing", plaid_item_sync.reload.status + + # This error should "bubble up" to the PlaidItem and Family sync results + account.expects(:perform_sync).with(account_sync).raises(StandardError.new("test account sync error")) + + # Since these are accessed through `parent`, they won't necessarily be the same + # instance we configured above + Account.any_instance.expects(:perform_post_sync).once + PlaidItem.any_instance.expects(:perform_post_sync).once + Family.any_instance.expects(:perform_post_sync).once + + Account.any_instance.expects(:broadcast_sync_complete).once + PlaidItem.any_instance.expects(:broadcast_sync_complete).once + Family.any_instance.expects(:broadcast_sync_complete).once + + account_sync.perform + + assert_equal "failed", plaid_item_sync.reload.status + assert_equal "failed", account_sync.reload.status + assert_equal "failed", family_sync.reload.status + end + + test "parent failure should not change status if child succeeds" do + family = families(:dylan_family) + plaid_item = plaid_items(:one) + account = accounts(:connected) + + family_sync = Sync.create!(syncable: family) + plaid_item_sync = Sync.create!(syncable: plaid_item, parent: family_sync) + account_sync = Sync.create!(syncable: account, parent: plaid_item_sync) + + assert_equal "pending", family_sync.status + assert_equal "pending", plaid_item_sync.status + assert_equal "pending", account_sync.status + + family.expects(:perform_sync).with(family_sync).raises(StandardError.new("test family sync error")) + + family_sync.perform + + assert_equal "failed", family_sync.reload.status + + plaid_item.expects(:perform_sync).with(plaid_item_sync).raises(StandardError.new("test plaid item sync error")) + + plaid_item_sync.perform + + assert_equal "failed", family_sync.reload.status + assert_equal "failed", plaid_item_sync.reload.status + + # Leaf level sync succeeds, but shouldn't change the status of the already-failed parent syncs + account.expects(:perform_sync).with(account_sync).once + + # Since these are accessed through `parent`, they won't necessarily be the same + # instance we configured above + Account.any_instance.expects(:perform_post_sync).once + PlaidItem.any_instance.expects(:perform_post_sync).once + Family.any_instance.expects(:perform_post_sync).once + + Account.any_instance.expects(:broadcast_sync_complete).once + PlaidItem.any_instance.expects(:broadcast_sync_complete).once + Family.any_instance.expects(:broadcast_sync_complete).once + + account_sync.perform + + assert_equal "failed", plaid_item_sync.reload.status + assert_equal "failed", family_sync.reload.status + assert_equal "completed", account_sync.reload.status end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 23f98faa..7eac9dde 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -17,6 +17,7 @@ require "rails/test_help" require "minitest/mock" require "minitest/autorun" require "mocha/minitest" +require "aasm/minitest" VCR.configure do |config| config.cassette_library_dir = "test/vcr_cassettes"