QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-04-07 14:31:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix linked account balance currency mismatch (#566)

Browse Source 
* Fix linked account balance currency mismatch

When linking accounts from providers (Lunchflow, SimpleFIN, Enable Banking),
the initial sync was creating balances before the correct currency was known.
This caused:
1. Opening anchor entry created with default currency (USD/EUR)
2. First sync created balances with wrong currency
3. Later syncs created balances with correct currency
4. Both currency balances existed, charts showed wrong (zero) values

Changes:
- Add `skip_initial_sync` parameter to `Account.create_and_sync`
- Skip initial sync for linked accounts (provider sync handles it)
- Add currency filter to ChartSeriesBuilder query to only fetch
  balances matching the account's current currency

* Add migration script and add tests

* Update schema.rb

---------

Signed-off-by: soky srm <sokysrm@gmail.com>
Co-authored-by: sokie <sokysrm@gmail.com>
This commit is contained in:
samuelcseto
2026-01-08 18:23:34 +01:00
committed by GitHub
parent 93a535f0ac
commit cb74856f61
8 changed files with 372 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
app/controllers/enable_banking_items_controller.rb
View File

@@ -269,13 +269,17 @@ class EnableBankingItemsController < ApplicationController
end
# Create the internal Account (uses save! internally, will raise on failure)
# Skip initial sync - provider sync will handle balance creation with correct currency
account = Account.create_and_sync(
family: Current.family,
name: enable_banking_account.name,
balance: enable_banking_account.current_balance || 0,
currency: enable_banking_account.currency || "EUR",
accountable_type: accountable_type,
accountable_attributes: {}
{
family: Current.family,
name: enable_banking_account.name,
balance: enable_banking_account.current_balance || 0,
currency: enable_banking_account.currency || "EUR",
accountable_type: accountable_type,
accountable_attributes: {}
},
skip_initial_sync: true
)
# Link account to enable_banking_account via account_providers

33
app/controllers/lunchflow_items_controller.rb
View File

@@ -182,13 +182,18 @@ class LunchflowItemsController < ApplicationController
end
# Create the internal Account with proper balance initialization
# Use lunchflow_account.currency (already parsed) and skip initial sync
# because the provider sync will set the correct currency from the balance API
account = Account.create_and_sync(
family: Current.family,
name: account_data[:name],
balance: 0, # Initial balance will be set during sync
currency: account_data[:currency] || "USD",
accountable_type: accountable_type,
accountable_attributes: {}
{
family: Current.family,
name: account_data[:name],
balance: 0, # Initial balance will be set during sync
currency: lunchflow_account.currency || "USD",
accountable_type: accountable_type,
accountable_attributes: {}
},
skip_initial_sync: true
)
# Link account to lunchflow_account via account_providers join table
@@ -605,13 +610,17 @@ class LunchflowItemsController < ApplicationController
selected_subtype = "credit_card" if selected_type == "CreditCard" && selected_subtype.blank?
# Create account with user-selected type and subtype (raises on failure)
# Skip initial sync - provider sync will handle balance creation with correct currency
account = Account.create_and_sync(
family: Current.family,
name: lunchflow_account.name,
balance: lunchflow_account.current_balance || 0,
currency: lunchflow_account.currency || "USD",
accountable_type: selected_type,
accountable_attributes: selected_subtype.present? ? { subtype: selected_subtype } : {}
{
family: Current.family,
name: lunchflow_account.name,
balance: lunchflow_account.current_balance || 0,
currency: lunchflow_account.currency || "USD",
accountable_type: selected_type,
accountable_attributes: selected_subtype.present? ? { subtype: selected_subtype } : {}
},
skip_initial_sync: true
)
# Link account to lunchflow_account via account_providers join table (raises on failure)

13
app/models/account.rb
View File

@@ -68,7 +68,7 @@ class Account < ApplicationRecord
end
class << self
def create_and_sync(attributes)
def create_and_sync(attributes, skip_initial_sync: false)
attributes[:accountable_attributes] ||= {} # Ensure accountable is created, even if empty
account = new(attributes.merge(cash_balance: attributes[:balance]))
initial_balance = attributes.dig(:accountable_attributes, :initial_balance)&.to_d
@@ -81,7 +81,9 @@ class Account < ApplicationRecord
raise result.error if result.error
end
account.sync_later
# Skip initial sync for linked accounts - the provider sync will handle balance creation
# after the correct currency is known
account.sync_later unless skip_initial_sync
account
end
@@ -130,7 +132,8 @@ class Account < ApplicationRecord
simplefin_account_id: simplefin_account.id
}
create_and_sync(attributes)
# Skip initial sync - provider sync will handle balance creation with correct currency
create_and_sync(attributes, skip_initial_sync: true)
end
def create_from_enable_banking_account(enable_banking_account, account_type, subtype = nil)
@@ -156,11 +159,13 @@ class Account < ApplicationRecord
accountable_attributes = {}
accountable_attributes[:subtype] = subtype if subtype.present?
# Skip initial sync - provider sync will handle balance creation with correct currency
create_and_sync(
attributes.merge(
accountable_type: account_type,
accountable_attributes: accountable_attributes
)
),
skip_initial_sync: true
)
end

1
app/models/balance/chart_series_builder.rb
View File

@@ -130,6 +130,7 @@ class Balance::ChartSeriesBuilder
b.flows_factor
FROM balances b
WHERE b.account_id = accounts.id
AND b.currency = accounts.currency
AND b.date <= d.date
ORDER BY b.date DESC
LIMIT 1

79
db/migrate/20260108000000_cleanup_orphaned_currency_balances.rb Normal file
View File

@@ -0,0 +1,79 @@
# frozen_string_literal: true
# This migration cleans up orphaned balance records that have a currency different
# from their account's current currency. This can happen when linked accounts
# (SimpleFIN, Lunchflow, Enable Banking, Plaid) were created with an initial sync
# before the correct currency was known from the provider.
#
# The fix in Account.create_and_sync with skip_initial_sync: true prevents this
# going forward, but existing data needs to be cleaned up.
class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2]
def up
# First, identify affected accounts for logging
affected_accounts = execute(<<~SQL).to_a
SELECT DISTINCT
a.id,
a.name,
a.currency as account_currency,
b.currency as orphaned_currency,
COUNT(b.id) as orphaned_balance_count
FROM accounts a
JOIN balances b ON a.id = b.account_id
WHERE b.currency != a.currency
AND (
a.simplefin_account_id IS NOT NULL
OR a.plaid_account_id IS NOT NULL
OR EXISTS (SELECT 1 FROM account_providers WHERE account_id = a.id)
)
GROUP BY a.id, a.name, a.currency, b.currency
ORDER BY a.name
SQL
if affected_accounts.any?
say "Found #{affected_accounts.size} account-currency combinations with orphaned balances:"
affected_accounts.each do |row|
say " - #{row['name']}: #{row['orphaned_balance_count']} balances in #{row['orphaned_currency']} (account is #{row['account_currency']})"
end
# Delete orphaned balances where currency doesn't match account currency
# Only for linked accounts (provider-connected accounts)
deleted_result = execute(<<~SQL)
DELETE FROM balances
WHERE id IN (
SELECT b.id
FROM balances b
JOIN accounts a ON b.account_id = a.id
WHERE b.currency != a.currency
AND (
a.simplefin_account_id IS NOT NULL
OR a.plaid_account_id IS NOT NULL
OR EXISTS (SELECT 1 FROM account_providers WHERE account_id = a.id)
)
)
SQL
say "Deleted orphaned balances from linked accounts"
# Get unique account IDs that need re-sync
account_ids = affected_accounts.map { |row| row["id"] }.uniq
say "Scheduling re-sync for #{account_ids.size} affected accounts..."
# Schedule re-sync for affected accounts to regenerate correct balances
# Use find_each to handle large datasets efficiently
Account.where(id: account_ids).find_each do |account|
account.sync_later
end
say "Scheduled re-sync for #{account_ids.size} affected accounts"
else
say "No orphaned currency balances found - database is clean"
end
end
def down
say "This migration cannot be fully reversed."
say "The deleted balances will be regenerated by the scheduled syncs."
say "If syncs haven't run yet, you may need to manually trigger them."
end
end

2
db/schema.rb generated
View File

@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.2].define(version: 2026_01_06_152346) do
ActiveRecord::Schema[7.2].define(version: 2026_01_08_131034) do
# These are extensions that must be enabled in order to support this database
enable_extension "pgcrypto"
enable_extension "plpgsql"

58
test/models/account_test.rb
View File

@@ -14,6 +14,64 @@ class AccountTest < ActiveSupport::TestCase
end
end
test "create_and_sync calls sync_later by default" do
Account.any_instance.expects(:sync_later).once
account = Account.create_and_sync({
family: @family,
name: "Test Account",
balance: 100,
currency: "USD",
accountable_type: "Depository",
accountable_attributes: {}
})
assert account.persisted?
assert_equal "USD", account.currency
assert_equal 100, account.balance
end
test "create_and_sync skips sync_later when skip_initial_sync is true" do
Account.any_instance.expects(:sync_later).never
account = Account.create_and_sync(
{
family: @family,
name: "Linked Account",
balance: 500,
currency: "EUR",
accountable_type: "Depository",
accountable_attributes: {}
},
skip_initial_sync: true
)
assert account.persisted?
assert_equal "EUR", account.currency
assert_equal 500, account.balance
end
test "create_and_sync creates opening anchor with correct currency" do
Account.any_instance.stubs(:sync_later)
account = Account.create_and_sync(
{
family: @family,
name: "Test Account",
balance: 1000,
currency: "GBP",
accountable_type: "Depository",
accountable_attributes: {}
},
skip_initial_sync: true
)
opening_anchor = account.valuations.opening_anchor.first
assert_not_nil opening_anchor
assert_equal "GBP", opening_anchor.entry.currency
assert_equal 1000, opening_anchor.entry.amount
end
test "gets short/long subtype label" do
investment = Investment.new(subtype: "hsa")
account = @family.accounts.create!(

193
test/models/balance/chart_series_builder_test.rb
View File

@@ -127,4 +127,197 @@ class Balance::ChartSeriesBuilderTest < ActiveSupport::TestCase
assert_equal expected, builder.balance_series.map { |v| v.value.amount }
end
test "uses balances matching account currency for correct chart data" do
# This test verifies that chart data is built from balances with proper currency.
# Data integrity is maintained by:
# 1. Account.create_and_sync with skip_initial_sync: true for linked accounts
# 2. Migration cleanup_orphaned_currency_balances for existing data
account = accounts(:depository)
account.balances.destroy_all
# Account is in USD, create balances in USD
create_balance(account: account, date: 2.days.ago.to_date, balance: 1000)
create_balance(account: account, date: 1.day.ago.to_date, balance: 1500)
create_balance(account: account, date: Date.current, balance: 2000)
builder = Balance::ChartSeriesBuilder.new(
account_ids: [ account.id ],
currency: "USD",
period: Period.custom(start_date: 2.days.ago.to_date, end_date: Date.current),
interval: "1 day"
)
series = builder.balance_series
assert_equal 3, series.size
assert_equal [ 1000, 1500, 2000 ], series.map { |v| v.value.amount }
end
test "balances are converted to target currency using exchange rates" do
# Create account with EUR currency
family = families(:dylan_family)
account = family.accounts.create!(
name: "EUR Account",
balance: 1000,
currency: "EUR",
accountable: Depository.new
)
account.balances.destroy_all
# Create balances in EUR (matching account currency)
create_balance(account: account, date: 1.day.ago.to_date, balance: 1000)
create_balance(account: account, date: Date.current, balance: 1200)
# Add exchange rate EUR -> USD
ExchangeRate.create!(date: 1.day.ago.to_date, from_currency: "EUR", to_currency: "USD", rate: 1.1)
# Request chart in USD (different from account's EUR)
builder = Balance::ChartSeriesBuilder.new(
account_ids: [ account.id ],
currency: "USD",
period: Period.custom(start_date: 1.day.ago.to_date, end_date: Date.current),
interval: "1 day"
)
series = builder.balance_series
# EUR balances converted to USD at 1.1 rate (LOCF for today)
assert_equal [ 1100, 1320 ], series.map { |v| v.value.amount }
end
test "linked account with orphaned currency balances shows correct values after cleanup" do
# This test reproduces the original bug scenario:
# 1. Linked account created with initial sync before correct currency was known
# 2. Opening anchor and first sync created balances with wrong currency (USD)
# 3. Provider sync updated account to correct currency (EUR) and created new balances
# 4. Both USD and EUR balances existed - charts showed wrong values
#
# The fix:
# 1. skip_initial_sync prevents this going forward
# 2. Migration cleans up orphaned balances for existing linked accounts
# Use the connected (linked) account fixture
linked_account = accounts(:connected)
linked_account.balances.destroy_all
# Simulate the bug: account is now EUR but has old USD balances from initial sync
linked_account.update!(currency: "EUR")
# Create orphaned balances in wrong currency (USD) - from initial sync before currency was known
Balance.create!(
account: linked_account,
date: 3.days.ago.to_date,
balance: 1000,
cash_balance: 1000,
currency: "USD", # Wrong currency!
start_cash_balance: 1000,
start_non_cash_balance: 0,
cash_inflows: 0,
cash_outflows: 0,
non_cash_inflows: 0,
non_cash_outflows: 0,
net_market_flows: 0,
cash_adjustments: 0,
non_cash_adjustments: 0,
flows_factor: 1
)
Balance.create!(
account: linked_account,
date: 2.days.ago.to_date,
balance: 1100,
cash_balance: 1100,
currency: "USD", # Wrong currency!
start_cash_balance: 1100,
start_non_cash_balance: 0,
cash_inflows: 0,
cash_outflows: 0,
non_cash_inflows: 0,
non_cash_outflows: 0,
net_market_flows: 0,
cash_adjustments: 0,
non_cash_adjustments: 0,
flows_factor: 1
)
# Create correct balances in EUR - from provider sync after currency was known
create_balance(account: linked_account, date: 1.day.ago.to_date, balance: 5000)
create_balance(account: linked_account, date: Date.current, balance: 5500)
# Verify we have both currency balances (the bug state)
assert_equal 2, linked_account.balances.where(currency: "USD").count
assert_equal 2, linked_account.balances.where(currency: "EUR").count
# Simulate migration cleanup: delete orphaned balances with wrong currency
linked_account.balances.where.not(currency: linked_account.currency).delete_all
# Verify cleanup removed orphaned balances
assert_equal 0, linked_account.balances.where(currency: "USD").count
assert_equal 2, linked_account.balances.where(currency: "EUR").count
# Now chart should show correct EUR values
builder = Balance::ChartSeriesBuilder.new(
account_ids: [ linked_account.id ],
currency: "EUR",
period: Period.custom(start_date: 2.days.ago.to_date, end_date: Date.current),
interval: "1 day"
)
series = builder.balance_series
# After cleanup: only EUR balances exist, chart shows correct values
# Day 2 ago: 0 (no EUR balance), Day 1 ago: 5000, Today: 5500
assert_equal [ 0, 5000, 5500 ], series.map { |v| v.value.amount }
end
test "chart ignores orphaned currency balances via currency filter" do
# This test verifies the currency filter correctly ignores orphaned balances.
# The filter `b.currency = accounts.currency` ensures only valid balances are used.
#
# Bug scenario: Account currency changed from USD to EUR after initial sync,
# leaving orphaned USD balances. Without the filter, charts would show wrong values.
linked_account = accounts(:connected)
linked_account.balances.destroy_all
# Account is EUR but has orphaned USD balances (bug state)
linked_account.update!(currency: "EUR")
# Create orphaned USD balance (wrong currency)
Balance.create!(
account: linked_account,
date: 1.day.ago.to_date,
balance: 9999,
cash_balance: 9999,
currency: "USD", # Wrong currency - doesn't match account.currency (EUR)
start_cash_balance: 9999,
start_non_cash_balance: 0,
cash_inflows: 0,
cash_outflows: 0,
non_cash_inflows: 0,
non_cash_outflows: 0,
net_market_flows: 0,
cash_adjustments: 0,
non_cash_adjustments: 0,
flows_factor: 1
)
# Chart correctly ignores USD balance because account.currency is EUR
builder = Balance::ChartSeriesBuilder.new(
account_ids: [ linked_account.id ],
currency: "EUR",
period: Period.custom(start_date: 1.day.ago.to_date, end_date: Date.current),
interval: "1 day"
)
series = builder.balance_series
# Currency filter ensures orphaned USD balance (9999) is ignored
# Chart shows zeros because no EUR balances exist
assert_equal 2, series.size
assert_equal [ 0, 0 ], series.map { |v| v.value.amount }
# Verify the orphaned balance still exists in DB (migration will clean it up)
assert_equal 1, linked_account.balances.where(currency: "USD").count
assert_equal 0, linked_account.balances.where(currency: "EUR").count
end
end