diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb
index c7d23cf51..d76578381 100644
--- a/app/controllers/accounts_controller.rb
+++ b/app/controllers/accounts_controller.rb
@@ -116,14 +116,11 @@ class AccountsController < ApplicationController
# Capture provider accounts before clearing links (so we can destroy them)
simplefin_account_to_destroy = @account.simplefin_account
- # Capture SnaptradeAccounts linked via AccountProvider
- # Destroying them will trigger delete_snaptrade_connection callback to free connection slots
- snaptrade_accounts_to_destroy = @account.account_providers
- .where(provider_type: "SnaptradeAccount")
- .map { |ap| SnaptradeAccount.find_by(id: ap.provider_id) }
- .compact
-
# Remove new system links (account_providers join table)
+ # SnaptradeAccount records are preserved (not destroyed) so users can relink later.
+ # This follows the Plaid pattern where the provider account survives as "unlinked".
+ # SnapTrade has limited connection slots (5 free), so preserving the record avoids
+ # wasting a slot on reconnect.
@account.account_providers.destroy_all
# Remove legacy system links (foreign keys)
@@ -135,11 +132,6 @@ class AccountsController < ApplicationController
# - SimplefinAccount only caches API data which is regenerated on reconnect
# - If user reconnects SimpleFin later, a new SimplefinAccount will be created
simplefin_account_to_destroy&.destroy!
-
- # Destroy SnaptradeAccount records to free up SnapTrade connection slots
- # The before_destroy callback will delete the connection from SnapTrade API
- # if no other accounts share the same authorization
- snaptrade_accounts_to_destroy.each(&:destroy!)
end
redirect_to accounts_path, notice: t("accounts.unlink.success")
diff --git a/app/controllers/snaptrade_items_controller.rb b/app/controllers/snaptrade_items_controller.rb
index fa4fb054d..dc01c21f3 100644
--- a/app/controllers/snaptrade_items_controller.rb
+++ b/app/controllers/snaptrade_items_controller.rb
@@ -106,27 +106,17 @@ class SnaptradeItemsController < ApplicationController
# Redirect user to SnapTrade connection portal
def connect
- # Ensure user is registered first
- unless @snaptrade_item.user_registered?
- begin
- @snaptrade_item.ensure_user_registered!
- rescue => e
- Rails.logger.error "SnapTrade registration error: #{e.class} - #{e.message}\n#{e.backtrace&.first(5)&.join("\n")}"
- redirect_to settings_providers_path, alert: t(".registration_failed", message: e.message)
- return
- end
- end
+ @snaptrade_item.ensure_user_registered! unless @snaptrade_item.user_registered?
- # Get the connection portal URL - include item ID in callback for proper routing
redirect_url = callback_snaptrade_items_url(item_id: @snaptrade_item.id)
-
- begin
- portal_url = @snaptrade_item.connection_portal_url(redirect_url: redirect_url)
- redirect_to portal_url, allow_other_host: true
- rescue => e
- Rails.logger.error "SnapTrade connection portal error: #{e.class} - #{e.message}\n#{e.backtrace&.first(5)&.join("\n")}"
- redirect_to settings_providers_path, alert: t(".portal_error", message: e.message)
- end
+ portal_url = @snaptrade_item.connection_portal_url(redirect_url: redirect_url)
+ redirect_to portal_url, allow_other_host: true
+ rescue ActiveRecord::Encryption::Errors::Decryption => e
+ Rails.logger.error "SnapTrade decryption error for item #{@snaptrade_item.id}: #{e.class} - #{e.message}\n#{e.backtrace&.first(5)&.join("\n")}"
+ redirect_to settings_providers_path, alert: t(".decryption_failed")
+ rescue => e
+ Rails.logger.error "SnapTrade connection error: #{e.class} - #{e.message}\n#{e.backtrace&.first(5)&.join("\n")}"
+ redirect_to settings_providers_path, alert: t(".connection_failed", message: e.message)
end
# Handle callback from SnapTrade after user connects brokerage
diff --git a/app/views/snaptrade_items/_connections_list.html.erb b/app/views/snaptrade_items/_connections_list.html.erb
index e747ec33d..3b329a7e6 100644
--- a/app/views/snaptrade_items/_connections_list.html.erb
+++ b/app/views/snaptrade_items/_connections_list.html.erb
@@ -53,7 +53,10 @@
">
<%= account[:name] %>
<% unless account[:linked] %>
- (<%= t("providers.snaptrade.needs_linking") %>)
+ <%= link_to "(#{t('providers.snaptrade.needs_linking')})",
+ setup_accounts_snaptrade_item_path(snaptrade_item),
+ class: "text-warning hover:text-primary underline",
+ data: { turbo_frame: :modal } %>
<% end %>
<% end %>
diff --git a/config/locales/views/snaptrade_items/en.yml b/config/locales/views/snaptrade_items/en.yml
index 779c8a93c..f246a9418 100644
--- a/config/locales/views/snaptrade_items/en.yml
+++ b/config/locales/views/snaptrade_items/en.yml
@@ -8,8 +8,8 @@ en:
destroy:
success: "Scheduled SnapTrade connection for deletion."
connect:
- registration_failed: "Failed to register with SnapTrade: %{message}"
- portal_error: "Failed to connect to SnapTrade: %{message}"
+ decryption_failed: "Unable to read SnapTrade credentials. Please delete and recreate this connection."
+ connection_failed: "Failed to connect to SnapTrade: %{message}"
callback:
success: "Brokerage connected! Please select which accounts to link."
no_item: "SnapTrade configuration not found."
diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb
index 4c690ce0b..c192c278b 100644
--- a/test/controllers/accounts_controller_test.rb
+++ b/test/controllers/accounts_controller_test.rb
@@ -165,6 +165,51 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to account_url(@account)
assert_equal "Account is already linked to a provider", flash[:alert]
end
+
+ test "unlink preserves SnaptradeAccount record" do
+ snaptrade_account = snaptrade_accounts(:fidelity_401k)
+ investment = accounts(:investment)
+ AccountProvider.create!(account: investment, provider: snaptrade_account)
+ investment.reload
+
+ assert investment.linked?
+
+ delete unlink_account_url(investment)
+ investment.reload
+
+ assert_not investment.linked?
+ assert_redirected_to accounts_path
+ # SnaptradeAccount should still exist (not destroyed)
+ assert SnaptradeAccount.exists?(snaptrade_account.id), "SnaptradeAccount should be preserved after unlink"
+ # But AccountProvider should be gone
+ assert_not AccountProvider.exists?(provider_type: "SnaptradeAccount", provider_id: snaptrade_account.id)
+ end
+
+ test "unlink does not enqueue SnapTrade cleanup job" do
+ snaptrade_account = snaptrade_accounts(:fidelity_401k)
+ investment = accounts(:investment)
+ AccountProvider.create!(account: investment, provider: snaptrade_account)
+ investment.reload
+
+ assert_no_enqueued_jobs(only: SnaptradeConnectionCleanupJob) do
+ delete unlink_account_url(investment)
+ end
+ end
+
+ test "unlink detaches holdings from SnapTrade provider" do
+ snaptrade_account = snaptrade_accounts(:fidelity_401k)
+ investment = accounts(:investment)
+ ap = AccountProvider.create!(account: investment, provider: snaptrade_account)
+
+ # Assign a holding to this provider
+ holding = holdings(:one)
+ holding.update!(account_provider: ap)
+
+ delete unlink_account_url(investment)
+ holding.reload
+
+ assert_nil holding.account_provider_id, "Holding should be detached from provider after unlink"
+ end
end
class AccountsControllerSimplefinCtaTest < ActionDispatch::IntegrationTest
diff --git a/test/controllers/snaptrade_items_controller_test.rb b/test/controllers/snaptrade_items_controller_test.rb
new file mode 100644
index 000000000..9f6f78944
--- /dev/null
+++ b/test/controllers/snaptrade_items_controller_test.rb
@@ -0,0 +1,41 @@
+require "test_helper"
+
+class SnaptradeItemsControllerTest < ActionDispatch::IntegrationTest
+ setup do
+ sign_in @user = users(:family_admin)
+ @snaptrade_item = snaptrade_items(:configured_item)
+ end
+
+ test "connect handles decryption error gracefully" do
+ SnaptradeItem.any_instance
+ .stubs(:user_registered?)
+ .raises(ActiveRecord::Encryption::Errors::Decryption.new("cannot decrypt"))
+
+ get connect_snaptrade_item_url(@snaptrade_item)
+
+ assert_redirected_to settings_providers_path
+ assert_match(/Unable to read SnapTrade credentials/, flash[:alert])
+ end
+
+ test "connect handles general error gracefully" do
+ SnaptradeItem.any_instance
+ .stubs(:user_registered?)
+ .raises(StandardError.new("something broke"))
+
+ get connect_snaptrade_item_url(@snaptrade_item)
+
+ assert_redirected_to settings_providers_path
+ assert_match(/Failed to connect/, flash[:alert])
+ end
+
+ test "connect redirects to portal when successful" do
+ portal_url = "https://app.snaptrade.com/portal/test123"
+
+ SnaptradeItem.any_instance.stubs(:user_registered?).returns(true)
+ SnaptradeItem.any_instance.stubs(:connection_portal_url).returns(portal_url)
+
+ get connect_snaptrade_item_url(@snaptrade_item)
+
+ assert_redirected_to portal_url
+ end
+end