diff --git a/app/assets/stylesheets/partials/_pagination.scss b/app/assets/stylesheets/partials/_pagination.scss index 488d8bce..e7b8c423 100644 --- a/app/assets/stylesheets/partials/_pagination.scss +++ b/app/assets/stylesheets/partials/_pagination.scss @@ -5,6 +5,12 @@ justify-content: center; margin-top: 3em; + .first { + border-right: 1px solid black; + margin-right: 0.5em; + padding-right: 0.5em; + } + .previous { border-right: 1px solid black; margin-right: 0.5em; diff --git a/app/assets/stylesheets/partials/_search.scss b/app/assets/stylesheets/partials/_search.scss index daeca2a8..80aee357 100644 --- a/app/assets/stylesheets/partials/_search.scss +++ b/app/assets/stylesheets/partials/_search.scss @@ -127,3 +127,13 @@ .about { margin-top: 5rem; } + +/* primo continuation partial */ +.primo-continuation { + background-color: #f8f9fa; + border: 1px solid #dee2e6; + border-radius: 0.375rem; + padding: 2rem; + margin: 2rem 0; + text-align: center; +} diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 7ddc5aad..f8fb3edd 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -19,6 +19,9 @@ def results else params[:tab] || 'primo' # Default to primo for new tabbed interface end + + # Include the active tab in the enhanced query so it's available for pagination and other uses. + params[:tab] = @active_tab unless Feature.enabled?(:gdt) @enhanced_query = Enhancer.new(params).enhanced_query # Route to appropriate search based on active tab @@ -47,28 +50,41 @@ def load_gdt_results @errors = extract_errors(response) return unless @errors.nil? - @pagination = Analyzer.new(@enhanced_query, response).pagination + @pagination = Analyzer.new(@enhanced_query, response, :timdex).pagination raw_results = extract_results(response) @results = NormalizeTimdexResults.new(raw_results, @enhanced_query[:q]).normalize @filters = extract_filters(response) end def load_primo_results + current_page = @enhanced_query[:page] || 1 + per_page = @enhanced_query[:per_page] || 20 + offset = (current_page - 1) * per_page + + # Check if we're beyond Primo API limits before making the request. + if offset >= Analyzer::PRIMO_MAX_OFFSET + @show_primo_continuation = true + return + end + primo_response = query_primo @results = NormalizePrimoResults.new(primo_response, @enhanced_query[:q]).normalize - # Enhanced pagination using cached response - if @results.present? - total_hits = primo_response.dig('info', 'total') || @results.count - per_page = @enhanced_query[:per_page] || 20 - current_page = @enhanced_query[:page] || 1 - - @pagination = { - hits: total_hits, - start: ((current_page - 1) * per_page) + 1, - end: [current_page * per_page, total_hits].min - } + # Handle empty results from Primo API. Sometimes Primo will return no results at a given offset, + # despite claiming in the initial query that more are available. This happens randomly and + # seemingly for no reason (well below the recommended offset of 2,000). While the bug also + # exists in Primo UI, sending users there seems like the best we can do. + if @results.empty? + docs = primo_response['docs'] if primo_response.is_a?(Hash) + if docs.nil? || docs.empty? + @show_primo_continuation = true + else + @errors = [{ 'message' => 'No more results available at this page number.' }] + end end + + # Use Analyzer for consistent pagination across all search types + @pagination = Analyzer.new(@enhanced_query, primo_response, :primo).pagination rescue StandardError => e @errors = handle_primo_errors(e) end @@ -80,7 +96,7 @@ def load_timdex_results @errors = extract_errors(response) return unless @errors.nil? - @pagination = Analyzer.new(@enhanced_query, response).pagination + @pagination = Analyzer.new(@enhanced_query, response, :timdex).pagination raw_results = extract_results(response) @results = NormalizeTimdexResults.new(raw_results, @enhanced_query[:q]).normalize end @@ -117,7 +133,10 @@ def query_primo Rails.cache.fetch("#{cache_key}/primo", expires_in: 12.hours) do primo_search = PrimoSearch.new per_page = @enhanced_query[:per_page] || 20 - primo_search.search(@enhanced_query[:q], per_page) + current_page = @enhanced_query[:page] || 1 + offset = (current_page - 1) * per_page + + primo_search.search(@enhanced_query[:q], per_page, offset) end end diff --git a/app/helpers/pagination_helper.rb b/app/helpers/pagination_helper.rb index 05870196..f31bbca9 100644 --- a/app/helpers/pagination_helper.rb +++ b/app/helpers/pagination_helper.rb @@ -1,14 +1,45 @@ module PaginationHelper + def first_url(query_params) + # Work with a copy to avoid mutating the original enhanced_query. + params_copy = query_params.dup + params_copy[:page] = 1 + + # Preserve the active tab in pagination URLs. + params_copy[:tab] = @active_tab if @active_tab.present? + + link_to results_path(params_copy), 'aria-label': 'First page', + data: { turbo_frame: 'search-results', turbo_action: 'advance' }, + rel: 'nofollow' do + '«« First'.html_safe + end + end + def next_url(query_params) - query_params[:page] = @pagination[:next] - link_to results_path(query_params), 'aria-label': 'Next page' do + # Work with a copy to avoid mutating the original enhanced_query. + params_copy = query_params.dup + params_copy[:page] = @pagination[:next] + + # Preserve the active tab in pagination URLs. + params_copy[:tab] = @active_tab if @active_tab.present? + + link_to results_path(params_copy), 'aria-label': 'Next page', + data: { turbo_frame: 'search-results', turbo_action: 'advance' }, + rel: 'nofollow' do 'Next »'.html_safe end end def prev_url(query_params) - query_params[:page] = @pagination[:prev] - link_to results_path(query_params), 'aria-label': 'Previous page' do + # Work with a copy to avoid mutating the original enhanced_query. + params_copy = query_params.dup + params_copy[:page] = @pagination[:prev] + + # Preserve the active tab in pagination URLs. + params_copy[:tab] = @active_tab if @active_tab.present? + + link_to results_path(params_copy), 'aria-label': 'Previous page', + data: { turbo_frame: 'search-results', turbo_action: 'advance' }, + rel: 'nofollow' do '« Previous'.html_safe end end diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 23284a9d..742398df 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -110,4 +110,13 @@ def extract_year(date, delimiter) date.split(delimiter).last end end + + def primo_search_url(query_term) + base_url = 'https://mit.primo.exlibrisgroup.com/discovery/search' + params = { + vid: ENV.fetch('PRIMO_VID'), + query: "any,contains,#{query_term}" + } + "#{base_url}?#{params.to_query}" + end end diff --git a/app/models/analyzer.rb b/app/models/analyzer.rb index 3361592a..1c92b8c7 100644 --- a/app/models/analyzer.rb +++ b/app/models/analyzer.rb @@ -3,19 +3,44 @@ class Analyzer RESULTS_PER_PAGE = 20 - def initialize(enhanced_query, response) + # Primo API theoretical maximum recommended offset is 2000 records (per Ex Libris documentation) + # but in practice, the API often can't deliver results beyond ~960 records for large result sets, + # likely due to performance constraints. + PRIMO_MAX_OFFSET = 960 + + def initialize(enhanced_query, response, source) + @source = source @pagination = {} @pagination[:hits] = hits(response) @pagination[:start] = ((enhanced_query[:page] - 1) * RESULTS_PER_PAGE) + 1 - @pagination[:end] = [enhanced_query[:page] * RESULTS_PER_PAGE, hits(response)].min + @pagination[:end] = [enhanced_query[:page] * RESULTS_PER_PAGE, @pagination[:hits]].min @pagination[:prev] = enhanced_query[:page] - 1 if enhanced_query[:page] > 1 - @pagination[:next] = next_page(enhanced_query[:page], @pagination[:hits]) + + next_page_num = next_page(enhanced_query[:page], @pagination[:hits]) + @pagination[:next] = next_page_num if next_page_num end private def hits(response) return 0 if response.nil? + + if @source == :primo + primo_hits(response) + elsif @source == :timdex + timdex_hits(response) + else + 0 + end + end + + def primo_hits(response) + return 0 unless response.is_a?(Hash) + + response.dig('info', 'total') || 0 + end + + def timdex_hits(response) return 0 unless response.is_a?(Hash) && response.key?(:data) && response[:data].key?('search') return 0 unless response[:data]['search'].is_a?(Hash) && response[:data]['search'].key?('hits') diff --git a/app/models/enhancer.rb b/app/models/enhancer.rb index 61e006c0..23ad8a21 100644 --- a/app/models/enhancer.rb +++ b/app/models/enhancer.rb @@ -13,6 +13,7 @@ def initialize(params) @enhanced_query[:page] = calculate_page(params[:page].to_i) @enhanced_query[:advanced] = 'true' if params[:advanced].present? @enhanced_query[:booleanType] = params[:booleanType] || 'AND' + @enhanced_query[:tab] = params[:tab] if params[:tab].present? if Feature.enabled?(:geodata) @enhanced_query[:geobox] = 'true' if params[:geobox] == 'true' diff --git a/app/models/primo_search.rb b/app/models/primo_search.rb index 925989d2..94a95984 100644 --- a/app/models/primo_search.rb +++ b/app/models/primo_search.rb @@ -1,22 +1,21 @@ # Searches Primo Search API and formats results # class PrimoSearch - def initialize validate_env @primo_http = HTTP.persistent(primo_api_url) @results = {} end - def search(term, per_page) - url = search_url(term, per_page) + def search(term, per_page, offset = 0) + url = search_url(term, per_page, offset) result = @primo_http.timeout(http_timeout) - .headers( - accept: 'application/json', - Authorization: "apikey #{primo_api_key}" - ) - .get(url) - + .headers( + accept: 'application/json', + Authorization: "apikey #{primo_api_key}" + ) + .get(url) + raise "Primo Error Detected: #{result.status}" unless result.status == 200 JSON.parse(result) @@ -26,15 +25,15 @@ def search(term, per_page) def validate_env missing_vars = [] - + missing_vars << 'PRIMO_API_URL' if primo_api_url.nil? missing_vars << 'PRIMO_API_KEY' if primo_api_key.nil? missing_vars << 'PRIMO_SCOPE' if primo_scope.nil? missing_vars << 'PRIMO_TAB' if primo_tab.nil? missing_vars << 'PRIMO_VID' if primo_vid.nil? - + return if missing_vars.empty? - + raise ArgumentError, "Required Primo environment variables are not set: #{missing_vars.join(', ')}" end @@ -65,8 +64,8 @@ def clean_term(term) end # Constructs the search URL with required parameters for Primo API - def search_url(term, per_page) - [ + def search_url(term, per_page, offset = 0) + url_parts = [ primo_api_url, '/search?q=any,contains,', clean_term(term), @@ -77,10 +76,18 @@ def search_url(term, per_page) '&scope=', primo_scope, '&limit=', - per_page, + per_page + ] + + # Add offset parameter for pagination (only if > 0) + url_parts += ['&offset=', offset] if offset > 0 + + url_parts += [ '&apikey=', primo_api_key - ].join + ] + + url_parts.join end # Timeout configuration for HTTP requests diff --git a/app/views/search/_pagination.html.erb b/app/views/search/_pagination.html.erb index ddf13ead..20b8b29f 100644 --- a/app/views/search/_pagination.html.erb +++ b/app/views/search/_pagination.html.erb @@ -1,14 +1,22 @@ <% return if @pagination.nil? %>