rails 4 simple form nested attributes multiple models error during update

Question

I am battling an error with nested attributes and trying to fix the cop error at the same time. So here is the walk through. A coupon code may be submitted with the form using nested attributes that may affect the price of the job. This only occurs if the coupon code is valid. In this scenario the coupon code has already been assigned so the first if coupon_code && coupon.nil? is triggered. When the form comes back around the flash message works correctly but simple form does not display the value. I could adjust simple form to have the value with an instance variable but I'm starting to smell something a bit off here in my logic. Also, the smell of Assignment Branch Condition is starting to worry me. I can move forward with this, but the user would like to see the code. I would too.

Cop Error:

app/controllers/payments_controller.rb:9:3: C: Assignment Branch Condition size for update is too high. [17.97/15]

Controller:

class PaymentsController < ApplicationController
  rescue_from ActiveRecord::RecordNotFound, with: :route_not_found_error

  Numeric.include CoreExtensions::Numeric::Percentage

  def update
    @job = Job.find(params[:job_id])
    coupon_code = params[:job][:coupon_attributes][:code]
    coupon = validate_coupon(coupon_code)
    if coupon_code && coupon.nil?
      @coupon_code = coupon_code
      flash.now[:error] = t('flash_messages.coupons.id.not_found')
      render 'payments/new', layout: 'nested/job/payment'
    else
      update_job(@job, coupon)
      update_coupon(coupon, @job) if coupon
      redirect_to @job.vanity_url
    end
  end

  def new
    @job = Job.find(params[:job_id])
    return if reroute?(@job)
    render 'payments/new', layout: 'nested/job/payment'
  end

  private

  def update_job(job, coupon)
    job.start_at = DateTime.now
    job.end_at = AppConfig.product['settings']['job_active_for_day_num'].days.from_now
    job.paid_at = DateTime.now
    job.price = price_job(coupon)
    # job.save
  end

  def validate_coupon(coupon_code)
    return nil unless coupon_code.present?
    coupon = Coupon.active.find_by_code(coupon_code)
    return nil unless coupon.present?
    coupon
  end

  def price_job(coupon)
    price = AppConfig.product['settings']['job_base_price']
    return price unless coupon
    price = coupon.percent_discount.percent_of(price)
    price
  end

  def update_coupon(coupon, job)
    coupon.job_id = job.id
    coupon.executed_at = DateTime.now
    coupon.save
  end
end

View:

ruby:
  content_for :body_id_class, 'PaymentNew'
  content_for :js_instance, 'viewPaymentNew'
  content_for :browser_title, 'Payment'
  job_base_price = AppConfig.product['settings']['job_base_price']
  coupon_code = @coupon_code ||= ''

= simple_form_for(@job, url: job_payment_path, html: { id: 'payment-processor-form' }) do |j|
  div[class='row']
    div[class='col-md-12']
      div[class='panel panel-default']
        div[class='panel-heading']
          h3[class='panel-title']
            |Total Cost
        div[class='panel-body']
          h2[class='job-cost' data-initial = "#{job_base_price}"]
            = number_to_currency(job_base_price)
        div[class='panel-heading']
          h3[class='panel-title']
            |Have a coupon?
        div[class='panel-body']
          div[class='row-inline']
            div[class='row-block row-block-one']
              = j.simple_fields_for :coupon_attributes, @job.coupon do |c|
                = c.input_field :code, maxlength: 50, id: 'coupon-code', class: 'form-control', data: { 'initial' => 0 }, value: coupon_code
            div[class='row-block']
              button[type='button' class='btn btn-primary' id='coupon-verify' ]
                |Verify
            p[class='help-hint']
              = t('simple_form.hints.coupon.code')

  div[class='row']
    div[class='col-md-12']
      = j.button :button, type: 'button', class: 'btn-primary text-uppercase', id: 'purchase-job' do
        = job_posting_button_step_label

Updates

1. Refactoring this code to work with the post below. Factories fixed factorygirl create model association NoMethodError: undefined method

Answer 1

You have quite a few code smells going on in that fat old controller. Most of them seem to be symtoms that all is not well on the model layer and that you are not modeling the domain very well.

You might want to consider something like this:

class Job < ActiveRecord::Base
  has_many :payments
end

class Payment < ActiveRecord::Base
  belongs_to :job
  belongs_to :coupon
end

class Coupon < ActiveRecord::Base
  validates_uniqueness_of :code
end

This will let our countroller focus on CRUD'ing a single resouce rather than trying to herd a bunch of cats.

So lets look at enforcing the business logic for coupons.

class Payment < ActiveRecord::Base
  belongs_to :job
  belongs_to :coupon

  validate :coupon_must_be_active

  attr_writer :coupon_code

  def coupon_code=(code)
    coupon = Coupon.find_by(code: code)
    @coupon_code = code
  end

  private 
  def coupon_must_be_active
    if coupon
      errors[:coupon] << "must be active." unless coupon.active?
    elsif @coupon_code.present? 
      errors[:coupon_code] << "is not valid."
    end
  end
end

The custom attribute writer loads the coupon from the a code. The validation sets up our business logic rules.

We really should do the same when it comes to the job pricing:

class Job < ActiveRecord::Base
  after_initialize :set_price

  def set_price
    self.price ||= AppConfig.product['settings']['job_base_price']
  end
end

class Payment < ActiveRecord::Base
  after_initialize :set_price
  validates_presence_of :job

  def net_price
     return job.price unless coupon
     job.price * (coupon.percent_discount * 00.1)
  end

  # ...
end

We can then write our controller like so:

class PaymentsController

  before_action :set_job

  # GET /jobs/:job_id/payments/new
  def new
    @payment = @job.payments.new
  end

  # POST /jobs/:job_id/payments
  def create
    @payment = @job.payments.create(payment_params)
  end

  # PATCH /jobs/:job_id/payments/:id
  def update
    @payment = @job.payments.find(params[:id])
  end

  private

    def set_job
      @job = Job.find(params[:job_id])
    end

    def payment_params
      params.require(:payment)
            .permit(:coupon_code)
    end
end

We can then simply setup the form with:

= simple_form_for([@job, @payment]) do |f|
  = f.input :coupon_code
  = f.submit

Note that you don't want to take the price from the user unless you intend to implement the honor system - you should get it from your models by setting up association callbacks.