debugrubyrailsMinor
Controller action to process orders and payments
Viewed 0 times
processactioncontrollerandorderspayments
Problem
I'm trying to reduce the if statements and simplify this create action.
I thought about moving most of this logic to a single
Basically, three things take place in this action for an
def create
@order = Order.create_from_result(JSON.parse(params['order']), JSON.parse(params['customer']))
# Ensure order saves before processing payment
if @order.persisted?
# Braintree transaction
result = @order.make_payment(params[:payment_method_nonce])
if result.success?
@order.paid!
@order.process!
render :json => { :status => 200 }
else
p "#Error: #{result.message}"
render :json => { :error => "#{result.message}" }
end
else
p "Error: Order did not save successfully"
render :json => { :error => "Something went wrong. Don't worry, no transaction was made." }
end
endI thought about moving most of this logic to a single
@order.process! call, but then I'm not sure of the best way to handle the Braintree transaction result & return any errors. Basically, three things take place in this action for an
@order:- Order is created from JSON result
- Order payment is made
- Order is processed (order data is crunched and sent to supplier)
Solution
I would suggest using a service object. I would like to see what others think about this. It looks like there's too much going on for the controller.
The controller should be responsible for execution and value return. The model for processing the object. The service for wiring up the object and giving it prepared params to do its job.
This is not tested, but just to give you an idea. I would go further and condense the
Your service would look something like this:
Some would argue that a service object adds a needless complication. They may have a point. I think it depends on each use case.
The controller should be responsible for execution and value return. The model for processing the object. The service for wiring up the object and giving it prepared params to do its job.
This is not tested, but just to give you an idea. I would go further and condense the
paid! and process! methods in the model so that the service just receives an error or an OK to give back to the controller.def create
@order = OrderCreationService.new(params).process
if @order.processed?
render json: { status: 200 }
else
render json: { error: @order.errors }
end
endYour service would look something like this:
require 'json'
class OrderCreationService
attr_reader :order_params, :customer_params, :payment_method_nonce, :errors
def initialize(params)
@order_params = JSON.parse(params['order'])
@customer_params = JSON.parse(params['customer'])
@payment_method_nonce = JSON.parse(params['payment_method_nonce'])
@errors = []
end
def process
order = Order.create_from_result(order_params, customer_params)
if order.persisted?
result = order.make_payment(payment_method_nonce)
if result.success?
order.process!
else
errors << result.message
end
else
errors << order.errors
end
self
end
def processed?
errors.none?
end
endSome would argue that a service object adds a needless complication. They may have a point. I think it depends on each use case.
Code Snippets
def create
@order = OrderCreationService.new(params).process
if @order.processed?
render json: { status: 200 }
else
render json: { error: @order.errors }
end
endrequire 'json'
class OrderCreationService
attr_reader :order_params, :customer_params, :payment_method_nonce, :errors
def initialize(params)
@order_params = JSON.parse(params['order'])
@customer_params = JSON.parse(params['customer'])
@payment_method_nonce = JSON.parse(params['payment_method_nonce'])
@errors = []
end
def process
order = Order.create_from_result(order_params, customer_params)
if order.persisted?
result = order.make_payment(payment_method_nonce)
if result.success?
order.process!
else
errors << result.message
end
else
errors << order.errors
end
self
end
def processed?
errors.none?
end
endContext
StackExchange Code Review Q#73465, answer score: 6
Revisions (0)
No revisions yet.