HiveBrain v1.2.0
Get Started
← Back to all entries
debugrubyrailsMinor

Controller action to process orders and payments

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
processactioncontrollerandorderspayments

Problem

I'm trying to reduce the if statements and simplify this create action.

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
end


I 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 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
end


Your 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
end


Some 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
end
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
end

Context

StackExchange Code Review Q#73465, answer score: 6

Revisions (0)

No revisions yet.