patternrubyrailsMinor
Cleaning up a multi-model view in Rails3
Viewed 0 times
multiviewrails3cleaningmodel
Problem
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:
My view is fairly standard and follows the typical pattern for multiple models. The most important code is the initial
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:
This is what feels wrong to me:
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
endMy 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
endI 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 "new"
end
end
endThis is what feels wrong to me:
- When signing up to my website, there will only ever be a single
Usercreated. However, because a user is associated via aCompany, 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 variableuser = 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_signuphelper 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
Solution
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
So far we have changes like this:
Now your controller can look like this:
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.
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
endNow 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
endI'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.
Code Snippets
# 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# 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
endContext
StackExchange Code Review Q#1804, answer score: 5
Revisions (0)
No revisions yet.