Exclude drafts from search results

[andyra]

Removes draft cards from search results

[flavorjones]

Hey @andyra, it looks like there are two separate but related changes in here:

1. Card.mentioning selects status: published
2. Search::Record.search joins against the cards table to select status: published

I'll write separately about each.

Card.mentioning

I don't think this change is necessary! Although Card.mentioning by itself will return cards in "draft" status, it's only called in two places.

The first call site is in app/controllers/prompts/cards_controller.rb (used for Lexxy card autocompletion), where it's added to a query that is already filtering for published cards:

https://github.com/basecamp/fizzy/blob/exclude-drafts-from-search-results/app/controllers/prompts/cards_controller.rb#L21-L30

    def search_cards
      published_cards
        .mentioning(params[:filter], user: Current.user)
        .reverse_chronologically
        .limit(MAX_RESULTS)
    end

    def published_cards
      Current.user.accessible_cards.published
    end

The second place is in app/models/filter.rb (which is what's used for the "F"-hotkeyed filter bar), and that is also being added to a query that already filtering for published cards:

https://github.com/basecamp/fizzy/blob/exclude-drafts-from-search-results/app/models/filter.rb#L19-L41

  def cards
    @cards ||= begin
      result = creator.accessible_cards.preloaded.published   # <--- "published" filter
      result = result.indexed_by(indexed_by)
      result = result.sorted_by(sorted_by)
      result = result.where(id: card_ids) if card_ids.present?
      result = result.where.missing(:not_now) unless include_not_now_cards?
      result = result.open unless include_closed_cards?
      result = result.unassigned if assignment_status.unassigned?
      result = result.assigned_to(assignees.ids) if assignees.present?
      result = result.where(creator_id: creators.ids) if creators.present?
      result = result.where(board: boards.ids) if boards.present?
      result = result.tagged_with(tags.ids) if tags.present?
      result = result.where(cards: { created_at: creation_window }) if creation_window
      result = result.closed_at_window(closure_window) if closure_window
      result = result.closed_by(closers) if closers.present?
      result = terms.reduce(result) do |result, term|
        result.mentioning(term, user: creator)               # <--- Card.mentioning
      end

      result.distinct
    end
  end

So I think this is an unnecessary change -- I don't think there's any way to get draft cards to show up in the "F"-hotkeyed filter bar or the Lexxy autocomplete prompt.

Search::Record.search

Here we do have a problem, and draft cards do show up in the "K"-hotkey search bar.

One way to fix this is to filter for card status at search time, which is what this change implements. And it works (although it doesn't have test coverage yet). But I'm not sure I like that we are introducing a new join against the cards table -- the search query previously just hits the search_records table.

I may be overly paranoid about search performance and maybe this is fine; but I do think we would benefit from keeping the search queries as simple as possible.

So two potential alternatives:

1. We could add the card status to the search table. One possible advantage here is that drafts are in the search index if we every want to do something with them; but this seems like an unlikely feature and I think YAGNI.
2. We could delay indexing cards until publish time, so the search index only contains published cards.

I'm leaning towards option 2 here, which is about the same complexity (in my imagination) but feels a bit cleaner to me. WDYT?

[flavorjones]

I've opened #2418 as an alternative approach, which omits draft cards from the search index entirely.

[andyra]

See, this is good feedback. And also why prompting Claude to build stuff I can't review myself is maybe not the most direct approach. Thanks for the alternative PR, @flavorjones! Much appreciated.