14

In my controller I have some code like...

...
if user.save
    something = Something.where("thing = ?", thing)
    if !(something.nil?)
        render json: { something: something }
    else
        #I WOULD LIKE TO ROLLBACK THE user.save HERE
    end
else
    render json: { error: user.errors.full_messages }, status: :bad_request
end

I have tried

raise ActiveRecord::Rollback, "Could not create new User, Something was not found."
render json: { error: "Could not create new User, Something was not found"}, status: :unprocessable_entity

in place of the ROLLBACK COMMENT area above, but this does not work. The user.save ends up going through. It spits something out to 'rails s', but it does not rollback the last transaction.

chris P
  • 6,359
  • 11
  • 40
  • 84

3 Answers3

16

If you want to use a transaction in the same sense you mentioned you could do something like this

User.transaction do
  if user.save
    something = Something.where("thing = ?", thing)
    if !(something.nil?)
      render json: { something: something }
    else
      raise ActiveRecord::Rollback
    end
  else
   render json: { error: user.errors.full_messages }, status: :bad_request
  end
end

Not sure if wrapping the response inside the transaction would work or not, but you'll need to test that.

PS: These two lines

something = Something.where("thing = ?", thing)
if !(something.nil?)

Are just equivalent to 

if Something.exists?(thing: thing)
Mohammad AbuShady
  • 40,884
  • 11
  • 78
  • 89
  • @AbuShady, don't you think it doesn't make any sense wrapping up `save` in another transaction whereas internally `save` is implemented using transaction! I think we should let him know what are the bad practices, instead of letting him do everything in his way. :) – Sharvy Ahmed Feb 21 '15 at 18:24
  • this transaction is controlled by me, i can choose when to rollback, and when not to, and it's used when you want to group a group of queries and keep it atomic (https://en.wikipedia.org/wiki/Atomicity_%28database_systems%29) for his condition it might be that the second condition is affected by the first save, so you can't test them together, so you need to be able to rollback if the 2nd condition fails – Mohammad AbuShady Feb 21 '15 at 18:27
  • Brother, I am well aware of the situation and rails is providing you callbacks for this reason. You can do it with after_save or before_save callback and it will rollback and undo everything as it promises. :) – Sharvy Ahmed Feb 21 '15 at 18:32
  • This condition is simple, or at least there's not much info provided, your answer is better if the second query is totally unrelated to the first save, but if it's chained in some way then you need to do a transaction, especially if the first save creates more saves for example. – Mohammad AbuShady Feb 21 '15 at 18:34
  • Then doing that in an elegant way would be using after_save, if I am not wrong, I am just asking, I'm always learning from mistakes. :) – Sharvy Ahmed Feb 21 '15 at 18:37
  • no, for example, in a cart system, you reserve an item ( query successful ), then create an order (query successful), and add that order to a cart (but then for some reason an error happens), what do you think would happen, a mess, but in the transaction, rails reverts every thing if any error happens, it would be like nothing ever happened – Mohammad AbuShady Feb 21 '15 at 18:39
  • Okay, then that would make sense. Thanks for the explanation. :) – Sharvy Ahmed Feb 21 '15 at 18:40
6

You need to wrap the code in a transaction for the rollback to work properly. Here's the documentation: http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html

Something like

ActiveRecord::Base.transaction do
  # the code from your question
end

The key is that both the user.save call (which modifies the DB) and the raise ActiveRecord::Rollback call need to be in that block.

alexcavalli
  • 526
  • 6
  • 10
1

You can do this instead:

something = Something.where(thing: thing)

if something && user.save
    render json: { something: something }
else
    render json: { error: user.errors.full_messages }, status: :bad_request
end
Sharvy Ahmed
  • 7,247
  • 1
  • 33
  • 46