Remove Search::Record.for_account
Instead we'll compute the table name dynamically based on Current.account where needed. Also we'll prevent searchable records from being saved if Current.account is not set, otherwise the after commit callbacks will fail.
This commit is contained in:
@@ -5,10 +5,8 @@ module Card::Searchable
|
||||
include ::Searchable
|
||||
|
||||
scope :mentioning, ->(query, user:) do
|
||||
search_records = Search::Record.for_account(user.account_id)
|
||||
|
||||
joins(search_records.card_join)
|
||||
.merge(search_records.for_query(query: Search::Query.wrap(query), user: user))
|
||||
joins(Search::Record.card_join)
|
||||
.merge(Search::Record.for_query(query: Search::Query.wrap(query), user: user))
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ module Searchable
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
before_save :ensure_current_account
|
||||
after_create_commit :create_in_search_index
|
||||
after_update_commit :update_in_search_index
|
||||
after_destroy_commit :remove_from_search_index
|
||||
@@ -12,22 +13,24 @@ module Searchable
|
||||
end
|
||||
|
||||
private
|
||||
def ensure_current_account
|
||||
unless Current.account
|
||||
raise "Current.account must be set to save #{self.class.name}"
|
||||
end
|
||||
end
|
||||
|
||||
def create_in_search_index
|
||||
search_record_class.create!(search_record_attributes)
|
||||
Search::Record.create!(search_record_attributes)
|
||||
end
|
||||
|
||||
def update_in_search_index
|
||||
search_record_class.find_or_initialize_by(searchable_type: self.class.name, searchable_id: id).tap do |record|
|
||||
Search::Record.find_or_initialize_by(searchable_type: self.class.name, searchable_id: id).tap do |record|
|
||||
record.update!(search_record_attributes)
|
||||
end
|
||||
end
|
||||
|
||||
def remove_from_search_index
|
||||
search_record_class.find_by(searchable_type: self.class.name, searchable_id: id)&.destroy
|
||||
end
|
||||
|
||||
def search_record_class
|
||||
Search::Record.for_account(account_id)
|
||||
Search::Record.find_by(searchable_type: self.class.name, searchable_id: id)&.destroy
|
||||
end
|
||||
|
||||
def search_record_attributes
|
||||
|
||||
@@ -4,6 +4,6 @@ module Search
|
||||
end
|
||||
|
||||
def self.results(query:, user:)
|
||||
Record.for_account(user.account_id).search(query: Query.wrap(query), user: user)
|
||||
Record.search(query: Query.wrap(query), user: user)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -11,10 +11,6 @@ module Search::Record::SQLite
|
||||
end
|
||||
|
||||
class_methods do
|
||||
def for_account(account_id)
|
||||
self
|
||||
end
|
||||
|
||||
def matching_scope(query, account_id)
|
||||
joins("INNER JOIN search_records_fts ON search_records_fts.rowid = #{table_name}.id")
|
||||
.where("search_records_fts MATCH ?", query)
|
||||
|
||||
@@ -8,11 +8,11 @@ module Search::Record::Trilogy
|
||||
end
|
||||
|
||||
class_methods do
|
||||
def table_name
|
||||
def compute_table_name
|
||||
if Current.account
|
||||
"search_records_#{shard_id_for_account(Current.account.id)}"
|
||||
else
|
||||
super
|
||||
raise "Current.account is not set; cannot determine shard for Search::Record"
|
||||
end
|
||||
end
|
||||
|
||||
@@ -20,10 +20,6 @@ module Search::Record::Trilogy
|
||||
Zlib.crc32(account_id.to_s) % SHARD_COUNT
|
||||
end
|
||||
|
||||
def for_account(account_id)
|
||||
self
|
||||
end
|
||||
|
||||
def matching_scope(query, account_id)
|
||||
stemmed_query = Search::Stemmer.stem(query)
|
||||
account_key = "account#{account_id}"
|
||||
|
||||
@@ -2,17 +2,24 @@ namespace :search do
|
||||
desc "Reindex all cards and comments in the search index"
|
||||
task reindex: :environment do
|
||||
puts "Clearing search records..."
|
||||
Search::Record::Trilogy::SHARD_COUNT.times do |shard_id|
|
||||
ActiveRecord::Base.connection.execute("DELETE FROM search_records_#{shard_id}")
|
||||
if ActiveRecord::Base.connection.adapter_name == "SQLite"
|
||||
ActiveRecord::Base.connection.execute("DELETE FROM search_records")
|
||||
ActiveRecord::Base.connection.execute("DELETE FROM search_records_fts")
|
||||
else
|
||||
Search::Record::Trilogy::SHARD_COUNT.times do |shard_id|
|
||||
ActiveRecord::Base.connection.execute("DELETE FROM search_records_#{shard_id}")
|
||||
end
|
||||
end
|
||||
|
||||
puts "Reindexing cards..."
|
||||
Card.find_each do |card|
|
||||
Current.account = card.account
|
||||
card.reindex
|
||||
end
|
||||
|
||||
puts "Reindexing comments..."
|
||||
Comment.find_each do |comment|
|
||||
Current.account = comment.account
|
||||
comment.reindex
|
||||
end
|
||||
|
||||
|
||||
@@ -31,4 +31,14 @@ class Card::SearchableTest < ActiveSupport::TestCase
|
||||
assert_includes results, card_in_board
|
||||
assert_not_includes results, card_in_other_board
|
||||
end
|
||||
|
||||
test "card requires Current.account to be set" do
|
||||
Current.account = nil
|
||||
|
||||
error = assert_raises(RuntimeError) do
|
||||
@board.cards.create!(title: "Test card", creator: @user)
|
||||
end
|
||||
|
||||
assert_match(/Current.account must be set to save Card/, error.message)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -10,18 +10,18 @@ class Comment::SearchableTest < ActiveSupport::TestCase
|
||||
test "comment search" do
|
||||
# Comment is indexed on create
|
||||
comment = @card.comments.create!(body: "searchable comment text", creator: @user)
|
||||
record = Search::Record.for_account(@account.id).find_by(searchable_type: "Comment", searchable_id: comment.id)
|
||||
record = Search::Record.find_by(searchable_type: "Comment", searchable_id: comment.id)
|
||||
assert_not_nil record
|
||||
|
||||
# Comment is updated in index
|
||||
comment.update!(body: "updated text")
|
||||
record = Search::Record.for_account(@account.id).find_by(searchable_type: "Comment", searchable_id: comment.id)
|
||||
record = Search::Record.find_by(searchable_type: "Comment", searchable_id: comment.id)
|
||||
assert_match /updat/, record.content
|
||||
|
||||
# Comment is removed from index on destroy
|
||||
comment_id = comment.id
|
||||
comment.destroy
|
||||
record = Search::Record.for_account(@account.id).find_by(searchable_type: "Comment", searchable_id: comment_id)
|
||||
record = Search::Record.find_by(searchable_type: "Comment", searchable_id: comment_id)
|
||||
assert_nil record
|
||||
|
||||
# Finding cards via comment search
|
||||
@@ -34,8 +34,18 @@ class Comment::SearchableTest < ActiveSupport::TestCase
|
||||
|
||||
# Comment stores parent card_id and board_id
|
||||
new_comment = @card.comments.create!(body: "test comment", creator: @user)
|
||||
record = Search::Record.for_account(@account.id).find_by(searchable_type: "Comment", searchable_id: new_comment.id)
|
||||
record = Search::Record.find_by(searchable_type: "Comment", searchable_id: new_comment.id)
|
||||
assert_equal @card.id, record.card_id
|
||||
assert_equal @board.id, record.board_id
|
||||
end
|
||||
|
||||
test "comment requires Current.account to be set" do
|
||||
Current.account = nil
|
||||
|
||||
error = assert_raises(RuntimeError) do
|
||||
@card.comments.create!(body: "Test comment", creator: @user)
|
||||
end
|
||||
|
||||
assert_match(/Current.account must be set to save Comment/, error.message)
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user