From dae0bb4c67c9f7f1eb475fb60f375118f93cdfd9 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 26 Mar 2019 17:01:19 +0800 Subject: [PATCH] FIX: Post blurb incorrect when search contains a phrase match. If the blurb generated is not around the search term, we will not be able to highlight it on the client side. --- .../lib/concerns/search-constants.js.es6.erb | 1 + .../discourse/lib/highlight-text.js.es6 | 8 ++++- lib/search.rb | 4 ++- lib/search/grouped_search_results.rb | 11 +++++- spec/components/search_spec.rb | 35 ++++++++++++++----- 5 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 app/assets/javascripts/discourse/lib/concerns/search-constants.js.es6.erb diff --git a/app/assets/javascripts/discourse/lib/concerns/search-constants.js.es6.erb b/app/assets/javascripts/discourse/lib/concerns/search-constants.js.es6.erb new file mode 100644 index 00000000000..4cb005cc687 --- /dev/null +++ b/app/assets/javascripts/discourse/lib/concerns/search-constants.js.es6.erb @@ -0,0 +1 @@ +export const PHRASE_MATCH_REGEXP_PATTERN = '<%= Search::PHRASE_MATCH_REGEXP_PATTERN %>'; diff --git a/app/assets/javascripts/discourse/lib/highlight-text.js.es6 b/app/assets/javascripts/discourse/lib/highlight-text.js.es6 index c9af7689a4b..8a0d4d48aef 100644 --- a/app/assets/javascripts/discourse/lib/highlight-text.js.es6 +++ b/app/assets/javascripts/discourse/lib/highlight-text.js.es6 @@ -1,7 +1,13 @@ +import { PHRASE_MATCH_REGEXP_PATTERN } from "discourse/lib/concerns/search-constants"; + export default function($elem, term) { if (!_.isEmpty(term)) { // special case ignore "l" which is used for magic sorting - let words = _.reject(term.match(/"[^"]+"|[^\s]+/g), t => t === "l"); + let words = _.reject( + term.match(new RegExp(`${PHRASE_MATCH_REGEXP_PATTERN}|[^\s]+`, "g")), + t => t === "l" + ); + words = words.map(w => w.replace(/^"(.*)"$/, "$1")); $elem.highlight(words, { className: "search-highlight", wordsOnly: true }); } diff --git a/lib/search.rb b/lib/search.rb index 2721f662557..d89e231560f 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -727,6 +727,8 @@ class Search end end + PHRASE_MATCH_REGEXP_PATTERN = '"([^"]+)"' + def posts_query(limit, opts = nil) opts ||= {} @@ -770,7 +772,7 @@ class Search # D is for cooked weights = @in_title ? 'A' : (SiteSetting.tagging_enabled ? 'ABCD' : 'ABD') posts = posts.where("post_search_data.search_data @@ #{ts_query(weight_filter: weights)}") - exact_terms = @term.scan(/"([^"]+)"/).flatten + exact_terms = @term.scan(Regexp.new(PHRASE_MATCH_REGEXP_PATTERN)).flatten exact_terms.each do |exact| posts = posts.where("posts.raw ilike :exact OR topics.title ilike :exact", exact: "%#{exact}%") diff --git a/lib/search/grouped_search_results.rb b/lib/search/grouped_search_results.rb index 68c257f3635..ffe5dcacf56 100644 --- a/lib/search/grouped_search_results.rb +++ b/lib/search/grouped_search_results.rb @@ -70,7 +70,16 @@ class Search if term terms = term.split(/\s+/) - blurb = TextHelper.excerpt(cooked, terms.first, radius: blurb_length / 2, seperator: " ") + phrase = terms.first + + if phrase =~ Regexp.new(Search::PHRASE_MATCH_REGEXP_PATTERN) + phrase = Regexp.last_match[1] + end + + blurb = TextHelper.excerpt(cooked, phrase, + radius: blurb_length / 2, + seperator: " " + ) end blurb = TextHelper.truncate(cooked, length: blurb_length, seperator: " ") if blurb.blank? diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index a3b8674fa16..18dd5d9022e 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -304,16 +304,35 @@ describe Search do end context 'searching for a post' do - let!(:reply) { Fabricate(:basic_reply, topic: topic, user: topic.user) } - let(:result) { Search.execute('quotes', type_filter: 'topic', include_blurbs: true) } + let!(:reply) do + Fabricate(:post_with_long_raw_content, + topic: topic, + user: topic.user, + ).tap { |post| post.update!(raw: "#{post.raw} elephant") } + end + + let(:expected_blurb) do + "...to satisfy any test conditions that require content longer than the typical test post raw content. elephant" + end it 'returns the post' do - expect(result).to be_present - expect(result.posts.length).to eq(1) - p = result.posts[0] - expect(p.topic.id).to eq(topic.id) - expect(p.id).to eq(reply.id) - expect(result.blurb(p)).to eq("this reply has no quotes") + result = Search.execute('elephant', + type_filter: 'topic', + include_blurbs: true + ) + + expect(result.posts).to contain_exactly(reply) + expect(result.blurb(reply)).to eq(expected_blurb) + end + + it 'returns the right post and blurb for searches with phrase' do + result = Search.execute('"elephant"', + type_filter: 'topic', + include_blurbs: true + ) + + expect(result.posts).to contain_exactly(reply) + expect(result.blurb(reply)).to eq(expected_blurb) end end