diff --git a/app/models/card/searchable.rb b/app/models/card/searchable.rb index 81e704e43..e2c0c11dc 100644 --- a/app/models/card/searchable.rb +++ b/app/models/card/searchable.rb @@ -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 diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb index 39e5d95f8..97534bbd7 100644 --- a/app/models/concerns/searchable.rb +++ b/app/models/concerns/searchable.rb @@ -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 diff --git a/app/models/search.rb b/app/models/search.rb index 264194f03..68dbbcd21 100644 --- a/app/models/search.rb +++ b/app/models/search.rb @@ -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 diff --git a/app/models/search/record/sqlite.rb b/app/models/search/record/sqlite.rb index 9517f9219..989f8d834 100644 --- a/app/models/search/record/sqlite.rb +++ b/app/models/search/record/sqlite.rb @@ -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) diff --git a/app/models/search/record/trilogy.rb b/app/models/search/record/trilogy.rb index 3620e530d..e9f1d74ab 100644 --- a/app/models/search/record/trilogy.rb +++ b/app/models/search/record/trilogy.rb @@ -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}" diff --git a/lib/tasks/search.rake b/lib/tasks/search.rake index c1ec0e28a..df39c3a70 100644 --- a/lib/tasks/search.rake +++ b/lib/tasks/search.rake @@ -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 diff --git a/test/models/card/searchable_test.rb b/test/models/card/searchable_test.rb index 18845ab7a..2dc8f13a6 100644 --- a/test/models/card/searchable_test.rb +++ b/test/models/card/searchable_test.rb @@ -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 diff --git a/test/models/comment/searchable_test.rb b/test/models/comment/searchable_test.rb index a8e52bfc2..3c7dd461f 100644 --- a/test/models/comment/searchable_test.rb +++ b/test/models/comment/searchable_test.rb @@ -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