Merge pull request #278 from MITLibraries/use-187-timdex-tabs

Add new search tabs and improve tab handling

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,21 @@ def set_active_tab
                   end
   end
 
+  def primo_tabs
+    %w[alma cdi primo]
+  end
+
+  def timdex_tabs
+    %w[aspace timdex timdex_alma website]
+  end
+
+  def all_tabs
+    ['all', *primo_tabs, *timdex_tabs]
+  end
+
   private
 
   def valid_tab?(tab)
-    %w[primo timdex all].include?(tab)
+    all_tabs.include?(tab)
   end
 end

app/controllers/search_controller.rb

@@ -23,15 +23,15 @@ def results
       render 'results_geo'
       return
     end
-    
+
     # Route to appropriate search based on active tab
     case @active_tab
-    when 'primo'
-      load_primo_results
-    when 'timdex'
-      load_timdex_results
     when 'all'
       load_all_results
+    when *primo_tabs
+      load_primo_results
+    when *timdex_tabs
+      load_timdex_results
     end
   end
 
@@ -108,7 +108,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 +119,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 +131,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 +151,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 +167,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'
+            else
               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
+    tab_label = label || target
+    if @active_tab == clean_target
+      link_to tab_label, 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 tab_label, 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/models/normalize_primo_record.rb

@@ -8,6 +8,7 @@ def initialize(record, query)
   def normalize
     {
       # Core fields
+      api: 'primo',
       title:,
       creators:,
       source:,

app/models/normalize_timdex_record.rb

@@ -8,6 +8,7 @@ def initialize(record, query)
   def normalize
     {
       # Core fields
+      api: 'timdex',
       title:,
       creators:,
       source:,

app/views/search/_source_tabs.html.erb

@@ -2,8 +2,15 @@
   <nav id="tabs" class="tab-navigation" aria-label="Result type navigation">
     <ul>
       <li><%= link_to_tab("All") %></li>
-      <li><%= link_to_tab("Primo") %></li>
-      <li><%= link_to_tab("TIMDEX") %></li>
+      <li><%= link_to_tab("Primo", "Articles and Catalog") %></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

@@ -22,21 +22,14 @@
         <p class="results-context-description">From all MIT Libraries sources</p>    
         <div id="results-layout-wrapper">  
           <main id="results">
-            <ol class="results-list" start="<%= @pagination[:start] %>">
-              <% case @active_tab %>
-              <% when 'primo' %>
-                <%= 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' %>
+            <ol class="results-list" start="<%= @pagination[:start] %>">             
+              <% @results.each do |result| %>
+                  <% if result[:api] == 'primo' %>
                     <%= render(partial: 'search/result_primo', locals: { result: result }) %>
-                  <% else %>
+                  <% elsif result[:api] == 'timdex' %>
                     <%= render(partial: 'search/result', locals: { result: result }) %>
                   <% end %>
                 <% end %>
-              <% end %>
             </ol>
             <%= render partial: "results_callouts" %>
           </main>

test/controllers/application_controller_unit_test.rb

@@ -0,0 +1,66 @@
+require 'test_helper'
+
+class ApplicationControllerUnitTest < ActionController::TestCase
+  setup do
+    @controller = ApplicationController.new
+  end
+
+  test '#primo_tabs returns expected tabs' do
+    assert_equal %w[alma cdi primo], @controller.primo_tabs
+  end
+
+  test '#timdex_tabs returns expected tabs' do
+    assert_equal %w[aspace timdex timdex_alma website], @controller.timdex_tabs
+  end
+
+  test '#all_tabs includes both primo and timdex tabs as well as all tab' do
+    expected = %w[all alma cdi primo aspace timdex timdex_alma website]
+    assert_equal expected, @controller.all_tabs
+  end
+
+  test '#valid_tab? returns true for valid tabs' do
+    assert @controller.send(:valid_tab?, 'all')
+    assert @controller.send(:valid_tab?, 'alma')
+    assert @controller.send(:valid_tab?, 'timdex')
+  end
+
+  test '#valid_tab? returns false for invalid tabs' do
+    refute @controller.send(:valid_tab?, 'invalid')
+    refute @controller.send(:valid_tab?, '')
+  end
+
+  test 'set_active_tab defaults @active_tab to all when no params or cookies are set' do
+    @controller.stubs(:params).returns({})
+    @controller.stubs(:cookies).returns({})
+    @controller.set_active_tab
+    assert_equal 'all', @controller.instance_variable_get(:@active_tab)
+  end
+
+  test 'set_active_tab sets @active_tab to tab when tab params is valid' do
+    @controller.stubs(:params).returns({ tab: 'primo' })
+    @controller.stubs(:cookies).returns({})
+    @controller.set_active_tab
+    assert_equal 'primo', @controller.instance_variable_get(:@active_tab)
+  end
+
+  test 'set_active_tab sets @active_tab to all when tab params is invalid and no cookie is set' do
+    @controller.stubs(:params).returns({ tab: 'supertab' })
+    @controller.stubs(:cookies).returns({})
+    @controller.set_active_tab
+    assert_equal 'all', @controller.instance_variable_get(:@active_tab)
+  end
+
+  test 'set_active_tab sets @active_tab to cookie value when tab params is invalid and valid cookie is set' do
+    @controller.stubs(:params).returns({ tab: 'supertab' })
+    @controller.stubs(:cookies).returns({ last_tab: 'timdex' })
+    @controller.set_active_tab
+    assert_equal 'timdex', @controller.instance_variable_get(:@active_tab)
+  end
+
+  test 'set_active_tab sets @active_tab to all value when tab params is invalid and cookie is invalid' do
+    @controller.stubs(:params).returns({ tab: 'supertab' })
+    @controller.stubs(:cookies).returns({ last_tab: 'woohoo' })
+    @controller.set_active_tab
+    assert_equal 'all', @controller.instance_variable_get(:@active_tab)
+  end
+end