From 42bbbf40ce6ef01218ab3f26fca329e18c3ab39c Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Mon, 17 Nov 2025 14:51:34 -0500 Subject: [PATCH 1/5] 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. --- README.md | 2 ++ app/controllers/application_controller.rb | 12 +++++++-- app/controllers/search_controller.rb | 29 ++++++++++++--------- app/helpers/search_helper.rb | 24 ++++++++++------- app/models/feature.rb | 2 +- app/views/search/_source_tabs.html.erb | 9 ++++++- app/views/search/results.html.erb | 4 +-- test/controllers/search_controller_test.rb | 30 ++++++++++++++++------ 8 files changed, 77 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 0ffc090f..a77eac57 100644 --- a/README.md +++ b/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. diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6bcec29c..5b0191b2 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 3157eab0..4a2ce4b3 100644 --- a/app/controllers/search_controller.rb +++ b/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,6 +170,10 @@ 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) @@ -176,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 @active_tab == 'timdex' || @active_tab == 'all' + elsif timdex_tabs.include? @active_tab TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query) end diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index b8f4ea34..85301d72 100644 --- a/app/helpers/search_helper.rb +++ b/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 diff --git a/app/models/feature.rb b/app/models/feature.rb index b36fd77e..1a40e35d 100644 --- a/app/models/feature.rb +++ b/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 # diff --git a/app/views/search/_source_tabs.html.erb b/app/views/search/_source_tabs.html.erb index d0f73c17..1454907c 100644 --- a/app/views/search/_source_tabs.html.erb +++ b/app/views/search/_source_tabs.html.erb @@ -3,7 +3,14 @@ \ No newline at end of file diff --git a/app/views/search/results.html.erb b/app/views/search/results.html.erb index eee01f2e..644cf69d 100644 --- a/app/views/search/results.html.erb +++ b/app/views/search/results.html.erb @@ -24,9 +24,9 @@
    <% 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| %> diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb index c652d1be..e8aa17b6 100644 --- a/test/controllers/search_controller_test.rb +++ b/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/ From 8290364ee1e11e75f515541bf2e4e6e05c110ea9 Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Tue, 18 Nov 2025 11:22:20 -0500 Subject: [PATCH 2/5] Update Primo tab display name --- app/views/search/_source_tabs.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/search/_source_tabs.html.erb b/app/views/search/_source_tabs.html.erb index 1454907c..c1d23ebc 100644 --- a/app/views/search/_source_tabs.html.erb +++ b/app/views/search/_source_tabs.html.erb @@ -2,7 +2,7 @@