FEATURE: allow searching public topics and personal messages simultaneously (#8784)

The new search modifier `in:all` can be used to include both public and personal messages in the same search.

Co-authored-by: adam j hartz <hz@mit.edu>
This commit is contained in:
adam j hartz 2020-01-28 05:11:33 -05:00 committed by David Taylor
parent 58d6ee36ee
commit e0605029dc
2 changed files with 98 additions and 7 deletions

View File

@ -28,7 +28,7 @@ class Search
end end
def self.facets def self.facets
%w(topic category user private_messages tags) %w(topic category user private_messages tags all_topics)
end end
def self.ts_config(locale = SiteSetting.default_locale) def self.ts_config(locale = SiteSetting.default_locale)
@ -176,6 +176,10 @@ class Search
@search_context = @guardian.user @search_context = @guardian.user
end end
if @search_all_topics
@opts[:type_filter] = "all_topics"
end
@results = GroupedSearchResults.new( @results = GroupedSearchResults.new(
@opts[:type_filter], @opts[:type_filter],
clean_term, clean_term,
@ -233,7 +237,7 @@ class Search
end end
# If the term is a number or url to a topic, just include that topic # If the term is a number or url to a topic, just include that topic
if @opts[:search_for_id] && (@results.type_filter == 'topic' || @results.type_filter == 'private_messages') if @opts[:search_for_id] && ['topic', 'private_messages', 'all_topics'].include?(@results.type_filter)
if @term =~ /^\d+$/ if @term =~ /^\d+$/
single_topic(@term.to_i) single_topic(@term.to_i)
else else
@ -666,6 +670,9 @@ class Search
elsif word == 'order:likes' elsif word == 'order:likes'
@order = :likes @order = :likes
nil nil
elsif word == 'in:all'
@search_all_topics = true
nil
elsif %w{in:private in:personal}.include?(word) # remove private after 2.4 release elsif %w{in:private in:personal}.include?(word) # remove private after 2.4 release
@search_pms = true @search_pms = true
nil nil
@ -813,12 +820,17 @@ class Search
posts = posts.where("topics.visible") unless is_topic_search posts = posts.where("topics.visible") unless is_topic_search
if opts[:private_messages] || (is_topic_search && @search_context.private_message?) if opts[:type_filter] === "private_messages" || (is_topic_search && @search_context.private_message?)
posts = posts.where("topics.archetype = ?", Archetype.private_message) posts = posts.where("topics.archetype = ?", Archetype.private_message)
unless @guardian.is_admin? unless @guardian.is_admin?
posts = posts.private_posts_for_user(@guardian.user) posts = posts.private_posts_for_user(@guardian.user)
end end
elsif opts[:type_filter] === "all_topics"
private_posts = posts.where("topics.archetype = ?", Archetype.private_message)
private_posts = private_posts.private_posts_for_user(@guardian.user) unless @guardian.is_admin?
posts = posts.where("topics.archetype <> ?", Archetype.private_message).or(private_posts)
else else
posts = posts.where("topics.archetype <> ?", Archetype.private_message) posts = posts.where("topics.archetype <> ?", Archetype.private_message)
end end
@ -864,7 +876,7 @@ class Search
posts = posts =
if @search_context.present? if @search_context.present?
if @search_context.is_a?(User) if @search_context.is_a?(User)
if opts[:private_messages] if opts[:type_filter] === "private_messages"
@direct_pms_only ? posts : posts.private_posts_for_user(@search_context) @direct_pms_only ? posts : posts.private_posts_for_user(@search_context)
else else
posts.where("posts.user_id = #{@search_context.id}") posts.where("posts.user_id = #{@search_context.id}")
@ -1018,10 +1030,10 @@ class Search
query = query =
if @order == :likes if @order == :likes
# likes are a pain to aggregate so skip # likes are a pain to aggregate so skip
posts_query(limit, private_messages: opts[:private_messages]) posts_query(limit, type_filter: opts[:type_filter])
.select('topics.id', "posts.post_number") .select('topics.id', "posts.post_number")
else else
posts_query(limit, aggregate_search: true, private_messages: opts[:private_messages]) posts_query(limit, aggregate_search: true, type_filter: opts[:type_filter])
.select('topics.id', "#{min_or_max}(posts.post_number) post_number") .select('topics.id', "#{min_or_max}(posts.post_number) post_number")
.group('topics.id') .group('topics.id')
end end
@ -1064,7 +1076,11 @@ class Search
def private_messages_search def private_messages_search
raise Discourse::InvalidAccess.new("anonymous can not search PMs") unless @guardian.user raise Discourse::InvalidAccess.new("anonymous can not search PMs") unless @guardian.user
aggregate_search(private_messages: true) aggregate_search(type_filter: "private_messages")
end
def all_topics_search
aggregate_search(type_filter: "all_topics")
end end
def topic_search def topic_search

View File

@ -294,6 +294,81 @@ describe Search do
expect(results.posts.size).to eq(0) expect(results.posts.size).to eq(0)
end end
end end
context 'all topics' do
let!(:u1) { Fabricate(:user, username: 'fred', name: 'bob jones', email: 'foo+1@bar.baz') }
let!(:u2) { Fabricate(:user, username: 'bob', name: 'fred jones', email: 'foo+2@bar.baz') }
let!(:u3) { Fabricate(:user, username: 'jones', name: 'bob fred', email: 'foo+3@bar.baz') }
let!(:u4) { Fabricate(:user, username: 'alice', name: 'bob fred', email: 'foo+4@bar.baz', admin: true) }
let!(:public_topic) { Fabricate(:topic, user: u1) }
let!(:public_post1) { Fabricate(:post, topic: public_topic, raw: "what do you want for breakfast? ham and eggs?", user: u1) }
let!(:public_post2) { Fabricate(:post, topic: public_topic, raw: "ham and spam", user: u2) }
let!(:private_topic) { Fabricate(:topic, user: u1, category_id: nil, archetype: 'private_message') }
let!(:private_post1) { Fabricate(:post, topic: private_topic, raw: "what do you want for lunch? ham and cheese?", user: u1) }
let!(:private_post2) { Fabricate(:post, topic: private_topic, raw: "cheese and spam", user: u2) }
it 'finds private messages' do
TopicAllowedUser.create!(user_id: u1.id, topic_id: private_topic.id)
TopicAllowedUser.create!(user_id: u2.id, topic_id: private_topic.id)
# private only
results = Search.execute('cheese',
type_filter: 'all_topics',
guardian: Guardian.new(u1))
expect(results.posts.length).to eq(1)
# public only
results = Search.execute('eggs',
type_filter: 'all_topics',
guardian: Guardian.new(u1))
expect(results.posts.length).to eq(1)
# both
results = Search.execute('spam',
type_filter: 'all_topics',
guardian: Guardian.new(u1))
expect(results.posts.length).to eq(2)
# nonparticipatory user
results = Search.execute('cheese',
type_filter: 'all_topics',
guardian: Guardian.new(u3))
expect(results.posts.length).to eq(0)
results = Search.execute('eggs',
type_filter: 'all_topics',
guardian: Guardian.new(u3))
expect(results.posts.length).to eq(1)
results = Search.execute('spam',
type_filter: 'all_topics',
guardian: Guardian.new(u3))
expect(results.posts.length).to eq(1)
# Admin
results = Search.execute('spam',
type_filter: 'all_topics',
guardian: Guardian.new(u4))
expect(results.posts.length).to eq(2)
# same keyword for different users
results = Search.execute('ham',
type_filter: 'all_topics',
guardian: Guardian.new(u1))
expect(results.posts.length).to eq(2)
results = Search.execute('ham',
type_filter: 'all_topics',
guardian: Guardian.new(u2))
expect(results.posts.length).to eq(2)
results = Search.execute('ham',
type_filter: 'all_topics',
guardian: Guardian.new(u3))
expect(results.posts.length).to eq(1)
end
end
end end
context 'topics' do context 'topics' do