Rails - Right / Better way to to a belongs_to / has_many relationship

Question

I am creating an application through which a user will be able to create an account. When they create an account, in the same form they will be able to create an organization that will then be tied to their user. Once that user has created their account (and an organization) other users will be able to create an account and use an "access code" to join that organization as well. Looking at the code may explain it better.

The reason i'm posting on SO is because i have a feeling there is a better / more efficient way to do it than what i am currently doing. I'm using nested_forms (maybe not correctly) and i don't think i'm doing the associations the right way because, for example, i haven't been able to get the edit form to fill out the organization fields.

I am using sorcery for the authentication as well.

users_controller.rb

def new
  @user = User.new
end

def create
  @user = User.new(user_params)
  if params[:user][:organization][:name].blank?
    flash.now[:error] = "You must specify an organization name."
    render :new
  else
    if params[:user][:organization][:access_code].blank?
      # create new organization
      @access_code = "#{SecureRandom.urlsafe_base64(16)}#{Time.now.to_i}"
      @organization = Organization.create(:name => params[:user][:organization][:name], :access_code => @access_code)
      @user.organization_id = @organization.id
      @user.is_admin = true
    else
      # try and add someone to an organization
      @organization = Organization.find(:all, conditions: ["name = ? AND access_code = ?", params[:user][:organization][:name], params[:user][:organization][:access_code]])
      if @organization.empty?
        flash.now[:error] = "No organization has been found with that name and access code."
        render :new
        return
      else
        @user.organization_id = @organization.first.id
      end
    end
    if @user.save
      user = login(@user.email, params[:user][:password])
      if user
        flash[:success] = "Your account has been successfully created!"
        redirect_to admin_dashboard_path
      end
    else
      flash.now[:error] = "Something went wrong! Please try again."
      render :new
    end
  end
end

def edit
  @user = User.find(params[:id])
end

def update
  @user = User.find(params[:id])
  if @user.is_admin?
    if params[:user][:organization][:name].blank? && params[:user][:organization][:name] != @user.organization.name
      params[:user][:organization][:name] = @user.organization.name
    end
    if params[:user][:organization][:access_code].blank? && params[:user][:organization][:access_code] != @user.organization.access_code
      params[:user][:organization][:access_code] = @user.organization.access_code
    end
    @organization = Organization.find(params[:user][:organization_id])
    @organization.name = params[:user][:organization][:name]
    @organization.access_code = params[:user][:organization][:access_code]
    @organization.save
  end
  if @user.update(user_params)
    flash[:success] = "Your settings have been updated!"
    redirect_to edit_admin_user_path(@user.id)
  else
    flash.now[:error] = "Something went wrong! Please try again."
    render :edit
  end
end

private
  def user_params
    params.require(:user).permit(:organization_id, :email, :password, :password_confirmation, :full_name, :remember_me, {:organization_attributes => [:name, :website, :description, :access_code]})
  end

users.rb

class User < ActiveRecord::Base
  authenticates_with_sorcery!

  belongs_to :organization

  VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i

  validates_presence_of :full_name
  validates_presence_of :email
  validates_uniqueness_of :email, :on => :create
  validates_format_of :email, :with => VALID_EMAIL_REGEX, :on => :create
  validates_presence_of :password, :on => :create
  validates_confirmation_of :password
end

organization.rb

class Organization < ActiveRecord::Base
  authenticates_with_sorcery!

  has_many :users, :dependent => :destroy

  accepts_nested_attributes_for :users

  validates_presence_of :name
end

new.html.erb

<% provide(:title, 'Create a User') %>

<h1>Create a User</h1>
<p>Use the form below to create an account.</p>
<%= nested_form_for([:admin, @user], html: {role: "form"}) do |f| %>
  <%= render "shared/error_messages", obj: @user %>
  <fieldset>
    <legend>User Information</legend>
    <div class="form-group">
      <%= f.label :full_name, "Full Name" %>
      <span class="help-block">How should others see you?</span>
      <%= f.text_field :full_name, class: "form-control" %>
    </div>
    <div class="form-group">
      <%= f.label :email %>
      <span class="help-block">Your email address is used as your login.</span>
      <%= f.text_field :email, class: "form-control" %>
    </div>
    <div class="form-group">
      <%= f.label :password %>
      <%= f.password_field :password, class: "form-control" %>
    </div>
    <div class="form-group">
      <%= f.label :password_confirmation, "Confirm Password" %>
      <%= f.password_field :password_confirmation, class: "form-control" %>
    </div>
  </fieldset>
  <%= f.fields_for :organization do |o| %>
    <fieldset>
      <legend>Associated Organization</legend>
      <div class="form-group">
        <%= o.label :name, "Organization Name" %>
        <span class="help-block">This is the name of the organization you are a part of.</span>
        <%= o.text_field :name, class: "form-control" %>
      </div>
      <div class="form-group">
        <%= o.label :access_code, "Organization Access Code" %>
        <span class="help-block">Leaving this field blank will setup a new organization.</span>
        <%= o.text_field :access_code, class: "form-control" %>
      </div>
    </fieldset>
  <% end %>
  <div class="form-actions">
    <%= f.submit "Create Account", class: "btn btn-primary" %>
    <%= link_to "Cancel", :back, class: "text-btn" %>
  </div>
<% end %>

edit.html.erb

<% provide(:title, "Edit User: #{@user.full_name} (#{@user.organization.name})") %>

<h1>Edit User: <%= @user.full_name %> (<%= @user.organization.name %>)</h1>
<p>Use the form below to manage your account.</p>
<%= nested_form_for([:admin, @user], html: {role: "form"}) do |f| %>
  <%= render "shared/error_messages", obj: @user %>
  <fieldset>
    <legend>User Information</legend>
    <div class="form-group">
      <%= f.label :full_name, "Full Name" %>
      <span class="help-block">How should others see you?</span>
      <%= f.text_field :full_name, class: "form-control" %>
    </div>
    <div class="form-group">
      <%= f.label :email %>
      <span class="help-block">Your email address is used as your login.</span>
      <%= f.text_field :email, class: "form-control" %>
    </div>
    <div class="form-group">
      <%= f.label :password %>
      <%= f.password_field :password, placeholder: "leave blank to keep password unchanged", class: "form-control" %>
    </div>
    <div class="form-group">
      <%= f.label :password_confirmation, "Confirm Password" %>
      <%= f.password_field :password_confirmation, class: "form-control" %>
    </div>
  </fieldset>
  <% if @user.is_admin? %>
    <%= f.fields_for :organization do |o| %>
      <fieldset>
        <legend>Associated Organization</legend>
        <div class="form-group">
          <%= o.label :name, "Organization Name" %>
          <span class="help-block">This is the name of the organization you are a part of.</span>
          <%= o.text_field :name, class: "form-control", value: @user.organization.name %>
        </div>
        <div class="form-group">
          <%= o.label :access_code, "Organization Access Code" %>
          <span class="help-block">Leaving this field blank will setup a new organization.</span>
          <%= o.text_field :access_code, class: "form-control", value: @user.organization.access_code %>
        </div>
      </fieldset>
    <% end %>
    <%= f.hidden_field :organization_id %>
  <% end %>
  <div class="form-actions">
    <%= f.submit "Update User", class: "btn btn-primary" %>
    <%= link_to "Cancel", :back, class: "text-btn" %>
  </div>
<% end %>

Ok, those are all the files making it happen. Now, i have the application doing almost everything i need it to do but this doesn't feel like production-level code to me.

One issue i know i am having is that if a user types something in the organization field and nothing else the controller will create and save the organization and then render the form back with the user validation errors. I don't want it to save the organization if there are validation errors in the user model.

I'm really just asking for advice if there is a better way of doing what i am trying to do. If you can't tell exactly what i'm trying to do with this code or have any questions please let me know!

Answer 1

Take a look at this post: http://blog.codeclimate.com/blog/2012/10/17/7-ways-to-decompose-fat-activerecord-models/

Of particular interest will be the section on "3. Extract Form Objects".