From f7064fd4ddc3defcd4404d7b88bbb05420506091 Mon Sep 17 00:00:00 2001 From: Harshit Chaudhary <55315065+Harry-kp@users.noreply.github.com> Date: Thu, 27 Feb 2025 01:43:51 +0530 Subject: [PATCH 01/47] fixed example account balance (#1910) --- app/views/onboardings/preferences.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/onboardings/preferences.html.erb b/app/views/onboardings/preferences.html.erb index 633b9145..a0e2330b 100644 --- a/app/views/onboardings/preferences.html.erb +++ b/app/views/onboardings/preferences.html.erb @@ -12,7 +12,7 @@
<%= tag.p t(".example"), class: "text-secondary text-sm" %> - <%= tag.p "$2,323.25", class: "text-primary font-medium text-2xl" %> + <%= tag.p format_money(Money.new(2325.25, params[:currency] || @user.family.currency)), class: "text-primary font-medium text-2xl" %>

+<%= format_money(Money.new(78.90, params[:currency] || @user.family.currency)) %> (+<%= format_money(Money.new(6.39, params[:currency] || @user.family.currency)) %>) From 8208722247e4d3c8c9f9130e088a6aa69835b75c Mon Sep 17 00:00:00 2001 From: Tony Vincent Date: Fri, 28 Feb 2025 13:49:12 +0100 Subject: [PATCH 02/47] Feat: Data "reset" button (#1913) * feat: Allow admins to delete family data * feat: Allow self-hosting users to delete cached data * Remove system tests --- .../settings/hostings_controller.rb | 10 ++++ app/controllers/users_controller.rb | 10 ++++ app/jobs/data_cache_clear_job.rb | 16 +++++++ app/jobs/family_reset_job.rb | 19 ++++++++ .../hostings/_danger_zone_settings.html.erb | 20 ++++++++ app/views/settings/hostings/show.html.erb | 4 ++ app/views/settings/profiles/show.html.erb | 48 +++++++++++++------ config/locales/views/settings/en.yml | 5 ++ config/locales/views/settings/hostings/en.yml | 9 ++++ config/locales/views/users/en.yml | 3 ++ config/routes.rb | 8 +++- .../settings/hostings_controller_test.rb | 35 ++++++++++++++ test/controllers/users_controller_test.rb | 35 ++++++++++++++ 13 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 app/jobs/data_cache_clear_job.rb create mode 100644 app/jobs/family_reset_job.rb create mode 100644 app/views/settings/hostings/_danger_zone_settings.html.erb diff --git a/app/controllers/settings/hostings_controller.rb b/app/controllers/settings/hostings_controller.rb index fa88d23c..0740b0bc 100644 --- a/app/controllers/settings/hostings_controller.rb +++ b/app/controllers/settings/hostings_controller.rb @@ -2,6 +2,7 @@ class Settings::HostingsController < ApplicationController layout "settings" before_action :raise_if_not_self_hosted + before_action :ensure_admin, only: :clear_cache def show @synth_usage = Current.family.synth_usage @@ -38,6 +39,11 @@ class Settings::HostingsController < ApplicationController render :show, status: :unprocessable_entity end + def clear_cache + DataCacheClearJob.perform_later(Current.family) + redirect_to settings_hosting_path, notice: t(".cache_cleared") + end + private def hosting_params params.require(:setting).permit(:render_deploy_hook, :upgrades_setting, :require_invite_for_signup, :require_email_confirmation, :synth_api_key) @@ -46,4 +52,8 @@ class Settings::HostingsController < ApplicationController def raise_if_not_self_hosted raise "Settings not available on non-self-hosted instance" unless self_hosted? end + + def ensure_admin + redirect_to settings_hosting_path, alert: t(".not_authorized") unless Current.user.admin? + end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d8d0de8a..9b82509d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,5 +1,6 @@ class UsersController < ApplicationController before_action :set_user + before_action :ensure_admin, only: :reset def update @user = Current.user @@ -26,6 +27,11 @@ class UsersController < ApplicationController end end + def reset + FamilyResetJob.perform_later(Current.family) + redirect_to settings_profile_path, notice: t(".success") + end + def destroy if @user.deactivate Current.session.destroy @@ -68,4 +74,8 @@ class UsersController < ApplicationController def set_user @user = Current.user end + + def ensure_admin + redirect_to settings_profile_path, alert: I18n.t("users.reset.unauthorized") unless Current.user.admin? + end end diff --git a/app/jobs/data_cache_clear_job.rb b/app/jobs/data_cache_clear_job.rb new file mode 100644 index 00000000..49e18880 --- /dev/null +++ b/app/jobs/data_cache_clear_job.rb @@ -0,0 +1,16 @@ +class DataCacheClearJob < ApplicationJob + queue_as :default + + def perform(family) + ActiveRecord::Base.transaction do + ExchangeRate.delete_all + Security::Price.delete_all + family.accounts.each do |account| + account.balances.delete_all + account.holdings.delete_all + end + + family.sync_later + end + end +end diff --git a/app/jobs/family_reset_job.rb b/app/jobs/family_reset_job.rb new file mode 100644 index 00000000..20dc2499 --- /dev/null +++ b/app/jobs/family_reset_job.rb @@ -0,0 +1,19 @@ +class FamilyResetJob < ApplicationJob + queue_as :default + + def perform(family) + # Delete all family data except users + ActiveRecord::Base.transaction do + # Delete accounts and related data + family.accounts.destroy_all + family.categories.destroy_all + family.tags.destroy_all + family.merchants.destroy_all + family.plaid_items.destroy_all + family.imports.destroy_all + family.budgets.destroy_all + + family.sync_later + end + end +end diff --git a/app/views/settings/hostings/_danger_zone_settings.html.erb b/app/views/settings/hostings/_danger_zone_settings.html.erb new file mode 100644 index 00000000..7f04c4e2 --- /dev/null +++ b/app/views/settings/hostings/_danger_zone_settings.html.erb @@ -0,0 +1,20 @@ +<% if Current.user.admin? %> +

+
+
+

<%= t("settings.hostings.show.clear_cache") %>

+

<%= t("settings.hostings.show.clear_cache_warning") %>

+
+ <%= + button_to t("settings.hostings.show.clear_cache"), clear_cache_settings_hosting_path, method: :delete, + class: "bg-orange-500 text-white text-sm font-medium rounded-lg px-4 py-2", + data: { turbo_confirm: { + title: t("settings.hostings.show.confirm_clear_cache.title"), + body: t("settings.hostings.show.confirm_clear_cache.body"), + accept: t("settings.hostings.show.clear_cache"), + acceptClass: "w-full bg-orange-500 text-white rounded-xl text-center p-[10px] border mb-2" + }} + %> +
+
+<% end %> diff --git a/app/views/settings/hostings/show.html.erb b/app/views/settings/hostings/show.html.erb index ca74914a..bd1916f3 100644 --- a/app/views/settings/hostings/show.html.erb +++ b/app/views/settings/hostings/show.html.erb @@ -11,3 +11,7 @@ <%= settings_section title: t(".invites") do %> <%= render "settings/hostings/invite_code_settings" %> <% end %> + +<%= settings_section title: t(".danger_zone") do %> + <%= render "settings/hostings/danger_zone_settings" %> +<% end %> diff --git a/app/views/settings/profiles/show.html.erb b/app/views/settings/profiles/show.html.erb index 165f9b1c..168f1472 100644 --- a/app/views/settings/profiles/show.html.erb +++ b/app/views/settings/profiles/show.html.erb @@ -127,20 +127,40 @@ <% end %> <%= settings_section title: t(".danger_zone_title") do %> -
-
-

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

-

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

+
+ <% if Current.user.admin? %> +
+
+

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

+

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

+
+ <%= + button_to t(".reset_account"), reset_user_path(@user), method: :delete, + class: "bg-orange-500 text-white text-sm font-medium rounded-lg px-4 py-2", + data: { turbo_confirm: { + title: t(".confirm_reset.title"), + body: t(".confirm_reset.body"), + accept: t(".reset_account"), + acceptClass: "w-full bg-orange-500 text-white rounded-xl text-center p-[10px] border mb-2" + }} + %> +
+ <% end %> +
+
+

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

+

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

+
+ <%= + button_to t(".delete_account"), user_path(@user), method: :delete, + class: "bg-red-500 text-white text-sm font-medium rounded-lg px-3 py-2", + data: { turbo_confirm: { + title: t(".confirm_delete.title"), + body: t(".confirm_delete.body"), + accept: t(".delete_account"), + acceptClass: "w-full bg-red-500 text-white rounded-xl text-center p-[10px] border mb-2" + }} + %>
- <%= - button_to t(".delete_account"), user_path(@user), method: :delete, - class: "bg-red-500 text-white text-sm font-medium rounded-lg px-3 py-2", - data: { turbo_confirm: { - title: t(".confirm_delete.title"), - body: t(".confirm_delete.body"), - accept: t(".delete_account"), - acceptClass: "w-full bg-red-500 text-white rounded-xl text-center p-[10px] border mb-2" - }} - %>
<% end %> diff --git a/config/locales/views/settings/en.yml b/config/locales/views/settings/en.yml index e7e7d627..8596904e 100644 --- a/config/locales/views/settings/en.yml +++ b/config/locales/views/settings/en.yml @@ -39,6 +39,9 @@ en: body: Are you sure you want to permanently delete your account? This action is irreversible. title: Delete account? + confirm_reset: + body: Are you sure you want to reset your account? This will delete all your accounts, categories, merchants, tags, and other data. This action cannot be undone. + title: Reset account? confirm_remove_invitation: body: Are you sure you want to remove the invitation for %{email}? title: Remove Invitation @@ -49,6 +52,8 @@ en: delete_account: Delete account delete_account_warning: Deleting your account will permanently remove all your data and cannot be undone. + reset_account: Reset account + reset_account_warning: Resetting your account will delete all your accounts, categories, merchants, tags, and other data, but keep your user account intact. email: Email first_name: First Name household_form_input_placeholder: Enter household name diff --git a/config/locales/views/settings/hostings/en.yml b/config/locales/views/settings/hostings/en.yml index 8cb7f6c4..bbcb8ee9 100644 --- a/config/locales/views/settings/hostings/en.yml +++ b/config/locales/views/settings/hostings/en.yml @@ -20,6 +20,12 @@ en: general: General Settings invites: Invite Codes title: Self-Hosting + danger_zone: Danger Zone + clear_cache: Clear data cache + clear_cache_warning: Clearing the data cache will remove all exchange rates, security prices, account balances, and other data. This will not delete accounts, transactions, categories, or other user-owned data. + confirm_clear_cache: + title: Clear data cache? + body: Are you sure you want to clear the data cache? This will remove all exchange rates, security prices, account balances, and other data. This action cannot be undone. synth_settings: api_calls_used: "%{used} / %{limit} API calls used (%{percentage})" description: Input the API key provided by Synth @@ -30,6 +36,8 @@ en: update: failure: Invalid setting value success: Settings updated + clear_cache: + cache_cleared: Data cache has been cleared. This may take a few moments to complete. upgrade_settings: description: Configure how your application receives updates latest_commit_description: Automatically update to the latest commit (unstable) @@ -40,3 +48,4 @@ en: manual_description: You control when to download and install updates manual_title: Manual title: Auto Upgrade + not_authorized: You are not authorized to perform this action diff --git a/config/locales/views/users/en.yml b/config/locales/views/users/en.yml index e9aae045..e04ebc8c 100644 --- a/config/locales/views/users/en.yml +++ b/config/locales/views/users/en.yml @@ -8,3 +8,6 @@ en: email_change_initiated: Please check your new email address for confirmation instructions. success: Your profile has been updated. + reset: + success: Your account has been reset. Data will be deleted in the background in some time. + unauthorized: You are not authorized to perform this action diff --git a/config/routes.rb b/config/routes.rb index 13f9f61f..30d3303e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,7 +18,9 @@ Rails.application.routes.draw do resource :password, only: %i[edit update] resource :email_confirmation, only: :new - resources :users, only: %i[update destroy] + resources :users, only: %i[update destroy] do + delete :reset, on: :member + end resource :onboarding, only: :show do collection do @@ -30,7 +32,9 @@ Rails.application.routes.draw do namespace :settings do resource :profile, only: [ :show, :destroy ] resource :preferences, only: :show - resource :hosting, only: %i[show update] + resource :hosting, only: %i[show update] do + delete :clear_cache, on: :collection + end resource :billing, only: :show resource :security, only: :show end diff --git a/test/controllers/settings/hostings_controller_test.rb b/test/controllers/settings/hostings_controller_test.rb index 283b70bc..3ee8a226 100644 --- a/test/controllers/settings/hostings_controller_test.rb +++ b/test/controllers/settings/hostings_controller_test.rb @@ -45,4 +45,39 @@ class Settings::HostingsControllerTest < ActionDispatch::IntegrationTest assert_equal NEW_RENDER_DEPLOY_HOOK, Setting.render_deploy_hook end end + + test "can clear data cache when self hosting is enabled" do + account = accounts(:investment) + holding = account.holdings.first + exchange_rate = exchange_rates(:one) + security_price = holding.security.prices.first + account_balance = account.balances.create!(date: Date.current, balance: 1000, currency: "USD") + + with_self_hosting do + perform_enqueued_jobs(only: DataCacheClearJob) do + delete clear_cache_settings_hosting_url + end + end + + assert_redirected_to settings_hosting_url + assert_equal I18n.t("settings.hostings.clear_cache.cache_cleared"), flash[:notice] + + assert_not ExchangeRate.exists?(exchange_rate.id) + assert_not Security::Price.exists?(security_price.id) + assert_not Account::Holding.exists?(holding.id) + assert_not Account::Balance.exists?(account_balance.id) + end + + test "can clear data only when admin" do + with_self_hosting do + sign_in users(:family_member) + + assert_no_enqueued_jobs do + delete clear_cache_settings_hosting_url + end + + assert_redirected_to settings_hosting_url + assert_equal I18n.t("settings.hostings.not_authorized"), flash[:alert] + end + end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 74e217f2..bd68fe77 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -31,6 +31,41 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_equal "Your profile has been updated.", flash[:notice] end + test "admin can reset family data" do + account = accounts(:investment) + category = categories(:income) + tag = tags(:one) + merchant = merchants(:netflix) + import = imports(:transaction) + budget = budgets(:one) + plaid_item = plaid_items(:one) + + perform_enqueued_jobs(only: FamilyResetJob) do + delete reset_user_url(@user) + end + + assert_redirected_to settings_profile_url + assert_equal I18n.t("users.reset.success"), flash[:notice] + + assert_not Account.exists?(account.id) + assert_not Category.exists?(category.id) + assert_not Tag.exists?(tag.id) + assert_not Merchant.exists?(merchant.id) + assert_not Import.exists?(import.id) + assert_not Budget.exists?(budget.id) + assert_not PlaidItem.exists?(plaid_item.id) + end + + test "non-admin cannot reset family data" do + sign_in @member = users(:family_member) + + delete reset_user_url(@member) + + assert_redirected_to settings_profile_url + assert_equal I18n.t("users.reset.unauthorized"), flash[:alert] + assert_no_enqueued_jobs only: FamilyResetJob + end + test "member can deactivate their account" do sign_in @member = users(:family_member) delete user_url(@member) From fae781e1be5b96b484d4b41a027abbbaa11e4d7a Mon Sep 17 00:00:00 2001 From: Tony Vincent Date: Fri, 28 Feb 2025 13:53:05 +0100 Subject: [PATCH 03/47] Make tags scrollable again (#1921) --- app/views/account/transactions/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/account/transactions/show.html.erb b/app/views/account/transactions/show.html.erb index e101e8cf..17ff31cf 100644 --- a/app/views/account/transactions/show.html.erb +++ b/app/views/account/transactions/show.html.erb @@ -81,7 +81,7 @@ label: t(".tags_label"), container_class: "h-40" }, - { "data-auto-submit-form-target": "auto" } %> + { "data-auto-submit-form-target": "auto", class: "overflow-y-auto border-none" } %> <% end %> <% end %> From 98c842d3b87c8bc46f503be487c4a4ceabf02474 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 08:23:46 -0500 Subject: [PATCH 04/47] Add note about self hosted versions prior to opening bugs --- .github/ISSUE_TEMPLATE/bug_report.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 389a4271..a2671474 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -10,6 +10,7 @@ assignees: '' **Where did this bug occur? (required)** - [ ] I am a self-hosted user reporting a bug from my self hosted app + - [ ] I have verified that I am running the **latest** version of the Maybe app (your app should be running [this version](https://github.com/maybe-finance/maybe/pkgs/container/maybe/364919621?tag=latest) before opening a bug) - [ ] I am a user of Maybe's paid app _Please note, if you are reporting a bug with sensitive data, please open an Intercom chat from within the app for help_ From 58cc09f5ae0592e6fc9aa4b34c50641241803891 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 08:25:20 -0500 Subject: [PATCH 05/47] Fix bad link in bug template Signed-off-by: Zach Gollwitzer --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index a2671474..203ba646 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -10,7 +10,7 @@ assignees: '' **Where did this bug occur? (required)** - [ ] I am a self-hosted user reporting a bug from my self hosted app - - [ ] I have verified that I am running the **latest** version of the Maybe app (your app should be running [this version](https://github.com/maybe-finance/maybe/pkgs/container/maybe/364919621?tag=latest) before opening a bug) + - [ ] I have verified that I am running the **latest** version of the Maybe app (your app should be running [this version](https://github.com/maybe-finance/maybe/pkgs/container/maybe) before opening a bug) - [ ] I am a user of Maybe's paid app _Please note, if you are reporting a bug with sensitive data, please open an Intercom chat from within the app for help_ From e771c8c1df0a1db322193377c10446138cb72422 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 08:35:14 -0500 Subject: [PATCH 06/47] Fix value wrapping on account balance in sidebar (#1922) --- app/views/accounts/_accountable_group.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/accounts/_accountable_group.html.erb b/app/views/accounts/_accountable_group.html.erb index 008cc4b1..b240a557 100644 --- a/app/views/accounts/_accountable_group.html.erb +++ b/app/views/accounts/_accountable_group.html.erb @@ -28,7 +28,7 @@
- <%= tag.p format_money(account.balance_money), class: "text-sm font-medium text-primary" %> + <%= 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 %>
From d6793dec0507ea50dc6b26be59a65983de5d63b6 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 08:36:57 -0500 Subject: [PATCH 07/47] Fix import configuration form so number format is applied (#1923) * Fix number format form error when loading import * Add test to verify import configuration was properly applied --- .../import/configurations_controller.rb | 1 + .../import/configurations_controller_test.rb | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/controllers/import/configurations_controller.rb b/app/controllers/import/configurations_controller.rb index 9a060f8f..3b9ca5b5 100644 --- a/app/controllers/import/configurations_controller.rb +++ b/app/controllers/import/configurations_controller.rb @@ -35,6 +35,7 @@ class Import::ConfigurationsController < ApplicationController :notes_col_label, :currency_col_label, :date_format, + :number_format, :signage_convention ) end diff --git a/test/controllers/import/configurations_controller_test.rb b/test/controllers/import/configurations_controller_test.rb index 95edf0c3..078c17fe 100644 --- a/test/controllers/import/configurations_controller_test.rb +++ b/test/controllers/import/configurations_controller_test.rb @@ -23,11 +23,24 @@ class Import::ConfigurationsControllerTest < ActionDispatch::IntegrationTest tags_col_label: "Tags", amount_col_label: "Amount", signage_convention: "inflows_positive", - account_col_label: "Account" + account_col_label: "Account", + number_format: "1.234,56" } } assert_redirected_to import_clean_url(@import) assert_equal "Import configured successfully.", flash[:notice] + + # Verify configurations were saved + @import.reload + assert_equal "Date", @import.date_col_label + assert_equal "%Y-%m-%d", @import.date_format + assert_equal "Name", @import.name_col_label + assert_equal "Category", @import.category_col_label + assert_equal "Tags", @import.tags_col_label + assert_equal "Amount", @import.amount_col_label + assert_equal "inflows_positive", @import.signage_convention + assert_equal "Account", @import.account_col_label + assert_equal "1.234,56", @import.number_format end end From 882857fcf0a6f5373de8e8ee4545fe703b62af4d Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 09:29:07 -0500 Subject: [PATCH 08/47] Add transitions to buttons and other common design system elements (#1924) --- app/assets/tailwind/maybe-design-system.css | 43 +++++++++++-------- .../account/transactions/bulk_edit.html.erb | 4 +- app/views/account/transactions/show.html.erb | 2 +- app/views/budgets/_picker.html.erb | 2 +- app/views/mfa/new.html.erb | 2 +- app/views/settings/profiles/show.html.erb | 6 +-- app/views/transactions/_header.html.erb | 2 +- 7 files changed, 34 insertions(+), 27 deletions(-) diff --git a/app/assets/tailwind/maybe-design-system.css b/app/assets/tailwind/maybe-design-system.css index 3f252db6..c37eb286 100644 --- a/app/assets/tailwind/maybe-design-system.css +++ b/app/assets/tailwind/maybe-design-system.css @@ -331,29 +331,13 @@ details>summary { @apply list-none; } - - select[multiple="multiple"] { - @apply py-2 pr-2 space-y-0.5 overflow-y-auto; - } - - select[multiple="multiple"] option { - @apply py-2 rounded-md; - } - - select[multiple="multiple"] option:checked { - @apply after:content-['\2713'] bg-white after:text-gray-500 after:ml-2; - } - - select[multiple="multiple"] option:active, - select[multiple="multiple"] option:focus { - @apply bg-white; - } } @layer components { /* Buttons */ .btn { @apply px-3 py-2 rounded-lg text-sm font-medium cursor-pointer disabled:cursor-not-allowed focus:outline-gray-500; + @apply transition-all duration-300; } .btn--primary { @@ -380,6 +364,25 @@ .form-field { @apply flex flex-col gap-1 relative px-3 py-2 rounded-md border bg-white border-alpha-black-100 shadow-xs w-full; @apply focus-within:border-gray-900 focus-within:shadow-none focus-within:ring-4 focus-within:ring-gray-100; + @apply transition-all duration-300; + + /* Add styles for multiple select within form fields */ + select[multiple] { + @apply py-2 pr-2 space-y-0.5 overflow-y-auto; + + option { + @apply py-2 rounded-md; + } + + option:checked { + @apply after:content-['\2713'] bg-white after:text-gray-500 after:ml-2; + } + + option:active, + option:focus { + @apply bg-white; + } + } } .form-field__label { @@ -392,6 +395,7 @@ @apply placeholder-shown:opacity-50; @apply disabled:text-gray-400; @apply text-ellipsis overflow-hidden whitespace-nowrap; + @apply transition-opacity duration-300; &select { @apply pr-8; @@ -410,6 +414,7 @@ .checkbox { &[type='checkbox'] { @apply rounded-sm; + @apply transition-colors duration-300; } } @@ -440,8 +445,10 @@ /* Switches */ .switch { @apply block bg-gray-100 w-9 h-5 rounded-full cursor-pointer; - @apply after:content-[''] after:block after:absolute after:top-0.5 after:left-0.5 after:bg-white after:w-4 after:h-4 after:rounded-full after:transition-transform after:duration-300 after:ease-in-out; + @apply after:content-[''] after:block after:absolute after:top-0.5 after:left-0.5 after:bg-white after:w-4 after:h-4 after:rounded-full; + @apply after:transition-transform after:duration-300 after:ease-in-out; @apply peer-checked:bg-green-600 peer-checked:after:translate-x-4; + @apply transition-colors duration-300; } /* Tooltips */ diff --git a/app/views/account/transactions/bulk_edit.html.erb b/app/views/account/transactions/bulk_edit.html.erb index 02afa497..b8317084 100644 --- a/app/views/account/transactions/bulk_edit.html.erb +++ b/app/views/account/transactions/bulk_edit.html.erb @@ -49,12 +49,12 @@
- <%= link_to t(".cancel"), transactions_path, class: "text-sm font-medium text-primary px-3 py-2" %> + <%= link_to t(".cancel"), transactions_path, class: "btn btn--ghost" %> <%= tag.button t(".save"), type: "button", data: { "bulk-select-scope-param": "bulk_update", action: "bulk-select#submitBulkRequest" }, - class: "px-3 py-2 bg-gray-900 text-white text-sm font-medium rounded-lg" %> + class: "btn btn--primary" %>
<% end %> diff --git a/app/views/account/transactions/show.html.erb b/app/views/account/transactions/show.html.erb index 17ff31cf..e101e8cf 100644 --- a/app/views/account/transactions/show.html.erb +++ b/app/views/account/transactions/show.html.erb @@ -81,7 +81,7 @@ label: t(".tags_label"), container_class: "h-40" }, - { "data-auto-submit-form-target": "auto", class: "overflow-y-auto border-none" } %> + { "data-auto-submit-form-target": "auto" } %> <% end %> <% end %> diff --git a/app/views/budgets/_picker.html.erb b/app/views/budgets/_picker.html.erb index 4aefbf62..8fb4bd54 100644 --- a/app/views/budgets/_picker.html.erb +++ b/app/views/budgets/_picker.html.erb @@ -38,7 +38,7 @@ <% param_key = Budget.date_to_param(date) %> <% if Budget.budget_date_valid?(date, family: family) %> - <%= link_to month_name, budget_path(param_key), data: { turbo_frame: "_top" }, class: "block px-3 py-2 text-sm text-primary hover:bg-gray-100 rounded-md" %> + <%= link_to month_name, budget_path(param_key), data: { turbo_frame: "_top" }, class: "btn btn--ghost" %> <% else %> <%= month_name %> <% end %> diff --git a/app/views/mfa/new.html.erb b/app/views/mfa/new.html.erb index 68bbcf54..fdf753c0 100644 --- a/app/views/mfa/new.html.erb +++ b/app/views/mfa/new.html.erb @@ -57,7 +57,7 @@ placeholder: t(".code_placeholder") %>
- <%= f.submit t(".verify_button"), class: "bg-gray-900 hover:bg-gray-700 cursor-pointer text-white rounded-lg px-3 py-2" %> + <%= f.submit t(".verify_button"), class: "btn btn--primary" %>
<% end %> diff --git a/app/views/settings/profiles/show.html.erb b/app/views/settings/profiles/show.html.erb index 168f1472..29a2f239 100644 --- a/app/views/settings/profiles/show.html.erb +++ b/app/views/settings/profiles/show.html.erb @@ -16,7 +16,7 @@ <%= form.text_field :last_name, placeholder: t(".last_name"), label: t(".last_name") %>
- <%= form.submit t(".save"), class: "bg-gray-900 hover:bg-gray-700 cursor-pointer text-white rounded-lg px-3 py-2" %> + <%= form.submit t(".save"), class: "btn btn--primary" %>
<% end %> @@ -136,7 +136,7 @@ <%= button_to t(".reset_account"), reset_user_path(@user), method: :delete, - class: "bg-orange-500 text-white text-sm font-medium rounded-lg px-4 py-2", + class: "btn btn--destructive", data: { turbo_confirm: { title: t(".confirm_reset.title"), body: t(".confirm_reset.body"), @@ -153,7 +153,7 @@ <%= button_to t(".delete_account"), user_path(@user), method: :delete, - class: "bg-red-500 text-white text-sm font-medium rounded-lg px-3 py-2", + class: "btn btn--destructive", data: { turbo_confirm: { title: t(".confirm_delete.title"), body: t(".confirm_delete.body"), diff --git a/app/views/transactions/_header.html.erb b/app/views/transactions/_header.html.erb index 1a53bf80..f2487458 100644 --- a/app/views/transactions/_header.html.erb +++ b/app/views/transactions/_header.html.erb @@ -16,7 +16,7 @@

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

<% end %> - <%= link_to new_account_transaction_path, class: "rounded-lg bg-gray-900 text-white flex items-center gap-1 justify-center hover:bg-gray-700 px-3 py-2", data: { turbo_frame: :modal } do %> + <%= link_to new_account_transaction_path, class: "btn btn--primary flex items-center gap-2", data: { turbo_frame: :modal } do %> <%= lucide_icon("plus", class: "w-5 h-5") %>

New transaction

<% end %> From 9138bd2b76a61b9f1bfda2003aba98acd59ab88c Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 09:34:14 -0500 Subject: [PATCH 09/47] Allow offline trade tickers (#1925) --- app/controllers/account/trades_controller.rb | 2 +- app/models/account/trade_builder.rb | 5 +++-- app/models/security.rb | 5 +++++ app/views/account/trades/_form.html.erb | 15 ++++++++++++--- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/controllers/account/trades_controller.rb b/app/controllers/account/trades_controller.rb index 6ace6538..fd9b7d48 100644 --- a/app/controllers/account/trades_controller.rb +++ b/app/controllers/account/trades_controller.rb @@ -10,7 +10,7 @@ class Account::TradesController < ApplicationController def create_entry_params params.require(:account_entry).permit( - :account_id, :date, :amount, :currency, :qty, :price, :ticker, :type, :transfer_account_id + :account_id, :date, :amount, :currency, :qty, :price, :ticker, :manual_ticker, :type, :transfer_account_id ).tap do |params| account_id = params.delete(:account_id) params[:account] = Current.family.accounts.find(account_id) diff --git a/app/models/account/trade_builder.rb b/app/models/account/trade_builder.rb index 704d851f..c632f272 100644 --- a/app/models/account/trade_builder.rb +++ b/app/models/account/trade_builder.rb @@ -2,7 +2,7 @@ class Account::TradeBuilder include ActiveModel::Model attr_accessor :account, :date, :amount, :currency, :qty, - :price, :ticker, :type, :transfer_account_id + :price, :ticker, :manual_ticker, :type, :transfer_account_id attr_reader :buildable @@ -110,8 +110,9 @@ class Account::TradeBuilder account.family end + # Users can either look up a ticker from our provider (Synth) or enter a manual, "offline" ticker (that we won't fetch prices for) def security - ticker_symbol, exchange_operating_mic = ticker.split("|") + ticker_symbol, exchange_operating_mic = ticker.present? ? ticker.split("|") : [ manual_ticker, nil ] Security.find_or_create_by(ticker: ticker_symbol, exchange_operating_mic: exchange_operating_mic) do |s| FetchSecurityInfoJob.perform_later(s.id) diff --git a/app/models/security.rb b/app/models/security.rb index 46672fb5..991ac202 100644 --- a/app/models/security.rb +++ b/app/models/security.rb @@ -1,5 +1,6 @@ class Security < ApplicationRecord include Providable + before_save :upcase_ticker has_many :trades, dependent: :nullify, class_name: "Account::Trade" @@ -9,6 +10,10 @@ class Security < ApplicationRecord validates :ticker, uniqueness: { scope: :exchange_operating_mic, case_sensitive: false } class << self + def provider + security_prices_provider + end + def search(query) security_prices_provider.search_securities( query: query[:search], diff --git a/app/views/account/trades/_form.html.erb b/app/views/account/trades/_form.html.erb index c8dae4d3..a74d401f 100644 --- a/app/views/account/trades/_form.html.erb +++ b/app/views/account/trades/_form.html.erb @@ -27,9 +27,18 @@ }} %> <% if %w[buy sell].include?(type) %> -
- <%= form.combobox :ticker, securities_path(country_code: Current.family.country), label: t(".holding"), placeholder: t(".ticker_placeholder"), required: true %> -
+ <% if Security.provider.present? %> +
+ <%= form.combobox :ticker, + securities_path(country_code: Current.family.country), + name_when_new: "account_entry[manual_ticker]", + label: t(".holding"), + placeholder: t(".ticker_placeholder"), + required: true %> +
+ <% else %> + <%= form.text_field :manual_ticker, label: "Ticker", placeholder: "AAPL", required: true %> + <% end %> <% end %> <%= form.date_field :date, label: true, value: Date.current, required: true %> From 624faa10d0fef27d1cb04fa40ed3d48f97766c48 Mon Sep 17 00:00:00 2001 From: Tony Vincent Date: Fri, 28 Feb 2025 15:35:00 +0100 Subject: [PATCH 10/47] fix: Don't show Billings on settings navbar when self-hosted (#1912) * Do not show billing settings navbar item when self hosted * Do not show billing settings navbar item when self hosted * Add condition to settings helper * Let Stripe::AuthenticationError bubble up --- app/controllers/subscriptions_controller.rb | 6 ++++++ app/helpers/settings_helper.rb | 7 ++++++- app/views/settings/_settings_nav.html.erb | 9 +++++---- config/locales/views/subscriptions/en.yml | 3 +++ test/controllers/subscriptions_controller_test.rb | 13 ++++++++++--- test/system/settings_test.rb | 6 ++++++ 6 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 config/locales/views/subscriptions/en.yml diff --git a/app/controllers/subscriptions_controller.rb b/app/controllers/subscriptions_controller.rb index e2c19535..64552508 100644 --- a/app/controllers/subscriptions_controller.rb +++ b/app/controllers/subscriptions_controller.rb @@ -1,4 +1,6 @@ class SubscriptionsController < ApplicationController + before_action :redirect_to_root_if_self_hosted + def new if Current.family.stripe_customer_id.blank? customer = stripe_client.v1.customers.create( @@ -44,4 +46,8 @@ class SubscriptionsController < ApplicationController def stripe_client @stripe_client ||= Stripe::StripeClient.new(ENV["STRIPE_SECRET_KEY"]) end + + def redirect_to_root_if_self_hosted + redirect_to root_path, alert: I18n.t("subscriptions.self_hosted_alert") if self_hosted? + end end diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb index 0eaecd85..e15414a5 100644 --- a/app/helpers/settings_helper.rb +++ b/app/helpers/settings_helper.rb @@ -4,7 +4,7 @@ module SettingsHelper { name: I18n.t("settings.settings_nav.preferences_label"), path: :settings_preferences_path }, { name: I18n.t("settings.settings_nav.security_label"), path: :settings_security_path }, { name: I18n.t("settings.settings_nav.self_hosting_label"), path: :settings_hosting_path, condition: :self_hosted? }, - { name: I18n.t("settings.settings_nav.billing_label"), path: :settings_billing_path }, + { name: I18n.t("settings.settings_nav.billing_label"), path: :settings_billing_path, condition: :not_self_hosted? }, { name: I18n.t("settings.settings_nav.accounts_label"), path: :accounts_path }, { name: I18n.t("settings.settings_nav.imports_label"), path: :imports_path }, { name: I18n.t("settings.settings_nav.tags_label"), path: :tags_path }, @@ -45,4 +45,9 @@ module SettingsHelper concat(next_setting) end end + + private + def not_self_hosted? + !self_hosted? + end end diff --git a/app/views/settings/_settings_nav.html.erb b/app/views/settings/_settings_nav.html.erb index 1934d921..632e774c 100644 --- a/app/views/settings/_settings_nav.html.erb +++ b/app/views/settings/_settings_nav.html.erb @@ -32,10 +32,11 @@ <%= render "settings/settings_nav_item", name: t(".self_hosting_label"), path: settings_hosting_path, icon: "database" %> <% end %> - -
  • - <%= render "settings/settings_nav_item", name: t(".billing_label"), path: settings_billing_path, icon: "circle-dollar-sign" %> -
  • + <% unless self_hosted? %> +
  • + <%= render "settings/settings_nav_item", name: t(".billing_label"), path: settings_billing_path, icon: "circle-dollar-sign" %> +
  • + <% end %>
  • <%= render "settings/settings_nav_item", name: t(".accounts_label"), path: accounts_path, icon: "layers" %> diff --git a/config/locales/views/subscriptions/en.yml b/config/locales/views/subscriptions/en.yml new file mode 100644 index 00000000..72ab67a9 --- /dev/null +++ b/config/locales/views/subscriptions/en.yml @@ -0,0 +1,3 @@ +en: + subscriptions: + self_hosted_alert: "Maybe+ is not available in self-hosted mode." diff --git a/test/controllers/subscriptions_controller_test.rb b/test/controllers/subscriptions_controller_test.rb index 3fda28d4..fe1b38d7 100644 --- a/test/controllers/subscriptions_controller_test.rb +++ b/test/controllers/subscriptions_controller_test.rb @@ -1,7 +1,14 @@ require "test_helper" class SubscriptionsControllerTest < ActionDispatch::IntegrationTest - # test "the truth" do - # assert true - # end + setup do + sign_in @user = users(:family_admin) + end + + test "redirects to settings if self hosting" do + Rails.application.config.app_mode.stubs(:self_hosted?).returns(true) + get subscription_path + assert_redirected_to root_path + assert_equal I18n.t("subscriptions.self_hosted_alert"), flash[:alert] + end end diff --git a/test/system/settings_test.rb b/test/system/settings_test.rb index 32f2b8c8..9d0338b0 100644 --- a/test/system/settings_test.rb +++ b/test/system/settings_test.rb @@ -46,6 +46,12 @@ class SettingsTest < ApplicationSystemTestCase assert_selector 'span[data-clipboard-target="iconSuccess"]', visible: true, count: 1 # text copied and icon changed to checkmark end + test "does not show billing link if self hosting" do + Rails.application.config.app_mode.stubs(:self_hosted?).returns(true) + open_settings_from_sidebar + assert_no_selector "li", text: I18n.t("settings.settings_nav.billing_label") + end + private def open_settings_from_sidebar From fa0248056dabb29693e190c6faedbdedc64b9d82 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 11:35:10 -0500 Subject: [PATCH 11/47] Show UI warning to user when they need provider data but have not setup Synth yet (#1926) * Simplify provider concerns * Update tests * Add UI warning for missing Synth key if family requires external data --- app/controllers/securities_controller.rb | 2 +- app/jobs/fetch_security_info_job.rb | 4 +- app/models/account/data_enricher.rb | 4 +- app/models/account/entry.rb | 2 +- app/models/account/entry/provided.rb | 11 ++++++ app/models/concerns/providable.rb | 35 ------------------ app/models/concerns/synthable.rb | 37 +++++++++++++++++++ app/models/exchange_rate/provided.rb | 15 ++++---- app/models/family.rb | 29 ++++++++------- app/models/security.rb | 17 +-------- app/models/security/price/provided.rb | 15 +++++--- app/models/security/provided.rb | 26 +++++++++++++ app/models/trade_import.rb | 17 ++------- app/models/upgrader/provided.rb | 5 ++- .../accounts/_account_sidebar_tabs.html.erb | 20 ++++++++++ .../configurations/_trade_import.html.erb | 5 +-- .../hostings/_synth_settings.html.erb | 9 ++++- test/models/exchange_rate_test.rb | 16 +++++--- test/models/security/price_test.rb | 17 ++++++--- test/models/trade_import_test.rb | 30 +++++---------- test/system/imports_test.rb | 2 + test/system/trades_test.rb | 2 +- 22 files changed, 184 insertions(+), 136 deletions(-) create mode 100644 app/models/account/entry/provided.rb delete mode 100644 app/models/concerns/providable.rb create mode 100644 app/models/concerns/synthable.rb create mode 100644 app/models/security/provided.rb diff --git a/app/controllers/securities_controller.rb b/app/controllers/securities_controller.rb index 5be3cbd9..6cfe9fd0 100644 --- a/app/controllers/securities_controller.rb +++ b/app/controllers/securities_controller.rb @@ -3,7 +3,7 @@ class SecuritiesController < ApplicationController query = params[:q] return render json: [] if query.blank? || query.length < 2 || query.length > 100 - @securities = Security.search({ + @securities = Security.search_provider({ search: query, country: params[:country_code] == "US" ? "US" : nil }) diff --git a/app/jobs/fetch_security_info_job.rb b/app/jobs/fetch_security_info_job.rb index aa64c169..484a47e1 100644 --- a/app/jobs/fetch_security_info_job.rb +++ b/app/jobs/fetch_security_info_job.rb @@ -2,7 +2,7 @@ class FetchSecurityInfoJob < ApplicationJob queue_as :latency_low def perform(security_id) - return unless Security.security_info_provider.present? + return unless Security.provider.present? security = Security.find(security_id) @@ -12,7 +12,7 @@ class FetchSecurityInfoJob < ApplicationJob params[:mic_code] = security.exchange_mic if security.exchange_mic.present? params[:operating_mic] = security.exchange_operating_mic if security.exchange_operating_mic.present? - security_info_response = Security.security_info_provider.fetch_security_info(**params) + security_info_response = Security.provider.fetch_security_info(**params) security.update( name: security_info_response.info.dig("name") diff --git a/app/models/account/data_enricher.rb b/app/models/account/data_enricher.rb index 59979df0..8d07eff8 100644 --- a/app/models/account/data_enricher.rb +++ b/app/models/account/data_enricher.rb @@ -1,6 +1,4 @@ class Account::DataEnricher - include Providable - attr_reader :account def initialize(account) @@ -37,7 +35,7 @@ class Account::DataEnricher candidates.each do |entry| begin - info = self.class.synth_provider.enrich_transaction(entry.name).info + info = entry.fetch_enrichment_info next unless info.present? diff --git a/app/models/account/entry.rb b/app/models/account/entry.rb index b53db19b..25065efd 100644 --- a/app/models/account/entry.rb +++ b/app/models/account/entry.rb @@ -1,5 +1,5 @@ class Account::Entry < ApplicationRecord - include Monetizable + include Monetizable, Provided monetize :amount diff --git a/app/models/account/entry/provided.rb b/app/models/account/entry/provided.rb new file mode 100644 index 00000000..c18654c9 --- /dev/null +++ b/app/models/account/entry/provided.rb @@ -0,0 +1,11 @@ +module Account::Entry::Provided + extend ActiveSupport::Concern + + include Synthable + + def fetch_enrichment_info + return nil unless synth_client.present? + + synth_client.enrich_transaction(name).info + end +end diff --git a/app/models/concerns/providable.rb b/app/models/concerns/providable.rb deleted file mode 100644 index 996efff8..00000000 --- a/app/models/concerns/providable.rb +++ /dev/null @@ -1,35 +0,0 @@ -# `Providable` serves as an extension point for integrating multiple providers. -# For an example of a multi-provider, multi-concept implementation, -# see: https://github.com/maybe-finance/maybe/pull/561 - -module Providable - extend ActiveSupport::Concern - - class_methods do - def security_prices_provider - synth_provider - end - - def security_info_provider - synth_provider - end - - def exchange_rates_provider - synth_provider - end - - def git_repository_provider - Provider::Github.new - end - - def synth_provider - api_key = self_hosted? ? Setting.synth_api_key : ENV["SYNTH_API_KEY"] - api_key.present? ? Provider::Synth.new(api_key) : nil - end - - private - def self_hosted? - Rails.application.config.app_mode.self_hosted? - end - end -end diff --git a/app/models/concerns/synthable.rb b/app/models/concerns/synthable.rb new file mode 100644 index 00000000..51adcade --- /dev/null +++ b/app/models/concerns/synthable.rb @@ -0,0 +1,37 @@ +module Synthable + extend ActiveSupport::Concern + + class_methods do + def synth_usage + synth_client&.usage + end + + def synth_overage? + synth_usage&.usage&.utilization.to_i >= 100 + end + + def synth_healthy? + synth_client&.healthy? + end + + def synth_client + api_key = ENV.fetch("SYNTH_API_KEY", Setting.synth_api_key) + + return nil unless api_key.present? + + Provider::Synth.new(api_key) + end + end + + def synth_client + self.class.synth_client + end + + def synth_usage + self.class.synth_usage + end + + def synth_overage? + self.class.synth_overage? + end +end diff --git a/app/models/exchange_rate/provided.rb b/app/models/exchange_rate/provided.rb index d1e2aea2..d010ff98 100644 --- a/app/models/exchange_rate/provided.rb +++ b/app/models/exchange_rate/provided.rb @@ -1,19 +1,18 @@ module ExchangeRate::Provided extend ActiveSupport::Concern - include Providable + include Synthable class_methods do - def provider_healthy? - exchange_rates_provider.present? && exchange_rates_provider.healthy? + def provider + synth_client end private - def fetch_rates_from_provider(from:, to:, start_date:, end_date: Date.current, cache: false) - return [] unless exchange_rates_provider.present? + return [] unless provider.present? - response = exchange_rates_provider.fetch_exchange_rates \ + response = provider.fetch_exchange_rates \ from: from, to: to, start_date: start_date, @@ -38,9 +37,9 @@ module ExchangeRate::Provided end def fetch_rate_from_provider(from:, to:, date:, cache: false) - return nil unless exchange_rates_provider.present? + return nil unless provider.present? - response = exchange_rates_provider.fetch_exchange_rate \ + response = provider.fetch_exchange_rate \ from: from, to: to, date: date diff --git a/app/models/family.rb b/app/models/family.rb index ff2c8b07..ffb14a7d 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -1,5 +1,5 @@ class Family < ApplicationRecord - include Providable, Plaidable, Syncable, AutoTransferMatchable + include Synthable, Plaidable, Syncable, AutoTransferMatchable DATE_FORMATS = [ [ "MM-DD-YYYY", "%m-%d-%Y" ], @@ -92,22 +92,25 @@ class Family < ApplicationRecord ).link_token end - def synth_usage - self.class.synth_provider&.usage - end - - def synth_overage? - self.class.synth_provider&.usage&.utilization.to_i >= 100 - end - - def synth_valid? - self.class.synth_provider&.healthy? - end - def subscribed? stripe_subscription_status == "active" end + def requires_data_provider? + # If family has any trades, they need a provider for historical prices + return true if trades.any? + + # If family has any accounts not denominated in the family's currency, they need a provider for historical exchange rates + return true if accounts.where.not(currency: self.currency).any? + + # If family has any entries in different currencies, they need a provider for historical exchange rates + uniq_currencies = entries.pluck(:currency).uniq + return true if uniq_currencies.count > 1 + return true if uniq_currencies.first != self.currency + + false + end + def primary_user users.order(:created_at).first end diff --git a/app/models/security.rb b/app/models/security.rb index 991ac202..6d94c798 100644 --- a/app/models/security.rb +++ b/app/models/security.rb @@ -1,5 +1,5 @@ class Security < ApplicationRecord - include Providable + include Provided before_save :upcase_ticker @@ -9,21 +9,6 @@ class Security < ApplicationRecord validates :ticker, presence: true validates :ticker, uniqueness: { scope: :exchange_operating_mic, case_sensitive: false } - class << self - def provider - security_prices_provider - end - - def search(query) - security_prices_provider.search_securities( - query: query[:search], - dataset: "limited", - country_code: query[:country], - exchange_operating_mic: query[:exchange_operating_mic] - ).securities.map { |attrs| new(**attrs) } - end - end - def current_price @current_price ||= Security::Price.find_price(security: self, date: Date.current) return nil if @current_price.nil? diff --git a/app/models/security/price/provided.rb b/app/models/security/price/provided.rb index e2a99774..aed56702 100644 --- a/app/models/security/price/provided.rb +++ b/app/models/security/price/provided.rb @@ -1,16 +1,19 @@ module Security::Price::Provided extend ActiveSupport::Concern - include Providable + include Synthable class_methods do - private + def provider + synth_client + end + private def fetch_price_from_provider(security:, date:, cache: false) - return nil unless security_prices_provider.present? + return nil unless provider.present? return nil unless security.has_prices? - response = security_prices_provider.fetch_security_prices \ + response = provider.fetch_security_prices \ ticker: security.ticker, mic_code: security.exchange_mic, start_date: date, @@ -31,11 +34,11 @@ module Security::Price::Provided end def fetch_prices_from_provider(security:, start_date:, end_date:, cache: false) - return [] unless security_prices_provider.present? + return [] unless provider.present? return [] unless security return [] unless security.has_prices? - response = security_prices_provider.fetch_security_prices \ + response = provider.fetch_security_prices \ ticker: security.ticker, mic_code: security.exchange_mic, start_date: start_date, diff --git a/app/models/security/provided.rb b/app/models/security/provided.rb new file mode 100644 index 00000000..4a4fd6a5 --- /dev/null +++ b/app/models/security/provided.rb @@ -0,0 +1,26 @@ +module Security::Provided + extend ActiveSupport::Concern + + include Synthable + + class_methods do + def provider + synth_client + end + + def search_provider(query) + response = provider.search_securities( + query: query[:search], + dataset: "limited", + country_code: query[:country], + exchange_operating_mic: query[:exchange_operating_mic] + ) + + if response.success? + response.securities.map { |attrs| new(**attrs) } + else + [] + end + end + end +end diff --git a/app/models/trade_import.rb b/app/models/trade_import.rb index ddaad904..549c9093 100644 --- a/app/models/trade_import.rb +++ b/app/models/trade_import.rb @@ -75,10 +75,7 @@ class TradeImport < Import return internal_security if internal_security.present? # If security prices provider isn't properly configured or available, create with nil exchange_operating_mic - provider = Security.security_prices_provider - unless provider.present? && provider.respond_to?(:search_securities) - return Security.find_or_create_by!(ticker: ticker, exchange_operating_mic: nil) - end + return Security.find_or_create_by!(ticker: ticker, exchange_operating_mic: nil) unless Security.provider.present? # Cache provider responses so that when we're looping through rows and importing, # we only hit our provider for the unique combinations of ticker / exchange_operating_mic @@ -86,18 +83,10 @@ class TradeImport < Import @provider_securities_cache ||= {} provider_security = @provider_securities_cache[cache_key] ||= begin - response = provider.search_securities( + Security.search_provider( query: ticker, exchange_operating_mic: exchange_operating_mic - ) - - if !response || !response.success? || !response.securities || response.securities.empty? - nil - else - response.securities.first - end - rescue => e - nil + ).first end return Security.find_or_create_by!(ticker: ticker, exchange_operating_mic: nil) if provider_security.nil? diff --git a/app/models/upgrader/provided.rb b/app/models/upgrader/provided.rb index 4b518e51..fc1e65b7 100644 --- a/app/models/upgrader/provided.rb +++ b/app/models/upgrader/provided.rb @@ -1,11 +1,14 @@ module Upgrader::Provided extend ActiveSupport::Concern - include Providable class_methods do private def fetch_latest_upgrade_candidates_from_provider git_repository_provider.fetch_latest_upgrade_candidates end + + def git_repository_provider + Provider::Github.new + end end end diff --git a/app/views/accounts/_account_sidebar_tabs.html.erb b/app/views/accounts/_account_sidebar_tabs.html.erb index d42fab86..bb409492 100644 --- a/app/views/accounts/_account_sidebar_tabs.html.erb +++ b/app/views/accounts/_account_sidebar_tabs.html.erb @@ -1,5 +1,25 @@ <%# locals: (family:) %> +<% if family.requires_data_provider? && family.synth_client.nil? %> +
    + +
    + <%= icon "triangle-alert", size: "sm" %> +

    Missing historical data

    +
    + + <%= lucide_icon "chevron-down", class: "text-yellow-600 group-open:transform group-open:rotate-180 w-5" %> +
    +
    +

    Maybe uses Synth API to fetch historical exchange rates, security prices, and more. This data is required to calculate accurate historical account balances.

    + +

    + <%= link_to "Add your Synth API key here.", settings_hosting_path, class: "text-yellow-600 underline" %> +

    +
    +
    +<% end %> +
    <%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" } %> - <% if Security.security_prices_provider.nil? %> + <% unless Security.provider %>

    - Note: The Synth provider is not configured. Exchange validation is disabled. - Securities will be created without exchange validation, and price history will not be available. + Note: The security prices provider is not configured. Your trade imports will work, but Maybe will not backfill price history. Please go to your settings to configure this.

    <% end %> diff --git a/app/views/settings/hostings/_synth_settings.html.erb b/app/views/settings/hostings/_synth_settings.html.erb index 9a4e9dd4..4ee9c182 100644 --- a/app/views/settings/hostings/_synth_settings.html.erb +++ b/app/views/settings/hostings/_synth_settings.html.erb @@ -1,7 +1,11 @@

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

    -

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

    + <% if ENV["SYNTH_API_KEY"].present? %> +

    You have successfully configured your Synth API key through the SYNTH_API_KEY environment variable.

    + <% else %> +

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

    + <% end %>
    <%= styled_form_with model: Setting.new, @@ -15,7 +19,8 @@ label: t(".label"), type: "password", placeholder: t(".placeholder"), - value: Setting.synth_api_key, + value: ENV.fetch("SYNTH_API_KEY", Setting.synth_api_key), + disabled: ENV["SYNTH_API_KEY"].present?, container_class: @synth_usage.present? && !@synth_usage.success? ? "border-red-500" : "", data: { "auto-submit-form-target": "auto" } %> <% end %> diff --git a/test/models/exchange_rate_test.rb b/test/models/exchange_rate_test.rb index 1a705a5b..7bc7ba61 100644 --- a/test/models/exchange_rate_test.rb +++ b/test/models/exchange_rate_test.rb @@ -5,14 +5,16 @@ class ExchangeRateTest < ActiveSupport::TestCase setup do @provider = mock - ExchangeRate.stubs(:exchange_rates_provider).returns(@provider) + ExchangeRate.stubs(:provider).returns(@provider) end test "exchange rate provider nil if no api key configured" do - ExchangeRate.unstub(:exchange_rates_provider) + ExchangeRate.unstub(:provider) + + Setting.stubs(:synth_api_key).returns(nil) with_env_overrides SYNTH_API_KEY: nil do - assert_not ExchangeRate.exchange_rates_provider + assert_not ExchangeRate.provider end end @@ -42,7 +44,9 @@ class ExchangeRateTest < ActiveSupport::TestCase end test "nil if rate is not found in DB and provider is disabled" do - ExchangeRate.unstub(:exchange_rates_provider) + ExchangeRate.unstub(:provider) + + Setting.stubs(:synth_api_key).returns(nil) with_env_overrides SYNTH_API_KEY: nil do assert_not ExchangeRate.find_rate(from: "USD", to: "EUR", date: Date.current) @@ -102,7 +106,9 @@ class ExchangeRateTest < ActiveSupport::TestCase end test "returns empty array if no rates found in DB or provider" do - ExchangeRate.unstub(:exchange_rates_provider) + ExchangeRate.unstub(:provider) + + Setting.stubs(:synth_api_key).returns(nil) with_env_overrides SYNTH_API_KEY: nil do assert_equal [], ExchangeRate.find_rates(from: "USD", to: "JPY", start_date: 10.days.ago.to_date) diff --git a/test/models/security/price_test.rb b/test/models/security/price_test.rb index d5021242..66b60469 100644 --- a/test/models/security/price_test.rb +++ b/test/models/security/price_test.rb @@ -5,14 +5,16 @@ class Security::PriceTest < ActiveSupport::TestCase setup do @provider = mock - Security::Price.stubs(:security_prices_provider).returns(@provider) + Security::Price.stubs(:provider).returns(@provider) end test "security price provider nil if no api key provided" do - Security::Price.unstub(:security_prices_provider) + Security::Price.unstub(:provider) + + Setting.stubs(:synth_api_key).returns(nil) with_env_overrides SYNTH_API_KEY: nil do - assert_not Security::Price.security_prices_provider + assert_not Security::Price.provider end end @@ -60,7 +62,10 @@ class Security::PriceTest < ActiveSupport::TestCase end test "returns nil if price not found in DB and provider disabled" do - Security::Price.unstub(:security_prices_provider) + Security::Price.unstub(:provider) + + Setting.stubs(:synth_api_key).returns(nil) + security = Security.new(ticker: "NVDA") with_env_overrides SYNTH_API_KEY: nil do @@ -105,7 +110,9 @@ class Security::PriceTest < ActiveSupport::TestCase end test "returns empty array if no prices found in DB or from provider" do - Security::Price.unstub(:security_prices_provider) + Security::Price.unstub(:provider) + + Setting.stubs(:synth_api_key).returns(nil) with_env_overrides SYNTH_API_KEY: nil do assert_equal [], Security::Price.find_prices(security: Security.new(ticker: "NVDA"), start_date: 10.days.ago.to_date, end_date: Date.current) diff --git a/test/models/trade_import_test.rb b/test/models/trade_import_test.rb index 2900034e..f0ee3e68 100644 --- a/test/models/trade_import_test.rb +++ b/test/models/trade_import_test.rb @@ -12,30 +12,20 @@ class TradeImportTest < ActiveSupport::TestCase # Create an existing AAPL security with no exchange_operating_mic aapl = Security.create!(ticker: "AAPL", exchange_operating_mic: nil) - provider = mock - # We should only hit the provider for GOOGL since AAPL already exists - provider.expects(:search_securities).with( + Security.expects(:search_provider).with( query: "GOOGL", exchange_operating_mic: "XNAS" - ).returns( - OpenStruct.new( - securities: [ - { - ticker: "GOOGL", - name: "Google Inc.", - country_code: "US", - exchange_mic: "XNGS", - exchange_operating_mic: "XNAS", - exchange_acronym: "NGS" - } - ], - success?: true, - raw_response: nil + ).returns([ + Security.new( + ticker: "GOOGL", + name: "Google Inc.", + country_code: "US", + exchange_mic: "XNGS", + exchange_operating_mic: "XNAS", + exchange_acronym: "NGS" ) - ).once - - Security.stubs(:security_prices_provider).returns(provider) + ]).once import = <<~CSV date,ticker,qty,price,currency,account,name,exchange_operating_mic diff --git a/test/system/imports_test.rb b/test/system/imports_test.rb index 5c936f93..85c4707c 100644 --- a/test/system/imports_test.rb +++ b/test/system/imports_test.rb @@ -52,6 +52,8 @@ class ImportsTest < ApplicationSystemTestCase end test "trade import" do + Security.stubs(:search_provider).returns([]) + visit new_import_path click_on "Import investments" diff --git a/test/system/trades_test.rb b/test/system/trades_test.rb index ab0c6109..a1f19139 100644 --- a/test/system/trades_test.rb +++ b/test/system/trades_test.rb @@ -10,7 +10,7 @@ class TradesTest < ApplicationSystemTestCase visit_account_portfolio - Security.stubs(:search).returns([ + Security.stubs(:search_provider).returns([ Security.new( ticker: "AAPL", name: "Apple Inc.", From 7c66f167500f478ea9a092b588c6444f93c7d19b Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 12:17:25 -0500 Subject: [PATCH 12/47] Invert liability graphs to have correct signage (#1928) --- app/models/account/chartable.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/account/chartable.rb b/app/models/account/chartable.rb index aba8415c..2770da3b 100644 --- a/app/models/account/chartable.rb +++ b/app/models/account/chartable.rb @@ -14,6 +14,7 @@ module Account::Chartable ]) balances = gapfill_balances(balances) + balances = invert_balances(balances) if favorable_direction == "down" values = [ nil, *balances ].each_cons(2).map do |prev, curr| Series::Value.new( @@ -69,6 +70,13 @@ module Account::Chartable SQL end + def invert_balances(balances) + balances.map do |balance| + balance.balance = -balance.balance + balance + end + end + def gapfill_balances(balances) gapfilled = [] From 4d0df9b950e214fa615695fb1b9e4328babeb18c Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 12:21:07 -0500 Subject: [PATCH 13/47] Escape quotations in CSV imports properly (#1929) * Parse quotes in imports * Update invalid CSV for test --- app/controllers/import/uploads_controller.rb | 4 +--- app/models/import.rb | 19 +++++++++++++------ test/fixtures/files/imports/invalid.csv | 4 +--- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/app/controllers/import/uploads_controller.rb b/app/controllers/import/uploads_controller.rb index f3c65d6e..8efc7c75 100644 --- a/app/controllers/import/uploads_controller.rb +++ b/app/controllers/import/uploads_controller.rb @@ -29,10 +29,8 @@ class Import::UploadsController < ApplicationController end def csv_valid?(str) - require "csv" - begin - csv = CSV.parse(str || "", headers: true, col_sep: upload_params[:col_sep]) + csv = Import.parse_csv_str(str, col_sep: upload_params[:col_sep]) return false if csv.headers.empty? return false if csv.count == 0 true diff --git a/app/models/import.rb b/app/models/import.rb index 9542e187..9a21d6bd 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -34,6 +34,18 @@ class Import < ApplicationRecord has_many :accounts, dependent: :destroy has_many :entries, dependent: :destroy, class_name: "Account::Entry" + class << self + def parse_csv_str(csv_str, col_sep: ",") + CSV.parse( + (csv_str || "").strip, + headers: true, + col_sep: col_sep, + converters: [ ->(str) { str&.strip } ], + liberal_parsing: true + ) + end + end + def publish_later raise "Import is not publishable" unless publishable? @@ -178,12 +190,7 @@ class Import < ApplicationRecord end def parsed_csv - @parsed_csv ||= CSV.parse( - (raw_file_str || "").strip, - headers: true, - col_sep: col_sep, - converters: [ ->(str) { str&.strip } ] - ) + @parsed_csv ||= self.class.parse_csv_str(raw_file_str, col_sep: col_sep) end def sanitize_number(value) diff --git a/test/fixtures/files/imports/invalid.csv b/test/fixtures/files/imports/invalid.csv index cae4503c..b8552222 100644 --- a/test/fixtures/files/imports/invalid.csv +++ b/test/fixtures/files/imports/invalid.csv @@ -1,3 +1 @@ -name,age -"John Doe,23 -"Jane Doe",25 \ No newline at end of file +name,description,amount,currency \ No newline at end of file From c95bb082a98be370c90fc87a912763950eb59714 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 28 Feb 2025 15:11:41 -0500 Subject: [PATCH 14/47] Bump to v0.4.3 Signed-off-by: Zach Gollwitzer --- config/initializers/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/version.rb b/config/initializers/version.rb index 57f1e0e6..385ca36d 100644 --- a/config/initializers/version.rb +++ b/config/initializers/version.rb @@ -10,7 +10,7 @@ module Maybe private def semver - "0.4.1" + "0.4.3" end end end From 4c4a4026c4ac728802f8a3581ed9919291778662 Mon Sep 17 00:00:00 2001 From: Tony Vincent Date: Mon, 3 Mar 2025 17:34:03 +0100 Subject: [PATCH 15/47] fix: Bug - Transcation Matching Dialog isn't Opening (#1942) --- app/controllers/account/transfer_matches_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/account/transfer_matches_controller.rb b/app/controllers/account/transfer_matches_controller.rb index b686c6c8..851f3ac3 100644 --- a/app/controllers/account/transfer_matches_controller.rb +++ b/app/controllers/account/transfer_matches_controller.rb @@ -3,7 +3,7 @@ class Account::TransferMatchesController < ApplicationController def new @accounts = Current.family.accounts.alphabetically.where.not(id: @entry.account_id) - @transfer_match_candidates = @entry.transfer_match_candidates + @transfer_match_candidates = @entry.account_transaction.transfer_match_candidates end def create From e907b073ed7b78434674983fbecb9c6ca917ac52 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 3 Mar 2025 12:47:20 -0500 Subject: [PATCH 16/47] Fix time period key conflicts (#1944) --- app/controllers/pages_controller.rb | 2 +- app/models/budget.rb | 2 +- app/models/period.rb | 55 ++++++++++++------------- app/views/accounts/chart.html.erb | 2 +- app/views/accounts/show/_chart.html.erb | 2 +- test/models/period_test.rb | 13 +++--- 6 files changed, 37 insertions(+), 39 deletions(-) diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 0b2b26b4..34d6cca3 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -2,7 +2,7 @@ class PagesController < ApplicationController skip_before_action :authenticate_user!, only: %i[early_access] def dashboard - @period = Period.from_key(params[:period], fallback: true) + @period = params[:period] ? Period.from_key(params[:period]) : Period.last_30_days @balance_sheet = Current.family.balance_sheet @accounts = Current.family.accounts.active.with_attached_logo diff --git a/app/models/budget.rb b/app/models/budget.rb index 66a4acb7..c2393da4 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -54,7 +54,7 @@ class Budget < ApplicationRecord end def period - Period.new(start_date: start_date, end_date: end_date) + Period.custom(start_date: start_date, end_date: end_date) end def to_param diff --git a/app/models/period.rb b/app/models/period.rb index 65825058..7e1a8008 100644 --- a/app/models/period.rb +++ b/app/models/period.rb @@ -1,9 +1,12 @@ class Period include ActiveModel::Validations, Comparable - attr_reader :start_date, :end_date + class InvalidKeyError < StandardError; end - validates :start_date, :end_date, presence: true + attr_reader :key, :start_date, :end_date + + validates :start_date, :end_date, presence: true, if: -> { PERIODS[key].nil? } + validates :key, presence: true, if: -> { start_date.nil? || end_date.nil? } validate :must_be_valid_date_range PERIODS = { @@ -64,18 +67,18 @@ class Period } class << self - def default - from_key("last_30_days") + def from_key(key) + unless PERIODS.key?(key) + raise InvalidKeyError, "Invalid period key: #{key}" + end + + start_date, end_date = PERIODS[key].fetch(:date_range) + + new(key: key, start_date: start_date, end_date: end_date) end - def from_key(key, fallback: false) - if PERIODS[key].present? - start_date, end_date = PERIODS[key].fetch(:date_range) - new(start_date: start_date, end_date: end_date) - else - return default if fallback - raise ArgumentError, "Invalid period key: #{key}" - end + def custom(start_date:, end_date:) + new(start_date: start_date, end_date: end_date) end def all @@ -85,12 +88,12 @@ class Period PERIODS.each do |key, period| define_singleton_method(key) do - start_date, end_date = period.fetch(:date_range) - new(start_date: start_date, end_date: end_date) + from_key(key) end end - def initialize(start_date:, end_date:, date_format: "%b %d, %Y") + def initialize(start_date: nil, end_date: nil, key: nil, date_format: "%b %d, %Y") + @key = key @start_date = start_date @end_date = end_date @date_format = date_format @@ -121,37 +124,33 @@ class Period end end - def key - PERIODS.find { |_, period| period.fetch(:date_range) == [ start_date, end_date ] }&.first - end - def label - if known? - PERIODS[key].fetch(:label) + if key_metadata + key_metadata.fetch(:label) else "Custom Period" end end def label_short - if known? - PERIODS[key].fetch(:label_short) + if key_metadata + key_metadata.fetch(:label_short) else - "CP" + "Custom" end end def comparison_label - if known? - PERIODS[key].fetch(:comparison_label) + if key_metadata + key_metadata.fetch(:comparison_label) else "#{start_date.strftime(@date_format)} to #{end_date.strftime(@date_format)}" end end private - def known? - key.present? + def key_metadata + @key_metadata ||= PERIODS[key] end def must_be_valid_date_range diff --git a/app/views/accounts/chart.html.erb b/app/views/accounts/chart.html.erb index 467fd35f..c5cc51eb 100644 --- a/app/views/accounts/chart.html.erb +++ b/app/views/accounts/chart.html.erb @@ -1,4 +1,4 @@ -<% period = Period.from_key(params[:period], fallback: true) %> +<% period = params[:period] ? Period.from_key(params[:period]) : Period.last_30_days %> <% series = @account.balance_series(period: period) %> <% trend = series.trend %> diff --git a/app/views/accounts/show/_chart.html.erb b/app/views/accounts/show/_chart.html.erb index 2b0ee941..d7427762 100644 --- a/app/views/accounts/show/_chart.html.erb +++ b/app/views/accounts/show/_chart.html.erb @@ -1,6 +1,6 @@ <%# locals: (account:, title: nil, tooltip: nil, **args) %> -<% period = Period.from_key(params[:period], fallback: true) %> +<% period = params[:period] ? Period.from_key(params[:period]) : Period.last_30_days %> <% default_value_title = account.asset? ? t(".balance") : t(".owed") %>
    diff --git a/test/models/period_test.rb b/test/models/period_test.rb index 1dfbf91d..eaea02c5 100644 --- a/test/models/period_test.rb +++ b/test/models/period_test.rb @@ -18,20 +18,19 @@ class PeriodTest < ActiveSupport::TestCase assert_includes error.message, "Start date must be before end date" end + test "can create custom period" do + period = Period.new(start_date: Date.current - 15.days, end_date: Date.current) + assert_equal "Custom Period", period.label + end + test "from_key returns period for valid key" do period = Period.from_key("last_30_days") assert_equal 30.days.ago.to_date, period.start_date assert_equal Date.current, period.end_date end - test "from_key with invalid key and fallback returns default period" do - period = Period.from_key("invalid_key", fallback: true) - assert_equal 30.days.ago.to_date, period.start_date - assert_equal Date.current, period.end_date - end - test "from_key with invalid key and no fallback raises error" do - assert_raises ArgumentError do + error = assert_raises(Period::InvalidKeyError) do Period.from_key("invalid_key") end end From c5da8ea5502f2732d01fcafebe43c0502fc22614 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 3 Mar 2025 12:47:30 -0500 Subject: [PATCH 17/47] Allow CSV imports to be configured with single or multi-account mode (#1943) * Allow CSV imports to be configured to a single account or multiple accounts * Initialize import directly from accounts page * Fix brakeman warnings * Fix schema * Fix Synth check --- app/controllers/import/confirms_controller.rb | 4 ++ app/controllers/import/uploads_controller.rb | 1 + app/controllers/imports_controller.rb | 3 +- app/models/family.rb | 2 +- app/models/import.rb | 13 ++++-- app/models/import/account_mapping.rb | 7 +++- app/models/trade_import.rb | 33 ++++++++++----- app/models/transaction_import.rb | 21 +++++++--- app/views/account/trades/_form.html.erb | 8 ++-- app/views/accounts/show/_menu.html.erb | 4 +- .../configurations/_mint_import.html.erb | 7 +++- .../configurations/_trade_import.html.erb | 6 ++- .../_transaction_import.html.erb | 5 ++- app/views/import/confirms/_mappings.html.erb | 2 +- app/views/import/uploads/show.html.erb | 42 +++++++++---------- app/views/imports/_import.html.erb | 4 ++ app/views/imports/_nav.html.erb | 2 +- ...3141007_add_optional_account_for_import.rb | 5 +++ db/schema.rb | 4 +- .../import/rows_controller_test.rb | 2 +- 20 files changed, 118 insertions(+), 57 deletions(-) create mode 100644 db/migrate/20250303141007_add_optional_account_for_import.rb diff --git a/app/controllers/import/confirms_controller.rb b/app/controllers/import/confirms_controller.rb index 0c1a8872..1a687d4b 100644 --- a/app/controllers/import/confirms_controller.rb +++ b/app/controllers/import/confirms_controller.rb @@ -4,6 +4,10 @@ class Import::ConfirmsController < ApplicationController before_action :set_import def show + if @import.mapping_steps.empty? + return redirect_to import_path(@import) + end + redirect_to import_clean_path(@import), alert: "You have invalid data, please edit until all errors are resolved" unless @import.cleaned? end diff --git a/app/controllers/import/uploads_controller.rb b/app/controllers/import/uploads_controller.rb index 8efc7c75..42e6c975 100644 --- a/app/controllers/import/uploads_controller.rb +++ b/app/controllers/import/uploads_controller.rb @@ -8,6 +8,7 @@ class Import::UploadsController < ApplicationController def update if csv_valid?(csv_str) + @import.account = Current.family.accounts.find_by(id: params.dig(:import, :account_id)) @import.assign_attributes(raw_file_str: csv_str, col_sep: upload_params[:col_sep]) @import.save!(validate: false) diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index 3fb704fe..e95e6e11 100644 --- a/app/controllers/imports_controller.rb +++ b/app/controllers/imports_controller.rb @@ -18,7 +18,8 @@ class ImportsController < ApplicationController end def create - import = Current.family.imports.create! import_params + account = Current.family.accounts.find_by(id: params.dig(:import, :account_id)) + import = Current.family.imports.create!(type: import_params[:type], account: account) redirect_to import_upload_path(import) end diff --git a/app/models/family.rb b/app/models/family.rb index ffb14a7d..0f71731f 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -106,7 +106,7 @@ class Family < ApplicationRecord # If family has any entries in different currencies, they need a provider for historical exchange rates uniq_currencies = entries.pluck(:currency).uniq return true if uniq_currencies.count > 1 - return true if uniq_currencies.first != self.currency + return true if uniq_currencies.count > 0 && uniq_currencies.first != self.currency false end diff --git a/app/models/import.rb b/app/models/import.rb index 9a21d6bd..600f30a7 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -1,6 +1,7 @@ class Import < ApplicationRecord TYPES = %w[TransactionImport TradeImport AccountImport MintImport].freeze SIGNAGE_CONVENTIONS = %w[inflows_positive inflows_negative] + SEPARATORS = [ [ "Comma (,)", "," ], [ "Semicolon (;)", ";" ] ].freeze NUMBER_FORMATS = { "1,234.56" => { separator: ".", delimiter: "," }, # US/UK/Asia @@ -10,6 +11,7 @@ class Import < ApplicationRecord }.freeze belongs_to :family + belongs_to :account, optional: true before_validation :set_default_number_format @@ -25,7 +27,7 @@ class Import < ApplicationRecord }, validate: true, default: "pending" validates :type, inclusion: { in: TYPES } - validates :col_sep, inclusion: { in: [ ",", ";" ] } + validates :col_sep, inclusion: { in: SEPARATORS.map(&:last) } validates :signage_convention, inclusion: { in: SIGNAGE_CONVENTIONS } validates :number_format, presence: true, inclusion: { in: NUMBER_FORMATS.keys } @@ -98,12 +100,17 @@ class Import < ApplicationRecord end def dry_run - { + mappings = { transactions: rows.count, - accounts: Import::AccountMapping.for_import(self).creational.count, categories: Import::CategoryMapping.for_import(self).creational.count, tags: Import::TagMapping.for_import(self).creational.count } + + mappings.merge( + accounts: Import::AccountMapping.for_import(self).creational.count, + ) if account.nil? + + mappings end def required_column_keys diff --git a/app/models/import/account_mapping.rb b/app/models/import/account_mapping.rb index c4c00414..9cf43f7a 100644 --- a/app/models/import/account_mapping.rb +++ b/app/models/import/account_mapping.rb @@ -1,5 +1,5 @@ class Import::AccountMapping < Import::Mapping - validates :mappable, presence: true, if: -> { key.blank? || !create_when_empty } + validates :mappable, presence: true, if: :requires_mapping? class << self def mapping_values(import) @@ -42,4 +42,9 @@ class Import::AccountMapping < Import::Mapping self.mappable = account save! end + + private + def requires_mapping? + (key.blank? || !create_when_empty) && import.account.nil? + end end diff --git a/app/models/trade_import.rb b/app/models/trade_import.rb index 549c9093..4bbffdaa 100644 --- a/app/models/trade_import.rb +++ b/app/models/trade_import.rb @@ -4,7 +4,11 @@ class TradeImport < Import mappings.each(&:create_mappable!) rows.each do |row| - account = mappings.accounts.mappable_for(row.account) + mapped_account = if account + account + else + mappings.accounts.mappable_for(row.account) + end # Try to find or create security with ticker only security = find_or_create_security( @@ -12,15 +16,15 @@ class TradeImport < Import exchange_operating_mic: row.exchange_operating_mic ) - entry = account.entries.build \ + entry = mapped_account.entries.build \ date: row.date_iso, amount: row.signed_amount, name: row.name, - currency: row.currency.presence || account.currency, + currency: row.currency.presence || mapped_account.currency, entryable: Account::Trade.new( security: security, qty: row.qty, - currency: row.currency.presence || account.currency, + currency: row.currency.presence || mapped_account.currency, price: row.price ), import: self @@ -31,7 +35,9 @@ class TradeImport < Import end def mapping_steps - [ Import::AccountMapping ] + base = [] + base << Import::AccountMapping if account.nil? + base end def required_column_keys @@ -39,14 +45,19 @@ class TradeImport < Import end def column_keys - %i[date ticker exchange_operating_mic currency qty price account name] + base = %i[date ticker exchange_operating_mic currency qty price name] + base.unshift(:account) if account.nil? + base end def dry_run - { - transactions: rows.count, + mappings = { transactions: rows.count } + + mappings.merge( accounts: Import::AccountMapping.for_import(self).creational.count - } + ) if account.nil? + + mappings end def csv_template @@ -57,7 +68,9 @@ class TradeImport < Import 05/17/2024,TSLA,XNAS,USD,2,700.50,Retirement Account,Tesla Inc. Purchase CSV - CSV.parse(template, headers: true) + csv = CSV.parse(template, headers: true) + csv.delete("account") if account.present? + csv end private diff --git a/app/models/transaction_import.rb b/app/models/transaction_import.rb index e2b1c3d6..2bb9d4d5 100644 --- a/app/models/transaction_import.rb +++ b/app/models/transaction_import.rb @@ -4,11 +4,16 @@ class TransactionImport < Import mappings.each(&:create_mappable!) rows.each do |row| - account = mappings.accounts.mappable_for(row.account) + mapped_account = if account + account + else + mappings.accounts.mappable_for(row.account) + end + category = mappings.categories.mappable_for(row.category) tags = row.tags_list.map { |tag| mappings.tags.mappable_for(tag) }.compact - entry = account.entries.build \ + entry = mapped_account.entries.build \ date: row.date_iso, amount: row.signed_amount, name: row.name, @@ -27,11 +32,15 @@ class TransactionImport < Import end def column_keys - %i[date amount name currency category tags account notes] + base = %i[date amount name currency category tags notes] + base.unshift(:account) if account.nil? + base end def mapping_steps - [ Import::CategoryMapping, Import::TagMapping, Import::AccountMapping ] + base = [ Import::CategoryMapping, Import::TagMapping ] + base << Import::AccountMapping if account.nil? + base end def csv_template @@ -42,6 +51,8 @@ class TransactionImport < Import 05/17/2024,-12.50,Coffee Shop,,,coffee,, CSV - CSV.parse(template, headers: true) + csv = CSV.parse(template, headers: true) + csv.delete("account") if account.present? + csv end end diff --git a/app/views/account/trades/_form.html.erb b/app/views/account/trades/_form.html.erb index a74d401f..89c8c151 100644 --- a/app/views/account/trades/_form.html.erb +++ b/app/views/account/trades/_form.html.erb @@ -29,11 +29,11 @@ <% if %w[buy sell].include?(type) %> <% if Security.provider.present? %>
    - <%= form.combobox :ticker, - securities_path(country_code: Current.family.country), + <%= form.combobox :ticker, + securities_path(country_code: Current.family.country), name_when_new: "account_entry[manual_ticker]", - label: t(".holding"), - placeholder: t(".ticker_placeholder"), + label: t(".holding"), + placeholder: t(".ticker_placeholder"), required: true %>
    <% else %> diff --git a/app/views/accounts/show/_menu.html.erb b/app/views/accounts/show/_menu.html.erb index f41723a6..57241528 100644 --- a/app/views/accounts/show/_menu.html.erb +++ b/app/views/accounts/show/_menu.html.erb @@ -20,8 +20,8 @@ <% end %> <% unless account.crypto? %> - <%= link_to new_import_path, - data: { turbo_frame: :modal }, + <%= button_to imports_path({ import: { type: account.investment? ? "TradeImport" : "TransactionImport", account_id: account.id } }), + data: { turbo_frame: :_top }, class: "block w-full py-2 px-3 space-x-2 text-primary hover:bg-gray-50 flex items-center rounded-lg" do %> <%= lucide_icon "download", class: "w-5 h-5 text-secondary" %> diff --git a/app/views/import/configurations/_mint_import.html.erb b/app/views/import/configurations/_mint_import.html.erb index 1aabce75..987f2fed 100644 --- a/app/views/import/configurations/_mint_import.html.erb +++ b/app/views/import/configurations/_mint_import.html.erb @@ -1,6 +1,6 @@ <%# locals: (import:) %> -
    +
    <%= lucide_icon("check-circle", class: "w-5 h-5 shrink-0 text-green-500") %>

    We have pre-configured your Mint import for you. Please proceed to the next step.

    @@ -21,7 +21,10 @@ <%= form.select :number_format, Import::NUMBER_FORMATS.keys, { label: "Format", prompt: "Select format" }, required: true %>
    - <%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" }, disabled: import.complete? %> + <% unless import.account.present? %> + <%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" }, disabled: import.complete? %> + <% end %> + <%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" }, disabled: import.complete? %> <%= form.select :category_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Category (optional)" }, disabled: import.complete? %> <%= form.select :tags_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Tags (optional)" }, disabled: import.complete? %> diff --git a/app/views/import/configurations/_trade_import.html.erb b/app/views/import/configurations/_trade_import.html.erb index f923940c..7db04ba9 100644 --- a/app/views/import/configurations/_trade_import.html.erb +++ b/app/views/import/configurations/_trade_import.html.erb @@ -20,7 +20,11 @@ <%= form.select :ticker_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Ticker" } %> <%= form.select :exchange_operating_mic_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Exchange Operating MIC" } %> <%= form.select :price_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Price" } %> - <%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" } %> + + <% unless import.account.present? %> + <%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" } %> + <% end %> + <%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" } %> <% unless Security.provider %> diff --git a/app/views/import/configurations/_transaction_import.html.erb b/app/views/import/configurations/_transaction_import.html.erb index 65c9f5f4..a75b1681 100644 --- a/app/views/import/configurations/_transaction_import.html.erb +++ b/app/views/import/configurations/_transaction_import.html.erb @@ -16,7 +16,10 @@ <%= form.select :number_format, Import::NUMBER_FORMATS.keys, { label: "Format", prompt: "Select format" }, required: true %>
    - <%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" } %> + <% unless import.account.present? %> + <%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" } %> + <% end %> + <%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" } %> <%= form.select :category_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Category (optional)" } %> <%= form.select :tags_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Tags (optional)" } %> diff --git a/app/views/import/confirms/_mappings.html.erb b/app/views/import/confirms/_mappings.html.erb index 2880ec6d..0f07eb2f 100644 --- a/app/views/import/confirms/_mappings.html.erb +++ b/app/views/import/confirms/_mappings.html.erb @@ -3,7 +3,7 @@ <% mappings = mapping_class.for_import(import) %> <% is_last_step = step_idx == import.mapping_steps.count - 1 %> -<% if mapping_class == Import::AccountMapping %> +<% if mapping_class == Import::AccountMapping && import.account.nil? %> <% if import.requires_account? %>
    <%= tag.p t(".no_accounts"), class: "text-sm" %> diff --git a/app/views/import/uploads/show.html.erb b/app/views/import/uploads/show.html.erb index 2f126a1c..ab7e4bb2 100644 --- a/app/views/import/uploads/show.html.erb +++ b/app/views/import/uploads/show.html.erb @@ -19,33 +19,33 @@
    -
    - <%= styled_form_with model: @import, scope: :import, url: import_upload_path(@import), multipart: true, class: "space-y-2" do |form| %> - <%= form.select :col_sep, [["Comma (,)", ","], ["Semicolon (;)", ";"]], label: true %> - <%= form.text_area :raw_file_str, + <% ["csv-paste-tab", "csv-upload-tab"].each do |tab| %> + <%= tag.div id: tab, data: { tabs_target: "tab" }, class: tab == "csv-upload-tab" ? "hidden" : "" do %> + <%= styled_form_with model: @import, scope: :import, url: import_upload_path(@import), multipart: true, class: "space-y-2" do |form| %> + <%= form.select :col_sep, Import::SEPARATORS, label: true %> + + <% unless @import.type == "MintImport" %> + <%= form.select :account_id, @import.family.accounts.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %> + <% end %> + + <% if tab == "csv-paste-tab" %> + <%= form.text_area :raw_file_str, rows: 10, required: true, placeholder: "Paste your CSV file contents here", "data-auto-submit-form-target": "auto" %> + <% else %> + + <% end %> - <%= form.submit "Upload CSV", disabled: @import.complete? %> + <%= form.submit "Upload CSV", disabled: @import.complete? %> + <% end %> <% end %> - -
    - - + <% end %>
    diff --git a/app/views/imports/_import.html.erb b/app/views/imports/_import.html.erb index 3ee7d68c..26c77ee9 100644 --- a/app/views/imports/_import.html.erb +++ b/app/views/imports/_import.html.erb @@ -2,6 +2,10 @@
    <%= link_to import_path(import), class: "text-sm text-primary hover:underline" do %> + <% if import.account.present? %> + <%= import.account.name + " " %> + <% end %> + <%= t(".label", type: import.type.titleize, datetime: import.updated_at.strftime("%b %-d, %Y at %l:%M %p")) %> <% end %> diff --git a/app/views/imports/_nav.html.erb b/app/views/imports/_nav.html.erb index b68b6ce1..0b9bd817 100644 --- a/app/views/imports/_nav.html.erb +++ b/app/views/imports/_nav.html.erb @@ -6,7 +6,7 @@ { name: "Clean", path: import_clean_path(import), is_complete: import.cleaned?, step_number: 3 }, { name: "Map", path: import_confirm_path(import), is_complete: import.publishable?, step_number: 4 }, { name: "Confirm", path: import_path(import), is_complete: import.complete?, step_number: 5 } -] %> +].reject { |step| step[:name] == "Map" && import.mapping_steps.empty? } %>
      <% steps.each_with_index do |step, idx| %> diff --git a/db/migrate/20250303141007_add_optional_account_for_import.rb b/db/migrate/20250303141007_add_optional_account_for_import.rb new file mode 100644 index 00000000..52da03d9 --- /dev/null +++ b/db/migrate/20250303141007_add_optional_account_for_import.rb @@ -0,0 +1,5 @@ +class AddOptionalAccountForImport < ActiveRecord::Migration[7.2] + def change + rename_column :imports, :original_account_id, :account_id + end +end diff --git a/db/schema.rb b/db/schema.rb index d78271ac..492edb1e 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_02_20_200735) do +ActiveRecord::Schema[7.2].define(version: 2025_03_03_141007) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -398,7 +398,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_20_200735) do t.datetime "updated_at", null: false t.string "col_sep", default: "," t.uuid "family_id", null: false - t.uuid "original_account_id" + t.uuid "account_id" t.string "type", null: false t.string "date_col_label", default: "date" t.string "amount_col_label", default: "amount" diff --git a/test/controllers/import/rows_controller_test.rb b/test/controllers/import/rows_controller_test.rb index d16fd143..e6394d94 100644 --- a/test/controllers/import/rows_controller_test.rb +++ b/test/controllers/import/rows_controller_test.rb @@ -22,7 +22,7 @@ class Import::RowsControllerTest < ActionDispatch::IntegrationTest get import_row_path(import, row) - assert_row_fields(row, [ :date, :ticker, :qty, :price, :currency, :account, :name ]) + assert_row_fields(row, [ :date, :ticker, :qty, :price, :currency, :account, :name, :account ]) assert_response :success end From 4e96ca83762b0c69a220ccb34f1d0a4331cde5dc Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 3 Mar 2025 14:34:56 -0500 Subject: [PATCH 18/47] Add manual Docker publishing trigger in GH action workflow --- .github/workflows/publish.yml | 9 +++++++++ Dockerfile | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 86ba83cf..ef6dbe94 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -1,6 +1,13 @@ name: Publish Docker image on: + workflow_dispatch: + inputs: + ref: + description: 'Git ref (tag or commit SHA) to build' + required: true + type: string + default: 'main' push: tags: - 'v*' @@ -33,6 +40,8 @@ jobs: steps: - name: Check out the repo uses: actions/checkout@v4 + with: + ref: ${{ github.event.inputs.ref || github.ref }} - name: Set up QEMU uses: docker/setup-qemu-action@v3 diff --git a/Dockerfile b/Dockerfile index aeb8aaa0..ff2d8d30 100644 --- a/Dockerfile +++ b/Dockerfile @@ -21,7 +21,7 @@ ENV RAILS_ENV="production" \ FROM base AS build # Install packages needed to build gems -RUN apt-get install --no-install-recommends -y build-essential libpq-dev pkg-config +RUN apt-get install --no-install-recommends -y build-essential libpq-dev git pkg-config # Install application gems COPY .ruby-version Gemfile Gemfile.lock ./ From cf0e57353382d2315e959d811f77989400fdb08e Mon Sep 17 00:00:00 2001 From: Bryan McKnight Date: Mon, 3 Mar 2025 15:37:12 -0600 Subject: [PATCH 19/47] Fix modal closing on color picker drag #1869 (#1931) * Replaced data-action click event with data-action mousedown to prevent the modal from hiding on mouse up whenever mouse down starts within the modal * Changed click events to mousedown within dialog elements to trigger the closing of the element --- app/views/account/transactions/bulk_edit.html.erb | 4 ++-- app/views/shared/_drawer.html.erb | 4 ++-- app/views/shared/_modal.html.erb | 2 +- app/views/shared/_modal_form.html.erb | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/account/transactions/bulk_edit.html.erb b/app/views/account/transactions/bulk_edit.html.erb index b8317084..ed9a6194 100644 --- a/app/views/account/transactions/bulk_edit.html.erb +++ b/app/views/account/transactions/bulk_edit.html.erb @@ -1,12 +1,12 @@ <%= turbo_frame_tag "bulk_transaction_edit_drawer" do %> <%= styled_form_with url: bulk_update_account_transactions_path, scope: "bulk_update", class: "h-full", data: { turbo_frame: "_top" } do |form| %>
      -
      +
      <%= lucide_icon("x", class: "w-5 h-5 shrink-0") %>
      diff --git a/app/views/shared/_drawer.html.erb b/app/views/shared/_drawer.html.erb index 35e225c2..4da7ccb0 100644 --- a/app/views/shared/_drawer.html.erb +++ b/app/views/shared/_drawer.html.erb @@ -3,11 +3,11 @@ <%= turbo_frame_tag "drawer" do %>
      -
      +
      <%= lucide_icon("x", class: "w-5 h-5 shrink-0") %>
      diff --git a/app/views/shared/_modal.html.erb b/app/views/shared/_modal.html.erb index dc0c5e90..2eb1b693 100644 --- a/app/views/shared/_modal.html.erb +++ b/app/views/shared/_modal.html.erb @@ -1,6 +1,6 @@ <%# locals: (content:, classes:) -%> <%= turbo_frame_tag "modal" do %> - +
      <%= content %>
      diff --git a/app/views/shared/_modal_form.html.erb b/app/views/shared/_modal_form.html.erb index a3d5456b..98d9b598 100644 --- a/app/views/shared/_modal_form.html.erb +++ b/app/views/shared/_modal_form.html.erb @@ -5,7 +5,7 @@

      <%= title %>

      - <%= lucide_icon("x", class: "cursor-pointer w-5 h-5 text-secondary", data: { action: "click->modal#close" }) %> + <%= lucide_icon("x", class: "cursor-pointer w-5 h-5 text-secondary", data: { action: "mousedown->modal#close" }) %>
      <% if subtitle.present? %> From 5b2fa3d707d790caf338f20b003350efc3145780 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 4 Mar 2025 07:50:21 -0500 Subject: [PATCH 20/47] Fix commit resolution for Docker builds --- .github/workflows/publish.yml | 1 + Dockerfile | 8 +++++--- config/initializers/version.rb | 6 +++++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index ef6dbe94..062d11b9 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -82,3 +82,4 @@ jobs: provenance: false # https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry#adding-a-description-to-multi-arch-images outputs: type=image,name=target,annotation-index.org.opencontainers.image.description=A multi-arch Docker image for the Maybe Rails app + build-args: BUILD_COMMIT_SHA=${{ github.sha }} diff --git a/Dockerfile b/Dockerfile index ff2d8d30..d98833b3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,14 +9,16 @@ WORKDIR /rails # Install base packages RUN apt-get update -qq && \ - apt-get install --no-install-recommends -y curl libvips postgresql-client git + apt-get install --no-install-recommends -y curl libvips postgresql-client # Set production environment +ARG BUILD_COMMIT_SHA ENV RAILS_ENV="production" \ BUNDLE_DEPLOYMENT="1" \ BUNDLE_PATH="/usr/local/bundle" \ - BUNDLE_WITHOUT="development" - + BUNDLE_WITHOUT="development" \ + BUILD_COMMIT_SHA=${BUILD_COMMIT_SHA} + # Throw-away build stage to reduce size of final image FROM base AS build diff --git a/config/initializers/version.rb b/config/initializers/version.rb index 385ca36d..40a27790 100644 --- a/config/initializers/version.rb +++ b/config/initializers/version.rb @@ -5,7 +5,11 @@ module Maybe end def commit_sha - `git rev-parse HEAD`.chomp + if Rails.env.production? + ENV["BUILD_COMMIT_SHA"] + else + `git rev-parse HEAD`.chomp + end end private From 0544089710d941da058bfdd4a062f2f42b318941 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 4 Mar 2025 13:10:01 -0500 Subject: [PATCH 21/47] Account-level import configuration templates (#1946) * Account-level import configuration templates * Default import to family's preferred date format --- app/controllers/import/rows_controller.rb | 4 +- app/controllers/import/uploads_controller.rb | 2 +- app/controllers/imports_controller.rb | 17 ++++++++- app/models/import.rb | 38 ++++++++++++++++++- app/models/import/account_mapping.rb | 7 +++- app/models/import/account_type_mapping.rb | 4 +- app/models/import/category_mapping.rb | 7 +++- app/models/import/mapping.rb | 15 +------- app/models/import/row.rb | 9 ++--- app/models/import/tag_mapping.rb | 8 +++- app/views/import/configurations/show.html.erb | 25 ++++++++++-- app/views/import/uploads/show.html.erb | 2 +- app/views/shared/_notification.html.erb | 2 +- config/routes.rb | 7 +++- .../import/uploads_controller_test.rb | 4 +- 15 files changed, 108 insertions(+), 43 deletions(-) diff --git a/app/controllers/import/rows_controller.rb b/app/controllers/import/rows_controller.rb index b5b9092c..a3905b14 100644 --- a/app/controllers/import/rows_controller.rb +++ b/app/controllers/import/rows_controller.rb @@ -2,9 +2,7 @@ class Import::RowsController < ApplicationController before_action :set_import_row def update - @row.assign_attributes(row_params) - @row.save!(validate: false) - @row.sync_mappings + @row.update_and_sync(row_params) redirect_to import_row_path(@row.import, @row) end diff --git a/app/controllers/import/uploads_controller.rb b/app/controllers/import/uploads_controller.rb index 42e6c975..d30e0082 100644 --- a/app/controllers/import/uploads_controller.rb +++ b/app/controllers/import/uploads_controller.rb @@ -12,7 +12,7 @@ class Import::UploadsController < ApplicationController @import.assign_attributes(raw_file_str: csv_str, col_sep: upload_params[:col_sep]) @import.save!(validate: false) - redirect_to import_configuration_path(@import), notice: "CSV uploaded successfully." + redirect_to import_configuration_path(@import, template_hint: true), notice: "CSV uploaded successfully." else flash.now[:alert] = "Must be valid CSV with headers and at least one row of data" diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index e95e6e11..c1b51c23 100644 --- a/app/controllers/imports_controller.rb +++ b/app/controllers/imports_controller.rb @@ -1,5 +1,5 @@ class ImportsController < ApplicationController - before_action :set_import, only: %i[show publish destroy revert] + before_action :set_import, only: %i[show publish destroy revert apply_template] def publish @import.publish_later @@ -19,7 +19,11 @@ class ImportsController < ApplicationController def create account = Current.family.accounts.find_by(id: params.dig(:import, :account_id)) - import = Current.family.imports.create!(type: import_params[:type], account: account) + import = Current.family.imports.create!( + type: import_params[:type], + account: account, + date_format: Current.family.date_format, + ) redirect_to import_upload_path(import) end @@ -37,6 +41,15 @@ class ImportsController < ApplicationController redirect_to imports_path, notice: "Import is reverting in the background." end + def apply_template + if @import.suggested_template + @import.apply_template!(@import.suggested_template) + redirect_to import_configuration_path(@import), notice: "Template applied." + else + redirect_to import_configuration_path(@import), alert: "No template found, please manually configure your import." + end + end + def destroy @import.destroy diff --git a/app/models/import.rb b/app/models/import.rb index 600f30a7..662b4cee 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -146,8 +146,20 @@ class Import < ApplicationRecord end def sync_mappings - mapping_steps.each do |mapping| - mapping.sync(self) + transaction do + mapping_steps.each do |mapping_class| + mappables_by_key = mapping_class.mappables_by_key(self) + + updated_mappings = mappables_by_key.map do |key, mappable| + mapping = mappings.find_or_initialize_by(key: key, import: self, type: mapping_class.name) + mapping.mappable = mappable + mapping.create_when_empty = key.present? && mappable.nil? + mapping + end + + updated_mappings.each { |m| m.save(validate: false) } + mapping_class.where.not(id: updated_mappings.map(&:id)).destroy_all + end end end @@ -183,6 +195,28 @@ class Import < ApplicationRecord family.accounts.empty? && has_unassigned_account? end + # Used to optionally pre-fill the configuration for the current import + def suggested_template + family.imports + .complete + .where(account: account, type: type) + .order(created_at: :desc) + .first + end + + def apply_template!(import_template) + update!( + import_template.attributes.slice( + "date_col_label", "amount_col_label", "name_col_label", + "category_col_label", "tags_col_label", "account_col_label", + "qty_col_label", "ticker_col_label", "price_col_label", + "entity_type_col_label", "notes_col_label", "currency_col_label", + "date_format", "signage_convention", "number_format", + "exchange_operating_mic_col_label" + ) + ) + end + private def import! # no-op, subclasses can implement for customization of algorithm diff --git a/app/models/import/account_mapping.rb b/app/models/import/account_mapping.rb index 9cf43f7a..67280da4 100644 --- a/app/models/import/account_mapping.rb +++ b/app/models/import/account_mapping.rb @@ -2,8 +2,11 @@ class Import::AccountMapping < Import::Mapping validates :mappable, presence: true, if: :requires_mapping? class << self - def mapping_values(import) - import.rows.map(&:account).uniq + def mappables_by_key(import) + unique_values = import.rows.map(&:account).uniq + accounts = import.family.accounts.where(name: unique_values).index_by(&:name) + + unique_values.index_with { |value| accounts[value] } end end diff --git a/app/models/import/account_type_mapping.rb b/app/models/import/account_type_mapping.rb index 8b60edcd..2d4b5431 100644 --- a/app/models/import/account_type_mapping.rb +++ b/app/models/import/account_type_mapping.rb @@ -2,8 +2,8 @@ class Import::AccountTypeMapping < Import::Mapping validates :value, presence: true class << self - def mapping_values(import) - import.rows.map(&:entity_type).uniq + def mappables_by_key(import) + import.rows.map(&:entity_type).uniq.index_with { nil } end end diff --git a/app/models/import/category_mapping.rb b/app/models/import/category_mapping.rb index 12302603..4b633ea4 100644 --- a/app/models/import/category_mapping.rb +++ b/app/models/import/category_mapping.rb @@ -1,7 +1,10 @@ class Import::CategoryMapping < Import::Mapping class << self - def mapping_values(import) - import.rows.map(&:category).uniq + def mappables_by_key(import) + unique_values = import.rows.map(&:category).uniq + categories = import.family.categories.where(name: unique_values).index_by(&:name) + + unique_values.index_with { |value| categories[value] } end end diff --git a/app/models/import/mapping.rb b/app/models/import/mapping.rb index a0a4bc8b..b783ef95 100644 --- a/app/models/import/mapping.rb +++ b/app/models/import/mapping.rb @@ -18,19 +18,8 @@ class Import::Mapping < ApplicationRecord find_by(key: key)&.mappable end - def sync(import) - unique_values = mapping_values(import).uniq - - unique_values.each do |value| - mapping = find_or_initialize_by(key: value, import: import, create_when_empty: value.present?) - mapping.save(validate: false) if mapping.new_record? - end - - where(import: import).where.not(key: unique_values).destroy_all - end - - def mapping_values(import) - raise NotImplementedError, "Subclass must implement mapping_values" + def mappables_by_key(import) + raise NotImplementedError, "Subclass must implement mappables_by_key" end end diff --git a/app/models/import/row.rb b/app/models/import/row.rb index d4316a60..622a9d0a 100644 --- a/app/models/import/row.rb +++ b/app/models/import/row.rb @@ -30,11 +30,10 @@ class Import::Row < ApplicationRecord end end - def sync_mappings - Import::CategoryMapping.sync(import) if import.column_keys.include?(:category) - Import::TagMapping.sync(import) if import.column_keys.include?(:tags) - Import::AccountMapping.sync(import) if import.column_keys.include?(:account) - Import::AccountTypeMapping.sync(import) if import.column_keys.include?(:entity_type) + def update_and_sync(params) + assign_attributes(params) + save!(validate: false) + import.sync_mappings end private diff --git a/app/models/import/tag_mapping.rb b/app/models/import/tag_mapping.rb index 899b4dc5..4b9a1be0 100644 --- a/app/models/import/tag_mapping.rb +++ b/app/models/import/tag_mapping.rb @@ -1,7 +1,11 @@ class Import::TagMapping < Import::Mapping class << self - def mapping_values(import) - import.rows.map(&:tags_list).flatten.uniq + def mappables_by_key(import) + unique_values = import.rows.map(&:tags_list).flatten.uniq + + tags = import.family.tags.where(name: unique_values).index_by(&:name) + + unique_values.index_with { |value| tags[value] } end end diff --git a/app/views/import/configurations/show.html.erb b/app/views/import/configurations/show.html.erb index a807c97e..aaefd4e6 100644 --- a/app/views/import/configurations/show.html.erb +++ b/app/views/import/configurations/show.html.erb @@ -4,14 +4,33 @@ <%= content_for :previous_path, import_upload_path(@import) %> -
      +<% if @import.suggested_template.present? && params[:template_hint] == "true" %> +
      +
      +

      + + <%= icon "sparkles" %> + + + Template configuration found +

      + +

      We found a configuration from a previous import for this account. Would you like to apply it to this import?

      + +
      + <%= link_to "Manually configure", import_configuration_path(@import), class: "btn btn--outline" %> + <%= button_to "Apply template", apply_template_import_path(@import), class: "btn btn--primary", method: :put, data: { turbo_frame: :_top } %> +
      +
      +
      +<% else %>

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

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

      -
      +
      <%= render partial: permitted_import_configuration_path(@import), locals: { import: @import } %>
      @@ -19,4 +38,4 @@
      <%= render "imports/table", headers: @import.csv_headers, rows: @import.csv_sample, caption: "Sample data from your uploaded CSV" %>
      -
      +<% end %> diff --git a/app/views/import/uploads/show.html.erb b/app/views/import/uploads/show.html.erb index ab7e4bb2..fdcf787a 100644 --- a/app/views/import/uploads/show.html.erb +++ b/app/views/import/uploads/show.html.erb @@ -24,7 +24,7 @@ <%= styled_form_with model: @import, scope: :import, url: import_upload_path(@import), multipart: true, class: "space-y-2" do |form| %> <%= form.select :col_sep, Import::SEPARATORS, label: true %> - <% unless @import.type == "MintImport" %> + <% if @import.type == "TransactionImport" || @import.type == "TradeImport" %> <%= form.select :account_id, @import.family.accounts.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %> <% end %> diff --git a/app/views/shared/_notification.html.erb b/app/views/shared/_notification.html.erb index 3e01536d..fbf4e2c9 100644 --- a/app/views/shared/_notification.html.erb +++ b/app/views/shared/_notification.html.erb @@ -16,7 +16,7 @@ <%= lucide_icon "check", class: "w-3 h-3" %>
      <% when :alert %> -
      +
      <%= lucide_icon "x", class: "w-3 h-3" %>
      <% end %> diff --git a/config/routes.rb b/config/routes.rb index 30d3303e..c714a3ef 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -68,8 +68,11 @@ Rails.application.routes.draw do resources :transfers, only: %i[new create destroy show update] resources :imports, only: %i[index new show create destroy] do - post :publish, on: :member - put :revert, on: :member + member do + post :publish + put :revert + put :apply_template + end resource :upload, only: %i[show update], module: :import resource :configuration, only: %i[show update], module: :import diff --git a/test/controllers/import/uploads_controller_test.rb b/test/controllers/import/uploads_controller_test.rb index eb7f418b..647815d4 100644 --- a/test/controllers/import/uploads_controller_test.rb +++ b/test/controllers/import/uploads_controller_test.rb @@ -19,7 +19,7 @@ class Import::UploadsControllerTest < ActionDispatch::IntegrationTest } } - assert_redirected_to import_configuration_url(@import) + assert_redirected_to import_configuration_url(@import, template_hint: true) assert_equal "CSV uploaded successfully.", flash[:notice] end @@ -31,7 +31,7 @@ class Import::UploadsControllerTest < ActionDispatch::IntegrationTest } } - assert_redirected_to import_configuration_url(@import) + assert_redirected_to import_configuration_url(@import, template_hint: true) assert_equal "CSV uploaded successfully.", flash[:notice] end From cf59fe45e7798cacacd3ffe913a5dfca921a0c9b Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 5 Mar 2025 09:30:47 -0500 Subject: [PATCH 22/47] Fix ticker filling when Synth is connected (#1950) --- app/controllers/securities_controller.rb | 5 +---- app/models/security/provided.rb | 2 ++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/controllers/securities_controller.rb b/app/controllers/securities_controller.rb index 6cfe9fd0..2c4124cf 100644 --- a/app/controllers/securities_controller.rb +++ b/app/controllers/securities_controller.rb @@ -1,10 +1,7 @@ class SecuritiesController < ApplicationController def index - query = params[:q] - return render json: [] if query.blank? || query.length < 2 || query.length > 100 - @securities = Security.search_provider({ - search: query, + search: params[:q], country: params[:country_code] == "US" ? "US" : nil }) end diff --git a/app/models/security/provided.rb b/app/models/security/provided.rb index 4a4fd6a5..c7e38fb5 100644 --- a/app/models/security/provided.rb +++ b/app/models/security/provided.rb @@ -9,6 +9,8 @@ module Security::Provided end def search_provider(query) + return [] if query[:search].blank? || query[:search].length < 2 + response = provider.search_securities( query: query[:search], dataset: "limited", From d66c37939acf6056c0d1eb6a57377874da10d158 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 5 Mar 2025 10:07:29 -0500 Subject: [PATCH 23/47] Update bug_report.md Signed-off-by: Zach Gollwitzer --- .github/ISSUE_TEMPLATE/bug_report.md | 51 +++++++++++++++++++++------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 203ba646..d7f8526c 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,32 +1,59 @@ --- name: Bug report -about: Create a report to help us improve +about: Open a bug report when you experience broken functionality within the latest version of the Maybe app title: 'Bug: [Add descriptive title here]' labels: '' assignees: '' --- -**Where did this bug occur? (required)** +## Before you start (required) -- [ ] I am a self-hosted user reporting a bug from my self hosted app - - [ ] I have verified that I am running the **latest** version of the Maybe app (your app should be running [this version](https://github.com/maybe-finance/maybe/pkgs/container/maybe) before opening a bug) -- [ ] I am a user of Maybe's paid app +### General checklist -_Please note, if you are reporting a bug with sensitive data, please open an Intercom chat from within the app for help_ +- [ ] I have removed personal / sensitive data from screenshots and logs +- [ ] I have searched [existing issues](https://github.com/maybe-finance/maybe/issues?q=is:issue) and [discussions](https://github.com/maybe-finance/maybe/discussions) to ensure this is not a duplicate issue + +### How are you using Maybe? + +- [ ] I am a paying Maybe customer (hosted version) +- [ ] I am a self-hosted user + +### Self hoster checklist + +_Paying, hosted users should delete this entire section._ + +If you are a self-hosted user, please complete all of the information below. Issues with incomplete information will be marked as `Needs Info` to help our small team prioritize bug fixes. + +- Self hosted app commit SHA (find in user menu): [enter commit sha here] + - [ ] I have confirmed that my app's commit is the latest version of Maybe +- Where are you hosting? + - [ ] Render + - [ ] Docker Compose + - [ ] Umbrel + - [ ] Other (please specify) + +--- + +## Bug description -**Describe the bug** A clear and concise description of what the bug is. -**To Reproduce** +### To Reproduce + +Be as specific as possible so Maybe maintainers can quickly reproduce the bug you're experiencing. + Steps to reproduce the behavior: + 1. Go to '...' 2. Click on '....' 3. Scroll down to '....' 4. See error -**Expected behavior** -A clear and concise description of what you expected to happen. +### Expected behavior -**Screenshots / Recordings** -If applicable, add screenshots or short video recordings to help show the bug in more detail. +What is the intended behavior that you would expect? + +### Screenshots and/or recordings + +We highly recommend providing additional context with screenshots and/or screen recordings. This will _significantly_ improve the chances of the bug being addressed and fixed quickly. From 8d0509fda03d5fb9f4ee2b7b01979c8779f38ffb Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 5 Mar 2025 10:15:12 -0500 Subject: [PATCH 24/47] Conditionally show commit sha Fixes #1951 --- app/views/users/_user_menu.html.erb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/views/users/_user_menu.html.erb b/app/views/users/_user_menu.html.erb index c8997be2..38e078a9 100644 --- a/app/views/users/_user_menu.html.erb +++ b/app/views/users/_user_menu.html.erb @@ -26,7 +26,10 @@

      Version: <%= link_to Maybe.version.to_release_tag, "https://github.com/maybe-finance/maybe/releases/tag/#{Maybe.version.to_release_tag}", target: "_blank", class: "hover:underline" %> - (<%= link_to Maybe.commit_sha.first(7), "https://github.com/maybe-finance/maybe/commit/#{Maybe.commit_sha}", target: "_blank", class: "hover:underline" %>) + + <% if Maybe.commit_sha.present? %> + (<%= link_to Maybe.commit_sha.first(7), "https://github.com/maybe-finance/maybe/commit/#{Maybe.commit_sha}", target: "_blank", class: "hover:underline" %>) + <% end %>

      <% end %> From e384369cfbb42e6f9f6cfe8cdb68245965b10683 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 5 Mar 2025 10:20:02 -0500 Subject: [PATCH 25/47] Adjust graph intervals to show more data Fixes #1948 --- app/models/period.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/period.rb b/app/models/period.rb index 7e1a8008..85ad2947 100644 --- a/app/models/period.rb +++ b/app/models/period.rb @@ -117,8 +117,8 @@ class Period end def interval - if days > 90 - "1 month" + if days > 366 + "1 week" else "1 day" end From eaa1b6abe02c97e8a25aae9393d349b07ee1dfcc Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 5 Mar 2025 11:01:07 -0500 Subject: [PATCH 26/47] Update issue templates --- .github/ISSUE_TEMPLATE/bug_report.md | 4 +++- .github/ISSUE_TEMPLATE/other.md | 35 ++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index d7f8526c..4f1db63d 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,6 +1,7 @@ --- name: Bug report -about: Open a bug report when you experience broken functionality within the latest version of the Maybe app +about: Open a bug report when you experience broken functionality within the latest + version of the Maybe app title: 'Bug: [Add descriptive title here]' labels: '' assignees: '' @@ -17,6 +18,7 @@ assignees: '' ### How are you using Maybe? - [ ] I am a paying Maybe customer (hosted version) + - Paying Maybe users can also open requests in Intercom (if there is sensitive info involved) - [ ] I am a self-hosted user ### Self hoster checklist diff --git a/.github/ISSUE_TEMPLATE/other.md b/.github/ISSUE_TEMPLATE/other.md index 6a230e6e..9e0c9148 100644 --- a/.github/ISSUE_TEMPLATE/other.md +++ b/.github/ISSUE_TEMPLATE/other.md @@ -7,15 +7,36 @@ assignees: '' --- -**PLEASE READ before opening an issue:** +## Before you start (required) -- Is this a feature request? Please [open a feature request discussion](https://github.com/maybe-finance/maybe/discussions/new?category=feature-requests). -- Do you need help or have a question? Please [open a discussion](https://github.com/maybe-finance/maybe/discussions/new/choose) or [join our Discord](https://link.maybe.co/discord) and post to the "help" channel. +### Is this a bug? ----------------------- +A bug is _broken functionality_ of the app (i.e. it prevents you from using the app). For bugs, please use the ["Bug Report" template](https://github.com/maybe-finance/maybe/issues) instead. -**Is this issue related to a problem? Please describe.** +### Is this a bug with _sensitive info_? -**Describe the work that needs to be done to address this issue** +If you are a _paying_ Maybe user, you can open a support request in Intercom. -**Additional context** +### Is this a feature request? + +A feature request is functionality that you would like that is not already on our [Roadmap](https://github.com/maybe-finance/maybe/wiki/Roadmap). + +All feature requests should be opened as Discussions here: + +https://github.com/maybe-finance/maybe/discussions/categories/feature-requests + +Be sure to search existing discussions prior to opening a new feature request. + +### Is this related to Docker and/or hosting for self hosting? + +If you are having a Docker configuration issue, please do not open a Github issue unless you've identified a bug in our Dockerfile. To get help with self hosting, there are several options: + +- **First**: Read our [self hosting guides](https://github.com/maybe-finance/maybe/tree/main/docs/hosting) and follow them step-by-step +- Open a [General Discussion](https://github.com/maybe-finance/maybe/discussions/categories/general) +- Make a post in the "Self hosted" channel in our [Discord](https://link.maybe.co/discord) + +--- + +## Issue description + +If your issue does not fall into the categories above, please provide a **descriptive and complete** overview of your issue. From 381e39bea8b4ebb768a8cda6c9d63e486904c959 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 5 Mar 2025 12:21:17 -0500 Subject: [PATCH 27/47] Fix: Purge stale holdings from accounts during sync (#1954) * Fix: Purge stale holdings from accounts during sync * Fix typo * Prevent Plaid holding deletions --- .../account/holdings_controller.rb | 9 +++-- app/controllers/plaid_items_controller.rb | 2 +- app/models/account/syncer.rb | 35 +++++++++++++------ app/views/account/holdings/index.html.erb | 8 ++--- app/views/account/holdings/show.html.erb | 30 ++++++++-------- test/models/account/syncer_test.rb | 2 ++ 6 files changed, 54 insertions(+), 32 deletions(-) diff --git a/app/controllers/account/holdings_controller.rb b/app/controllers/account/holdings_controller.rb index bfda09fb..9ded4165 100644 --- a/app/controllers/account/holdings_controller.rb +++ b/app/controllers/account/holdings_controller.rb @@ -9,9 +9,12 @@ class Account::HoldingsController < ApplicationController end def destroy - @holding.destroy_holding_and_entries! - - flash[:notice] = t(".success") + if @holding.account.plaid_account_id.present? + flash[:alert] = "You cannot delete this holding" + else + @holding.destroy_holding_and_entries! + flash[:notice] = t(".success") + end respond_to do |format| format.html { redirect_back_or_to account_path(@holding.account) } diff --git a/app/controllers/plaid_items_controller.rb b/app/controllers/plaid_items_controller.rb index 406da016..37efd5e3 100644 --- a/app/controllers/plaid_items_controller.rb +++ b/app/controllers/plaid_items_controller.rb @@ -22,7 +22,7 @@ class PlaidItemsController < ApplicationController end respond_to do |format| - format.html { redirect_to accounts_path } + format.html { redirect_back_or_to accounts_path } format.json { head :ok } end end diff --git a/app/models/account/syncer.rb b/app/models/account/syncer.rb index 8aeb0ba0..cd664e66 100644 --- a/app/models/account/syncer.rb +++ b/app/models/account/syncer.rb @@ -10,11 +10,11 @@ class Account::Syncer holdings = sync_holdings balances = sync_balances(holdings) account.reload - update_account_info(balances, holdings) unless account.plaid_account_id.present? + update_account_info(balances, holdings) unless plaid_sync? convert_records_to_family_currency(balances, holdings) unless account.currency == account.family.currency # Enrich if user opted in or if we're syncing transactions from a Plaid account on the hosted app - if account.family.data_enrichment_enabled? || (account.plaid_account_id.present? && Rails.application.config.app_mode.hosted?) + if account.family.data_enrichment_enabled? || (plaid_sync? && Rails.application.config.app_mode.hosted?) account.enrich_data else Rails.logger.info("Data enrichment is disabled, skipping enrichment for account #{account.id}") @@ -41,15 +41,13 @@ class Account::Syncer def sync_holdings calculator = Account::HoldingCalculator.new(account) - calculated_holdings = calculator.calculate(reverse: account.plaid_account_id.present?) + calculated_holdings = calculator.calculate(reverse: plaid_sync?) current_time = Time.now Account.transaction do load_holdings(calculated_holdings) - - # Purge outdated holdings - account.holdings.delete_by("date < ? OR security_id NOT IN (?)", account_start_date, calculated_holdings.map(&:security_id)) + purge_outdated_holdings unless plaid_sync? end calculated_holdings @@ -57,13 +55,11 @@ class Account::Syncer def sync_balances(holdings) calculator = Account::BalanceCalculator.new(account, holdings: holdings) - calculated_balances = calculator.calculate(reverse: account.plaid_account_id.present?, start_date: start_date) + calculated_balances = calculator.calculate(reverse: plaid_sync?, start_date: start_date) Account.transaction do load_balances(calculated_balances) - - # Purge outdated balances - account.balances.delete_by("date < ?", account_start_date) + purge_outdated_balances end calculated_balances @@ -131,4 +127,23 @@ class Account::Syncer unique_by: %i[account_id security_id date currency] ) end + + def purge_outdated_balances + account.balances.delete_by("date < ?", account_start_date) + end + + def plaid_sync? + account.plaid_account_id.present? + end + + def purge_outdated_holdings + portfolio_security_ids = account.entries.account_trades.map { |entry| entry.entryable.security_id }.uniq + + # If there are no securities in the portfolio, delete all holdings + if portfolio_security_ids.empty? + account.holdings.delete_all + else + account.holdings.delete_by("date < ? OR security_id NOT IN (?)", account_start_date, portfolio_security_ids) + end + end end diff --git a/app/views/account/holdings/index.html.erb b/app/views/account/holdings/index.html.erb index a4dfeb4d..6b80ab59 100644 --- a/app/views/account/holdings/index.html.erb +++ b/app/views/account/holdings/index.html.erb @@ -21,12 +21,12 @@
      + <%= render "account/holdings/cash", account: @account %> + + <%= render "account/holdings/ruler" %> + <% if @account.current_holdings.any? %> - <%= render "account/holdings/cash", account: @account %> - <%= render "account/holdings/ruler" %> <%= render partial: "account/holdings/holding", collection: @account.current_holdings, spacer_template: "ruler" %> - <% else %> -

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

      <% end %>
      diff --git a/app/views/account/holdings/show.html.erb b/app/views/account/holdings/show.html.erb index 1dd09244..b783ac5b 100644 --- a/app/views/account/holdings/show.html.erb +++ b/app/views/account/holdings/show.html.erb @@ -87,26 +87,28 @@
      -
      - -

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

      - <%= lucide_icon "chevron-down", class: "group-open:transform group-open:rotate-180 text-secondary w-5" %> -
      + <% unless @holding.account.plaid_account_id.present? %> +
      + +

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

      + <%= lucide_icon "chevron-down", class: "group-open:transform group-open:rotate-180 text-secondary w-5" %> +
      -
      -
      -
      -

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

      -

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

      -
      +
      +
      +
      +

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

      +

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

      +
      - <%= button_to t(".delete"), + <%= button_to t(".delete"), account_holding_path(@holding), method: :delete, class: "rounded-lg px-3 py-2 text-red-500 text-sm font-medium border border-secondary", data: { turbo_confirm: true } %> +
      -
      -
      +
      + <% end %>
      <% end %> diff --git a/test/models/account/syncer_test.rb b/test/models/account/syncer_test.rb index 261ab28c..5cc85ee9 100644 --- a/test/models/account/syncer_test.rb +++ b/test/models/account/syncer_test.rb @@ -17,6 +17,8 @@ class Account::SyncerTest < ActiveSupport::TestCase @account.family.update! currency: "USD" @account.update! currency: "EUR" + @account.entries.create!(date: 1.day.ago.to_date, currency: "EUR", amount: 500, name: "Buy AAPL", entryable: Account::Trade.new(security: securities(:aapl), qty: 10, price: 50, currency: "EUR")) + ExchangeRate.create!(date: 1.day.ago.to_date, from_currency: "EUR", to_currency: "USD", rate: 1.2) ExchangeRate.create!(date: Date.current, from_currency: "EUR", to_currency: "USD", rate: 2) From 071ad52c7fb2a7e4d22640991bd39ec863c0bd50 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Wed, 5 Mar 2025 13:04:45 -0600 Subject: [PATCH 28/47] Potential fix for MFA login issues --- app/controllers/mfa_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/mfa_controller.rb b/app/controllers/mfa_controller.rb index ec3071e5..da161476 100644 --- a/app/controllers/mfa_controller.rb +++ b/app/controllers/mfa_controller.rb @@ -29,7 +29,8 @@ class MfaController < ApplicationController if @user&.verify_otp?(params[:code]) session.delete(:mfa_user_id) @session = create_session_for(@user) - redirect_to root_path + Rails.logger.info "MFA verification successful for user #{@user.id}. Session created: #{@session.id}" + redirect_to root_path, turbo: false else flash.now[:alert] = t(".invalid_code") render :verify, status: :unprocessable_entity From e49bda4a2e2edbf7a65c5eb5f17beed8f4292b08 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Wed, 5 Mar 2025 13:10:53 -0600 Subject: [PATCH 29/47] Another attempt at fixing MFA issues --- app/controllers/concerns/authentication.rb | 14 ++++++++++++-- app/controllers/mfa_controller.rb | 9 +++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 9dc23942..69cc666d 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -28,12 +28,22 @@ module Authentication end def find_session_by_cookie - Session.find_by(id: cookies.signed[:session_token]) + cookie_value = cookies.signed[:session_token] + Rails.logger.info "Looking for session with cookie value: #{cookie_value.present? ? 'present' : 'missing'}" + session = Session.find_by(id: cookie_value) + Rails.logger.info "Session found: #{session.present? ? 'yes' : 'no'}" + session end def create_session_for(user) session = user.sessions.create! - cookies.signed.permanent[:session_token] = { value: session.id, httponly: true } + Rails.logger.info "Setting session cookie with value: #{session.id}" + # Explicitly set SameSite attribute and ensure cookie is set properly + cookies.signed.permanent[:session_token] = { + value: session.id, + httponly: true, + same_site: :lax + } session end diff --git a/app/controllers/mfa_controller.rb b/app/controllers/mfa_controller.rb index da161476..ea8d388c 100644 --- a/app/controllers/mfa_controller.rb +++ b/app/controllers/mfa_controller.rb @@ -30,6 +30,15 @@ class MfaController < ApplicationController session.delete(:mfa_user_id) @session = create_session_for(@user) Rails.logger.info "MFA verification successful for user #{@user.id}. Session created: #{@session.id}" + + # Explicitly set the cookie again to ensure it's properly set + cookies.signed.permanent[:session_token] = { + value: @session.id, + httponly: true, + same_site: :lax + } + + # Use turbo: false to ensure a full page reload redirect_to root_path, turbo: false else flash.now[:alert] = t(".invalid_code") From 28bfcda50af28667eb296703e43905133ab57878 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Wed, 5 Mar 2025 13:20:36 -0600 Subject: [PATCH 30/47] Temporary additional logging to continue debugging MFA issues --- app/controllers/concerns/authentication.rb | 43 ++++++++++++----- app/controllers/mfa_controller.rb | 54 ++++++++++++++++------ app/controllers/sessions_controller.rb | 10 ++++ 3 files changed, 81 insertions(+), 26 deletions(-) diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 69cc666d..fcba1483 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -16,12 +16,19 @@ module Authentication private def authenticate_user! + Rails.logger.info "Authentication#authenticate_user! - Checking for session cookie" + if session_record = find_session_by_cookie + Rails.logger.info "Authentication#authenticate_user! - Found valid session: #{session_record.id} for user: #{session_record.user_id}" Current.session = session_record else + Rails.logger.info "Authentication#authenticate_user! - No valid session found" + if self_hosted_first_login? + Rails.logger.info "Authentication#authenticate_user! - Self-hosted first login detected, redirecting to registration" redirect_to new_registration_url else + Rails.logger.info "Authentication#authenticate_user! - Redirecting to login page" redirect_to new_session_url end end @@ -29,21 +36,35 @@ module Authentication def find_session_by_cookie cookie_value = cookies.signed[:session_token] - Rails.logger.info "Looking for session with cookie value: #{cookie_value.present? ? 'present' : 'missing'}" - session = Session.find_by(id: cookie_value) - Rails.logger.info "Session found: #{session.present? ? 'yes' : 'no'}" - session + Rails.logger.info "Authentication#find_session_by_cookie - Looking for session with cookie value: #{cookie_value.present? ? 'present' : 'missing'}" + + if cookie_value.present? + session = Session.find_by(id: cookie_value) + Rails.logger.info "Authentication#find_session_by_cookie - Session found: #{session.present? ? 'yes' : 'no'}" + + if session.present? + Rails.logger.info "Authentication#find_session_by_cookie - Session belongs to user: #{session.user_id}" + end + + session + else + Rails.logger.info "Authentication#find_session_by_cookie - No session cookie found" + nil + end end def create_session_for(user) + Rails.logger.info "Authentication#create_session_for - Creating session for user: #{user.id}" session = user.sessions.create! - Rails.logger.info "Setting session cookie with value: #{session.id}" - # Explicitly set SameSite attribute and ensure cookie is set properly - cookies.signed.permanent[:session_token] = { - value: session.id, - httponly: true, - same_site: :lax - } + Rails.logger.info "Authentication#create_session_for - Session created with ID: #{session.id}" + + Rails.logger.info "Authentication#create_session_for - Setting session cookie" + cookies.signed.permanent[:session_token] = { value: session.id, httponly: true } + + Rails.logger.info "Authentication#create_session_for - Cookie set, verifying..." + cookie_value = cookies.signed[:session_token] + Rails.logger.info "Authentication#create_session_for - Cookie verification: #{cookie_value == session.id ? 'successful' : 'failed'}" + session end diff --git a/app/controllers/mfa_controller.rb b/app/controllers/mfa_controller.rb index ea8d388c..925a03ef 100644 --- a/app/controllers/mfa_controller.rb +++ b/app/controllers/mfa_controller.rb @@ -3,50 +3,74 @@ class MfaController < ApplicationController skip_authentication only: [ :verify, :verify_code ] def new + Rails.logger.info "MfaController#new - User: #{Current.user.id} accessing MFA setup" redirect_to root_path if Current.user.otp_required? Current.user.setup_mfa! unless Current.user.otp_secret.present? end def create + Rails.logger.info "MfaController#create - User: #{Current.user.id} attempting to enable MFA" if Current.user.verify_otp?(params[:code]) + Rails.logger.info "MfaController#create - MFA verification successful for user: #{Current.user.id}" Current.user.enable_mfa! @backup_codes = Current.user.otp_backup_codes + Rails.logger.info "MfaController#create - Generated backup codes for user: #{Current.user.id}" render :backup_codes else + Rails.logger.info "MfaController#create - MFA verification failed for user: #{Current.user.id}" Current.user.disable_mfa! redirect_to new_mfa_path, alert: t(".invalid_code") end end def verify + Rails.logger.info "MfaController#verify - Attempting to verify MFA for user_id from session: #{session[:mfa_user_id]}" @user = User.find_by(id: session[:mfa_user_id]) - redirect_to new_session_path unless @user + + if @user + Rails.logger.info "MfaController#verify - Found user: #{@user.id} for MFA verification" + else + Rails.logger.info "MfaController#verify - No user found for MFA verification, redirecting to login" + redirect_to new_session_path + end end def verify_code + Rails.logger.info "MfaController#verify_code - Attempting to verify MFA code for user_id from session: #{session[:mfa_user_id]}" @user = User.find_by(id: session[:mfa_user_id]) - if @user&.verify_otp?(params[:code]) - session.delete(:mfa_user_id) - @session = create_session_for(@user) - Rails.logger.info "MFA verification successful for user #{@user.id}. Session created: #{@session.id}" - - # Explicitly set the cookie again to ensure it's properly set - cookies.signed.permanent[:session_token] = { - value: @session.id, - httponly: true, - same_site: :lax - } - - # Use turbo: false to ensure a full page reload - redirect_to root_path, turbo: false + if @user + Rails.logger.info "MfaController#verify_code - Found user: #{@user.id} for MFA verification" else + Rails.logger.info "MfaController#verify_code - No user found for MFA verification" + end + + if @user&.verify_otp?(params[:code]) + Rails.logger.info "MfaController#verify_code - MFA code verification successful for user: #{@user.id}" + session.delete(:mfa_user_id) + Rails.logger.info "MfaController#verify_code - Deleted mfa_user_id from session" + + @session = create_session_for(@user) + Rails.logger.info "MfaController#verify_code - Created session: #{@session.id} for user: #{@user.id}" + + # Log cookie information + Rails.logger.info "MfaController#verify_code - Cookie details:" + Rails.logger.info " - session_token present: #{cookies.signed[:session_token].present?}" + Rails.logger.info " - session_token value: #{cookies.signed[:session_token]}" + Rails.logger.info " - all cookies: #{cookies.to_h.keys.join(', ')}" + + # Simply redirect to root path with data-turbo="false" + Rails.logger.info "MfaController#verify_code - Redirecting to root_path with data-turbo=false" + redirect_to root_path, data: { turbo: false } + else + Rails.logger.info "MfaController#verify_code - MFA code verification failed for user: #{@user&.id}" flash.now[:alert] = t(".invalid_code") render :verify, status: :unprocessable_entity end end def disable + Rails.logger.info "MfaController#disable - User: #{Current.user.id} disabling MFA" Current.user.disable_mfa! redirect_to settings_security_path, notice: t(".success") end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 3b7357f8..88c788b9 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -5,24 +5,34 @@ class SessionsController < ApplicationController layout "auth" def new + Rails.logger.info "SessionsController#new - Rendering login form" end def create + Rails.logger.info "SessionsController#create - Attempting to authenticate user with email: #{params[:email]}" + if user = User.authenticate_by(email: params[:email], password: params[:password]) + Rails.logger.info "SessionsController#create - Authentication successful for user: #{user.id}" + if user.otp_required? + Rails.logger.info "SessionsController#create - MFA required for user: #{user.id}, redirecting to MFA verification" session[:mfa_user_id] = user.id redirect_to verify_mfa_path else + Rails.logger.info "SessionsController#create - MFA not required for user: #{user.id}, creating session" @session = create_session_for(user) + Rails.logger.info "SessionsController#create - Session created: #{@session.id}, redirecting to root_path" redirect_to root_path end else + Rails.logger.info "SessionsController#create - Authentication failed for email: #{params[:email]}" flash.now[:alert] = t(".invalid_credentials") render :new, status: :unprocessable_entity end end def destroy + Rails.logger.info "SessionsController#destroy - Destroying session: #{@session.id} for user: #{Current.user.id}" @session.destroy redirect_to new_session_path, notice: t(".logout_successful") end From f7fa8fa08510210fe8df92d6e8d6e95dbc500d0a Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Wed, 5 Mar 2025 13:32:53 -0600 Subject: [PATCH 31/47] Disable turbo on login forms --- app/views/mfa/verify.html.erb | 2 +- app/views/sessions/new.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/mfa/verify.html.erb b/app/views/mfa/verify.html.erb index 67476e5a..05fb8b9e 100644 --- a/app/views/mfa/verify.html.erb +++ b/app/views/mfa/verify.html.erb @@ -3,7 +3,7 @@ header_description t(".description") %> -<%= styled_form_with url: verify_mfa_path, method: :post, class: "space-y-4" do |form| %> +<%= styled_form_with url: verify_mfa_path, method: :post, class: "space-y-4", data: { turbo: false } do |form| %> <%= form.text_field :code, required: true, autofocus: true, diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 174a76d3..2f5228e4 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -2,7 +2,7 @@ header_title t(".title") %> -<%= styled_form_with url: sessions_path, class: "space-y-4" do |form| %> +<%= styled_form_with url: sessions_path, class: "space-y-4", data: { turbo: false } do |form| %> <%= form.email_field :email, label: t(".email"), autofocus: false, autocomplete: "email", required: "required", placeholder: t(".email_placeholder") %> <%= form.password_field :password, label: t(".password"), required: "required" %> From cffafd23f0cfefb1d0929b13ac75be5d94661027 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Wed, 5 Mar 2025 13:44:56 -0600 Subject: [PATCH 32/47] Logger cleanup --- app/controllers/concerns/authentication.rb | 27 +---------------- app/controllers/mfa_controller.rb | 35 ++-------------------- app/controllers/sessions_controller.rb | 10 ------- 3 files changed, 3 insertions(+), 69 deletions(-) diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index fcba1483..28758d9d 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -16,19 +16,12 @@ module Authentication private def authenticate_user! - Rails.logger.info "Authentication#authenticate_user! - Checking for session cookie" - if session_record = find_session_by_cookie - Rails.logger.info "Authentication#authenticate_user! - Found valid session: #{session_record.id} for user: #{session_record.user_id}" Current.session = session_record else - Rails.logger.info "Authentication#authenticate_user! - No valid session found" - if self_hosted_first_login? - Rails.logger.info "Authentication#authenticate_user! - Self-hosted first login detected, redirecting to registration" redirect_to new_registration_url else - Rails.logger.info "Authentication#authenticate_user! - Redirecting to login page" redirect_to new_session_url end end @@ -36,35 +29,17 @@ module Authentication def find_session_by_cookie cookie_value = cookies.signed[:session_token] - Rails.logger.info "Authentication#find_session_by_cookie - Looking for session with cookie value: #{cookie_value.present? ? 'present' : 'missing'}" if cookie_value.present? - session = Session.find_by(id: cookie_value) - Rails.logger.info "Authentication#find_session_by_cookie - Session found: #{session.present? ? 'yes' : 'no'}" - - if session.present? - Rails.logger.info "Authentication#find_session_by_cookie - Session belongs to user: #{session.user_id}" - end - - session + Session.find_by(id: cookie_value) else - Rails.logger.info "Authentication#find_session_by_cookie - No session cookie found" nil end end def create_session_for(user) - Rails.logger.info "Authentication#create_session_for - Creating session for user: #{user.id}" session = user.sessions.create! - Rails.logger.info "Authentication#create_session_for - Session created with ID: #{session.id}" - - Rails.logger.info "Authentication#create_session_for - Setting session cookie" cookies.signed.permanent[:session_token] = { value: session.id, httponly: true } - - Rails.logger.info "Authentication#create_session_for - Cookie set, verifying..." - cookie_value = cookies.signed[:session_token] - Rails.logger.info "Authentication#create_session_for - Cookie verification: #{cookie_value == session.id ? 'successful' : 'failed'}" - session end diff --git a/app/controllers/mfa_controller.rb b/app/controllers/mfa_controller.rb index 925a03ef..5d1fef0b 100644 --- a/app/controllers/mfa_controller.rb +++ b/app/controllers/mfa_controller.rb @@ -3,74 +3,43 @@ class MfaController < ApplicationController skip_authentication only: [ :verify, :verify_code ] def new - Rails.logger.info "MfaController#new - User: #{Current.user.id} accessing MFA setup" redirect_to root_path if Current.user.otp_required? Current.user.setup_mfa! unless Current.user.otp_secret.present? end def create - Rails.logger.info "MfaController#create - User: #{Current.user.id} attempting to enable MFA" if Current.user.verify_otp?(params[:code]) - Rails.logger.info "MfaController#create - MFA verification successful for user: #{Current.user.id}" Current.user.enable_mfa! @backup_codes = Current.user.otp_backup_codes - Rails.logger.info "MfaController#create - Generated backup codes for user: #{Current.user.id}" render :backup_codes else - Rails.logger.info "MfaController#create - MFA verification failed for user: #{Current.user.id}" Current.user.disable_mfa! redirect_to new_mfa_path, alert: t(".invalid_code") end end def verify - Rails.logger.info "MfaController#verify - Attempting to verify MFA for user_id from session: #{session[:mfa_user_id]}" @user = User.find_by(id: session[:mfa_user_id]) - if @user - Rails.logger.info "MfaController#verify - Found user: #{@user.id} for MFA verification" - else - Rails.logger.info "MfaController#verify - No user found for MFA verification, redirecting to login" + if @user.nil? redirect_to new_session_path end end def verify_code - Rails.logger.info "MfaController#verify_code - Attempting to verify MFA code for user_id from session: #{session[:mfa_user_id]}" @user = User.find_by(id: session[:mfa_user_id]) - if @user - Rails.logger.info "MfaController#verify_code - Found user: #{@user.id} for MFA verification" - else - Rails.logger.info "MfaController#verify_code - No user found for MFA verification" - end - if @user&.verify_otp?(params[:code]) - Rails.logger.info "MfaController#verify_code - MFA code verification successful for user: #{@user.id}" session.delete(:mfa_user_id) - Rails.logger.info "MfaController#verify_code - Deleted mfa_user_id from session" - @session = create_session_for(@user) - Rails.logger.info "MfaController#verify_code - Created session: #{@session.id} for user: #{@user.id}" - - # Log cookie information - Rails.logger.info "MfaController#verify_code - Cookie details:" - Rails.logger.info " - session_token present: #{cookies.signed[:session_token].present?}" - Rails.logger.info " - session_token value: #{cookies.signed[:session_token]}" - Rails.logger.info " - all cookies: #{cookies.to_h.keys.join(', ')}" - - # Simply redirect to root path with data-turbo="false" - Rails.logger.info "MfaController#verify_code - Redirecting to root_path with data-turbo=false" - redirect_to root_path, data: { turbo: false } + redirect_to root_path else - Rails.logger.info "MfaController#verify_code - MFA code verification failed for user: #{@user&.id}" flash.now[:alert] = t(".invalid_code") render :verify, status: :unprocessable_entity end end def disable - Rails.logger.info "MfaController#disable - User: #{Current.user.id} disabling MFA" Current.user.disable_mfa! redirect_to settings_security_path, notice: t(".success") end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 88c788b9..3b7357f8 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -5,34 +5,24 @@ class SessionsController < ApplicationController layout "auth" def new - Rails.logger.info "SessionsController#new - Rendering login form" end def create - Rails.logger.info "SessionsController#create - Attempting to authenticate user with email: #{params[:email]}" - if user = User.authenticate_by(email: params[:email], password: params[:password]) - Rails.logger.info "SessionsController#create - Authentication successful for user: #{user.id}" - if user.otp_required? - Rails.logger.info "SessionsController#create - MFA required for user: #{user.id}, redirecting to MFA verification" session[:mfa_user_id] = user.id redirect_to verify_mfa_path else - Rails.logger.info "SessionsController#create - MFA not required for user: #{user.id}, creating session" @session = create_session_for(user) - Rails.logger.info "SessionsController#create - Session created: #{@session.id}, redirecting to root_path" redirect_to root_path end else - Rails.logger.info "SessionsController#create - Authentication failed for email: #{params[:email]}" flash.now[:alert] = t(".invalid_credentials") render :new, status: :unprocessable_entity end end def destroy - Rails.logger.info "SessionsController#destroy - Destroying session: #{@session.id} for user: #{Current.user.id}" @session.destroy redirect_to new_session_path, notice: t(".logout_successful") end From 9627a6bf6fb69e5f54df5ab24c6b67596f6666ed Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 5 Mar 2025 15:38:31 -0500 Subject: [PATCH 33/47] Add tagged logging to sync process (#1956) * Add tagged logging to sync process * Reduce logging in syncer * Typo --- app/models/account/balance_calculator.rb | 15 +++++---- app/models/account/holding_calculator.rb | 11 +++++-- app/models/account/syncer.rb | 39 ++++++++++++++++-------- app/models/plaid_item.rb | 20 ++++++++++-- app/models/sync.rb | 33 ++++++++++++-------- config/environments/production.rb | 7 +++-- 6 files changed, 86 insertions(+), 39 deletions(-) diff --git a/app/models/account/balance_calculator.rb b/app/models/account/balance_calculator.rb index d5b3b3d1..ca292dc5 100644 --- a/app/models/account/balance_calculator.rb +++ b/app/models/account/balance_calculator.rb @@ -5,13 +5,16 @@ class Account::BalanceCalculator end def calculate(reverse: false, start_date: nil) - cash_balances = reverse ? reverse_cash_balances : forward_cash_balances + Rails.logger.tagged("Account::BalanceCalculator") do + Rails.logger.info("Calculating cash balances with strategy: #{reverse ? "reverse sync" : "forward sync"}") + cash_balances = reverse ? reverse_cash_balances : forward_cash_balances - cash_balances.map do |balance| - holdings_value = converted_holdings.select { |h| h.date == balance.date }.sum(&:amount) - balance.balance = balance.balance + holdings_value - balance - end.compact + cash_balances.map do |balance| + holdings_value = converted_holdings.select { |h| h.date == balance.date }.sum(&:amount) + balance.balance = balance.balance + holdings_value + balance + end.compact + end end private diff --git a/app/models/account/holding_calculator.rb b/app/models/account/holding_calculator.rb index c9327daa..edb55acf 100644 --- a/app/models/account/holding_calculator.rb +++ b/app/models/account/holding_calculator.rb @@ -5,9 +5,14 @@ class Account::HoldingCalculator end def calculate(reverse: false) - preload_securities - calculated_holdings = reverse ? reverse_holdings : forward_holdings - gapfill_holdings(calculated_holdings) + Rails.logger.tagged("Account::HoldingCalculator") do + preload_securities + + Rails.logger.info("Calculating holdings with strategy: #{reverse ? "reverse sync" : "forward sync"}") + calculated_holdings = reverse ? reverse_holdings : forward_holdings + + gapfill_holdings(calculated_holdings) + end end private diff --git a/app/models/account/syncer.rb b/app/models/account/syncer.rb index cd664e66..d664b8f1 100644 --- a/app/models/account/syncer.rb +++ b/app/models/account/syncer.rb @@ -5,19 +5,34 @@ class Account::Syncer end def run - account.family.auto_match_transfers! + Rails.logger.tagged("Account::Syncer") do + Rails.logger.info("Finding potential transfers to auto-match") + account.family.auto_match_transfers! - holdings = sync_holdings - balances = sync_balances(holdings) - account.reload - update_account_info(balances, holdings) unless plaid_sync? - convert_records_to_family_currency(balances, holdings) unless account.currency == account.family.currency + holdings = sync_holdings + Rails.logger.info("Calculated #{holdings.size} holdings") - # Enrich if user opted in or if we're syncing transactions from a Plaid account on the hosted app - if account.family.data_enrichment_enabled? || (plaid_sync? && Rails.application.config.app_mode.hosted?) - account.enrich_data - else - Rails.logger.info("Data enrichment is disabled, skipping enrichment for account #{account.id}") + balances = sync_balances(holdings) + Rails.logger.info("Calculated #{balances.size} balances") + + account.reload + + unless plaid_sync? + update_account_info(balances, holdings) + end + + unless account.currency == account.family.currency + Rails.logger.info("Converting #{balances.size} balances and #{holdings.size} holdings from #{account.currency} to #{account.family.currency}") + convert_records_to_family_currency(balances, holdings) + end + + # Enrich if user opted in or if we're syncing transactions from a Plaid account on the hosted app + if account.family.data_enrichment_enabled? || (plaid_sync? && Rails.application.config.app_mode.hosted?) + Rails.logger.info("Enriching transaction data for account #{account.name}") + account.enrich_data + else + Rails.logger.info("Data enrichment disabled for account #{account.name}") + end end end @@ -43,8 +58,6 @@ class Account::Syncer calculator = Account::HoldingCalculator.new(account) calculated_holdings = calculator.calculate(reverse: plaid_sync?) - current_time = Time.now - Account.transaction do load_holdings(calculated_holdings) purge_outdated_holdings unless plaid_sync? diff --git a/app/models/plaid_item.rb b/app/models/plaid_item.rb index 1a49675b..c76af8aa 100644 --- a/app/models/plaid_item.rb +++ b/app/models/plaid_item.rb @@ -41,8 +41,11 @@ class PlaidItem < ApplicationRecord update!(last_synced_at: Time.current) begin + Rails.logger.info("Fetching and loading Plaid data") plaid_data = fetch_and_load_plaid_data update!(status: :good) if requires_update? + + Rails.logger.info("Plaid data fetched and loaded") plaid_data rescue Plaid::ApiError => e handle_plaid_error(e) @@ -83,12 +86,17 @@ class PlaidItem < ApplicationRecord private 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_provider.get_item(access_token).item update!(available_products: item.available_products, billed_products: item.billed_products) - # Fetch and store institution details + # 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, @@ -96,12 +104,14 @@ class PlaidItem < ApplicationRecord institution_color: institution.institution.primary_color ) rescue Plaid::ApiError => e - Rails.logger.warn("Error fetching institution details for item #{id}: #{e.message}") + 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 || [] + 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) @@ -109,10 +119,12 @@ class PlaidItem < ApplicationRecord 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})" transaction do internal_plaid_accounts.each do |internal_plaid_account| added = fetched_transactions.added.select { |t| t.account_id == internal_plaid_account.plaid_id } @@ -126,10 +138,12 @@ class PlaidItem < ApplicationRecord 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})" transaction do internal_plaid_accounts.each do |internal_plaid_account| transactions = fetched_investments.transactions.select { |t| t.account_id == internal_plaid_account.plaid_id } @@ -141,10 +155,12 @@ class PlaidItem < ApplicationRecord 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})" transaction do internal_plaid_accounts.each do |internal_plaid_account| credit = fetched_liabilities.credit&.find { |l| l.account_id == internal_plaid_account.plaid_id } diff --git a/app/models/sync.rb b/app/models/sync.rb index 65e474d7..64446cdd 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -6,38 +6,47 @@ class Sync < ApplicationRecord scope :ordered, -> { order(created_at: :desc) } def perform - start! + Rails.logger.tagged("Sync", id, syncable_type, syncable_id) do + start! - begin - data = syncable.sync_data(start_date: start_date) - update!(data: data) if data - complete! - rescue StandardError => error - fail! error - raise error if Rails.env.development? - ensure - syncable.post_sync + begin + data = syncable.sync_data(start_date: start_date) + update!(data: data) if data + complete! + rescue StandardError => error + fail! error + raise error if Rails.env.development? + ensure + Rails.logger.info("Sync completed, starting post-sync") + + syncable.post_sync + + Rails.logger.info("Post-sync completed") + end end end private def start! + Rails.logger.info("Starting sync") update! status: :syncing end def complete! + Rails.logger.info("Sync completed") update! status: :completed, last_ran_at: Time.current end def fail!(error) + Rails.logger.error("Sync failed: #{error.message}") + Sentry.capture_exception(error) do |scope| - scope.set_context("sync", { id: id }) + scope.set_context("sync", { id: id, syncable_type: syncable_type, syncable_id: syncable_id }) end update!( status: :failed, error: error.message, - error_backtrace: error.backtrace&.first(10), last_ran_at: Time.current ) end diff --git a/config/environments/production.rb b/config/environments/production.rb index fba29091..4670492d 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -49,17 +49,18 @@ Rails.application.configure do config.assume_ssl = ActiveModel::Type::Boolean.new.cast(ENV.fetch("RAILS_ASSUME_SSL", true)) # Log to Logtail if API key is present, otherwise log to STDOUT - config.logger = if ENV["LOGTAIL_API_KEY"].present? + base_logger = if ENV["LOGTAIL_API_KEY"].present? Logtail::Logger.create_default_logger( ENV["LOGTAIL_API_KEY"], telemetry_host: "in.logs.betterstack.com" ) else ActiveSupport::Logger.new(STDOUT) - .tap { |logger| logger.formatter = ::Logger::Formatter.new } - .then { |logger| ActiveSupport::TaggedLogging.new(logger) } + .tap { |logger| logger.formatter = ::Logger::Formatter.new } end + config.logger = ActiveSupport::TaggedLogging.new(base_logger) + # Prepend all log lines with the following tags. config.log_tags = [ :request_id ] From 372b64ffea651bfdede2a990876d102b53e5ba38 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 5 Mar 2025 16:02:07 -0500 Subject: [PATCH 34/47] Fix Plaid sync error when current balance is null --- app/models/plaid_account.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/plaid_account.rb b/app/models/plaid_account.rb index 186105ce..77a4288d 100644 --- a/app/models/plaid_account.rb +++ b/app/models/plaid_account.rb @@ -20,7 +20,7 @@ class PlaidAccount < ApplicationRecord find_or_create_by!(plaid_id: plaid_data.account_id) do |a| a.account = family.accounts.new( name: plaid_data.name, - balance: plaid_data.balances.current, + balance: plaid_data.balances.current || plaid_data.balances.available, currency: plaid_data.balances.iso_currency_code, accountable: TYPE_MAPPING[plaid_data.type].new ) From 26762477a323b63c46f4c7064e1ed361d781e320 Mon Sep 17 00:00:00 2001 From: Nikhil Badyal <59223300+nikhilbadyal@users.noreply.github.com> Date: Fri, 7 Mar 2025 20:35:54 +0530 Subject: [PATCH 35/47] Preference to set default_period (#1941) --- app/controllers/accounts_controller.rb | 1 + app/controllers/concerns/accountable_resource.rb | 2 +- app/controllers/concerns/periodable.rb | 14 ++++++++++++++ app/controllers/pages_controller.rb | 2 +- app/controllers/users_controller.rb | 2 +- app/models/user.rb | 1 + app/views/accounts/chart.html.erb | 5 ++--- app/views/accounts/show/_chart.html.erb | 2 +- app/views/settings/preferences/show.html.erb | 5 +++++ config/locales/views/settings/en.yml | 1 + .../20250304140435_add_default_period_to_users.rb | 5 +++++ db/schema.rb | 3 ++- 12 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 app/controllers/concerns/periodable.rb create mode 100644 db/migrate/20250304140435_add_default_period_to_users.rb diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index df2b4d3c..5be606d2 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -1,5 +1,6 @@ class AccountsController < ApplicationController before_action :set_account, only: %i[sync chart sparkline] + include Periodable def index @manual_accounts = family.accounts.manual.alphabetically diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index 16467e36..524be0fc 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -2,7 +2,7 @@ module AccountableResource extend ActiveSupport::Concern included do - include ScrollFocusable + include ScrollFocusable, Periodable before_action :set_account, only: [ :show, :edit, :update, :destroy ] before_action :set_link_token, only: :new diff --git a/app/controllers/concerns/periodable.rb b/app/controllers/concerns/periodable.rb new file mode 100644 index 00000000..8cf02395 --- /dev/null +++ b/app/controllers/concerns/periodable.rb @@ -0,0 +1,14 @@ +module Periodable + extend ActiveSupport::Concern + + included do + before_action :set_period + end + + private + def set_period + @period = Period.from_key(params[:period] || Current.user&.default_period) + rescue Period::InvalidKeyError + @period = Period.last_30_days + end +end diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 34d6cca3..f2a91f62 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -1,8 +1,8 @@ class PagesController < ApplicationController skip_before_action :authenticate_user!, only: %i[early_access] + include Periodable def dashboard - @period = params[:period] ? Period.from_key(params[:period]) : Period.last_30_days @balance_sheet = Current.family.balance_sheet @accounts = Current.family.accounts.active.with_attached_logo diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9b82509d..4300477d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -66,7 +66,7 @@ class UsersController < ApplicationController def user_params params.require(:user).permit( - :first_name, :last_name, :email, :profile_image, :redirect_to, :delete_profile_image, :onboarded_at, :show_sidebar, + :first_name, :last_name, :email, :profile_image, :redirect_to, :delete_profile_image, :onboarded_at, :show_sidebar, :default_period, family_attributes: [ :name, :currency, :country, :locale, :date_format, :timezone, :id, :data_enrichment_enabled ] ) end diff --git a/app/models/user.rb b/app/models/user.rb index 24932861..479ce225 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -9,6 +9,7 @@ class User < ApplicationRecord validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP } validate :ensure_valid_profile_image + validates :default_period, inclusion: { in: Period::PERIODS.keys } normalizes :email, with: ->(email) { email.strip.downcase } normalizes :unconfirmed_email, with: ->(email) { email&.strip&.downcase } diff --git a/app/views/accounts/chart.html.erb b/app/views/accounts/chart.html.erb index c5cc51eb..22e2528e 100644 --- a/app/views/accounts/chart.html.erb +++ b/app/views/accounts/chart.html.erb @@ -1,5 +1,4 @@ -<% period = params[:period] ? Period.from_key(params[:period]) : Period.last_30_days %> -<% series = @account.balance_series(period: period) %> +<% series = @account.balance_series(period: @period) %> <% trend = series.trend %> <%= turbo_frame_tag dom_id(@account, :chart_details) do %> @@ -13,7 +12,7 @@ <% end %> <% end %> - <%= tag.span period.comparison_label, class: "text-secondary" %> + <%= tag.span @period.comparison_label, class: "text-secondary" %>
      diff --git a/app/views/accounts/show/_chart.html.erb b/app/views/accounts/show/_chart.html.erb index d7427762..c78a3676 100644 --- a/app/views/accounts/show/_chart.html.erb +++ b/app/views/accounts/show/_chart.html.erb @@ -1,6 +1,6 @@ <%# locals: (account:, title: nil, tooltip: nil, **args) %> -<% period = params[:period] ? Period.from_key(params[:period]) : Period.last_30_days %> +<% period = @period || Period.last_30_days %> <% default_value_title = account.asset? ? t(".balance") : t(".owed") %>
      diff --git a/app/views/settings/preferences/show.html.erb b/app/views/settings/preferences/show.html.erb index c51444dc..e053d969 100644 --- a/app/views/settings/preferences/show.html.erb +++ b/app/views/settings/preferences/show.html.erb @@ -25,6 +25,11 @@ { label: t(".date_format") }, { data: { auto_submit_form_target: "auto" } } %> + <%= form.select :default_period, + Period.all.map { |period| [ period.label, period.key ] }, + { label: t(".default_period") }, + { data: { auto_submit_form_target: "auto" } } %> + <%= family_form.select :country, country_options, { label: t(".country") }, diff --git a/config/locales/views/settings/en.yml b/config/locales/views/settings/en.yml index 8596904e..8bc0dbf0 100644 --- a/config/locales/views/settings/en.yml +++ b/config/locales/views/settings/en.yml @@ -20,6 +20,7 @@ en: date_format: Date format general_subtitle: Configure your preferences general_title: General + default_period: Default Period language: Language page_title: Preferences theme_dark: Dark diff --git a/db/migrate/20250304140435_add_default_period_to_users.rb b/db/migrate/20250304140435_add_default_period_to_users.rb new file mode 100644 index 00000000..ee2f5e2f --- /dev/null +++ b/db/migrate/20250304140435_add_default_period_to_users.rb @@ -0,0 +1,5 @@ +class AddDefaultPeriodToUsers < ActiveRecord::Migration[7.2] + def change + add_column :users, :default_period, :string, default: "last_30_days", null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 492edb1e..59eabedf 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_03_03_141007) do +ActiveRecord::Schema[7.2].define(version: 2025_03_04_140435) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -675,6 +675,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_03_03_141007) do t.boolean "otp_required", default: false, null: false t.string "otp_backup_codes", default: [], array: true t.boolean "show_sidebar", default: true + t.string "default_period", default: "last_30_days", null: false t.index ["email"], name: "index_users_on_email", unique: true t.index ["family_id"], name: "index_users_on_family_id" t.index ["otp_secret"], name: "index_users_on_otp_secret", unique: true, where: "(otp_secret IS NOT NULL)" From eac5d5e6630db73ef97e9a0bbaa994a1ea79bfca Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 7 Mar 2025 17:35:55 -0500 Subject: [PATCH 36/47] Populate holdings for "offline" securities properly (#1958) * Placeholder logic for missing prices * Generate holdings properly for "offline" securities * Separate forward and reverse calculators for holdings and balances * Remove unnecessary currency conversion during sync * Clearer sync process * Move price caching logic to dedicated model * Base holding calculator * Base calculator for balances * Finish balance calculators * Better naming * Logs cleanup * Remove stale data type * Remove stale test * Fix price lookup logic for holdings sync * Fix Plaid item sync regression * Remove temp logging * Calculate cash and holdings series * Add holdings, cash, and balance series dropdown for investments --- app/controllers/accounts_controller.rb | 1 + .../concerns/accountable_resource.rb | 1 + app/controllers/concerns/auto_sync.rb | 1 + app/models/account.rb | 33 +-- app/models/account/balance/base_calculator.rb | 35 ++++ .../account/balance/forward_calculator.rb | 28 +++ .../account/balance/reverse_calculator.rb | 32 +++ app/models/account/balance/sync_cache.rb | 46 +++++ app/models/account/balance/syncer.rb | 69 +++++++ app/models/account/balance_calculator.rb | 124 ------------ app/models/account/chartable.rb | 59 ++++-- app/models/account/enrichable.rb | 12 ++ app/models/account/holding.rb | 2 +- app/models/account/holding/base_calculator.rb | 63 ++++++ .../account/holding/forward_calculator.rb | 21 ++ app/models/account/holding/gapfillable.rb | 38 ++++ app/models/account/holding/portfolio_cache.rb | 132 ++++++++++++ .../account/holding/reverse_calculator.rb | 38 ++++ app/models/account/holding/syncer.rb | 58 ++++++ app/models/account/holding_calculator.rb | 188 ------------------ app/models/account/linkable.rb | 18 ++ app/models/account/syncer.rb | 162 --------------- app/models/plaid_item.rb | 5 + app/views/accounts/chart.html.erb | 2 +- app/views/accounts/show/_chart.html.erb | 19 +- app/views/investments/show.html.erb | 1 + .../balance/forward_calculator_test.rb | 74 +++++++ .../balance/reverse_calculator_test.rb | 59 ++++++ test/models/account/balance/syncer_test.rb | 51 +++++ .../models/account/balance_calculator_test.rb | 156 --------------- .../holding/forward_calculator_test.rb | 146 ++++++++++++++ .../account/holding/portfolio_cache_test.rb | 63 ++++++ .../reverse_calculator_test.rb} | 86 +------- test/models/account/holding/syncer_test.rb | 29 +++ test/models/account/syncer_test.rb | 65 ------ 35 files changed, 1109 insertions(+), 808 deletions(-) create mode 100644 app/models/account/balance/base_calculator.rb create mode 100644 app/models/account/balance/forward_calculator.rb create mode 100644 app/models/account/balance/reverse_calculator.rb create mode 100644 app/models/account/balance/sync_cache.rb create mode 100644 app/models/account/balance/syncer.rb delete mode 100644 app/models/account/balance_calculator.rb create mode 100644 app/models/account/enrichable.rb create mode 100644 app/models/account/holding/base_calculator.rb create mode 100644 app/models/account/holding/forward_calculator.rb create mode 100644 app/models/account/holding/gapfillable.rb create mode 100644 app/models/account/holding/portfolio_cache.rb create mode 100644 app/models/account/holding/reverse_calculator.rb create mode 100644 app/models/account/holding/syncer.rb delete mode 100644 app/models/account/holding_calculator.rb create mode 100644 app/models/account/linkable.rb delete mode 100644 app/models/account/syncer.rb create mode 100644 test/models/account/balance/forward_calculator_test.rb create mode 100644 test/models/account/balance/reverse_calculator_test.rb create mode 100644 test/models/account/balance/syncer_test.rb delete mode 100644 test/models/account/balance_calculator_test.rb create mode 100644 test/models/account/holding/forward_calculator_test.rb create mode 100644 test/models/account/holding/portfolio_cache_test.rb rename test/models/account/{holding_calculator_test.rb => holding/reverse_calculator_test.rb} (61%) create mode 100644 test/models/account/holding/syncer_test.rb delete mode 100644 test/models/account/syncer_test.rb diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 5be606d2..33b3980a 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -18,6 +18,7 @@ class AccountsController < ApplicationController end def chart + @chart_view = params[:chart_view] || "balance" render layout: "application" end diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index 524be0fc..d7d3b169 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -23,6 +23,7 @@ module AccountableResource end def show + @chart_view = params[:chart_view] || "balance" @q = params.fetch(:q, {}).permit(:search) entries = @account.entries.search(@q).reverse_chronological diff --git a/app/controllers/concerns/auto_sync.rb b/app/controllers/concerns/auto_sync.rb index d9e616e4..970eec0a 100644 --- a/app/controllers/concerns/auto_sync.rb +++ b/app/controllers/concerns/auto_sync.rb @@ -13,6 +13,7 @@ module AutoSync def family_needs_auto_sync? return false unless Current.family.present? + return false unless Current.family.accounts.active.any? (Current.family.last_synced_at&.to_date || 1.day.ago) < Date.current end diff --git a/app/models/account.rb b/app/models/account.rb index 75752077..0c037609 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -1,11 +1,10 @@ class Account < ApplicationRecord - include Syncable, Monetizable, Issuable, Chartable + include Syncable, Monetizable, Issuable, Chartable, Enrichable, Linkable validates :name, :balance, :currency, presence: true belongs_to :family belongs_to :import, optional: true - belongs_to :plaid_account, optional: true has_many :import_mappings, as: :mappable, dependent: :destroy, class_name: "Import::Mapping" has_many :entries, dependent: :destroy, class_name: "Account::Entry" @@ -75,7 +74,16 @@ class Account < ApplicationRecord def sync_data(start_date: nil) update!(last_synced_at: Time.current) - Syncer.new(self, start_date: start_date).run + Rails.logger.info("Auto-matching transfers") + family.auto_match_transfers! + + Rails.logger.info("Processing balances (#{linked? ? 'reverse' : 'forward'})") + sync_balances + + if enrichable? + Rails.logger.info("Enriching transaction data") + enrich_data + end end def post_sync @@ -93,10 +101,6 @@ class Account < ApplicationRecord holdings.where(currency: currency, date: holdings.maximum(:date)).order(amount: :desc) end - def enrich_data - DataEnricher.new(self).run - end - def update_with_sync!(attributes) should_update_balance = attributes[:balance] && attributes[:balance].to_d != balance @@ -123,11 +127,14 @@ class Account < ApplicationRecord end end - def sparkline_series - cache_key = family.build_cache_key("#{id}_sparkline") - - Rails.cache.fetch(cache_key) do - balance_series - end + def start_date + first_entry_date = entries.minimum(:date) || Date.current + first_entry_date - 1.day 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/balance/base_calculator.rb b/app/models/account/balance/base_calculator.rb new file mode 100644 index 00000000..7acb51e8 --- /dev/null +++ b/app/models/account/balance/base_calculator.rb @@ -0,0 +1,35 @@ +class Account::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 ||= Account::Balance::SyncCache.new(account) + end + + def build_balance(date, cash_balance, holdings_value) + Account::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/account/balance/forward_calculator.rb b/app/models/account/balance/forward_calculator.rb new file mode 100644 index 00000000..503e5b79 --- /dev/null +++ b/app/models/account/balance/forward_calculator.rb @@ -0,0 +1,28 @@ +class Account::Balance::ForwardCalculator < Account::Balance::BaseCalculator + private + def calculate_balances + current_cash_balance = 0 + next_cash_balance = nil + + @balances = [] + + account.start_date.upto(Date.current).each do |date| + entries = sync_cache.get_entries(date) + holdings = sync_cache.get_holdings(date) + holdings_value = holdings.sum(&:amount) + valuation = sync_cache.get_valuation(date) + + next_cash_balance = if valuation + valuation.amount - holdings_value + else + calculate_next_balance(current_cash_balance, entries, direction: :forward) + end + + @balances << build_balance(date, next_cash_balance, holdings_value) + + current_cash_balance = next_cash_balance + end + + @balances + end +end diff --git a/app/models/account/balance/reverse_calculator.rb b/app/models/account/balance/reverse_calculator.rb new file mode 100644 index 00000000..151f4036 --- /dev/null +++ b/app/models/account/balance/reverse_calculator.rb @@ -0,0 +1,32 @@ +class Account::Balance::ReverseCalculator < Account::Balance::BaseCalculator + private + def calculate_balances + current_cash_balance = account.cash_balance + previous_cash_balance = nil + + @balances = [] + + Date.current.downto(account.start_date).map do |date| + entries = sync_cache.get_entries(date) + holdings = sync_cache.get_holdings(date) + holdings_value = holdings.sum(&:amount) + valuation = sync_cache.get_valuation(date) + + previous_cash_balance = if valuation + valuation.amount - holdings_value + else + calculate_next_balance(current_cash_balance, entries, direction: :reverse) + end + + if valuation.present? + @balances << build_balance(date, previous_cash_balance, holdings_value) + else + @balances << build_balance(date, current_cash_balance, holdings_value) + end + + current_cash_balance = previous_cash_balance + end + + @balances + end +end diff --git a/app/models/account/balance/sync_cache.rb b/app/models/account/balance/sync_cache.rb new file mode 100644 index 00000000..1fb7ea7f --- /dev/null +++ b/app/models/account/balance/sync_cache.rb @@ -0,0 +1,46 @@ +class Account::Balance::SyncCache + def initialize(account) + @account = account + end + + def get_valuation(date) + converted_entries.find { |e| e.date == date && e.account_valuation? } + end + + def get_holdings(date) + converted_holdings.select { |h| h.date == date } + end + + def get_entries(date) + converted_entries.select { |e| e.date == date && (e.account_transaction? || e.account_trade?) } + end + + private + attr_reader :account + + def converted_entries + @converted_entries ||= account.entries.order(:date).to_a.map do |e| + converted_entry = e.dup + converted_entry.amount = converted_entry.amount_money.exchange_to( + account.currency, + date: e.date, + fallback_rate: 1 + ).amount + converted_entry.currency = account.currency + converted_entry + end + end + + def converted_holdings + @converted_holdings ||= account.holdings.map do |h| + converted_holding = h.dup + converted_holding.amount = converted_holding.amount_money.exchange_to( + account.currency, + date: h.date, + fallback_rate: 1 + ).amount + converted_holding.currency = account.currency + converted_holding + end + end +end diff --git a/app/models/account/balance/syncer.rb b/app/models/account/balance/syncer.rb new file mode 100644 index 00000000..cc8ca68b --- /dev/null +++ b/app/models/account/balance/syncer.rb @@ -0,0 +1,69 @@ +class Account::Balance::Syncer + attr_reader :account, :strategy + + def initialize(account, strategy:) + @account = account + @strategy = strategy + end + + def sync_balances + Account::Balance.transaction do + sync_holdings + calculate_balances + + Rails.logger.info("Persisting #{@balances.size} balances") + persist_balances + + purge_stale_balances + + if strategy == :forward + update_account_info + end + end + end + + private + def sync_holdings + @holdings = Account::Holding::Syncer.new(account, strategy: strategy).sync_holdings + end + + def update_account_info + calculated_balance = @balances.sort_by(&:date).last&.balance || 0 + calculated_holdings_value = @holdings.select { |h| h.date == Date.current }.sum(&:amount) || 0 + calculated_cash_balance = calculated_balance - calculated_holdings_value + + Rails.logger.info("Balance update: cash=#{calculated_cash_balance}, total=#{calculated_balance}") + + account.update!( + balance: calculated_balance, + cash_balance: calculated_cash_balance + ) + end + + def calculate_balances + @balances = calculator.calculate + end + + def persist_balances + current_time = Time.now + account.balances.upsert_all( + @balances.map { |b| b.attributes + .slice("date", "balance", "cash_balance", "currency") + .merge("updated_at" => current_time) }, + unique_by: %i[account_id date currency] + ) + end + + def purge_stale_balances + deleted_count = account.balances.delete_by("date < ?", account.start_date) + Rails.logger.info("Purged #{deleted_count} stale balances") if deleted_count > 0 + end + + def calculator + if strategy == :reverse + Account::Balance::ReverseCalculator.new(account) + else + Account::Balance::ForwardCalculator.new(account) + end + end +end diff --git a/app/models/account/balance_calculator.rb b/app/models/account/balance_calculator.rb deleted file mode 100644 index ca292dc5..00000000 --- a/app/models/account/balance_calculator.rb +++ /dev/null @@ -1,124 +0,0 @@ -class Account::BalanceCalculator - def initialize(account, holdings: nil) - @account = account - @holdings = holdings || [] - end - - def calculate(reverse: false, start_date: nil) - Rails.logger.tagged("Account::BalanceCalculator") do - Rails.logger.info("Calculating cash balances with strategy: #{reverse ? "reverse sync" : "forward sync"}") - cash_balances = reverse ? reverse_cash_balances : forward_cash_balances - - cash_balances.map do |balance| - holdings_value = converted_holdings.select { |h| h.date == balance.date }.sum(&:amount) - balance.balance = balance.balance + holdings_value - balance - end.compact - end - end - - private - attr_reader :account, :holdings - - def oldest_date - converted_entries.first ? converted_entries.first.date - 1.day : Date.current - end - - def reverse_cash_balances - prior_balance = account.cash_balance - - Date.current.downto(oldest_date).map do |date| - entries_for_date = converted_entries.select { |e| e.date == date } - holdings_for_date = converted_holdings.select { |h| h.date == date } - - valuation = entries_for_date.find { |e| e.account_valuation? } - - current_balance = if valuation - # To get this to a cash valuation, we back out holdings value on day - valuation.amount - holdings_for_date.sum(&:amount) - else - transactions = entries_for_date.select { |e| e.account_transaction? || e.account_trade? } - - calculate_balance(prior_balance, transactions) - end - - balance_record = Account::Balance.new( - account: account, - date: date, - balance: valuation ? current_balance : prior_balance, - cash_balance: valuation ? current_balance : prior_balance, - currency: account.currency - ) - - prior_balance = current_balance - - balance_record - end - end - - def forward_cash_balances - prior_balance = 0 - current_balance = nil - - oldest_date.upto(Date.current).map do |date| - entries_for_date = converted_entries.select { |e| e.date == date } - holdings_for_date = converted_holdings.select { |h| h.date == date } - - valuation = entries_for_date.find { |e| e.account_valuation? } - - current_balance = if valuation - # To get this to a cash valuation, we back out holdings value on day - valuation.amount - holdings_for_date.sum(&:amount) - else - transactions = entries_for_date.select { |e| e.account_transaction? || e.account_trade? } - - calculate_balance(prior_balance, transactions, inverse: true) - end - - balance_record = Account::Balance.new( - account: account, - date: date, - balance: current_balance, - cash_balance: current_balance, - currency: account.currency - ) - - prior_balance = current_balance - - balance_record - end - end - - def converted_entries - @converted_entries ||= @account.entries.order(:date).to_a.map do |e| - converted_entry = e.dup - converted_entry.amount = converted_entry.amount_money.exchange_to( - account.currency, - date: e.date, - fallback_rate: 1 - ).amount - converted_entry.currency = account.currency - converted_entry - end - end - - def converted_holdings - @converted_holdings ||= holdings.map do |h| - converted_holding = h.dup - converted_holding.amount = converted_holding.amount_money.exchange_to( - account.currency, - date: h.date, - fallback_rate: 1 - ).amount - converted_holding.currency = account.currency - converted_holding - end - end - - def calculate_balance(prior_balance, transactions, inverse: false) - flows = transactions.sum(&:amount) - negated = inverse ? account.asset? : account.liability? - flows *= -1 if negated - prior_balance + flows - end -end diff --git a/app/models/account/chartable.rb b/app/models/account/chartable.rb index 2770da3b..f251e7f1 100644 --- a/app/models/account/chartable.rb +++ b/app/models/account/chartable.rb @@ -2,7 +2,9 @@ module Account::Chartable extend ActiveSupport::Concern class_methods do - def balance_series(currency:, period: Period.last_30_days, favorable_direction: "up") + def balance_series(currency:, period: Period.last_30_days, favorable_direction: "up", view: :balance) + raise ArgumentError, "Invalid view type" unless [ :balance, :cash_balance, :holdings_balance ].include?(view.to_sym) + balances = Account::Balance.find_by_sql([ balance_series_query, { @@ -21,8 +23,8 @@ module Account::Chartable date: curr.date, date_formatted: I18n.l(curr.date, format: :long), trend: Trend.new( - current: Money.new(curr.balance, currency), - previous: prev.nil? ? nil : Money.new(prev.balance, currency), + current: Money.new(balance_value_for(curr, view), currency), + previous: prev.nil? ? nil : Money.new(balance_value_for(prev, view), currency), favorable_direction: favorable_direction ) ) @@ -33,8 +35,8 @@ module Account::Chartable end_date: period.end_date, interval: period.interval, trend: Trend.new( - current: Money.new(balances.last&.balance || 0, currency), - previous: Money.new(balances.first&.balance || 0, currency), + current: Money.new(balance_value_for(balances.last, view) || 0, currency), + previous: Money.new(balance_value_for(balances.first, view) || 0, currency), favorable_direction: favorable_direction ), values: values @@ -52,6 +54,8 @@ module Account::Chartable SELECT d.date, SUM(CASE WHEN accounts.classification = 'asset' THEN ab.balance ELSE -ab.balance END * COALESCE(er.rate, 1)) as balance, + SUM(CASE WHEN accounts.classification = 'asset' THEN ab.cash_balance ELSE -ab.cash_balance END * COALESCE(er.rate, 1)) as cash_balance, + SUM(CASE WHEN accounts.classification = 'asset' THEN ab.balance - ab.cash_balance ELSE 0 END * COALESCE(er.rate, 1)) as holdings_balance, COUNT(CASE WHEN accounts.currency <> :target_currency AND er.rate IS NULL THEN 1 END) as missing_rates FROM dates d LEFT JOIN accounts ON accounts.id IN (#{all.select(:id).to_sql}) @@ -70,26 +74,46 @@ module Account::Chartable SQL end + def balance_value_for(balance_record, view) + return 0 if balance_record.nil? + + case view.to_sym + when :balance then balance_record.balance + when :cash_balance then balance_record.cash_balance + when :holdings_balance then balance_record.holdings_balance + else + raise ArgumentError, "Invalid view type: #{view}" + end + end + def invert_balances(balances) balances.map do |balance| balance.balance = -balance.balance + balance.cash_balance = -balance.cash_balance + balance.holdings_balance = -balance.holdings_balance balance end end def gapfill_balances(balances) gapfilled = [] + prev = nil - prev_balance = nil - - [ nil, *balances ].each_cons(2).each_with_index do |(prev, curr), index| - if index == 0 && curr.balance.nil? - curr.balance = 0 # Ensure all series start with a non-nil balance - elsif curr.balance.nil? - curr.balance = prev.balance + balances.each do |curr| + if prev.nil? + # Initialize first record with zeros if nil + curr.balance ||= 0 + curr.cash_balance ||= 0 + curr.holdings_balance ||= 0 + else + # Copy previous values for nil fields + curr.balance ||= prev.balance + curr.cash_balance ||= prev.cash_balance + curr.holdings_balance ||= prev.holdings_balance end gapfilled << curr + prev = curr end gapfilled @@ -100,11 +124,20 @@ module Account::Chartable classification == "asset" ? "up" : "down" end - def balance_series(period: Period.last_30_days) + def balance_series(period: Period.last_30_days, view: :balance) self.class.where(id: self.id).balance_series( currency: currency, period: period, + view: view, favorable_direction: favorable_direction ) end + + def sparkline_series + cache_key = family.build_cache_key("#{id}_sparkline") + + Rails.cache.fetch(cache_key) do + balance_series + end + end end diff --git a/app/models/account/enrichable.rb b/app/models/account/enrichable.rb new file mode 100644 index 00000000..236cce58 --- /dev/null +++ b/app/models/account/enrichable.rb @@ -0,0 +1,12 @@ +module Account::Enrichable + extend ActiveSupport::Concern + + def enrich_data + DataEnricher.new(self).run + end + + private + def enrichable? + family.data_enrichment_enabled? || (linked? && Rails.application.config.app_mode.hosted?) + end +end diff --git a/app/models/account/holding.rb b/app/models/account/holding.rb index eb6e35ef..ba7a7e2d 100644 --- a/app/models/account/holding.rb +++ b/app/models/account/holding.rb @@ -1,5 +1,5 @@ class Account::Holding < ApplicationRecord - include Monetizable + include Monetizable, Gapfillable monetize :amount diff --git a/app/models/account/holding/base_calculator.rb b/app/models/account/holding/base_calculator.rb new file mode 100644 index 00000000..4359e9ab --- /dev/null +++ b/app/models/account/holding/base_calculator.rb @@ -0,0 +1,63 @@ +class Account::Holding::BaseCalculator + attr_reader :account + + def initialize(account) + @account = account + end + + def calculate + Rails.logger.tagged(self.class.name) do + holdings = calculate_holdings + Account::Holding.gapfill(holdings) + end + end + + private + def portfolio_cache + @portfolio_cache ||= Account::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) + portfolio.map do |security_id, qty| + price = portfolio_cache.get_price(security_id, date) + + if price.nil? + Rails.logger.warn "No price found for security #{security_id} on #{date}" + next + end + + Account::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/account/holding/forward_calculator.rb b/app/models/account/holding/forward_calculator.rb new file mode 100644 index 00000000..afb6b71f --- /dev/null +++ b/app/models/account/holding/forward_calculator.rb @@ -0,0 +1,21 @@ +class Account::Holding::ForwardCalculator < Account::Holding::BaseCalculator + private + def portfolio_cache + @portfolio_cache ||= Account::Holding::PortfolioCache.new(account) + end + + def calculate_holdings + current_portfolio = generate_starting_portfolio + next_portfolio = {} + holdings = [] + + account.start_date.upto(Date.current).each do |date| + trades = portfolio_cache.get_trades(date: date) + next_portfolio = transform_portfolio(current_portfolio, trades, direction: :forward) + holdings += build_holdings(next_portfolio, date) + current_portfolio = next_portfolio + end + + holdings + end +end diff --git a/app/models/account/holding/gapfillable.rb b/app/models/account/holding/gapfillable.rb new file mode 100644 index 00000000..e2462a6f --- /dev/null +++ b/app/models/account/holding/gapfillable.rb @@ -0,0 +1,38 @@ +module Account::Holding::Gapfillable + extend ActiveSupport::Concern + + class_methods do + def gapfill(holdings) + filled_holdings = [] + + holdings.group_by { |h| h.security_id }.each do |security_id, security_holdings| + next if security_holdings.empty? + + sorted = security_holdings.sort_by(&:date) + previous_holding = sorted.first + + sorted.first.date.upto(Date.current) do |date| + holding = security_holdings.find { |h| h.date == date } + + if holding + filled_holdings << holding + previous_holding = holding + else + # Create a new holding based on the previous day's data + filled_holdings << Account::Holding.new( + account: previous_holding.account, + security: previous_holding.security, + date: date, + qty: previous_holding.qty, + price: previous_holding.price, + currency: previous_holding.currency, + amount: previous_holding.amount + ) + end + end + end + + filled_holdings + end + end +end diff --git a/app/models/account/holding/portfolio_cache.rb b/app/models/account/holding/portfolio_cache.rb new file mode 100644 index 00000000..6a839382 --- /dev/null +++ b/app/models/account/holding/portfolio_cache.rb @@ -0,0 +1,132 @@ +class Account::Holding::PortfolioCache + attr_reader :account, :use_holdings + + class SecurityNotFound < StandardError + def initialize(security_id, account_id) + super("Security id=#{security_id} not found in portfolio cache for account #{account_id}. This should not happen unless securities were preloaded incorrectly.") + end + end + + def initialize(account, use_holdings: false) + @account = account + @use_holdings = use_holdings + load_prices + end + + def get_trades(date: nil) + if date.blank? + trades + else + trades.select { |t| t.date == date } + end + end + + def get_price(security_id, date) + security = @security_cache[security_id] + raise SecurityNotFound.new(security_id, account.id) unless security + + price = security[:prices].select { |p| p.price.date == date }.min_by(&:priority)&.price + + return nil unless price + + price_money = Money.new(price.price, price.currency) + + converted_amount = price_money.exchange_to(account.currency, fallback_rate: 1).amount + + Security::Price.new( + security_id: security_id, + date: price.date, + price: converted_amount, + currency: price.currency + ) + end + + def get_securities + @security_cache.map { |_, v| v[:security] } + end + + private + PriceWithPriority = Data.define(:price, :priority) + + def trades + @trades ||= account.entries.includes(entryable: :security).account_trades.chronological.to_a + end + + def holdings + @holdings ||= account.holdings.chronological.to_a + end + + def collect_unique_securities + unique_securities_from_trades = trades.map(&:entryable).map(&:security).uniq + + return unique_securities_from_trades unless use_holdings + + unique_securities_from_holdings = holdings.map(&:security).uniq + + (unique_securities_from_trades + unique_securities_from_holdings).uniq + end + + # Loads all known prices for all securities in the account with priority based on source: + # 1 - DB or provider prices + # 2 - Trade prices + # 3 - Holding prices + def load_prices + @security_cache = {} + securities = collect_unique_securities + + Rails.logger.info "Preloading #{securities.size} securities for account #{account.id}" + + securities.each do |security| + Rails.logger.info "Loading security: ID=#{security.id} Ticker=#{security.ticker}" + + # Highest priority prices + db_or_provider_prices = Security::Price.find_prices( + security: security, + start_date: account.start_date, + end_date: Date.current + ).map do |price| + PriceWithPriority.new( + price: price, + priority: 1 + ) + end + + # Medium priority prices from trades + trade_prices = trades + .select { |t| t.entryable.security_id == security.id } + .map do |trade| + PriceWithPriority.new( + price: Security::Price.new( + security: security, + price: trade.entryable.price, + currency: trade.entryable.currency, + date: trade.date + ), + priority: 2 + ) + end + + # Low priority prices from holdings (if applicable) + holding_prices = if use_holdings + holdings.select { |h| h.security_id == security.id }.map do |holding| + PriceWithPriority.new( + price: Security::Price.new( + security: security, + price: holding.price, + currency: holding.currency, + date: holding.date + ), + priority: 3 + ) + end + else + [] + end + + @security_cache[security.id] = { + security: security, + prices: db_or_provider_prices + trade_prices + holding_prices + } + end + end +end diff --git a/app/models/account/holding/reverse_calculator.rb b/app/models/account/holding/reverse_calculator.rb new file mode 100644 index 00000000..d3677c88 --- /dev/null +++ b/app/models/account/holding/reverse_calculator.rb @@ -0,0 +1,38 @@ +class Account::Holding::ReverseCalculator < Account::Holding::BaseCalculator + 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 + # trades that make up those holdings. + def portfolio_cache + @portfolio_cache ||= Account::Holding::PortfolioCache.new(account, use_holdings: true) + end + + def calculate_holdings + current_portfolio = generate_starting_portfolio + previous_portfolio = {} + + holdings = [] + + Date.current.downto(account.start_date).each do |date| + today_trades = portfolio_cache.get_trades(date: date) + previous_portfolio = transform_portfolio(current_portfolio, today_trades, direction: :reverse) + holdings += build_holdings(current_portfolio, date) + current_portfolio = previous_portfolio + end + + holdings + end + + # Since this is a reverse sync, we start with today's holdings + def generate_starting_portfolio + holding_quantities = empty_portfolio + + todays_holdings = account.holdings.where(date: Date.current) + + todays_holdings.each do |holding| + holding_quantities[holding.security_id] = holding.qty + end + + holding_quantities + end +end diff --git a/app/models/account/holding/syncer.rb b/app/models/account/holding/syncer.rb new file mode 100644 index 00000000..bfccd6f0 --- /dev/null +++ b/app/models/account/holding/syncer.rb @@ -0,0 +1,58 @@ +class Account::Holding::Syncer + def initialize(account, strategy:) + @account = account + @strategy = strategy + end + + def sync_holdings + calculate_holdings + + Rails.logger.info("Persisting #{@holdings.size} holdings") + persist_holdings + + if strategy == :forward + purge_stale_holdings + end + + @holdings + end + + private + attr_reader :account, :strategy + + def calculate_holdings + @holdings = calculator.calculate + end + + def persist_holdings + current_time = Time.now + + account.holdings.upsert_all( + @holdings.map { |h| h.attributes + .slice("date", "currency", "qty", "price", "amount", "security_id") + .merge("account_id" => account.id, "updated_at" => current_time) }, + unique_by: %i[account_id security_id date currency] + ) + end + + def purge_stale_holdings + portfolio_security_ids = account.entries.account_trades.map { |entry| entry.entryable.security_id }.uniq + + # If there are no securities in the portfolio, delete all holdings + if portfolio_security_ids.empty? + Rails.logger.info("Clearing all holdings (no securities)") + account.holdings.delete_all + else + deleted_count = account.holdings.delete_by("date < ? OR security_id NOT IN (?)", account.start_date, portfolio_security_ids) + Rails.logger.info("Purged #{deleted_count} stale holdings") if deleted_count > 0 + end + end + + def calculator + if strategy == :reverse + Account::Holding::ReverseCalculator.new(account) + else + Account::Holding::ForwardCalculator.new(account) + end + end +end diff --git a/app/models/account/holding_calculator.rb b/app/models/account/holding_calculator.rb deleted file mode 100644 index edb55acf..00000000 --- a/app/models/account/holding_calculator.rb +++ /dev/null @@ -1,188 +0,0 @@ -class Account::HoldingCalculator - def initialize(account) - @account = account - @securities_cache = {} - end - - def calculate(reverse: false) - Rails.logger.tagged("Account::HoldingCalculator") do - preload_securities - - Rails.logger.info("Calculating holdings with strategy: #{reverse ? "reverse sync" : "forward sync"}") - calculated_holdings = reverse ? reverse_holdings : forward_holdings - - gapfill_holdings(calculated_holdings) - end - end - - private - attr_reader :account, :securities_cache - - def reverse_holdings - current_holding_quantities = load_current_holding_quantities - prior_holding_quantities = {} - - holdings = [] - - Date.current.downto(portfolio_start_date).map do |date| - today_trades = trades.select { |t| t.date == date } - prior_holding_quantities = calculate_portfolio(current_holding_quantities, today_trades) - holdings += generate_holding_records(current_holding_quantities, date) - current_holding_quantities = prior_holding_quantities - end - - holdings - end - - def forward_holdings - prior_holding_quantities = load_empty_holding_quantities - current_holding_quantities = {} - - holdings = [] - - portfolio_start_date.upto(Date.current).map do |date| - today_trades = trades.select { |t| t.date == date } - current_holding_quantities = calculate_portfolio(prior_holding_quantities, today_trades, inverse: true) - holdings += generate_holding_records(current_holding_quantities, date) - prior_holding_quantities = current_holding_quantities - end - - holdings - end - - def generate_holding_records(portfolio, date) - Rails.logger.info "[HoldingCalculator] Generating holdings for #{portfolio.size} securities on #{date}" - - portfolio.map do |security_id, qty| - security = securities_cache[security_id] - - if security.blank? - Rails.logger.error "[HoldingCalculator] Security #{security_id} not found in cache for account #{account.id}" - next - end - - price = security.dig(:prices)&.find { |p| p.date == date } - - if price.blank? - Rails.logger.info "[HoldingCalculator] No price found for security #{security_id} on #{date}" - next - end - - converted_price = Money.new(price.price, price.currency).exchange_to(account.currency, fallback_rate: 1).amount - - account.holdings.build( - security: security.dig(:security), - date: date, - qty: qty, - price: converted_price, - currency: account.currency, - amount: qty * converted_price - ) - end.compact - end - - def gapfill_holdings(holdings) - filled_holdings = [] - - holdings.group_by { |h| h.security_id }.each do |security_id, security_holdings| - next if security_holdings.empty? - - sorted = security_holdings.sort_by(&:date) - previous_holding = sorted.first - - sorted.first.date.upto(Date.current) do |date| - holding = security_holdings.find { |h| h.date == date } - - if holding - filled_holdings << holding - previous_holding = holding - else - # Create a new holding based on the previous day's data - filled_holdings << account.holdings.build( - security: previous_holding.security, - date: date, - qty: previous_holding.qty, - price: previous_holding.price, - currency: previous_holding.currency, - amount: previous_holding.amount - ) - end - end - end - - filled_holdings - end - - def trades - @trades ||= account.entries.includes(entryable: :security).account_trades.chronological.to_a - end - - def portfolio_start_date - trades.first ? trades.first.date - 1.day : Date.current - end - - def preload_securities - # Get securities from trades and current holdings - securities = trades.map(&:entryable).map(&:security).uniq - securities += account.holdings.where(date: Date.current).map(&:security) - securities.uniq! - - Rails.logger.info "[HoldingCalculator] Preloading #{securities.size} securities for account #{account.id}" - - securities.each do |security| - begin - Rails.logger.info "[HoldingCalculator] Loading security: ID=#{security.id} Ticker=#{security.ticker}" - - prices = Security::Price.find_prices( - security: security, - start_date: portfolio_start_date, - end_date: Date.current - ) - - Rails.logger.info "[HoldingCalculator] Found #{prices.size} prices for security #{security.id}" - - @securities_cache[security.id] = { - security: security, - prices: prices - } - rescue => e - Rails.logger.error "[HoldingCalculator] Error processing security #{security.id}: #{e.message}" - Rails.logger.error "[HoldingCalculator] Security details: #{security.attributes}" - Rails.logger.error e.backtrace.join("\n") - next # Skip this security and continue with others - end - end - end - - def calculate_portfolio(holding_quantities, today_trades, inverse: false) - new_quantities = holding_quantities.dup - - today_trades.each do |trade| - security_id = trade.entryable.security_id - qty_change = inverse ? trade.entryable.qty : -trade.entryable.qty - new_quantities[security_id] = (new_quantities[security_id] || 0) + qty_change - end - - new_quantities - end - - def load_empty_holding_quantities - holding_quantities = {} - - trades.map { |t| t.entryable.security_id }.uniq.each do |security_id| - holding_quantities[security_id] = 0 - end - - holding_quantities - end - - def load_current_holding_quantities - holding_quantities = load_empty_holding_quantities - - account.holdings.where(date: Date.current, currency: account.currency).map do |holding| - holding_quantities[holding.security_id] = holding.qty - end - - holding_quantities - end -end diff --git a/app/models/account/linkable.rb b/app/models/account/linkable.rb new file mode 100644 index 00000000..ee0871bd --- /dev/null +++ b/app/models/account/linkable.rb @@ -0,0 +1,18 @@ +module Account::Linkable + extend ActiveSupport::Concern + + included do + belongs_to :plaid_account, optional: true + end + + # A "linked" account gets transaction and balance data from a third party like Plaid + def linked? + plaid_account_id.present? + end + + # An "offline" or "unlinked" account is one where the user tracks values and + # adds transactions manually, without the help of a data provider + def unlinked? + !linked? + end +end diff --git a/app/models/account/syncer.rb b/app/models/account/syncer.rb deleted file mode 100644 index d664b8f1..00000000 --- a/app/models/account/syncer.rb +++ /dev/null @@ -1,162 +0,0 @@ -class Account::Syncer - def initialize(account, start_date: nil) - @account = account - @start_date = start_date - end - - def run - Rails.logger.tagged("Account::Syncer") do - Rails.logger.info("Finding potential transfers to auto-match") - account.family.auto_match_transfers! - - holdings = sync_holdings - Rails.logger.info("Calculated #{holdings.size} holdings") - - balances = sync_balances(holdings) - Rails.logger.info("Calculated #{balances.size} balances") - - account.reload - - unless plaid_sync? - update_account_info(balances, holdings) - end - - unless account.currency == account.family.currency - Rails.logger.info("Converting #{balances.size} balances and #{holdings.size} holdings from #{account.currency} to #{account.family.currency}") - convert_records_to_family_currency(balances, holdings) - end - - # Enrich if user opted in or if we're syncing transactions from a Plaid account on the hosted app - if account.family.data_enrichment_enabled? || (plaid_sync? && Rails.application.config.app_mode.hosted?) - Rails.logger.info("Enriching transaction data for account #{account.name}") - account.enrich_data - else - Rails.logger.info("Data enrichment disabled for account #{account.name}") - end - end - end - - private - attr_reader :account, :start_date - - def account_start_date - @account_start_date ||= (account.entries.chronological.first&.date || Date.current) - 1.day - end - - def update_account_info(balances, holdings) - new_balance = balances.sort_by(&:date).last.balance - new_holdings_value = holdings.select { |h| h.date == Date.current }.sum(&:amount) - new_cash_balance = new_balance - new_holdings_value - - account.update!( - balance: new_balance, - cash_balance: new_cash_balance - ) - end - - def sync_holdings - calculator = Account::HoldingCalculator.new(account) - calculated_holdings = calculator.calculate(reverse: plaid_sync?) - - Account.transaction do - load_holdings(calculated_holdings) - purge_outdated_holdings unless plaid_sync? - end - - calculated_holdings - end - - def sync_balances(holdings) - calculator = Account::BalanceCalculator.new(account, holdings: holdings) - calculated_balances = calculator.calculate(reverse: plaid_sync?, start_date: start_date) - - Account.transaction do - load_balances(calculated_balances) - purge_outdated_balances - end - - calculated_balances - end - - def convert_records_to_family_currency(balances, holdings) - from_currency = account.currency - to_currency = account.family.currency - - exchange_rates = ExchangeRate.find_rates( - from: from_currency, - to: to_currency, - start_date: balances.min_by(&:date).date - ) - - converted_balances = balances.map do |balance| - exchange_rate = exchange_rates.find { |er| er.date == balance.date } - - next unless exchange_rate.present? - - account.balances.build( - date: balance.date, - balance: exchange_rate.rate * balance.balance, - currency: to_currency - ) - end.compact - - converted_holdings = holdings.map do |holding| - exchange_rate = exchange_rates.find { |er| er.date == holding.date } - - next unless exchange_rate.present? - - account.holdings.build( - security: holding.security, - date: holding.date, - qty: holding.qty, - price: exchange_rate.rate * holding.price, - amount: exchange_rate.rate * holding.amount, - currency: to_currency - ) - end.compact - - Account.transaction do - load_balances(converted_balances) - load_holdings(converted_holdings) - end - end - - def load_balances(balances = []) - current_time = Time.now - account.balances.upsert_all( - balances.map { |b| b.attributes - .slice("date", "balance", "cash_balance", "currency") - .merge("updated_at" => current_time) }, - unique_by: %i[account_id date currency] - ) - end - - def load_holdings(holdings = []) - current_time = Time.now - account.holdings.upsert_all( - holdings.map { |h| h.attributes - .slice("date", "currency", "qty", "price", "amount", "security_id") - .merge("updated_at" => current_time) }, - unique_by: %i[account_id security_id date currency] - ) - end - - def purge_outdated_balances - account.balances.delete_by("date < ?", account_start_date) - end - - def plaid_sync? - account.plaid_account_id.present? - end - - def purge_outdated_holdings - portfolio_security_ids = account.entries.account_trades.map { |entry| entry.entryable.security_id }.uniq - - # If there are no securities in the portfolio, delete all holdings - if portfolio_security_ids.empty? - account.holdings.delete_all - else - account.holdings.delete_by("date < ? OR security_id NOT IN (?)", account_start_date, portfolio_security_ids) - end - end -end diff --git a/app/models/plaid_item.rb b/app/models/plaid_item.rb index c76af8aa..9ffdadf1 100644 --- a/app/models/plaid_item.rb +++ b/app/models/plaid_item.rb @@ -45,6 +45,11 @@ class PlaidItem < ApplicationRecord plaid_data = fetch_and_load_plaid_data update!(status: :good) if requires_update? + # Schedule account syncs + accounts.each do |account| + account.sync_later(start_date: start_date) + end + Rails.logger.info("Plaid data fetched and loaded") plaid_data rescue Plaid::ApiError => e diff --git a/app/views/accounts/chart.html.erb b/app/views/accounts/chart.html.erb index 22e2528e..6be29472 100644 --- a/app/views/accounts/chart.html.erb +++ b/app/views/accounts/chart.html.erb @@ -1,4 +1,4 @@ -<% series = @account.balance_series(period: @period) %> +<% series = @account.balance_series(period: @period, view: @chart_view) %> <% trend = series.trend %> <%= turbo_frame_tag dom_id(@account, :chart_details) do %> diff --git a/app/views/accounts/show/_chart.html.erb b/app/views/accounts/show/_chart.html.erb index c78a3676..e6dadae8 100644 --- a/app/views/accounts/show/_chart.html.erb +++ b/app/views/accounts/show/_chart.html.erb @@ -1,6 +1,6 @@ -<%# locals: (account:, title: nil, tooltip: nil, **args) %> +<%# locals: (account:, title: nil, tooltip: nil, chart_view: nil, **args) %> -<% period = @period || Period.last_30_days %> +<% period = @period || Period.last_30_days %> <% default_value_title = account.asset? ? t(".balance") : t(".owed") %>
      @@ -15,11 +15,22 @@
      <%= form_with url: request.path, method: :get, data: { controller: "auto-submit-form" } do |form| %> - <%= period_select form: form, selected: period %> +
      + <% if chart_view.present? %> + <%= form.select :chart_view, + [["Total value", "balance"], ["Holdings", "holdings_balance"], ["Cash", "cash_balance"]], + { selected: chart_view }, + class: "border border-secondary rounded-lg text-sm pr-7 cursor-pointer text-primary focus:outline-hidden focus:ring-0", + data: { "auto-submit-form-target": "auto" } + %> + <% end %> + + <%= period_select form: form, selected: period %> +
      <% end %>
      - <%= turbo_frame_tag dom_id(account, :chart_details), src: chart_account_path(account, period: period.key) do %> + <%= turbo_frame_tag dom_id(account, :chart_details), src: chart_account_path(account, period: period.key, chart_view: chart_view) do %> <%= render "accounts/chart_loader" %> <% end %>
      diff --git a/app/views/investments/show.html.erb b/app/views/investments/show.html.erb index a1e34e49..7bd7da3b 100644 --- a/app/views/investments/show.html.erb +++ b/app/views/investments/show.html.erb @@ -7,6 +7,7 @@ <%= render "accounts/show/chart", account: @account, title: t(".chart_title"), + chart_view: @chart_view, tooltip: render( "investments/value_tooltip", balance: @account.balance_money, diff --git a/test/models/account/balance/forward_calculator_test.rb b/test/models/account/balance/forward_calculator_test.rb new file mode 100644 index 00000000..cb96572f --- /dev/null +++ b/test/models/account/balance/forward_calculator_test.rb @@ -0,0 +1,74 @@ +require "test_helper" + +class Account::Balance::ForwardCalculatorTest < ActiveSupport::TestCase + include Account::EntriesTestHelper + + setup do + @account = families(:empty).accounts.create!( + name: "Test", + balance: 20000, + cash_balance: 20000, + currency: "USD", + accountable: Investment.new + ) + end + + # When syncing forwards, we don't care about the account balance. We generate everything based on entries, starting from 0. + test "no entries sync" do + assert_equal 0, @account.balances.count + + expected = [ 0, 0 ] + calculated = Account::Balance::ForwardCalculator.new(@account).calculate + + assert_equal expected, calculated.map(&:balance) + end + + test "valuations sync" do + create_valuation(account: @account, date: 4.days.ago.to_date, amount: 17000) + create_valuation(account: @account, date: 2.days.ago.to_date, amount: 19000) + + expected = [ 0, 17000, 17000, 19000, 19000, 19000 ] + calculated = Account::Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + + assert_equal expected, calculated + end + + test "transactions sync" do + create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) # income + create_transaction(account: @account, date: 2.days.ago.to_date, amount: 100) # expense + + expected = [ 0, 500, 500, 400, 400, 400 ] + calculated = Account::Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + + assert_equal expected, calculated + end + + test "multi-entry sync" do + create_transaction(account: @account, date: 8.days.ago.to_date, amount: -5000) + create_valuation(account: @account, date: 6.days.ago.to_date, amount: 17000) + create_transaction(account: @account, date: 6.days.ago.to_date, amount: -500) + create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) + create_valuation(account: @account, date: 3.days.ago.to_date, amount: 17000) + create_transaction(account: @account, date: 1.day.ago.to_date, amount: 100) + + expected = [ 0, 5000, 5000, 17000, 17000, 17500, 17000, 17000, 16900, 16900 ] + calculated = Account::Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + + assert_equal expected, calculated + end + + test "multi-currency sync" do + ExchangeRate.create! date: 1.day.ago.to_date, from_currency: "EUR", to_currency: "USD", rate: 1.2 + + create_transaction(account: @account, date: 3.days.ago.to_date, amount: -100, currency: "USD") + create_transaction(account: @account, date: 2.days.ago.to_date, amount: -300, currency: "USD") + + # Transaction in different currency than the account's main currency + create_transaction(account: @account, date: 1.day.ago.to_date, amount: -500, currency: "EUR") # €500 * 1.2 = $600 + + expected = [ 0, 100, 400, 1000, 1000 ] + calculated = Account::Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + + assert_equal expected, calculated + end +end diff --git a/test/models/account/balance/reverse_calculator_test.rb b/test/models/account/balance/reverse_calculator_test.rb new file mode 100644 index 00000000..e81c9eb5 --- /dev/null +++ b/test/models/account/balance/reverse_calculator_test.rb @@ -0,0 +1,59 @@ +require "test_helper" + +class Account::Balance::ReverseCalculatorTest < ActiveSupport::TestCase + include Account::EntriesTestHelper + + setup do + @account = families(:empty).accounts.create!( + name: "Test", + balance: 20000, + cash_balance: 20000, + currency: "USD", + accountable: Investment.new + ) + end + + # When syncing backwards, we start with the account balance and generate everything from there. + test "no entries sync" do + assert_equal 0, @account.balances.count + + expected = [ @account.balance, @account.balance ] + calculated = Account::Balance::ReverseCalculator.new(@account).calculate + + assert_equal expected, calculated.map(&:balance) + end + + test "valuations sync" do + create_valuation(account: @account, date: 4.days.ago.to_date, amount: 17000) + create_valuation(account: @account, date: 2.days.ago.to_date, amount: 19000) + + expected = [ 17000, 17000, 19000, 19000, 20000, 20000 ] + calculated = Account::Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + + assert_equal expected, calculated + end + + test "transactions sync" do + create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) # income + create_transaction(account: @account, date: 2.days.ago.to_date, amount: 100) # expense + + expected = [ 19600, 20100, 20100, 20000, 20000, 20000 ] + calculated = Account::Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + + assert_equal expected, calculated + end + + test "multi-entry sync" do + create_transaction(account: @account, date: 8.days.ago.to_date, amount: -5000) + create_valuation(account: @account, date: 6.days.ago.to_date, amount: 17000) + create_transaction(account: @account, date: 6.days.ago.to_date, amount: -500) + create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) + create_valuation(account: @account, date: 3.days.ago.to_date, amount: 17000) + create_transaction(account: @account, date: 1.day.ago.to_date, amount: 100) + + expected = [ 12000, 17000, 17000, 17000, 16500, 17000, 17000, 20100, 20000, 20000 ] + calculated = Account::Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + + assert_equal expected, calculated + end +end diff --git a/test/models/account/balance/syncer_test.rb b/test/models/account/balance/syncer_test.rb new file mode 100644 index 00000000..72dfc568 --- /dev/null +++ b/test/models/account/balance/syncer_test.rb @@ -0,0 +1,51 @@ +require "test_helper" + +class Account::Balance::SyncerTest < ActiveSupport::TestCase + include Account::EntriesTestHelper + + setup do + @account = families(:empty).accounts.create!( + name: "Test", + balance: 20000, + cash_balance: 20000, + currency: "USD", + accountable: Investment.new + ) + end + + test "syncs balances" do + Account::Holding::Syncer.any_instance.expects(:sync_holdings).returns([]).once + + @account.expects(:start_date).returns(2.days.ago.to_date) + + Account::Balance::ForwardCalculator.any_instance.expects(:calculate).returns( + [ + Account::Balance.new(date: 1.day.ago.to_date, balance: 1000, cash_balance: 1000, currency: "USD"), + Account::Balance.new(date: Date.current, balance: 1000, cash_balance: 1000, currency: "USD") + ] + ) + + assert_difference "@account.balances.count", 2 do + Account::Balance::Syncer.new(@account, strategy: :forward).sync_balances + end + end + + test "purges stale balances and holdings" do + # Balance before start date is stale + @account.expects(:start_date).returns(2.days.ago.to_date).twice + stale_balance = Account::Balance.new(date: 3.days.ago.to_date, balance: 10000, cash_balance: 10000, currency: "USD") + + Account::Balance::ForwardCalculator.any_instance.expects(:calculate).returns( + [ + stale_balance, + Account::Balance.new(date: 2.days.ago.to_date, balance: 10000, cash_balance: 10000, currency: "USD"), + Account::Balance.new(date: 1.day.ago.to_date, balance: 1000, cash_balance: 1000, currency: "USD"), + Account::Balance.new(date: Date.current, balance: 1000, cash_balance: 1000, currency: "USD") + ] + ) + + assert_difference "@account.balances.count", 3 do + Account::Balance::Syncer.new(@account, strategy: :forward).sync_balances + end + end +end diff --git a/test/models/account/balance_calculator_test.rb b/test/models/account/balance_calculator_test.rb deleted file mode 100644 index 2f3879a8..00000000 --- a/test/models/account/balance_calculator_test.rb +++ /dev/null @@ -1,156 +0,0 @@ -require "test_helper" - -class Account::BalanceCalculatorTest < ActiveSupport::TestCase - include Account::EntriesTestHelper - - setup do - @account = families(:empty).accounts.create!( - name: "Test", - balance: 20000, - cash_balance: 20000, - currency: "USD", - accountable: Investment.new - ) - end - - # When syncing backwards, we start with the account balance and generate everything from there. - test "reverse no entries sync" do - assert_equal 0, @account.balances.count - - expected = [ @account.balance ] - calculated = Account::BalanceCalculator.new(@account).calculate(reverse: true) - - assert_equal expected, calculated.map(&:balance) - end - - # When syncing forwards, we don't care about the account balance. We generate everything based on entries, starting from 0. - test "forward no entries sync" do - assert_equal 0, @account.balances.count - - expected = [ 0 ] - calculated = Account::BalanceCalculator.new(@account).calculate - - assert_equal expected, calculated.map(&:balance) - end - - test "forward valuations sync" do - create_valuation(account: @account, date: 4.days.ago.to_date, amount: 17000) - create_valuation(account: @account, date: 2.days.ago.to_date, amount: 19000) - - expected = [ 0, 17000, 17000, 19000, 19000, 19000 ] - calculated = Account::BalanceCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) - - assert_equal expected, calculated - end - - test "reverse valuations sync" do - create_valuation(account: @account, date: 4.days.ago.to_date, amount: 17000) - create_valuation(account: @account, date: 2.days.ago.to_date, amount: 19000) - - expected = [ 17000, 17000, 19000, 19000, 20000, 20000 ] - calculated = Account::BalanceCalculator.new(@account).calculate(reverse: true).sort_by(&:date).map(&:balance) - - assert_equal expected, calculated - end - - test "forward transactions sync" do - create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) # income - create_transaction(account: @account, date: 2.days.ago.to_date, amount: 100) # expense - - expected = [ 0, 500, 500, 400, 400, 400 ] - calculated = Account::BalanceCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) - - assert_equal expected, calculated - end - - test "reverse transactions sync" do - create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) # income - create_transaction(account: @account, date: 2.days.ago.to_date, amount: 100) # expense - - expected = [ 19600, 20100, 20100, 20000, 20000, 20000 ] - calculated = Account::BalanceCalculator.new(@account).calculate(reverse: true).sort_by(&:date).map(&:balance) - - assert_equal expected, calculated - end - - test "reverse multi-entry sync" do - create_transaction(account: @account, date: 8.days.ago.to_date, amount: -5000) - create_valuation(account: @account, date: 6.days.ago.to_date, amount: 17000) - create_transaction(account: @account, date: 6.days.ago.to_date, amount: -500) - create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) - create_valuation(account: @account, date: 3.days.ago.to_date, amount: 17000) - create_transaction(account: @account, date: 1.day.ago.to_date, amount: 100) - - expected = [ 12000, 17000, 17000, 17000, 16500, 17000, 17000, 20100, 20000, 20000 ] - calculated = Account::BalanceCalculator.new(@account).calculate(reverse: true) .sort_by(&:date).map(&:balance) - - assert_equal expected, calculated - end - - test "forward multi-entry sync" do - create_transaction(account: @account, date: 8.days.ago.to_date, amount: -5000) - create_valuation(account: @account, date: 6.days.ago.to_date, amount: 17000) - create_transaction(account: @account, date: 6.days.ago.to_date, amount: -500) - create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) - create_valuation(account: @account, date: 3.days.ago.to_date, amount: 17000) - create_transaction(account: @account, date: 1.day.ago.to_date, amount: 100) - - expected = [ 0, 5000, 5000, 17000, 17000, 17500, 17000, 17000, 16900, 16900 ] - calculated = Account::BalanceCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) - - assert_equal expected, calculated - end - - test "investment balance sync" do - @account.update!(cash_balance: 18000) - - # Transactions represent deposits / withdrawals from the brokerage account - # Ex: We deposit $20,000 into the brokerage account - create_transaction(account: @account, date: 2.days.ago.to_date, amount: -20000) - - # Trades either consume cash (buy) or generate cash (sell). They do NOT change total balance, but do affect composition of cash/holdings. - # Ex: We buy 20 shares of MSFT at $100 for a total of $2000 - create_trade(securities(:msft), account: @account, date: 1.day.ago.to_date, qty: 20, price: 100) - - holdings = [ - Account::Holding.new(date: Date.current, security: securities(:msft), amount: 2000, currency: "USD"), - Account::Holding.new(date: 1.day.ago.to_date, security: securities(:msft), amount: 2000, currency: "USD"), - Account::Holding.new(date: 2.days.ago.to_date, security: securities(:msft), amount: 0, currency: "USD") - ] - - expected = [ 0, 20000, 20000, 20000 ] - calculated_backwards = Account::BalanceCalculator.new(@account, holdings: holdings).calculate(reverse: true).sort_by(&:date).map(&:balance) - calculated_forwards = Account::BalanceCalculator.new(@account, holdings: holdings).calculate.sort_by(&:date).map(&:balance) - - assert_equal calculated_forwards, calculated_backwards - assert_equal expected, calculated_forwards - end - - test "multi-currency sync" do - ExchangeRate.create! date: 1.day.ago.to_date, from_currency: "EUR", to_currency: "USD", rate: 1.2 - - create_transaction(account: @account, date: 3.days.ago.to_date, amount: -100, currency: "USD") - create_transaction(account: @account, date: 2.days.ago.to_date, amount: -300, currency: "USD") - - # Transaction in different currency than the account's main currency - create_transaction(account: @account, date: 1.day.ago.to_date, amount: -500, currency: "EUR") # €500 * 1.2 = $600 - - expected = [ 0, 100, 400, 1000, 1000 ] - calculated = Account::BalanceCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) - - assert_equal expected, calculated - end - - private - def create_holding(date:, security:, amount:) - Account::Holding.create!( - account: @account, - security: security, - date: date, - qty: 0, # not used - price: 0, # not used - amount: amount, - currency: @account.currency - ) - end -end diff --git a/test/models/account/holding/forward_calculator_test.rb b/test/models/account/holding/forward_calculator_test.rb new file mode 100644 index 00000000..70fd0e2a --- /dev/null +++ b/test/models/account/holding/forward_calculator_test.rb @@ -0,0 +1,146 @@ +require "test_helper" + +class Account::Holding::ForwardCalculatorTest < ActiveSupport::TestCase + include Account::EntriesTestHelper + + setup do + @account = families(:empty).accounts.create!( + name: "Test", + balance: 20000, + cash_balance: 20000, + currency: "USD", + accountable: Investment.new + ) + end + + test "no holdings" do + calculated = Account::Holding::ForwardCalculator.new(@account).calculate + assert_equal [], calculated + end + + test "forward portfolio calculation" do + load_prices + + # Build up to 10 shares of VOO (current value $5000) + create_trade(@voo, qty: 20, date: 3.days.ago.to_date, price: 470, account: @account) + create_trade(@voo, qty: -15, date: 2.days.ago.to_date, price: 480, account: @account) + create_trade(@voo, qty: 5, date: 1.day.ago.to_date, price: 490, account: @account) + + # Amazon won't exist in current holdings because qty is zero, but should show up in historical portfolio + create_trade(@amzn, qty: 1, date: 2.days.ago.to_date, price: 200, account: @account) + create_trade(@amzn, qty: -1, date: 1.day.ago.to_date, price: 200, account: @account) + + # Build up to 100 shares of WMT (current value $10000) + create_trade(@wmt, qty: 100, date: 1.day.ago.to_date, price: 100, account: @account) + + expected = [ + # 4 days ago + Account::Holding.new(security: @voo, date: 4.days.ago.to_date, qty: 0, price: 460, amount: 0), + Account::Holding.new(security: @wmt, date: 4.days.ago.to_date, qty: 0, price: 100, amount: 0), + Account::Holding.new(security: @amzn, date: 4.days.ago.to_date, qty: 0, price: 200, amount: 0), + + # 3 days ago + Account::Holding.new(security: @voo, date: 3.days.ago.to_date, qty: 20, price: 470, amount: 9400), + Account::Holding.new(security: @wmt, date: 3.days.ago.to_date, qty: 0, price: 100, amount: 0), + Account::Holding.new(security: @amzn, date: 3.days.ago.to_date, qty: 0, price: 200, amount: 0), + + # 2 days ago + Account::Holding.new(security: @voo, date: 2.days.ago.to_date, qty: 5, price: 480, amount: 2400), + Account::Holding.new(security: @wmt, date: 2.days.ago.to_date, qty: 0, price: 100, amount: 0), + Account::Holding.new(security: @amzn, date: 2.days.ago.to_date, qty: 1, price: 200, amount: 200), + + # 1 day ago + Account::Holding.new(security: @voo, date: 1.day.ago.to_date, qty: 10, price: 490, amount: 4900), + Account::Holding.new(security: @wmt, date: 1.day.ago.to_date, qty: 100, price: 100, amount: 10000), + Account::Holding.new(security: @amzn, date: 1.day.ago.to_date, qty: 0, price: 200, amount: 0), + + # Today + Account::Holding.new(security: @voo, date: Date.current, qty: 10, price: 500, amount: 5000), + Account::Holding.new(security: @wmt, date: Date.current, qty: 100, price: 100, amount: 10000), + Account::Holding.new(security: @amzn, date: Date.current, qty: 0, price: 200, amount: 0) + ] + + calculated = Account::Holding::ForwardCalculator.new(@account).calculate + + assert_equal expected.length, calculated.length + assert_holdings(expected, calculated) + end + + # Carries the previous record forward if no holding exists for a date + # to ensure that net worth historical rollups have a value for every date + test "uses locf to fill missing holdings" do + load_prices + + create_trade(@wmt, qty: 100, date: 1.day.ago.to_date, price: 100, account: @account) + + expected = [ + Account::Holding.new(security: @wmt, date: 2.days.ago.to_date, qty: 0, price: 100, amount: 0), + Account::Holding.new(security: @wmt, date: 1.day.ago.to_date, qty: 100, price: 100, amount: 10000), + Account::Holding.new(security: @wmt, date: Date.current, qty: 100, price: 100, amount: 10000) + ] + + # Price missing today, so we should carry forward the holding from 1 day ago + Security.stubs(:find).returns(@wmt) + Security::Price.stubs(:find_price).with(security: @wmt, date: 2.days.ago.to_date).returns(Security::Price.new(price: 100)) + Security::Price.stubs(:find_price).with(security: @wmt, date: 1.day.ago.to_date).returns(Security::Price.new(price: 100)) + Security::Price.stubs(:find_price).with(security: @wmt, date: Date.current).returns(nil) + + calculated = Account::Holding::ForwardCalculator.new(@account).calculate + + assert_equal expected.length, calculated.length + assert_holdings(expected, calculated) + end + + test "offline tickers sync holdings based on most recent trade price" do + offline_security = Security.create!(ticker: "OFFLINE", name: "Offline Ticker") + + create_trade(offline_security, qty: 1, date: 3.days.ago.to_date, price: 90, account: @account) + create_trade(offline_security, qty: 1, date: 1.day.ago.to_date, price: 100, account: @account) + + expected = [ + Account::Holding.new(security: offline_security, date: 3.days.ago.to_date, qty: 1, price: 90, amount: 90), + Account::Holding.new(security: offline_security, date: 2.days.ago.to_date, qty: 1, price: 90, amount: 90), + Account::Holding.new(security: offline_security, date: 1.day.ago.to_date, qty: 2, price: 100, amount: 200), + Account::Holding.new(security: offline_security, date: Date.current, qty: 2, price: 100, amount: 200) + ] + + calculated = Account::Holding::ForwardCalculator.new(@account).calculate + + assert_equal expected.length, calculated.length + assert_holdings(expected, calculated) + end + + private + def assert_holdings(expected, calculated) + expected.each do |expected_entry| + calculated_entry = calculated.find { |c| c.security == expected_entry.security && c.date == expected_entry.date } + + assert_equal expected_entry.qty, calculated_entry.qty, "Qty mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}" + assert_equal expected_entry.price, calculated_entry.price, "Price mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}" + assert_equal expected_entry.amount, calculated_entry.amount, "Amount mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}" + end + end + + def load_prices + @voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF") + Security::Price.create!(security: @voo, date: 4.days.ago.to_date, price: 460) + Security::Price.create!(security: @voo, date: 3.days.ago.to_date, price: 470) + Security::Price.create!(security: @voo, date: 2.days.ago.to_date, price: 480) + Security::Price.create!(security: @voo, date: 1.day.ago.to_date, price: 490) + Security::Price.create!(security: @voo, date: Date.current, price: 500) + + @wmt = Security.create!(ticker: "WMT", name: "Walmart Inc.") + Security::Price.create!(security: @wmt, date: 4.days.ago.to_date, price: 100) + Security::Price.create!(security: @wmt, date: 3.days.ago.to_date, price: 100) + Security::Price.create!(security: @wmt, date: 2.days.ago.to_date, price: 100) + Security::Price.create!(security: @wmt, date: 1.day.ago.to_date, price: 100) + Security::Price.create!(security: @wmt, date: Date.current, price: 100) + + @amzn = Security.create!(ticker: "AMZN", name: "Amazon.com Inc.") + Security::Price.create!(security: @amzn, date: 4.days.ago.to_date, price: 200) + Security::Price.create!(security: @amzn, date: 3.days.ago.to_date, price: 200) + Security::Price.create!(security: @amzn, date: 2.days.ago.to_date, price: 200) + Security::Price.create!(security: @amzn, date: 1.day.ago.to_date, price: 200) + Security::Price.create!(security: @amzn, date: Date.current, price: 200) + end +end diff --git a/test/models/account/holding/portfolio_cache_test.rb b/test/models/account/holding/portfolio_cache_test.rb new file mode 100644 index 00000000..b973fa00 --- /dev/null +++ b/test/models/account/holding/portfolio_cache_test.rb @@ -0,0 +1,63 @@ +require "test_helper" + +class Account::Holding::PortfolioCacheTest < ActiveSupport::TestCase + include Account::EntriesTestHelper + + setup do + # Prices, highest to lowest priority + @db_price = 210 + @provider_price = 220 + @trade_price = 200 + @holding_price = 250 + + @account = families(:empty).accounts.create!(name: "Test Brokerage", balance: 10000, currency: "USD", accountable: Investment.new) + @test_security = Security.create!(name: "Test Security", ticker: "TEST") + + @trade = create_trade(@test_security, account: @account, qty: 1, date: Date.current, price: @trade_price) + @holding = Account::Holding.create!(security: @test_security, account: @account, date: Date.current, qty: 1, price: @holding_price, amount: @holding_price, currency: "USD") + Security::Price.create!(security: @test_security, date: Date.current, price: @db_price) + end + + test "gets price from DB if available" do + cache = Account::Holding::PortfolioCache.new(@account) + + assert_equal @db_price, cache.get_price(@test_security.id, Date.current).price + end + + test "if no price in DB, try fetching from provider" do + Security::Price.destroy_all + Security::Price.expects(:find_prices) + .with(security: @test_security, start_date: @account.start_date, end_date: Date.current) + .returns([ + Security::Price.new(security: @test_security, date: Date.current, price: @provider_price, currency: "USD") + ]) + + cache = Account::Holding::PortfolioCache.new(@account) + + assert_equal @provider_price, cache.get_price(@test_security.id, Date.current).price + end + + test "if no price from db or provider, try getting the price from trades" do + Security::Price.destroy_all # No DB prices + Security::Price.expects(:find_prices) + .with(security: @test_security, start_date: @account.start_date, end_date: Date.current) + .returns([]) # No provider prices + + cache = Account::Holding::PortfolioCache.new(@account) + + assert_equal @trade_price, cache.get_price(@test_security.id, Date.current).price + end + + test "if no price from db, provider, or trades, search holdings" do + Security::Price.destroy_all # No DB prices + Security::Price.expects(:find_prices) + .with(security: @test_security, start_date: @account.start_date, end_date: Date.current) + .returns([]) # No provider prices + + @account.entries.destroy_all # No prices from trades + + cache = Account::Holding::PortfolioCache.new(@account, use_holdings: true) + + assert_equal @holding_price, cache.get_price(@test_security.id, Date.current).price + end +end diff --git a/test/models/account/holding_calculator_test.rb b/test/models/account/holding/reverse_calculator_test.rb similarity index 61% rename from test/models/account/holding_calculator_test.rb rename to test/models/account/holding/reverse_calculator_test.rb index 154c8afe..6e9535e5 100644 --- a/test/models/account/holding_calculator_test.rb +++ b/test/models/account/holding/reverse_calculator_test.rb @@ -1,6 +1,6 @@ require "test_helper" -class Account::HoldingCalculatorTest < ActiveSupport::TestCase +class Account::Holding::ReverseCalculatorTest < ActiveSupport::TestCase include Account::EntriesTestHelper setup do @@ -14,10 +14,8 @@ class Account::HoldingCalculatorTest < ActiveSupport::TestCase end test "no holdings" do - forward = Account::HoldingCalculator.new(@account).calculate - reverse = Account::HoldingCalculator.new(@account).calculate(reverse: true) - assert_equal forward, reverse - assert_equal [], forward + calculated = Account::Holding::ReverseCalculator.new(@account).calculate + assert_equal [], calculated end # Should be able to handle this case, although we should not be reverse-syncing an account without provided current day holdings @@ -28,7 +26,7 @@ class Account::HoldingCalculatorTest < ActiveSupport::TestCase create_trade(voo, qty: -10, date: Date.current, price: 470, account: @account) - calculated = Account::HoldingCalculator.new(@account).calculate(reverse: true) + calculated = Account::Holding::ReverseCalculator.new(@account).calculate assert_equal 2, calculated.length end @@ -74,7 +72,7 @@ class Account::HoldingCalculatorTest < ActiveSupport::TestCase Account::Holding.new(security: @amzn, date: Date.current, qty: 0, price: 200, amount: 0) ] - calculated = Account::HoldingCalculator.new(@account).calculate(reverse: true) + calculated = Account::Holding::ReverseCalculator.new(@account).calculate assert_equal expected.length, calculated.length @@ -87,80 +85,6 @@ class Account::HoldingCalculatorTest < ActiveSupport::TestCase end end - test "forward portfolio calculation" do - load_prices - - # Build up to 10 shares of VOO (current value $5000) - create_trade(@voo, qty: 20, date: 3.days.ago.to_date, price: 470, account: @account) - create_trade(@voo, qty: -15, date: 2.days.ago.to_date, price: 480, account: @account) - create_trade(@voo, qty: 5, date: 1.day.ago.to_date, price: 490, account: @account) - - # Amazon won't exist in current holdings because qty is zero, but should show up in historical portfolio - create_trade(@amzn, qty: 1, date: 2.days.ago.to_date, price: 200, account: @account) - create_trade(@amzn, qty: -1, date: 1.day.ago.to_date, price: 200, account: @account) - - # Build up to 100 shares of WMT (current value $10000) - create_trade(@wmt, qty: 100, date: 1.day.ago.to_date, price: 100, account: @account) - - expected = [ - # 4 days ago - Account::Holding.new(security: @voo, date: 4.days.ago.to_date, qty: 0, price: 460, amount: 0), - Account::Holding.new(security: @wmt, date: 4.days.ago.to_date, qty: 0, price: 100, amount: 0), - Account::Holding.new(security: @amzn, date: 4.days.ago.to_date, qty: 0, price: 200, amount: 0), - - # 3 days ago - Account::Holding.new(security: @voo, date: 3.days.ago.to_date, qty: 20, price: 470, amount: 9400), - Account::Holding.new(security: @wmt, date: 3.days.ago.to_date, qty: 0, price: 100, amount: 0), - Account::Holding.new(security: @amzn, date: 3.days.ago.to_date, qty: 0, price: 200, amount: 0), - - # 2 days ago - Account::Holding.new(security: @voo, date: 2.days.ago.to_date, qty: 5, price: 480, amount: 2400), - Account::Holding.new(security: @wmt, date: 2.days.ago.to_date, qty: 0, price: 100, amount: 0), - Account::Holding.new(security: @amzn, date: 2.days.ago.to_date, qty: 1, price: 200, amount: 200), - - # 1 day ago - Account::Holding.new(security: @voo, date: 1.day.ago.to_date, qty: 10, price: 490, amount: 4900), - Account::Holding.new(security: @wmt, date: 1.day.ago.to_date, qty: 100, price: 100, amount: 10000), - Account::Holding.new(security: @amzn, date: 1.day.ago.to_date, qty: 0, price: 200, amount: 0), - - # Today - Account::Holding.new(security: @voo, date: Date.current, qty: 10, price: 500, amount: 5000), - Account::Holding.new(security: @wmt, date: Date.current, qty: 100, price: 100, amount: 10000), - Account::Holding.new(security: @amzn, date: Date.current, qty: 0, price: 200, amount: 0) - ] - - calculated = Account::HoldingCalculator.new(@account).calculate - - assert_equal expected.length, calculated.length - assert_holdings(expected, calculated) - end - - # Carries the previous record forward if no holding exists for a date - # to ensure that net worth historical rollups have a value for every date - test "uses locf to fill missing holdings" do - load_prices - - create_trade(@wmt, qty: 100, date: 1.day.ago.to_date, price: 100, account: @account) - - expected = [ - Account::Holding.new(security: @wmt, date: 2.days.ago.to_date, qty: 0, price: 100, amount: 0), - Account::Holding.new(security: @wmt, date: 1.day.ago.to_date, qty: 100, price: 100, amount: 10000), - Account::Holding.new(security: @wmt, date: Date.current, qty: 100, price: 100, amount: 10000) - ] - - # Price missing today, so we should carry forward the holding from 1 day ago - Security.stubs(:find).returns(@wmt) - Security::Price.stubs(:find_price).with(security: @wmt, date: 2.days.ago.to_date).returns(Security::Price.new(price: 100)) - Security::Price.stubs(:find_price).with(security: @wmt, date: 1.day.ago.to_date).returns(Security::Price.new(price: 100)) - Security::Price.stubs(:find_price).with(security: @wmt, date: Date.current).returns(nil) - - calculated = Account::HoldingCalculator.new(@account).calculate - - assert_equal expected.length, calculated.length - - assert_holdings(expected, calculated) - end - private def assert_holdings(expected, calculated) expected.each do |expected_entry| diff --git a/test/models/account/holding/syncer_test.rb b/test/models/account/holding/syncer_test.rb new file mode 100644 index 00000000..43ca5dcb --- /dev/null +++ b/test/models/account/holding/syncer_test.rb @@ -0,0 +1,29 @@ +require "test_helper" + +class Account::Holding::SyncerTest < ActiveSupport::TestCase + include Account::EntriesTestHelper + + setup do + @family = families(:empty) + @account = @family.accounts.create!(name: "Test", balance: 20000, cash_balance: 20000, currency: "USD", accountable: Investment.new) + @aapl = securities(:aapl) + end + + test "syncs holdings" do + create_trade(@aapl, account: @account, qty: 1, price: 200, date: Date.current) + + # Should have yesterday's and today's holdings + assert_difference "@account.holdings.count", 2 do + Account::Holding::Syncer.new(@account, strategy: :forward).sync_holdings + end + end + + test "purges stale holdings for unlinked accounts" do + # Since the account has no entries, there should be no holdings + Account::Holding.create!(account: @account, security: @aapl, qty: 1, price: 100, amount: 100, currency: "USD", date: Date.current) + + assert_difference "Account::Holding.count", -1 do + Account::Holding::Syncer.new(@account, strategy: :forward).sync_holdings + end + end +end diff --git a/test/models/account/syncer_test.rb b/test/models/account/syncer_test.rb deleted file mode 100644 index 5cc85ee9..00000000 --- a/test/models/account/syncer_test.rb +++ /dev/null @@ -1,65 +0,0 @@ -require "test_helper" - -class Account::SyncerTest < ActiveSupport::TestCase - include Account::EntriesTestHelper - - setup do - @account = families(:empty).accounts.create!( - name: "Test", - balance: 20000, - cash_balance: 20000, - currency: "USD", - accountable: Investment.new - ) - end - - test "converts foreign account balances and holdings to family currency" do - @account.family.update! currency: "USD" - @account.update! currency: "EUR" - - @account.entries.create!(date: 1.day.ago.to_date, currency: "EUR", amount: 500, name: "Buy AAPL", entryable: Account::Trade.new(security: securities(:aapl), qty: 10, price: 50, currency: "EUR")) - - ExchangeRate.create!(date: 1.day.ago.to_date, from_currency: "EUR", to_currency: "USD", rate: 1.2) - ExchangeRate.create!(date: Date.current, from_currency: "EUR", to_currency: "USD", rate: 2) - - Account::BalanceCalculator.any_instance.expects(:calculate).returns( - [ - Account::Balance.new(date: 1.day.ago.to_date, balance: 1000, cash_balance: 1000, currency: "EUR"), - Account::Balance.new(date: Date.current, balance: 1000, cash_balance: 1000, currency: "EUR") - ] - ) - - Account::HoldingCalculator.any_instance.expects(:calculate).returns( - [ - Account::Holding.new(security: securities(:aapl), date: 1.day.ago.to_date, qty: 10, price: 50, amount: 500, currency: "EUR"), - Account::Holding.new(security: securities(:aapl), date: Date.current, qty: 10, price: 50, amount: 500, currency: "EUR") - ] - ) - - Account::Syncer.new(@account).run - - assert_equal [ 1000, 1000 ], @account.balances.where(currency: "EUR").chronological.map(&:balance) - assert_equal [ 1200, 2000 ], @account.balances.where(currency: "USD").chronological.map(&:balance) - assert_equal [ 500, 500 ], @account.holdings.where(currency: "EUR").chronological.map(&:amount) - assert_equal [ 600, 1000 ], @account.holdings.where(currency: "USD").chronological.map(&:amount) - end - - test "purges stale balances and holdings" do - # Old, out of range holdings and balances - @account.holdings.create!(security: securities(:aapl), date: 10.years.ago.to_date, currency: "USD", qty: 100, price: 100, amount: 10000) - @account.balances.create!(date: 10.years.ago.to_date, currency: "USD", balance: 10000, cash_balance: 10000) - - assert_equal 1, @account.holdings.count - assert_equal 1, @account.balances.count - - Account::Syncer.new(@account).run - - @account.reload - - assert_equal 0, @account.holdings.count - - # Balance sync always creates 1 balance if no entries present. - assert_equal 1, @account.balances.count - assert_equal 0, @account.balances.first.balance - end -end From 5f8a3c9f508cfbc24f265b9d8f6a6e79dae84ece Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 7 Mar 2025 17:48:26 -0500 Subject: [PATCH 37/47] Search securities with correct exchange mic --- app/models/security/price/provided.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/security/price/provided.rb b/app/models/security/price/provided.rb index aed56702..c429e0a6 100644 --- a/app/models/security/price/provided.rb +++ b/app/models/security/price/provided.rb @@ -15,7 +15,7 @@ module Security::Price::Provided response = provider.fetch_security_prices \ ticker: security.ticker, - mic_code: security.exchange_mic, + mic_code: security.exchange_operating_mic, start_date: date, end_date: date @@ -40,7 +40,7 @@ module Security::Price::Provided response = provider.fetch_security_prices \ ticker: security.ticker, - mic_code: security.exchange_mic, + mic_code: security.exchange_operating_mic, start_date: start_date, end_date: end_date From 86bf47a32ead80b33161d5bc5afaf78ee20f432f Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 7 Mar 2025 18:02:08 -0500 Subject: [PATCH 38/47] Ensure holdings are normalized to account currency --- app/models/account/holding/portfolio_cache.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/account/holding/portfolio_cache.rb b/app/models/account/holding/portfolio_cache.rb index 6a839382..bb6035cf 100644 --- a/app/models/account/holding/portfolio_cache.rb +++ b/app/models/account/holding/portfolio_cache.rb @@ -37,7 +37,7 @@ class Account::Holding::PortfolioCache security_id: security_id, date: price.date, price: converted_amount, - currency: price.currency + currency: account.currency ) end From a3cd5f4f1d589b933d8a400eaf63744b4427da96 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 7 Mar 2025 19:09:54 -0500 Subject: [PATCH 39/47] Format money for trade history in holdings drawer (#1961) * Format money for trade history in holdings drawer * Fix broken tests * Lint fix --- app/views/account/holdings/show.html.erb | 2 +- app/views/accounts/show/_chart.html.erb | 3 +-- lib/money/formatting.rb | 10 +++++++++- test/models/security/price_test.rb | 6 +++--- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/views/account/holdings/show.html.erb b/app/views/account/holdings/show.html.erb index b783ac5b..12199512 100644 --- a/app/views/account/holdings/show.html.erb +++ b/app/views/account/holdings/show.html.erb @@ -73,7 +73,7 @@ ".trade_history_entry", qty: trade_entry.account_trade.qty, security: trade_entry.account_trade.security.ticker, - price: format_money(trade_entry.account_trade.price) + price: trade_entry.account_trade.price_money.format ) %>

      diff --git a/app/views/accounts/show/_chart.html.erb b/app/views/accounts/show/_chart.html.erb index e6dadae8..47c91fb0 100644 --- a/app/views/accounts/show/_chart.html.erb +++ b/app/views/accounts/show/_chart.html.erb @@ -21,8 +21,7 @@ [["Total value", "balance"], ["Holdings", "holdings_balance"], ["Cash", "cash_balance"]], { selected: chart_view }, class: "border border-secondary rounded-lg text-sm pr-7 cursor-pointer text-primary focus:outline-hidden focus:ring-0", - data: { "auto-submit-form-target": "auto" } - %> + data: { "auto-submit-form-target": "auto" } %> <% end %> <%= period_select form: form, selected: period %> diff --git a/lib/money/formatting.rb b/lib/money/formatting.rb index 25185b1a..cd160bef 100644 --- a/lib/money/formatting.rb +++ b/lib/money/formatting.rb @@ -13,7 +13,7 @@ module Money::Formatting local_option_overrides = locale_options(locale) { - unit: currency.symbol, + unit: get_symbol, precision: currency.default_precision, delimiter: currency.delimiter, separator: currency.separator, @@ -22,6 +22,14 @@ module Money::Formatting end private + def get_symbol + if currency.symbol == "$" && currency.iso_code != "USD" + [ currency.iso_code.first(2), currency.symbol ].join + else + currency.symbol + end + end + def locale_options(locale) case [ currency.iso_code, locale.to_sym ] when [ "EUR", :nl ], [ "EUR", :pt ] diff --git a/test/models/security/price_test.rb b/test/models/security/price_test.rb index 66b60469..32dd00f3 100644 --- a/test/models/security/price_test.rb +++ b/test/models/security/price_test.rb @@ -33,7 +33,7 @@ class Security::PriceTest < ActiveSupport::TestCase tomorrow = Date.current + 1.day @provider.expects(:fetch_security_prices) - .with(ticker: security.ticker, mic_code: security.exchange_mic, start_date: tomorrow, end_date: tomorrow) + .with(ticker: security.ticker, mic_code: security.exchange_operating_mic, start_date: tomorrow, end_date: tomorrow) .once .returns( OpenStruct.new( @@ -54,7 +54,7 @@ class Security::PriceTest < ActiveSupport::TestCase Security::Price.delete_all # Clear any existing prices @provider.expects(:fetch_security_prices) - .with(ticker: security.ticker, mic_code: security.exchange_mic, start_date: Date.current, end_date: Date.current) + .with(ticker: security.ticker, mic_code: security.exchange_operating_mic, start_date: Date.current, end_date: Date.current) .once .returns(OpenStruct.new(success?: false)) @@ -91,7 +91,7 @@ class Security::PriceTest < ActiveSupport::TestCase @provider.expects(:fetch_security_prices) .with(ticker: security.ticker, - mic_code: security.exchange_mic, + mic_code: security.exchange_operating_mic, start_date: 2.days.ago.to_date, end_date: 2.days.ago.to_date) .returns(OpenStruct.new(success?: true, prices: [ { date: 2.days.ago.to_date, price: missing_price, currency: "USD" } ])) From 4b19ca50ebc901096937e74cbd20a42c2efbaf9c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Mar 2025 09:13:05 -0400 Subject: [PATCH 40/47] Bump i18n-tasks from 1.0.14 to 1.0.15 (#1974) Bumps [i18n-tasks](https://github.com/glebm/i18n-tasks) from 1.0.14 to 1.0.15. - [Release notes](https://github.com/glebm/i18n-tasks/releases) - [Changelog](https://github.com/glebm/i18n-tasks/blob/main/CHANGES.md) - [Commits](https://github.com/glebm/i18n-tasks/compare/v1.0.14...v1.0.15) --- updated-dependencies: - dependency-name: i18n-tasks dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 5ff46a2a..a18204df 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -208,7 +208,7 @@ GEM railties (>= 7.0.0) i18n (1.14.7) concurrent-ruby (~> 1.0) - i18n-tasks (1.0.14) + i18n-tasks (1.0.15) activesupport (>= 4.0.2) ast (>= 2.1.0) erubi @@ -217,6 +217,7 @@ GEM parser (>= 3.2.2.1) rails-i18n rainbow (>= 2.2.2, < 4.0) + ruby-progressbar (~> 1.8, >= 1.8.1) terminal-table (>= 1.5.1) image_processing (1.14.0) mini_magick (>= 4.9.5, < 6) @@ -293,28 +294,28 @@ GEM net-smtp (0.5.0) net-protocol nio4r (2.7.4) - nokogiri (1.18.2-aarch64-linux-gnu) + nokogiri (1.18.3-aarch64-linux-gnu) racc (~> 1.4) - nokogiri (1.18.2-aarch64-linux-musl) + nokogiri (1.18.3-aarch64-linux-musl) racc (~> 1.4) - nokogiri (1.18.2-arm-linux-gnu) + nokogiri (1.18.3-arm-linux-gnu) racc (~> 1.4) - nokogiri (1.18.2-arm-linux-musl) + nokogiri (1.18.3-arm-linux-musl) racc (~> 1.4) - nokogiri (1.18.2-arm64-darwin) + nokogiri (1.18.3-arm64-darwin) racc (~> 1.4) - nokogiri (1.18.2-x86_64-darwin) + nokogiri (1.18.3-x86_64-darwin) racc (~> 1.4) - nokogiri (1.18.2-x86_64-linux-gnu) + nokogiri (1.18.3-x86_64-linux-gnu) racc (~> 1.4) - nokogiri (1.18.2-x86_64-linux-musl) + nokogiri (1.18.3-x86_64-linux-musl) racc (~> 1.4) octokit (9.2.0) faraday (>= 1, < 3) sawyer (~> 0.9) pagy (9.3.3) parallel (1.26.3) - parser (3.3.7.0) + parser (3.3.7.1) ast (~> 2.4.1) racc pg (1.5.9) @@ -341,7 +342,7 @@ GEM nio4r (~> 2.0) raabro (1.4.0) racc (1.8.1) - rack (3.1.10) + rack (3.1.11) rack-mini-profiler (3.3.1) rack (>= 1.2.0) rack-session (2.1.0) @@ -472,7 +473,7 @@ GEM sorbet-runtime (0.5.11813) stimulus-rails (1.3.4) railties (>= 6.0.0) - stringio (3.1.3) + stringio (3.1.5) stripe (13.4.1) tailwindcss-rails (4.0.0) railties (>= 7.0.0) @@ -517,7 +518,7 @@ GEM websocket-extensions (0.1.5) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (2.7.1) + zeitwerk (2.7.2) PLATFORMS aarch64-linux From 2e4180fbf0e301b7167618d432d38548ed769352 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Mar 2025 09:13:15 -0400 Subject: [PATCH 41/47] Bump turbo-rails from 2.0.11 to 2.0.13 (#1973) Bumps [turbo-rails](https://github.com/hotwired/turbo-rails) from 2.0.11 to 2.0.13. - [Release notes](https://github.com/hotwired/turbo-rails/releases) - [Commits](https://github.com/hotwired/turbo-rails/compare/v2.0.11...v2.0.13) --- updated-dependencies: - dependency-name: turbo-rails dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a18204df..797cd3cc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -489,9 +489,9 @@ GEM unicode-display_width (>= 1.1.1, < 4) thor (1.3.2) timeout (0.4.3) - turbo-rails (2.0.11) - actionpack (>= 6.0.0) - railties (>= 6.0.0) + turbo-rails (2.0.13) + actionpack (>= 7.1.0) + railties (>= 7.1.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (3.1.4) From dc44da6c00bb463c291ab24496191e45423eafa3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Mar 2025 09:13:25 -0400 Subject: [PATCH 42/47] Bump redcarpet from 3.6.0 to 3.6.1 (#1972) Bumps [redcarpet](https://github.com/vmg/redcarpet) from 3.6.0 to 3.6.1. - [Release notes](https://github.com/vmg/redcarpet/releases) - [Changelog](https://github.com/vmg/redcarpet/blob/master/CHANGELOG.md) - [Commits](https://github.com/vmg/redcarpet/compare/v3.6.0...v3.6.1) --- updated-dependencies: - dependency-name: redcarpet dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 797cd3cc..ea60d695 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -396,7 +396,7 @@ GEM logger rdoc (6.12.0) psych (>= 4.0.0) - redcarpet (3.6.0) + redcarpet (3.6.1) regexp_parser (2.10.0) reline (0.6.0) io-console (~> 0.5) From 3f8351abfe3824754f05480ac6599ee4ab3c130a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Mar 2025 09:13:38 -0400 Subject: [PATCH 43/47] Bump rubocop-rails-omakase from 1.0.0 to 1.1.0 (#1971) Bumps [rubocop-rails-omakase](https://github.com/rails/rubocop-rails-omakase) from 1.0.0 to 1.1.0. - [Release notes](https://github.com/rails/rubocop-rails-omakase/releases) - [Commits](https://github.com/rails/rubocop-rails-omakase/compare/v1.0.0...v1.1.0) --- updated-dependencies: - dependency-name: rubocop-rails-omakase dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index ea60d695..4f68d95c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -248,6 +248,7 @@ GEM logger (~> 1.6) letter_opener (1.10.0) launchy (>= 2.2, < 4) + lint_roller (1.1.0) listen (3.9.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) @@ -406,34 +407,33 @@ GEM chunky_png (~> 1.0) rqrcode_core (~> 1.0) rqrcode_core (1.2.0) - rubocop (1.71.0) + rubocop (1.73.2) json (~> 2.3) - language_server-protocol (>= 3.17.0) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) parallel (~> 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 2.9.3, < 3.0) - rubocop-ast (>= 1.36.2, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.38.0) + rubocop-ast (1.38.1) parser (>= 3.3.1.0) - rubocop-minitest (0.36.0) - rubocop (>= 1.61, < 2.0) - rubocop-ast (>= 1.31.1, < 2.0) - rubocop-performance (1.23.1) - rubocop (>= 1.48.1, < 2.0) - rubocop-ast (>= 1.31.1, < 2.0) - rubocop-rails (2.29.1) + rubocop-performance (1.24.0) + lint_roller (~> 1.1) + rubocop (>= 1.72.1, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) + rubocop-rails (2.30.3) activesupport (>= 4.2.0) + lint_roller (~> 1.1) rack (>= 1.1) - rubocop (>= 1.52.0, < 2.0) - rubocop-ast (>= 1.31.1, < 2.0) - rubocop-rails-omakase (1.0.0) - rubocop - rubocop-minitest - rubocop-performance - rubocop-rails + rubocop (>= 1.72.1, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) + rubocop-rails-omakase (1.1.0) + rubocop (>= 1.72) + rubocop-performance (>= 1.24) + rubocop-rails (>= 2.30) ruby-lsp (0.23.9) language_server-protocol (~> 3.17.0) prism (>= 1.2, < 2.0) From 045fa1931c08d38ad5ff29bd3d2ba3a06112687d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Mar 2025 09:15:51 -0400 Subject: [PATCH 44/47] Bump good_job from 4.9.0 to 4.9.3 (#1969) Bumps [good_job](https://github.com/bensheldon/good_job) from 4.9.0 to 4.9.3. - [Release notes](https://github.com/bensheldon/good_job/releases) - [Changelog](https://github.com/bensheldon/good_job/blob/main/CHANGELOG.md) - [Commits](https://github.com/bensheldon/good_job/compare/v4.9.0...v4.9.3) --- updated-dependencies: - dependency-name: good_job dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 4f68d95c..f5db9a44 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -192,7 +192,7 @@ GEM raabro (~> 1.4) globalid (1.2.1) activesupport (>= 6.1) - good_job (4.9.0) + good_job (4.9.3) activejob (>= 6.1.0) activerecord (>= 6.1.0) concurrent-ruby (>= 1.3.1) From 9dcb9e8ed2eebf7f9acea229b18c2ae9fd291b02 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Mar 2025 09:16:01 -0400 Subject: [PATCH 45/47] Bump webmock from 3.25.0 to 3.25.1 (#1968) Bumps [webmock](https://github.com/bblimke/webmock) from 3.25.0 to 3.25.1. - [Changelog](https://github.com/bblimke/webmock/blob/master/CHANGELOG.md) - [Commits](https://github.com/bblimke/webmock/compare/v3.25.0...v3.25.1) --- updated-dependencies: - dependency-name: webmock dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index f5db9a44..86eb1249 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -507,7 +507,7 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) - webmock (3.25.0) + webmock (3.25.1) addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) From c66401dc0f58d9300f37cd0eb050f54abf68712b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Mar 2025 09:34:15 -0400 Subject: [PATCH 46/47] Bump stripe from 13.4.1 to 13.5.0 (#1970) Bumps [stripe](https://github.com/stripe/stripe-ruby) from 13.4.1 to 13.5.0. - [Release notes](https://github.com/stripe/stripe-ruby/releases) - [Changelog](https://github.com/stripe/stripe-ruby/blob/master/CHANGELOG.md) - [Commits](https://github.com/stripe/stripe-ruby/compare/v13.4.1...v13.5.0) --- updated-dependencies: - dependency-name: stripe dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 86eb1249..6fe445f8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -474,7 +474,7 @@ GEM stimulus-rails (1.3.4) railties (>= 6.0.0) stringio (3.1.5) - stripe (13.4.1) + stripe (13.5.0) tailwindcss-rails (4.0.0) railties (>= 7.0.0) tailwindcss-ruby (~> 4.0) From 15d59959cfb4d55ff51b91d7172c68a46234a6e4 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Mon, 10 Mar 2025 11:20:58 -0500 Subject: [PATCH 47/47] Fix issue of syncing notice covering up user menu --- app/views/layouts/shared/_htmldoc.html.erb | 4 ++-- app/views/shared/_syncing_notice.html.erb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/layouts/shared/_htmldoc.html.erb b/app/views/layouts/shared/_htmldoc.html.erb index b53b5880..6123b318 100644 --- a/app/views/layouts/shared/_htmldoc.html.erb +++ b/app/views/layouts/shared/_htmldoc.html.erb @@ -6,8 +6,8 @@ -
      -
      +
      +
      <%= render_flash_notifications %> <% if Current.family&.syncing? %> diff --git a/app/views/shared/_syncing_notice.html.erb b/app/views/shared/_syncing_notice.html.erb index 791a265b..8458a423 100644 --- a/app/views/shared/_syncing_notice.html.erb +++ b/app/views/shared/_syncing_notice.html.erb @@ -1,4 +1,4 @@ -<%= tag.div id: "syncing-notice", class: "flex gap-3 rounded-lg border bg-white p-4 group max-w-80 shadow-xs border-alpha-black-25" do %> +<%= tag.div id: "syncing-notice", class: "flex gap-3 rounded-lg border bg-white p-4 group w-full shadow-xs border-alpha-black-25" do %>
      <%= lucide_icon "loader", class: "w-5 h-5 text-secondary animate-pulse" %>