<%= simplefin_item.institution_name %>
- <% end %> -(deletion in progress...)
+<%= t(".deletion_in_progress") %>
<% end %>+ <%= simplefin_item.institution_summary %> +
+ <% end %> <% if simplefin_item.syncing? %>- <%= simplefin_item.last_synced_at ? "Last synced #{time_ago_in_words(simplefin_item.last_synced_at)} ago" : "Never synced" %> + <% if simplefin_item.last_synced_at %> + <% if simplefin_item.sync_status_summary %> + <%= t(".status_with_summary", timestamp: time_ago_in_words(simplefin_item.last_synced_at), summary: simplefin_item.sync_status_summary) %> + <% else %> + <%= t(".status", timestamp: time_ago_in_words(simplefin_item.last_synced_at)) %> + <% end %> + <% else %> + <%= t(".status_never") %> + <% end %>
<% end %>New accounts ready to set up
-Choose account types for your newly imported SimpleFin accounts.
+<%= t(".setup_needed") %>
+<%= t(".setup_description") %>
<%= render DS::Link.new( - text: "Set Up New Accounts", + text: t(".setup_action"), icon: "settings", variant: "primary", href: setup_accounts_simplefin_item_path(simplefin_item) @@ -94,8 +108,8 @@No accounts found
-This connection doesn't have any synchronized accounts yet.
+<%= t(".no_accounts_title") %>
+<%= t(".no_accounts_description") %>
+ Your SimpleFin connection needs to be updated: +
+<%= @error_message %>
+<%= t(".setup_token.help_text") %>
++ Historical Data Range: +
+ <%= form.date_field :sync_start_date, + label: "Start syncing transactions from:", + value: @simplefin_item.sync_start_date || 1.year.ago.to_date, + min: 3.years.ago.to_date, + max: Date.current, + class: "w-full max-w-xs", + help_text: "Select how far back you want to sync transaction history. Maximum 3 years of history available." %> +@@ -82,7 +103,8 @@ variant: "primary", icon: "plus", type: "submit", - class: "flex-1" + class: "flex-1", + data: { loading_button_target: "button" } ) %> <%= render DS::Link.new( text: "Cancel", diff --git a/config/locales/views/simplefin_items/en.yml b/config/locales/views/simplefin_items/en.yml new file mode 100644 index 000000000..866e8cfcf --- /dev/null +++ b/config/locales/views/simplefin_items/en.yml @@ -0,0 +1,47 @@ +--- +en: + simplefin_items: + create: + success: SimpleFin connection added successfully! Your accounts will appear shortly as they sync in the background. + errors: + blank_token: Please enter a SimpleFin setup token. + invalid_token: Invalid setup token. Please check that you copied the complete token from SimpleFin Bridge. + token_compromised: The setup token may be compromised, expired, or already used. Please create a new one. + create_failed: "Failed to connect: %{message}" + unexpected: An unexpected error occurred. Please try again or contact support. + destroy: + success: SimpleFin connection will be removed + update: + success: SimpleFin connection updated successfully! Your accounts are being reconnected. + errors: + blank_token: Please enter a SimpleFin setup token. + invalid_token: Invalid setup token. Please check that you copied the complete token from SimpleFin Bridge. + token_compromised: The setup token may be compromised, expired, or already used. Please create a new one. + update_failed: "Failed to update connection: %{message}" + unexpected: An unexpected error occurred. Please try again or contact support. + edit: + setup_token: + label: "SimpleFin Setup Token:" + placeholder: "Paste your SimpleFin setup token here..." + help_text: "The token should be a long string starting with letters and numbers" + complete_account_setup: + success: SimpleFin accounts have been set up successfully! Your transactions and holdings are being imported in the background. + simplefin_item: + add_new: Add new connection + confirm_accept: Delete connection + confirm_body: This will permanently delete all the accounts in this group and all associated data. + confirm_title: Delete SimpleFin connection? + delete: Delete + deletion_in_progress: "(deletion in progress...)" + error: Error occurred while syncing data + no_accounts_description: This connection doesn't have any synchronized accounts yet. + no_accounts_title: No accounts found + requires_update: Requires re-authentication + setup_needed: New accounts ready to set up + setup_description: Choose account types for your newly imported SimpleFin accounts. + setup_action: Set Up New Accounts + status: Last synced %{timestamp} ago + status_never: Never synced + status_with_summary: "Last synced %{timestamp} ago • %{summary}" + syncing: Syncing... + update: Update connection \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 7c049f6c4..3c48d7cfe 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -255,7 +255,7 @@ Rails.application.routes.draw do end end - resources :simplefin_items, only: %i[index new create show destroy] do + resources :simplefin_items, only: %i[index new create show edit update destroy] do member do post :sync get :setup_accounts diff --git a/db/migrate/20250813144520_add_institution_fields_to_simplefin_items.rb b/db/migrate/20250813144520_add_institution_fields_to_simplefin_items.rb new file mode 100644 index 000000000..86c0806ab --- /dev/null +++ b/db/migrate/20250813144520_add_institution_fields_to_simplefin_items.rb @@ -0,0 +1,33 @@ +class AddInstitutionFieldsToSimplefinItems < ActiveRecord::Migration[7.2] + def up + # Only add the new fields that don't already exist + # institution_id, institution_name, institution_url, and raw_institution_payload + # already exist from the original create_simplefin_items migration + add_column :simplefin_items, :institution_domain, :string + add_column :simplefin_items, :institution_color, :string + + # Add indexes for performance on commonly queried institution fields + add_index :simplefin_items, :institution_id + add_index :simplefin_items, :institution_domain + add_index :simplefin_items, :institution_name + + # Enforce NOT NULL constraints on required fields + change_column_null :simplefin_items, :institution_id, false + change_column_null :simplefin_items, :institution_name, false + end + + def down + # Revert NOT NULL constraints + change_column_null :simplefin_items, :institution_id, true + change_column_null :simplefin_items, :institution_name, true + + # Remove indexes + remove_index :simplefin_items, :institution_id + remove_index :simplefin_items, :institution_domain + remove_index :simplefin_items, :institution_name + + # Remove the new columns + remove_column :simplefin_items, :institution_domain + remove_column :simplefin_items, :institution_color + end +end diff --git a/db/migrate/20250901004029_add_sync_start_date_to_simplefin_items.rb b/db/migrate/20250901004029_add_sync_start_date_to_simplefin_items.rb new file mode 100644 index 000000000..6c4cb53b6 --- /dev/null +++ b/db/migrate/20250901004029_add_sync_start_date_to_simplefin_items.rb @@ -0,0 +1,5 @@ +class AddSyncStartDateToSimplefinItems < ActiveRecord::Migration[7.2] + def change + add_column :simplefin_items, :sync_start_date, :date + end +end diff --git a/db/migrate/20250901005519_add_sync_stats_to_syncs.rb b/db/migrate/20250901005519_add_sync_stats_to_syncs.rb new file mode 100644 index 000000000..207508631 --- /dev/null +++ b/db/migrate/20250901005519_add_sync_stats_to_syncs.rb @@ -0,0 +1,5 @@ +class AddSyncStatsToSyncs < ActiveRecord::Migration[7.2] + def change + add_column :syncs, :sync_stats, :text + end +end diff --git a/db/migrate/20250901182423_add_raw_holdings_payload_to_simplefin_accounts.rb b/db/migrate/20250901182423_add_raw_holdings_payload_to_simplefin_accounts.rb new file mode 100644 index 000000000..f836d60fa --- /dev/null +++ b/db/migrate/20250901182423_add_raw_holdings_payload_to_simplefin_accounts.rb @@ -0,0 +1,5 @@ +class AddRawHoldingsPayloadToSimplefinAccounts < ActiveRecord::Migration[7.2] + def change + add_column :simplefin_accounts, :raw_holdings_payload, :jsonb + end +end diff --git a/db/migrate/20250901183328_add_external_id_and_cost_basis_to_holdings.rb b/db/migrate/20250901183328_add_external_id_and_cost_basis_to_holdings.rb new file mode 100644 index 000000000..06ea5f7d5 --- /dev/null +++ b/db/migrate/20250901183328_add_external_id_and_cost_basis_to_holdings.rb @@ -0,0 +1,6 @@ +class AddExternalIdAndCostBasisToHoldings < ActiveRecord::Migration[7.2] + def change + add_column :holdings, :external_id, :string + add_column :holdings, :cost_basis, :decimal + end +end diff --git a/db/migrate/20250903010617_add_unique_index_to_holdings_external_id.rb b/db/migrate/20250903010617_add_unique_index_to_holdings_external_id.rb new file mode 100644 index 000000000..62352b67f --- /dev/null +++ b/db/migrate/20250903010617_add_unique_index_to_holdings_external_id.rb @@ -0,0 +1,5 @@ +class AddUniqueIndexToHoldingsExternalId < ActiveRecord::Migration[7.2] + def change + add_index :holdings, [ :account_id, :external_id ], unique: true, name: 'index_holdings_on_account_and_external_id' + end +end diff --git a/db/migrate/20250903015009_fix_holdings_cost_basis_and_external_id_constraints.rb b/db/migrate/20250903015009_fix_holdings_cost_basis_and_external_id_constraints.rb new file mode 100644 index 000000000..25efc1ae4 --- /dev/null +++ b/db/migrate/20250903015009_fix_holdings_cost_basis_and_external_id_constraints.rb @@ -0,0 +1,9 @@ +class FixHoldingsCostBasisAndExternalIdConstraints < ActiveRecord::Migration[7.2] + def change + change_column :holdings, :cost_basis, :decimal, precision: 19, scale: 4 + add_index :holdings, [ :account_id, :external_id ], + unique: true, + where: "external_id IS NOT NULL", + name: "idx_holdings_on_account_id_external_id_unique" + end +end diff --git a/db/schema.rb b/db/schema.rb index 90d190dcf..315069252 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_08_08_143007) do +ActiveRecord::Schema[7.2].define(version: 2025_09_03_015009) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -29,7 +29,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_08_08_143007) do t.uuid "accountable_id" t.decimal "balance", precision: 19, scale: 4 t.string "currency" - t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true + t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true t.uuid "import_id" t.uuid "plaid_account_id" t.decimal "cash_balance", precision: 19, scale: 4, default: "0.0" @@ -293,6 +293,10 @@ ActiveRecord::Schema[7.2].define(version: 2025_08_08_143007) do t.string "currency", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "external_id" + t.decimal "cost_basis", precision: 19, scale: 4 + t.index ["account_id", "external_id"], name: "idx_holdings_on_account_id_external_id_unique", unique: true, where: "(external_id IS NOT NULL)" + t.index ["account_id", "external_id"], name: "index_holdings_on_account_and_external_id", unique: true t.index ["account_id", "security_id", "date", "currency"], name: "idx_on_account_id_security_id_date_currency_5323e39f8b", unique: true t.index ["account_id"], name: "index_holdings_on_account_id" t.index ["security_id"], name: "index_holdings_on_security_id" @@ -698,6 +702,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_08_08_143007) do t.datetime "balance_date" t.jsonb "extra" t.jsonb "org_data" + t.jsonb "raw_holdings_payload" t.index ["account_id"], name: "index_simplefin_accounts_on_account_id" t.index ["simplefin_item_id"], name: "index_simplefin_accounts_on_simplefin_item_id" end @@ -716,6 +721,9 @@ ActiveRecord::Schema[7.2].define(version: 2025_08_08_143007) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.boolean "pending_account_setup", default: false, null: false + t.string "institution_domain" + t.string "institution_color" + t.date "sync_start_date" t.index ["family_id"], name: "index_simplefin_items_on_family_id" t.index ["status"], name: "index_simplefin_items_on_status" end @@ -749,6 +757,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_08_08_143007) do t.datetime "failed_at" t.date "window_start_date" t.date "window_end_date" + t.text "sync_stats" t.index ["parent_id"], name: "index_syncs_on_parent_id" t.index ["status"], name: "index_syncs_on_status" t.index ["syncable_type", "syncable_id"], name: "index_syncs_on_syncable" diff --git a/test/controllers/simplefin_items_controller_test.rb b/test/controllers/simplefin_items_controller_test.rb index 1ba03fd4a..26d26475a 100644 --- a/test/controllers/simplefin_items_controller_test.rb +++ b/test/controllers/simplefin_items_controller_test.rb @@ -32,14 +32,207 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest delete simplefin_item_url(@simplefin_item) end - assert_redirected_to simplefin_items_path + assert_redirected_to accounts_path @simplefin_item.reload assert @simplefin_item.scheduled_for_deletion? end test "should sync simplefin item" do post sync_simplefin_item_url(@simplefin_item) - assert_redirected_to simplefin_item_path(@simplefin_item) - assert_equal "Sync started", flash[:notice] + assert_redirected_to accounts_path + end + + test "should get edit" do + @simplefin_item.update!(status: :requires_update) + get edit_simplefin_item_url(@simplefin_item) + assert_response :success + end + + test "should update simplefin item with valid token" do + @simplefin_item.update!(status: :requires_update) + + # Mock the SimpleFin provider to prevent real API calls + mock_provider = mock() + mock_provider.expects(:claim_access_url).with("valid_token").returns("https://example.com/new_access") + mock_provider.expects(:get_accounts).returns({ accounts: [] }).at_least_once + Provider::Simplefin.expects(:new).returns(mock_provider).at_least_once + + # Let the real create_simplefin_item! method run - don't mock it + + patch simplefin_item_url(@simplefin_item), params: { + simplefin_item: { setup_token: "valid_token" } + } + + assert_redirected_to accounts_path + assert_match(/updated successfully/, flash[:notice]) + @simplefin_item.reload + assert @simplefin_item.scheduled_for_deletion? + end + + test "should handle update with invalid token" do + @simplefin_item.update!(status: :requires_update) + + patch simplefin_item_url(@simplefin_item), params: { + simplefin_item: { setup_token: "" } + } + + assert_response :unprocessable_entity + assert_includes response.body, "Please enter a SimpleFin setup token" + end + + test "should transfer accounts when updating simplefin item token" do + @simplefin_item.update!(status: :requires_update) + + # Create old SimpleFin accounts linked to Maybe accounts + old_simplefin_account1 = @simplefin_item.simplefin_accounts.create!( + name: "Test Checking", + account_id: "sf_account_123", + currency: "USD", + current_balance: 1000, + account_type: "depository" + ) + old_simplefin_account2 = @simplefin_item.simplefin_accounts.create!( + name: "Test Savings", + account_id: "sf_account_456", + currency: "USD", + current_balance: 5000, + account_type: "depository" + ) + + # Create Maybe accounts linked to the SimpleFin accounts + maybe_account1 = Account.create!( + family: @family, + name: "Checking Account", + balance: 1000, + currency: "USD", + accountable_type: "Depository", + accountable: Depository.create!(subtype: "checking"), + simplefin_account_id: old_simplefin_account1.id + ) + maybe_account2 = Account.create!( + family: @family, + name: "Savings Account", + balance: 5000, + currency: "USD", + accountable_type: "Depository", + accountable: Depository.create!(subtype: "savings"), + simplefin_account_id: old_simplefin_account2.id + ) + + # Update old SimpleFin accounts to reference the Maybe accounts + old_simplefin_account1.update!(account: maybe_account1) + old_simplefin_account2.update!(account: maybe_account2) + + # Mock only the external API calls, let business logic run + mock_provider = mock() + mock_provider.expects(:claim_access_url).with("valid_token").returns("https://example.com/new_access") + mock_provider.expects(:get_accounts).returns({ + accounts: [ + { + id: "sf_account_123", + name: "Test Checking", + type: "depository", + currency: "USD", + balance: 1000, + transactions: [] + }, + { + id: "sf_account_456", + name: "Test Savings", + type: "depository", + currency: "USD", + balance: 5000, + transactions: [] + } + ] + }).at_least_once + Provider::Simplefin.expects(:new).returns(mock_provider).at_least_once + + # Perform the update + patch simplefin_item_url(@simplefin_item), params: { + simplefin_item: { setup_token: "valid_token" } + } + + assert_redirected_to accounts_path + assert_match(/updated successfully/, flash[:notice]) + + # Verify accounts were transferred to new SimpleFin accounts + assert Account.exists?(maybe_account1.id), "maybe_account1 should still exist" + assert Account.exists?(maybe_account2.id), "maybe_account2 should still exist" + + maybe_account1.reload + maybe_account2.reload + + # Find the new SimpleFin item that was created + new_simplefin_item = @family.simplefin_items.where.not(id: @simplefin_item.id).first + assert_not_nil new_simplefin_item, "New SimpleFin item should have been created" + + new_sf_account1 = new_simplefin_item.simplefin_accounts.find_by(account_id: "sf_account_123") + new_sf_account2 = new_simplefin_item.simplefin_accounts.find_by(account_id: "sf_account_456") + + assert_not_nil new_sf_account1, "New SimpleFin account with ID sf_account_123 should exist" + assert_not_nil new_sf_account2, "New SimpleFin account with ID sf_account_456 should exist" + + assert_equal new_sf_account1.id, maybe_account1.simplefin_account_id + assert_equal new_sf_account2.id, maybe_account2.simplefin_account_id + + # Verify old SimpleFin accounts no longer reference Maybe accounts + old_simplefin_account1.reload + old_simplefin_account2.reload + assert_nil old_simplefin_account1.account + assert_nil old_simplefin_account2.account + + # Verify old SimpleFin item is scheduled for deletion + @simplefin_item.reload + assert @simplefin_item.scheduled_for_deletion? + end + + test "should handle partial account matching during token update" do + @simplefin_item.update!(status: :requires_update) + + # Create old SimpleFin account + old_simplefin_account = @simplefin_item.simplefin_accounts.create!( + name: "Test Checking", + account_id: "sf_account_123", + currency: "USD", + current_balance: 1000, + account_type: "depository" + ) + + # Create Maybe account linked to the SimpleFin account + maybe_account = Account.create!( + family: @family, + name: "Checking Account", + balance: 1000, + currency: "USD", + accountable_type: "Depository", + accountable: Depository.create!(subtype: "checking"), + simplefin_account_id: old_simplefin_account.id + ) + old_simplefin_account.update!(account: maybe_account) + + # Mock only the external API calls, let business logic run + mock_provider = mock() + mock_provider.expects(:claim_access_url).with("valid_token").returns("https://example.com/new_access") + # Return empty accounts list to simulate account was removed from bank + mock_provider.expects(:get_accounts).returns({ accounts: [] }).at_least_once + Provider::Simplefin.expects(:new).returns(mock_provider).at_least_once + + # Perform update + patch simplefin_item_url(@simplefin_item), params: { + simplefin_item: { setup_token: "valid_token" } + } + + assert_redirected_to accounts_path + + # Verify Maybe account still linked to old SimpleFin account (no transfer occurred) + maybe_account.reload + old_simplefin_account.reload + assert_equal old_simplefin_account.id, maybe_account.simplefin_account_id + assert_equal maybe_account, old_simplefin_account.account + + # Old item still scheduled for deletion + @simplefin_item.reload + assert @simplefin_item.scheduled_for_deletion? end end diff --git a/test/models/simplefin_item_test.rb b/test/models/simplefin_item_test.rb index 30a429867..e6f8514c8 100644 --- a/test/models/simplefin_item_test.rb +++ b/test/models/simplefin_item_test.rb @@ -61,4 +61,88 @@ class SimplefinItemTest < ActiveSupport::TestCase assert_equal [ @simplefin_item, item_for_deletion ].sort_by(&:created_at).reverse, ordered_items.to_a end + + test "upserts institution data correctly" do + org_data = { + id: "bank123", + name: "Test Bank", + domain: "testbank.com", + url: "https://testbank.com", + "sfin-url": "https://sfin.testbank.com" + } + + @simplefin_item.upsert_institution_data!(org_data) + + assert_equal "bank123", @simplefin_item.institution_id + assert_equal "Test Bank", @simplefin_item.institution_name + assert_equal "testbank.com", @simplefin_item.institution_domain + assert_equal "https://testbank.com", @simplefin_item.institution_url + assert_equal org_data.stringify_keys, @simplefin_item.raw_institution_payload + end + + test "institution display name fallback works" do + # No institution data + assert_equal @simplefin_item.name, @simplefin_item.institution_display_name + + # With institution name + @simplefin_item.update!(institution_name: "Chase Bank") + assert_equal "Chase Bank", @simplefin_item.institution_display_name + + # With domain fallback + @simplefin_item.update!(institution_name: nil, institution_domain: "chase.com") + assert_equal "chase.com", @simplefin_item.institution_display_name + end + + test "connected institutions returns unique institutions" do + # Create accounts with different institutions + account1 = @simplefin_item.simplefin_accounts.create!( + name: "Checking", + account_id: "acc1", + currency: "USD", + account_type: "checking", + current_balance: 1000, + org_data: { "name" => "Chase Bank", "domain" => "chase.com" } + ) + + account2 = @simplefin_item.simplefin_accounts.create!( + name: "Savings", + account_id: "acc2", + currency: "USD", + account_type: "savings", + current_balance: 2000, + org_data: { "name" => "Wells Fargo", "domain" => "wellsfargo.com" } + ) + + institutions = @simplefin_item.connected_institutions + assert_equal 2, institutions.count + assert_includes institutions.map { |i| i["name"] }, "Chase Bank" + assert_includes institutions.map { |i| i["name"] }, "Wells Fargo" + end + + test "institution summary with multiple institutions" do + # No institutions + assert_equal "No institutions connected", @simplefin_item.institution_summary + + # One institution + @simplefin_item.simplefin_accounts.create!( + name: "Checking", + account_id: "acc1", + currency: "USD", + account_type: "checking", + current_balance: 1000, + org_data: { "name" => "Chase Bank" } + ) + assert_equal "Chase Bank", @simplefin_item.institution_summary + + # Multiple institutions + @simplefin_item.simplefin_accounts.create!( + name: "Savings", + account_id: "acc2", + currency: "USD", + account_type: "savings", + current_balance: 2000, + org_data: { "name" => "Wells Fargo" } + ) + assert_equal "2 institutions", @simplefin_item.institution_summary + end end