Ruby ActiveRecord importing library

Question

In my Rails app I need to import some files from CSV and Excel files. I needed it in 2 models so I have written lib/importable.rb:

module Importable
  def self.included(base)
    base.send :extend, ActAsMethods
  end

  module ActAsMethods
    def act_as_importable(&block)
      @import_block = block
      send :extend, ImportableMethods
    end

    attr_reader :import_block
  end

  module ImportableMethods
    def import_from(file, *args)
      raise 'This model is not importable' if import_block.nil?

      send(:before_import, *args) if respond_to? :before_import

      sheet = open_spreadsheet(file)

      header = sheet.row(1).map(&:to_sym)

      (2..sheet.last_row).map do |i|
        row = Hash[header.zip(sheet.row(i))]

        import_block.call(row, *args)
      end
    end

    private
    def open_spreadsheet(file)
      logger.info "Importing file #{file.original_filename}"
      case File.extname(file.original_filename)
      when '.csv'  then Csv.new(file.path, nil, :ignore)
      when '.xls'  then Excel.new(file.path, nil, :ignore)
      when '.xlsx' then Excelx.new(file.path, nil, :ignore)
      when '.ods'  then Openoffice.new(file.path, nil, :ignore)
      else raise "Unknown file type: #{file.original_filename}"
      end
    rescue
      raise "Cannot open file: #{file.original_filename}"
    end
  end
end

class ActiveRecord::Base
  include Importable
end

And in model I need to write, i.e.:

class Invoice < ActiveRecord::Base
  act_as_importable do |row, company|
    invoice = company.invoices.find_or_initialize_by_name(row[:name].to_s)

    vat_in = if row[:client].is_a? String
               row[:client]
             else
               row[:client].to_i
             end
    client = company.clients.find_by_vat_in(vat_in.to_s)

    invoice.client = client

    invoice.pay_date = row[:pay_date]
    invoice.full_amount = row[:full_amount].to_i
    invoice.paid_amount = row[:paid_amount].to_i

    invoice.save!

    invoice
  end
end

The importing in controller looks like:

  def save
    file = params[:file]

    logger.debug file.path

    Invoice.transaction do
      @invoices = Invoice.import_from(file, current_company)
      @invoices.each(&:save)
    end
  rescue RuntimeError => error
    logger.debug error
    redirect_to import_company_clients_path, alert: I18n.t('invalid_file')
  rescue ActiveRecord::RecordInvalid
    redirect_to import_company_clients_path, alert: I18n.t('invalid_record')
  else
    respond_to do |format|
      format.html
      format.json { render json: @invoices }
    end
  end

But IMHO it's a little overbloated. Have you any suggedtions to make it more reagable and Gemified?

Answer 1

Notes:

• Your code looks pretty good, but I personally wouldn't have written it as a mixable module, this code seemps completely independent from ActiveRecord. I'd write a SpreadSheetReader class and just use it from wherever you want (this way the code is more modular).

• Where is import_from called from? it seems some code is missing.

• This vat_in conditional seems too verbose. Which are the possible values of row[:client]?

I'd write something like:

company = company_from_somewhere_i_dont_know
columns = [:name, :client, <other columns>...]

SpreadSheetReader.rows(:columns => columns).map do |row|
  invoice = company.invoices.find_or_initialize_by_name(row[:name].to_s)
  vat_in = row[:client].is_a?(String) ? row[:client] : row[:client].to_i.to_s
  client = company.clients.find_by_vat_in!(vat_in)
  invoice.update_attributes!({
    :pay_date => row[:pay_date],
    :full_amount => row[:full_amount].to_i,
    :paid_amount => row[:paid_amount].to_i,
  })
  invoice
end