Skip to content

Commit 628b05d

Browse files
jazairijazairi
authored
Merge pull request #57 from MITLibraries/rdi-144-highlights-in-results
Add highlighted search terms to results
2 parents 42d09a9 + 15abbf2 commit 628b05d

32 files changed

+435
-140
lines changed

‎app/assets/stylesheets/partials/_results.scss‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@
2727
}
2828
}
2929

30+
.result-highlights {
31+
font-size: $fs-small;
32+
ul {
33+
list-style: none;
34+
}
35+
}
36+
3037
.result-get {
3138
padding-top: 5px;
3239
font-size: 1.4rem;

‎app/helpers/search_helper.rb‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
module SearchHelper
2+
def displayed_fields
3+
['title', 'title.exact_value', 'content_type', 'dates.value', 'contributors.value']
4+
end
5+
6+
def trim_highlights(result)
7+
return unless result['highlight']&.any?
8+
9+
result['highlight'].reject { |h| displayed_fields.include? h['matchedField'] }
10+
end
11+
end

‎app/models/timdex_search.rb‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ class TimdexSearch < TimdexBase
4545
kind
4646
value
4747
}
48+
highlight {
49+
matchedField
50+
matchedPhrases
51+
}
4852
}
4953
aggregations {
5054
contentFormat {

‎app/views/search/_highlights.html.erb‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<% highlights = trim_highlights(result) %>
2+
<% return unless highlights&.any? %>
3+
4+
<h3 class="hd-6">Other fields matching your search terms:</h3>
5+
<ul>
6+
<% highlights.each do |h| %>
7+
<% h['matchedPhrases'].each do |phrase| %>
8+
<li><strong><%= h['matchedField'] %>:</strong> <%= sanitize(phrase, tags: ['span']) %></li>
9+
<% end %>
10+
<% end %>
11+
</ul>

‎app/views/search/_result.html.erb‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@
2222
<%= contributors.uniq.map { |contrib| link_to contrib, results_path({ advanced: true, contributors: contrib }) }.join(' ; ').html_safe %>
2323
</p>
2424

25+
<div class="result-highlights">
26+
<%= render partial: 'search/highlights', locals: { result: result } %>
27+
</div>
28+
2529
<div class="result-get">
2630
<%= link_to 'View online', "##{Random.rand(100000)}", class: 'button button-primary green' %>
2731
</div>

‎config/schema/schema.json‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,55 @@
566566
"enumValues": null,
567567
"possibleTypes": null
568568
},
569+
{
570+
"kind": "OBJECT",
571+
"name": "Highlight",
572+
"description": null,
573+
"fields": [
574+
{
575+
"name": "matchedField",
576+
"description": null,
577+
"args": [
578+
579+
],
580+
"type": {
581+
"kind": "SCALAR",
582+
"name": "String",
583+
"ofType": null
584+
},
585+
"isDeprecated": false,
586+
"deprecationReason": null
587+
},
588+
{
589+
"name": "matchedPhrases",
590+
"description": null,
591+
"args": [
592+
593+
],
594+
"type": {
595+
"kind": "LIST",
596+
"name": null,
597+
"ofType": {
598+
"kind": "NON_NULL",
599+
"name": null,
600+
"ofType": {
601+
"kind": "SCALAR",
602+
"name": "String",
603+
"ofType": null
604+
}
605+
}
606+
},
607+
"isDeprecated": false,
608+
"deprecationReason": null
609+
}
610+
],
611+
"inputFields": null,
612+
"interfaces": [
613+
614+
],
615+
"enumValues": null,
616+
"possibleTypes": null
617+
},
569618
{
570619
"kind": "OBJECT",
571620
"name": "Holding",
@@ -1540,6 +1589,28 @@
15401589
"isDeprecated": false,
15411590
"deprecationReason": null
15421591
},
1592+
{
1593+
"name": "highlight",
1594+
"description": null,
1595+
"args": [
1596+
1597+
],
1598+
"type": {
1599+
"kind": "LIST",
1600+
"name": null,
1601+
"ofType": {
1602+
"kind": "NON_NULL",
1603+
"name": null,
1604+
"ofType": {
1605+
"kind": "OBJECT",
1606+
"name": "Highlight",
1607+
"ofType": null
1608+
}
1609+
}
1610+
},
1611+
"isDeprecated": false,
1612+
"deprecationReason": null
1613+
},
15431614
{
15441615
"name": "holdings",
15451616
"description": null,
@@ -1994,6 +2065,20 @@
19942065
"isDeprecated": false,
19952066
"deprecationReason": null
19962067
},
2068+
{
2069+
"name": "score",
2070+
"description": null,
2071+
"args": [
2072+
2073+
],
2074+
"type": {
2075+
"kind": "SCALAR",
2076+
"name": "String",
2077+
"ofType": null
2078+
},
2079+
"isDeprecated": false,
2080+
"deprecationReason": null
2081+
},
19972082
{
19982083
"name": "source",
19992084
"description": null,
@@ -3414,7 +3499,7 @@
34143499
"deprecationReason": null
34153500
},
34163501
{
3417-
"name": "specifiedByUrl",
3502+
"name": "specifiedByURL",
34183503
"description": null,
34193504
"args": [
34203505

‎test/controllers/search_controller_test.rb‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,29 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
157157
end
158158
end
159159

160+
test 'results with valid query have query highlights' do
161+
VCR.use_cassette('data basic controller',
162+
allow_playback_repeats: true,
163+
match_requests_on: %i[method uri body]) do
164+
get '/results?q=data'
165+
assert_response :success
166+
assert_select '#results .result-highlights ul li', { minimum: 1 }
167+
end
168+
end
169+
170+
test 'highlights partial is not rendered for results with no relevant highlights' do
171+
VCR.use_cassette('advanced title data',
172+
allow_playback_repeats: true,
173+
match_requests_on: %i[method uri body]) do
174+
get '/results?title=data&advanced=true'
175+
assert_response :success
176+
177+
# We shouldn't see any highlighted terms because all of the matches will be on title, which is included in
178+
# SearchHelper#displayed_fields
179+
assert_select '#results .result-highlights ul li', { count: 0 }
180+
end
181+
end
182+
160183
test 'searches with zero results are handled gracefully' do
161184
VCR.use_cassette('timdex no results',
162185
allow_playback_repeats: true,

‎test/helpers/search_helper_test.rb‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
require 'test_helper'
2+
3+
class SearchHelperTest < ActionView::TestCase
4+
include SearchHelper
5+
6+
test 'removes displayed fields from highlights' do
7+
result = { 'highlight' => [{ 'matchedField' => 'title', 'matchedPhrases' => 'Very important data' },
8+
{ 'matchedField' => 'title.exact_value', 'matchedPhrases' => 'Very important data' },
9+
{ 'matchedField' => 'content_type', 'matchedPhrases' => 'Dataset' },
10+
{ 'matchedField' => 'dates.value', 'matchedPhrases' => '2022' },
11+
{ 'matchedField' => 'contributors.value', 'matchedPhrases' => 'Jane Datascientist' }] }
12+
assert_empty trim_highlights(result)
13+
end
14+
15+
test 'does not remove undisplayed fields from highlights' do
16+
result = { 'highlight' => [{ 'matchedField' => 'summary', 'matchedPhrases' => 'Have some data' }] }
17+
assert_equal [{ 'matchedField' => 'summary', 'matchedPhrases' => 'Have some data' }], trim_highlights(result)
18+
end
19+
20+
test 'returns correct set of highlights when result includes displayed and undisplayed fields' do
21+
result = { 'highlight' => [{ 'matchedField' => 'title', 'matchedPhrases' => 'Very important data' },
22+
{ 'matchedField' => 'content_type', 'matchedPhrases' => 'Dataset' },
23+
{ 'matchedField' => 'summary', 'matchedPhrases' => '2022' },
24+
{ 'matchedField' => 'citation', 'matchedPhrases' => 'Datascientist, Jane' }] }
25+
assert_equal [{ 'matchedField' => 'summary', 'matchedPhrases' => '2022' },
26+
{ 'matchedField' => 'citation', 'matchedPhrases' => 'Datascientist, Jane' }], trim_highlights(result)
27+
end
28+
end

‎test/integration/error_resilience_test.rb‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
require 'test_helper'
22

33
class ErrorResilienceTest < ActionDispatch::IntegrationTest
4+
# When regenerating cassettes, timdex_error.yml needs to be manually edited so the response status code is 500 and the
5+
# message is Internal Server Error.
46
test 'search results page renders with an error message if the API errors' do
57
VCR.use_cassette('timdex error',
68
allow_playback_repeats: true,

‎test/models/analyzer_test.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,15 @@ class AnalyzerTest < ActiveSupport::TestCase
4040
end
4141
end
4242

43+
# When regenerating cassettes, the page param of the eq hash in this test must be updated to match the current last
44+
# page returned by the API.
4345
test 'analyzer pagination does not include last page value on last page of results' do
4446
VCR.use_cassette('data last page',
4547
allow_playback_repeats: true,
4648
match_requests_on: %i[method uri body]) do
4749
eq = {
4850
q: 'data',
49-
page: 17
51+
page: 28
5052
}
5153
query = { 'q' => 'data', 'from' => '320' }
5254
response = TimdexBase::Client.query(TimdexSearch::Query, variables: query)

0 commit comments

Comments
 (0)