From 0d138df952dfe6541cef86882445d0c874d45b64 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 22 Jan 2026 14:17:44 -0500 Subject: [PATCH 1/2] Guard search indexing with searchable? check Only index cards and comments if their card is `published?`, using a new method `searchable?` that is implemented on both Card and Comment. This prevents draft cards and their comments from being indexed. When drafts are published, the existing `update_in_search_index` callback creates the record via `upsert!`, or else ensures unpublished records are removed from the index. Co-Authored-By: Claude Opus 4.5 --- app/models/card/searchable.rb | 4 ++ app/models/comment/searchable.rb | 4 ++ app/models/concerns/searchable.rb | 11 +++++- test/models/card/searchable_test.rb | 52 ++++++++++++++++++++++++++ test/models/comment/searchable_test.rb | 11 ++++++ test/models/search_test.rb | 5 +++ 6 files changed, 85 insertions(+), 2 deletions(-) diff --git a/app/models/card/searchable.rb b/app/models/card/searchable.rb index 8cc108b23..d5f53205d 100644 --- a/app/models/card/searchable.rb +++ b/app/models/card/searchable.rb @@ -25,4 +25,8 @@ module Card::Searchable def search_board_id board_id end + + def searchable? + published? + end end diff --git a/app/models/comment/searchable.rb b/app/models/comment/searchable.rb index 626d2be64..3194f9042 100644 --- a/app/models/comment/searchable.rb +++ b/app/models/comment/searchable.rb @@ -20,4 +20,8 @@ module Comment::Searchable def search_board_id card.board_id end + + def searchable? + card.published? + end end diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb index 17d1f186a..2be34fe8d 100644 --- a/app/models/concerns/searchable.rb +++ b/app/models/concerns/searchable.rb @@ -15,11 +15,17 @@ module Searchable private def create_in_search_index - search_record_class.create!(search_record_attributes) + if searchable? + search_record_class.create!(search_record_attributes) + end end def update_in_search_index - search_record_class.upsert!(search_record_attributes) + if searchable? + search_record_class.upsert!(search_record_attributes) + else + remove_from_search_index + end end def remove_from_search_index @@ -53,4 +59,5 @@ module Searchable # - search_content: returns content string # - search_card_id: returns the card id (self.id for cards, card_id for comments) # - search_board_id: returns the board id + # - searchable?: returns whether this record should be indexed end diff --git a/test/models/card/searchable_test.rb b/test/models/card/searchable_test.rb index a67890620..bf3de621f 100644 --- a/test/models/card/searchable_test.rb +++ b/test/models/card/searchable_test.rb @@ -3,6 +3,16 @@ require "test_helper" class Card::SearchableTest < ActiveSupport::TestCase include SearchTestHelper + test "searchable? returns true for published cards" do + card = @board.cards.create!(title: "Published Card", status: "published", creator: @user) + assert card.searchable? + end + + test "searchable? returns false for draft cards" do + card = @board.cards.create!(title: "Draft Card", status: "drafted", creator: @user) + assert_not card.searchable? + end + test "card search" do # Searching by title card = @board.cards.create!(title: "layout is broken", status: "published", creator: @user) @@ -78,4 +88,46 @@ class Card::SearchableTest < ActiveSupport::TestCase assert_equal 0, fts_count, "FTS entry should be deleted" end end + + test "updating a draft card does not index it" do + search_record_class = Search::Record.for(@user.account_id) + + card = @board.cards.create!(title: "Draft card", creator: @user, status: "drafted") + assert_nil search_record_class.find_by(searchable_type: "Card", searchable_id: card.id) + + card.update!(title: "Updated draft card") + assert_nil search_record_class.find_by(searchable_type: "Card", searchable_id: card.id), + "Draft card should not be indexed after update" + + results = Card.mentioning("Updated", user: @user) + assert_not_includes results, card + end + + test "publishing a draft card indexes it" do + search_record_class = Search::Record.for(@user.account_id) + + card = @board.cards.create!(title: "Draft to publish", creator: @user, status: "drafted") + assert_nil search_record_class.find_by(searchable_type: "Card", searchable_id: card.id) + + card.publish + search_record = search_record_class.find_by(searchable_type: "Card", searchable_id: card.id) + assert_not_nil search_record, "Published card should be indexed" + assert_equal card.id, search_record.card_id + + results = Card.mentioning("publish", user: @user) + assert_includes results, card + end + + test "unpublishing a draft card removes it from the search index" do + search_record_class = Search::Record.for(@user.account_id) + + card = @board.cards.create!(title: "Draft to publish", creator: @user, status: "published") + assert_not_nil search_record_class.find_by(searchable_type: "Card", searchable_id: card.id) + + card.update!(status: "drafted") + + assert_nil search_record_class.find_by(searchable_type: "Card", searchable_id: card.id) + results = Card.mentioning("publish", user: @user) + assert_not_includes results, card + end end diff --git a/test/models/comment/searchable_test.rb b/test/models/comment/searchable_test.rb index c1a62e607..b867c9d92 100644 --- a/test/models/comment/searchable_test.rb +++ b/test/models/comment/searchable_test.rb @@ -7,6 +7,17 @@ class Comment::SearchableTest < ActiveSupport::TestCase @card = @board.cards.create!(title: "Test Card", status: "published", creator: @user) end + test "searchable? returns true for comments on published cards" do + comment = @card.comments.create!(body: "test comment", creator: @user) + assert comment.searchable? + end + + test "searchable? returns false for comments on draft cards" do + draft_card = @board.cards.create!(title: "Draft Card", status: "drafted", creator: @user) + comment = draft_card.comments.build(body: "test comment", creator: @user) + assert_not comment.searchable? + end + test "comment search" do search_record_class = Search::Record.for(@user.account_id) # Comment is indexed on create diff --git a/test/models/search_test.rb b/test/models/search_test.rb index 343295dd0..44996404b 100644 --- a/test/models/search_test.rb +++ b/test/models/search_test.rb @@ -15,6 +15,11 @@ class SearchTest < ActiveSupport::TestCase results = Search::Record.for(@user.account_id).search("overflowing", user: @user) assert results.find { |it| it.card_id == comment_card.id && it.searchable_type == "Comment" } + # Drafted cards are excluded from search results + drafted_card = @board.cards.create!(title: "drafted searchable content", creator: @user, status: "drafted") + results = Search::Record.for(@user.account_id).search("drafted", user: @user) + assert_not results.find { |it| it.card_id == drafted_card.id } + # Don't include inaccessible boards other_user = User.create!(name: "Other User", account: @account) inaccessible_board = Board.create!(name: "Inaccessible Board", account: @account, creator: other_user) From a9c6bc2a283b45630f06633606a03d7a4a7a8544 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 23 Jan 2026 10:57:50 -0500 Subject: [PATCH 2/2] Add migration to remove draft cards from search index One-time script to clean up any search records that were indexed before the searchable? guard was added. Co-Authored-By: Claude Opus 4.5 --- ...23-remove-draft-cards-from-search-index.rb | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100755 script/migrations/20260123-remove-draft-cards-from-search-index.rb diff --git a/script/migrations/20260123-remove-draft-cards-from-search-index.rb b/script/migrations/20260123-remove-draft-cards-from-search-index.rb new file mode 100755 index 000000000..35afd98e8 --- /dev/null +++ b/script/migrations/20260123-remove-draft-cards-from-search-index.rb @@ -0,0 +1,22 @@ +#!/usr/bin/env ruby + +require_relative "../../config/environment" + +total_deleted = 0 + +Account.find_each do |account| + search_record_class = Search::Record.for(account.id) + + # Find search records for draft cards (both Card and Comment searchables) + draft_card_ids = Card.where(account_id: account.id, status: "drafted").pluck(:id) + + if draft_card_ids.any? + count = search_record_class.where(card_id: draft_card_ids).delete_all + if count > 0 + puts "#{account.name}: deleted #{count} search records for draft cards" + total_deleted += count + end + end +end + +puts "Migration completed! Total deleted: #{total_deleted}"