FIX: Limits for PM and group header search (#16887)

When searching for PMs or PMs in a group inbox, results in the header search were not being limited to 5 with a "More" link to the full page search. This PR fixes that.

It also simplifies the logic and updates the search API docs to include recently added `in:messages` and `group_messages:groupname` options.
This commit is contained in:
Penar Musaraj 2022-05-24 11:31:24 -04:00 committed by GitHub
parent 0403a8633b
commit 8222810099
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 49 additions and 11 deletions

View File

@ -130,7 +130,7 @@ class SearchController < ApplicationController
result.error = I18n.t("rate_limiter.slow_down")
elsif site_overloaded?
result = GroupedSearchResults.new(
type_filter: search_args["type_filter"],
type_filter: search_args[:type_filter],
term: params[:term],
search_context: context
)

View File

@ -236,12 +236,13 @@ class Search
term: clean_term,
blurb_term: term,
search_context: @search_context,
blurb_length: @blurb_length
blurb_length: @blurb_length,
is_header_search: !use_full_page_limit
)
end
def limit
if @opts[:type_filter].present? && @opts[:type_filter] != "exclude_topics"
if use_full_page_limit
Search.per_filter + 1
else
Search.per_facet + 1
@ -260,6 +261,10 @@ class Search
@valid
end
def use_full_page_limit
@opts[:search_type] == :full_page || Topic === @search_context
end
def self.execute(term, opts = nil)
self.new(term, opts).execute
end

View File

@ -31,7 +31,7 @@ class Search
BLURB_LENGTH = 200
def initialize(type_filter:, term:, search_context:, blurb_length: nil, blurb_term: nil)
def initialize(type_filter:, term:, search_context:, blurb_length: nil, blurb_term: nil, is_header_search: false)
@type_filter = type_filter
@term = term
@blurb_term = blurb_term || term
@ -43,6 +43,7 @@ class Search
@tags = []
@groups = []
@error = nil
@is_header_search = is_header_search
end
def error=(error)
@ -85,10 +86,9 @@ class Search
def add(object)
type = object.class.to_s.downcase.pluralize
if @type_filter.present? && public_send(type).length == Search.per_filter
if !@is_header_search && public_send(type).length == Search.per_filter
@more_full_page_results = true
elsif !@type_filter.present? && public_send(type).length == Search.per_facet
elsif @is_header_search && public_send(type).length == Search.per_facet
instance_variable_set("@more_#{type}".to_sym, true)
else
(self.public_send(type)) << object

View File

@ -2183,24 +2183,26 @@ describe Search do
let!(:post3) { Fabricate(:post, raw: 'hello hello hello') }
let!(:post4) { Fabricate(:post, raw: 'hello hello') }
let!(:post5) { Fabricate(:post, raw: 'hello') }
before do
Search.stubs(:per_filter).returns(number_of_results)
end
it 'returns more results flag' do
results = Search.execute('hello', type_filter: 'topic')
results2 = Search.execute('hello', type_filter: 'topic', page: 2)
results = Search.execute('hello', search_type: :full_page, type_filter: 'topic')
results2 = Search.execute('hello', search_type: :full_page, type_filter: 'topic', page: 2)
expect(results.posts.length).to eq(number_of_results)
expect(results.posts.map(&:id)).to eq([post1.id, post2.id])
expect(results.more_full_page_results).to eq(true)
expect(results2.posts.length).to eq(number_of_results)
expect(results2.posts.map(&:id)).to eq([post3.id, post4.id])
expect(results2.more_full_page_results).to eq(true)
end
it 'correctly search with page parameter' do
search = Search.new('hello', type_filter: 'topic', page: 3)
search = Search.new('hello', search_type: :full_page, type_filter: 'topic', page: 3)
results = search.execute
expect(search.offset).to eq(2 * number_of_results)
@ -2209,6 +2211,36 @@ describe Search do
expect(results.more_full_page_results).to eq(nil)
end
it 'returns more results flag for header searches' do
results = Search.execute('hello', search_type: :header)
expect(results.posts.length).to eq(Search.per_facet)
expect(results.more_posts).to eq(nil) # not 6 posts yet
post6 = Fabricate(:post, raw: 'hello post #6')
results = Search.execute('hello', search_type: :header)
expect(results.posts.length).to eq(Search.per_facet)
expect(results.more_posts).to eq(true)
end
end
context 'header in-topic search' do
let!(:topic) { Fabricate(:topic, title: 'This is a topic with a bunch of posts') }
let!(:post1) { Fabricate(:post, topic: topic, raw: 'hola amiga') }
let!(:post2) { Fabricate(:post, topic: topic, raw: 'hola amigo') }
let!(:post3) { Fabricate(:post, topic: topic, raw: 'hola chica') }
let!(:post4) { Fabricate(:post, topic: topic, raw: 'hola chico') }
let!(:post5) { Fabricate(:post, topic: topic, raw: 'hola hermana') }
let!(:post6) { Fabricate(:post, topic: topic, raw: 'hola hermano') }
let!(:post7) { Fabricate(:post, topic: topic, raw: 'hola chiquito') }
it 'does not use per_facet pagination' do
search = Search.new('hola', search_type: :header, search_context: topic)
results = search.execute
expect(results.posts.length).to eq(7)
expect(results.more_posts).to eq(nil)
end
end
context 'in:tagged' do

View File

@ -30,9 +30,10 @@ describe 'groups' do
- `after:`: `yyyy-mm-dd`
- `order:`: `latest`, `likes`, `views`, `latest_topic`
- `assigned:`: username (without `@`)
- `in:`: `title`, `likes`, `personal`, `seen`, `unseen`, `posted`, `created`, `watching`, `tracking`, `bookmarks`, `assigned`, `unassigned`, `first`, `pinned`, `wiki`
- `in:`: `title`, `likes`, `personal`, `messages`, `seen`, `unseen`, `posted`, `created`, `watching`, `tracking`, `bookmarks`, `assigned`, `unassigned`, `first`, `pinned`, `wiki`
- `with:`: `images`
- `status:`: `open`, `closed`, `public`, `archived`, `noreplies`, `single_user`, `solved`, `unsolved`
- `group_messages:`: groupname
- `min_posts:`: 1
- `max_posts:`: 10
- `min_views:`: 1