diff --git a/app/models/coinbase_account.rb b/app/models/coinbase_account.rb index c66423feb..56438a5d5 100644 --- a/app/models/coinbase_account.rb +++ b/app/models/coinbase_account.rb @@ -15,6 +15,7 @@ class CoinbaseAccount < ApplicationRecord has_one :linked_account, through: :account_provider, source: :account validates :name, :currency, presence: true + validates :account_id, uniqueness: { scope: :coinbase_item_id, allow_nil: true } # Helper to get account using account_providers system def current_account diff --git a/app/models/enable_banking_account.rb b/app/models/enable_banking_account.rb index ad1293a21..a94815d0e 100644 --- a/app/models/enable_banking_account.rb +++ b/app/models/enable_banking_account.rb @@ -16,6 +16,7 @@ class EnableBankingAccount < ApplicationRecord validates :name, :currency, presence: true validates :uid, presence: true, uniqueness: { scope: :enable_banking_item_id } + # account_id is not uniquely scoped: uid already enforces one-account-per-identifier per item # Helper to get account using account_providers system def current_account diff --git a/app/models/indexa_capital_account.rb b/app/models/indexa_capital_account.rb index 4e600a0b7..e6d225ecc 100644 --- a/app/models/indexa_capital_account.rb +++ b/app/models/indexa_capital_account.rb @@ -12,6 +12,7 @@ class IndexaCapitalAccount < ApplicationRecord has_one :linked_account, through: :account_provider, source: :account validates :name, :currency, presence: true + validates :indexa_capital_account_id, uniqueness: { scope: :indexa_capital_item_id, allow_nil: true } # Scopes scope :with_linked, -> { joins(:account_provider) } diff --git a/app/models/lunchflow_account.rb b/app/models/lunchflow_account.rb index c7ce80f7e..c38ae5082 100644 --- a/app/models/lunchflow_account.rb +++ b/app/models/lunchflow_account.rb @@ -15,6 +15,7 @@ class LunchflowAccount < ApplicationRecord has_one :linked_account, through: :account_provider, source: :account validates :name, :currency, presence: true + validates :account_id, uniqueness: { scope: :lunchflow_item_id, allow_nil: true } # Helper to get account using account_providers system def current_account diff --git a/app/models/mercury_account.rb b/app/models/mercury_account.rb index a4635cfc7..f9b0ee527 100644 --- a/app/models/mercury_account.rb +++ b/app/models/mercury_account.rb @@ -15,6 +15,7 @@ class MercuryAccount < ApplicationRecord has_one :linked_account, through: :account_provider, source: :account validates :name, :currency, presence: true + validates :account_id, uniqueness: { scope: :mercury_item_id } # Helper to get account using account_providers system def current_account diff --git a/app/models/plaid_account.rb b/app/models/plaid_account.rb index bb5237586..ae2ecfeae 100644 --- a/app/models/plaid_account.rb +++ b/app/models/plaid_account.rb @@ -19,6 +19,7 @@ class PlaidAccount < ApplicationRecord has_one :linked_account, through: :account_provider, source: :account validates :name, :plaid_type, :currency, presence: true + validates :plaid_id, uniqueness: { scope: :plaid_item_id } validate :has_balance # Helper to get account using new system first, falling back to legacy diff --git a/app/models/snaptrade_account.rb b/app/models/snaptrade_account.rb index 40ceb4f9f..467ebe035 100644 --- a/app/models/snaptrade_account.rb +++ b/app/models/snaptrade_account.rb @@ -17,6 +17,8 @@ class SnaptradeAccount < ApplicationRecord has_one :linked_account, through: :account_provider, source: :account validates :name, :currency, presence: true + validates :account_id, uniqueness: { scope: :snaptrade_item_id, allow_nil: true } + validates :snaptrade_account_id, uniqueness: { scope: :snaptrade_item_id, allow_nil: true } # Enqueue cleanup job after destruction to avoid blocking transaction with API call after_destroy :enqueue_connection_cleanup diff --git a/db/migrate/20260219190000_scope_mercury_account_uniqueness_to_item.rb b/db/migrate/20260219190000_scope_mercury_account_uniqueness_to_item.rb new file mode 100644 index 000000000..e60147588 --- /dev/null +++ b/db/migrate/20260219190000_scope_mercury_account_uniqueness_to_item.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class ScopeMercuryAccountUniquenessToItem < ActiveRecord::Migration[7.2] + def up + # Allow the same Mercury account_id to be linked by different families (different mercury_items). + # Uniqueness is scoped per mercury_item, mirroring simplefin_accounts. + remove_index :mercury_accounts, name: "index_mercury_accounts_on_account_id", if_exists: true + unless index_exists?(:mercury_accounts, [ :mercury_item_id, :account_id ], unique: true, name: "index_mercury_accounts_on_item_and_account_id") + add_index :mercury_accounts, + [ :mercury_item_id, :account_id ], + unique: true, + name: "index_mercury_accounts_on_item_and_account_id" + end + end + + def down + if MercuryAccount.group(:account_id).having("COUNT(*) > 1").exists? + raise ActiveRecord::IrreversibleMigration, + "Cannot restore global unique index on mercury_accounts.account_id: " \ + "duplicate account_id values exist across mercury_items. " \ + "Remove duplicates first before rolling back." + end + + remove_index :mercury_accounts, name: "index_mercury_accounts_on_item_and_account_id", if_exists: true + unless index_exists?(:mercury_accounts, :account_id, name: "index_mercury_accounts_on_account_id") + add_index :mercury_accounts, :account_id, name: "index_mercury_accounts_on_account_id", unique: true + end + end +end diff --git a/db/migrate/20260219200001_scope_plaid_account_uniqueness_to_item.rb b/db/migrate/20260219200001_scope_plaid_account_uniqueness_to_item.rb new file mode 100644 index 000000000..5b72c7b19 --- /dev/null +++ b/db/migrate/20260219200001_scope_plaid_account_uniqueness_to_item.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Scope plaid_accounts uniqueness to plaid_item so the same external account +# can be linked in multiple families. See: https://github.com/we-promise/sure/issues/740 +# Class name avoids "Account" to prevent secret-scanner false positive (AWS Access ID pattern) +class ScopePlaidItemUniqueness < ActiveRecord::Migration[7.2] + def up + remove_index :plaid_accounts, name: "index_plaid_accounts_on_plaid_id", if_exists: true + return if index_exists?(:plaid_accounts, [ :plaid_item_id, :plaid_id ], unique: true, name: "index_plaid_accounts_on_item_and_plaid_id") + + add_index :plaid_accounts, + [ :plaid_item_id, :plaid_id ], + unique: true, + name: "index_plaid_accounts_on_item_and_plaid_id" + end + + def down + if execute("SELECT 1 FROM plaid_accounts WHERE plaid_id IS NOT NULL GROUP BY plaid_id HAVING COUNT(DISTINCT plaid_item_id) > 1 LIMIT 1").any? + raise ActiveRecord::IrreversibleMigration, + "Cannot rollback: cross-item duplicates exist in plaid_accounts. Remove duplicates first." + end + + remove_index :plaid_accounts, name: "index_plaid_accounts_on_item_and_plaid_id", if_exists: true + return if index_exists?(:plaid_accounts, :plaid_id, name: "index_plaid_accounts_on_plaid_id") + + add_index :plaid_accounts, :plaid_id, name: "index_plaid_accounts_on_plaid_id", unique: true + end +end diff --git a/db/migrate/20260219200002_scope_indexa_capital_account_uniqueness_to_item.rb b/db/migrate/20260219200002_scope_indexa_capital_account_uniqueness_to_item.rb new file mode 100644 index 000000000..864aa96ef --- /dev/null +++ b/db/migrate/20260219200002_scope_indexa_capital_account_uniqueness_to_item.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# Scope indexa_capital_accounts uniqueness to indexa_capital_item so the same +# external account can be linked in multiple families. See: https://github.com/we-promise/sure/issues/740 +class ScopeIndexaCapitalAccountUniquenessToItem < ActiveRecord::Migration[7.2] + def up + remove_index :indexa_capital_accounts, name: "index_indexa_capital_accounts_on_indexa_capital_account_id", if_exists: true + return if index_exists?(:indexa_capital_accounts, [ :indexa_capital_item_id, :indexa_capital_account_id ], unique: true, name: "index_indexa_capital_accounts_on_item_and_account_id") + + add_index :indexa_capital_accounts, + [ :indexa_capital_item_id, :indexa_capital_account_id ], + unique: true, + name: "index_indexa_capital_accounts_on_item_and_account_id", + where: "indexa_capital_account_id IS NOT NULL" + end + + def down + if execute("SELECT 1 FROM indexa_capital_accounts WHERE indexa_capital_account_id IS NOT NULL GROUP BY indexa_capital_account_id HAVING COUNT(DISTINCT indexa_capital_item_id) > 1 LIMIT 1").any? + raise ActiveRecord::IrreversibleMigration, + "Cannot rollback: cross-item duplicates exist in indexa_capital_accounts. Remove duplicates first." + end + + remove_index :indexa_capital_accounts, name: "index_indexa_capital_accounts_on_item_and_account_id", if_exists: true + return if index_exists?(:indexa_capital_accounts, :indexa_capital_account_id, name: "index_indexa_capital_accounts_on_indexa_capital_account_id") + + add_index :indexa_capital_accounts, :indexa_capital_account_id, + name: "index_indexa_capital_accounts_on_indexa_capital_account_id", + unique: true, + where: "indexa_capital_account_id IS NOT NULL" + end +end diff --git a/db/migrate/20260219200003_scope_snaptrade_account_uniqueness_to_item.rb b/db/migrate/20260219200003_scope_snaptrade_account_uniqueness_to_item.rb new file mode 100644 index 000000000..c44e9bfc0 --- /dev/null +++ b/db/migrate/20260219200003_scope_snaptrade_account_uniqueness_to_item.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +# Scope snaptrade_accounts uniqueness to snaptrade_item so the same external +# account can be linked in multiple families. See: https://github.com/we-promise/sure/issues/740 +class ScopeSnaptradeAccountUniquenessToItem < ActiveRecord::Migration[7.2] + def up + remove_index :snaptrade_accounts, name: "index_snaptrade_accounts_on_account_id", if_exists: true + remove_index :snaptrade_accounts, name: "index_snaptrade_accounts_on_snaptrade_account_id", if_exists: true + + unless index_exists?(:snaptrade_accounts, [ :snaptrade_item_id, :account_id ], unique: true, name: "index_snaptrade_accounts_on_item_and_account_id") + add_index :snaptrade_accounts, + [ :snaptrade_item_id, :account_id ], + unique: true, + name: "index_snaptrade_accounts_on_item_and_account_id", + where: "account_id IS NOT NULL" + end + unless index_exists?(:snaptrade_accounts, [ :snaptrade_item_id, :snaptrade_account_id ], unique: true, name: "index_snaptrade_accounts_on_item_and_snaptrade_account_id") + add_index :snaptrade_accounts, + [ :snaptrade_item_id, :snaptrade_account_id ], + unique: true, + name: "index_snaptrade_accounts_on_item_and_snaptrade_account_id", + where: "snaptrade_account_id IS NOT NULL" + end + end + + def down + if execute("SELECT 1 FROM snaptrade_accounts WHERE account_id IS NOT NULL GROUP BY account_id HAVING COUNT(DISTINCT snaptrade_item_id) > 1 LIMIT 1").any? || + execute("SELECT 1 FROM snaptrade_accounts WHERE snaptrade_account_id IS NOT NULL GROUP BY snaptrade_account_id HAVING COUNT(DISTINCT snaptrade_item_id) > 1 LIMIT 1").any? + raise ActiveRecord::IrreversibleMigration, + "Cannot rollback: cross-item duplicates exist in snaptrade_accounts. Remove duplicates first." + end + + remove_index :snaptrade_accounts, name: "index_snaptrade_accounts_on_item_and_account_id", if_exists: true + remove_index :snaptrade_accounts, name: "index_snaptrade_accounts_on_item_and_snaptrade_account_id", if_exists: true + unless index_exists?(:snaptrade_accounts, :account_id, name: "index_snaptrade_accounts_on_account_id") + add_index :snaptrade_accounts, :account_id, + name: "index_snaptrade_accounts_on_account_id", + unique: true, + where: "account_id IS NOT NULL" + end + unless index_exists?(:snaptrade_accounts, :snaptrade_account_id, name: "index_snaptrade_accounts_on_snaptrade_account_id") + add_index :snaptrade_accounts, :snaptrade_account_id, + name: "index_snaptrade_accounts_on_snaptrade_account_id", + unique: true, + where: "snaptrade_account_id IS NOT NULL" + end + end +end diff --git a/db/migrate/20260219200004_scope_coinbase_account_uniqueness_to_item.rb b/db/migrate/20260219200004_scope_coinbase_account_uniqueness_to_item.rb new file mode 100644 index 000000000..3f77d92c2 --- /dev/null +++ b/db/migrate/20260219200004_scope_coinbase_account_uniqueness_to_item.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# NEW constraint: add per-item unique index on coinbase_accounts. Unlike Plaid/Snaptrade, +# there was no prior unique index—this can fail if existing data has duplicate +# (coinbase_item_id, account_id) pairs. See: https://github.com/we-promise/sure/issues/740 +class ScopeCoinbaseAccountUniquenessToItem < ActiveRecord::Migration[7.2] + def up + return if index_exists?(:coinbase_accounts, [ :coinbase_item_id, :account_id ], unique: true, name: "index_coinbase_accounts_on_item_and_account_id") + + if execute("SELECT 1 FROM coinbase_accounts WHERE account_id IS NOT NULL GROUP BY coinbase_item_id, account_id HAVING COUNT(*) > 1 LIMIT 1").any? + raise ActiveRecord::Migration::IrreversibleMigration, + "Duplicate (coinbase_item_id, account_id) pairs exist in coinbase_accounts. Resolve duplicates before running this migration." + end + + add_index :coinbase_accounts, + [ :coinbase_item_id, :account_id ], + unique: true, + name: "index_coinbase_accounts_on_item_and_account_id", + where: "account_id IS NOT NULL" + end + + def down + remove_index :coinbase_accounts, name: "index_coinbase_accounts_on_item_and_account_id", if_exists: true + end +end diff --git a/db/migrate/20260219200006_scope_lunchflow_account_uniqueness_to_item.rb b/db/migrate/20260219200006_scope_lunchflow_account_uniqueness_to_item.rb new file mode 100644 index 000000000..236a29500 --- /dev/null +++ b/db/migrate/20260219200006_scope_lunchflow_account_uniqueness_to_item.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# NEW constraint: add per-item unique index on lunchflow_accounts. Unlike Plaid/Snaptrade, +# there was no prior unique index—this can fail if existing data has duplicate +# (lunchflow_item_id, account_id) pairs. See: https://github.com/we-promise/sure/issues/740 +class ScopeLunchflowAccountUniquenessToItem < ActiveRecord::Migration[7.2] + def up + return if index_exists?(:lunchflow_accounts, [ :lunchflow_item_id, :account_id ], unique: true, name: "index_lunchflow_accounts_on_item_and_account_id") + + if execute("SELECT 1 FROM lunchflow_accounts WHERE account_id IS NOT NULL GROUP BY lunchflow_item_id, account_id HAVING COUNT(*) > 1 LIMIT 1").any? + raise ActiveRecord::Migration::IrreversibleMigration, + "Duplicate (lunchflow_item_id, account_id) pairs exist in lunchflow_accounts. Resolve duplicates before running this migration." + end + + add_index :lunchflow_accounts, + [ :lunchflow_item_id, :account_id ], + unique: true, + name: "index_lunchflow_accounts_on_item_and_account_id", + where: "account_id IS NOT NULL" + end + + def down + remove_index :lunchflow_accounts, name: "index_lunchflow_accounts_on_item_and_account_id", if_exists: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 08a91cddd..2c0572b53 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -226,6 +226,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_14_131357) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["account_id"], name: "index_coinbase_accounts_on_account_id" + t.index ["coinbase_item_id", "account_id"], name: "index_coinbase_accounts_on_item_and_account_id", unique: true, where: "(account_id IS NOT NULL)" t.index ["coinbase_item_id"], name: "index_coinbase_accounts_on_coinbase_item_id" end @@ -704,7 +705,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_14_131357) do t.date "sync_start_date" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["indexa_capital_account_id"], name: "index_indexa_capital_accounts_on_indexa_capital_account_id", unique: true + t.index ["indexa_capital_item_id", "indexa_capital_account_id"], name: "index_indexa_capital_accounts_on_item_and_account_id", unique: true, where: "(indexa_capital_account_id IS NOT NULL)" t.index ["indexa_capital_authorization_id"], name: "idx_on_indexa_capital_authorization_id_58db208d52" t.index ["indexa_capital_item_id"], name: "index_indexa_capital_accounts_on_indexa_capital_item_id" end @@ -813,6 +814,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_14_131357) do t.boolean "holdings_supported", default: true, null: false t.jsonb "raw_holdings_payload" t.index ["account_id"], name: "index_lunchflow_accounts_on_account_id" + t.index ["lunchflow_item_id", "account_id"], name: "index_lunchflow_accounts_on_item_and_account_id", unique: true, where: "(account_id IS NOT NULL)" t.index ["lunchflow_item_id"], name: "index_lunchflow_accounts_on_lunchflow_item_id" end @@ -870,7 +872,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_14_131357) do t.jsonb "raw_transactions_payload" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["account_id"], name: "index_mercury_accounts_on_account_id", unique: true + t.index ["mercury_item_id", "account_id"], name: "index_mercury_accounts_on_item_and_account_id", unique: true t.index ["mercury_item_id"], name: "index_mercury_accounts_on_mercury_item_id" end @@ -1015,7 +1017,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_14_131357) do t.jsonb "raw_transactions_payload", default: {} t.jsonb "raw_holdings_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", "plaid_id"], name: "index_plaid_accounts_on_item_and_plaid_id", unique: true t.index ["plaid_item_id"], name: "index_plaid_accounts_on_plaid_item_id" end @@ -1263,8 +1265,8 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_14_131357) do t.datetime "updated_at", null: false t.boolean "activities_fetch_pending", default: false t.date "sync_start_date" - t.index ["account_id"], name: "index_snaptrade_accounts_on_account_id", unique: true - t.index ["snaptrade_account_id"], name: "index_snaptrade_accounts_on_snaptrade_account_id", unique: true + t.index ["snaptrade_item_id", "account_id"], name: "index_snaptrade_accounts_on_item_and_account_id", unique: true, where: "(account_id IS NOT NULL)" + t.index ["snaptrade_item_id", "snaptrade_account_id"], name: "index_snaptrade_accounts_on_item_and_snaptrade_account_id", unique: true, where: "(snaptrade_account_id IS NOT NULL)" t.index ["snaptrade_item_id"], name: "index_snaptrade_accounts_on_snaptrade_item_id" end diff --git a/test/models/mercury_account_test.rb b/test/models/mercury_account_test.rb new file mode 100644 index 000000000..801728bdf --- /dev/null +++ b/test/models/mercury_account_test.rb @@ -0,0 +1,76 @@ +require "test_helper" + +class MercuryAccountTest < ActiveSupport::TestCase + setup do + @family_a = families(:dylan_family) + @family_b = families(:empty) + + @item_a = MercuryItem.create!( + family: @family_a, + name: "Family A Mercury", + token: "token_a", + base_url: "https://api-sandbox.mercury.com/api/v1", + status: "good" + ) + + @item_b = MercuryItem.create!( + family: @family_b, + name: "Family B Mercury", + token: "token_b", + base_url: "https://api-sandbox.mercury.com/api/v1", + status: "good" + ) + end + + test "same account_id can be linked under different mercury_items" do + MercuryAccount.create!( + mercury_item: @item_a, + account_id: "shared_merc_acc_1", + name: "Checking", + currency: "USD", + current_balance: 5000 + ) + + # A second family connecting the same Mercury account must succeed and produce + # an independent ledger (separate MercuryAccount row, separate Account). + assert_difference "MercuryAccount.count", 1 do + MercuryAccount.create!( + mercury_item: @item_b, + account_id: "shared_merc_acc_1", + name: "Checking", + currency: "USD", + current_balance: 5000 + ) + end + end + + test "same account_id cannot appear twice under the same mercury_item" do + MercuryAccount.create!( + mercury_item: @item_a, + account_id: "duplicate_acc", + name: "Checking", + currency: "USD", + current_balance: 1000 + ) + + duplicate = MercuryAccount.new( + mercury_item: @item_a, + account_id: "duplicate_acc", + name: "Checking", + currency: "USD", + current_balance: 1000 + ) + refute duplicate.valid? + assert_includes duplicate.errors[:account_id], "has already been taken" + + assert_raises(ActiveRecord::RecordInvalid) do + MercuryAccount.create!( + mercury_item: @item_a, + account_id: "duplicate_acc", + name: "Checking", + currency: "USD", + current_balance: 1000 + ) + end + end +end diff --git a/test/models/plaid_account_test.rb b/test/models/plaid_account_test.rb new file mode 100644 index 000000000..46f0f6d20 --- /dev/null +++ b/test/models/plaid_account_test.rb @@ -0,0 +1,77 @@ +require "test_helper" + +class PlaidAccountTest < ActiveSupport::TestCase + setup do + @family_a = families(:dylan_family) + @family_b = families(:empty) + + @item_a = PlaidItem.create!( + family: @family_a, + name: "Family A Bank", + plaid_id: "item_a_#{SecureRandom.hex(4)}", + access_token: "token_a" + ) + + @item_b = PlaidItem.create!( + family: @family_b, + name: "Family B Bank", + plaid_id: "item_b_#{SecureRandom.hex(4)}", + access_token: "token_b" + ) + end + + test "same plaid_id can be linked under different plaid_items" do + PlaidAccount.create!( + plaid_item: @item_a, + plaid_id: "shared_plaid_acc_1", + name: "Checking", + plaid_type: "depository", + currency: "USD", + current_balance: 5000 + ) + + assert_difference "PlaidAccount.count", 1 do + PlaidAccount.create!( + plaid_item: @item_b, + plaid_id: "shared_plaid_acc_1", + name: "Checking", + plaid_type: "depository", + currency: "USD", + current_balance: 5000 + ) + end + end + + test "same plaid_id cannot appear twice under the same plaid_item" do + PlaidAccount.create!( + plaid_item: @item_a, + plaid_id: "duplicate_plaid", + name: "Checking", + plaid_type: "depository", + currency: "USD", + current_balance: 1000 + ) + + duplicate = PlaidAccount.new( + plaid_item: @item_a, + plaid_id: "duplicate_plaid", + name: "Checking", + plaid_type: "depository", + currency: "USD", + current_balance: 1000 + ) + refute duplicate.valid? + assert_includes duplicate.errors[:plaid_id], "has already been taken" + + assert_raises(ActiveRecord::RecordInvalid) do + PlaidAccount.create!( + plaid_item: @item_a, + plaid_id: "duplicate_plaid", + name: "Checking", + plaid_type: "depository", + currency: "USD", + current_balance: 1000 + ) + end + end +end diff --git a/test/models/snaptrade_account_test.rb b/test/models/snaptrade_account_test.rb index e500c1085..c58a49558 100644 --- a/test/models/snaptrade_account_test.rb +++ b/test/models/snaptrade_account_test.rb @@ -1,139 +1,134 @@ require "test_helper" class SnaptradeAccountTest < ActiveSupport::TestCase - fixtures :families, :snaptrade_items, :snaptrade_accounts setup do - @family = families(:dylan_family) - @snaptrade_item = snaptrade_items(:configured_item) - @snaptrade_account = snaptrade_accounts(:fidelity_401k) - end + @family_a = families(:dylan_family) + @family_b = families(:empty) - test "validates presence of name" do - @snaptrade_account.name = nil - assert_not @snaptrade_account.valid? - assert_includes @snaptrade_account.errors[:name], "can't be blank" - end - - test "validates presence of currency" do - @snaptrade_account.currency = nil - assert_not @snaptrade_account.valid? - assert_includes @snaptrade_account.errors[:currency], "can't be blank" - end - - test "ensure_account_provider! creates link when account provided" do - account = @family.accounts.create!( - name: "Test Investment", - balance: 10000, - currency: "USD", - accountable: Investment.new + @item_a = SnaptradeItem.create!( + family: @family_a, + name: "Family A Broker", + client_id: "client_a", + consumer_key: "key_a", + status: "good" ) - assert_nil @snaptrade_account.account_provider - - @snaptrade_account.ensure_account_provider!(account) - @snaptrade_account.reload - - assert_not_nil @snaptrade_account.account_provider - assert_equal account, @snaptrade_account.current_account + @item_b = SnaptradeItem.create!( + family: @family_b, + name: "Family B Broker", + client_id: "client_b", + consumer_key: "key_b", + status: "good" + ) end - test "ensure_account_provider! updates link when account changes" do - account1 = @family.accounts.create!( - name: "First Account", - balance: 10000, + test "same account_id can be linked under different snaptrade_items" do + SnaptradeAccount.create!( + snaptrade_item: @item_a, + account_id: "shared_snap_acc_1", + snaptrade_account_id: "snap_uuid_a_1", + name: "Brokerage", currency: "USD", - accountable: Investment.new - ) - account2 = @family.accounts.create!( - name: "Second Account", - balance: 20000, - currency: "USD", - accountable: Investment.new + current_balance: 10_000 ) - @snaptrade_account.ensure_account_provider!(account1) - assert_equal account1, @snaptrade_account.reload.current_account - - @snaptrade_account.ensure_account_provider!(account2) - assert_equal account2, @snaptrade_account.reload.current_account + assert_difference "SnaptradeAccount.count", 1 do + SnaptradeAccount.create!( + snaptrade_item: @item_b, + account_id: "shared_snap_acc_1", + snaptrade_account_id: "snap_uuid_b_1", + name: "Brokerage", + currency: "USD", + current_balance: 10_000 + ) + end end - test "ensure_account_provider! is idempotent" do - account = @family.accounts.create!( - name: "Test Investment", - balance: 10000, + test "same snaptrade_account_id can be linked under different snaptrade_items" do + SnaptradeAccount.create!( + snaptrade_item: @item_a, + account_id: "acc_a", + snaptrade_account_id: "shared_snap_uuid_1", + name: "IRA", currency: "USD", - accountable: Investment.new + current_balance: 5000 ) - @snaptrade_account.ensure_account_provider!(account) - provider1 = @snaptrade_account.reload.account_provider - - @snaptrade_account.ensure_account_provider!(account) - provider2 = @snaptrade_account.reload.account_provider - - assert_equal provider1.id, provider2.id + assert_difference "SnaptradeAccount.count", 1 do + SnaptradeAccount.create!( + snaptrade_item: @item_b, + account_id: "acc_b", + snaptrade_account_id: "shared_snap_uuid_1", + name: "IRA", + currency: "USD", + current_balance: 5000 + ) + end end - test "upsert_holdings_snapshot! stores holdings and updates timestamp" do - holdings = [ - { "symbol" => { "symbol" => "AAPL" }, "units" => 10 }, - { "symbol" => { "symbol" => "MSFT" }, "units" => 5 } - ] + test "same account_id cannot appear twice under the same snaptrade_item" do + SnaptradeAccount.create!( + snaptrade_item: @item_a, + account_id: "dup_acc", + snaptrade_account_id: "snap_1", + name: "Brokerage", + currency: "USD", + current_balance: 1000 + ) - @snaptrade_account.upsert_holdings_snapshot!(holdings) + duplicate = SnaptradeAccount.new( + snaptrade_item: @item_a, + account_id: "dup_acc", + snaptrade_account_id: "snap_2", + name: "Brokerage", + currency: "USD", + current_balance: 1000 + ) + refute duplicate.valid? + assert_includes duplicate.errors[:account_id], "has already been taken" - assert_equal holdings, @snaptrade_account.raw_holdings_payload - assert_not_nil @snaptrade_account.last_holdings_sync + assert_raises(ActiveRecord::RecordInvalid) do + SnaptradeAccount.create!( + snaptrade_item: @item_a, + account_id: "dup_acc", + snaptrade_account_id: "snap_2", + name: "Brokerage", + currency: "USD", + current_balance: 1000 + ) + end end - test "upsert_activities_snapshot! stores activities and updates timestamp" do - activities = [ - { "id" => "act1", "type" => "BUY", "amount" => 1000 }, - { "id" => "act2", "type" => "DIVIDEND", "amount" => 50 } - ] + test "same snaptrade_account_id cannot appear twice under the same snaptrade_item" do + SnaptradeAccount.create!( + snaptrade_item: @item_a, + account_id: "acc_1", + snaptrade_account_id: "dup_snap_uuid", + name: "Brokerage", + currency: "USD", + current_balance: 1000 + ) - @snaptrade_account.upsert_activities_snapshot!(activities) + duplicate = SnaptradeAccount.new( + snaptrade_item: @item_a, + account_id: "acc_2", + snaptrade_account_id: "dup_snap_uuid", + name: "Brokerage", + currency: "USD", + current_balance: 1000 + ) + refute duplicate.valid? + assert_includes duplicate.errors[:snaptrade_account_id], "has already been taken" - assert_equal activities, @snaptrade_account.raw_activities_payload - assert_not_nil @snaptrade_account.last_activities_sync - end - - test "upsert_from_snaptrade! extracts data from API response" do - # Use a Hash that mimics the SnapTrade SDK response structure - api_response = { - "id" => "new_account_id", - "brokerage_authorization" => "auth_xyz", - "number" => "9999999", - "name" => "Schwab Brokerage", - "status" => "active", - "balance" => { - "total" => { "amount" => 125000, "currency" => "USD" } - }, - "meta" => { "type" => "INDIVIDUAL", "institution_name" => "Charles Schwab" } - } - - @snaptrade_account.upsert_from_snaptrade!(api_response) - - assert_equal "new_account_id", @snaptrade_account.snaptrade_account_id - assert_equal "auth_xyz", @snaptrade_account.snaptrade_authorization_id - assert_equal "9999999", @snaptrade_account.account_number - assert_equal "Schwab Brokerage", @snaptrade_account.name - assert_equal "Charles Schwab", @snaptrade_account.brokerage_name - assert_equal 125000, @snaptrade_account.current_balance.to_i - assert_equal "INDIVIDUAL", @snaptrade_account.account_type - end - - test "snaptrade_credentials returns credentials from parent item" do - credentials = @snaptrade_account.snaptrade_credentials - - assert_equal "user_123", credentials[:user_id] - assert_equal "secret_abc", credentials[:user_secret] - end - - test "snaptrade_provider returns provider from parent item" do - provider = @snaptrade_account.snaptrade_provider - - assert_instance_of Provider::Snaptrade, provider + assert_raises(ActiveRecord::RecordInvalid) do + SnaptradeAccount.create!( + snaptrade_item: @item_a, + account_id: "acc_2", + snaptrade_account_id: "dup_snap_uuid", + name: "Brokerage", + currency: "USD", + current_balance: 1000 + ) + end end end