CODEWITHSUNDEEP
ruby-on-railsactiverecord
medBouzid
medBouzid

Reputation: 8402

Catch ActiveRecord::RecordNotUnique and all other exceptions after

I have a uniqueness constraint in my LineItem model that looks like this :

class LineItem < ApplicationRecord
    # this add uniqueness validation to [cart_id + product_id] together
    validates :cart_id, uniqueness: { scope: [:product_id] }
end

I added indexes + unique:true to these columns in my database as well for more security

In my LineItemsController#create I have the following

class LineItemsController < ApplicationController

    def create 

        @cart = Cart.find(session[:cart_id]

        product = Product.find(params[:product_id]

        @line_item = @cart.add_product(product, params[:licence])

        respond_to do |format|

            @line_item.save!

            format.html { redirect_to products_url }
            format.js 

         rescue ActiveRecord::RecordNotUnique

          @cart.line_items
             .find_by(product_id: params[:product_id])
             .update(licence_type: params[:licence], price: product.price)

          format.js

        end

    end

end

What I am trying to do is : if the user add a line item that already has the same product_id and cart_id then update the licence_type column with params[:licence]

Am using rescue ActiveRecord::RecordNotUnique for this purpose :

1 - is this a good way to do it (so I can save my self an additional request that check every time if the record exists in the database) ?

2 - How I can catch any other exceptions/errors other than ActiveRecord::RecordNotUnique ? I thought to add another rescue Exception => e at the bottom so I can catch all other exception but I think I read somewhere that catching general exception like that isn't good and that I should use something like rescue => e instead ?

any code snippet is appreciated, thanks!

Upvotes: 1

Views: 577

Answers (1)

Adam Lassek
Adam Lassek

Reputation: 35505

Never rely on uniqueness validation to save you from duplicate data. It has a well-established weakness to race conditions.

What you probably want in this case is find_or_initialize_by.

line_item = @cart.line_items.find_or_initialize_by(product_id: params[:product_id])
line_item.license_type = params[:license]
line_item.save

In addition to that, you should move the uniqueness constraint on this data into the DB. The specific way of doing that depends on what your DB is.

It's okay to keep the validator, but treat it like front-end validation; there as a convenience, to create a better user experience. But don't rely on it to keep your data consistent, that is the job of your DB.

Another comment on the code sample:

rescue has to be placed either within a begin..end block or as part of a method body. But in your case you have it inside of a do..end block without a surrounding begin..end, which will not work.

Upvotes: 1

Related Questions