Adds Primo API-based tabs

Why are these changes being introduced:

* Breaking out Primo results into Articles (CDI) and Catalog (Alma)
  tabs is how we want to present Primo results to users when they are
  not using the All tab

Relevant ticket(s):

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

How does this address that need:

* Adds new tabs for Articles and Catalog in the search results UI
* Creates Feature flag `primo_tabs` to enable/disable full Primo results
* Modifies PrimoSearch to accept a new `tab` parameter to set the scope
* Adds documentation as to what Primo Scopes and Tabs mean and what
  values are available

Document any side effects to this change:

* I've moved some Primo Scope and Tab values to code rather than ENV as
  they are more tightly coupled to the Primo API behavior than ENV
  implies. Additionally, we use those ENV values to construct links in
  the UI and those values need to be different than the API calls due to
  our desire to send users to a specific UI regardless of how we queried
  the API. The UI values are still set via ENV at this time.

Author: JPrevost

README.md

@@ -98,8 +98,9 @@ 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
+- `FEATURE_TAB_PRIMO_ALL`: Display a tab for displaying the combined Primo data (CDI + Alma)
+- `FEATURE_TAB_TIMDEX_ALL`: Display a tab for displaying the combined TIMDEX data. `TIMDEX_INDEX` affects which data appears in this tab.
+- `FEATURE_TAB_TIMDEX_ALMA`: Display a tab for displaying Alma data from TIMDEX. `TIMDEX_INDEX` must include `Alma` data or no results will return.
 - `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/search_controller.rb

@@ -139,7 +139,7 @@ def fetch_primo_data
     primo_response = query_primo(per_page, offset)
     hits = primo_response.dig('info', 'total') || 0
     results = NormalizePrimoResults.new(primo_response, @enhanced_query[:q]).normalize
-    pagination = Analyzer.new(@enhanced_query, hits , :primo).pagination
+    pagination = Analyzer.new(@enhanced_query, hits, :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
@@ -226,7 +226,7 @@ def query_primo(per_page, offset)
     cache_key = generate_cache_key(@enhanced_query)
 
     Rails.cache.fetch("#{cache_key}/primo", expires_in: 12.hours) do
-      primo_search = PrimoSearch.new
+      primo_search = PrimoSearch.new(@enhanced_query[:tab])
       primo_search.search(@enhanced_query[:q], per_page, offset)
     end
   end

app/models/feature.rb

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

app/models/primo_search.rb

@@ -1,12 +1,23 @@
 # Searches Primo Search API and formats results
 #
 class PrimoSearch
-  def initialize
+  # Initializes the PrimoSearch with the given query parameters
+  # @param tab [String] Current `active_tab` value from SearchController. Used to set Primo tab/scope.
+  #   Defaults to 'all'.
+  # @return [PrimoSearch] An instance of PrimoSearch
+  def initialize(tab = 'all')
+    @tab = tab
+
     validate_env
     @primo_http = HTTP.persistent(primo_api_url)
     @results = {}
   end
 
+  # Performs a search against the Primo API
+  # @param term [String] The search term
+  # @param per_page [Integer] Number of results per page
+  # @param offset [Integer] The result offset for pagination
+  # @return [Hash] Parsed JSON response from Primo API
   def search(term, per_page, offset = 0)
     url = search_url(term, per_page, offset)
     result = @primo_http.timeout(http_timeout)
@@ -46,14 +57,39 @@ def primo_api_key
     ENV.fetch('PRIMO_API_KEY', nil)
   end
 
+  # In Primo API, Search scopes determines which records are being searched
+  # Primo VE configuration calls this `Search Profiles` and uses `Scope` differently.
+  # For Primo VE API we want to be doing &scope={Primo VE Search Profile}
+  #
+  # Available scopes for USE
+  #   all_use: includes CDI configured like Primo UI + Alma (does not include ASpace)
+  #   all: same as all_use but includes ASpace
+  #   catalog_use: just Alma
+  #   cdi_use: just CDI configured like Primo UI
+  #
+  # The scope we use will be driven by the tab provided during initialization
   def primo_scope
-    ENV.fetch('PRIMO_SCOPE', nil)
+    case @tab
+    when 'cdi'
+      'cdi_use'
+    when 'alma'
+      'catalog_use'
+    else
+      'all_use'
+    end
   end
 
+  # In Primo, Tabs act as "search scope slots". They contain one or more Search Profile (which Primo API calls `scopes`).
+  # Primo VE configuration refers to these as `Search Profile Slots`
+  # Configured tabs in our Primo
+  #   all: scopes(all, all_filtered, catalog, cdi, CourseReserves)
+  #   bento: scopes(cdi, catalog, bento_catalog, all_use)
+  #   USE: scopes(all_use, all, catalog_use, cdi_use)
   def primo_tab
-    ENV.fetch('PRIMO_TAB', nil)
+    'use'
   end
 
+  # In Primo API, a vid is...[documentaiton needed!]
   def primo_vid
     ENV.fetch('PRIMO_VID', nil)
   end

app/views/search/_source_tabs.html.erb

@@ -2,7 +2,14 @@
   <nav id="tabs" class="tab-navigation" aria-label="Result type navigation">
     <ul>
       <li><%= link_to_tab("All") %></li>
-      <li><%= link_to_tab("Primo", "Articles and Catalog") %></li>
+
+      <% if Feature.enabled?(:tab_primo_all) %>
+        <li><%= link_to_tab("Primo", "Articles and Catalog") %></li>
+      <% end %>
+
+      <li><%= link_to_tab("cdi", "Articles") %></li>
+      <li><%= link_to_tab("alma", "Catalog") %></li>
+
       <% if Feature.enabled?(:tab_timdex_all) %>
         <li><%= link_to_tab("TIMDEX") %></li>
       <% end %>

test/controllers/search_controller_test.rb

@@ -655,11 +655,13 @@ def source_filter_count(controller)
   end
 
   test 'results respects primo tab parameter' do
-    mock_primo_search_success
+    ClimateControl.modify FEATURE_TAB_PRIMO_ALL: 'true' do
+      mock_primo_search_success
 
-    get '/results?q=test&tab=primo'
-    assert_response :success
-    assert_select 'a.tab-link.active[href*="tab=primo"]', count: 1
+      get '/results?q=test&tab=primo'
+      assert_response :success
+      assert_select 'a.tab-link.active[href*="tab=primo"]', count: 1
+    end
   end
 
   test 'results respects timdex all tab parameter' do
@@ -688,7 +690,10 @@ def source_filter_count(controller)
     get '/results?q=test&tab=primo'
     assert_response :success
     assert_select '.tab-navigation', count: 1
-    assert_select 'a[href*="tab=primo"]', count: 1
+    assert_select 'a[href*="tab=all"]', count: 1
+    assert_select 'a[href*="tab=cdi"]', count: 1
+    assert_select 'a[href*="tab=alma"]', count: 1
+    # assert_select 'a[href*="tab=primo"]', 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

test/models/primo_search_test.rb

@@ -28,12 +28,11 @@ class PrimoSearchTest < ActiveSupport::TestCase
   end
 
   test 'raises error when multiple environment variables are missing' do
-    ClimateControl.modify(PRIMO_API_URL: nil, PRIMO_SCOPE: nil, PRIMO_VID: nil) do
+    ClimateControl.modify(PRIMO_API_URL: nil, PRIMO_VID: nil) do
       error = assert_raises(ArgumentError) do
         PrimoSearch.new
       end
       assert_match(/PRIMO_API_URL/, error.message)
-      assert_match(/PRIMO_SCOPE/, error.message)
       assert_match(/PRIMO_VID/, error.message)
     end
   end