1
\$\begingroup\$

This is an app that I started on a while back to get digging into Rails. It's grown and is now a functioning app. However, the user model is now over 200 lines long. I would love your opinion on how to clean this up.

# == Schema Information
#
# Table name: users
#
#  id                     :integer          not null, primary key
#  name                   :string(255)
#  email                  :string(255)
#  created_at             :datetime
#  updated_at             :datetime
#  password_digest        :string(255)
#  remember_token         :string(255)
#  admin                  :boolean          default(FALSE)
#  password_reset_token   :string(255)
#  password_reset_sent_at :datetime
#  admin_note             :text(800)
#  applications_count     :integer
#  admin_link             :string(255)
#  sourced                :boolean          default(FALSE)
#

class User < ActiveRecord::Base
  VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i
  default_scope { order('users.id DESC') } 

  before_save { self.email = email.downcase }
  before_create :create_remember_token
  before_create :ensure_common_app
  before_create :ensure_admin_link

  has_one  :common_app, dependent: :destroy, inverse_of: :user
  has_one  :video, inverse_of: :user, dependent: :destroy
  has_one  :extra_info, inverse_of: :user, dependent: :destroy
  has_many :cities, through: :common_app
  has_many :industries, through: :common_app
  has_many :applications, dependent: :destroy
  has_many :jobs, through: :applications

  has_secure_password validations: false

  validates :name, presence: true,
   length: { maximum: 50 }

  validates :email, presence: true, 
    format: { with: VALID_EMAIL_REGEX },
    uniqueness: { case_sensitive: false } 

  validates :password, length: { minimum: 6 }, allow_blank: true
  validates :password, presence: true, on: :create

  accepts_nested_attributes_for :common_app, 
    reject_if:  :all_blank, 
    allow_destroy:  true

  accepts_nested_attributes_for :extra_info, 
    reject_if:  :all_blank, 
    allow_destroy:  true

  scope :proactive, -> { where(sourced: false) }
  scope :sourced, -> { where(sourced: true) }

  scope :with_dependents, -> do
    User.includes(:common_app)
      .includes(:video)
      .includes(:applications)
      .includes(:jobs)
  end

  scope :for_profile, -> do 
      User.includes(applications: :job)
      .includes(:video)
      .includes(common_app: [:cities, :industries])
  end

  searchable do
    text :name, :email, :admin_note

    text :current_city  do
      common_app.try(:current_city)
    end

    text :nationality do
      common_app.try(:nationality)
    end

    text :china_contrib do
      common_app.try(:china_contrib)
    end

    text :china_time do
      common_app.try(:china_time)
    end

    text :job_interest do
      common_app.try(:job_interest)
    end

    text :china_goals do
      common_app.try(:china_goals)
    end

    integer :grad_year do
      common_app.try(:grad_year)
    end

    integer :city_ids, multiple: true do
      common_app.try(:city_ids)
    end

    integer :industry_ids, multiple: true do
      common_app.try(:industry_ids)
    end

    integer :role_type_ids, multiple: true do
      common_app.try(:role_type_ids)
    end

    boolean :has_video do
      common_app.try(:has_video)
    end

    text :extra_info_education do
     extra_info.try(:education)
    end

    text :extra_info_experience_1 do
     extra_info.try(:experience_1)
    end

    text :extra_info_experience_2 do
     extra_info.try(:experience_2)
    end
  end

  def potential_jobs
    Job.includes(:cities, :industries)
      .where('cities.id IN (?)', self.common_app.city_ids)
      .where('industries.id IN (?)', self.common_app.industry_ids)
      .where('jobs.id NOT IN (?)', self.jobs.map(&:id).concat([0]))
  end

  def self.works(param)
    !param.blank?
  end

  def self.encrypt(token)
    Digest::SHA1.hexdigest(token.to_s)
  end

  def self.new_remember_token
    SecureRandom.urlsafe_base64
  end

  def first_name
    self.name.split(" ").first
  end

  def last_name
    self.name.split(" ").last
  end

  def generate_email
    email_tag = "#{self.first_name}-#{self.last_name}-#{Time.now.to_date.to_s}"
    self.email = "#{email_tag}@atlas-china.com"

    self
  end

  def generate_pass
    self.password = ("a".."z").to_a.sample(8).join("")

    self
  end

  def generate_token(column) 
    begin
      self[column] = SecureRandom.urlsafe_base64
    end while User.exists?(column => self[column])
  end

  def generate_password_reset
    generate_token(:password_reset_token)
    self.password_reset_sent_at = Time.zone.now
    save! 

    self.password_reset_token
  end

  def send_password_reset
    generate_password_reset
    UserMailer.password_reset(self).deliver
  end

  private
  def ensure_common_app 
    self.common_app || self.build_common_app
  end

  def create_remember_token
    self.remember_token = User.encrypt(User.new_remember_token)
  end

  def ensure_admin_link
    self.generate_token(:admin_link)
  end
end
\$\endgroup\$
2
\$\begingroup\$

A few tips that come into my mind:

  1. Extract Sunspot search logic to a separate class. It's not really a user's concern.

  2. Inline some scopes, i.e.:

    scope :with_dependents, -> { includes(:common_app, :video, :applications, :jobs) }
    

    (This is actually more readable in my opinion.)

  3. Extract password management and encryption to separate class (again, not really the user's concern):

    class PasswordManager
      attr_reader :user
    
      def initialize(user)
        @user = user
      end
    
      def generate_password
        user.password = ("a".."z").to_a.sample(8).join("")
      end
    end
    
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.