more specs, improved error hadling

Author: Fivell

config/locales/en.yml

@@ -2,6 +2,7 @@ en:
   active_admin:
     import: "Import"
   active_admin_import:
+    file_error: "Error: %{message}"
     file_format_error: "You can import only valid csv file"
     file_empty_error: "You can't import empty file"
     no_file_error: "Please, select file to import"

config/locales/it.yml

@@ -2,6 +2,7 @@ it:
   active_admin:
     import: "Importa"
   active_admin_import:
+    file_error: "Errore: %{message}"
     file_format_error: "Puoi importare solo CSV conformi"
     no_file_error: "Per favore, seleziona un file da importare"
     file_empty_error: "Non è possibile importare file vuoto"

lib/active_admin_import/dsl.rb

@@ -1,7 +1,7 @@
 module ActiveAdminImport
   module DSL
-  
-    
+
+
     # Declares import functionality
     #
     # Options
@@ -24,8 +24,8 @@ module DSL
     #
 
     def active_admin_import(options = {}, &block)
-      options.assert_valid_keys(*VALID_OPTIONS) 
-      
+      options.assert_valid_keys(*VALID_OPTIONS)
+
       default_options = {
           back: {action: :import},
           csv_options: {},
@@ -52,7 +52,6 @@ def active_admin_import(options = {}, &block)
           link_to(I18n.t('active_admin_import.import_model', model: options[:resource_label]), action: :import)
         end
 
-
         @active_admin_import_model = options[:template_object]
         render template: options[:template]
       end
@@ -61,8 +60,6 @@ def active_admin_import(options = {}, &block)
         link_to(I18n.t('active_admin_import.import_model', model: options[:resource_label]), action: :import)
       end
 
-
-
       collection_action :do_import, method: :post do
         authorize!(ActiveAdminImport::Auth::IMPORT, active_admin_config.resource_class)
 
@@ -76,21 +73,24 @@ def active_admin_import(options = {}, &block)
           if block_given?
             instance_eval &block
           else
-           
+
             model_name = options[:resource_label].downcase
             plural_model_name = options[:plural_resource_label].downcase
-            
-            
-            
-            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)
+
+
+            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)
+              end
             end
           end
         rescue ActiveRecord::Import::MissingColumnError, NoMethodError => e
-          flash[:error] = e.message
+          flash[:error] = I18n.t('active_admin_import.file_error', message: e.message)
         end
         redirect_to options[:back]
       end

lib/active_admin_import/import_result.rb

@@ -24,5 +24,9 @@ def has_failed?
       @failed.any?
     end
 
+    def empty?
+      total == 0
+    end
+
   end
 end

spec/fixtures/files/author.csv

@@ -0,0 +1,2 @@
+Name,Last name,Birthday
+John,Doe,1986-05-01

spec/fixtures/csv/authors.csv spec/fixtures/files/authors.csvspec/fixtures/csv/authors.csv renamed to spec/fixtures/files/authors.csv

@@ -0,0 +1 @@
+Name,Last name,Birthday

spec/fixtures/csv/authors_no_headers.csv …ec/fixtures/files/authors_no_headers.csvspec/fixtures/csv/authors_no_headers.csv renamed to spec/fixtures/files/authors_no_headers.csv spec/fixtures/csv/authors_no_headers.csv renamed to spec/fixtures/files/authors_no_headers.csv

@@ -8,8 +8,23 @@
     visit '/admin/authors/import'
   end
 
-  def upload_file!(name)
-    attach_file('active_admin_import_model_file', File.expand_path("./spec/fixtures/csv/#{name}.csv"))
+  def with_zipped_csv(name, &block)
+
+    zip_file = File.expand_path("./spec/fixtures/files/#{name}.zip")
+
+    begin
+      Zip::File.open(zip_file, Zip::File::CREATE) do |z|
+        z.add "#{name}.csv", File.expand_path("./spec/fixtures/files/#{name}.csv")
+      end
+      instance_eval &block
+    ensure
+      File.delete zip_file rescue nil
+    end
+
+  end
+
+  def upload_file!(name, ext='csv')
+    attach_file('active_admin_import_model_file', File.expand_path("./spec/fixtures/files/#{name}.#{ext}"))
     find_button('Import').click
   end
 
@@ -29,23 +44,72 @@ def upload_file!(name)
 
   context "import file" do
 
+    [:empty, :only_headers].each do |file|
+      context "when #{file} file" do
+        it "should render warning" do
+
+          upload_file!(file)
+          expect(page).to have_content I18n.t('active_admin_import.file_empty_error')
+          expect(Author.count).to eq(0)
+        end
+      end
+    end
+
+    context "when no file" do
+      it "should render error" do
+        find_button('Import').click
+        expect(Author.count).to eq(0)
+        expect(page).to have_content I18n.t('active_admin_import.no_file_error')
+      end
+    end
+
     context "with headers" do
 
-      it "imports file" do
+      it "should import file with many records" do
         upload_file!(:authors)
         expect(page).to have_content "Successfully imported 2 authors"
         expect(Author.count).to eq(2)
       end
+
+      it "should import file with 1 record" do
+        upload_file!(:author)
+        expect(page).to have_content "Successfully imported 1 author"
+        expect(Author.count).to eq(1)
+      end
     end
 
 
     context "without headers" do
+      context "with known csv headers" do
+        before do
+          allow_any_instance_of(ActiveAdminImport::Model).to receive(:csv_headers).and_return(['Name', 'Last name', 'Birthday'])
+        end
+
+        it "should import file" do
+          upload_file!(:authors_no_headers)
+          expect(page).to have_content "Successfully imported 2 authors"
+          expect(Author.count).to eq(2)
+        end
+      end
 
-      it "imports file" do
-        allow_any_instance_of(ActiveAdminImport::Model).to receive(:csv_headers).and_return(['Name','Last name','Birthday'])      
-        upload_file!(:authors_no_headers)
-        expect(page).to have_content "Successfully imported 2 authors"
-        expect(Author.count).to eq(2)
+      context "with unknown csv headers" do
+        it "should render error" do
+          upload_file!(:authors_no_headers)
+          expect(page).to have_content "Error:"
+          expect(Author.count).to eq(0)
+        end
+      end
+
+    end
+
+
+    context "when zipped" do
+      it "should import file" do
+        with_zipped_csv(:authors) do
+          upload_file!(:authors, :zip)
+          expect(page).to have_content "Successfully imported 2 authors"
+          expect(Author.count).to eq(2)
+        end
       end
     end