mirror of
https://github.com/we-promise/sure.git
synced 2026-04-19 20:14:08 +00:00
Exclude tax-advantaged account activity from budget & add provider data quality warnings (#724)
* Add tax-advantaged account exclusions and investment data warnings * Address PR review feedback: translations + cache key stability - Add proper translations for provider warnings in 8 locales (de, es, nb, pt-BR, ro, tr, zh-CN, zh-TW) - Fix cache key stability: use SHA256.hexdigest instead of Array#hash (randomized per process) --------- Co-authored-by: luckyPipewrench <luckypipewrench@proton.me>
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
# Orchestrates the sync process for a CoinStats connection.
|
||||
# Imports data, processes holdings, and schedules account syncs.
|
||||
class CoinstatsItem::Syncer
|
||||
include SyncStats::Collector
|
||||
|
||||
attr_reader :coinstats_item
|
||||
|
||||
# @param coinstats_item [CoinstatsItem] Item to sync
|
||||
@@ -40,6 +42,10 @@ class CoinstatsItem::Syncer
|
||||
sync.update!(status_text: I18n.t("models.coinstats_item.syncer.processing_holdings")) if sync.respond_to?(:status_text)
|
||||
coinstats_item.process_accounts
|
||||
|
||||
# CoinStats provides transactions but not activity labels (Buy, Sell, Dividend, etc.)
|
||||
# Warn users that this may affect budget accuracy
|
||||
collect_investment_data_quality_warning(sync, linked_accounts)
|
||||
|
||||
# Phase 4: Schedule balance calculations for linked accounts
|
||||
sync.update!(status_text: I18n.t("models.coinstats_item.syncer.calculating_balances")) if sync.respond_to?(:status_text)
|
||||
coinstats_item.schedule_account_syncs(
|
||||
@@ -58,4 +64,22 @@ class CoinstatsItem::Syncer
|
||||
def perform_post_sync
|
||||
# no-op
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# Collects a data quality warning for all CoinStats accounts.
|
||||
# CoinStats cannot provide activity labels (Buy, Sell, Dividend, etc.) for transactions,
|
||||
# which may affect budget accuracy.
|
||||
def collect_investment_data_quality_warning(sync, linked_coinstats_accounts)
|
||||
# All CoinStats accounts are crypto/investment accounts
|
||||
return if linked_coinstats_accounts.empty?
|
||||
|
||||
collect_data_quality_stats(sync,
|
||||
warnings: linked_coinstats_accounts.size,
|
||||
details: [ {
|
||||
message: I18n.t("provider_warnings.limited_investment_data"),
|
||||
severity: "warning"
|
||||
} ]
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -86,6 +86,31 @@ class Family < ApplicationRecord
|
||||
categories.find_by(name: Category.investment_contributions_name)
|
||||
end
|
||||
|
||||
# Returns account IDs for tax-advantaged accounts (401k, IRA, HSA, etc.)
|
||||
# Used to exclude these accounts from budget/cashflow calculations.
|
||||
# Tax-advantaged accounts are retirement savings, not daily expenses.
|
||||
def tax_advantaged_account_ids
|
||||
@tax_advantaged_account_ids ||= begin
|
||||
# Investment accounts derive tax_treatment from subtype
|
||||
tax_advantaged_subtypes = Investment::SUBTYPES.select do |_, meta|
|
||||
meta[:tax_treatment].in?(%i[tax_deferred tax_exempt tax_advantaged])
|
||||
end.keys
|
||||
|
||||
investment_ids = accounts
|
||||
.joins("INNER JOIN investments ON investments.id = accounts.accountable_id AND accounts.accountable_type = 'Investment'")
|
||||
.where(investments: { subtype: tax_advantaged_subtypes })
|
||||
.pluck(:id)
|
||||
|
||||
# Crypto accounts have an explicit tax_treatment column
|
||||
crypto_ids = accounts
|
||||
.joins("INNER JOIN cryptos ON cryptos.id = accounts.accountable_id AND accounts.accountable_type = 'Crypto'")
|
||||
.where(cryptos: { tax_treatment: %w[tax_deferred tax_exempt] })
|
||||
.pluck(:id)
|
||||
|
||||
investment_ids + crypto_ids
|
||||
end
|
||||
end
|
||||
|
||||
def investment_statement
|
||||
@investment_statement ||= InvestmentStatement.new(self)
|
||||
end
|
||||
|
||||
@@ -21,14 +21,29 @@ class IncomeStatement::CategoryStats
|
||||
def sanitized_query_sql
|
||||
ActiveRecord::Base.sanitize_sql_array([
|
||||
query_sql,
|
||||
{
|
||||
target_currency: @family.currency,
|
||||
interval: @interval,
|
||||
family_id: @family.id
|
||||
}
|
||||
sql_params
|
||||
])
|
||||
end
|
||||
|
||||
def sql_params
|
||||
params = {
|
||||
target_currency: @family.currency,
|
||||
interval: @interval,
|
||||
family_id: @family.id
|
||||
}
|
||||
|
||||
ids = @family.tax_advantaged_account_ids
|
||||
params[:tax_advantaged_account_ids] = ids if ids.present?
|
||||
|
||||
params
|
||||
end
|
||||
|
||||
def exclude_tax_advantaged_sql
|
||||
ids = @family.tax_advantaged_account_ids
|
||||
return "" if ids.empty?
|
||||
"AND a.id NOT IN (:tax_advantaged_account_ids)"
|
||||
end
|
||||
|
||||
def query_sql
|
||||
<<~SQL
|
||||
WITH period_totals AS (
|
||||
@@ -51,6 +66,7 @@ class IncomeStatement::CategoryStats
|
||||
AND ae.excluded = false
|
||||
AND (t.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true
|
||||
AND (t.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true
|
||||
#{exclude_tax_advantaged_sql}
|
||||
GROUP BY c.id, period, CASE WHEN t.kind = 'investment_contribution' THEN 'expense' WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END
|
||||
)
|
||||
SELECT
|
||||
|
||||
@@ -20,14 +20,29 @@ class IncomeStatement::FamilyStats
|
||||
def sanitized_query_sql
|
||||
ActiveRecord::Base.sanitize_sql_array([
|
||||
query_sql,
|
||||
{
|
||||
target_currency: @family.currency,
|
||||
interval: @interval,
|
||||
family_id: @family.id
|
||||
}
|
||||
sql_params
|
||||
])
|
||||
end
|
||||
|
||||
def sql_params
|
||||
params = {
|
||||
target_currency: @family.currency,
|
||||
interval: @interval,
|
||||
family_id: @family.id
|
||||
}
|
||||
|
||||
ids = @family.tax_advantaged_account_ids
|
||||
params[:tax_advantaged_account_ids] = ids if ids.present?
|
||||
|
||||
params
|
||||
end
|
||||
|
||||
def exclude_tax_advantaged_sql
|
||||
ids = @family.tax_advantaged_account_ids
|
||||
return "" if ids.empty?
|
||||
"AND a.id NOT IN (:tax_advantaged_account_ids)"
|
||||
end
|
||||
|
||||
def query_sql
|
||||
<<~SQL
|
||||
WITH period_totals AS (
|
||||
@@ -48,6 +63,7 @@ class IncomeStatement::FamilyStats
|
||||
AND ae.excluded = false
|
||||
AND (t.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true
|
||||
AND (t.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true
|
||||
#{exclude_tax_advantaged_sql}
|
||||
GROUP BY period, CASE WHEN t.kind = 'investment_contribution' THEN 'expense' WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END
|
||||
)
|
||||
SELECT
|
||||
|
||||
@@ -71,8 +71,9 @@ class IncomeStatement::Totals
|
||||
)
|
||||
WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
|
||||
AND ae.excluded = false
|
||||
AND a.family_id = :family_id
|
||||
AND a.family_id = :family_id
|
||||
AND a.status IN ('draft', 'active')
|
||||
#{exclude_tax_advantaged_sql}
|
||||
GROUP BY c.id, c.parent_id, CASE WHEN at.kind = 'investment_contribution' THEN 'expense' WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END;
|
||||
SQL
|
||||
end
|
||||
@@ -101,8 +102,9 @@ class IncomeStatement::Totals
|
||||
OR at.investment_activity_label NOT IN ('Transfer', 'Sweep In', 'Sweep Out', 'Exchange')
|
||||
)
|
||||
AND ae.excluded = false
|
||||
AND a.family_id = :family_id
|
||||
AND a.family_id = :family_id
|
||||
AND a.status IN ('draft', 'active')
|
||||
#{exclude_tax_advantaged_sql}
|
||||
GROUP BY c.id, c.parent_id, CASE WHEN at.kind = 'investment_contribution' THEN 'expense' WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END
|
||||
SQL
|
||||
end
|
||||
@@ -120,12 +122,26 @@ class IncomeStatement::Totals
|
||||
end
|
||||
|
||||
def sql_params
|
||||
{
|
||||
params = {
|
||||
target_currency: @family.currency,
|
||||
family_id: @family.id,
|
||||
start_date: @date_range.begin,
|
||||
end_date: @date_range.end
|
||||
}
|
||||
|
||||
# Add tax-advantaged account IDs if any exist
|
||||
ids = @family.tax_advantaged_account_ids
|
||||
params[:tax_advantaged_account_ids] = ids if ids.present?
|
||||
|
||||
params
|
||||
end
|
||||
|
||||
# Returns SQL clause to exclude tax-advantaged accounts from budget calculations.
|
||||
# Tax-advantaged accounts (401k, IRA, HSA, etc.) are retirement savings, not daily expenses.
|
||||
def exclude_tax_advantaged_sql
|
||||
ids = @family.tax_advantaged_account_ids
|
||||
return "" if ids.empty?
|
||||
"AND a.id NOT IN (:tax_advantaged_account_ids)"
|
||||
end
|
||||
|
||||
def validate_date_range!
|
||||
|
||||
@@ -36,6 +36,9 @@ class LunchflowItem::Syncer
|
||||
lunchflow_item.process_accounts
|
||||
Rails.logger.info "LunchflowItem::Syncer - Finished processing accounts"
|
||||
|
||||
# Warn about limited investment data for investment/crypto accounts
|
||||
collect_investment_data_quality_warning(sync, linked_accounts)
|
||||
|
||||
# Phase 4: Schedule balance calculations for linked accounts
|
||||
sync.update!(status_text: "Calculating balances...") if sync.respond_to?(:status_text)
|
||||
lunchflow_item.schedule_account_syncs(
|
||||
@@ -61,4 +64,26 @@ class LunchflowItem::Syncer
|
||||
def perform_post_sync
|
||||
# no-op
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# Collects a data quality warning if any linked accounts are investment or crypto accounts.
|
||||
# Lunchflow cannot provide activity labels (Buy, Sell, Dividend, etc.) for investment transactions,
|
||||
# which may affect budget accuracy.
|
||||
def collect_investment_data_quality_warning(sync, linked_lunchflow_accounts)
|
||||
investment_accounts = linked_lunchflow_accounts.select do |la|
|
||||
account = la.current_account
|
||||
account&.accountable_type.in?(%w[Investment Crypto])
|
||||
end
|
||||
|
||||
return if investment_accounts.empty?
|
||||
|
||||
collect_data_quality_stats(sync,
|
||||
warnings: investment_accounts.size,
|
||||
details: [ {
|
||||
message: I18n.t("provider_warnings.limited_investment_data"),
|
||||
severity: "warning"
|
||||
} ]
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -69,6 +69,9 @@ class SimplefinItem::Syncer
|
||||
collect_skip_stats(sync, skipped_entries: skipped_entries)
|
||||
end
|
||||
|
||||
# Warn about limited investment data for investment/crypto accounts
|
||||
collect_investment_data_quality_warning(sync, linked_simplefin_accounts)
|
||||
|
||||
sync.update!(status_text: "Calculating balances...") if sync.respond_to?(:status_text)
|
||||
simplefin_item.schedule_account_syncs(
|
||||
parent_sync: sync,
|
||||
@@ -225,6 +228,26 @@ class SimplefinItem::Syncer
|
||||
}
|
||||
end
|
||||
|
||||
# Collects a data quality warning if any linked accounts are investment or crypto accounts.
|
||||
# SimpleFIN cannot provide activity labels (Buy, Sell, Dividend, etc.) for investment transactions,
|
||||
# which may affect budget accuracy.
|
||||
def collect_investment_data_quality_warning(sync, linked_simplefin_accounts)
|
||||
investment_accounts = linked_simplefin_accounts.select do |sfa|
|
||||
account = sfa.current_account
|
||||
account&.accountable_type.in?(%w[Investment Crypto])
|
||||
end
|
||||
|
||||
return if investment_accounts.empty?
|
||||
|
||||
collect_data_quality_stats(sync,
|
||||
warnings: investment_accounts.size,
|
||||
details: [ {
|
||||
message: I18n.t("provider_warnings.limited_investment_data"),
|
||||
severity: "warning"
|
||||
} ]
|
||||
)
|
||||
end
|
||||
|
||||
def mark_failed(sync, error)
|
||||
# If already completed, do not attempt to fail to avoid AASM InvalidTransition
|
||||
if sync.respond_to?(:status) && sync.status.to_s == "completed"
|
||||
|
||||
@@ -44,10 +44,18 @@ class Transaction::Search
|
||||
end
|
||||
|
||||
# Computes totals for the specific search
|
||||
# Note: Excludes tax-advantaged accounts (401k, IRA, etc.) from totals calculation
|
||||
# because those transactions are retirement savings, not daily income/expenses.
|
||||
def totals
|
||||
@totals ||= begin
|
||||
Rails.cache.fetch("transaction_search_totals/#{cache_key_base}") do
|
||||
result = transactions_scope
|
||||
scope = transactions_scope
|
||||
|
||||
# Exclude tax-advantaged accounts from totals calculation
|
||||
tax_advantaged_ids = family.tax_advantaged_account_ids
|
||||
scope = scope.where.not(accounts: { id: tax_advantaged_ids }) if tax_advantaged_ids.present?
|
||||
|
||||
result = scope
|
||||
.select(
|
||||
"COALESCE(SUM(CASE WHEN transactions.kind = 'investment_contribution' THEN ABS(entries.amount * COALESCE(er.rate, 1)) WHEN entries.amount >= 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment') THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as expense_total",
|
||||
"COALESCE(SUM(CASE WHEN entries.amount < 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution') THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as income_total",
|
||||
@@ -74,7 +82,8 @@ class Transaction::Search
|
||||
[
|
||||
family.id,
|
||||
Digest::SHA256.hexdigest(attributes.sort.to_h.to_json), # cached by filters
|
||||
family.entries_cache_version
|
||||
family.entries_cache_version,
|
||||
Digest::SHA256.hexdigest(family.tax_advantaged_account_ids.sort.to_json) # stable across processes
|
||||
].join("/")
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user