ankane / searchkick

Intelligent search made easy
MIT License
6.49k stars 750 forks source link

detection of relation vs model doesn't handle unscope #1597

Open Roguelazer opened 1 year ago

Roguelazer commented 1 year ago

Describe the bug I have an application using Rails 6 and Rails-Admin. Upgrading searchkick to 5.x makes all rails_admin pages fail with "search must be called on model, not relation" (because rails_admin always uses a scope). However, even if I exit the scope by calling unscoped, this failure still occurs. I believe that in addition to checking for current_scope.nil?, searchkick should also check for current_scope == {}

To reproduce Use this code to reproduce when possible:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  gem "sqlite3", platform: :ruby
  gem "activerecord", "~>6.0", require: "active_record"
  gem "activejob", "~>6.0", require: "active_job"
  gem "searchkick", "~>5.1"
  gem "elasticsearch", ">= 7.5", "< 7.14"
end

puts "Searchkick version: #{Searchkick::VERSION}"
puts "Server version: #{Searchkick.server_version}"

ActiveRecord::Base.establish_connection adapter: "sqlite3", database: ":memory:"
ActiveJob::Base.queue_adapter = :inline

ActiveRecord::Schema.define do
  create_table :examples do |t|
    t.string :name
    t.string :description
  end
end

class Example < ActiveRecord::Base
  searchkick

  scope :with_name, ->() { where("NAME IS NOT NULL") }

  def self.custom_search(query)
    unscoped do
      search(query, fields: [:name]).response
    end
  end
end

Example.reindex
Example.create!(name: "Test", description: "Foo")
Example.create!(name: "Test 2", description: "Bar")
Example.create!( description: "Baz")
Example.search_index.refresh
# this should work
p Example.with_name.custom_search("test")

Additional context

Changing relation? to the following definition make unscoped work:

module Searchkick
  def self.relation?(klass)
    if klass.respond_to?(:current_scope)
      !(klass.current_scope.nil? || klass.send(:scope_attributes) == {})
    else
      klass.is_a?(Mongoid::Criteria) || !Mongoid::Threaded.current_scope(klass).nil?
    end
  end
end
ankane commented 1 year ago

Hey @Roguelazer, the search method needs to be called on the model, not a relation. You can use the model method to get it from the relation.

Roguelazer commented 1 year ago

Unfortunately, that doesn't quite work; the model appears to still have a current_scope if it's called from within a scope.