Adds persistence to Tab selection across searches

Why are these changes being introduced:

* Running multiple searches always reset to the Primo tab. It is
  expected that isn't what users want. They want to stay on the tab
  they selected.

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/USE-135
* https://mitlibraries.atlassian.net/browse/USE-156

How does this address that need:

* Updates the tab links to set a cookie with the selected tab
  when clicked.
* Updates the search form to include the active tab as a hidden
  field so that when a new search is performed, the selected tab
  is preserved.
* Updates the Tabs Turbo Frame to target the whole page to allow the
  form element to be updated
  * This also fixed the issue with back links not updating the form
    keyword input correctly which was a separately reported bug.

Document any side effects to this change:

* Refactor to extract some logic from the SearchController to
ApplicationController to allow reuse in BasicSearchController.
* Minor refactor to conditional logic to always use case and eliminate
  the if statement that was used only for GeoData. Also normalized to
  `geodata` language (was `gdt`) to match other wording used in the app.

Author: JPrevost

README.md

@@ -82,6 +82,7 @@ See `Optional Environment Variables` for more information.
 - `PRIMO_SCOPE`: The Primo Search API `scope` param (set to `cdi` for CDI-scoped results).
 - `PRIMO_TAB`: The Primo Search API `tab` param (typically `all`).
 - `PRIMO_VID`: The Primo Search API `vid` (or 'view ID`) param.
+- `SECRET_KEY_BASE`: You can generate this via `bin/rails secret`. Please do not re-use the production value locally.
 - `SYNDETICS_PRIMO_URL`: The Syndetics API URL for Primo. This is used to construct thumbnail URLs.
 - `TIMDEX_GRAPHQL`: Set this to the URL of the GraphQL endpoint. There is no default value in the application.
 

app/assets/stylesheets/partials/_search.scss

@@ -196,3 +196,8 @@
 .tab-link {
   color: #fff;
 }
+
+/* temp style to visualize active tab. Save us from this Dave! */
+#tabs .active{
+  background-color: red;;
+}

app/controllers/application_controller.rb

@@ -1,3 +1,23 @@
 class ApplicationController < ActionController::Base
   helper Mitlibraries::Theme::Engine.helpers
+
+  # Set active tab based on feature flag and params
+  # Also stores the last used tab in a cookie for future searches when passed via params.
+  # We set this in a session cookie to persist user preference across searches.
+  # Clicking on a different tab will update the cookie.
+  def set_active_tab
+    @active_tab = if Feature.enabled?(:geodata)
+                    # Determine which tab to load - default to primo unless gdt is enabled
+                    'geodata'
+                  elsif params[:tab].present?
+                    # If params[:tab] is set, use it and set session
+                    cookies[:last_tab] = params[:tab]
+                  elsif cookies[:last_tab].present?
+                    # Otherwise, check for last used tab in session
+                    cookies[:last_tab]
+                  else
+                    # Default behavior when no tab is specified in params or session
+                    cookies[:last_tab] = 'primo'
+                  end
+  end
 end

app/controllers/basic_search_controller.rb

@@ -1,3 +1,5 @@
 class BasicSearchController < ApplicationController
+  before_action :set_active_tab
+
   def index; end
 end

app/controllers/search_controller.rb

@@ -1,5 +1,6 @@
 class SearchController < ApplicationController
   before_action :validate_q!, only: %i[results]
+  before_action :set_active_tab, only: %i[results]
 
   before_action :validate_geobox_presence!, only: %i[results]
   before_action :validate_geobox_range!, only: %i[results]
@@ -13,29 +14,17 @@ def results
     # inject session preference for boolean type if it is present
     params[:booleanType] = cookies[:boolean_type] || 'AND'
 
-    # Determine which tab to load - default to primo unless gdt is enabled
-    @active_tab = if Feature.enabled?(:geodata)
-                    'gdt' # Keep existing GDT behavior unchanged
-                  else
-                    params[:tab] || 'primo' # Default to primo for new tabbed interface
-                  end
-
-    # Include the active tab in the enhanced query so it's available for pagination and other uses.
-    params[:tab] = @active_tab unless Feature.enabled?(:gdt)
     @enhanced_query = Enhancer.new(params).enhanced_query
 
     # Route to appropriate search based on active tab
-    if Feature.enabled?(:geodata)
-      # Keep existing GDT behavior unchanged
+    case @active_tab
+    when 'primo'
+      load_primo_results
+    when 'timdex'
+      load_timdex_results
+    when 'geodata'
       load_gdt_results
       render 'results_geo'
-    else
-      case @active_tab
-      when 'primo'
-        load_primo_results
-      when 'timdex'
-        load_timdex_results
-      end
     end
   end
 
@@ -112,7 +101,7 @@ def query_timdex(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 @active_tab == 'gdt'
+      raw = if @active_tab == 'geodata'
               execute_geospatial_query(query)
             elsif @active_tab == 'timdex'
               TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query)

app/views/search/_form.html.erb

@@ -2,6 +2,7 @@
   <h3>What can we help you find?</h3>
   <div class="form-wrapper">
     <input id="basic-search-main" type="search" class="field field-text basic-search-input" name="q" title="Keyword anywhere" placeholder="Search for anything" value="<%= params[:q] %>" required>
+    <input id="tab-to-target" type="hidden" name="tab" value="<%= @active_tab %>">
     <button type="submit" class="btn button-primary">Search</button>
   </div>
 </form>

app/views/search/_source_tabs.html.erb

@@ -2,9 +2,9 @@
   <div id="tabs" class="tab-navigation top-space">
     <%= link_to "Primo", results_path(params.permit(:q, :per_page, :page).merge(tab: 'primo')), 
                 class: "tab-link #{'active' if @active_tab == 'primo'}", 
-                data: { turbo_frame: "search-results", turbo_action: "advance" } %>
+                data: { turbo_frame: "_top", turbo_action: "advance" } %>
     <%= link_to "TIMDEX", results_path(params.permit(:q, :per_page, :page).merge(tab: 'timdex')), 
                 class: "tab-link #{'active' if @active_tab == 'timdex'}", 
-                data: { turbo_frame: "search-results", turbo_action: "advance" } %>
+                data: { turbo_frame: "_top", turbo_action: "advance" } %>
   </div>
 

config/initializers/session_store.rb

@@ -0,0 +1,5 @@
+# Be sure to restart your server when you modify this file.
+# Note, you must set `SECRET_KEY_BASE` environment variable before running the application or sessions will not work.
+# Changing `SECRET_KEY_BASE` will invalidate all existing sessions.
+# You can generate a new secret key by running `bin/rails secret` command.
+Rails.application.config.session_store :cookie_store, key: '_use_session', secure: false, same_site: :strict

test/controllers/application_controller_test.rb

@@ -0,0 +1,73 @@
+require 'test_helper'
+
+class ApplicationControllerTest < ActionDispatch::IntegrationTest
+  test 'set_active_tab sets default to primo when no feature flag or params' do
+    assert_nil cookies[:last_tab]
+
+    get root_path
+    assert_select '#tab-to-target' do
+      assert_select '[value=?]', 'primo'
+      refute_select '[value=?]', 'geodata'
+      refute_select '[value=?]', 'timdex'
+    end
+
+    assert_equal cookies[:last_tab], 'primo'
+  end
+
+  test 'set_active_tab sets to geodata when feature flag enabled' do
+    skip 'Geodata uses a different form so we do no set the tab this way'
+    ClimateControl.modify FEATURE_GEODATA: 'true' do
+      get root_path
+      assert_select '#tab-to-target' do
+        refute_select '[value=?]', 'primo'
+        assert_select '[value=?]', 'geodata'
+        refute_select '[value=?]', 'timdex'
+      end
+    end
+  end
+
+  test 'set_active_tab sets to geodata when feature flag enabled even if param is passed' do
+    skip 'Geodata uses a different form'
+    ClimateControl.modify FEATURE_GEODATA: 'true' do
+      get root_path, params: { tab: 'primo' }
+      assert_select '#tab-to-target' do
+        refute_select '[value=?]', 'primo'
+        assert_select '[value=?]', 'geodata'
+        refute_select '[value=?]', 'timdex'
+      end
+    end
+  end
+
+  test 'set_active_tab sets to param tab when provided' do
+    get root_path, params: { tab: 'timdex' }
+
+    assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'primo'
+      refute_select '[value=?]', 'geodata'
+      assert_select '[value=?]', 'timdex'
+    end
+  end
+
+  test 'set_active_tab sets to param tab when provided even if cookie is set and updates cookie' do
+    cookies[:last_tab] = 'timdex'
+    get root_path, params: { tab: 'geodata' }
+
+    assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'primo'
+      assert_select '[value=?]', 'geodata'
+      refute_select '[value=?]', 'timdex'
+    end
+
+    assert_equal cookies[:last_tab], 'geodata'
+  end
+
+  test 'set_active_tab uses cookie last_tab when no param provided' do
+    cookies[:last_tab] = 'timdex'
+    get root_path
+    assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'primo'
+      refute_select '[value=?]', 'geodata'
+      assert_select '[value=?]', 'timdex'
+    end
+  end
+end