fix: compose successive batch_slice_columns calls (#186)

batch_slice_columns memoized a single index set for the whole import, so a
second call reused the first call's column positions against already-sliced
rows — corrupting values instead of narrowing further (e.g. the name column
received the last_name).

Reset the working headers to the parsed source set at the start of each batch,
so each call computes indices from the current headers and successive calls
narrow the previous call's result. @source_headers holds the complete parsed
header row; @headers is its per-batch working copy. Cover successive calls
across one and multiple batches.

Author: Fivell

lib/active_admin_import/importer.rb

@@ -82,23 +82,15 @@ def batch_replace(header_key, options)
     # end
     #
     def batch_slice_columns(slice_columns)
-      # Only set @use_indexes for the first batch so that @use_indexes are in correct
-      # position for subsequent batches
-      unless defined?(@use_indexes)
-        @use_indexes = []
-        headers.values.each_with_index do |val, index|
-          @use_indexes << index if val.in?(slice_columns)
-        end
-        return csv_lines if @use_indexes.empty?
-
-        # slice CSV headers
-        @headers = headers.to_a.values_at(*@use_indexes).to_h
-      end
+      columns = headers.values
+      indexes = columns.each_index.select { |i| columns[i].in?(slice_columns) }
+      return csv_lines if indexes.empty?
 
-      # slice CSV values
-      csv_lines.map! do |line|
-        line.values_at(*@use_indexes)
-      end
+      # @headers is reset to the full set at the start of every batch (see
+      # #batch_import), so each call narrows the previous call's result and every
+      # batch slices the same way — calling this more than once now composes (#186).
+      @headers = headers.to_a.values_at(*indexes).to_h
+      csv_lines.map! { |line| line.values_at(*indexes) }
     end
 
     def values_at(header_key)
@@ -129,17 +121,18 @@ def process_file
     end
 
     def prepare_headers
-      headers = self.headers.present? ? self.headers : yield
-      blank_positions = headers.each_index.select { |i| headers[i].to_s.strip.empty? }
+      names = self.headers.present? ? self.headers : yield
+      blank_positions = names.each_index.select { |i| names[i].to_s.strip.empty? }
       unless blank_positions.empty?
         raise ActiveAdminImport::Exception,
               "blank column header at column #{blank_positions.map { |i| i + 1 }.join(', ')}"
       end
 
-      headers = headers.map(&:to_s)
-      @headers = Hash[headers.zip(headers.map { |el| el.underscore.gsub(/\s+/, '_') })].with_indifferent_access
-      @headers.merge!(options[:headers_rewrites].symbolize_keys.slice(*@headers.symbolize_keys.keys))
-      @headers
+      names = names.map(&:to_s)
+      # @source_headers is the complete parsed header row; batch_import copies it
+      # into the per-batch working @headers (see #batch_import).
+      @source_headers = Hash[names.zip(names.map { |el| el.underscore.gsub(/\s+/, '_') })].with_indifferent_access
+      @source_headers.merge!(options[:headers_rewrites].symbolize_keys.slice(*@source_headers.symbolize_keys.keys))
     end
 
     def run_callback(name)
@@ -148,6 +141,9 @@ def run_callback(name)
 
     def batch_import
       batch_result = nil
+      # Every batch re-parses full-width rows, so restore the full header set
+      # before slicing; batch_slice_columns then narrows this working copy.
+      @headers = @source_headers.dup
       @resource.transaction do
         run_callback(:before_batch_import)
         batch_result = resource.import(headers.values, csv_lines, import_options)

spec/import_spec.rb

@@ -656,6 +656,46 @@ def upload_file!(name, ext = 'csv')
     end
   end
 
+  # Issue #186: each call must slice the result of the previous one, not re-apply
+  # the first call's indices to an already-sliced row.
+  context "with successive batch_slice_columns calls" do
+    let(:batch_size) { 2 }
+
+    before do
+      # validate: false isolates the slicing behaviour from the model's
+      # uniqueness validation, which would otherwise reject the second batch's
+      # author for sharing a NULL last_name with the first.
+      add_author_resource template_object: ActiveAdminImport::Model.new,
+                          validate: false,
+                          before_batch_import: lambda { |importer|
+                            importer.batch_slice_columns(%w(name last_name))
+                            importer.batch_slice_columns(%w(name))
+                          },
+                          batch_size: batch_size
+      visit "/admin/authors/import"
+      upload_file!(:authors)
+    end
+
+    shared_examples_for "only the name column kept" do
+      it "narrows the columns progressively" do
+        expect(Author.pluck(:name, :last_name, :birthday)).to match_array(
+          [
+            ["Jane", nil, nil],
+            ["John", nil, nil]
+          ]
+        )
+      end
+    end
+
+    it_behaves_like "only the name column kept"
+
+    context "across multiple batches" do
+      let(:batch_size) { 1 }
+
+      it_behaves_like "only the name column kept"
+    end
+  end
+
   context 'with invalid options' do
     let(:options) { { invalid_option: :invalid_value } }