Implement unified 'all' search tab

Why these changes are being introduced:

A key requirement for USE UI is a tab that combines results from
multiple sources. The current interface requires users to search each
source separately, creating friction in the discovery process.

Relevant ticket(s):

- [USE-176](https://mitlibraries.atlassian.net/browse/USE-176)
- [USE-177](https://mitlibraries.atlassian.net/browse/USE-177)

How this addresses that need:

- Add new 'all' tab as the default search mode
- Implement parallel API fetching using Ruby threads to query both
  Primo and TIMDEX simultaneously
- Use zipper merge pattern to interleave results from both sources
- Update view templates to render source-specific results for 'all' tab
- Add tab parameter validation to prevent invalid selections
- Change default tab from 'primo' to 'all' in ApplicationController

Side effects of this change:

- Default search behavior changes from Primo-only to combined results
- A `valid_tab?` method now confirms that the tab param is one of
'primo,' 'timdex', or 'all'
- Possible increased server load due to parallel API calls (I haven't
confirmed this, but it seems likely)
- Some peripherally related tests have been updated to account for new
behavior
- Pagination is not yet implemented (this will be done in USE-178 and
USE-179)

Author: jazairi

app/controllers/application_controller.rb

@@ -9,15 +9,21 @@ def set_active_tab
     @active_tab = if Feature.enabled?(:geodata)
                     # Determine which tab to load - default to primo unless gdt is enabled
                     'geodata'
-                  elsif params[:tab].present?
-                    # If params[:tab] is set, use it and set session
+                  elsif params[:tab].present? && valid_tab?(params[:tab])
+                    # If params[:tab] is set and valid, use it and set session
                     cookies[:last_tab] = params[:tab]
-                  elsif cookies[:last_tab].present?
-                    # Otherwise, check for last used tab in session
+                  elsif cookies[:last_tab].present? && valid_tab?(cookies[:last_tab])
+                    # Otherwise, check for last used tab in session if valid
                     cookies[:last_tab]
                   else
                     # Default behavior when no tab is specified in params or session
-                    cookies[:last_tab] = 'primo'
+                    cookies[:last_tab] = 'all'
                   end
   end
+
+  private
+
+  def valid_tab?(tab)
+    %w[primo timdex all].include?(tab)
+  end
 end

app/controllers/search_controller.rb

@@ -23,6 +23,8 @@ def results
       load_primo_results
     when 'timdex'
       load_timdex_results
+    when 'all'
+      load_all_results
     when 'geodata'
       load_gdt_results
       render 'results_geo'
@@ -66,49 +68,96 @@ def load_gdt_results
   end
 
   def load_primo_results
+    data = fetch_primo_data
+    @results = data[:results]
+    @pagination = data[:pagination]
+    @errors = data[:errors]
+    @show_primo_continuation = data[:show_continuation]
+  end
+
+  def load_timdex_results
+    data = fetch_timdex_data
+    @results = data[:results]
+    @pagination = data[:pagination]
+    @errors = data[:errors]
+  end
+
+  def load_all_results
+    # Parallel fetching from both APIs
+    primo_thread = Thread.new { fetch_primo_data }
+    timdex_thread = Thread.new { fetch_timdex_data }
+
+    # Wait for both threads to complete
+    primo_data = primo_thread.value
+    timdex_data = timdex_thread.value
+
+    # Collect any errors from either API
+    all_errors = []
+    all_errors.concat(primo_data[:errors]) if primo_data[:errors]
+    all_errors.concat(timdex_data[:errors]) if timdex_data[:errors]
+    @errors = all_errors.any? ? all_errors : nil
+
+    # Zipper merge results from both APIs
+    primo_results = primo_data[:results] || []
+    timdex_results = timdex_data[:results] || []
+    @results = primo_results.zip(timdex_results).flatten.compact
+
+    # For now, just use primo pagination as a placeholder
+    @pagination = primo_data[:pagination] || {}
+    
+    # Handle primo continuation for high page numbers
+    @show_primo_continuation = primo_data[:show_continuation] || false
+  end
+
+  def fetch_primo_data
     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
+      return { results: [], pagination: {}, errors: nil, show_continuation: true }
     end
 
     primo_response = query_primo
-    @results = NormalizePrimoResults.new(primo_response, @enhanced_query[:q]).normalize
+    results = NormalizePrimoResults.new(primo_response, @enhanced_query[:q]).normalize
+    pagination = Analyzer.new(@enhanced_query, primo_response, :primo).pagination
 
     # 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?
+    show_continuation = false
+    errors = nil
+    
+    if results.empty?
       docs = primo_response['docs'] if primo_response.is_a?(Hash)
       if docs.nil? || docs.empty?
         # Only show continuation for pagination scenarios (page > 1), not for searches with no results
-        @show_primo_continuation = true if current_page > 1
+        show_continuation = true if current_page > 1
       else
-        @errors = [{ 'message' => 'No more results available at this page number.' }]
+        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
+    { results: results, pagination: pagination, errors: errors, show_continuation: show_continuation }
   rescue StandardError => e
-    @errors = handle_primo_errors(e)
+    { results: [], pagination: {}, errors: handle_primo_errors(e), show_continuation: false }
   end
 
-  def load_timdex_results
+  def fetch_timdex_data
     query = QueryBuilder.new(@enhanced_query).query
     response = query_timdex(query)
-
-    @errors = extract_errors(response)
-    return unless @errors.nil?
-
-    @pagination = Analyzer.new(@enhanced_query, response, :timdex).pagination
-    raw_results = extract_results(response)
-    @results = NormalizeTimdexResults.new(raw_results, @enhanced_query[:q]).normalize
+    errors = extract_errors(response)
+    
+    if errors.nil?
+      pagination = Analyzer.new(@enhanced_query, response, :timdex).pagination
+      raw_results = extract_results(response)
+      results = NormalizeTimdexResults.new(raw_results, @enhanced_query[:q]).normalize
+      { results: results, pagination: pagination, errors: nil }
+    else
+      { results: [], pagination: {}, errors: errors }
+    end
   end
 
   def active_filters
@@ -123,7 +172,7 @@ def query_timdex(query)
     Rails.cache.fetch("#{cache_key}/#{@active_tab}", expires_in: 12.hours) do
       raw = if @active_tab == 'geodata'
               execute_geospatial_query(query)
-            elsif @active_tab == 'timdex'
+            elsif @active_tab == 'timdex' || @active_tab == 'all'
               TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query)
             end
 

app/views/search/_source_tabs.html.erb

@@ -1,6 +1,9 @@
   <!-- Tab Navigation -->
   <nav id="tabs" class="tab-navigation" aria-label="Result type navigation">
     <ul>
+      <li><%= link_to "All", results_path(params.permit(:q, :per_page, :booleanType, :tab).merge(tab: 'all')),
+                class: "tab-link #{'active' if @active_tab == 'all'}",
+                data: { turbo_frame: "search-results", turbo_action: "advance" } %></li>
       <li>
         <%= link_to "Primo", results_path(params.permit(:q, :per_page, :booleanType, :tab).merge(tab: 'primo')),
                 class: "tab-link #{'active' if @active_tab == 'primo'}",

app/views/search/results.html.erb

@@ -23,6 +23,14 @@
               <%= render(partial: 'search/result_primo', collection: @results, as: :result) %>
             <% when 'timdex' %>
               <%= render(partial: 'search/result', collection: @results, as: :result) %>
+            <% when 'all' %>
+              <% @results.each do |result| %>
+                <% if result[:source] == 'Primo' %>
+                  <%= render(partial: 'search/result_primo', locals: { result: result }) %>
+                <% else %>
+                  <%= render(partial: 'search/result', locals: { result: result }) %>
+                <% end %>
+              <% end %>
             <% end %>
           </ol>
         <% elsif @errors.blank? %>

test/controllers/application_controller_test.rb

@@ -1,17 +1,18 @@
 require 'test_helper'
 
 class ApplicationControllerTest < ActionDispatch::IntegrationTest
-  test 'set_active_tab sets default to primo when no feature flag or params' do
+  test 'set_active_tab sets default to all when no feature flag or params' do
     assert_nil cookies[:last_tab]
 
     get root_path
     assert_select '#tab-to-target' do
-      assert_select '[value=?]', 'primo'
+      assert_select '[value=?]', 'all'
       refute_select '[value=?]', 'geodata'
+      refute_select '[value=?]', 'primo'
       refute_select '[value=?]', 'timdex'
     end
 
-    assert_equal cookies[:last_tab], 'primo'
+    assert_equal cookies[:last_tab], 'all'
   end
 
   test 'set_active_tab sets to geodata when feature flag enabled' do
@@ -32,7 +33,7 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
       get root_path, params: { tab: 'primo' }
       assert_select '#tab-to-target' do
         refute_select '[value=?]', 'primo'
-        assert_select '[value=?]', 'geodata'
+        assert_select '[value=?]', 'all'
         refute_select '[value=?]', 'timdex'
       end
     end
@@ -43,31 +44,95 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
 
     assert_select '#tab-to-target' do
       refute_select '[value=?]', 'primo'
-      refute_select '[value=?]', 'geodata'
+      refute_select '[value=?]', 'all'
       assert_select '[value=?]', 'timdex'
     end
   end
 
   test 'set_active_tab sets to param tab when provided even if cookie is set and updates cookie' do
     cookies[:last_tab] = 'timdex'
-    get root_path, params: { tab: 'geodata' }
+    get root_path, params: { tab: 'primo' }
 
     assert_select '#tab-to-target' do
-      refute_select '[value=?]', 'primo'
-      assert_select '[value=?]', 'geodata'
+      assert_select '[value=?]', 'primo'
+      refute_select '[value=?]', 'geodata'
       refute_select '[value=?]', 'timdex'
+      refute_select '[value=?]', 'all'
     end
 
-    assert_equal cookies[:last_tab], 'geodata'
+    assert_equal cookies[:last_tab], 'primo'
   end
 
   test 'set_active_tab uses cookie last_tab when no param provided' do
     cookies[:last_tab] = 'timdex'
     get root_path
     assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'all'
       refute_select '[value=?]', 'primo'
-      refute_select '[value=?]', 'geodata'
       assert_select '[value=?]', 'timdex'
     end
   end
+
+  test 'valid_tab returns true for valid tabs' do
+    app_controller = ApplicationController.new
+
+    assert app_controller.send(:valid_tab?, 'primo')
+    assert app_controller.send(:valid_tab?, 'timdex') 
+    assert app_controller.send(:valid_tab?, 'all')
+  end
+
+  test 'valid_tab returns false for invalid tabs' do
+    app_controller = ApplicationController.new
+
+    refute app_controller.send(:valid_tab?, 'foo')
+    refute app_controller.send(:valid_tab?, '')
+    refute app_controller.send(:valid_tab?, nil)
+  end
+
+  test 'set_active_tab ignores invalid tab parameter and uses default' do
+    get root_path, params: { tab: 'invalid_tab' }
+
+    assert_select '#tab-to-target' do
+      assert_select '[value=?]', 'all'
+      refute_select '[value=?]', 'invalid_tab'
+    end
+
+    assert_equal cookies[:last_tab], 'all'
+  end
+
+  test 'set_active_tab ignores invalid cookie value and uses default' do
+    cookies[:last_tab] = 'invalid_cookie_value'
+    get root_path
+
+    assert_select '#tab-to-target' do
+      assert_select '[value=?]', 'all'
+      refute_select '[value=?]', 'invalid_cookie_value'
+    end
+
+    assert_equal cookies[:last_tab], 'all'
+  end
+
+  test 'set_active_tab prioritizes valid param over invalid cookie' do
+    cookies[:last_tab] = 'invalid_cookie'
+    get root_path, params: { tab: 'timdex' }
+
+    assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'invalid_cookie'
+      assert_select '[value=?]', 'timdex'
+    end
+
+    assert_equal cookies[:last_tab], 'timdex'
+  end
+
+  test 'set_active_tab falls back to valid cookie when param is invalid' do
+    cookies[:last_tab] = 'primo'
+    get root_path, params: { tab: 'foo' }
+
+    assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'foo'
+      assert_select '[value=?]', 'primo'
+    end
+
+    assert_equal cookies[:last_tab], 'primo'
+  end
 end