Add new search tabs and improve tab handling

Why are these changes being introduced:

* `website` and `aspace` tabs were needed in the search interface to
  allow users to filter results by source
* The `timdex` tab is not needed for user facing interfaces, but is
  useful for internal testing and development, so a feature flag
  was added to control its visibility
* The `timdex_alma` tab will be useful to help us decide if we want a TIMDEX or Primo based Alma tab

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/USE-187

How does this address that need:

* Introduces 'website' and 'aspace' tabs to the search interface
* refactors tab logic to support dynamic tab lists
* adds a feature flag for displaying the TIMDEX tab
* Updates tab rendering, controller logic, and tests to accommodate new
  tabs and feature flag.
* Adds support for tabs with spaces in their names
* Adds support for tab labels that differ from their internal names
  which will be important to allow UXWS to change labels without having
  to update valid tab lists in the code.

Author: JPrevost

README.md

@@ -98,6 +98,8 @@ See `Optional Environment Variables` for more information.
 mode. Note that this is currently intended _only_ for the geodata app and
 may have unexpected consequences if applied to other TIMDEX UI apps.
 - `FEATURE_SIMULATE_SEARCH_LATENCY`: DO NOT SET IN PRODUCTION. Set to ensure a minimum of a one second delay in returning search results. Useful to see spinners/loaders. Only introduces delay for results that take less than one second to complete.
+- `FEATURE_TAB_TIMDEX_ALL`: Display a tab for displaying the combined TIMDEX data
+- `FEATURE_TAB_TIMDEX_ALMA`: Display a tab for displaying Alma data from TIMDEX
 - `FILTER_ACCESS_TO_FILES`: The name to use instead of "Access to files" for that filter / aggregation.
 - `FILTER_CONTENT_TYPE`: The name to use instead of "Content type" for that filter / aggregation.
 - `FILTER_CONTRIBUTOR`: The name to use instead of "Contributor" for that filter / aggregation.

app/controllers/application_controller.rb

@@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base
   def set_active_tab
     # GeoData doesn't use the tab system.
     return if Feature.enabled?(:geodata)
-    
+
     @active_tab = if params[:tab].present? && valid_tab?(params[:tab])
                     # If params[:tab] is set and valid, use it and set session
                     cookies[:last_tab] = params[:tab]
@@ -21,9 +21,17 @@ def set_active_tab
                   end
   end
 
+  def primo_tabs
+    %w[all alma cdi primo]
+  end
+
+  def timdex_tabs
+    %w[all aspace timdex timdex_alma website]
+  end
+
   private
 
   def valid_tab?(tab)
-    %w[primo timdex all].include?(tab)
+    (primo_tabs << timdex_tabs).flatten.uniq.include?(tab)
   end
 end

app/controllers/search_controller.rb

@@ -23,15 +23,18 @@ def results
       render 'results_geo'
       return
     end
-    
+
+    @primo_tabs = primo_tabs
+    @timdex_tabs = timdex_tabs
+
     # Route to appropriate search based on active tab
     case @active_tab
-    when 'primo'
+    when 'all' # all tab must come first to avoid matching primo or timdex arrays which contain all
+      load_all_results
+    when *primo_tabs
       load_primo_results
-    when 'timdex'
+    when *timdex_tabs
       load_timdex_results
-    when 'all'
-      load_all_results
     end
   end
 
@@ -108,7 +111,7 @@ def load_all_results
 
     # 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
@@ -119,9 +122,7 @@ def fetch_primo_data
     offset = (current_page - 1) * per_page
 
     # Check if we're beyond Primo API limits before making the request.
-    if offset >= Analyzer::PRIMO_MAX_OFFSET
-      return { results: [], pagination: {}, errors: nil, show_continuation: true }
-    end
+    return { results: [], pagination: {}, errors: nil, show_continuation: true } if offset >= Analyzer::PRIMO_MAX_OFFSET
 
     primo_response = query_primo
     results = NormalizePrimoResults.new(primo_response, @enhanced_query[:q]).normalize
@@ -133,7 +134,7 @@ def fetch_primo_data
     # exists in Primo UI, sending users there seems like the best we can do.
     show_continuation = false
     errors = nil
-    
+
     if results.empty?
       docs = primo_response['docs'] if primo_response.is_a?(Hash)
       if docs.nil? || docs.empty?
@@ -153,7 +154,7 @@ def fetch_timdex_data
     query = QueryBuilder.new(@enhanced_query).query
     response = query_timdex(query)
     errors = extract_errors(response)
-    
+
     if errors.nil?
       pagination = Analyzer.new(@enhanced_query, response, :timdex).pagination
       raw_results = extract_results(response)
@@ -169,14 +170,18 @@ def active_filters
   end
 
   def query_timdex(query)
+    query[:sourceFilter] = ['MIT Libraries Website', 'LibGuides'] if @active_tab == 'website'
+    query[:sourceFilter] = ['MIT ArchivesSpace'] if @active_tab == 'aspace'
+    query[:sourceFilter] = ['MIT Alma'] if @active_tab == 'timdex_alma'
+
     # We generate unique cache keys to avoid naming collisions.
     cache_key = generate_cache_key(query)
 
     # Builder hands off to wrapper which returns raw results here.
     Rails.cache.fetch("#{cache_key}/#{@active_tab}", expires_in: 12.hours) do
       raw = if Feature.enabled?(:geodata)
               execute_geospatial_query(query)
-            elsif @active_tab == 'timdex' || @active_tab == 'all'
+            elsif timdex_tabs.include? @active_tab
               TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query)
             end
 

app/helpers/search_helper.rb

@@ -22,16 +22,22 @@ def link_to_result(result)
     end
   end
 
-  def link_to_tab(target)
-    if @active_tab == target.downcase
-      link_to target, results_path(params.permit(:q, :per_page, :booleanType, :tab).merge(tab: target.downcase)),
-        aria: { current: "page" },
-        class: "active tab-link",
-        data: { turbo_frame: "search-results", turbo_action: "advance" }
+  # Generates a link for a search results tab, marking it as active if it matches the current active tab.
+  # @param target [String] The name of the tab to link to
+  # @param label [String] The display label for the tab (optional)
+  # @return [String] HTML link element for the tab
+  def link_to_tab(target, label = nil)
+    clean_target = target.downcase.gsub(' ', '_').downcase
+    target = label || target
+    if @active_tab == clean_target
+      link_to target, results_path(params.permit(:q, :per_page, :booleanType, :tab).merge(tab: clean_target)),
+              aria: { current: 'page' },
+              class: 'active tab-link',
+              data: { turbo_frame: 'search-results', turbo_action: 'advance' }
     else
-      link_to target, results_path(params.permit(:q, :per_page, :booleanType, :tab).merge(tab: target.downcase)),
-        class: "tab-link",
-        data: { turbo_frame: "search-results", turbo_action: "advance" }
+      link_to target, results_path(params.permit(:q, :per_page, :booleanType, :tab).merge(tab: clean_target)),
+              class: 'tab-link',
+              data: { turbo_frame: 'search-results', turbo_action: 'advance' }
     end
   end
 

app/models/feature.rb

@@ -33,7 +33,7 @@
 #
 class Feature
   # List of all valid features in the application
-  VALID_FEATURES = %i[geodata boolean_picker simulate_search_latency].freeze
+  VALID_FEATURES = %i[geodata boolean_picker simulate_search_latency tab_timdex_all tab_timdex_alma].freeze
 
   # Check if a feature is enabled by name
   #

app/views/search/_source_tabs.html.erb

@@ -3,7 +3,14 @@
     <ul>
       <li><%= link_to_tab("All") %></li>
       <li><%= link_to_tab("Primo") %></li>
-      <li><%= link_to_tab("TIMDEX") %></li>
+      <% if Feature.enabled?(:tab_timdex_all) %>
+        <li><%= link_to_tab("TIMDEX") %></li>
+      <% end %>
+      <% if Feature.enabled?(:tab_timdex_alma) %>
+        <li><%= link_to_tab("timdex_alma", "Alma (TIMDEX)") %></li>
+      <% end %>
+      <li><%= link_to_tab("Website") %></li>
+      <li><%= link_to_tab("aspace", "Archival materials") %></li>
     </ul>
   </nav>
 

app/views/search/results.html.erb

@@ -24,9 +24,9 @@
           <main id="results">
             <ol class="results-list" start="<%= @pagination[:start] %>">
               <% case @active_tab %>
-              <% when 'primo' %>
+              <% when *@primo_tabs %>
                 <%= render(partial: 'search/result_primo', collection: @results, as: :result) %>
-              <% when 'timdex' %>
+              <% when *@timdex_tabs %>
                 <%= render(partial: 'search/result', collection: @results, as: :result) %>
               <% when 'all' %>
                 <% @results.each do |result| %>

test/controllers/search_controller_test.rb

@@ -593,12 +593,24 @@ def source_filter_count(controller)
     assert_select 'a.tab-link.active[href*="tab=primo"]', count: 1
   end
 
-  test 'results respects timdex tab parameter' do
-    mock_timdex_search_success
+  test 'results respects timdex all tab parameter' do
+    ClimateControl.modify FEATURE_TAB_TIMDEX_ALL: 'true' 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
+      get '/results?q=test&tab=timdex'
+      assert_response :success
+      assert_select 'a.tab-link.active[href*="tab=timdex"]', count: 1
+    end
+  end
+
+  test 'results respects timdex alma tab parameter' do
+    ClimateControl.modify FEATURE_TAB_TIMDEX_ALMA: 'true' do
+      mock_timdex_search_success
+
+      get '/results?q=test&tab=timdex_alma'
+      assert_response :success
+      assert_select 'a.tab-link.active[href*="tab=timdex_alma"]', count: 1
+    end
   end
 
   test 'results shows tab navigation when GeoData is disabled' do
@@ -608,7 +620,9 @@ def source_filter_count(controller)
     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
+    # assert_select 'a[href*="tab=timdex"]', count: 1
+    assert_select 'a[href*="tab=aspace"]', count: 1
+    assert_select 'a[href*="tab=website"]', count: 1
   end
 
   test 'results handles primo search errors gracefully' do
@@ -726,7 +740,7 @@ def source_filter_count(controller)
 
     get '/results?q=test&tab=all'
     assert_response :success
-    
+
     # Verify that we get results from both sources
     assert_select '.record-title', text: /Sample Primo Document Title/
     assert_select '.record-title', text: /Sample TIMDEX Document Title/
@@ -768,7 +782,7 @@ def source_filter_count(controller)
 
     get '/results?q=test&tab=all&page=49'
     assert_response :success
-    
+
     # Should show primo continuation partial
     assert_select '.primo-continuation', count: 1
     assert_select '.primo-continuation h2', text: /Continue your search in Search Our Collections/