diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index f1efd795..02772968 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -15,59 +15,102 @@ def results # inject session preference for boolean type if it is present params[:booleanType] = cookies[:boolean_type] || 'AND' - # hand off to Enhancer chain + # Determine which tab to load - default to primo unless gdt is enabled + @active_tab = if Flipflop.enabled?(:gdt) + 'gdt' # Keep existing GDT behavior unchanged + else + params[:tab] || 'primo' # Default to primo for new tabbed interface + end @enhanced_query = Enhancer.new(params).enhanced_query - # hand off enhanced query to builder - query = QueryBuilder.new(@enhanced_query).query + # Route to appropriate search based on active tab + if Flipflop.enabled?(:gdt) + # Keep existing GDT behavior unchanged + load_gdt_results + else + case @active_tab + when 'primo' + load_primo_results + when 'timdex' + load_timdex_results + end + end + end - # Create cache key for this query - # Sorting query hash to ensure consistent key generation regardless of the parameter order - sorted_query = query.sort_by { |k, v| k.to_sym }.to_h - cache_key = Digest::MD5.hexdigest(sorted_query.to_s) + private - # builder hands off to wrapper which returns raw results here - # We are using two difference caches to allow for Geo and USE to be cached separately. This ensures we don't have - # cache key collission for these two different query types. In practice, the likelihood of this happening is low, - # as the query parameters are different for each type and they won't often be run with the same cache backend other - # than locally, but this is a safeguard. - # The response type is a GraphQL::Client::Response, which is not directly serializable, so we convert it to a hash. - response = if Flipflop.enabled?(:gdt) - Rails.cache.fetch("#{cache_key}/geo", expires_in: 12.hours) do - raw = execute_geospatial_query(query) - { - data: raw.data.to_h, - errors: raw.errors.details.to_h - } - end - else - Rails.cache.fetch("#{cache_key}/use", expires_in: 12.hours) do - raw = TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query) - { - data: raw.data.to_h, - errors: raw.errors.details.to_h - } - end - end + def load_gdt_results + query = QueryBuilder.new(@enhanced_query).query + + response = cache_timdex_query(query) # Handle errors @errors = extract_errors(response) - - # Analayze results - # The @pagination instance variable includes info about next/previous pages (where they exist) to assist the UI. @pagination = Analyzer.new(@enhanced_query, response).pagination if @errors.nil? - - # Display results @results = extract_results(response) @filters = extract_filters(response) end - private + def load_primo_results + begin + primo_search = PrimoSearch.new + per_page = params[:per_page] || 20 + primo_response = primo_search.search(params[:q], per_page) + + @results = NormalizePrimoResults.new(primo_response, params[:q]).normalize + + # Basic pagination for now. + if @results.present? + @pagination = { + hits: @results.count, + start: 1, + end: @results.count + } + end + + rescue StandardError => e + @errors = handle_primo_errors(e) + end + end + + def load_timdex_results + query = QueryBuilder.new(@enhanced_query).query + response = cache_timdex_query(query) + + @errors = extract_errors(response) + @pagination = Analyzer.new(@enhanced_query, response).pagination if @errors.nil? + @results = extract_results(response) + end def active_filters ENV.fetch('ACTIVE_FILTERS', '').split(',').map(&:strip) end + def cache_timdex_query(query) + # Create cache key for this query + # Sorting query hash to ensure consistent key generation regardless of the parameter order + sorted_query = query.sort_by { |k, v| k.to_sym }.to_h + cache_key = Digest::MD5.hexdigest(sorted_query.to_s) + + # builder hands off to wrapper which returns raw results here + # We are using two difference caches to allow for Geo and USE to be cached separately. This ensures we don't have + # cache key collision for these two different query types. In practice, the likelihood of this happening is low, + # as the query parameters are different for each type and they won't often be run with the same cache backend other + # than locally, but this is a safeguard. + # The response type is a GraphQL::Client::Response, which is not directly serializable, so we convert it to a hash. + Rails.cache.fetch("#{cache_key}/#{@active_tab}", expires_in: 12.hours) do + raw = if @active_tab == 'gdt' + execute_geospatial_query(query) + elsif @active_tab == 'timdex' + TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query) + end + { + data: raw.data.to_h, + errors: raw.errors.details.to_h + } + end + end + def execute_geospatial_query(query) if query['geobox'] == 'true' && query[:geodistance] == 'true' TimdexBase::Client.query(TimdexSearch::AllQuery, variables: query) @@ -214,4 +257,16 @@ def validate_geobox_values! flash[:error] = 'Maximum latitude cannot exceed minimum latitude.' redirect_to root_url end + + def handle_primo_errors(error) + Rails.logger.error("Primo search error: #{error.message}") + + if error.is_a?(ArgumentError) + [{ 'message' => 'Primo search is not properly configured.' }] + elsif error.is_a?(HTTP::TimeoutError) + [{ 'message' => 'The Primo service is currently slow to respond. Please try again.' }] + else + [{ 'message' => error.message }] + end + end end diff --git a/app/views/search/_form.html.erb b/app/views/search/_form.html.erb index b0287460..81f02734 100644 --- a/app/views/search/_form.html.erb +++ b/app/views/search/_form.html.erb @@ -31,118 +31,117 @@ keyword_placeholder = search_required ? "Enter your search" : "Keyword anywhere" <%= 'aria-describedby=site-desc' if Flipflop.enabled?(:gdt) %>> <% if Flipflop.enabled?(:gdt) %> -
> - - <%= geobox_label %> - - -
- Search within a geospatial bounding box -

* All fields in this section are required

-
-
- - " - id="geobox-minLongitude" name="geoboxMinLongitude" value="<%= params[:geoboxMinLongitude] %>" - <%= 'required' if geobox_required %> - aria-describedby="minLong-desc"> - - A decimal between -180.0 and 180.0 (Ex: -73.507239) +
> + + <%= geobox_label %> + + +
+ Search within a geospatial bounding box +

* All fields in this section are required

+
+
+ + " + id="geobox-minLongitude" name="geoboxMinLongitude" value="<%= params[:geoboxMinLongitude] %>" + <%= 'required' if geobox_required %> + aria-describedby="minLong-desc"> + + A decimal between -180.0 and 180.0 (Ex: -73.507239) + Western Hemisphere is negative + +
+
+ + " + id="geobox-minLatitude" name="geoboxMinLatitude" value="<%= params[:geoboxMinLatitude] %>" + <%= 'required' if geobox_required %> + aria-describedby="minLat-desc"> + + A decimal between -90.0 and 90.0 (Ex: 41.239083) + Southern Hemisphere is negative + +
+
+ + " + id="geobox-maxLongitude" name="geoboxMaxLongitude" value="<%= params[:geoboxMaxLongitude] %>" + <%= 'required' if geobox_required %> + aria-describedby="maxLong-desc"> + + A decimal between -180.0 and 180.0 (Ex: -69.928713) Western Hemisphere is negative -
-
- - " - id="geobox-minLatitude" name="geoboxMinLatitude" value="<%= params[:geoboxMinLatitude] %>" - <%= 'required' if geobox_required %> - aria-describedby="minLat-desc"> - - A decimal between -90.0 and 90.0 (Ex: 41.239083) - Southern Hemisphere is negative - -
-
- +
+
+ + " + id="geobox-maxLatitude" name="geoboxMaxLatitude" value="<%= params[:geoboxMaxLatitude] %>" + <%= 'required' if geobox_required %> + aria-describedby="maxLat-desc"> + + A decimal between -90.0 and 90.0 (Ex: 42.886759) + Southern Hemisphere is negative + +
+
+
+
+
> + + <%= geodistance_label %> + + +
+ Search within a distance of a geographic point +

* All fields in this section are required

+
+
+ + " + id="geodistance-latitude" name="geodistanceLatitude" + value="<%= params[:geodistanceLatitude] %>" aria-describedby="lat-desc" + <%= 'required' if geodistance_required %> + aria-describedby="lat-desc"> + + A decimal between -90.0 and 90.0 (Ex: 42.279594) + Southern Hemisphere is negative + +
+
+ " - id="geobox-maxLongitude" name="geoboxMaxLongitude" value="<%= params[:geoboxMaxLongitude] %>" - <%= 'required' if geobox_required %> - aria-describedby="maxLong-desc"> - - A decimal between -180.0 and 180.0 (Ex: -69.928713) - Western Hemisphere is negative - -
-
- - " - id="geobox-maxLatitude" name="geoboxMaxLatitude" value="<%= params[:geoboxMaxLatitude] %>" - <%= 'required' if geobox_required %> - aria-describedby="maxLat-desc"> - - A decimal between -90.0 and 90.0 (Ex: 42.886759) - Southern Hemisphere is negative - -
+ class="field field-text <%= "required" if geodistance_required %>" + id="geodistance-longitude" name="geodistanceLongitude" + value="<%= params[:geodistanceLongitude] %>" aria-describedby="long-desc" + <%= 'required' if geodistance_required %> + aria-describedby="long-desc"> + + A decimal between -180.0 and 180.0 (Ex: -83.732124) + Western Hemisphere is negative +
-
-
-
> - - <%= geodistance_label %> - - -
- Search within a distance of a geographic point -

* All fields in this section are required

-
-
- - " - id="geodistance-latitude" name="geodistanceLatitude" - value="<%= params[:geodistanceLatitude] %>" aria-describedby="lat-desc" - <%= 'required' if geodistance_required %> - aria-describedby="lat-desc"> - - A decimal between -90.0 and 90.0 (Ex: 42.279594) - Southern Hemisphere is negative - -
-
- - " - id="geodistance-longitude" name="geodistanceLongitude" - value="<%= params[:geodistanceLongitude] %>" aria-describedby="long-desc" - <%= 'required' if geodistance_required %> - aria-describedby="long-desc"> - - A decimal between -180.0 and 180.0 (Ex: -83.732124) - Western Hemisphere is negative - -
-
- - " - id="geodistance-distance" name="geodistanceDistance" - value="<%= params[:geodistanceDistance] %>" aria-describedby="distance-desc" - <%= 'required' if geodistance_required %> - aria-describedby="distance-desc"> - - Distance is in meters by default; add other units if preferred (Ex: '100km' or '50mi') - -
+
+ + " + id="geodistance-distance" name="geodistanceDistance" + value="<%= params[:geodistanceDistance] %>" aria-describedby="distance-desc" + <%= 'required' if geodistance_required %> + aria-describedby="distance-desc"> + + Distance is in meters by default; add other units if preferred (Ex: '100km' or '50mi') +
-
-
- <% end %> +
+
+
> @@ -158,33 +157,11 @@ keyword_placeholder = search_required ? "Enter your search" : "Keyword anywhere"
- +
- <% unless Flipflop.enabled?(:gdt) %> -
- - -
- -
- - -
- -
- - -
- <% end %> -
- - <% unless Flipflop.enabled?(:gdt) %> -
- <%# https://www.w3.org/WAI/tutorials/forms/grouping/ %> -
- Limit search to checked sources. - - <% timdex_sources.each do |source| %> - <%= source_checkbox(source, params) %> - <% end %> -
-
- <% end %>
+ <% end %>
diff --git a/app/views/search/_result_primo.html.erb b/app/views/search/_result_primo.html.erb new file mode 100644 index 00000000..318eb664 --- /dev/null +++ b/app/views/search/_result_primo.html.erb @@ -0,0 +1,77 @@ +
  • +
    +

    + Title: + <% if result_primo['links']&.find { |link| link['kind'] == 'full record' } %> + <%= link_to(result_primo['title'], result_primo['links'].find { |link| link['kind'] == 'full record' }['url'], target: '_blank', rel: 'noopener') %> + <% else %> + <%= result_primo['title'] %> + <% end %> +

    + +

    + <%= result_primo['format'] %> + <%= result_primo['year'] %> +

    + + <% if result_primo['creators'].present? %> + Contributors: +
      + <% result_primo['creators'].each do |creator| %> +
    • + <% if creator[:link] %> + <%= link_to creator[:value], creator[:link] %> + <% else %> + <%= creator[:value] %> + <% end %> +
      • + <% end %> +
    + <% end %> + + <% if result_primo['container'].present? %> +

    + Published in: + <%= result_primo['container'] %> +

    + <% end %> + + <% if result_primo['citation'].present? %> +

    + Citation: + <%= result_primo['citation'] %> +

    + <% end %> + + <% if result_primo['summary'].present? %> +

    + Summary: + <%= truncate(result_primo['summary'], length: 300) %> +

    + <% end %> + + <% if result_primo['subjects'].present? %> +

    + Subjects: + <%= result_primo['subjects'].join('; ') %> +

    + <% end %> + + <% if result_primo['links'].present? %> +
      + <% result_primo['links'].each do |link| %> +
    • + <%= link_to link['kind'].titleize, link['url'], class: 'link-button' %> +
      • + <% end %> +
    + <% end %> + + <% if result_primo['availability'].present? %> +

    + Availability: + <%= result_primo['availability'] %> +

    + <% end %> +
    +
    • \ No newline at end of file diff --git a/app/views/search/_search_summary_use.html.erb b/app/views/search/_search_summary_use.html.erb new file mode 100644 index 00000000..d59db3bd --- /dev/null +++ b/app/views/search/_search_summary_use.html.erb @@ -0,0 +1,7 @@ +<% return unless params[:q].present? %> + + \ No newline at end of file diff --git a/app/views/search/results.html.erb b/app/views/search/results.html.erb index aa89ac79..98f9cb51 100644 --- a/app/views/search/results.html.erb +++ b/app/views/search/results.html.erb @@ -9,7 +9,11 @@ <%= render partial: "shared/site_title" %> <%= render partial: "form" %> - <%= render partial: "search_summary" %> + <% if Flipflop.enabled?(:gdt) %> + <%= render partial: "search_summary" %> + <% else %> + <%= render partial: "search_summary_use" %> + <% end %> <% if ENV.fetch('FACT_PANELS_ENABLED', false).present? %>
    @@ -22,42 +26,59 @@ <%= render(partial: 'shared/error', collection: @errors) %> -
    - <% if @filters.present? %> - - <% end %> - -
    - <% if @results.present? %> -

    <%= results_summary(@pagination[:hits]) %> returned

    -
      - <% if Flipflop.enabled?(:gdt) %> - <%= render(partial: 'search/result_geo', collection: @results) %> - <% else %> - <%= render(partial: 'search/result', collection: @results) %> - <% end %> -
    - <% else %> -
    -

    No results found for your search

    -
    - <% end %> -
    + + <% elsif @errors.blank? %> +
    +

    No results found for your search

    +
    + <% end %> + +
    + <% end %> <%= render partial: 'shared/ask', locals: { display: 'aside' } if @results.blank? %> <% if @results.present? %> diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb index c33385ed..6f3028bb 100644 --- a/test/controllers/search_controller_test.rb +++ b/test/controllers/search_controller_test.rb @@ -6,56 +6,128 @@ def setup @test_strategy.switch!(:gdt, false) end + def mock_primo_search_success + # Mock the Primo search components to avoid external API calls + sample_doc = { + 'title' => 'Sample Primo Document Title', + 'format' => 'Article', + 'year' => '2025', + 'creators' => [ + { value: 'Foo Barston', link: nil }, + { value: 'Baz Quxley', link: nil } + ], + 'links' => [{ 'kind' => 'full record', 'url' => 'https://example.com/record' }] + } + + mock_primo = mock('primo_search') + mock_primo.expects(:search).returns({ 'docs' => [sample_doc], 'total' => 1 }) + PrimoSearch.expects(:new).returns(mock_primo) + + mock_normalizer = mock('normalizer') + mock_normalizer.expects(:normalize).returns([sample_doc]) + NormalizePrimoResults.expects(:new).returns(mock_normalizer) + end + + def mock_timdex_search_success + # Mock the TIMDEX GraphQL client to avoid external API calls + sample_result = { + 'title' => 'Sample TIMDEX Document Title', + 'timdexRecordId' => 'sample-record-123', + 'contentType' => [{ 'value' => 'Article' }], + 'dates' => [{ 'kind' => 'Publication date', 'value' => '2023' }], + 'contributors' => [{ 'value' => 'Foo Barston', 'kind' => 'Creator' }], + 'highlight' => [ + { + 'matchedField' => 'summary', + 'matchedPhrases' => ['sample document'] + } + ], + 'sourceLink' => 'https://example.com/record' + } + + mock_response = mock('timdex_response') + mock_errors = mock('timdex_errors') + mock_errors.stubs(:details).returns({}) + mock_errors.stubs(:to_h).returns({}) + mock_response.stubs(:errors).returns(mock_errors) + + mock_data = mock('timdex_data') + mock_search = mock('timdex_search') + mock_search.stubs(:to_h).returns({ + 'hits' => 1, + 'aggregations' => {}, + 'records' => [sample_result] + }) + mock_data.stubs(:search).returns(mock_search) + mock_data.stubs(:to_h).returns({ + 'search' => { + 'hits' => 1, + 'aggregations' => {}, + 'records' => [sample_result] + } + }) + mock_response.stubs(:data).returns(mock_data) + + TimdexBase::Client.expects(:query).returns(mock_response) + end + test 'index shows basic search form by default' do get '/' assert_response :success assert_select 'form#basic-search', { count: 1 } - - details_div = assert_select('details#advanced-search-panel') - assert_nil details_div.attribute('open') + assert_select 'details#advanced-search-panel', count: 0 end test 'index shows advanced search form with URL parameter' do - get '/?advanced=true' - - assert_response :success - - details_div = assert_select('details#advanced-search-panel') - assert details_div.attribute('open') - end - - test 'advanced search form appears on results page with URL parameter' do - VCR.use_cassette('advanced', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - get '/results?advanced=true' - + if Flipflop.enabled?(:gdt) + get '/?advanced=true' assert_response :success - details_div = assert_select('details#advanced-search-panel') assert details_div.attribute('open') + else + skip('Advanced search functionality not implemented in USE UI') + end + end + + test 'advanced search form appears on results page with URL parameter' do + if Flipflop.enabled?(:gdt) + VCR.use_cassette('advanced', + allow_playback_repeats: true, + match_requests_on: %i[method uri body]) do + get '/results?advanced=true' + assert_response :success + details_div = assert_select('details#advanced-search-panel') + assert details_div.attribute('open') + end + else + skip('Advanced search functionality not implemented in USE UI') end end test 'search form includes a number of fields' do get '/' - # Please note that this test confirms fields in the DOM - but not whether - # they are visible. Fields in a hidden details panel are still in the DOM, - # but not visible or reachable via keyboard interaction. + # Basic search field should always be present assert_select 'input#basic-search-main', { count: 1 } - assert_select 'input#advanced-citation', { count: 1 } - assert_select 'input#advanced-contributors', { count: 1 } - assert_select 'input#advanced-fundingInformation', { count: 1 } - assert_select 'input#advanced-identifiers', { count: 1 } - assert_select 'input#advanced-locations', { count: 1 } - assert_select 'input#advanced-subjects', { count: 1 } - assert_select 'input#advanced-title', { count: 1 } - assert_select 'input.source', { minimum: 3 } + + if Flipflop.enabled?(:gdt) + # Please note that this test confirms fields in the DOM - but not whether + # they are visible. Fields in a hidden details panel are still in the DOM, + # but not visible or reachable via keyboard interaction. + assert_select 'input#advanced-citation', { count: 1 } + assert_select 'input#advanced-contributors', { count: 1 } + assert_select 'input#advanced-fundingInformation', { count: 1 } + assert_select 'input#advanced-identifiers', { count: 1 } + assert_select 'input#advanced-locations', { count: 1 } + assert_select 'input#advanced-subjects', { count: 1 } + assert_select 'input#advanced-title', { count: 1 } + assert_select 'input.source', { minimum: 3 } + end end test 'advanced search source checkboxes can be controlled by env' do + skip('Advanced search functionality not implemented in USE UI') get '/' assert_select 'input.source', { minimum: 3 } @@ -88,69 +160,98 @@ def setup assert_equal 'A search term is required.', flash[:error] end - test 'results with valid query displays the query' do - VCR.use_cassette('timdex hallo', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - get '/results?q=hallo' - assert_response :success - assert_nil flash[:error] + test 'primo results with valid query displays the query' do + mock_primo_search_success + get '/results?q=hallo' + assert_response :success + assert_nil flash[:error] - assert_select 'input[value=?]', 'hallo' - end + assert_select 'input[value=?]', 'hallo' end - test 'results with valid query shows search form' do - VCR.use_cassette('timdex hallo', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - get '/results?q=hallo' - assert_response :success + test 'timdex results with valid query displays the query' do + mock_timdex_search_success + get '/results?q=hallo&tab=timdex' + assert_response :success + assert_nil flash[:error] - assert_select 'form#basic-search', { count: 1 } - end + assert_select 'input[value=?]', 'hallo' end - test 'results with valid query populates search form with query' do - VCR.use_cassette('data basic controller', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - get '/results?q=data' - assert_response :success + test 'primo results with valid query shows search form' do + mock_primo_search_success + get '/results?q=hallo' + assert_response :success - assert_select '#basic-search-main[value=data]' - end + assert_select 'form#basic-search', { count: 1 } end - test 'results with valid query has div for hints if any fact panels are enabled' do - VCR.use_cassette('data basic controller', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: 'fakepanel') do - get '/results?q=data' - end + test 'timdex results with valid query shows search form' do + mock_timdex_search_success + get '/results?q=hallo&tab=timdex' + assert_response :success - assert_response :success + assert_select 'form#basic-search', { count: 1 } + end + + test 'primo results with valid query populates search form with query' do + mock_primo_search_success + get '/results?q=data' + assert_response :success + + assert_select '#basic-search-main[value=data]' + end - assert_select '#hint' + test 'timdex results with valid query populates search form with query' do + mock_timdex_search_success + get '/results?q=data&tab=timdex' + assert_response :success + + assert_select '#basic-search-main[value=data]' + end + + test 'primo results with valid query has div for hints if any fact panels are enabled' do + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: 'fakepanel') do + get '/results?q=data' end + + assert_response :success + assert_select '#hint' end - test 'results with valid query has no div for hints if no fact panels are enabled' do - VCR.use_cassette('data basic controller', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: '') do - get '/results?q=data' - end + test 'timdex results with valid query has div for hints if any fact panels are enabled' do + mock_timdex_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: 'fakepanel') do + get '/results?q=data&tab=timdex' + end - assert_response :success + assert_response :success + assert_select '#hint' + end - assert_select('#hint', 0) + test 'primo results with valid query has no div for hints if no fact panels are enabled' do + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: '') do + get '/results?q=data' end + + assert_response :success + assert_select('#hint', 0) + end + + test 'timdex results with valid query has no div for hints if no fact panels are enabled' do + mock_timdex_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: '') do + get '/results?q=data&tab=timdex' + end + + assert_response :success + assert_select('#hint', 0) end test 'results with valid query has div for filters which is populated' do + skip('Filters not implemented in USE UI') VCR.use_cassette('data basic controller', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -162,6 +263,7 @@ def setup end test 'a filter category lists available filters with names and values' do + skip('Filters not implemented in USE UI') VCR.use_cassette('data basic controller', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -172,47 +274,55 @@ def setup end end - test 'results with valid query has div for pagination' do - VCR.use_cassette('data basic controller', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - get '/results?q=data' - assert_response :success + test 'primo results with valid query has div for pagination' do + mock_primo_search_success + get '/results?q=data' + assert_response :success + assert_select '#pagination' + end - assert_select '#pagination' - end + test 'timdex results with valid query has div for pagination' do + mock_timdex_search_success + get '/results?q=data&tab=timdex' + assert_response :success + assert_select '#pagination' end - test 'results with valid query has div for results which is populated' do - VCR.use_cassette('data basic controller', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - get '/results?q=data' - assert_response :success - assert_select '#results' - assert_select '#results .record-title', { minimum: 1 } - end + test 'primo results with valid query has div for results which is populated' do + mock_primo_search_success + get '/results?q=data' + assert_response :success + assert_select '#results' + assert_select '#results .record-title', { minimum: 1 } end - test 'results with valid query include links' do - VCR.use_cassette('data basic controller', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - get '/results?q=data' - assert_select '#results .record-title a' do |value| - refute_nil(value.xpath('./@href').text) - end - end + test 'timdex results with valid query has div for results which is populated' do + mock_timdex_search_success + get '/results?q=data&tab=timdex' + assert_response :success + assert_select '#results' + assert_select '#results .record-title', { minimum: 1 } end - test 'results with valid query have query highlights' do - VCR.use_cassette('data basic controller', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - get '/results?q=data' - assert_response :success - assert_select '#results .result-highlights ul li', { minimum: 1 } - end + test 'primo results with valid query include links' do + mock_primo_search_success + get '/results?q=data' + assert_response :success + assert_select '#results .record-title a' + end + + test 'timdex results with valid query include links' do + mock_timdex_search_success + get '/results?q=data&tab=timdex' + assert_response :success + assert_select '#results .record-title a' + end + + test 'timdex results with valid query have query highlights' do + mock_timdex_search_success + get '/results?q=data&tab=timdex' + assert_response :success + assert_select '#results .result-highlights ul li', { minimum: 1 } end test 'highlights partial is not rendered for results with no relevant highlights' do @@ -232,13 +342,13 @@ def setup VCR.use_cassette('timdex no results', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do - get '/results?q=asdfiouwenlasd' + get '/results?q=asdfiouwenlasd&tab=timdex' assert_response :success # Result list contents state "no results" assert_select '#results' assert_select '#results', { count: 1 } - assert_select '#results p', 'No results found for your search' + assert_select '#results .no-results p', 'No results found for your search' # Filter sidebar is not shown assert_select '#filters', { count: 0 } @@ -252,117 +362,90 @@ def setup end test 'searches with ISSN display issn fact card when enabled' do - VCR.use_cassette('timdex 1234-5678', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: 'issn') do - get '/results?q=1234-5678' - end - assert_response :success - - actual_div = assert_select('div[data-content-loader-url-value]') - assert_equal '/issn?issn=1234-5678', - actual_div.attribute('data-content-loader-url-value').value + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: 'issn') do + get '/results?q=1234-5678' end + assert_response :success + + actual_div = assert_select('div[data-content-loader-url-value]') + assert_equal '/issn?issn=1234-5678', + actual_div.attribute('data-content-loader-url-value').value end test 'searches with ISSN do not display issn fact card when not enabled' do - VCR.use_cassette('timdex 1234-5678', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: '') do - get '/results?q=1234-5678' - end - assert_response :success - - assert_select('div[data-content-loader-url-value].fact-container', 0) + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: '') do + get '/results?q=1234-5678' end + assert_response :success + + assert_select('div[data-content-loader-url-value].fact-container', 0) end test 'searches with ISBN insert isbn dom element when enabled' do - VCR.use_cassette('timdex 9781857988536', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: 'isbn') do - get '/results?q=9781857988536' - end + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: 'isbn') do + get '/results?q=9781857988536' + end - assert_response :success + assert_response :success - actual_div = assert_select('div[data-content-loader-url-value]') - assert_equal '/isbn?isbn=9781857988536', - actual_div.attribute('data-content-loader-url-value').value - end + actual_div = assert_select('div[data-content-loader-url-value]') + assert_equal '/isbn?isbn=9781857988536', + actual_div.attribute('data-content-loader-url-value').value end test 'searches with ISBN do not insert isbn dom element when not enabled' do - VCR.use_cassette('timdex 9781857988536', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: '') do - get '/results?q=9781857988536' - end - - assert_response :success - - assert_select('div[data-content-loader-url-value].fact-container', 0) + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: '') do + get '/results?q=9781857988536' end + + assert_response :success + assert_select('div[data-content-loader-url-value].fact-container', 0) end test 'searches with DOI insert doi dom element when enabled' do - VCR.use_cassette('timdex 10.1038.nphys1170 ', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: 'doi') do - get '/results?q=10.1038/nphys1170 ' - end - assert_response :success - - actual_div = assert_select('div[data-content-loader-url-value]') - assert_equal '/doi?doi=10.1038%2Fnphys1170', - actual_div.attribute('data-content-loader-url-value').value + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: 'doi') do + get '/results?q=10.1038/nphys1170 ' end + assert_response :success + + actual_div = assert_select('div[data-content-loader-url-value]') + assert_equal '/doi?doi=10.1038%2Fnphys1170', + actual_div.attribute('data-content-loader-url-value').value end test 'searches with DOI do not insert doi dom element when not enabled' do - VCR.use_cassette('timdex 10.1038.nphys1170 ', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: '') do - get '/results?q=10.1038/nphys1170 ' - end - assert_response :success - - assert_select('div[data-content-loader-url-value].fact-container', 0) + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: '') do + get '/results?q=10.1038/nphys1170 ' end + assert_response :success + assert_select('div[data-content-loader-url-value].fact-container', 0) end test 'searches with pmid insert pmid dom element when enabled' do - VCR.use_cassette('timdex PMID 35649707', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: 'pmid') do - get '/results?q=PMID: 35649707' - end - assert_response :success - - actual_div = assert_select('div[data-content-loader-url-value]') - assert_equal '/pmid?pmid=PMID%3A+35649707', - actual_div.attribute('data-content-loader-url-value').value + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: 'pmid') do + get '/results?q=PMID: 35649707' end + assert_response :success + + actual_div = assert_select('div[data-content-loader-url-value]') + assert_equal '/pmid?pmid=PMID%3A+35649707', + actual_div.attribute('data-content-loader-url-value').value end test 'searches with pmid do not insert pmid dom element when not enabled' do - VCR.use_cassette('timdex PMID 35649707', - allow_playback_repeats: true, - match_requests_on: %i[method uri body]) do - ClimateControl.modify(FACT_PANELS_ENABLED: '') do - get '/results?q=PMID: 35649707' - end - assert_response :success - - assert_select('div[data-content-loader-url-value].fact-container', 0) + mock_primo_search_success + ClimateControl.modify(FACT_PANELS_ENABLED: '') do + get '/results?q=PMID: 35649707' end + assert_response :success + assert_select('div[data-content-loader-url-value].fact-container', 0) end test 'TACOS intervention is inserted when TACOS enabled' do @@ -391,6 +474,7 @@ def setup # Advanced search behavior test 'advanced search by keyword' do + skip('Advanced search not implemented in USE UI') VCR.use_cassette('advanced keyword asdf', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -403,6 +487,7 @@ def setup end test 'can search an advanced field without a keyword search' do + skip('Advanced search not implemented in USE UI') # note, this confirms we only validate param[:q] is present for basic searches VCR.use_cassette('advanced citation asdf', allow_playback_repeats: true, @@ -414,6 +499,7 @@ def setup end test 'advanced search can accept values from all fields' do + skip('Advanced search not implemented in USE UI') VCR.use_cassette('advanced all', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -445,6 +531,7 @@ def setup end test 'advanced search form retains values with spaces' do + skip('Advanced search not implemented in USE UI') VCR.use_cassette('advanced all spaces', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -479,6 +566,7 @@ def source_filter_count(controller) end test 'advanced search can limit to a single source' do + skip('Advanced search not implemented in USE UI') VCR.use_cassette('advanced source limit to one source', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -496,6 +584,7 @@ def source_filter_count(controller) end test 'advanced search defaults to all sources' do + skip('Advanced search not implemented in USE UI') VCR.use_cassette('advanced source defaults to all', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -513,6 +602,7 @@ def source_filter_count(controller) end test 'advanced search can limit to multiple sources' do + skip('Advanced search not implemented in USE UI') VCR.use_cassette('advanced source limit to two sources', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -533,6 +623,7 @@ def source_filter_count(controller) end test 'applications can customize the displayed filters via ENV' do + skip('Filters not implemented in USE UI') VCR.use_cassette('data basic controller', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -571,6 +662,7 @@ def source_filter_count(controller) end test 'clear all filters button does not appear with zero filters in query' do + skip('Filters not implemented in USE UI') VCR.use_cassette('data basic controller', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -581,6 +673,7 @@ def source_filter_count(controller) end test 'clear all filters button does not appear with one filter in query' do + skip('Filters not implemented in USE UI') VCR.use_cassette('filter one', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -595,6 +688,7 @@ def source_filter_count(controller) end test 'clear all filters button appears with more than filter in query' do + skip('Filters not implemented in USE UI') VCR.use_cassette('filter multiple', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do @@ -608,4 +702,57 @@ def source_filter_count(controller) assert_select '.clear-filters', count: 1 end end + + # Tab functionality tests for USE + test 'results defaults to primo tab when no tab parameter provided' do + mock_primo_search_success + + get '/results?q=test' + assert_response :success + assert_select 'a.tab-link.active[href*="tab=primo"]', count: 1 + end + + test 'results respects primo tab parameter' do + mock_primo_search_success + + get '/results?q=test&tab=primo' + assert_response :success + assert_select 'a.tab-link.active[href*="tab=primo"]', count: 1 + end + + test 'results respects timdex tab parameter' do + mock_timdex_search_success + + get '/results?q=test&tab=timdex' + assert_response :success + assert_select 'a.tab-link.active[href*="tab=timdex"]', count: 1 + end + + test 'results shows tab navigation when gdt is disabled' do + mock_primo_search_success + + get '/results?q=test' + assert_response :success + assert_select '.tab-navigation', count: 1 + assert_select 'a[href*="tab=primo"]', count: 1 + assert_select 'a[href*="tab=timdex"]', count: 1 + end + + test 'results handles primo search errors gracefully' do + PrimoSearch.expects(:new).raises(StandardError.new('API Error')) + + get '/results?q=test&tab=primo' + assert_response :success + assert_select '.alert', count: 1 + assert_select '.alert', text: /API Error/ + end + + test 'results uses simplified search summary for USE app' do + mock_primo_search_success + + get '/results?q=test' + assert_response :success + assert_select 'aside.search-summary', count: 1 + assert_select 'aside.search-summary', text: /You searched for: test/ + end end diff --git a/test/integration/error_resilience_test.rb b/test/integration/error_resilience_test.rb index d157f796..319238ec 100644 --- a/test/integration/error_resilience_test.rb +++ b/test/integration/error_resilience_test.rb @@ -7,7 +7,7 @@ class ErrorResilienceTest < ActionDispatch::IntegrationTest VCR.use_cassette('timdex error', allow_playback_repeats: true, match_requests_on: %i[method uri body]) do - get '/results?q=poverty' + get '/results?q=poverty&tab=timdex' assert_response :success assert_select('.alert', true) assert_select('.alert') do |value|