Primo/timdex partials respected

* Adds a lot more tests of the application controller logic by introducing unit tests in addition to Integration tests
* Adds an `api` key to both normalized timdex and primo records that we can use in the views
* Removes redundant logic for `all` tab in views and normalizes to use newly introduced `api` key
* Ensures timdex queries always run when they should be removing condition that unnecessarily complicated all tab queries

Author: JPrevost

app/controllers/application_controller.rb

@@ -22,16 +22,20 @@ def set_active_tab
   end
 
   def primo_tabs
-    %w[all alma cdi primo]
+    %w[alma cdi primo]
   end
 
   def timdex_tabs
-    %w[all aspace timdex timdex_alma website]
+    %w[aspace timdex timdex_alma website]
+  end
+
+  def all_tabs
+    ['all', *primo_tabs, *timdex_tabs]
   end
 
   private
 
   def valid_tab?(tab)
-    (primo_tabs << timdex_tabs).flatten.uniq.include?(tab)
+    all_tabs.include?(tab)
   end
 end

app/controllers/search_controller.rb

@@ -181,7 +181,7 @@ def query_timdex(query)
     Rails.cache.fetch("#{cache_key}/#{@active_tab}", expires_in: 12.hours) do
       raw = if Feature.enabled?(:geodata)
               execute_geospatial_query(query)
-            elsif timdex_tabs.include? @active_tab
+            else
               TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query)
             end
 

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/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_tabs %>
-                <%= render(partial: 'search/result_primo', collection: @results, as: :result) %>
-              <% when *@timdex_tabs %>
-                <%= 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

test/controllers/search_controller_test.rb

@@ -4,6 +4,7 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
   def mock_primo_search_success
     # Mock the Primo search components to avoid external API calls
     sample_doc = {
+      api: 'primo',
       title: 'Sample Primo Document Title',
       format: 'Article',
       year: '2025',
@@ -26,6 +27,7 @@ def mock_primo_search_success
   def mock_timdex_search_success
     # Mock the TIMDEX GraphQL client to avoid external API calls
     sample_result = {
+      'api' => 'timdex',
       'title' => 'Sample TIMDEX Document Title',
       'timdexRecordId' => 'sample-record-123',
       'contentType' => [{ 'value' => 'Article' }],

test/models/normalize_primo_record_test.rb

@@ -423,4 +423,9 @@ def cdi_record
     assert_not_nil dedup_url
     assert_match %r{/discovery/search\?}, dedup_url
   end
+
+  test 'includes primo as source api' do
+    normalized = NormalizePrimoRecord.new(full_record, 'test query').normalize
+    assert_equal 'primo', normalized[:api]
+  end
 end

test/models/normalize_primo_results_test.rb

@@ -67,4 +67,13 @@ def empty_results
     normalizer = NormalizePrimoResults.new(nil, 'test query')
     assert_equal 0, normalizer.total_results
   end
+
+  test 'all primo results includes primo as source api' do
+    normalizer = NormalizePrimoResults.new(sample_results, 'test query')
+    normalized = normalizer.normalize
+
+    normalized.each do |result|
+      assert_equal 'primo', result[:api]
+    end
+  end
 end

test/models/normalize_timdex_record_test.rb

@@ -218,4 +218,9 @@ def minimal_record
     assert_not_includes normalized.keys, 'other_availability'
     assert_not_includes normalized.keys, 'container'
   end
+
+  test 'includes timdex as source api' do
+    normalized = NormalizeTimdexRecord.new(full_record, 'test').normalize
+    assert_equal 'timdex', normalized[:api]
+  end
 end