About add_more_topics_if_expected, and a bug inside it


(Ballistic Tire) #1

I was wondering the logic behind “add_more_topics_if_expected”, why/when do we need the extra posts.

def find_grouped_results

  if @results.type_filter.present?
    ...
    send("#{@results.type_filter}_search")
  else
    ...
    topic_search
  end

  add_more_topics_if_expected
  @results
...
end

# Add more topics if we expected them
def add_more_topics_if_expected
  expected_topics = 0
  expected_topics = Search.facets.size unless @results.type_filter.present?
  expected_topics = Search.per_facet * Search.facets.size if @results.type_filter == 'topic' 
  expected_topics -= @results.posts.length
  if expected_topics > 0
    extra_posts = posts_query(expected_topics * Search.burst_factor)
    extra_posts = extra_posts.where("posts.topic_id NOT in (?)", @results.posts.map(&:topic_id)) if @results.posts.present?
    extra_posts.each do |post|
      @results.add(post)
      expected_topics -= 1
      break if expected_topics == 0
    end
  end
end

There is one bug we need to fix - the last page of topic_search. In this case expected_topics > 0, and we added the previous pages’ results back.

Proposed fix:

expected_topics = Search.per_facet * Search.facets.size if @results.type_filter == 'topic' && offset <= 0

Before sending a PR, I want to understand the original logic.