Merge pull request #10 from thambley/issue1

Don't access the database unless and until necessary

.gemignore

@@ -0,0 +1 @@
+.bundle/*

.gitignore

@@ -8,3 +8,4 @@ doc
 .rbenv-version
 .ruby-version
 /Gemfile.lock
+gemfiles/*.gemfile.lock

CHANGELOG.md

@@ -1,17 +1,30 @@
 # Changelog
 
-## 1.0.2
+## 1.0.5
 
-### Bug Fixes
+### Updates
 
-* Fixes undefined local variable or `method max_per_page` [#3][] by [@rewritten][]
+* Fix #1 - Unnecessary database access
+* Fix broken tests
+
+## 1.0.4
+
+### Updates
+
+* Minor bug fixes / typo corrections
 
 ## 1.0.3
 
 ### Updates
 
 * Move require rake from gemspec to lib/activeadmin-xls.rb [#4][] by [@ejaypcanaria][]
 
+## 1.0.2
+
+### Bug Fixes
+
+* Fixes undefined local variable or `method max_per_page` [#3][] by [@rewritten][]
+
 <!--- Link List --->
 [#3]: https://github.com/thambley/activeadmin-xls/issues/3
 [#4]: https://github.com/thambley/activeadmin-xls/pull/4

Gemfile

@@ -1,30 +1,17 @@
 source 'https://rubygems.org'
 
-gem 'activeadmin', '~> 1.0'
 gem 'spreadsheet', '~> 1.1', '>= 1.1.4'
 
 group :development, :test do
-  gem 'haml', require: false
   gem 'rails-i18n' # Gives us default i18n for many languages
-  gem 'rdiscount' # For yard
-  gem 'sprockets'
   gem 'sqlite3'
   gem 'yard'
 end
 
 group :test do
-  gem 'capybara'
   gem 'cucumber-rails', require: false
   gem 'database_cleaner'
-  gem 'guard-coffeescript'
-  gem 'guard-rspec'
-  gem 'inherited_resources'
-  gem 'jasmine'
-  gem 'jslint_on_rails', '~> 1.0.6'
-  gem 'launchy'
-  gem 'rspec-mocks'
-  gem 'rspec-rails'
-  gem 'sass-rails'
-  gem 'shoulda-matchers', '1.0.0'
+  gem 'rspec-mocks', '~> 3.7'
+  gem 'rspec-rails', '~> 3.7'
   gem 'simplecov', require: false
 end

README.md

@@ -113,15 +113,35 @@ end
 ## Specs
 
 Running specs for this gem requires that you construct a rails application.
-To execute the specs, navigate to the gem directory,
-run bundle install and run these to rake tasks:
+
+To execute the specs, navigate to the gem directory, run bundle install and run these to rake tasks:
+
+### Rails 3.2
+
+```text
+bundle install --gemfile=gemfiles/rails_32.gemfile
+```
+
+```text
+BUNDLE_GEMFILE=gemfiles/rails_32.gemfile bundle exec rake setup
+```
+
+```text
+BUNDLE_GEMFILE=gemfiles/rails_32.gemfile bundle exec rake
+```
+
+### Rails 4.2
+
+```text
+bundle install --gemfile=gemfiles/rails_42.gemfile
+```
 
 ```text
-bundle exec rake setup
+BUNDLE_GEMFILE=gemfiles/rails_42.gemfile bundle exec rake setup
 ```
 
 ```text
-bundle exec rake
+BUNDLE_GEMFILE=gemfiles/rails_42.gemfile bundle exec rake
 ```
 
 ## Copyright and License

Rakefile

@@ -1,23 +1,23 @@
 #!/usr/bin/env rake
-require "activeadmin"
-require "rspec/core/rake_task"
+require 'rspec/core/rake_task'
 
-desc "Creates a test rails app for the specs to run against"
+desc 'Creates a test rails app for the specs to run against'
 task :setup do
   require 'rails/version'
-  system("mkdir spec/rails") unless File.exists?("spec/rails")
+  system('mkdir spec/rails') unless File.exist?('spec/rails')
+  puts "system \"bundle exec rails new spec/rails/rails-#{Rails::VERSION::STRING} -m spec/support/rails_template_with_data.rb\""
   system "bundle exec rails new spec/rails/rails-#{Rails::VERSION::STRING} -m spec/support/rails_template_with_data.rb"
 end
 
 RSpec::Core::RakeTask.new
-task :default => :spec
-task :test => :spec
+task default: :spec
+task test: :spec
 
-desc "build the gem"
+desc 'build the gem'
 task :build do
-  system "gem build activeadmin-xls.gemspec"
+  system 'gem build activeadmin-xls.gemspec'
 end
-desc "build and release the gem"
-task :release => :build do
+desc 'build and release the gem'
+task release: :build do
   system "gem push activeadmin-xls-#{ActiveAdmin::Xls::VERSION}.gem"
 end

activeadmin-xls.gemspec

@@ -15,9 +15,11 @@ Gem::Specification.new do |s|
   s.description = <<-DESC
   This gem provides excel/xls downloads for resources in Active Admin.
   DESC
-  s.files = `git ls-files`.split("\n").sort
-  s.test_files = `git ls-files -- {spec}/*`.split("\n")
-  s.test_files = Dir.glob('{spec/**/*}')
+
+  git_tracked_files = `git ls-files`.split("\n").sort
+  gem_ignored_files = `git ls-files -i -X .gemignore`.split("\n")
+
+  s.files = git_tracked_files - gem_ignored_files
 
   s.add_runtime_dependency 'activeadmin', '>= 0.6.6', '< 2'
   s.add_runtime_dependency 'spreadsheet', '~> 1.0'

gemfiles/rails_32.gemfile

@@ -0,0 +1,30 @@
+#!/usr/bin/env ruby
+source 'https://rubygems.org'
+
+ruby_major_version = RUBY_VERSION.split('.')[0].to_i
+ruby_minor_version = RUBY_VERSION.split('.')[1].to_i
+
+eval_gemfile(File.expand_path(File.join('..', 'Gemfile'), __dir__))
+
+gem 'rails', '3.2.22.5'
+
+gem 'activeadmin', '0.6.6'
+
+group :assets do
+  gem 'coffee-rails', '~> 3.2.1'
+  gem 'sass-rails',   '~> 3.2.3'
+
+  # See https://github.com/sstephenson/execjs#readme for more supported runtimes
+  # gem 'therubyracer', :platforms => :ruby
+
+  gem 'uglifier', '>= 1.0.3'
+end
+
+group :test do
+  gem 'shoulda-matchers', '~> 2.8.0'
+  if ruby_major_version > 2 || (ruby_major_version == 2 && ruby_minor_version > 1)
+    gem 'test-unit', '~> 3.0'
+  end
+end
+
+gemspec path: "../"

gemfiles/rails_42.gemfile

@@ -0,0 +1,22 @@
+#!/usr/bin/env ruby
+source 'https://rubygems.org'
+
+ruby_major_version = RUBY_VERSION.split('.')[0].to_i
+ruby_minor_version = RUBY_VERSION.split('.')[1].to_i
+
+eval_gemfile(File.expand_path(File.join('..', 'Gemfile'), __dir__))
+
+gem 'activeadmin', '1.0.0'
+gem 'devise', '~> 4.2'
+gem 'rails', '4.2.10'
+gem 'turbolinks', '~> 5.0.0'
+gem 'tzinfo-data'
+
+group :test do
+  gem 'shoulda-matchers', '~> 3.1'
+  if ruby_major_version > 2 || (ruby_major_version == 2 && ruby_minor_version > 1)
+    gem 'test-unit', '~> 3.0'
+  end
+end
+
+gemspec path: "../"

lib/active_admin/xls/builder.rb

@@ -29,7 +29,10 @@ class Builder
       #   @see ActiveAdmin::Axlsx::DSL
       def initialize(resource_class, options = {}, &block)
         @skip_header = false
-        @columns = resource_columns(resource_class)
+        @resource_class = resource_class
+        @columns = []
+        @columns_loaded = false
+        @column_updates = []
         parse_options options
         instance_eval(&block) if block_given?
       end
@@ -60,9 +63,7 @@ def skip_header
 
       # The scope to use when looking up column names to generate the
       # report header
-      def i18n_scope
-        @i18n_scope ||= nil
-      end
+      attr_reader :i18n_scope
 
       # This is the I18n scope that will be used when looking up your
       # colum names in the current I18n locale.
@@ -83,7 +84,12 @@ def before_filter(&block)
       end
 
       # The columns this builder will be serializing
-      attr_reader :columns
+      def columns
+        # execute each update from @column_updates
+        # set @columns_loaded = true
+        load_columns unless @columns_loaded
+        @columns
+      end
 
       # The collection we are serializing.
       # @note This is only available after serialize has been called,
@@ -94,34 +100,50 @@ def before_filter(&block)
       # only render specific columns. To remove specific columns use
       # ignore_column.
       def clear_columns
+        @columns_loaded = true
+        @column_updates = []
+
         @columns = []
       end
 
       # Clears the default columns array so you can whitelist only the columns
       # you want to export
-      def whitelist
-        @columns = []
-      end
+      alias whitelist clear_columns
 
       # Add a column
       # @param [Symbol] name The name of the column.
       # @param [Proc] block A block of code that is executed on the resource
       #                     when generating row data for this column.
       def column(name, &block)
-        @columns << Column.new(name, block)
+        if @columns_loaded
+          columns << Column.new(name, block)
+        else
+          column_lambda = lambda do
+            column(name, &block)
+          end
+          @column_updates << column_lambda
+        end
       end
 
       # removes columns by name
       # each column_name should be a symbol
       def delete_columns(*column_names)
-        @columns.delete_if { |column| column_names.include?(column.name) }
+        if @columns_loaded
+          columns.delete_if { |column| column_names.include?(column.name) }
+        else
+          delete_lambda = lambda do
+            delete_columns(*column_names)
+          end
+          @column_updates << delete_lambda
+        end
       end
 
       # Serializes the collection provided
       # @return [Spreadsheet::Workbook]
-      def serialize(collection, view_context)
+      def serialize(collection, view_context = nil)
         @collection = collection
         @view_context = view_context
+        load_columns unless @columns_loaded
         apply_filter @before_filter
         export_collection(collection)
         apply_filter @after_filter
@@ -145,6 +167,15 @@ def localized_name(i18n_scope = nil)
 
       private
 
+      def load_columns
+        return if @columns_loaded
+        @columns = resource_columns(@resource_class)
+        @columns_loaded = true
+        @column_updates.each(&:call)
+        @column_updates = []
+        columns
+      end
+
       def to_stream
         stream = StringIO.new('')
         book.write stream
@@ -158,11 +189,11 @@ def clean_up
 
       def export_collection(collection)
         return if columns.none?
-        row_index = 0
+        row_index = sheet.dimensions[1]
 
         unless @skip_header
-          header_row(collection)
-          row_index = 1
+          header_row(sheet.row(row_index), collection)
+          row_index += 1
         end
 
         collection.each do |resource|
@@ -173,14 +204,13 @@ def export_collection(collection)
 
       # tranform column names into array of localized strings
       # @return [Array]
-      def header_row(collection)
-        row = sheet.row(0)
+      def header_row(row, collection)
         apply_format_to_row(row, create_format(header_format))
         fill_row(row, header_data_for(collection))
       end
 
       def header_data_for(collection)
-        resource = collection.first
+        resource = collection.first || @resource_class.new
         columns.map do |column|
           column.localized_name(i18n_scope) if in_scope(resource, column)
         end.compact

lib/active_admin/xls/resource_controller_extension.rb

@@ -10,15 +10,22 @@ def self.included(base)
 
       def index_with_xls
         index_without_xls do |format|
-          yield format if block_given?
-
           format.xls do
-            xls = active_admin_config.xls_builder.serialize(collection,
-                                                            view_context)
+            xls_collection = if method(:find_collection).arity.zero?
+                               collection
+                             else
+                               find_collection except: :pagination
+                             end
+            xls = active_admin_config.xls_builder.serialize(
+              xls_collection,
+              view_context
+            )
             send_data(xls,
                       filename: xls_filename,
                       type: Mime::Type.lookup_by_extension(:xls))
           end
+
+          yield(format) if block_given?
         end
       end
 

lib/active_admin/xls/version.rb

@@ -1,5 +1,5 @@
 module ActiveAdmin
   module Xls
-    VERSION = '1.0.4'.freeze
+    VERSION = '1.0.5'.freeze
   end
 end