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?

• Dogbert: Any reason you're using a Signup model and controller, instead of doing it directly from Company?
• Gavin Miller: @Dogbert - yes. I haven't gotten there yet, but I will be adding an additional model under the signup model.
• Dogbert: I was going to suggest a solution that would do away with the signup model completely. Could you explain a bit more about what kind of things you would be adding to the signup model, that can't be added to Company or Users?
• Gavin Miller: @Dogbert - I'll be adding an ActiveMerchant model when I get around to the credit card processing part of the application. So a Signup will have a Company and a CreditCard model.

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.