Update and save value in rails

Question

The following method checks if a coupon is valid or not. I would like to update the total amount in the current_cart when coupon is valid. The validation process works fine but for some reason it won’t update the amount in here cart/show.html.erb <%= number_to_currency current_cart.amount %>

coupons_controller.rb

def redeem
    @coupon = Coupon.find_by_discount_code(params[:discount_code])
 if @coupon.blank?
    redirect_to :back
    flash[:notice] = "Coupon is Invalid!"
 else 
    @current_cart = Cart.find(current_cart.id)
    @current_cart.amount = @current_cart.amount - @coupon.amount
    @current_cart.save
    redirect_to :back
    flash[:notice] = "Coupon is Valid!"
 end
end

application_controller.rb

def current_cart 
  Cart.find(session[:cart_id])
rescue ActiveRecord::RecordNotFound 
  cart = Cart.create 
  session[:cart_id] = cart.id
  cart
end

  helper_method :current_cart
end

class Cart < ApplicationRecord
 has_many :line_items, dependent: :destroy

  def add_product(product_id)
    current_item = line_items.find_by_product_id(product_id) 
    if current_item
    current_item.quantity += 1
    else
    current_item = line_items.build(product_id: product_id)
    end
    current_item
  end


 def total_price
    line_items.to_a.sum { |item| item.total_price }
 end

 def tax_rate
   tax_amount = 0 
   line_items.each do |item|
   tax_amount += (item.product.price * item.quantity) *      item.product.tax_rate.rate
   end
   tax_amount
 end    

 def amount
   line_items.to_a.sum { |item| item.total_price } + tax_rate + fee
 end

 def fee
   shipment_amount = 0 
   line_items.each do |item|
    if item.quantity <= 1
    shipment_amount = item.product.shipment.fee
    else
    shipment_amount = item.product.multiple_shipment.fee
    end
  end
  shipment_amount
 end
end

Answer 1

As you can see here:

def amount
  line_items.to_a.sum { |item| item.total_price } + tax_rate + fee
end

amount is not a variable, but a method which adds together the prices of each item in the cart. You cannot change amount. The most straight-forward solution would be to add a new discount row to your Cart table:

class AddDiscountToCarts < ActiveRecord::Migration
  def change
    add_column :carts, :discount, :decimal, default: 0
  end
end

Then add it to your amount method:

def amount
  line_items.to_a.sum { |item| item.total_price } + tax_rate + fee - discount
end

And in your controller:

def redeem
  @coupon = Coupon.find_by_discount_code(params[:discount_code])
  if @coupon.blank?
    redirect_to :back
    flash[:notice] = "Coupon is Invalid!"
  else 
    @current_cart = Cart.find(current_cart.id)
    @current_cart.discount = @coupon.amount
    @current_cart.save
    redirect_to :back
    flash[:notice] = "Coupon is Valid!"
  end
end

Be warned: this code will still add full tax to your cart, even if the discount brings it down to 0. This is because your tax_rate method recalculates the amount using line items. You should never calculate your price multiple times because it'll introduce bugs and cost you extra work. Generally, tax rates are determined by location and applied to the total purchase after discounts. For some reason, you have itemized tax rates, which complicates things. But you could theoretically calculate a weighted average tax rate:

def tax_rate
  total_cost = line_items.inject(0) { |cost, item| cost += item.total_price } 
  weighted_rate = line_items.inject(0) do |rate, item|
    rate += (item.total_price * item.product.tax_rate.rate) / total_cost
  end
  return weighted_rate
end  

And then use it in your amount function this way:

def amount
  (line_items.to_a.sum { |item| item.total_price } - discount + fee) * tax_rate
end

Answer 2

Your problem likely lies in these two lines:

@current_cart.amount = @current_cart.amount - @coupon.amount
@current_cart.save

Because you're not responding to the result of #save in any way, that can silently fail without giving any feedback to the user.

You have two options.

1) Cause it to raise an exception on fail:

@current_cart.save!

2) Add another layer of feedback if #save fails.

@current_cart.amount = @current_cart.amount - @coupon.amount

flash[:notice] =
  if @current_cart.save
    "Coupon is Valid!"
  else
    "Something went wrong."
  end

redirect_to :back

If you need to debug what's wrong further, a simple thing to do is log any errors from #save to the console:

if @current_cart.save
  "Coupon is Valid!"
else
  puts @current_cart.errors.to_json
  "Something went wrong."
end

Bonus tip: I'm trying to make sense of why you have a current_cart method and are doing the same work that it does to set a @current_cart instance variable. You should just be accessing that object via the current_cart method.

Because it's defined as a helper_method, you should be able to access that in the view without needing to set an instance variable as well.

If you cache the result of current_cart in an instance variable, then you can call it as many times as you want within the controller and view without any performance penalty:

def current_cart 
  @current_cart ||= Cart.find(session[:cart_id])
rescue ActiveRecord::RecordNotFound 
  @current_cart = Cart.create! # You probably want this to throw an exception if it fails
  session[:cart_id] = cart.id
  @current_cart
end

helper_method :current_cart

Answer 3

It looks like

@current_cart.amount = current_cart.amount - @coupon.amount
current_cart.save

Is missing the @.

Try with:

@current_cart.amount = @current_cart.amount - @coupon.amount
@current_cart.save

Answer 4

Update your else clause to access the current_cart in a consistent manner.

else 
    @current_cart = Cart.find(current_cart.id)
    @current_cart.amount = @current_cart.amount - @coupon.amount
    @current_cart.save
    redirect_to :back
    flash[:notice] = "Coupon is Valid!"
end