ruby on rails query

Question

    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'

Show us action of create on OrdersController and output of rake routes — rails_id
– rails_id, Commented Jun 5, 2013 at 17:48

Daniel · Accepted Answer · 2013-06-05 19:45:09Z

1

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

edited Jun 5, 2013 at 19:45

answered Jun 5, 2013 at 19:27

Daniel

4,1933 gold badges29 silver badges46 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

baihu Over a year ago

Thanks, that solved the error but the logic does not work. Even when I hard code this, nothing changes def remove_items [at]ci = Product.find(10) [at]ci.quantity = 0 end Is there an issue with the instance of the object or the actual syntax itself? Thanks

Josh · Accepted Answer · 2013-06-05 18:23:56Z

0

Looks like you're buding a shopping cart app from scratch. Have you considered using an existing platform, like Spree?

answered Jun 5, 2013 at 18:23

Josh

8,6155 gold badges38 silver badges35 bronze badges

1 Comment

baihu Over a year ago

I didn't know such a thing existed but I like the experience of building something myself anyway.

Josh · Accepted Answer · 2013-06-06 00:37:19Z

0

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.

answered Jun 6, 2013 at 0:37

Josh

8,6155 gold badges38 silver badges35 bronze badges

Collectives™ on Stack Overflow

ruby on rails query

3 Answers 3

1 Comment

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related