Reputation: 461

ruby on rails query

    def remove_items
    line_items.each do |item|
        @ci = Product.find(item.id)
        @ci.quantity = @ci.quantity.to_i - 1
end

Hello, I am trying to use the id of the item and then match the id with a product and then minus 1 to the quantity property of that product.

I currently get this error though.

TypeError in OrdersController#create

can't convert nil into String

What is wrong? Thanks

OrderController#create Please bear in mind code is scruffy due to being in progress. :)

def create
@order = current_cart.build_order(params[:order])
@order.ip_address = request.remote_ip
@cart = current_cart

  if @order.save
      if @order.purchase
        @order.status = "paid"                      
        @cart.remove_items          
        @cart.destroy
        render :action => "success"
      else
      @order.status = "failed"
      @cart.destroy
         render :action => "failure"
    end
  else
    render action: "new"        
  end

end

I think this is the stack trace

[0m
  ←[1m←[35mLineItem Load (0.0ms)←[0m  SELECT "line_items".* FROM "line_items" WH
ERE "line_items"."cart_id" = 129
  ←[1m←[36mProduct Load (0.0ms)←[0m  ←[1mSELECT "products".* FROM "products" WHE
RE "products"."id" = ? LIMIT 1←[0m  [["id", 147]]
Completed 500 Internal Server Error in 5762ms

TypeError (can't convert nil into String):
  app/controllers/application_controller.rb:60:in `+'
  app/controllers/application_controller.rb:60:in `record_not_found'

Upvotes: 1

Answers (3)

Josh

Reputation: 8586

It's a better idea to have the DB decrement all the Products at once:

def remove_items
    Product.where(id:line_items.map(&:id)).update_all('quantity = quantity - 1')
end

OR even better:

def remove_items
    Product.decrement_counter(:quantity, line_items.map(&:id) )
end

These are faster, avoid errors if the product can not be found, and also avoid a Race Condition if you have multiple processes running concurrently.

Upvotes: 0

Daniel

Reputation: 4173

According to your comment this should solve the problem:

# application_controller.rb
def record_not_found
  flash[:alert] = "Cannot find record number #{params[:id]}. Displaying all records." 
  redirect_to root_path 
end

But if I were you I wouldn't output the params[:id] in the alert message. Just say that the record has not been found, basta.

flash[:alert] = "Cannot find record. Displaying all records."

You can also do this in one line:

redirect_to root_path, alert: "Cannot find record. Displaying all records."

To fix the logic in your remove_items method you need to actually save the object at the end:

def remove_items
    line_items.each do |item|
        ci = Product.find(item.id)
        ci.quantity = ci.quantity.to_i - 1
        ci.save
    end
end