diff --git a/app/controllers/enable_banking_items_controller.rb b/app/controllers/enable_banking_items_controller.rb index 7ad9cd31e..ae69543e7 100644 --- a/app/controllers/enable_banking_items_controller.rb +++ b/app/controllers/enable_banking_items_controller.rb @@ -430,7 +430,7 @@ class EnableBankingItemsController < ApplicationController # Guard: only manual accounts can be linked (no existing provider links or legacy IDs) if @account.account_providers.any? || @account.plaid_account_id.present? || @account.simplefin_account_id.present? - flash[:alert] = "Only manual accounts can be linked" + flash[:alert] = t("enable_banking_items.link_existing_account.errors.only_manual") if turbo_frame_request? return render turbo_stream: Array(flash_notification_stream_items) else @@ -441,7 +441,7 @@ class EnableBankingItemsController < ApplicationController # Verify the Enable Banking account belongs to this family's Enable Banking items unless enable_banking_account.enable_banking_item.present? && Current.family.enable_banking_items.include?(enable_banking_account.enable_banking_item) - flash[:alert] = "Invalid Enable Banking account selected" + flash[:alert] = t("enable_banking_items.link_existing_account.errors.invalid_enable_banking_account") if turbo_frame_request? render turbo_stream: Array(flash_notification_stream_items) else @@ -466,9 +466,12 @@ class EnableBankingItemsController < ApplicationController # follows the chosen account. if previous_account && previous_account.id != @account.id && previous_account.family_id == @account.family_id begin - previous_account.disable! + # Disabled accounts still appear (greyed-out) in the manual list after a full refresh. + # Use the app's standard deletion path (async) so the duplicate disappears and the + # "pending_deletion" state remains truthful in the UI. + previous_account.destroy_later if previous_account.may_mark_for_deletion? rescue => e - Rails.logger.warn("Failed to disable orphaned account #{previous_account.id}: #{e.class} - #{e.message}") + Rails.logger.warn("Failed to cleanup orphaned account #{previous_account.id}: #{e.class} - #{e.message}") end end end @@ -489,7 +492,7 @@ class EnableBankingItemsController < ApplicationController @enable_banking_items = Current.family.enable_banking_items.ordered.includes(:syncs) build_enable_banking_maps_for(@enable_banking_items) - flash[:notice] = "Account successfully linked to Enable Banking" + flash[:notice] = t("enable_banking_items.link_existing_account.success") @account.reload manual_accounts_stream = if @manual_accounts.any? turbo_stream.update( @@ -513,7 +516,7 @@ class EnableBankingItemsController < ApplicationController turbo_stream.replace("modal", view_context.turbo_frame_tag("modal")) ] + Array(flash_notification_stream_items) else - redirect_to accounts_path(cache_bust: SecureRandom.hex(6)), notice: "Account successfully linked to Enable Banking", status: :see_other + redirect_to accounts_path(cache_bust: SecureRandom.hex(6)), notice: t("enable_banking_items.link_existing_account.success"), status: :see_other end end diff --git a/app/controllers/simplefin_items_controller.rb b/app/controllers/simplefin_items_controller.rb index 858771d8f..03a5322de 100644 --- a/app/controllers/simplefin_items_controller.rb +++ b/app/controllers/simplefin_items_controller.rb @@ -323,12 +323,20 @@ class SimplefinItemsController < ApplicationController def select_existing_account @account = Current.family.accounts.find(params[:account_id]) - # Filter out SimpleFIN accounts that are already linked to any account - # (either via account_provider or legacy account association) + # Allow explicit relinking by listing all available SimpleFIN accounts for the family. + # The UI will surface the current mapping (if any), and the action will move the link. @available_simplefin_accounts = Current.family.simplefin_items - .includes(:simplefin_accounts) + .includes(simplefin_accounts: [ :account, { account_provider: :account } ]) .flat_map(&:simplefin_accounts) - .reject { |sfa| sfa.account_provider.present? || sfa.account.present? } + # After provider setup, SFAs may already have an AccountProvider (linked to the freshly + # created duplicate accounts). During relink, we need to show those SFAs until the legacy + # link (`Account.simplefin_account_id`) has been cleared. + # + # Eligibility rule: + # - Show SFAs that are still legacy-linked (`sfa.account.present?`) => candidates to move. + # - Show SFAs that are fully unlinked (no legacy account and no account_provider) => candidates to link. + # - Hide SFAs that are linked via AccountProvider but no longer legacy-linked => already relinked. + .select { |sfa| sfa.account.present? || sfa.account_provider.nil? } .sort_by { |sfa| sfa.updated_at || sfa.created_at } .reverse @@ -342,7 +350,7 @@ class SimplefinItemsController < ApplicationController # Guard: only manual accounts can be linked (no existing provider links or legacy IDs) if @account.account_providers.any? || @account.plaid_account_id.present? || @account.simplefin_account_id.present? - flash[:alert] = "Only manual accounts can be linked" + flash[:alert] = t("simplefin_items.link_existing_account.errors.only_manual") if turbo_frame_request? return render turbo_stream: Array(flash_notification_stream_items) else @@ -352,7 +360,7 @@ class SimplefinItemsController < ApplicationController # Verify the SimpleFIN account belongs to this family's SimpleFIN items unless Current.family.simplefin_items.include?(simplefin_account.simplefin_item) - flash[:alert] = "Invalid SimpleFIN account selected" + flash[:alert] = t("simplefin_items.link_existing_account.errors.invalid_simplefin_account") if turbo_frame_request? render turbo_stream: Array(flash_notification_stream_items) else @@ -364,9 +372,10 @@ class SimplefinItemsController < ApplicationController # Relink behavior: detach any legacy link and point provider link at the chosen account Account.transaction do simplefin_account.lock! - # Clear legacy association if present - if simplefin_account.account_id.present? - simplefin_account.update!(account_id: nil) + + # Clear legacy association if present (Account.simplefin_account_id) + if (legacy_account = simplefin_account.account) + legacy_account.update!(simplefin_account_id: nil) end # Upsert the AccountProvider mapping deterministically @@ -382,9 +391,13 @@ class SimplefinItemsController < ApplicationController if previous_account && previous_account.id != @account.id && previous_account.family_id == @account.family_id begin previous_account.reload - # Only disable if the previous account is truly orphaned (no other provider links) + # Only hide if the previous account is truly orphaned (no other provider links) if previous_account.account_providers.none? - previous_account.disable! + # Disabled accounts still appear (greyed-out) in the manual list; for relink + # consolidation we want the duplicate to disappear from the UI. + # Use the app's standard deletion path (async) so the "pending_deletion" state + # remains truthful in the UI. + previous_account.destroy_later if previous_account.may_mark_for_deletion? else Rails.logger.info("Skipped disabling account ##{previous_account.id} after relink because it still has active provider links") end @@ -410,7 +423,7 @@ class SimplefinItemsController < ApplicationController @simplefin_items = Current.family.simplefin_items.ordered.includes(:syncs) build_simplefin_maps_for(@simplefin_items) - flash[:notice] = "Account successfully linked to SimpleFIN" + flash[:notice] = t("simplefin_items.link_existing_account.success") @account.reload manual_accounts_stream = if @manual_accounts.any? turbo_stream.update( @@ -434,7 +447,7 @@ class SimplefinItemsController < ApplicationController turbo_stream.replace("modal", view_context.turbo_frame_tag("modal")) ] + Array(flash_notification_stream_items) else - redirect_to accounts_path(cache_bust: SecureRandom.hex(6)), notice: "Account successfully linked to SimpleFIN", status: :see_other + redirect_to accounts_path(cache_bust: SecureRandom.hex(6)), notice: t("simplefin_items.link_existing_account.success"), status: :see_other end end diff --git a/app/models/provider/lunchflow_adapter.rb b/app/models/provider/lunchflow_adapter.rb index ec5b3ef92..02623da59 100644 --- a/app/models/provider/lunchflow_adapter.rb +++ b/app/models/provider/lunchflow_adapter.rb @@ -7,7 +7,7 @@ class Provider::LunchflowAdapter < Provider::Base # Define which account types this provider supports def self.supported_account_types - %w[Depository CreditCard Loan] + %w[Depository CreditCard Loan Investment] end # Returns connection configurations for this provider diff --git a/app/models/simplefin_item/unlinking.rb b/app/models/simplefin_item/unlinking.rb index a4113cb36..fb815753a 100644 --- a/app/models/simplefin_item/unlinking.rb +++ b/app/models/simplefin_item/unlinking.rb @@ -39,8 +39,10 @@ module SimplefinItem::Unlinking end # Legacy FK fallback: ensure any legacy link is cleared - if sfa.account_id.present? - sfa.update!(account: nil) + # NOTE: `SimplefinAccount#account_id` is the provider's external identifier. + # The legacy link is `Account.simplefin_account_id` (accessible via `sfa.account`). + if sfa.account.present? + sfa.account.update!(simplefin_account_id: nil) end end rescue => e diff --git a/app/views/simplefin_items/_simplefin_item.html.erb b/app/views/simplefin_items/_simplefin_item.html.erb index f8c916f61..680a89d04 100644 --- a/app/views/simplefin_items/_simplefin_item.html.erb +++ b/app/views/simplefin_items/_simplefin_item.html.erb @@ -147,8 +147,15 @@ <% end %> - <%# Sync summary (collapsible) %> - <% stats = (@simplefin_sync_stats_map || {})[simplefin_item.id] || {} %> + <%# Sync summary (collapsible) + Prefer controller-provided map; fallback to latest sync stats so Turbo broadcasts + can render the summary without requiring a full page refresh. %> + <% stats = if defined?(@simplefin_sync_stats_map) && @simplefin_sync_stats_map + @simplefin_sync_stats_map[simplefin_item.id] || {} + else + # `latest_sync` is private on Syncable; access via association for broadcast renders. + simplefin_item.syncs.ordered.first&.sync_stats || {} + end %> <% if stats.present? %>
diff --git a/app/views/simplefin_items/select_existing_account.html.erb b/app/views/simplefin_items/select_existing_account.html.erb index 76cecef96..41db812dd 100644 --- a/app/views/simplefin_items/select_existing_account.html.erb +++ b/app/views/simplefin_items/select_existing_account.html.erb @@ -6,10 +6,10 @@ <% dialog.with_body do %> <% if @available_simplefin_accounts.blank? %>
-

All SimpleFIN accounts appear to be linked already.

+

<%= t("simplefin_items.select_existing_account.no_accounts_found") %>

    -
  • If you just connected or synced, try again after the sync completes.
    • -
  • To link a different account, first unlink it from the account’s actions menu.
    • +
  • <%= t("simplefin_items.select_existing_account.wait_for_sync") %>
    • +
  • <%= t("simplefin_items.select_existing_account.check_provider_health") %>
<% else %> @@ -24,6 +24,9 @@ <%= sfa.currency %> • Balance: <%= number_to_currency((sfa.current_balance || sfa.available_balance || 0), unit: sfa.currency) %> + <% if sfa.current_account.present? %> + <%= t("simplefin_items.select_existing_account.currently_linked_to", account_name: sfa.current_account.name) %> + <% end %> <% end %> diff --git a/config/locales/views/enable_banking_items/en.yml b/config/locales/views/enable_banking_items/en.yml new file mode 100644 index 000000000..c52867096 --- /dev/null +++ b/config/locales/views/enable_banking_items/en.yml @@ -0,0 +1,8 @@ +--- +en: + enable_banking_items: + link_existing_account: + success: Account successfully linked to Enable Banking + errors: + only_manual: Only manual accounts can be linked + invalid_enable_banking_account: Invalid Enable Banking account selected diff --git a/config/locales/views/simplefin_items/en.yml b/config/locales/views/simplefin_items/en.yml index af71acae6..5ff871805 100644 --- a/config/locales/views/simplefin_items/en.yml +++ b/config/locales/views/simplefin_items/en.yml @@ -57,4 +57,15 @@ en: title: "Link %{account_name} to SimpleFIN" description: Select a SimpleFIN account to link to your existing account cancel: Cancel - link_account: Link account \ No newline at end of file + link_account: Link account + no_accounts_found: No SimpleFIN accounts found for this family. + wait_for_sync: If you just connected or synced, try again after the sync completes. + unlink_to_move: To move a link, first unlink it from the account’s actions menu. + all_accounts_already_linked: All SimpleFIN accounts appear to be linked already. + currently_linked_to: "Currently linked to: %{account_name}" + + link_existing_account: + success: Account successfully linked to SimpleFIN + errors: + only_manual: Only manual accounts can be linked + invalid_simplefin_account: Invalid SimpleFIN account selected \ No newline at end of file diff --git a/config/locales/views/simplefin_items/pt-BR.yml b/config/locales/views/simplefin_items/pt-BR.yml index 340449e6a..2804b4c74 100644 --- a/config/locales/views/simplefin_items/pt-BR.yml +++ b/config/locales/views/simplefin_items/pt-BR.yml @@ -58,3 +58,7 @@ pt-BR: description: Selecione uma conta do SimpleFIN para vincular à sua conta existente cancel: Cancelar link_account: Vincular conta + no_accounts_found: Nenhuma conta SimpleFIN encontrada para esta família. + wait_for_sync: Se você acabou de conectar ou sincronizar, tente novamente após a conclusão da sincronização. + check_provider_health: Verifique se sua conexão SimpleFIN está ativa em Configurações → Provedores. + currently_linked_to: "Atualmente vinculada a: %{account_name}" diff --git a/test/controllers/simplefin_items_controller_test.rb b/test/controllers/simplefin_items_controller_test.rb index e4fd74b36..43a100d88 100644 --- a/test/controllers/simplefin_items_controller_test.rb +++ b/test/controllers/simplefin_items_controller_test.rb @@ -95,16 +95,56 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest assert_response :see_other - # Reload and assert: account A should still be enabled (not disabled) because it has another provider link + # Reload and assert: account A should not be hidden because it has another provider link account_a.reload assert account_a.account_providers.any?, "expected previous account to still have provider links" - refute account_a.disabled?, "previous account should not be disabled when still linked to other providers" + refute_equal "pending_deletion", account_a.status, "previous account should not be hidden when still linked to other providers" # And the AccountProvider for sfa_primary should now point to account B ap = AccountProvider.find_by(provider: sfa_primary) assert_equal account_b.id, ap.account_id end + test "relink hides a previously linked orphaned duplicate account" do + account_a = Account.create!( + family: @family, + name: "Duplicate A", + balance: 0, + currency: "USD", + accountable_type: "Depository", + accountable: Depository.create!(subtype: "checking") + ) + + account_b = Account.create!( + family: @family, + name: "Target B", + balance: 0, + currency: "USD", + accountable_type: "Depository", + accountable: Depository.create!(subtype: "savings") + ) + + sfa = @simplefin_item.simplefin_accounts.create!( + name: "SF For Duplicate", + account_id: "sf_dup_1", + account_type: "depository", + currency: "USD", + current_balance: 0 + ) + + AccountProvider.create!(account: account_a, provider: sfa) + + post link_existing_account_simplefin_items_path, params: { + account_id: account_b.id, + simplefin_account_id: sfa.id + } + + assert_response :see_other + + account_a.reload + assert_equal "pending_deletion", account_a.status, "expected orphaned duplicate to be hidden after relink" + end + test "should get edit" do @simplefin_item.update!(status: :requires_update) get edit_simplefin_item_url(@simplefin_item) @@ -301,12 +341,70 @@ class SimplefinItemsControllerTest < ActionDispatch::IntegrationTest assert @simplefin_item.scheduled_for_deletion? end - test "select_existing_account renders empty-state modal when no available simplefin accounts" do + test "select_existing_account renders empty-state modal when no simplefin accounts exist" do account = accounts(:depository) get select_existing_account_simplefin_items_url(account_id: account.id) assert_response :success - assert_includes @response.body, "All SimpleFIN accounts appear to be linked already." + assert_includes @response.body, "No SimpleFIN accounts found for this family." + end + + test "select_existing_account lists simplefin accounts even when they are already linked" do + account = accounts(:depository) + + sfa = @simplefin_item.simplefin_accounts.create!( + name: "Linked SF", + account_id: "sf_linked_123", + currency: "USD", + current_balance: 10, + account_type: "depository" + ) + + linked_account = Account.create!( + family: @family, + name: "Existing Linked Account", + currency: "USD", + balance: 0, + accountable_type: "Depository", + accountable: Depository.create!(subtype: "checking") + ) + # Model the pre-relink state: the provider account is linked to a newly set up duplicate + # via the legacy FK, and may also have an AccountProvider. + linked_account.update!(simplefin_account_id: sfa.id) + sfa.update!(account: linked_account) + AccountProvider.create!(account: linked_account, provider: sfa) + + get select_existing_account_simplefin_items_url(account_id: account.id) + assert_response :success + assert_includes @response.body, "Linked SF" + assert_includes @response.body, "Currently linked to: Existing Linked Account" + end + + test "select_existing_account hides simplefin accounts after they have been relinked" do + account = accounts(:depository) + + sfa = @simplefin_item.simplefin_accounts.create!( + name: "Relinked SF", + account_id: "sf_relinked_123", + currency: "USD", + current_balance: 10, + account_type: "depository" + ) + + # Simulate post-relink state: legacy link cleared, AccountProvider exists. + linked_account = Account.create!( + family: @family, + name: "Final Linked Account", + currency: "USD", + balance: 0, + accountable_type: "Depository", + accountable: Depository.create!(subtype: "checking") + ) + AccountProvider.create!(account: linked_account, provider: sfa) + + get select_existing_account_simplefin_items_url(account_id: account.id) + assert_response :success + refute_includes @response.body, "Relinked SF" end test "destroy should unlink provider links and legacy fk" do # Create SFA and linked Account with AccountProvider diff --git a/test/models/provider/lunchflow_adapter_test.rb b/test/models/provider/lunchflow_adapter_test.rb new file mode 100644 index 000000000..79047d58f --- /dev/null +++ b/test/models/provider/lunchflow_adapter_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class Provider::LunchflowAdapterTest < ActiveSupport::TestCase + test "supports Investment accounts" do + assert_includes Provider::LunchflowAdapter.supported_account_types, "Investment" + end +end