From be5de57d5af1e98c4a8a32ae5d79498f26ba6b78 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 23 May 2025 10:25:02 -0400 Subject: [PATCH] Account processor test cases --- app/models/plaid_account.rb | 10 + .../investments/holdings_processor.rb | 2 - .../investments/transactions_processor.rb | 2 - app/models/plaid_account/processor.rb | 18 +- ...455_add_raw_payloads_to_plaid_accounts.rb} | 11 +- db/schema.rb | 14 +- test/fixtures/accounts.yml | 3 +- test/fixtures/plaid_accounts.yml | 6 + test/models/plaid_account/processor_test.rb | 172 ++++++++++++++++++ 9 files changed, 218 insertions(+), 20 deletions(-) rename db/migrate/{20250518133020_add_raw_payload_to_plaid_entities.rb => 20250523131455_add_raw_payloads_to_plaid_accounts.rb} (50%) create mode 100644 test/models/plaid_account/processor_test.rb diff --git a/app/models/plaid_account.rb b/app/models/plaid_account.rb index 5a1120c2..949167ce 100644 --- a/app/models/plaid_account.rb +++ b/app/models/plaid_account.rb @@ -3,6 +3,9 @@ class PlaidAccount < ApplicationRecord has_one :account, dependent: :destroy + validates :name, :plaid_type, :currency, presence: true + validate :has_balance + def upsert_plaid_snapshot!(account_snapshot) assign_attributes( current_balance: account_snapshot.balances.current, @@ -41,4 +44,11 @@ class PlaidAccount < ApplicationRecord save! end + + private + # Plaid guarantees at least one of these. This validation is a sanity check for that guarantee. + def has_balance + return if current_balance.present? || available_balance.present? + errors.add(:base, "Plaid account must have either current or available balance") + end end diff --git a/app/models/plaid_account/investments/holdings_processor.rb b/app/models/plaid_account/investments/holdings_processor.rb index 8becfa90..b5671151 100644 --- a/app/models/plaid_account/investments/holdings_processor.rb +++ b/app/models/plaid_account/investments/holdings_processor.rb @@ -1,6 +1,4 @@ class PlaidAccount::Investments::HoldingsProcessor - include PlaidAccount::Securitizable - def initialize(plaid_account, security_resolver:) @plaid_account = plaid_account @security_resolver = security_resolver diff --git a/app/models/plaid_account/investments/transactions_processor.rb b/app/models/plaid_account/investments/transactions_processor.rb index b15bc9f0..faa285fc 100644 --- a/app/models/plaid_account/investments/transactions_processor.rb +++ b/app/models/plaid_account/investments/transactions_processor.rb @@ -1,6 +1,4 @@ class PlaidAccount::Investments::TransactionsProcessor - include PlaidAccount::Securitizable - def initialize(plaid_account, security_resolver:) @plaid_account = plaid_account @security_resolver = security_resolver diff --git a/app/models/plaid_account/processor.rb b/app/models/plaid_account/processor.rb index 5db0e470..1a8205a1 100644 --- a/app/models/plaid_account/processor.rb +++ b/app/models/plaid_account/processor.rb @@ -34,16 +34,17 @@ class PlaidAccount::Processor plaid_account_id: plaid_account.id ) - # Name is the only attribute a user can override for Plaid accounts - account.enrich_attribute( - :name, - plaid_account.name, + # Name and subtype are the only attributes a user can override for Plaid accounts + account.enrich_attributes( + { + name: plaid_account.name, + subtype: map_subtype(plaid_account.plaid_type, plaid_account.plaid_subtype) + }, source: "plaid" ) account.assign_attributes( accountable: map_accountable(plaid_account.plaid_type), - subtype: map_subtype(plaid_account.plaid_type, plaid_account.plaid_subtype), balance: balance_calculator.balance, currency: plaid_account.currency, cash_balance: balance_calculator.cash_balance @@ -62,17 +63,18 @@ class PlaidAccount::Processor def process_investments PlaidAccount::Investments::TransactionsProcessor.new(plaid_account, security_resolver: security_resolver).process PlaidAccount::Investments::HoldingsProcessor.new(plaid_account, security_resolver: security_resolver).process + rescue => e report_exception(e) end def process_liabilities case [ plaid_account.plaid_type, plaid_account.plaid_subtype ] when [ "credit", "credit card" ] - PlaidAccount::CreditLiabilityProcessor.new(plaid_account).process + PlaidAccount::Liabilities::CreditProcessor.new(plaid_account).process when [ "loan", "mortgage" ] - PlaidAccount::MortgageLiabilityProcessor.new(plaid_account).process + PlaidAccount::Liabilities::MortgageProcessor.new(plaid_account).process when [ "loan", "student" ] - PlaidAccount::StudentLoanLiabilityProcessor.new(plaid_account).process + PlaidAccount::Liabilities::StudentLoanProcessor.new(plaid_account).process end rescue => e report_exception(e) diff --git a/db/migrate/20250518133020_add_raw_payload_to_plaid_entities.rb b/db/migrate/20250523131455_add_raw_payloads_to_plaid_accounts.rb similarity index 50% rename from db/migrate/20250518133020_add_raw_payload_to_plaid_entities.rb rename to db/migrate/20250523131455_add_raw_payloads_to_plaid_accounts.rb index 6c2be250..b9bcf884 100644 --- a/db/migrate/20250518133020_add_raw_payload_to_plaid_entities.rb +++ b/db/migrate/20250523131455_add_raw_payloads_to_plaid_accounts.rb @@ -1,11 +1,20 @@ -class AddRawPayloadToPlaidEntities < ActiveRecord::Migration[7.2] +class AddRawPayloadsToPlaidAccounts < ActiveRecord::Migration[7.2] def change add_column :plaid_items, :raw_payload, :jsonb, default: {} add_column :plaid_items, :raw_institution_payload, :jsonb, default: {} + change_column_null :plaid_items, :plaid_id, false + add_index :plaid_items, :plaid_id, unique: true + add_column :plaid_accounts, :raw_payload, :jsonb, default: {} add_column :plaid_accounts, :raw_transactions_payload, :jsonb, default: {} add_column :plaid_accounts, :raw_investments_payload, :jsonb, default: {} add_column :plaid_accounts, :raw_liabilities_payload, :jsonb, default: {} + + change_column_null :plaid_accounts, :plaid_id, false + change_column_null :plaid_accounts, :plaid_type, false + change_column_null :plaid_accounts, :currency, false + change_column_null :plaid_accounts, :name, false + add_index :plaid_accounts, :plaid_id, unique: true end end diff --git a/db/schema.rb b/db/schema.rb index c4d0be5f..9cb2c7f4 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_05_22_201031) do +ActiveRecord::Schema[7.2].define(version: 2025_05_23_131455) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -420,13 +420,13 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_22_201031) do create_table "plaid_accounts", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "plaid_item_id", null: false - t.string "plaid_id" - t.string "plaid_type" + t.string "plaid_id", null: false + t.string "plaid_type", null: false t.string "plaid_subtype" t.decimal "current_balance", precision: 19, scale: 4 t.decimal "available_balance", precision: 19, scale: 4 - t.string "currency" - t.string "name" + t.string "currency", null: false + t.string "name", null: false t.string "mask" t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -434,13 +434,14 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_22_201031) do t.jsonb "raw_transactions_payload", default: {} t.jsonb "raw_investments_payload", default: {} t.jsonb "raw_liabilities_payload", default: {} + t.index ["plaid_id"], name: "index_plaid_accounts_on_plaid_id", unique: true t.index ["plaid_item_id"], name: "index_plaid_accounts_on_plaid_item_id" end create_table "plaid_items", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "family_id", null: false t.string "access_token" - t.string "plaid_id" + t.string "plaid_id", null: false t.string "name" t.string "next_cursor" t.boolean "scheduled_for_deletion", default: false @@ -456,6 +457,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_22_201031) do t.jsonb "raw_payload", default: {} t.jsonb "raw_institution_payload", default: {} t.index ["family_id"], name: "index_plaid_items_on_family_id" + t.index ["plaid_id"], name: "index_plaid_items_on_plaid_id", unique: true end create_table "properties", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| diff --git a/test/fixtures/accounts.yml b/test/fixtures/accounts.yml index 810dbf41..ec8668e5 100644 --- a/test/fixtures/accounts.yml +++ b/test/fixtures/accounts.yml @@ -24,9 +24,10 @@ depository: connected: family: dylan_family - name: Connected Account + name: Plaid Depository Account balance: 5000 currency: USD + subtype: checking accountable_type: Depository accountable: two plaid_account: one diff --git a/test/fixtures/plaid_accounts.yml b/test/fixtures/plaid_accounts.yml index 63d25646..3679e767 100644 --- a/test/fixtures/plaid_accounts.yml +++ b/test/fixtures/plaid_accounts.yml @@ -1,3 +1,9 @@ one: + current_balance: 1000 + available_balance: 1000 + currency: USD + name: Plaid Depository Account plaid_item: one plaid_id: "acc_mock_1" + plaid_type: depository + plaid_subtype: checking \ No newline at end of file diff --git a/test/models/plaid_account/processor_test.rb b/test/models/plaid_account/processor_test.rb new file mode 100644 index 00000000..ec75296d --- /dev/null +++ b/test/models/plaid_account/processor_test.rb @@ -0,0 +1,172 @@ +require "test_helper" + +class PlaidAccount::ProcessorTest < ActiveSupport::TestCase + setup do + @plaid_account = plaid_accounts(:one) + end + + test "processes new account and assigns attributes" do + Account.destroy_all # Clear out internal accounts so we start fresh + + expect_default_subprocessor_calls + + @plaid_account.update!( + plaid_id: "test_plaid_id", + plaid_type: "depository", + plaid_subtype: "checking", + current_balance: 1000, + available_balance: 1000, + currency: "USD", + name: "Test Plaid Account", + mask: "1234" + ) + + assert_difference "Account.count" do + PlaidAccount::Processor.new(@plaid_account).process + end + + @plaid_account.reload + + account = Account.order(created_at: :desc).first + assert_equal "Test Plaid Account", account.name + assert_equal @plaid_account.id, account.plaid_account_id + assert_equal "checking", account.subtype + assert_equal 1000, account.balance + assert_equal 1000, account.cash_balance + assert_equal "USD", account.currency + assert_equal "Depository", account.accountable_type + assert_equal "checking", account.subtype + end + + test "processing is idempotent with updates and enrichments" do + expect_default_subprocessor_calls + + assert_equal "Plaid Depository Account", @plaid_account.account.name + assert_equal "checking", @plaid_account.account.subtype + + @plaid_account.account.update!( + name: "User updated name", + subtype: "savings", + balance: 2000 # User cannot override balance. This will be overridden by the processor on next processing + ) + + @plaid_account.account.lock_attr!(:name) + @plaid_account.account.lock_attr!(:subtype) + @plaid_account.account.lock_attr!(:balance) # Even if balance somehow becomes locked, Plaid ignores it and overrides it + + assert_no_difference "Account.count" do + PlaidAccount::Processor.new(@plaid_account).process + end + + @plaid_account.reload + + assert_equal "User updated name", @plaid_account.account.name + assert_equal "savings", @plaid_account.account.subtype + assert_equal @plaid_account.current_balance, @plaid_account.account.balance # Overriden by processor + end + + test "account processing failure halts further processing" do + Account.any_instance.stubs(:save!).raises(StandardError.new("Test error")) + + PlaidAccount::Transactions::Processor.any_instance.expects(:process).never + PlaidAccount::Investments::TransactionsProcessor.any_instance.expects(:process).never + PlaidAccount::Investments::HoldingsProcessor.any_instance.expects(:process).never + + expect_no_investment_balance_calculator_calls + expect_no_liability_processor_calls + + assert_raises(StandardError) do + PlaidAccount::Processor.new(@plaid_account).process + end + end + + test "product processing failure reports exception and continues processing" do + PlaidAccount::Transactions::Processor.any_instance.stubs(:process).raises(StandardError.new("Test error")) + + # Subsequent product processors still run + expect_investment_product_processor_calls + + assert_nothing_raised do + PlaidAccount::Processor.new(@plaid_account).process + end + end + + test "calculates balance using BalanceCalculator for investment accounts" do + @plaid_account.update!(plaid_type: "investment") + + PlaidAccount::Investments::BalanceCalculator.any_instance.expects(:balance).returns(1000).once + PlaidAccount::Investments::BalanceCalculator.any_instance.expects(:cash_balance).returns(1000).once + + PlaidAccount::Processor.new(@plaid_account).process + end + + test "processes credit liability data" do + expect_investment_product_processor_calls + expect_no_investment_balance_calculator_calls + expect_depository_product_processor_calls + + @plaid_account.update!(plaid_type: "credit", plaid_subtype: "credit card") + + PlaidAccount::Liabilities::CreditProcessor.any_instance.expects(:process).once + PlaidAccount::Liabilities::MortgageProcessor.any_instance.expects(:process).never + PlaidAccount::Liabilities::StudentLoanProcessor.any_instance.expects(:process).never + + PlaidAccount::Processor.new(@plaid_account).process + end + + test "processes mortgage liability data" do + expect_investment_product_processor_calls + expect_no_investment_balance_calculator_calls + expect_depository_product_processor_calls + + @plaid_account.update!(plaid_type: "loan", plaid_subtype: "mortgage") + + PlaidAccount::Liabilities::CreditProcessor.any_instance.expects(:process).never + PlaidAccount::Liabilities::MortgageProcessor.any_instance.expects(:process).once + PlaidAccount::Liabilities::StudentLoanProcessor.any_instance.expects(:process).never + + PlaidAccount::Processor.new(@plaid_account).process + end + + test "processes student loan liability data" do + expect_investment_product_processor_calls + expect_no_investment_balance_calculator_calls + expect_depository_product_processor_calls + + @plaid_account.update!(plaid_type: "loan", plaid_subtype: "student") + + PlaidAccount::Liabilities::CreditProcessor.any_instance.expects(:process).never + PlaidAccount::Liabilities::MortgageProcessor.any_instance.expects(:process).never + PlaidAccount::Liabilities::StudentLoanProcessor.any_instance.expects(:process).once + + PlaidAccount::Processor.new(@plaid_account).process + end + + private + def expect_investment_product_processor_calls + PlaidAccount::Investments::TransactionsProcessor.any_instance.expects(:process).once + PlaidAccount::Investments::HoldingsProcessor.any_instance.expects(:process).once + end + + def expect_depository_product_processor_calls + PlaidAccount::Transactions::Processor.any_instance.expects(:process).once + end + + def expect_no_investment_balance_calculator_calls + PlaidAccount::Investments::BalanceCalculator.any_instance.expects(:balance).never + PlaidAccount::Investments::BalanceCalculator.any_instance.expects(:cash_balance).never + end + + def expect_no_liability_processor_calls + PlaidAccount::Liabilities::CreditProcessor.any_instance.expects(:process).never + PlaidAccount::Liabilities::MortgageProcessor.any_instance.expects(:process).never + PlaidAccount::Liabilities::StudentLoanProcessor.any_instance.expects(:process).never + end + + def expect_default_subprocessor_calls + expect_depository_product_processor_calls + expect_investment_product_processor_calls + expect_no_investment_balance_calculator_calls + expect_no_liability_processor_calls + end +end