Search code examples
ruby-on-railsruby-on-rails-3activerecordruby-on-rails-3.1ruby-on-rails-3.2

Should I wrap this controller code in an ActiveRecord transaction?


I have a Create method in my API PeopleController that creates a Person, Company, Email Addresses and Phone Numbers. I have a feeling that I need to wrap all this code in a transaction, so that everything rolls back if something goes wrong at the end of the code block (if person.save fails at the bottom, I still end up with Company, Emails, etc in the database).

What is the best way to do this? And can I put all that conditional logic inside of the transaction block as well?

def create
  person = Person.new(person_params)
  person.created_by = current_user.id
  person.account = current_account

  if params["company"] && params["company"].length > 1
    person.company_id = Company.where(name: params["company"], account_id: current_account.id).first_or_create.id
  end

  if params["emails"]
    if params["emails"]["primary"].length > 4
     person.email_addresses << EmailAddress.create(address: params["emails"]["primary"], addressType: params["emails"]["primary_type"])
    end
    if params["emails"]["secondary"] && params["emails"]["secondary"].length > 4
      person.email_addresses << EmailAddress.create(address: params["emails"]["secondary"], addressType: params["emails"]["secondary_type"])
    end
  end

  if params["phonenumbers"]
    if params["phonenumbers"]["primary"].length > 4
     person.phone_numbers << PhoneNumber.create(number: params["phonenumbers"]["primary"], numberType: params["phonenumbers"]["primary_type"])
    end
    if params["phonenumbers"]["secondary"] && params["phonenumbers"]["secondary"].length > 4
      person.phone_numbers << PhoneNumber.create(numberType: params["phonenumbers"]["secondary"], numberType: params["phonenumbers"]["secondary_type"])
    end
  end

  person.save

  render json: "Person saved."
end

Solution

  • Yes you should. What about also moving the entire thing in a model method?

    Also, there is another important fact that you should not be aware of. If you do this:

    person = Person.new
    person.phone_numbers << PhoneNumber.new(...)
    person.save
    

    Rails will auto build the transaction and generate both objects.
    This should allow you to improve code.