Merge pull request #64 from ifyouseewendy/feature/add_batch_transaction_option

Feature/add batch transaction option

Author: Fivell

README.md

@@ -65,6 +65,7 @@ Tool                    | Description
 :csv_options			|hash with column separator, row separator, etc 
 :validate				|bool means perform validations or not
 :batch_size				|integer value of max  record count inserted by 1 query/transaction
+:batch_transaction    |bool (false by default), if transaction is used when batch importing and works when :validate is set to true
 :before_import			|proc for before import action, hook called with  importer object
 :after_import			|proc for after import action, hook called with  importer object
 :before_batch_import	|proc for before each batch action, called with  importer object

lib/active_admin_import/dsl.rb

@@ -8,6 +8,7 @@ module DSL
     # +back+:: resource action to redirect after processing
     # +csv_options+:: hash to override default CSV options
     # +batch_size+:: integer value of max  record count inserted by 1 query/transaction
+    # +batch_transaction+:: bool (false by default), if transaction is used when batch importing and works when :validate is set to true
     # +before_import+:: proc for before import action, hook called with  importer object
     # +after_import+:: proc for after import action, hook called with  importer object
     # +before_batch_import+:: proc for before each batch action, called with  importer object
@@ -31,11 +32,12 @@ module DSL
       if result.empty?
         flash[:warning] = I18n.t('active_admin_import.file_empty_error')
       else
-        if result.has_imported?
-          flash[:notice] = I18n.t('active_admin_import.imported', count: result.imported_qty, model: model_name, plural_model: plural_model_name)
-        end
         if result.has_failed?
           flash[:error] = I18n.t('active_admin_import.failed', count: result.failed.count, model: model_name, plural_model: plural_model_name)
+          return if options[:batch_transaction]
+        end
+        if result.has_imported?
+          flash[:notice] = I18n.t('active_admin_import.imported', count: result.imported_qty, model: model_name, plural_model: plural_model_name)
         end
       end
     end

lib/active_admin_import/importer.rb

@@ -16,6 +16,7 @@ class Importer
         :after_batch_import,
         :headers_rewrites,
         :batch_size,
+        :batch_transaction,
         :csv_options
     ].freeze
 
@@ -47,7 +48,7 @@ def import
     end
 
     def import_options
-      @import_options ||= options.slice(:validate, :on_duplicate_key_update, :ignore, :timestamps)
+      @import_options ||= options.slice(:validate, :on_duplicate_key_update, :ignore, :timestamps, :batch_transaction)
     end
 
     def batch_replace(header_key, options)
@@ -97,12 +98,14 @@ def run_callback(name)
     end
 
     def batch_import
+      batch_result = nil
       @resource.transaction do
         run_callback(:before_batch_import)
         batch_result = resource.import(headers.values, csv_lines, import_options)
+        raise ActiveRecord::Rollback if import_options[:batch_transaction] && batch_result.failed_instances.any?
         run_callback(:after_batch_import)
-        batch_result
       end
+      batch_result
     end
 
     def assign_options(options)

lib/active_admin_import/options.rb

@@ -6,6 +6,7 @@ module Options
         :csv_options,
         :validate,
         :batch_size,
+        :batch_transaction,
         :before_import,
         :after_import,
         :before_batch_import,
@@ -40,4 +41,4 @@ def self.options_for(config, options= {})
     end
 
   end
-end
+end

spec/fixtures/files/authors_invalid_model.csv

@@ -0,0 +1,3 @@
+Name,Last name,Birthday
+Jim,Doe,1986-05-01
+Jane,Roe,1986-05-01

spec/import_spec.rb

@@ -292,14 +292,41 @@ def upload_file!(name, ext='csv')
         end
       end
 
-      context "with invalid data insert" do
+      context "with invalid data insert on DB constraint" do
+        # :name field has an uniq index
         it "should render error" do
           upload_file!(:authors_invalid_db)
           expect(page).to have_content "Error:"
           expect(Author.count).to eq(0)
         end
       end
 
+      context "with invalid data insert on model validation" do
+        let(:options) { { validate: true } }
+
+        before do
+          Author.create!(name: "John", last_name: "Doe")
+        end
+
+        it "should render both successful and failed message" do
+          upload_file!(:authors_invalid_model)
+          expect(page).to have_content "Failed to import 1 author"
+          expect(page).to have_content "Successfully imported 1 author"
+          expect(Author.count).to eq(2)
+        end
+
+        context "use batch_transaction to make transaction work on model validation" do
+          let(:options) { { validate: true, batch_transaction: true } }
+
+          it "should render only the failed message" do
+            upload_file!(:authors_invalid_model)
+            expect(page).to     have_content "Failed to import 1 author"
+            expect(page).to_not have_content "Successfully imported"
+            expect(Author.count).to eq(1)
+          end
+        end
+      end
+
       context "with invalid records" do
         context "with validation" do
           it "should render error" do
@@ -402,4 +429,4 @@ def upload_file!(name, ext='csv')
 
   end
 
-end
+end

spec/support/rails_template.rb

@@ -4,7 +4,7 @@
 generate :model, 'post title:string:uniq body:text author:references'
 
 #Add validation
-inject_into_file "app/models/author.rb", "  validates_presence_of :name\n", after: "Base\n"
+inject_into_file "app/models/author.rb", "  validates_presence_of :name\n  validates_uniqueness_of :last_name\n", after: "Base\n"
 inject_into_file "app/models/post.rb", "   validates_presence_of :author\n", after: ":author\n"
 
 # Configure default_url_options in test environment