From cfadff641f1b05c9bf5fe11b849a7a786f2cbb3a Mon Sep 17 00:00:00 2001 From: dataCenter430 <161712630+dataCenter430@users.noreply.github.com> Date: Thu, 19 Feb 2026 11:51:42 -0700 Subject: [PATCH] Fix crypto subtype for trades api (#1022) * fix: crypto subtype not persisted by permitting :subtype in CryptosController * Backfill crypto subtype for existig accounts so Trades API works * fix: backfill only unlinked cryptos; use raw SQL in migration; deterministic redirect in test * Update schema.rb for BackfillcryptoSubtypeForTrades migration --------- Signed-off-by: dataCenter430 <161712630+dataCenter430@users.noreply.github.com> --- app/controllers/cryptos_controller.rb | 2 +- ...0000_backfill_crypto_subtype_for_trades.rb | 30 +++++++++++++ db/schema.rb | 2 +- test/controllers/cryptos_controller_test.rb | 43 +++++++++++++++++++ test/models/crypto_test.rb | 6 +++ 5 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20260218120000_backfill_crypto_subtype_for_trades.rb diff --git a/app/controllers/cryptos_controller.rb b/app/controllers/cryptos_controller.rb index 87bd53414..2489416e8 100644 --- a/app/controllers/cryptos_controller.rb +++ b/app/controllers/cryptos_controller.rb @@ -1,5 +1,5 @@ class CryptosController < ApplicationController include AccountableResource - permitted_accountable_attributes :id, :tax_treatment + permitted_accountable_attributes :id, :subtype, :tax_treatment end diff --git a/db/migrate/20260218120000_backfill_crypto_subtype_for_trades.rb b/db/migrate/20260218120000_backfill_crypto_subtype_for_trades.rb new file mode 100644 index 000000000..3377d01fd --- /dev/null +++ b/db/migrate/20260218120000_backfill_crypto_subtype_for_trades.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class BackfillCryptoSubtypeForTrades < ActiveRecord::Migration[7.2] + def up + # Crypto accounts created via the UI before the controller permitted :subtype + # had subtype NULL, so supports_trades? was false and the Trades API returned 422. + # Backfill to "exchange" only for manual (unlinked) crypto accounts so they can use + # the Trades API. Skip accounts linked to a provider (e.g. CoinStats wallet) which + # intentionally leave subtype NULL and must remain wallet/sync-only. + # Uses raw SQL to avoid coupling to the Crypto model (see Rails migration guidelines). + say_with_time "Backfilling crypto subtype for manual accounts only" do + execute <<-SQL.squish + UPDATE cryptos + SET subtype = 'exchange' + WHERE subtype IS NULL + AND id IN ( + SELECT a.accountable_id + FROM accounts a + WHERE a.accountable_type = 'Crypto' + AND NOT EXISTS (SELECT 1 FROM account_providers ap WHERE ap.account_id = a.id) + ) + SQL + end + end + + def down + # No-op: we cannot distinguish backfilled records from user-chosen "exchange", + # so reverting would incorrectly clear legitimately set subtypes. + end +end diff --git a/db/schema.rb b/db/schema.rb index cd9839dce..d87e5720a 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: 2026_02_17_120000) do +ActiveRecord::Schema[7.2].define(version: 2026_02_18_120000) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" diff --git a/test/controllers/cryptos_controller_test.rb b/test/controllers/cryptos_controller_test.rb index c0a9d1b20..823930ac5 100644 --- a/test/controllers/cryptos_controller_test.rb +++ b/test/controllers/cryptos_controller_test.rb @@ -6,5 +6,48 @@ class CryptosControllerTest < ActionDispatch::IntegrationTest setup do sign_in @user = users(:family_admin) @account = accounts(:crypto) + @family = @user.family + end + + test "create persists subtype so account supports trades" do + Family.any_instance.stubs(:get_link_token).returns("test-link-token") + + assert_difference "@family.accounts.count", 1 do + post cryptos_path, params: { + account: { + name: "Crypto Exchange Account", + balance: 0, + currency: @family.currency, + accountable_type: "Crypto", + accountable_attributes: { subtype: "exchange", tax_treatment: "taxable" } + } + } + end + + assert_response :redirect + created = Account.find(URI(response.location).path.split("/").last) + assert_redirected_to created + assert_equal "exchange", created.accountable.subtype, "subtype must be persisted for trades API" + assert created.supports_trades?, "exchange crypto account must support trades" + end + + test "update persists subtype" do + @account.accountable.update_column(:subtype, nil) + assert_nil @account.reload.accountable.subtype + refute @account.supports_trades? + + patch crypto_path(@account), params: { + account: { + name: @account.name, + balance: @account.balance, + currency: @account.currency, + accountable_attributes: { id: @account.accountable_id, subtype: "exchange", tax_treatment: "taxable" } + } + } + + assert_redirected_to @account + @account.reload + assert_equal "exchange", @account.accountable.subtype + assert @account.supports_trades? end end diff --git a/test/models/crypto_test.rb b/test/models/crypto_test.rb index d32ba009b..f352028d6 100644 --- a/test/models/crypto_test.rb +++ b/test/models/crypto_test.rb @@ -27,4 +27,10 @@ class CryptoTest < ActiveSupport::TestCase assert crypto.tax_deferred? assert_not crypto.tax_exempt? end + + test "supports_trades? is true only for exchange subtype" do + assert Crypto.new(subtype: "exchange").supports_trades? + refute Crypto.new(subtype: "wallet").supports_trades? + refute Crypto.new(subtype: nil).supports_trades? + end end