diff --git a/app/assets/stylesheets/views/_search.scss b/app/assets/stylesheets/views/_search.scss index a80ff8d401..6290a05c6e 100644 --- a/app/assets/stylesheets/views/_search.scss +++ b/app/assets/stylesheets/views/_search.scss @@ -228,6 +228,20 @@ main.search { } } + li.descoped-results { + border-left: 4px solid $govuk-blue; + padding: $gutter-half 0 0 $gutter-half; + margin-bottom: $gutter; + + .descope-message { + @include core-16; + } + + ol { + padding: $gutter-half 0 0; + } + } + h3 { @include core-24; margin: 0; diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index df3d6794a0..9f46cc4081 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -17,7 +17,13 @@ def index search_response = search_client.search(search_params) @search_term = search_params.search_term - @results = SearchResultsPresenter.new(search_response, search_params) + + if (search_response["scope"].present?) + @results = ScopedSearchResultsPresenter.new(search_response, search_params) + else + @results = SearchResultsPresenter.new(search_response, search_params) + end + @facets = search_response["facets"] @spelling_suggestion = @results.spelling_suggestion diff --git a/app/presenters/scoped_search_results_presenter.rb b/app/presenters/scoped_search_results_presenter.rb new file mode 100644 index 0000000000..3dc869b8d7 --- /dev/null +++ b/app/presenters/scoped_search_results_presenter.rb @@ -0,0 +1,53 @@ +class ScopedSearchResultsPresenter < SearchResultsPresenter + + def to_hash + super.merge({ + is_scoped?: true, + scope_title: scope_title, + unscoped_results_any?: unscoped_results.any?, + unscoped_result_count: result_count_string(unscoped_result_count), + }) + end + +private + + attr_reader :unscoped_results + + def filter_fields + [] + end + + def scope_title + search_response["scope"]["title"] + end + + def results + result_list = search_response["results"].map { |result| build_scoped_result(result).to_hash } + + if unscoped_results.any? + insertion_point = [result_list.count, 3].min + unscoped_results_sublist = { results: unscoped_results, is_multiple_results: true } + + result_list.insert(insertion_point, unscoped_results_sublist) + end + result_list + end + + def unscoped_result_count + search_response["unscoped_results"]["total"] + end + + def unscoped_results + @unscoped_results ||= build_result_presenters + + end + + def build_result_presenters + search_response["unscoped_results"]["results"].map { |result| build_result(result).to_hash } + end + + def build_scoped_result(result) + ScopedResult.new(search_parameters, result) + end + +end diff --git a/app/presenters/search_result.rb b/app/presenters/search_result.rb index def82cc5fc..de8fb50f38 100644 --- a/app/presenters/search_result.rb +++ b/app/presenters/search_result.rb @@ -35,18 +35,18 @@ def self.result_accessor(*keys) end end - result_accessor :link, :title, :format, :es_score + result_accessor :link, :title, :format, :es_score, :section, :subsection, :subsubsection - # Avoid the mundanity of creating these all by hand by making - # dynamic method and accessors. - %w(section subsection subsubsection).each do |key| - define_method "formatted_#{key}_name" do - mapped_name(send(key)) || humanized_name(send(key)) - end + def formatted_section_name + mapped_name(section) || humanized_name(section) + end - define_method key do - result[key] - end + def formatted_subsection_name + mapped_name(subsection) || humanized_name(subsection) + end + + def formatted_subsubsection_name + mapped_name(subsubsection) || humanized_name(subsubsection) end # External links have a truncated version of their URLs displayed on the diff --git a/app/presenters/search_results_presenter.rb b/app/presenters/search_results_presenter.rb index 1f48a1ba99..f38b0629b5 100644 --- a/app/presenters/search_results_presenter.rb +++ b/app/presenters/search_results_presenter.rb @@ -17,9 +17,9 @@ def to_hash { query: search_parameters.search_term, result_count: result_count, - result_count_string: result_count_string, + result_count_string: result_count_string(result_count), results_any?: results.any?, - results: results.map { |result| result.to_hash }, + results: results, filter_fields: filter_fields, debug_score: search_parameters.debug_score, has_next_page?: has_next_page?, @@ -29,25 +29,19 @@ def to_hash previous_page_link: previous_page_link, previous_page_label: previous_page_label, first_result_number: (search_parameters.start + 1), - is_scoped?: is_scoped?, - scope_title: scope_title, } end def filter_fields - if is_scoped? - [] - else - search_response["facets"].map do |field, value| - external = SearchParameters::external_field_name(field) - facet_params = search_parameters.filter(external) - facet = SearchFacetPresenter.new(value, facet_params) - { - field: external, - field_title: FACET_TITLES.fetch(field, field), - options: facet.to_hash, - } - end + search_response["facets"].map do |field, value| + external = SearchParameters::external_field_name(field) + facet_params = search_parameters.filter(external) + facet = SearchFacetPresenter.new(value, facet_params) + { + field: external, + field_title: FACET_TITLES.fetch(field, field), + options: facet.to_hash, + } end end @@ -72,18 +66,16 @@ def result_count search_response["total"].to_i end - def result_count_string - pluralize(number_with_delimiter(result_count), "result") + def result_count_string(count) + pluralize(number_with_delimiter(count), "result") end def results - search_response["results"].map { |result| build_result(result) } + search_response["results"].map { |result| build_result(result).to_hash } end def build_result(result) - if is_scoped? - ScopedResult.new(search_parameters, result) - elsif result["document_type"] == "group" + if result["document_type"] == "group" GroupResult.new(search_parameters, result) elsif result["document_type"] && result["document_type"] != "edition" NonEditionResult.new(search_parameters, result) @@ -127,16 +119,6 @@ def previous_page_label end end - def is_scoped? - search_response[:scope].present? - end - - def scope_title - if is_scoped? - search_response[:scope][:title] - end - end - private attr_reader :search_parameters, :search_response diff --git a/app/views/search/_result.mustache b/app/views/search/_result.mustache new file mode 100644 index 0000000000..84222643f9 --- /dev/null +++ b/app/views/search/_result.mustache @@ -0,0 +1,85 @@ + +

{{title}}

+ + {{#debug_score}} +

+ {{link}} +

+

+ Score: {{es_score}} + Format: {{#government}}government{{/government}} {{format}} +

+ {{/debug_score}} + + {{#external}} +

+ Part of + {{ display_link }} +

+ {{/external}} + + {{#section}} +

+ Part of + {{formatted_section_name}} + {{#formatted_subsection_name}} + , + {{ formatted_subsection_name }} + {{/formatted_subsection_name}} + {{#formatted_subsubsection_name}} + , + {{ formatted_subsubsection_name }} + {{/formatted_subsubsection_name}} +

+ {{/section}} + + {{#metadata_any?}} + + {{/metadata_any?}} + + {{#historic?}} + {{#government_name}} +

+ First published during the {{government_name}} +

+ {{/government_name}} + {{/historic?}} + +

{{description}}

+ + {{#sections_present?}} + + {{/sections_present?}} + + {{#examples_present?}} + + {{/examples_present?}} + + {{^examples_present?}} + {{#suggested_filter_present?}} +

{{suggested_filter_title}}

+ {{/suggested_filter_present?}} + {{/examples_present?}} + + diff --git a/app/views/search/_results_list.mustache b/app/views/search/_results_list.mustache index b34c41b1ac..369885bff0 100644 --- a/app/views/search/_results_list.mustache +++ b/app/views/search/_results_list.mustache @@ -2,7 +2,9 @@ {{#is_scoped?}} {{ result_count_string }} found in
{{scope_title}}
- Display results from all of GOV.UK + {{#unscoped_results_any?}} + Display {{unscoped_result_count}} from all of GOV.UK + {{/unscoped_results_any?}} {{/is_scoped?}} {{^is_scoped?}} {{ result_count_string }} found @@ -12,91 +14,12 @@ {{#results_any?}}
    {{#results}} - -

    {{title}}

    - - {{#debug_score}} -

    - {{link}} -

    -

    - Score: {{es_score}} - Format: {{#government}}government{{/government}} {{format}} -

    - {{/debug_score}} - - {{#external}} -

    - Part of - {{ display_link }} -

    - {{/external}} - - {{#section}} -

    - Part of - {{formatted_section_name}} - {{#formatted_subsection_name}} - , - {{ formatted_subsection_name }} - {{/formatted_subsection_name}} - {{#formatted_subsubsection_name}} - , - {{ formatted_subsubsection_name }} - {{/formatted_subsubsection_name}} -

    - {{/section}} - - {{#metadata_any?}} -
      - {{#metadata}} -
    • {{{.}}}
      • - {{/metadata}} -
    - {{/metadata_any?}} - - {{#historic?}} - {{#government_name}} -

    - First published during the {{government_name}} -

    - {{/government_name}} - {{/historic?}} - -

    {{description}}

    - - {{#sections_present?}} -
      - {{#sections}} -
    • {{title}}
      • - {{/sections}} -
    - {{/sections_present?}} - - {{#examples_present?}} - - {{/examples_present?}} - - {{^examples_present?}} - {{#suggested_filter_present?}} -

    {{suggested_filter_title}}

    - {{/suggested_filter_present?}} - {{/examples_present?}} - - + {{#is_multiple_results}} + {{>search/_sublist}} + {{/is_multiple_results}} + {{^is_multiple_results}} + {{>search/_result}} + {{/is_multiple_results}} {{/results}}

diff --git a/app/views/search/_sublist.mustache b/app/views/search/_sublist.mustache new file mode 100644 index 0000000000..df8d0bef78 --- /dev/null +++ b/app/views/search/_sublist.mustache @@ -0,0 +1,11 @@ +
  • +
    + Display {{unscoped_result_count}} from all of GOV.UK +
    +
      + {{#results}} + {{>search/_result}} + {{/results}} +
    +
    • + diff --git a/lib/search_api.rb b/lib/search_api.rb index b3e1bf41ca..9ba89a0f20 100644 --- a/lib/search_api.rb +++ b/lib/search_api.rb @@ -31,9 +31,10 @@ def search_results def scope_info if is_scoped? && scope_object.present? { - scope: { - title: scope_object.title, - } + "scope" => { + "title" => scope_object.title, + }, + "unscoped_results" => unscoped_results, } else {} @@ -45,7 +46,7 @@ def rummager_params end def scope_object - @scope_object ||= api.unified_search(filter_link: scope_object_link, count: 1, fields: %w{title}).results.first + @scope_object ||= api.unified_search(filter_link: scope_object_link, count: "1", fields: %w{title}).results.first end def is_scoped? @@ -53,7 +54,15 @@ def is_scoped? end def scope_object_link - params.filter('manual').first + @scope_object_link ||= params.filter('manual').first + end + + def unscoped_results + @unscoped_results ||= api.unified_search(unscoped_rummager_request).to_hash + end + + def unscoped_rummager_request + rummager_params.except(:filter_manual).merge(count: "3", reject_manual: scope_object_link) end end end diff --git a/test/unit/presenters/scoped_search_results_presenter_test.rb b/test/unit/presenters/scoped_search_results_presenter_test.rb new file mode 100644 index 0000000000..35b2e1eff6 --- /dev/null +++ b/test/unit/presenters/scoped_search_results_presenter_test.rb @@ -0,0 +1,115 @@ +require_relative "../../test_helper" + +class ScopedSearchResultsPresenterTest < ActiveSupport::TestCase + setup do + @scope_title = stub + @unscoped_result_count = stub + + @scoped_results = [{"title"=> "scoped_result_1"}, + {"title"=> "scoped_result_2"}, + {"title"=> "scoped_result_3"}, + {"title"=> "scoped_result_4"}, + ] + @unscoped_results = [{"title"=> "unscoped_result_1"}, + {"title"=> "unscoped_result_2"}, + {"title"=> "unscoped_result_3"}, + ] + + @search_response = { "result_count" => "50", + "results" => @scoped_results, + "scope" => { + "title" => @scope_title, + }, + "unscoped_results" => { + "total" => @unscoped_result_count, + "results" => @unscoped_results, + }, + } + + + @search_parameters = stub(search_term: 'words', + debug_score: 1, + start: 1, + count: 1, + build_link: 1, + ) + end + + should "return a hash that has is_scoped set to true" do + results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters) + assert_equal true, results.to_hash[:is_scoped?] + end + + should "return a hash with the scope_title set to the scope title from the @search_response" do + results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters) + assert_equal @scope_title, results.to_hash[:scope_title] + end + + should "return a hash result count set to the scope title from the @search_response" do + results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters) + assert_equal "#{@unscoped_result_count} results", results.to_hash[:unscoped_result_count] + end + + context "presentable result list" do + + should "return all scoped results with unscoped results inserted at position 4" do + results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters).to_hash + + ## + # This test is asserting that the format of `presentable_list` is: + # [result, result, result, {results: list_of_results, is_multiple_results: true}, result ...] + # Where list_of_results are the top three results from an unscoped request to rummager + # and a flag `is_multiple_results` set to true. + ## + + simplified_expected_results_list = [{ "title"=> "scoped_result_1" }, + { "title"=> "scoped_result_2" }, + { "title"=> "scoped_result_3" }, + { "is_multiple_results" => true, + "results" => [{ "title"=> "unscoped_result_1" }, + { "title"=> "unscoped_result_2" }, + { "title"=> "unscoped_result_3" }, + ] + }, + { "title"=> "scoped_result_4" }, + ] + + + # Scoped results + simplified_expected_results_list[0..2].each_with_index do | result, i | + assert_equal result["title"], results[:results][i][:title] + end + + # Check un-scoped sub-list has flag + assert_equal true, results[:results][3][:is_multiple_results] + + # iterate unscoped sublist of results + simplified_expected_results_list[3]["results"].each_with_index do | result, i | + assert_equal result["title"], results[:results][3][:results][i][:title] + end + + # check remaining result + assert_equal simplified_expected_results_list[4]["title"], results[:results][4][:title] + end + end + + context "no scoped results returned" do + setup do + @no_results = [] + @search_response["unscoped_results"]["results"] = @no_results + end + + should "not not include unscoped results in the presentable_list if there aren't any" do + results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters).to_hash + + @scoped_results.each_with_index do | result, i | + assert_equal result["title"], results[:results][i][:title] + end + end + + should "not set unscoped_results_any? to false" do + results = ScopedSearchResultsPresenter.new(@search_response, @search_parameters).to_hash + assert_equal false, results.to_hash[:unscoped_results_any?] + end + end +end diff --git a/test/unit/search_api_test.rb b/test/unit/search_api_test.rb index 690f4e66ba..e090f034df 100644 --- a/test/unit/search_api_test.rb +++ b/test/unit/search_api_test.rb @@ -5,7 +5,7 @@ class SearchAPITest < ActiveSupport::TestCase setup do @rummager_api = stub - @rummager_params = stub + @rummager_params = stub(except: {}) @search_params = stub(rummager_parameters: @rummager_params) @search_api = SearchAPI.new(@rummager_api) @search_results = stub @@ -24,26 +24,35 @@ class SearchAPITest < ActiveSupport::TestCase end end - context "given an search scoped to a manual" do + context "given a search scoped to a manual" do setup do @manual_link = 'manual/manual-name' @manual_title = 'Manual Title' + @govuk_result_title = "GOV.UK result" + @search_params.expects(:filtered_by?).with('manual').returns(true) @search_params.expects(:filter).with('manual').returns([@manual_link]) @manual_search_response = stub(results: [stub(title: @manual_title)]) + @unscoped_search_response = stub(to_hash: {title: @govuk_result_title}) - @rummager_api.expects(:unified_search).with(filter_link: @manual_link, count: 1, fields: %w{title}).returns(@manual_search_response) + @rummager_api.expects(:unified_search).with(count: "3", reject_manual: @manual_link).returns(@unscoped_search_response) + @rummager_api.expects(:unified_search).with(filter_link: @manual_link, count: "1", fields: %w{title}).returns(@manual_search_response) end - should "returns search results from rummager" do + should "return search results from rummager" do search_response = @search_api.search(@search_params) assert_equal(@search_results, search_response.fetch(:results)) end - should "returns manual from rummager" do + should "return manual from rummager" do + search_response = @search_api.search(@search_params) + assert_equal({ "title" => @manual_title }, search_response.fetch("scope")) + end + + should "return three results for the whole of gov.uk from rummager" do search_response = @search_api.search(@search_params) - assert_equal({title: @manual_title}, search_response.fetch(:scope)) + assert_equal({ title: @govuk_result_title }, search_response.fetch("unscoped_results")) end end end