Cleaning up a multi-model view in Rails3

Question

I'm new to rails and have built a multi-model form. It's working, but some of the code doesn't feel right.

The model code is below. I'm using Devise for my user authentication:

# models/signup.rb
class Signup < ActiveRecord::Base
  has_one :company
  accepts_nested_attributes_for :company
end

# models/company.rb
class Company < ActiveRecord::Base
  belongs_to  :signup
  has_many    :users

  accepts_nested_attributes_for :users
end

# models/user.rb
class User < ActiveRecord::Base
  devise :database_authenticatable, :confirmable, :recoverable, 
    :rememberable, :trackable, :validatable, :registerable

  attr_accessible :email, :password, :password_confirmation

  belongs_to :company
end

My view is fairly standard and follows the typical pattern for multiple models. The most important code is the initial Signup creation which is done via the following helper:

# signup is created via signups#new 
def setup_signup(signup)
  signup.company ||= Company.new
  if signup.company.users[0] == nil
    1.times { signup.company.users.build }
  end
  signup
end

I had to add a condition to not create additional users, because if the form validation failed it would create another user.

Finally my controller code looks like this:

# controllers/signups_controller.rb
class SignupsController < ApplicationController
  respond_to :html

  def new
    @signup = Signup.new
  end

  def create
    @signup = Signup.new(params[:signup])

    # The first user to signup for a company should be an admin user 
    # But this feels really awkward
    @signup.company.users.first.admin = true

    # Skip confirmation for the first user to signup
    @signup.company.users.first.skip_confirmation!

    if @signup.save
      sign_in_and_redirect(@signup.company.users.first)
    else
      render :action => "new" 
    end
  end
end

This is what feels wrong to me:

• When signing up to my website, there will only ever be a single User created. However, because a user is associated via a Company, and a Company has_many users I have to access this user in an awkward manner: signup.company.users.first. Obviously I could create an instance variable user = signup.company.users.first, and if that's the best solution, I'll take it, but it feels like there should be a better solution.
• The setup_signup helper method requires checking if a user has already been created so a second user isn't created: if signup.company.users[0] == nil. To me if feels like Rails ought to have a cleaner way of handling this.
• It feels like my create action is too fat for a controller, and that there ought to be a better way to set the admin boolean.

Am I right? Are there cleaner ways to accomplish these tasks?

Answer 1

Firstly (and yes I did read the comments) the signup model seems unessential.

So firstly we should do away with that, if you need to associate payment details then that could easily be done directly on the company model.

Secondly you controller is getting a bit cuddly. In favour of having "fat model, skinny controller" let's move all that signup logic into the company model in the form of a Company::register method.

So far we have changes like this:

# app/models/company.rb
class Company < ActiveRecord::Base
  has_many :users
  accepts_nested_attributes_for :users

  def self.register(params)
    company = self.new(params)

    company.users.first.admin = true
    company.users.first.skip_confirmation!

    if company.save
      company
    else
      false
    end
  end
end

Now your controller can look like this:

# app/controllers/companies_controller.rb
class CompaniesController < ApplicationController

  def new
    @company = Company.new
  end

  def create
    if @company = Company.register(params[:company])
      sign_in_and_redirect(@company.users.first)
    else
      render :new
    end
  end
end

I'm also pretty sure that it's unessential to use that helper you have above. The form builder should be able to work it out from the model accepting nested attributes.