Error when trying to chain class method in control

2019-09-01 20:46发布

问题:

I am trying to chain a few class methods from my User model to perform a faceted search. When the code runs it returns the following error

undefined method `has_skill_categories' for #<Array:0x000001026d3de8>

Can you show me how to call these methods from the model in the controller by chaining them together?

Here is my code:

experts_controller.erb

class ExpertsController < ApplicationController
  layout 'experts'

  def index

    @users = User.text_search(params[:query])
              .has_marketing_assets(params[:marketing_platforms])
              .has_skill_categories(params[:skills])
              .search_for_user_country(params[:user][:country])
  end

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

user.erb

class User < ActiveRecord::Base

  # Include default devise modules. Others available are:
  # :confirmable, :lockable, :timeoutable and :omniauthable
  devise :database_authenticatable, :registerable,
         :recoverable, :rememberable, :trackable, :validatable

  has_many :marketing_assets
  has_many :marketing_platforms, through: :marketing_assets
  has_many :my_skills
  has_many :skills, through: :my_skills
  has_many :past_works
  has_many :past_work_types, through: :past_works

  validates :first_name, :last_name, presence: true

  include PgSearch
  pg_search_scope :search, against: [:first_name, :last_name, :company, :description, :job_title, :website, :email, :country, :city, :state],
                  using: {tsearch: {dictionary: 'english'}},
                  associated_against: {:skills => :name, :past_works => [:description, :title, :url], :marketing_assets => [:platform, :description, :url], :past_work_types => :name,
                                       :marketing_platforms => :name}

  def self.text_search(query)
    if query.present?
      search(query)
    else
      User.all
    end
  end


  def self.has_marketing_assets(platforms)
    if platforms.present?
      @platforms = MarketingPlatform.all
      platforms_count = platforms.count
      where_clause_platforms = 'SELECT *
                                FROM Users
                                WHERE Users.id IN
                                (SELECT Users.id
                                FROM users
                                INNER JOIN marketing_assets ON users.id = marketing_assets.user_id
                                WHERE marketing_assets.marketing_platform_id= '
      n = 0

      if platforms.count > 0

        platforms.each do |platform|
          n += 1
          where_clause_platforms = where_clause_platforms + platform
          if n < platforms_count
            where_clause_platforms = where_clause_platforms + ' OR marketing_assets.marketing_platform_id= '
          end
        end

        where_clause_platforms = where_clause_platforms + " GROUP BY users.id
                                                          HAVING COUNT(DISTINCT marketing_assets.marketing_platform_id) = #{platforms.count})"
        find_by_sql(where_clause_platforms)

      else
        return
      end
    end
  end


  def self.has_skill_categories(skills)
    if skills.present?

      skills_count = skills.count
      where_clause_skills = 'SELECT *
                                      FROM Users
                                      WHERE Users.id IN
                                      (SELECT Users.id
                                      FROM users
                                      INNER JOIN my_skills ON users.id = my_skills.user_id
                                      WHERE my_skills.skill_id= '
      n = 0

      if skills_count > 0

        skills.each do |skill|
          n += 1
          where_clause_skills = where_clause_skills + skill
          if n < skills_count
            where_clause_skills = where_clause_skills + ' OR my_skills.skill_id= '
          end
        end

        where_clause_skills = where_clause_skills + "GROUP BY users.id
                                                        HAVING COUNT(DISTINCT my_skills.skill_id) = #{skills.count})"
        find_by_sql(where_clause_skills)


      else
        return
      end
    end
  end


  def self.search_for_user_country(country)
    if country.present?
      where('country = ?', "#{country}")
    else
      return
    end
  end

end

回答1:

First off, in order to chain your methods, you should be returning an ActiveRecord query object. Calling return without an argument will return nil, which is not chainable. You should instead return where(), which would return the current collection with no modifications.

The reason you are getting the error above is because find_by_sql returns results as an array, not a scoped query like where does. So, as you are doing it now, I don't think there's a way to chain them. But that's probably a good thing because it will force you to rewrite your queries and scopes without raw sql statements.

I would highly recommend reviewing the Rails Guides on Active Record Querying, and avoid writing raw SQL statements if at all possible in a Rails project. This could greatly simplify your methodology. You should never put raw user input into SQL queries, which it looks like you are doing in multiple places in your code. Rails provides an advanced query interface to protect you and your data, and the SQL statement you are building above is extremely vulnerable to injection attacks.

With the correct combination of scope and association calls (which can use scopes defined on the associated model), you could probably clean a lot of that code up and greatly improve the security of your application.

Update

It looks to me like your queries could greatly be simplified using scopes and #merge.

def self.has_skill_categories(skill_ids)
  joins(:my_skills).merge Skill.where(id: skill_ids)
end

def self.has_marketing_assets(platform_ids)
  joins(:marketing_assets).merge MarketingAsset.where(marketing_platform_id: platform_ids)
end

Those may not get you exactly what you're going for, but from what I can tell, it should be close, and show you how you can use the built-in ActiveRecord query interface to build complex queries without ever writing any raw SQL.