fix: Resolve infinite sync loop on SnapTrade setup accounts page (#1256)

* Fix infinite sync loop on SnapTrade setup accounts page

* Address PR feedback: behavior assertions & stale sync recovery

- Uses `latest_sync.completed?` so we don't drop dropped/failed syncs
- Replaces `assigns` checks with `assert_select` DOM checks
- Adds required IDs/classes to the html template for assertions

app/controllers/snaptrade_items_controller.rb

@@ -149,8 +149,12 @@ class SnaptradeItemsController < ApplicationController
 
     no_accounts = @unlinked_accounts.blank? && @linked_accounts.blank?
 
-    # If no accounts and not syncing, trigger a sync
-    if no_accounts && !@snaptrade_item.syncing?
+    # We trigger an initial or recovery sync if there are no accounts, we aren't currently syncing,
+    # and the last attempt didn't successfully complete. (If it completed and found 0 accounts, we stop here to avoid an infinite loop.)
+    latest_sync = @snaptrade_item.syncs.ordered.first
+    should_sync = latest_sync.nil? || !latest_sync.completed?
+
+    if no_accounts && !@snaptrade_item.syncing? && should_sync
       @snaptrade_item.sync_later
     end
 

app/views/snaptrade_items/setup_accounts.html.erb

@@ -34,7 +34,7 @@
 
       <% if @waiting_for_sync %>
         <%# Syncing state - show spinner with manual refresh option %>
-        <div class="flex flex-col items-center justify-center py-6 space-y-3">
+        <div id="snaptrade-sync-spinner" class="flex flex-col items-center justify-center py-6 space-y-3">
           <div class="animate-spin rounded-full h-6 w-6 border-b-2 border-primary"></div>
           <p class="text-secondary text-center">
             <%= t("snaptrade_items.setup_accounts.loading", default: "Fetching accounts from SnapTrade...") %>
@@ -60,7 +60,7 @@
         </div>
       <% elsif @no_accounts_found %>
         <%# No accounts found after sync completed %>
-        <div class="flex flex-col items-center justify-center py-6 space-y-3">
+        <div class="no-accounts-found flex flex-col items-center justify-center py-6 space-y-3">
           <%= icon "alert-circle", size: "lg", class: "text-warning" %>
           <p class="text-primary text-center font-medium">
             <%= t("snaptrade_items.setup_accounts.no_accounts_title", default: "No Accounts Found") %>

test/controllers/snaptrade_items_controller_test.rb

@@ -103,4 +103,66 @@ class SnaptradeItemsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to settings_providers_path
     assert_match(/not found/i, flash[:alert])
   end
+
+  # --- setup_accounts throttle-sync fix ---
+  #
+  # The fix on setup_accounts ensures sync_later is only called when there are no
+  # accounts AND the item has never been synced (last_synced_at.blank?).  This
+  # prevents the infinite-spinner loop where every page load re-triggered a sync
+  # even after SnapTrade already confirmed 0 linked accounts.
+  #
+  # Three view-state branches we need to cover:
+  #   A) No accounts + never synced  → trigger sync, render spinner
+  #   B) No accounts + synced once, now idle → skip sync, show "no accounts found"
+  #   C) No accounts + synced once, still syncing → show spinner, do NOT re-queue
+
+  test "setup_accounts triggers sync and shows spinner when item has no accounts and has never been synced" do
+    # Pre-condition: no snaptrade_accounts and no completed syncs (last_synced_at is nil)
+    @snaptrade_item.snaptrade_accounts.destroy_all
+    @snaptrade_item.syncs.destroy_all
+
+    assert_difference "Sync.count", 1 do
+      get setup_accounts_snaptrade_item_url(@snaptrade_item)
+    end
+
+    assert_response :success
+    assert_select "#snaptrade-sync-spinner", count: 1, message: "Expected the spinner to be shown on first visit with no accounts"
+    assert_select ".no-accounts-found", count: 0, message: "Expected the no-accounts UI to be hidden while syncing"
+  end
+
+  test "setup_accounts shows no-accounts-found state after a completed sync returns zero accounts" do
+    # Pre-condition: no snaptrade_accounts, but there IS a past completed sync
+    @snaptrade_item.snaptrade_accounts.destroy_all
+    @snaptrade_item.syncs.destroy_all
+    @snaptrade_item.syncs.create!(status: :completed, completed_at: 1.minute.ago)
+
+    # Item is not currently syncing → @syncing is false
+    assert_not @snaptrade_item.reload.syncing?, "Item should not be syncing for this test"
+
+    assert_no_difference "Sync.count" do
+      get setup_accounts_snaptrade_item_url(@snaptrade_item)
+    end
+
+    assert_response :success
+    assert_select ".no-accounts-found", count: 1, message: "Expected the no-accounts UI to be shown after a completed sync with zero accounts"
+    assert_select "#snaptrade-sync-spinner", count: 0, message: "Expected the spinner to be hidden when there is no active sync"
+  end
+
+  test "setup_accounts does not re-queue a sync when a sync is already in progress" do
+    # Pre-condition: no accounts, one past completed sync, + one visible (in-flight) sync
+    @snaptrade_item.snaptrade_accounts.destroy_all
+    @snaptrade_item.syncs.destroy_all
+    @snaptrade_item.syncs.create!(status: :completed, completed_at: 5.minutes.ago)
+    @snaptrade_item.syncs.create!(status: :pending, created_at: 1.minute.ago)   # visible/in-flight
+
+    assert @snaptrade_item.reload.syncing?, "Item should be syncing for this test"
+
+    assert_no_difference "Sync.count" do
+      get setup_accounts_snaptrade_item_url(@snaptrade_item)
+    end
+
+    assert_response :success
+    assert_select "#snaptrade-sync-spinner", count: 1, message: "Expected the spinner to be shown while sync is in progress"
+    assert_select ".no-accounts-found", count: 0, message: "Expected the no-accounts UI to be hidden while a sync is active"
+  end
 end