Merge pull request #257 from MITLibraries/use-95-pagination

Fix Turbo Frames pagination bug

Author: jazairi

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;

app/assets/stylesheets/partials/_search.scss

@@ -182,3 +182,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;
+}

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
 

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

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

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')
 

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'

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

app/views/search/_pagination.html.erb

@@ -1,14 +1,22 @@
 <% return if @pagination.nil? %>
 
 <nav class="pagination-container" aria-label="Pagination">
+  <div class="first">
+    <% current_page = @enhanced_query[:page] || 1 %>
+    <% if current_page > 2 %>
+      <%= first_url(@enhanced_query) %>
+    <% else %>
+      First
+    <% end %>
+  </div>
   <div class="previous">
     <% if @pagination[:prev] %>
       <%= prev_url(@enhanced_query) %>
     <% else %>
-      First
+      Previous
     <% end %>
   </div>
-  <div class="current"><%= @pagination[:start] %> - <%= @pagination[:end] %> of <%= @pagination[:hits] %></div>
+  <div class="current"><%= @pagination[:start] %> - <%= @pagination[:end] %> of <%= number_with_delimiter(@pagination[:hits]) %></div>
   <div class="next">
     <% if @pagination[:next] %>
       <%= next_url(@enhanced_query) %>

app/views/search/_primo_continuation.html.erb

@@ -0,0 +1,13 @@
+<div class="primo-continuation">
+  <h2 class="hd-3">Continue your search in Search Our Collections</h2>
+  <p>
+    You've reached the pagination limit for embedded search results. To see more results and access 
+    additional search features, continue your search in the full Search Our Collections interface.
+  </p>
+  <div>
+    <%= link_to "Continue in Search Our Collections", 
+        primo_search_url(@enhanced_query[:q]), 
+        class: "button-primary", 
+        rel: "nofollow" %>
+  </div>
+</div>