QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-04-18 03:24:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: streamline SimpleFIN connection updates for improved efficiency (#631)

Browse Source 
- Introduced `update_access_url!` method to reuse existing SimpleFIN items during reconnections, preserving account linkages.
- Refactored `SimplefinConnectionUpdateJob` to update access URLs in place without creating new items or transferring accounts.
- Adjusted sync logic to leverage `repair_stale_linkages` for seamless reconnections.
- Enhanced `SimplefinItem::Importer` to auto-recover the `good` status if no auth errors are found during sync.
- Updated tests to validate in-place updates and preserved account relationships.

Co-authored-by: Josh Waldrep <joshua.waldrep5+github@gmail.com>
This commit is contained in:
LPW
2026-01-13 03:39:26 -05:00
committed by GitHub
parent 320e087a22
commit 7c3af7d85e
4 changed files with 94 additions and 221 deletions
Download Patch File Download Diff File Expand all files Collapse all files

166
app/jobs/simplefin_connection_update_job.rb
View File

@@ -2,166 +2,30 @@ class SimplefinConnectionUpdateJob < ApplicationJob
queue_as :high_priority
# Disable automatic retries for this job since the setup token is single-use.
# If the token claim succeeds but import fails, retrying would fail at claim.
# If the token claim succeeds but sync fails, retrying would fail at claim.
discard_on Provider::Simplefin::SimplefinError do |job, error|
Rails.logger.error(
"SimplefinConnectionUpdateJob discarded: #{error.class} - #{error.message} " \
"(family_id=#{job.arguments.first[:family_id]}, old_item_id=#{job.arguments.first[:old_simplefin_item_id]})"
"(family_id=#{job.arguments.first[:family_id]}, item_id=#{job.arguments.first[:old_simplefin_item_id]})"
)
end
def perform(family_id:, old_simplefin_item_id:, setup_token:)
family = Family.find(family_id)
old_item = family.simplefin_items.find(old_simplefin_item_id)
simplefin_item = family.simplefin_items.find(old_simplefin_item_id)
# Step 1: Claim the token and create the new item.
# This is the critical step - if it fails, we can safely retry.
# If it succeeds, the token is consumed and we must not retry the claim.
updated_item = family.create_simplefin_item!(
setup_token: setup_token,
item_name: old_item.name
# Step 1: Claim the new token and update the existing item's access_url.
# This preserves all existing account linkages - no need to transfer anything.
simplefin_item.update_access_url!(setup_token: setup_token)
# Step 2: Sync the item to import fresh data.
# The existing repair_stale_linkages logic handles cases where SimpleFIN
# account IDs changed (e.g., user re-added institution in SimpleFIN Bridge).
simplefin_item.sync_later
Rails.logger.info(
"SimplefinConnectionUpdateJob: Successfully updated SimplefinItem #{simplefin_item.id} " \
"with new access_url for family #{family_id}"
)
# Step 2: Import accounts from SimpleFin.
# If this fails, we have an orphaned item but the token is already consumed.
# We handle this gracefully by marking the item and continuing.
begin
updated_item.import_latest_simplefin_data
rescue => e
Rails.logger.error(
"SimplefinConnectionUpdateJob: import failed for new item #{updated_item.id}: " \
"#{e.class} - #{e.message}. Item created but may need manual sync."
)
# Mark the item as needing attention but don't fail the job entirely.
# The item exists and can be synced manually later.
updated_item.update!(status: :requires_update)
# Still proceed to transfer accounts and schedule old item deletion
end
# Step 3: Transfer account links from old to new item.
# This is idempotent and safe to retry.
# Check for linked accounts via BOTH legacy FK and AccountProvider.
ActiveRecord::Base.transaction do
old_item.simplefin_accounts.includes(:account, account_provider: :account).each do |old_account|
# Get the linked account via either system
linked_account = old_account.current_account
next unless linked_account.present?
new_simplefin_account = find_matching_simplefin_account(old_account, updated_item.simplefin_accounts)
next unless new_simplefin_account
# Update legacy FK
linked_account.update!(simplefin_account_id: new_simplefin_account.id)
# Also migrate AccountProvider if it exists
if old_account.account_provider.present?
old_account.account_provider.update!(
provider_type: "SimplefinAccount",
provider_id: new_simplefin_account.id
)
else
# Create AccountProvider for consistency
new_simplefin_account.ensure_account_provider!
end
end
end
# Schedule deletion outside transaction to avoid race condition where
# the job is enqueued even if the transaction rolls back
old_item.destroy_later
# Only mark as good if import succeeded (status wasn't set to requires_update above)
updated_item.update!(status: :good) unless updated_item.requires_update?
end
private
# Find a matching SimpleFin account in the new item's accounts.
# Uses a multi-tier matching strategy:
# 1. Exact account_id match (preferred)
# 2. Fingerprint match (name + institution + account_type)
# 3. Fuzzy name match with same institution (fallback)
def find_matching_simplefin_account(old_account, new_accounts)
exact_match = new_accounts.find_by(account_id: old_account.account_id)
return exact_match if exact_match
old_fingerprint = account_fingerprint(old_account)
fingerprint_match = new_accounts.find { |new_account| account_fingerprint(new_account) == old_fingerprint }
return fingerprint_match if fingerprint_match
old_institution = extract_institution_id(old_account)
old_name_normalized = normalize_account_name(old_account.name)
new_accounts.find do |new_account|
new_institution = extract_institution_id(new_account)
new_name_normalized = normalize_account_name(new_account.name)
next false unless old_institution.present? && old_institution == new_institution
names_similar?(old_name_normalized, new_name_normalized)
end
end
def account_fingerprint(simplefin_account)
institution_id = extract_institution_id(simplefin_account)
name_normalized = normalize_account_name(simplefin_account.name)
account_type = simplefin_account.account_type.to_s.downcase
"#{institution_id}:#{name_normalized}:#{account_type}"
end
def extract_institution_id(simplefin_account)
org_data = simplefin_account.org_data
return nil unless org_data.is_a?(Hash)
org_data["id"] || org_data["domain"] || org_data["name"]&.downcase&.gsub(/\s+/, "_")
end
def normalize_account_name(name)
return "" if name.blank?
name.to_s
.downcase
.gsub(/[^a-z0-9]/, "")
end
def names_similar?(name1, name2)
return false if name1.blank? || name2.blank?
return true if name1 == name2
return true if name1.include?(name2) || name2.include?(name1)
longer = [ name1.length, name2.length ].max
return false if longer == 0
# Use Levenshtein distance for more accurate similarity
distance = levenshtein_distance(name1, name2)
similarity = 1.0 - (distance.to_f / longer)
similarity >= 0.8
end
# Compute Levenshtein edit distance between two strings
def levenshtein_distance(s1, s2)
m, n = s1.length, s2.length
return n if m.zero?
return m if n.zero?
# Use a single array and update in place for memory efficiency
prev_row = (0..n).to_a
curr_row = []
(1..m).each do |i|
curr_row[0] = i
(1..n).each do |j|
cost = s1[i - 1] == s2[j - 1] ? 0 : 1
curr_row[j] = [
prev_row[j] + 1, # deletion
curr_row[j - 1] + 1, # insertion
prev_row[j - 1] + cost # substitution
].min
end
prev_row, curr_row = curr_row, prev_row
end
prev_row[n]
end
end

14
app/models/simplefin_item.rb
View File

@@ -55,6 +55,20 @@ class SimplefinItem < ApplicationRecord
SimplefinItem::Importer.new(self, simplefin_provider: simplefin_provider, sync: sync).import
end
# Update the access_url by claiming a new setup token.
# This is used when reconnecting an existing SimpleFIN connection.
# Unlike create_simplefin_item!, this updates in-place, preserving all account linkages.
def update_access_url!(setup_token:)
new_access_url = simplefin_provider.claim_access_url(setup_token)
update!(
access_url: new_access_url,
status: :good
)
self
end
def process_accounts
# Process accounts linked via BOTH legacy FK and AccountProvider
# Use direct query to ensure fresh data from DB, bypassing any association cache

20
app/models/simplefin_item/importer.rb
View File

@@ -45,6 +45,11 @@ class SimplefinItem::Importer
Rails.logger.info "SimplefinItem::Importer - Using REGULAR SYNC (last_synced_at=#{simplefin_item.last_synced_at&.strftime('%Y-%m-%d %H:%M')})"
import_regular_sync
end
# Reset status to good if no auth errors occurred in this sync.
# This allows the item to recover automatically when a bank's auth issue is resolved
# in SimpleFIN Bridge, without requiring the user to manually reconnect.
maybe_clear_requires_update_status
rescue RateLimitedError => e
stats["rate_limited"] = true
stats["rate_limited_at"] = Time.current.iso8601
@@ -321,6 +326,21 @@ class SimplefinItem::Importer
sync.update_columns(sync_stats: merged) # avoid callbacks/validations during tight loops
end
# Reset status to good if no auth errors occurred in this sync.
# This allows automatic recovery when a bank's auth issue is resolved in SimpleFIN Bridge.
def maybe_clear_requires_update_status
return unless simplefin_item.requires_update?
auth_errors = stats.dig("error_buckets", "auth").to_i
if auth_errors.zero?
simplefin_item.update!(status: :good)
Rails.logger.info(
"SimpleFIN: cleared requires_update status for item ##{simplefin_item.id} " \
"(no auth errors in this sync)"
)
end
end
def import_with_chunked_history
# SimpleFin's actual limit is 60 days (not 365 as documented)
# Use 60-day chunks to stay within limits

115
test/controllers/simplefin_items_controller_test.rb
View File

@@ -182,20 +182,21 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest
assert_includes response.body, I18n.t("simplefin_items.update.errors.blank_token", default: "Please enter a SimpleFIN setup token")
end
test "should transfer accounts when updating simplefin item token" do
test "should update simplefin item access_url in place preserving account linkages" do
@simplefin_item.update!(status: :requires_update)
original_item_id = @simplefin_item.id
token = Base64.strict_encode64("https://example.com/claim")
# Create old SimpleFIN accounts linked to Maybe accounts
old_simplefin_account1 = @simplefin_item.simplefin_accounts.create!(
# Create SimpleFIN accounts linked to Maybe accounts
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!(
simplefin_account2 = @simplefin_item.simplefin_accounts.create!(
name: "Test Savings",
account_id: "sf_account_456",
currency: "USD",
@@ -211,7 +212,7 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest
currency: "USD",
accountable_type: "Depository",
accountable: Depository.create!(subtype: "checking"),
simplefin_account_id: old_simplefin_account1.id
simplefin_account_id: simplefin_account1.id
)
maybe_account2 = Account.create!(
family: @family,
@@ -220,40 +221,19 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest
currency: "USD",
accountable_type: "Depository",
accountable: Depository.create!(subtype: "savings"),
simplefin_account_id: old_simplefin_account2.id
simplefin_account_id: 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)
# Update SimpleFIN accounts to reference the Maybe accounts
simplefin_account1.update!(account: maybe_account1)
simplefin_account2.update!(account: maybe_account2)
# Mock only the external API calls, let business logic run
# Mock only the external API calls
mock_provider = mock()
mock_provider.expects(:claim_access_url).with(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 (async job), but execute enqueued jobs inline so we can
# assert the link transfers.
# Perform the update - job updates access_url and enqueues sync
perform_enqueued_jobs(only: SimplefinConnectionUpdateJob) do
patch simplefin_item_url(@simplefin_item), params: {
simplefin_item: { setup_token: token }
@@ -263,40 +243,33 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to accounts_path
assert_equal "SimpleFIN connection updated.", 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"
# Verify the same SimpleFIN item was updated (not a new one created)
@simplefin_item.reload
assert_equal original_item_id, @simplefin_item.id
assert_equal "https://example.com/new_access", @simplefin_item.access_url
assert_equal "good", @simplefin_item.status
# Verify no duplicate SimpleFIN items were created
assert_equal 1, @family.simplefin_items.count
# Verify account linkages remain intact
maybe_account1.reload
maybe_account2.reload
assert_equal simplefin_account1.id, maybe_account1.simplefin_account_id
assert_equal simplefin_account2.id, maybe_account2.simplefin_account_id
# 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
# The old item will be deleted asynchronously; until then, legacy links should be moved.
# Verify old SimpleFIN item is scheduled for deletion
@simplefin_item.reload
assert @simplefin_item.scheduled_for_deletion?
# Verify item is NOT scheduled for deletion (we updated it, not replaced it)
assert_not @simplefin_item.scheduled_for_deletion?
end
test "should handle partial account matching during token update" do
test "should preserve account linkages when reconnecting even if accounts change" do
@simplefin_item.update!(status: :requires_update)
original_item_id = @simplefin_item.id
token = Base64.strict_encode64("https://example.com/claim")
# Create old SimpleFIN account
old_simplefin_account = @simplefin_item.simplefin_accounts.create!(
# Create SimpleFIN account linked to Maybe account
simplefin_account = @simplefin_item.simplefin_accounts.create!(
name: "Test Checking",
account_id: "sf_account_123",
currency: "USD",
@@ -312,18 +285,16 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest
currency: "USD",
accountable_type: "Depository",
accountable: Depository.create!(subtype: "checking"),
simplefin_account_id: old_simplefin_account.id
simplefin_account_id: simplefin_account.id
)
old_simplefin_account.update!(account: maybe_account)
simplefin_account.update!(account: maybe_account)
# Mock only the external API calls, let business logic run
# Mock only the external API calls
mock_provider = mock()
mock_provider.expects(:claim_access_url).with(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
# Perform update - job updates access_url and enqueues sync
perform_enqueued_jobs(only: SimplefinConnectionUpdateJob) do
patch simplefin_item_url(@simplefin_item), params: {
simplefin_item: { setup_token: token }
@@ -332,15 +303,19 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest
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.current_account
# Old item still scheduled for deletion
# Verify item was updated in place
@simplefin_item.reload
assert @simplefin_item.scheduled_for_deletion?
assert_equal original_item_id, @simplefin_item.id
assert_equal "https://example.com/new_access", @simplefin_item.access_url
# Verify account linkage remains intact (linkage preserved regardless of sync results)
maybe_account.reload
simplefin_account.reload
assert_equal simplefin_account.id, maybe_account.simplefin_account_id
assert_equal maybe_account, simplefin_account.current_account
# Item is NOT scheduled for deletion (we updated it, not replaced it)
assert_not @simplefin_item.scheduled_for_deletion?
end
test "select_existing_account renders empty-state modal when no simplefin accounts exist" do