Allow CSV backfill into already-linked accounts

app/controllers/import/mappings_controller.rb

@@ -24,7 +24,11 @@ def set_import
     def mappable
       return nil unless mappable_class.present?
 
-      @mappable ||= mappable_class.find_by(id: mapping_params[:mappable_id], family: Current.family)
+      if mappable_class == Account
+        Import::AccountMapping.importable_accounts(@import).find_by(id: mapping_params[:mappable_id])
+      else
+        mappable_class.find_by(id: mapping_params[:mappable_id], family: Current.family)
+      end
     end
 
     def create_when_empty

app/controllers/import/uploads_controller.rb

@@ -2,6 +2,7 @@ class Import::UploadsController < ApplicationController
   layout "imports"
 
   before_action :set_import
+  before_action :set_account_options, only: %i[show update]
 
   def show
   end
@@ -19,7 +20,7 @@ def update
     elsif @import.is_a?(SureImport)
       update_sure_import_upload
     elsif csv_valid?(csv_str)
-      @import.account = import_account_id.present? ? accessible_accounts.find(import_account_id) : nil
+      @import.account = import_account_id.present? ? Import::AccountMapping.importable_accounts(@import).find(import_account_id) : nil
       @import.assign_attributes(raw_file_str: csv_str, col_sep: upload_params[:col_sep])
       @import.save!(validate: false)
 
@@ -66,6 +67,12 @@ def set_import
       @import = Current.family.imports.find(params[:import_id])
     end
 
+    def set_account_options
+      writable_accounts = Current.family.accounts.writable_by(Current.user).visible.alphabetically
+      @qif_account_options = writable_accounts.pluck(:name, :id)
+      @csv_account_options = Import::AccountMapping.importable_accounts(@import).visible.alphabetically.pluck(:name, :id)
+    end
+
     def handle_qif_upload
       unless QifParser.valid?(csv_str)
         flash.now[:alert] = "Must be a valid QIF file"
@@ -78,7 +85,7 @@ def handle_qif_upload
       end
 
       ActiveRecord::Base.transaction do
-        @import.account = accessible_accounts.find(import_account_id)
+        @import.account = Current.family.accounts.writable_by(Current.user).find(import_account_id)
         @import.raw_file_str = QifParser.normalize_encoding(csv_str)
         @import.save!(validate: false)
         @import.generate_rows_from_csv

app/controllers/imports_controller.rb

@@ -9,7 +9,7 @@ def update
     account_id = params.dig(:pdf_import, :account_id) || params.dig(:import, :account_id)
 
     if account_id.present?
-      account = accessible_accounts.find_by(id: account_id)
+      account = Current.family.accounts.writable_by(Current.user).find_by(id: account_id)
       unless account
         redirect_back_or_to import_path(@import), alert: t("imports.update.invalid_account", default: "Account not found.")
         return
@@ -98,7 +98,11 @@ def create
     type = params.dig(:import, :type).to_s
     type = "TransactionImport" unless Import::TYPES.include?(type)
 
-    account = accessible_accounts.find_by(id: params.dig(:import, :account_id))
+    account = Import::AccountMapping.account_scope(
+      family: Current.family,
+      user: Current.user,
+      allow_linked: type == "TransactionImport"
+    ).find_by(id: params.dig(:import, :account_id))
     import = Current.family.imports.create!(
       type: type,
       account: account,

app/models/import/account_mapping.rb

@@ -4,14 +4,38 @@ class Import::AccountMapping < Import::Mapping
   class << self
     def mappables_by_key(import)
       unique_values = import.rows.map(&:account).uniq
-      accounts = import.family.accounts.where(name: unique_values).index_by(&:name)
+      accounts = importable_accounts(import).where(name: unique_values).index_by(&:name)
 
       unique_values.index_with { |value| accounts[value] }
     end
+
+    # Writable accounts the current user may target for this import.
+    # Linked (provider-managed) accounts are only offered for import types that
+    # reconcile against provider-synced rows (TransactionImport). TradeImport
+    # inserts unconditionally and would duplicate Plaid/Questrade trades.
+    def importable_accounts(import)
+      account_scope(
+        family: import.family,
+        user: Current.user,
+        allow_linked: allows_linked_account_targets?(import)
+      )
+    end
+
+    def account_scope(family:, user: nil, allow_linked: false)
+      scope = family.accounts
+      scope = scope.writable_by(user) if user
+      return scope if allow_linked
+
+      scope.where(id: family.accounts.manual.select(:id))
+    end
+
+    def allows_linked_account_targets?(import)
+      import.is_a?(TransactionImport)
+    end
   end
 
   def selectable_values
-    family_accounts = import.family.accounts.manual.alphabetically.map { |account| [ account.name, account.id ] }
+    family_accounts = self.class.importable_accounts(import).visible.alphabetically.map { |account| [ account.name, account.id ] }
 
     unless key.blank?
       family_accounts.unshift [ "Add as new account", CREATE_NEW_KEY ]

app/models/transaction_import.rb

@@ -33,24 +33,37 @@ def import!
         # Check for duplicate transactions using the adapter's deduplication logic
         # Pass claimed_entry_ids to exclude entries we've already matched in this import
         # This ensures identical rows within the CSV are all imported as separate transactions
+        #
+        # Only rows carrying a real CSV name may claim a provider-synced entry.
+        # A blank name cell falls back to default_row_name, so row.name is never
+        # actually blank -- gating on presence alone would always be true and let
+        # a placeholder row match a provider entry on date+amount+placeholder.
+        # Named rows still skip already-synced overlap (Enable Banking's ~90-day
+        # window); placeholder rows only reconcile against manual/CSV entries.
         adapter = Account::ProviderImportAdapter.new(mapped_account)
         duplicate_entry = adapter.find_duplicate_transaction(
           date: row.date_iso,
           amount: row.signed_amount,
           currency: effective_currency,
           name: row.name,
-          exclude_entry_ids: claimed_entry_ids
+          exclude_entry_ids: claimed_entry_ids,
+          include_provider_entries: csv_provided_name?(row)
         )
 
         if duplicate_entry
-          # Update existing transaction instead of creating a new one
+          claimed_entry_ids.add(duplicate_entry.id)
+
+          # Already synced from a provider — skip creating a CSV duplicate and
+          # do not mark the provider-owned row import_locked.
+          next if duplicate_entry.external_id.present?
+
+          # Update existing manual/CSV transaction instead of creating a new one
           duplicate_entry.transaction.category = category if category.present?
           duplicate_entry.transaction.tags = tags if tags.any?
           duplicate_entry.notes = row.notes if row.notes.present?
           duplicate_entry.import = self
           duplicate_entry.import_locked = true  # Protect from provider sync overwrites
           updated_entries << duplicate_entry
-          claimed_entry_ids.add(duplicate_entry.id)
         else
           # Create new transaction (no duplicate found)
           # Mark as import_locked to protect from provider sync overwrites
@@ -116,4 +129,30 @@ def csv_template
     csv.delete("account") if account.present?
     csv
   end
+
+  private
+    # True when the row's name came from the CSV rather than the
+    # default_row_name placeholder substituted for a blank cell. A row whose
+    # name cell literally holds the placeholder text still counts as provided,
+    # so it reconciles against provider-synced history like any other named row.
+    def csv_provided_name?(row)
+      return false if row.name.blank?
+      return true unless row.name == default_row_name
+
+      csv_name_cells[row.source_row_number].present?
+    end
+
+    # Maps source_row_number (1-based, assigned in Import#generate_rows_from_csv)
+    # to the raw name cell, so a supplied placeholder is distinguishable from a
+    # blank one. Empty without a CSV behind the import, which keeps the
+    # conservative "not provided" answer.
+    def csv_name_cells
+      @csv_name_cells ||= if raw_file_str.blank?
+        {}
+      else
+        csv_rows.each_with_index.to_h do |csv_row, index|
+          [ index + 1, csv_value(csv_row, name_col_label, "name") ]
+        end
+      end
+    end
 end

app/views/import/uploads/show.html.erb

@@ -56,7 +56,7 @@
 
     <%= styled_form_with model: @import, scope: :import, url: import_upload_path(@import), multipart: true, class: "space-y-4" do |form| %>
       <%= form.select :account_id,
-            Current.user.accessible_accounts.visible.alphabetically.pluck(:name, :id),
+            @qif_account_options,
             { label: t(".qif_account_label"), include_blank: t(".qif_account_placeholder"), selected: @import.account_id },
             required: true %>
 
@@ -112,7 +112,7 @@
             <%= form.select :col_sep, Import.separator_options, label: true %>
 
             <% if @import.type == "TransactionImport" || @import.type == "TradeImport" %>
-              <%= form.select :account_id, Current.user.accessible_accounts.visible.alphabetically.pluck(:name, :id), { label: t(".account_optional_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>
+              <%= form.select :account_id, @csv_account_options, { label: t(".account_optional_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>
             <% end %>
 
             <label for="import_import_file_csv" class="flex flex-col items-center justify-center w-full h-64 border border-secondary border-dashed rounded-xl cursor-pointer" data-controller="file-upload" data-file-upload-target="uploadArea">
@@ -144,7 +144,7 @@
             <%= form.select :col_sep, Import.separator_options, label: true %>
 
             <% if @import.type == "TransactionImport" || @import.type == "TradeImport" %>
-              <%= form.select :account_id, Current.user.accessible_accounts.visible.alphabetically.pluck(:name, :id), { label: t(".account_optional_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>
+              <%= form.select :account_id, @csv_account_options, { label: t(".account_optional_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>
             <% end %>
 
             <%= form.text_area :raw_file_str,

test/controllers/import/mappings_controller_test.rb

@@ -26,4 +26,75 @@ class Import::MappingsControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to import_confirm_path(@import)
   end
+
+  test "account mapping lists connected accounts as targets" do
+    @import.update!(
+      raw_file_str: <<~CSV,
+        date,amount,account
+        #{Date.current.iso8601},25,Imported Checking
+      CSV
+      date_col_label: "date",
+      amount_col_label: "amount",
+      account_col_label: "account",
+      date_format: "%Y-%m-%d"
+    )
+    @import.generate_rows_from_csv
+    @import.sync_mappings
+
+    get import_confirm_path(@import, step: 3)
+
+    assert_response :success
+    assert_select "select option[value='#{accounts(:connected).id}']", text: "Plaid Depository Account"
+  end
+
+  test "account mapping excludes accounts the user cannot write" do
+    sign_in users(:family_member)
+
+    @import.update!(
+      raw_file_str: <<~CSV,
+        date,amount,account
+        #{Date.current.iso8601},25,Credit Card
+      CSV
+      date_col_label: "date",
+      amount_col_label: "amount",
+      account_col_label: "account",
+      date_format: "%Y-%m-%d"
+    )
+    @import.generate_rows_from_csv
+    @import.sync_mappings
+
+    get import_confirm_path(@import, step: 3)
+
+    assert_response :success
+    assert_select "select option", text: "Add as new account"
+    assert_select "select option", text: "Credit Card", count: 0
+  end
+
+  test "trade import account mapping excludes linked accounts without reconciliation" do
+    linked = accounts(:connected)
+    assert linked.linked?
+
+    trade_import = imports(:trade)
+    trade_import.update!(
+      raw_file_str: <<~CSV,
+        date,ticker,qty,price,account
+        #{Date.current.iso8601},AAPL,1,100,#{linked.name}
+      CSV
+      date_col_label: "date",
+      ticker_col_label: "ticker",
+      qty_col_label: "qty",
+      price_col_label: "price",
+      account_col_label: "account",
+      date_format: "%Y-%m-%d"
+    )
+    trade_import.generate_rows_from_csv
+    trade_import.sync_mappings
+
+    get import_confirm_path(trade_import, step: 1)
+
+    assert_response :success
+    assert_select "select option", text: "Add as new account"
+    assert_select "select option[value='#{linked.id}']", count: 0
+    assert_nil trade_import.mappings.accounts.find_by(key: linked.name)&.mappable
+  end
 end

test/controllers/import/uploads_controller_test.rb

@@ -47,6 +47,17 @@ class Import::UploadsControllerTest < ActionDispatch::IntegrationTest
     assert_select 'select[name="import[account_id]"] option', text: "Plaid Depository Account", count: 0
   end
 
+  test "trade import account select excludes linked accounts without reconciliation" do
+    linked = accounts(:connected)
+    trade_import = imports(:trade)
+
+    get import_upload_url(trade_import)
+
+    assert_response :success
+    assert_select 'select[name="import[account_id]"] option[value=?]', linked.id, count: 0
+    assert_select 'select[name="import[account_id]"] option', text: accounts(:investment).name
+  end
+
   test "respects SURE_IMPORT_MAX_NDJSON_SIZE_MB when uploading Sure import file (#3010)" do
     configured_limit = 2.megabytes
     SureImport.stubs(:max_ndjson_size).returns(configured_limit)

test/controllers/imports_controller_test.rb

@@ -327,8 +327,8 @@ class ImportsControllerTest < ActionDispatch::IntegrationTest
     sign_in users(:family_member)
     patch import_url(pdf_import), params: { import: { account_id: account.id } }
 
-    assert_redirected_to account_url(account)
-    assert_equal I18n.t("accounts.not_authorized"), flash[:alert]
+    assert_redirected_to import_url(pdf_import)
+    assert_equal I18n.t("imports.update.invalid_account", default: "Account not found."), flash[:alert]
     assert_nil pdf_import.reload.account
     assert_nil statement.reload.account
   end