mirror of
https://github.com/we-promise/sure.git
synced 2026-04-19 20:14:08 +00:00
Providers factory (#250)
* Implement providers factory * Multiple providers sync support - Proper Multi-Provider Syncing: When you click sync on an account with multiple providers (e.g., both Plaid and SimpleFin), all provider items are synced - Better API: The existing account.providers method already returns all providers, and account.provider returns the first one for backward compatibility - Correct Holdings Deletion Logic: Holdings can only be deleted if ALL providers allow it, preventing accidental deletions that would be recreated on next sync TODO: validate this is the way we want to go? We would need to check holdings belong to which account, and then check provider allows deletion. More complex - Database Constraints: The existing validations ensure an account can have at most one provider of each type (one PlaidAccount, one SimplefinAccount, etc.) * Add generic provider_import_adapter * Finish unified import strategy * Update app/models/plaid_account.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <sokysrm@gmail.com> * Update app/models/provider/factory.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <sokysrm@gmail.com> * Fix account linked by plaid_id instead of external_id * Parse numerics to BigDecimal Parse numerics to BigDecimal before computing amount; guard nils. Avoid String * String and float drift; also normalize date. * Fix incorrect usage of assert_raises. * Fix linter * Fix processor test. * Update current_balance_manager.rb * Test fixes * Fix plaid linked account test * Add support for holding per account_provider * Fix proper account access Also fix account deletion for simpefin too * FIX match tests for consistency * Some more factory updates * Fix account schema for multipe providers Can do: - Account #1 → PlaidAccount + SimplefinAccount (multiple different providers) - Account #2 → PlaidAccount only - Account #3 → SimplefinAccount only Cannot do: - Account #1 → PlaidAccount + PlaidAccount (duplicate provider type) - PlaidAccount #123 → Account #1 + Account #2 (provider linked to multiple accounts) * Fix account setup - An account CAN have multiple providers (the schema shows account_providers with unique index on [account_id, provider_type]) - Each provider should maintain its own separate entries - We should NOT update one provider's entry when another provider syncs * Fix linter and guard migration * FIX linter issues. * Fixes - Remove duplicated index - Pass account_provider_id - Guard holdings call to avoid NoMethodError * Update schema and provider import fix * Plaid doesn't allow holdings deletion * Use ClimateControl for proper env setup * No need for this in .git --------- Signed-off-by: soky srm <sokysrm@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
This commit is contained in:
@@ -17,21 +17,6 @@ class SimplefinAccount::Investments::HoldingsProcessor
|
||||
security = resolve_security(symbol, simplefin_holding["description"])
|
||||
next unless security.present?
|
||||
|
||||
# Use external_id for precise matching
|
||||
external_id = "simplefin_#{holding_id}"
|
||||
|
||||
# Use the created timestamp as the holding date, fallback to current date
|
||||
holding_date = parse_holding_date(simplefin_holding["created"]) || Date.current
|
||||
|
||||
holding = account.holdings.find_or_initialize_by(
|
||||
external_id: external_id
|
||||
) do |h|
|
||||
# Set required fields on initialization
|
||||
h.security = security
|
||||
h.date = holding_date
|
||||
h.currency = simplefin_holding["currency"] || "USD"
|
||||
end
|
||||
|
||||
# Parse all the data SimpleFin provides
|
||||
qty = parse_decimal(simplefin_holding["shares"])
|
||||
market_value = parse_decimal(simplefin_holding["market_value"])
|
||||
@@ -44,22 +29,22 @@ class SimplefinAccount::Investments::HoldingsProcessor
|
||||
parse_decimal(simplefin_holding["purchase_price"]) || 0
|
||||
end
|
||||
|
||||
holding.assign_attributes(
|
||||
# Use the created timestamp as the holding date, fallback to current date
|
||||
holding_date = parse_holding_date(simplefin_holding["created"]) || Date.current
|
||||
|
||||
import_adapter.import_holding(
|
||||
security: security,
|
||||
date: holding_date,
|
||||
currency: simplefin_holding["currency"] || "USD",
|
||||
qty: qty,
|
||||
price: price,
|
||||
quantity: qty,
|
||||
amount: market_value,
|
||||
cost_basis: cost_basis
|
||||
currency: simplefin_holding["currency"] || "USD",
|
||||
date: holding_date,
|
||||
price: price,
|
||||
cost_basis: cost_basis,
|
||||
external_id: "simplefin_#{holding_id}",
|
||||
account_provider_id: simplefin_account.account_provider&.id,
|
||||
source: "simplefin",
|
||||
delete_future_holdings: false # SimpleFin tracks each holding uniquely
|
||||
)
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
holding.save!
|
||||
|
||||
# With external_id matching, each holding is uniquely tracked
|
||||
# No need to delete other holdings since each has its own lifecycle
|
||||
end
|
||||
rescue => e
|
||||
ctx = (defined?(symbol) && symbol.present?) ? " #{symbol}" : ""
|
||||
Rails.logger.error "Error processing SimpleFin holding#{ctx}: #{e.message}"
|
||||
@@ -70,8 +55,12 @@ class SimplefinAccount::Investments::HoldingsProcessor
|
||||
private
|
||||
attr_reader :simplefin_account
|
||||
|
||||
def import_adapter
|
||||
@import_adapter ||= Account::ProviderImportAdapter.new(account)
|
||||
end
|
||||
|
||||
def account
|
||||
simplefin_account.account
|
||||
simplefin_account.current_account
|
||||
end
|
||||
|
||||
def holdings_data
|
||||
|
||||
@@ -6,7 +6,7 @@ class SimplefinAccount::Investments::TransactionsProcessor
|
||||
end
|
||||
|
||||
def process
|
||||
return unless simplefin_account.account&.accountable_type == "Investment"
|
||||
return unless simplefin_account.current_account&.accountable_type == "Investment"
|
||||
return unless simplefin_account.raw_transactions_payload.present?
|
||||
|
||||
transactions_data = simplefin_account.raw_transactions_payload
|
||||
@@ -20,7 +20,7 @@ class SimplefinAccount::Investments::TransactionsProcessor
|
||||
attr_reader :simplefin_account
|
||||
|
||||
def account
|
||||
simplefin_account.account
|
||||
simplefin_account.current_account
|
||||
end
|
||||
|
||||
def process_investment_transaction(transaction_data)
|
||||
@@ -30,28 +30,23 @@ class SimplefinAccount::Investments::TransactionsProcessor
|
||||
posted_date = parse_date(data[:posted])
|
||||
external_id = "simplefin_#{data[:id]}"
|
||||
|
||||
# Check if entry already exists
|
||||
existing_entry = Entry.find_by(plaid_id: external_id)
|
||||
|
||||
unless existing_entry
|
||||
# For investment accounts, create as regular transaction
|
||||
# In the future, we could detect trade patterns and create Trade entries
|
||||
transaction = Transaction.new(external_id: external_id)
|
||||
|
||||
Entry.create!(
|
||||
account: account,
|
||||
name: data[:description] || "Investment transaction",
|
||||
amount: amount,
|
||||
date: posted_date,
|
||||
currency: account.currency,
|
||||
entryable: transaction,
|
||||
plaid_id: external_id
|
||||
)
|
||||
end
|
||||
# Use the unified import adapter for consistent handling
|
||||
import_adapter.import_transaction(
|
||||
external_id: external_id,
|
||||
amount: amount,
|
||||
currency: account.currency,
|
||||
date: posted_date,
|
||||
name: data[:description] || "Investment transaction",
|
||||
source: "simplefin"
|
||||
)
|
||||
rescue => e
|
||||
Rails.logger.error("Failed to process SimpleFin investment transaction #{data[:id]}: #{e.message}")
|
||||
end
|
||||
|
||||
def import_adapter
|
||||
@import_adapter ||= Account::ProviderImportAdapter.new(account)
|
||||
end
|
||||
|
||||
def parse_amount(amount_value)
|
||||
parsed_amount = case amount_value
|
||||
when String
|
||||
|
||||
@@ -5,7 +5,7 @@ class SimplefinAccount::Liabilities::CreditProcessor
|
||||
end
|
||||
|
||||
def process
|
||||
return unless simplefin_account.account&.accountable_type == "CreditCard"
|
||||
return unless simplefin_account.current_account&.accountable_type == "CreditCard"
|
||||
|
||||
# Update credit card specific attributes if available
|
||||
update_credit_attributes
|
||||
@@ -14,18 +14,26 @@ class SimplefinAccount::Liabilities::CreditProcessor
|
||||
private
|
||||
attr_reader :simplefin_account
|
||||
|
||||
def import_adapter
|
||||
@import_adapter ||= Account::ProviderImportAdapter.new(account)
|
||||
end
|
||||
|
||||
def account
|
||||
simplefin_account.account
|
||||
simplefin_account.current_account
|
||||
end
|
||||
|
||||
def update_credit_attributes
|
||||
# SimpleFin provides available_balance which could be credit limit for cards
|
||||
available_balance = simplefin_account.raw_payload&.dig("available-balance")
|
||||
|
||||
if available_balance.present? && account.accountable.respond_to?(:available_credit=)
|
||||
if available_balance.present?
|
||||
credit_limit = parse_decimal(available_balance)
|
||||
account.accountable.available_credit = credit_limit if credit_limit > 0
|
||||
account.accountable.save!
|
||||
if credit_limit > 0
|
||||
import_adapter.update_accountable_attributes(
|
||||
attributes: { available_credit: credit_limit },
|
||||
source: "simplefin"
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ class SimplefinAccount::Liabilities::LoanProcessor
|
||||
end
|
||||
|
||||
def process
|
||||
return unless simplefin_account.account&.accountable_type == "Loan"
|
||||
return unless simplefin_account.current_account&.accountable_type == "Loan"
|
||||
|
||||
# Update loan specific attributes if available
|
||||
update_loan_attributes
|
||||
@@ -15,7 +15,7 @@ class SimplefinAccount::Liabilities::LoanProcessor
|
||||
attr_reader :simplefin_account
|
||||
|
||||
def account
|
||||
simplefin_account.account
|
||||
simplefin_account.current_account
|
||||
end
|
||||
|
||||
def update_loan_attributes
|
||||
|
||||
@@ -9,7 +9,7 @@ class SimplefinAccount::Processor
|
||||
# Processing the account is the first step and if it fails, we halt
|
||||
# Each subsequent step can fail independently, but we continue processing
|
||||
def process
|
||||
unless simplefin_account.account.present?
|
||||
unless simplefin_account.current_account.present?
|
||||
return
|
||||
end
|
||||
|
||||
@@ -24,13 +24,13 @@ class SimplefinAccount::Processor
|
||||
def process_account!
|
||||
# This should not happen in normal flow since accounts are created manually
|
||||
# during setup, but keeping as safety check
|
||||
if simplefin_account.account.blank?
|
||||
if simplefin_account.current_account.blank?
|
||||
Rails.logger.error("SimpleFin account #{simplefin_account.id} has no associated Account - this should not happen after manual setup")
|
||||
return
|
||||
end
|
||||
|
||||
# Update account balance and cash balance from latest SimpleFin data
|
||||
account = simplefin_account.account
|
||||
account = simplefin_account.current_account
|
||||
balance = simplefin_account.current_balance || simplefin_account.available_balance || 0
|
||||
|
||||
# SimpleFin returns negative balances for credit cards (liabilities)
|
||||
@@ -60,7 +60,7 @@ class SimplefinAccount::Processor
|
||||
end
|
||||
|
||||
def process_investments
|
||||
return unless simplefin_account.account&.accountable_type == "Investment"
|
||||
return unless simplefin_account.current_account&.accountable_type == "Investment"
|
||||
SimplefinAccount::Investments::TransactionsProcessor.new(simplefin_account).process
|
||||
SimplefinAccount::Investments::HoldingsProcessor.new(simplefin_account).process
|
||||
rescue => e
|
||||
@@ -68,7 +68,7 @@ class SimplefinAccount::Processor
|
||||
end
|
||||
|
||||
def process_liabilities
|
||||
case simplefin_account.account&.accountable_type
|
||||
case simplefin_account.current_account&.accountable_type
|
||||
when "CreditCard"
|
||||
SimplefinAccount::Liabilities::CreditProcessor.new(simplefin_account).process
|
||||
when "Loan"
|
||||
|
||||
@@ -37,6 +37,6 @@ class SimplefinAccount::Transactions::Processor
|
||||
end
|
||||
|
||||
def account
|
||||
simplefin_account.account
|
||||
simplefin_account.current_account
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user