Ruby block returning inappropriate value

Question

Following up this screencast, and its someway returning 2..3 instead if Product records

def save
    puts "--- imported_products: #{imported_products.inspect}"
    // --- imported_products: 2..3
    if imported_products.map(&:valid?).all?
      imported_products.each(&:save!)
      true
    else
      imported_products.each_with_index do |product, index|
        product.errors.full_messages.each do |message|
          errors.add :base, "Row #{index+2}: #{message}"
        end
      end
      false
    end
 end

 def imported_products
   @imported_products ||= load_imported_products
 end

 def load_imported_products
   spreadsheet = open_spreadsheet
   header = spreadsheet.row(1)
   (2..spreadsheet.last_row).each do |i|
     row = Hash[[header, spreadsheet.row(i)].transpose]
     product = Product.find_by_id(row['id']) || Product.new
     product.attributes = row.to_hash.slice(*accessible_attributes)
     product
   end
 end

Answer 1

Your load_imported_products method includes an each block. That block is the last 'line' of your method, so the return value of the block becomes the return value of the method.

Try the following:

def load_imported_products
  spreadsheet = open_spreadsheet
  header = spreadsheet.row(1)
  products = []
  (2..spreadsheet.last_row).each do |i|
    row = Hash[[header, spreadsheet.row(i)].transpose]
    product = Product.find_by_id(row['id']) || Product.new
    product.attributes = row.to_hash.slice(*accessible_attributes)
    products << product
  end
  products
end

Answer 2

Or use map

def load_imported_products
  spreadsheet = open_spreadsheet
  header = spreadsheet.row(1)
  products = (2..spreadsheet.last_row).map do |i|
    row = Hash[[header, spreadsheet.row(i)].transpose]
    product = Product.find(row['id'].to_i) || Product.new
    product.attributes = row.to_hash.slice(*accessible_attributes)
    product
  end
end

Also find_by_id is not needed the find method uses id although I forced it to an integer in case it was nil or stored as a string.

Answer 3

If you're talking about the load_imported_products method, then try adding each product to an array, than then return the array.

I'm not sure exactly what that method is returning, but you probably need to explicitly return a collection of products.

So

def load_imported_products
   products = []
   spreadsheet = open_spreadsheet
   header = spreadsheet.row(1)
   (2..spreadsheet.last_row).each do |i|
     row = Hash[[header, spreadsheet.row(i)].transpose]
     product = Product.find_by_id(row['id']) || Product.new
     product.attributes = row.to_hash.slice(*accessible_attributes)
     products << product
   end
   return products
 end