Skip to content

Commit 46b581a

Browse files
author
Alice Bartlett
committed
Expose unscoped results to mustache view
This commit: - adds methods to `scoped_search_results_presenter` to show unscoped results from rummager in the view. - Amends `search_results_presenter` to put the `.to_hash` call inside the `.map` on the result builder. So now whenever `results` is called the returned array is read to be sent to the view. This is because we call `results` from `scoped_search_results_presenter` now too. - Adds a template for unscoped results - adds tests for `scoped_search_results_presenter` `presentable_result_list` is the starting point for the biggest change here. I'm not sure the implementation choices here are very obvious so I'll explain them. The design (results with an embedded list of other results) is difficult to represent semantically in HTML. Ideally the extra results would be in an `<aside>` tag outside of the result list, but the design has them interrupting the list so that sighted users will notice them. A compromise that I'm happy with is to have them nested in the list of results like so: ``` <li>result 1</li> <li>result 2</li> <li>result 3</li> <li>More results from GOV.UK <ol> <li>..</li> <li>..</li> <li>..</li> </ol> </li> <li>result 4</li> etc ``` Given this markup, and the constraint of using a logic-less templating language (mustache) the best way I could thing to achieve this HTML was to pass mustache an object that contains this structure. The responsibility for mashing up the scoped results and unscoped results falls to `presentable_result_list`. This software pattern is a bit uncomfortable because it pushes the design of the interface ("the unscoped results should be nested") onto the presenter, though it should be the view's responsibility. However it is the only way I can think to write it.
1 parent 01e73e1 commit 46b581a

File tree

5 files changed

+172
-5
lines changed

5 files changed

+172
-5
lines changed

Diff for: app/presenters/scoped_search_results_presenter.rb

+35-1
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,50 @@ def to_hash
44
super.merge({
55
is_scoped?: true,
66
scope_title: scope_title,
7+
unscoped_results_any?: unscoped_results.any?,
8+
unscoped_result_count: result_count_string(unscoped_result_count),
79
})
810
end
911

1012
private
1113

14+
attr_reader :unscoped_results
15+
1216
def filter_fields
17+
[]
1318
end
1419

1520
def scope_title
16-
search_response[:scope][:title]
21+
search_response["scope"]["title"]
22+
end
23+
24+
def results
25+
result_list = search_response["results"].map { |result| build_scoped_result(result).to_hash }
26+
27+
if unscoped_results.any?
28+
insertion_point = [result_list.count, 3].min
29+
unscoped_results_sublist = { results: unscoped_results, is_multiple_results: true }
30+
31+
result_list.insert(insertion_point, unscoped_results_sublist)
32+
end
33+
result_list
34+
end
35+
36+
def unscoped_result_count
37+
search_response["unscoped_results"]["total"]
38+
end
39+
40+
def unscoped_results
41+
@unscoped_results ||= build_result_presenters
42+
43+
end
44+
45+
def build_result_presenters
46+
search_response["unscoped_results"]["results"].map { |result| build_result(result).to_hash }
47+
end
48+
49+
def build_scoped_result(result)
50+
ScopedResult.new(search_parameters, result)
1751
end
1852

1953
end

Diff for: app/presenters/search_results_presenter.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def to_hash
1919
result_count: result_count,
2020
result_count_string: result_count_string(result_count),
2121
results_any?: results.any?,
22-
results: results.map { |result| result.to_hash },
22+
results: results,
2323
filter_fields: filter_fields,
2424
debug_score: search_parameters.debug_score,
2525
has_next_page?: has_next_page?,
@@ -71,7 +71,7 @@ def result_count_string(count)
7171
end
7272

7373
def results
74-
search_response["results"].map { |result| build_result(result) }
74+
search_response["results"].map { |result| build_result(result).to_hash }
7575
end
7676

7777
def build_result(result)

Diff for: app/views/search/_results_list.mustache

+9-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
{{#is_scoped?}}
33
<strong>{{ result_count_string }} found in</strong><br>
44
{{scope_title}}<br>
5-
<a href='/search?q={{query}}'>Display results from all of GOV.UK</a>
5+
{{#unscoped_results_any?}}
6+
<a href='/search?q={{query}}'>Display {{unscoped_result_count}} from all of GOV.UK</a>
7+
{{/unscoped_results_any?}}
68
{{/is_scoped?}}
79
{{^is_scoped?}}
810
{{ result_count_string }} found
@@ -12,7 +14,12 @@
1214
{{#results_any?}}
1315
<ol class="results-list{{#debug_score}} debug{{/debug_score}}" id="js-live-search-results" start="{{first_result_number}}">
1416
{{#results}}
15-
{{>search/_result}}
17+
{{#is_multiple_results}}
18+
{{>search/_sublist}}
19+
{{/is_multiple_results}}
20+
{{^is_multiple_results}}
21+
{{>search/_result}}
22+
{{/is_multiple_results}}
1623
{{/results}}
1724
</ol>
1825

Diff for: app/views/search/_sublist.mustache

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<li class='descoped-results'>
2+
<div class='descope-message'>
3+
<a href='/search?q={{query}}'>Display {{unscoped_result_count}} from all of GOV.UK</a>
4+
</div>
5+
<ol>
6+
{{#results}}
7+
{{>search/_result}}
8+
{{/results}}
9+
</ol>
10+
</li>
11+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
require_relative "../../test_helper"
2+
3+
class ScopedSearchResultsPresenterTest < ActiveSupport::TestCase
4+
setup do
5+
@scope_title = stub
6+
@unscoped_result_count = stub
7+
8+
@scoped_results = [{"title"=> "scoped_result_1"},
9+
{"title"=> "scoped_result_2"},
10+
{"title"=> "scoped_result_3"},
11+
{"title"=> "scoped_result_4"},
12+
]
13+
@unscoped_results = [{"title"=> "unscoped_result_1"},
14+
{"title"=> "unscoped_result_2"},
15+
{"title"=> "unscoped_result_3"},
16+
]
17+
18+
@search_response = { "result_count" => "50",
19+
"results" => @scoped_results,
20+
"scope" => {
21+
"title" => @scope_title,
22+
},
23+
"unscoped_results" => {
24+
"total" => @unscoped_result_count,
25+
"results" => @unscoped_results,
26+
},
27+
}
28+
29+
30+
@search_parameters = stub(search_term: 'words',
31+
debug_score: 1,
32+
start: 1,
33+
count: 1,
34+
build_link: 1,
35+
)
36+
end
37+
38+
should "return a hash that has is_scoped set to true" do
39+
results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters)
40+
assert_equal true, results.to_hash[:is_scoped?]
41+
end
42+
43+
should "return a hash with the scope_title set to the scope title from the @search_response" do
44+
results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters)
45+
assert_equal @scope_title, results.to_hash[:scope_title]
46+
end
47+
48+
should "return a hash result count set to the scope title from the @search_response" do
49+
results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters)
50+
assert_equal "#{@unscoped_result_count} results", results.to_hash[:unscoped_result_count]
51+
end
52+
53+
context "presentable result list" do
54+
55+
should "return all scoped results with unscoped results inserted at position 4" do
56+
results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters).to_hash
57+
58+
##
59+
# This test is asserting that the format of `presentable_list` is:
60+
# [result, result, result, {results: list_of_results, is_multiple_results: true}, result ...]
61+
# Where list_of_results are the top three results from an unscoped request to rummager
62+
# and a flag `is_multiple_results` set to true.
63+
##
64+
65+
simplified_expected_results_list = [{ "title"=> "scoped_result_1" },
66+
{ "title"=> "scoped_result_2" },
67+
{ "title"=> "scoped_result_3" },
68+
{ "is_multiple_results" => true,
69+
"results" => [{ "title"=> "unscoped_result_1" },
70+
{ "title"=> "unscoped_result_2" },
71+
{ "title"=> "unscoped_result_3" },
72+
]
73+
},
74+
{ "title"=> "scoped_result_4" },
75+
]
76+
77+
78+
# Scoped results
79+
simplified_expected_results_list[0..2].each_with_index do | result, i |
80+
assert_equal result["title"], results[:results][i][:title]
81+
end
82+
83+
# Check un-scoped sub-list has flag
84+
assert_equal true, results[:results][3][:is_multiple_results]
85+
86+
# iterate unscoped sublist of results
87+
simplified_expected_results_list[3]["results"].each_with_index do | result, i |
88+
assert_equal result["title"], results[:results][3][:results][i][:title]
89+
end
90+
91+
# check remaining result
92+
assert_equal simplified_expected_results_list[4]["title"], results[:results][4][:title]
93+
end
94+
end
95+
96+
context "no scoped results returned" do
97+
setup do
98+
@no_results = []
99+
@search_response["unscoped_results"]["results"] = @no_results
100+
end
101+
102+
should "not not include unscoped results in the presentable_list if there aren't any" do
103+
results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters).to_hash
104+
105+
@scoped_results.each_with_index do | result, i |
106+
assert_equal result["title"], results[:results][i][:title]
107+
end
108+
end
109+
110+
should "not set unscoped_results_any? to false" do
111+
results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters).to_hash
112+
assert_equal false, results.to_hash[:unscoped_results_any?]
113+
end
114+
end
115+
end

0 commit comments

Comments
 (0)