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\$

    1 Answer 1

    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 
    \$\endgroup\$
    0

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.