Added targert model check for 'import_from_hash' method.

[karlwilbur]

I added a check for 'import_from_hash' on target_model, if it exists. This allow for overridding the default create! process.

[antonzaytsev]

Am I right method "import_from_hash" is your "handmade" method in model ? Can you please show me source code of it so I can understand how we can modify code in gem to make you happy?

[karlwilbur]

I could but it would be custom for each and every model. It would be up to each model to know how it is supposed to be created. The import_from_hash in a user object would be completely different than for a group object or for role object. On Mar 26, 2013 3:00 AM, "Anton" notifications@github.com wrote:

Am I right method "import_from_hash" is your "handmade" method in model ? can you please show me source code of it so I can understand how we can modify code in gem to make you happy.

— Reply to this email directly or view it on GitHubhttps://github.com/krhorst/active_admin_importable/pull/9#issuecomment-15444183 .

[antonzaytsev]

Please show me source in at least 2 models of method import_from_hash

[Fivell]

@karlwilbur, it seems like virtual attributes can be used in this case

[karlwilbur]

Here are snippets from a couple classes:

class Tea < ActiveRecord::Base

  ## Static method for imports from active_admin_importable
  def self.import_from_hash(row)

    # look up by row's ID to update existing records
    if row.has_key? :id 
      if !row[:id].empty? 
        begin
          tea = self.find row[:id] 
        rescue
          # Not found in DB, create new record
          tea = self.new
        end
      end      
      # Remove ID, we will not try to create any records with IDs that don't already exist
      row.delete :id
    end

    # assign all other valid properties from the import
    row.each { |index, value| 
      case index
        when :brand
          tea.brand = Brand.find_by_name value
        when :brand_id
          tea.brand = Brand.find value
        when :type
          tea.tea_type = TeaType.find_by_name value
        when :tea_type_id
          tea.tea_type = TeaType.find value
        else
          if tea.has_attribute? index 
            tea[index] = value
          end
      end
    }
    tea.save!
  end

end

class User < ActiveRecord::Base

  ## Static method for imports from active_admin_importable
  def self.import_from_hash(row)

    # look up by row's ID to update existing records
    if row.has_key? :id 
      if !row[:id].empty? 
        begin
          user = self.find row[:id] 
        rescue
          # Not found in DB, create new record
          user = self.new
        end
      end      
      # Remove ID, we will not try to create any records with IDs that don't already exist
      row.delete :id
    end

    # assign all other valid properties from the import
    row.each { |index, value| 
      case index
        when :role
          user.role = Role.find_or_create_by_name value
        when :role_id
          user.role = Role.find value
        when :groups
          user.groups = value.split(',').map(&:strip).each { |v| Group.find_or_create_by_name v }
        when :group_ids
          user.groups = value.split(',').map(&:strip).each { |v| Group.find v }
        else
          if user.has_attribute? index 
            user[index] = value
          end
      end
    }
    user.save!
  end

end

[krhorst]

Might this be better served by allowing an optional block to be passed into the call to active_admin_importable, replacing the default create! behavior? Like so:

ActiveAdmin.register Product do
   active_admin_importable do |model, hash|
      store = Store.find_by_name(hash[:store_name])
      hash[:store_id] = store.id
      hash.delete(:store_name)
      model.create!(hash)
   end
end

It would then be able to support this behavior as well, by calling the model method in that block

ActiveAdmin.register Product do
   active_admin_importable do |model, hash|
       model.import_from_hash(hash)
   end
end

[karlwilbur]

Yeah! I think that would work. I didn't even think of that. It would just be called for each CSV row.

Also having a method named active_admin_importable within the ActiveAdmin resource makes much more sense than having specific model methods named inside active_admin_importable.

Karl Wilbur 513-322-2481 karl@karlwilbur.net

On Tue, Mar 26, 2013 at 10:28 AM, krhorst notifications@github.com wrote:

Might this be better served by allowing an optional block to be passed into the call to active_admin_importable, replacing the default create! behavior? Like so:

ActiveAdmin.register Product do active_admin_importable do |model, hash| store = Store.find_by_name(hash.store_name) hash.store_id = store.id model.create!(hash) end end

It would then be able to support this behavior as well, by calling the model method in that block

ActiveAdmin.register Product do active_admin_importable do |model, hash| model.import_from_hash(hash) end end

— Reply to this email directly or view it on GitHubhttps://github.com/krhorst/active_admin_importable/pull/9#issuecomment-15461218 .

[krhorst]

Great! Just updated the call to active_admin_importable to accept a block for custom processing.