def cancel
begin
to_bank = @transfer.main_to_bank
to_bank.with_lock do
to_bank.locked_balance -= @transfer.amount
to_bank.available_balance += @transfer.amount
to_bank.save!
@transfer.cancel
@transfer.save!
end
rescue ActiveRecord::ActiveRecordError => e
redirect_to admin_transfer_url(@transfer), alert: "Error while cancelling."
return
end
redirect_to admin_transfer_url(@transfer), notice: 'Transfer was successfully cancelled.'
end
I would want to refactor the above code to the Transfer model or some other place, because this same code is used elsewhere. However, ActiveRecord does some transaction magic within the model, so I'm worried I might introduce some unexpected side effects by simply moving the code under the model.
Are my worries unfounded and how would the above code typically be refactored to be outside of the controller for reusability?
Update: This seems like the perfect spot for a service object, as described here http://blog.codeclimate.com/blog/2012/10/17/7-ways-to-decompose-fat-activerecord-models/.
1) As you mention in your update, this is a perfect fit for service objects. Put them in a directory like app/services since anything in /app is autoloaded. There are two popular ways of implementing them:
As a static class:
AccountService.transfer(from_account, to_account, amount)
As an object:
service = AccountService.new(from_account, to_account)
service.transfer(amount)
I prefer option one coming from a Java enterprise development background where you would use Java beans similarly.
I would also recommend returning result objects from all services as a rule. This means you create a small class called "ServiceResult" which contains a boolean flag of whether or not the call was successful, a user friendly message and optionally a result object (which is the method return value if you didn't have service result objects). In other words checking the result from the controller or any other place would be:
response = AccountService.transfer(from, to, amount)
if response.success?
flash[:notice] = response.message
else
flash[:alert] = response.message
end
You can always refactor this into a method:
flash_service_result response
After adding some helper methods to your services, a service method could look like this:
def self.transfer(from_account, to_account, amount)
ActiveRecord::Base.transaction do
..do stuff..
from_account.save!
to_account.save!
service_success("Transfer succesfull...")
end
rescue SomeException => error
service_error("Failed to transfer...")
rescue ActiveRecord::RecordInvalid => invalid_record_error
service_validation_error(invalid_record_error)
end
When using result objects, never raise an exception from the service that you expect to be handled (checked exceptions in other languages).
2) Using the active record transactional methods will behave the same regardless of where it's called from. It will not add any side effects. So yes you can call it from a controller or service.
