Fix Turbo Frames pagination bug

Why these changes are being introduced:

Navigation past page 2 in search results is broken due to Turbo Frame
state management issues. Pagination controls were outside the Turbo
Frame, causing full page reloads that lost tab context and broke the
pagination flow.

Additionally, Primo API has practical limits around 960 records (or
thereabouts), despite claiming millions of results and a recommended
offset limit of 2,000 and a hardcoded limit of 5,000. This leads to
inconsistent behavior at high page counts (errors, timeouts, etc).

Relevant ticket(s):

- [USE-95](https://mitlibraries.atlassian.net/browse/USE-95)
- [USE-110](https://mitlibraries.atlassian.net/browse/USE-110)

How this addresses that need:

- Moves pagination controls inside the `search-results` Turbo Frame to
maintain state during navigation.
- Adds `data-turbo-action='advance'` to pagination links for proper URL
updates and browser history management.
- Adds `rel='nofollow'` to pagination links to prevent bot crawling.
- Set PRIMO_MAX_OFFSET at 960 based on real-world API testing.
- Implement continuation page for Primo results beyond pagination
limits.
- Consolidates result count display logic to show `10,000+` for all
sources when appropriate.
- Adds a `First` affordance for users to return quickly to the first
page.

Side effects of this change:
- Pagination now uses AJAX updates instead of full page reloads,
providing faster navigation. (This is marginal given how slow the Primo
API is.)
- Users will see a "Continue in Search Our Collections" page when
reaching Primo's practical pagination limits. It's notable that Primo UI
has the same bug as the Search API, so they will continue to experience
frustration at higher page counts regardless of which interface they
use.
- The URL updates provided by `data-turbo-action='advance'` also fixes
USE-110 (ensuring consistent browser state).
- Some autoformatting applied by VSCode around integers (e.g., 10000
becomes 10_000).
- I have serious concerns about the Primo Search API performance. It is
so slow that it times out occasionally after 60 seconds. This makes
pagination an inherently poor user experience, as the problem seems to
get worse at higher page numbers. I strongly urge us to reconsider
making Primo the default tab. If that is not an option, we should
evaluate creative workarounds. (E.g., perhaps a higher per-page count
would dissuade users from using pagination.) Caching alone is unlikely
to help in this case.

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

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

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>