7
\$\begingroup\$

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:

  1. Order is created from JSON result
  2. Order payment is made
  3. Order is processed (order data is crunched and sent to supplier)
\$\endgroup\$
6
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.