Why doesn't validation in a model referenced by a callback cause my original transaction to fail?

Question

I have a model with an after_create callback. This callback causes a new record to be created in another model. However if a validation fails in the child record creation, the original transaction is still being saved.

This doesn't seem right. According to Rails docs the whole thing is wrapped in a transaction. Am I doing something wrong?

class ServiceProvision < ActiveRecord::Base  
  has_one :cash_receipt
  after_create :receive_payment_for_service_provision, :if => Proc.new { |sp| sp.immediate_settlement == true } 

  private

  def receive_payment_for_service_provision
    cash_account = CashAccount.find_by_currency_id_and_institution_id( self.currency_id, self.institution_id )
    CashReceipt.create( :account_id => account.id, :service_provision_id => self.id, :amount => self.amount, :currency_id => self.currency.id, :cash_account_id => ( cash_account ? cash_account.id : nil ) )
  end
end

class CashReceipt < ActiveRecord::Base 
  belongs_to :service_provision
  validates_presence_of :cash_account_id
end

The CashReceipt does fail and returns an error when its passed nil for the cash_account_id, however my new ServiceProvision object is still being saved.

it "should fail if a cash account doesn't exist for the currency and institution" do
  currency = Factory.create( :currency )
  institution = Factory.create( :institution )
  service_provision = Factory.build( :service_provision, :currency_id => currency.id, :institution_id => institution.id, :immediate_settlement => true ) 

  service_provision.save.should == false
  service_provision.should have( 1 ).error     
end


'ServiceProvision service provision creation should raise an error if a cash account doesn't exist for the currency and institution' FAILED expected: false,
     got: true (using ==)

This seems to contradict this from the docs

Both Base#save and Base#destroy come wrapped in a transaction that ensures that whatever you do in validations or callbacks will happen under the protected cover of a transaction. So you can use validations to check for values that the transaction depends on or you can raise exceptions in the callbacks to rollback, including after_* callbacks.

And if I manually try to cancel the transaction in the callback like so:

cr = CashReceipt.create( :account_id => account.id, :service_provision_id => self.id, :amount => self.amount, :currency_id => self.currency.id, :cash_account_id => ( cash_account ? cash_account.id : nil ) )
unless cr.errors.empty?
  errors.add_to_base("Error while creating CashReciept [#{cr.errors}].")                 
  return false
end

then the new ServiceProvision object is still saved.

Answer 1

You have to check the execution status of CashReceipt.create call in receive_payment_for_service_proviion method.

  def receive_payment_for_service_provision
    cash_account = CashAccount.find_by_currency_id_and_institution_id( self.currency_id, self.institution_id )
    cr = CashReceipt.create( :account_id => account.id, :service_provision_id => self.id, :amount => self.amount, :currency_id => self.currency.id, :cash_account_id => ( cash_account ? cash_account.id : nil ) )
    unless cr.errors.empty?
      # Make the ServiceProvision instance invalid
      errors.add_to_base("Error while creating CashReciept [#{cr.errors}].")                 
      return false # terminate the callback chain and roll back the TX immediately.
    end
  end

PS: You can simplify your after_create specification as follows:

after_create :receive_payment_for_service_provision, :if => :immediate_settlement? 

Answer 2

Rollbacks only happen automatically with before callbacks:

The whole callback chain is wrapped in a transaction. If any before callback method returns exactly false or raises an exception the execution chain gets halted and a ROLLBACK is issued. After callbacks can only accomplish that by raising an exception.

This makes sense because it allows for AR to prime the model and save it in memory before applying the transaction. Since you've done an after it has no knowledge of what to rollback too. Why not try before_save and see what you get.

Answer 3

Move the CacheReceipt creation to before_validation filter. Since you have a has_one association on ServiceProvision, the CacheReceipt object will have the correct :service_provision_id after save. Your code will be as follows:

before_validation :receive_payment_for_service_provision, :if => :immediate_settlement?  

def receive_payment_for_service_provision
  cash_account = CashAccount.find_by_currency_id_and_institution_id( self.currency_id, self.institution_id )
  self.cash_receipt.build(:account_id => account.id, 
                          :amount => self.amount, 
                          :currency_id => self.currency.id,  
                          :cash_account_id => ( cash_account ? cash_account.id : nil ) )
end

Now the save on ServiceProvision instance will return false if there are errors while saving the associated CacheReceipt.

Answer 4

Thanks to @KandadaBoggu, who led me to the solution...

Turns out the solution is to change the callback to before_create, and then do this:

  def receive_payment_for_service_provision
    cash_account = CashAccount.find_by_currency_id_and_institution_id( self.currency_id, self.institution_id )
    cr = self.create_cash_receipt( :account_id => account.id, 
                              :amount => self.amount, 
                              :currency_id => self.currency.id,  
                              :cash_account_id => ( cash_account ? cash_account.id : nil ) )
    unless cr.errors.empty?
      errors.add_to_base( "Error while creating CashReciept [#{cr.errors}]." )                 
      return false
    end
  end

In other words, we still need to manually check for validation errors in the association.