Add support to unlink lunch flow accounts (#318)

* Add support to unlink lunch flow accounts

* add support to link and unlink to any provider

* Fix tests and query

* Let's keep Amr happy about his brand

* Wrap unlink operations in a transaction and add error handling.

* Fix tests

---------

Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>

app/controllers/accounts_controller.rb

@@ -1,5 +1,5 @@
 class AccountsController < ApplicationController
-  before_action :set_account, only: %i[sync sparkline toggle_active show destroy]
+  before_action :set_account, only: %i[sync sparkline toggle_active show destroy unlink confirm_unlink select_provider]
   include Periodable
 
   def index
@@ -17,7 +17,7 @@ class AccountsController < ApplicationController
 
   def sync_all
     family.sync_later
-    redirect_to accounts_path, notice: "Syncing accounts..."
+    redirect_to accounts_path, notice: t("accounts.sync_all.syncing")
   end
 
   def show
@@ -71,10 +71,93 @@ class AccountsController < ApplicationController
 
   def destroy
     if @account.linked?
-      redirect_to account_path(@account), alert: "Cannot delete a linked account"
+      redirect_to account_path(@account), alert: t("accounts.destroy.cannot_delete_linked")
     else
       @account.destroy_later
-      redirect_to accounts_path, notice: "Account scheduled for deletion"
+      redirect_to accounts_path, notice: t("accounts.destroy.success", type: @account.accountable_type)
+    end
+  end
+
+  def confirm_unlink
+    unless @account.linked?
+      redirect_to account_path(@account), alert: t("accounts.unlink.not_linked")
+    end
+  end
+
+  def unlink
+    unless @account.linked?
+      redirect_to account_path(@account), alert: t("accounts.unlink.not_linked")
+      return
+    end
+
+    begin
+      Account.transaction do
+        # Remove new system links (account_providers join table)
+        @account.account_providers.destroy_all
+
+        # Remove legacy system links (foreign keys)
+        @account.update!(plaid_account_id: nil, simplefin_account_id: nil)
+      end
+
+      redirect_to accounts_path, notice: t("accounts.unlink.success")
+    rescue ActiveRecord::RecordInvalid => e
+      redirect_to account_path(@account), alert: t("accounts.unlink.error", error: e.message)
+    rescue StandardError => e
+      Rails.logger.error "Failed to unlink account #{@account.id}: #{e.message}"
+      redirect_to account_path(@account), alert: t("accounts.unlink.error", error: t("accounts.unlink.generic_error"))
+    end
+  end
+
+  def select_provider
+    if @account.linked?
+      redirect_to account_path(@account), alert: t("accounts.select_provider.already_linked")
+      return
+    end
+
+    @available_providers = []
+
+    # Check SimpleFIN
+    if family.can_connect_simplefin?
+      @available_providers << {
+        name: "SimpleFIN",
+        key: "simplefin",
+        description: "Connect to your bank via SimpleFIN",
+        path: select_existing_account_simplefin_items_path(account_id: @account.id)
+      }
+    end
+
+    # Check Plaid US
+    if family.can_connect_plaid_us?
+      @available_providers << {
+        name: "Plaid",
+        key: "plaid_us",
+        description: "Connect to your US bank via Plaid",
+        path: select_existing_account_plaid_items_path(account_id: @account.id, region: "us")
+      }
+    end
+
+    # Check Plaid EU
+    if family.can_connect_plaid_eu?
+      @available_providers << {
+        name: "Plaid (EU)",
+        key: "plaid_eu",
+        description: "Connect to your EU bank via Plaid",
+        path: select_existing_account_plaid_items_path(account_id: @account.id, region: "eu")
+      }
+    end
+
+    # Check Lunch Flow
+    if family.can_connect_lunchflow?
+      @available_providers << {
+        name: "Lunch Flow",
+        key: "lunchflow",
+        description: "Connect to your bank via Lunch Flow",
+        path: select_existing_account_lunchflow_items_path(account_id: @account.id)
+      }
+    end
+
+    if @available_providers.empty?
+      redirect_to account_path(@account), alert: t("accounts.select_provider.no_providers")
     end
   end
 

app/controllers/lunchflow_items_controller.rb

@@ -98,7 +98,7 @@ class LunchflowItemsController < ApplicationController
 
     # Create or find lunchflow_item for this family
     lunchflow_item = Current.family.lunchflow_items.first_or_create!(
-      name: "Lunchflow Connection"
+      name: "Lunch Flow Connection"
     )
 
     # Fetch account details from API
@@ -279,7 +279,7 @@ class LunchflowItemsController < ApplicationController
 
     # Create or find lunchflow_item for this family
     lunchflow_item = Current.family.lunchflow_items.first_or_create!(
-      name: "Lunchflow Connection"
+      name: "Lunch Flow Connection"
     )
 
     # Fetch account details from API
@@ -338,7 +338,7 @@ class LunchflowItemsController < ApplicationController
 
   def create
     @lunchflow_item = Current.family.lunchflow_items.build(lunchflow_params)
-    @lunchflow_item.name = "Lunchflow Connection"
+    @lunchflow_item.name = "Lunch Flow Connection"
 
     if @lunchflow_item.save
       # Trigger initial sync to fetch accounts

app/controllers/plaid_items_controller.rb

@@ -48,6 +48,48 @@ class PlaidItemsController < ApplicationController
     end
   end
 
+  def select_existing_account
+    @account = Current.family.accounts.find(params[:account_id])
+    @region = params[:region] || "us"
+
+    # Get all Plaid accounts from this family's Plaid items for the specified region
+    # that are not yet linked to any account
+    @available_plaid_accounts = Current.family.plaid_items
+      .where(plaid_region: @region)
+      .includes(:plaid_accounts)
+      .flat_map(&:plaid_accounts)
+      .select { |pa| pa.account_provider.nil? && pa.account.nil? } # Not linked via new or legacy system
+
+    if @available_plaid_accounts.empty?
+      redirect_to account_path(@account), alert: "No available Plaid accounts to link. Please connect a new Plaid account first."
+    end
+  end
+
+  def link_existing_account
+    @account = Current.family.accounts.find(params[:account_id])
+    plaid_account = PlaidAccount.find(params[:plaid_account_id])
+
+    # Verify the Plaid account belongs to this family's Plaid items
+    unless Current.family.plaid_items.include?(plaid_account.plaid_item)
+      redirect_to account_path(@account), alert: "Invalid Plaid account selected"
+      return
+    end
+
+    # Verify the Plaid account is not already linked
+    if plaid_account.account_provider.present? || plaid_account.account.present?
+      redirect_to account_path(@account), alert: "This Plaid account is already linked"
+      return
+    end
+
+    # Create the link via AccountProvider
+    AccountProvider.create!(
+      account: @account,
+      provider: plaid_account
+    )
+
+    redirect_to accounts_path, notice: "Account successfully linked to Plaid"
+  end
+
   private
     def set_plaid_item
       @plaid_item = Current.family.plaid_items.find(params[:id])

app/controllers/simplefin_items_controller.rb

@@ -186,6 +186,46 @@ class SimplefinItemsController < ApplicationController
     redirect_to accounts_path, notice: t(".success")
   end
 
+  def select_existing_account
+    @account = Current.family.accounts.find(params[:account_id])
+
+    # Get all SimpleFIN accounts from this family's SimpleFIN items
+    # that are not yet linked to any account
+    @available_simplefin_accounts = Current.family.simplefin_items
+      .includes(:simplefin_accounts)
+      .flat_map(&:simplefin_accounts)
+      .select { |sa| sa.account_provider.nil? && sa.account.nil? } # Not linked via new or legacy system
+
+    if @available_simplefin_accounts.empty?
+      redirect_to account_path(@account), alert: "No available SimpleFIN accounts to link. Please connect a new SimpleFIN account first."
+    end
+  end
+
+  def link_existing_account
+    @account = Current.family.accounts.find(params[:account_id])
+    simplefin_account = SimplefinAccount.find(params[:simplefin_account_id])
+
+    # Verify the SimpleFIN account belongs to this family's SimpleFIN items
+    unless Current.family.simplefin_items.include?(simplefin_account.simplefin_item)
+      redirect_to account_path(@account), alert: "Invalid SimpleFIN account selected"
+      return
+    end
+
+    # Verify the SimpleFIN account is not already linked
+    if simplefin_account.account_provider.present? || simplefin_account.account.present?
+      redirect_to account_path(@account), alert: "This SimpleFIN account is already linked"
+      return
+    end
+
+    # Create the link via AccountProvider
+    AccountProvider.create!(
+      account: @account,
+      provider: simplefin_account
+    )
+
+    redirect_to accounts_path, notice: "Account successfully linked to SimpleFIN"
+  end
+
   private
 
     def set_simplefin_item

app/models/account.rb

@@ -22,7 +22,11 @@ class Account < ApplicationRecord
   scope :assets, -> { where(classification: "asset") }
   scope :liabilities, -> { where(classification: "liability") }
   scope :alphabetically, -> { order(:name) }
-  scope :manual, -> { left_joins(:account_providers).where(account_providers: { id: nil }) }
+  scope :manual, -> {
+    left_joins(:account_providers)
+      .where(account_providers: { id: nil })
+      .where(plaid_account_id: nil, simplefin_account_id: nil)
+  }
 
   has_one_attached :logo
 

app/models/account/linkable.rb

@@ -12,7 +12,7 @@ module Account::Linkable
 
   # A "linked" account gets transaction and balance data from a third party like Plaid or SimpleFin
   def linked?
-    account_providers.any?
+    account_providers.any? || plaid_account.present? || simplefin_account.present?
   end
 
   # An "offline" or "unlinked" account is one where the user tracks values and
@@ -43,7 +43,14 @@ module Account::Linkable
 
   # Convenience method to get the provider name
   def provider_name
-    provider&.provider_name
+    # Try new system first
+    return provider&.provider_name if provider.present?
+
+    # Fall back to legacy system
+    return "plaid" if plaid_account.present?
+    return "simplefin" if simplefin_account.present?
+
+    nil
   end
 
   # Check if account is linked to a specific provider

app/models/plaid_item.rb

@@ -16,12 +16,22 @@ class PlaidItem < ApplicationRecord
   has_one_attached :logo
 
   has_many :plaid_accounts, dependent: :destroy
-  has_many :accounts, through: :plaid_accounts
+  has_many :legacy_accounts, through: :plaid_accounts, source: :account
 
   scope :active, -> { where(scheduled_for_deletion: false) }
   scope :ordered, -> { order(created_at: :desc) }
   scope :needs_update, -> { where(status: :requires_update) }
 
+  # Get accounts from both new and legacy systems
+  def accounts
+    # Preload associations to avoid N+1 queries
+    plaid_accounts
+      .includes(:account, account_provider: :account)
+      .map(&:current_account)
+      .compact
+      .uniq
+  end
+
   def get_update_link_token(webhooks_url:, redirect_url:)
     family.get_link_token(
       webhooks_url: webhooks_url,

app/models/simplefin_item.rb

@@ -18,12 +18,22 @@ class SimplefinItem < ApplicationRecord
   has_one_attached :logo
 
   has_many :simplefin_accounts, dependent: :destroy
-  has_many :accounts, through: :simplefin_accounts
+  has_many :legacy_accounts, through: :simplefin_accounts, source: :account
 
   scope :active, -> { where(scheduled_for_deletion: false) }
   scope :ordered, -> { order(created_at: :desc) }
   scope :needs_update, -> { where(status: :requires_update) }
 
+  # Get accounts from both new and legacy systems
+  def accounts
+    # Preload associations to avoid N+1 queries
+    simplefin_accounts
+      .includes(:account, account_provider: :account)
+      .map(&:current_account)
+      .compact
+      .uniq
+  end
+
   def destroy_later
     update!(scheduled_for_deletion: true)
     DestroyJob.perform_later(self)

app/views/accounts/_account.html.erb

@@ -33,13 +33,20 @@
           <%= icon("pencil-line", size: "sm") %>
         <% end %>
 
-        <% if !account.account_providers.exists? && (account.accountable_type == "Depository" || account.accountable_type == "CreditCard") %>
-          <%= link_to select_existing_account_lunchflow_items_path(account_id: account.id, return_to: return_to),
+        <% if !account.linked? && (account.accountable_type == "Depository" || account.accountable_type == "CreditCard") %>
+          <%= link_to select_provider_account_path(account),
                       data: { turbo_frame: :modal },
                       class: "group-hover/account:flex hidden hover:opacity-80 items-center justify-center gap-1",
-                      title: t("accounts.account.link_lunchflow") do %>
+                      title: t("accounts.account.link_provider") do %>
             <%= icon("link", size: "sm") %>
           <% end %>
+        <% elsif account.linked? %>
+          <%= link_to confirm_unlink_account_path(account),
+                      data: { turbo_frame: :modal },
+                      class: "group-hover/account:flex hidden hover:opacity-80 items-center justify-center gap-1",
+                      title: t("accounts.account.unlink_provider") do %>
+            <%= icon("unlink", size: "sm") %>
+          <% end %>
         <% end %>
       <% end %>
     </div>

app/views/accounts/confirm_unlink.html.erb

@@ -0,0 +1,31 @@
+<%= render DS::Dialog.new do |dialog| %>
+  <% dialog.with_header(title: t("accounts.confirm_unlink.title")) %>
+
+  <% dialog.with_body do %>
+    <p class="text-secondary text-sm mb-4">
+      <%= t("accounts.confirm_unlink.description_html", account_name: @account.name, provider_name: @account.provider_name) %>
+    </p>
+
+    <div class="mb-4 p-3 bg-yellow-50 border border-yellow-200 rounded-lg">
+      <div class="flex items-start gap-2">
+        <%= icon "alert-triangle", class: "w-4 h-4 text-yellow-600 mt-0.5 flex-shrink-0" %>
+        <div class="text-xs">
+          <p class="font-medium text-yellow-900 mb-1"><%= t("accounts.confirm_unlink.warning_title") %></p>
+          <ul class="text-yellow-700 list-disc list-inside space-y-1">
+            <li><%= t("accounts.confirm_unlink.warning_no_sync") %></li>
+            <li><%= t("accounts.confirm_unlink.warning_manual_updates") %></li>
+            <li><%= t("accounts.confirm_unlink.warning_transactions_kept") %></li>
+            <li><%= t("accounts.confirm_unlink.warning_can_delete") %></li>
+          </ul>
+        </div>
+      </div>
+    </div>
+
+    <%= render DS::Button.new(
+      text: t("accounts.confirm_unlink.confirm_button"),
+      href: unlink_account_path(@account),
+      method: :delete,
+      full_width: true,
+      data: { turbo_frame: "_top" }) %>
+  <% end %>
+<% end %>

app/views/accounts/select_provider.html.erb

@@ -0,0 +1,23 @@
+<%= render DS::Dialog.new do |dialog| %>
+  <% dialog.with_header(title: t("accounts.select_provider.title")) %>
+
+  <% dialog.with_body do %>
+    <p class="text-secondary text-sm mb-4">
+      <%= t("accounts.select_provider.description", account_name: @account.name) %>
+    </p>
+
+    <div class="space-y-2">
+      <% @available_providers.each do |provider| %>
+        <%= link_to provider[:path], data: { turbo_frame: :modal }, class: "block p-4 border border-primary rounded-lg hover:bg-container-hover transition-colors" do %>
+          <div class="flex items-center justify-between">
+            <div>
+              <p class="font-medium text-primary"><%= provider[:name] %></p>
+              <p class="text-sm text-secondary"><%= provider[:description] %></p>
+            </div>
+            <%= icon("chevron-right", size: "sm", class: "text-secondary") %>
+          </div>
+        <% end %>
+      <% end %>
+    </div>
+  <% end %>
+<% end %>

app/views/plaid_items/select_existing_account.html.erb

@@ -0,0 +1,45 @@
+<%= turbo_frame_tag "modal" do %>
+  <%= render DS::Dialog.new do |dialog| %>
+    <% dialog.with_header(title: t(".title", account_name: @account.name)) %>
+
+    <% dialog.with_body do %>
+      <div class="space-y-4">
+        <p class="text-sm text-secondary">
+          <%= t(".description") %>
+        </p>
+
+        <form action="<%= link_existing_account_plaid_items_path %>" method="post" class="space-y-4" data-turbo-frame="_top">
+          <%= hidden_field_tag :authenticity_token, form_authenticity_token %>
+          <%= hidden_field_tag :account_id, @account.id %>
+
+          <div class="space-y-2">
+            <% @available_plaid_accounts.each do |plaid_account| %>
+              <label class="flex items-start gap-3 p-3 border border-primary rounded-lg hover:bg-subtle cursor-pointer transition-colors">
+                <%= radio_button_tag "plaid_account_id", plaid_account.id, false, class: "mt-1" %>
+                <div class="flex-1">
+                  <div class="font-medium text-sm text-primary">
+                    <%= plaid_account.name %>
+                  </div>
+                  <div class="text-xs text-secondary mt-1">
+                    <% if plaid_account.mask.present? %>
+                      ****<%= plaid_account.mask %> •
+                    <% end %>
+                    <%= plaid_account.plaid_item.name %> • <%= plaid_account.currency %> • <%= plaid_account.plaid_type %>
+                  </div>
+                </div>
+              </label>
+            <% end %>
+          </div>
+
+          <div class="flex gap-2 justify-end pt-4">
+            <%= link_to t(".cancel"), accounts_path,
+                class: "inline-flex items-center gap-1 px-3 py-2 text-sm font-medium rounded-lg text-primary button-bg-secondary hover:button-bg-secondary-hover",
+                data: { turbo_frame: "_top", action: "DS--dialog#close" } %>
+            <%= submit_tag t(".link_account"),
+                class: "inline-flex items-center gap-1 px-3 py-2 text-sm font-medium rounded-lg text-inverse bg-inverse hover:bg-inverse-hover disabled:button-bg-disabled" %>
+          </div>
+        </form>
+      </div>
+    <% end %>
+  <% end %>
+<% end %>

app/views/simplefin_items/select_existing_account.html.erb

@@ -0,0 +1,45 @@
+<%= turbo_frame_tag "modal" do %>
+  <%= render DS::Dialog.new do |dialog| %>
+    <% dialog.with_header(title: t(".title", account_name: @account.name)) %>
+
+    <% dialog.with_body do %>
+      <div class="space-y-4">
+        <p class="text-sm text-secondary">
+          <%= t(".description") %>
+        </p>
+
+        <form action="<%= link_existing_account_simplefin_items_path %>" method="post" class="space-y-4" data-turbo-frame="_top">
+          <%= hidden_field_tag :authenticity_token, form_authenticity_token %>
+          <%= hidden_field_tag :account_id, @account.id %>
+
+          <div class="space-y-2">
+            <% @available_simplefin_accounts.each do |simplefin_account| %>
+              <label class="flex items-start gap-3 p-3 border border-primary rounded-lg hover:bg-subtle cursor-pointer transition-colors">
+                <%= radio_button_tag "simplefin_account_id", simplefin_account.id, false, class: "mt-1" %>
+                <div class="flex-1">
+                  <div class="font-medium text-sm text-primary">
+                    <%= simplefin_account.name %>
+                  </div>
+                  <div class="text-xs text-secondary mt-1">
+                    <% if simplefin_account.org_data&.dig("name").present? %>
+                      <%= simplefin_account.org_data["name"] %> •
+                    <% end %>
+                    <%= simplefin_account.simplefin_item.name %> • <%= simplefin_account.currency %>
+                  </div>
+                </div>
+              </label>
+            <% end %>
+          </div>
+
+          <div class="flex gap-2 justify-end pt-4">
+            <%= link_to t(".cancel"), accounts_path,
+                class: "inline-flex items-center gap-1 px-3 py-2 text-sm font-medium rounded-lg text-primary button-bg-secondary hover:button-bg-secondary-hover",
+                data: { turbo_frame: "_top", action: "DS--dialog#close" } %>
+            <%= submit_tag t(".link_account"),
+                class: "inline-flex items-center gap-1 px-3 py-2 text-sm font-medium rounded-lg text-inverse bg-inverse hover:bg-inverse-hover disabled:button-bg-disabled" %>
+          </div>
+        </form>
+      </div>
+    <% end %>
+  <% end %>
+<% end %>

config/locales/views/accounts/en.yml

@@ -3,6 +3,8 @@ en:
   accounts:
     account:
       link_lunchflow: Link with Lunch Flow
+      link_provider: Link with provider
+      unlink_provider: Unlink from provider
       troubleshoot: Troubleshoot
     chart:
       data_not_available: Data not available for the selected period
@@ -10,6 +12,7 @@ en:
       success: "%{type} account created"
     destroy:
       success: "%{type} account scheduled for deletion"
+      cannot_delete_linked: "Cannot delete a linked account. Please unlink it first."
     empty:
       empty_message: Add an account either via connection, importing or entering manually.
       new_account: New account
@@ -24,6 +27,8 @@ en:
         other_accounts: Other accounts
       new_account: New account
       sync: Sync all
+    sync_all:
+      syncing: "Syncing accounts..."
     new:
       import_accounts: Import accounts
       method_selector:
@@ -85,6 +90,25 @@ en:
       credit_card: Credit Card
       loan: Loan
       other_liability: Other Liability
+    confirm_unlink:
+      title: Unlink account from provider?
+      description_html: "You are about to unlink <strong>%{account_name}</strong> from <strong>%{provider_name}</strong>. This will convert it to a manual account."
+      warning_title: What this means
+      warning_no_sync: The account will no longer sync automatically with your bank
+      warning_manual_updates: You will need to add transactions and update balances manually
+      warning_transactions_kept: All existing transactions and balances will be preserved
+      warning_can_delete: After unlinking, you will be able to delete the account if needed
+      confirm_button: Confirm and unlink
+    unlink:
+      success: "Account unlinked successfully. It is now a manual account."
+      not_linked: "Account is not linked to a provider"
+      error: "Failed to unlink account: %{error}"
+      generic_error: "An unexpected error occurred. Please try again."
+    select_provider:
+      title: Select a provider to link
+      description: "Choose which provider you want to use to link %{account_name}"
+      already_linked: "Account is already linked to a provider"
+      no_providers: "No providers are currently configured"
 
   email_confirmations:
     new:

config/locales/views/plaid_items/en.yml

@@ -24,3 +24,8 @@ en:
       status_never: Requires data sync
       syncing: Syncing...
       update: Update connection
+    select_existing_account:
+      title: "Link %{account_name} to Plaid"
+      description: Select a Plaid account to link to your existing account
+      cancel: Cancel
+      link_account: Link account

config/locales/views/simplefin_items/en.yml

@@ -44,4 +44,9 @@ en:
       status_never: Never synced
       status_with_summary: "Last synced %{timestamp} ago • %{summary}"
       syncing: Syncing...
-      update: Update connection
+      update: Update connection
+    select_existing_account:
+      title: "Link %{account_name} to SimpleFIN"
+      description: Select a SimpleFIN account to link to your existing account
+      cancel: Cancel
+      link_account: Link account

config/routes.rb

@@ -196,6 +196,9 @@ Rails.application.routes.draw do
       post :sync
       get :sparkline
       patch :toggle_active
+      get :select_provider
+      get :confirm_unlink
+      delete :unlink
     end
 
     collection do
@@ -281,12 +284,22 @@ Rails.application.routes.draw do
   end
 
   resources :plaid_items, only: %i[new edit create destroy] do
+    collection do
+      get :select_existing_account
+      post :link_existing_account
+    end
+
     member do
       post :sync
     end
   end
 
   resources :simplefin_items, only: %i[index new create show edit update destroy] do
+    collection do
+      get :select_existing_account
+      post :link_existing_account
+    end
+
     member do
       post :sync
       get :setup_accounts

test/controllers/accounts_controller_test.rb

@@ -30,7 +30,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     delete account_url(@account)
     assert_redirected_to accounts_path
     assert_enqueued_with job: DestroyJob
-    assert_equal "Account scheduled for deletion", flash[:notice]
+    assert_equal "Depository account scheduled for deletion", flash[:notice]
   end
 
   test "syncing linked account triggers sync for all provider items" do
@@ -57,4 +57,90 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     post sync_account_url(@account)
     assert_redirected_to account_url(@account)
   end
+
+  test "confirms unlink for linked account" do
+    plaid_account = plaid_accounts(:one)
+    AccountProvider.create!(account: @account, provider: plaid_account)
+
+    get confirm_unlink_account_url(@account)
+    assert_response :success
+  end
+
+  test "redirects when confirming unlink for unlinked account" do
+    get confirm_unlink_account_url(@account)
+    assert_redirected_to account_url(@account)
+    assert_equal "Account is not linked to a provider", flash[:alert]
+  end
+
+  test "unlinks linked account successfully with new system" do
+    plaid_account = plaid_accounts(:one)
+    AccountProvider.create!(account: @account, provider: plaid_account)
+    @account.reload
+
+    assert @account.linked?
+
+    delete unlink_account_url(@account)
+    @account.reload
+
+    assert_not @account.linked?
+    assert_redirected_to accounts_path
+    assert_equal "Account unlinked successfully. It is now a manual account.", flash[:notice]
+  end
+
+  test "unlinks linked account successfully with legacy system" do
+    plaid_account = plaid_accounts(:one)
+    @account.update!(plaid_account_id: plaid_account.id)
+    @account.reload
+
+    assert @account.linked?
+
+    delete unlink_account_url(@account)
+    @account.reload
+
+    assert_not @account.linked?
+    assert_nil @account.plaid_account_id
+    assert_redirected_to accounts_path
+    assert_equal "Account unlinked successfully. It is now a manual account.", flash[:notice]
+  end
+
+  test "redirects when unlinking unlinked account" do
+    delete unlink_account_url(@account)
+    assert_redirected_to account_url(@account)
+    assert_equal "Account is not linked to a provider", flash[:alert]
+  end
+
+  test "unlinked account can be deleted" do
+    plaid_account = plaid_accounts(:one)
+    AccountProvider.create!(account: @account, provider: plaid_account)
+    @account.reload
+
+    # Cannot delete while linked
+    delete account_url(@account)
+    assert_redirected_to account_url(@account)
+    assert_equal "Cannot delete a linked account. Please unlink it first.", flash[:alert]
+
+    # Unlink the account
+    delete unlink_account_url(@account)
+    @account.reload
+
+    # Now can delete
+    delete account_url(@account)
+    assert_redirected_to accounts_path
+    assert_enqueued_with job: DestroyJob
+    assert_equal "Depository account scheduled for deletion", flash[:notice]
+  end
+
+  test "select_provider shows available providers" do
+    get select_provider_account_url(@account)
+    assert_response :success
+  end
+
+  test "select_provider redirects for already linked account" do
+    plaid_account = plaid_accounts(:one)
+    AccountProvider.create!(account: @account, provider: plaid_account)
+
+    get select_provider_account_url(@account)
+    assert_redirected_to account_url(@account)
+    assert_equal "Account is already linked to a provider", flash[:alert]
+  end
 end

test/controllers/plaid_items_controller_test.rb

@@ -46,4 +46,45 @@ class PlaidItemsControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to accounts_path
   end
+
+  test "select_existing_account redirects when no available plaid accounts" do
+    account = accounts(:depository)
+
+    get select_existing_account_plaid_items_url(account_id: account.id, region: "us")
+    assert_redirected_to account_path(account)
+    assert_equal "No available Plaid accounts to link. Please connect a new Plaid account first.", flash[:alert]
+  end
+
+  test "link_existing_account links plaid account to existing account" do
+    account = accounts(:depository)
+
+    # Create a new unlinked plaid_account for testing
+    plaid_account = PlaidAccount.create!(
+      plaid_item: plaid_items(:one),
+      name: "Test Plaid Account",
+      plaid_id: "test_acc_123",
+      plaid_type: "depository",
+      plaid_subtype: "checking",
+      currency: "USD",
+      current_balance: 1000,
+      available_balance: 1000
+    )
+
+    assert_not account.linked?
+    assert_nil plaid_account.account
+    assert_nil plaid_account.account_provider
+
+    assert_difference "AccountProvider.count", 1 do
+      post link_existing_account_plaid_items_url, params: {
+        account_id: account.id,
+        plaid_account_id: plaid_account.id
+      }
+    end
+
+    account.reload
+    assert account.linked?, "Account should be linked after creating AccountProvider"
+    assert_equal 1, account.account_providers.count
+    assert_redirected_to accounts_path
+    assert_equal "Account successfully linked to Plaid", flash[:notice]
+  end
 end

test/controllers/simplefin_items_controller_test.rb

@@ -235,4 +235,12 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest
     @simplefin_item.reload
     assert @simplefin_item.scheduled_for_deletion?
   end
+
+  test "select_existing_account redirects when no available simplefin accounts" do
+    account = accounts(:depository)
+
+    get select_existing_account_simplefin_items_url(account_id: account.id)
+    assert_redirected_to account_path(account)
+    assert_equal "No available SimpleFIN accounts to link. Please connect a new SimpleFIN account first.", flash[:alert]
+  end
 end