From c63ae793313ce56c96599654b19109d09acb6e63 Mon Sep 17 00:00:00 2001 From: Benjamin Armintor Date: Tue, 3 Jan 2023 14:34:03 -0500 Subject: [PATCH] implement a configuration object for Blacklight::Configuration.track_search_session - allow configuration of components for applied parameters and item pagination - only build server-side search tracking data if using server-side tracking - extract applied_params html into a component --- .../server_applied_params_component.html.erb | 4 ++ .../server_applied_params_component.rb | 13 ++++++ .../concerns/blacklight/search_context.rb | 30 ++++++++----- app/helpers/blacklight/url_helper_behavior.rb | 19 +++++--- app/views/catalog/_show_main_content.html.erb | 4 +- app/views/catalog/index.html.erb | 3 ++ app/views/catalog/show.html.erb | 7 +-- lib/blacklight/configuration.rb | 18 ++++++++ .../configuration/session_tracking_config.rb | 45 +++++++++++++++++++ .../blacklight/document_component_spec.rb | 2 +- .../server_applied_params_component_spec.rb | 29 ++++++++++++ spec/controllers/bookmarks_controller_spec.rb | 1 + .../session_tracking_config_spec.rb | 37 +++++++++++++++ 13 files changed, 185 insertions(+), 27 deletions(-) create mode 100644 app/components/blacklight/search_context/server_applied_params_component.html.erb create mode 100644 app/components/blacklight/search_context/server_applied_params_component.rb create mode 100644 lib/blacklight/configuration/session_tracking_config.rb create mode 100644 spec/components/blacklight/search_context/server_applied_params_component_spec.rb create mode 100644 spec/lib/blacklight/configuration/session_tracking_config_spec.rb diff --git a/app/components/blacklight/search_context/server_applied_params_component.html.erb b/app/components/blacklight/search_context/server_applied_params_component.html.erb new file mode 100644 index 0000000000..7254cce572 --- /dev/null +++ b/app/components/blacklight/search_context/server_applied_params_component.html.erb @@ -0,0 +1,4 @@ +
+ <%= render 'start_over' %> + <%= link_back_to_catalog class: 'btn btn-outline-secondary' %> +
diff --git a/app/components/blacklight/search_context/server_applied_params_component.rb b/app/components/blacklight/search_context/server_applied_params_component.rb new file mode 100644 index 0000000000..d7783c714d --- /dev/null +++ b/app/components/blacklight/search_context/server_applied_params_component.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Blacklight + module SearchContext + class ServerAppliedParamsComponent < Blacklight::Component + delegate :current_search_session, :link_back_to_catalog, to: :helpers + + def render? + current_search_session + end + end + end +end diff --git a/app/controllers/concerns/blacklight/search_context.rb b/app/controllers/concerns/blacklight/search_context.rb index fa3b0f5880..ccee37d6bf 100644 --- a/app/controllers/concerns/blacklight/search_context.rb +++ b/app/controllers/concerns/blacklight/search_context.rb @@ -101,6 +101,8 @@ def agent_is_crawler? end def find_or_initialize_search_session_from_params params + return unless blacklight_config.track_search_session_config.storage == 'server' + params_copy = params.reject { |k, v| blacklisted_search_session_params.include?(k.to_sym) || v.blank? } return if params_copy.reject { |k, _v| [:action, :controller].include? k.to_sym }.blank? @@ -131,25 +133,31 @@ def blacklisted_search_session_params # calls setup_previous_document then setup_next_document. # used in the show action for single view pagination. def setup_next_and_previous_documents - if search_session['counter'] && current_search_session - index = search_session['counter'].to_i - 1 - response, documents = search_service.previous_and_next_documents_for_search index, search_state.reset(current_search_session.query_params) + return { counter: params[:counter] } if setup_next_and_previous_on_client? + return nil unless setup_next_and_previous_on_server? - search_session['total'] = response.total - { prev: documents.first, next: documents.last } - end + index = search_session['counter'].to_i - 1 + response, documents = search_service.previous_and_next_documents_for_search index, search_state.reset(current_search_session.query_params) + + search_session['total'] = response.total + { prev: documents.first, next: documents.last } rescue Blacklight::Exceptions::InvalidRequest => e logger&.warn "Unable to setup next and previous documents: #{e}" nil end + def setup_next_and_previous_on_server? + search_session['counter'] && current_search_session && blacklight_config.track_search_session_config.storage == 'server' + end + + def setup_next_and_previous_on_client? + params[:counter] && blacklight_config.track_search_session_config.storage == 'client' + end + def page_links_document_path(document, counter) return nil unless document + return search_state.url_for_document(document, counter: counter) if blacklight_config.view_config(:show).route - if blacklight_config.view_config(:show).route - search_state.url_for_document(document, counter: counter) - else - solr_document_path(document, counter: counter) - end + solr_document_path(document, counter: counter) end end diff --git a/app/helpers/blacklight/url_helper_behavior.rb b/app/helpers/blacklight/url_helper_behavior.rb index 16b5ddb0b0..bbcc9832ba 100644 --- a/app/helpers/blacklight/url_helper_behavior.rb +++ b/app/helpers/blacklight/url_helper_behavior.rb @@ -80,21 +80,24 @@ def link_to_next_document(next_document, classes: 'next', **addl_link_opts) # @example # session_tracking_params(SolrDocument.new(id: 123), 7) # => { data: { context_href: '/catalog/123/track?counter=7&search_id=999' } } - def session_tracking_params document, counter - path = session_tracking_path(document, per_page: params.fetch(:per_page, search_session['per_page']), counter: counter, search_id: current_search_session.try(:id), document_id: document&.id) - - if path.nil? - return {} + def session_tracking_params document, counter, per_page: search_session['per_page'], search_id: current_search_session&.id + path_params = { per_page: params.fetch(:per_page, per_page), counter: counter, search_id: search_id } + if blacklight_config.track_search_session_config.storage == 'server' + path_params[:document_id] = document&.id + path_params[:search_id] = search_id end + path = session_tracking_path(document, path_params) + return {} if path.nil? - { data: { context_href: path, turbo_prefetch: false } } + context_method = blacklight_config.track_search_session_config.storage == 'client' ? 'get' : 'post' + { data: { context_href: path, context_method: context_method, turbo_prefetch: false } } end private :session_tracking_params ## # Get the URL for tracking search sessions across pages using polymorphic routing def session_tracking_path document, params = {} - return if document.nil? || !blacklight_config&.track_search_session + return if document.nil? || !blacklight_config.track_search_session_config.storage if main_app.respond_to?(controller_tracking_method) return main_app.public_send(controller_tracking_method, params.merge(id: document)) @@ -105,6 +108,8 @@ def session_tracking_path document, params = {} end def controller_tracking_method + return blacklight_config.track_search_session_config.url_helper if blacklight_config.track_search_session_config.url_helper + "track_#{controller_name}_path" end diff --git a/app/views/catalog/_show_main_content.html.erb b/app/views/catalog/_show_main_content.html.erb index cf9360c208..b50c6bfacd 100644 --- a/app/views/catalog/_show_main_content.html.erb +++ b/app/views/catalog/_show_main_content.html.erb @@ -1,6 +1,6 @@ -<%= render(Blacklight::SearchContextComponent.new(search_context: @search_context, search_session: search_session, current_document: @document)) %> +<%= render blacklight_config.track_search_session_config.item_pagination_component.new(search_context: @search_context, search_session: search_session, current_document: @document) if blacklight_config.track_search_session_config.item_pagination_component %> -<% @page_title = t('blacklight.search.show.title', document_title: Deprecation.silence(Blacklight::BlacklightHelperBehavior) { document_show_html_title }, application_name: application_name).html_safe %> +<% @page_title = t('blacklight.search.show.title', document_title: document_presenter(@document).html_title, application_name: application_name).html_safe %> <% content_for(:head) { render_link_rel_alternates } %> <%= render (blacklight_config.view_config(:show).document_component || Blacklight::DocumentComponent).new(presenter: document_presenter(@document), component: :div, title_component: :h1, show: true) do |component| %> diff --git a/app/views/catalog/index.html.erb b/app/views/catalog/index.html.erb index c0af2dbe12..5dd4f3bf61 100644 --- a/app/views/catalog/index.html.erb +++ b/app/views/catalog/index.html.erb @@ -1,3 +1,6 @@ +<% content_for(:head) do %> + +<% end %> <% content_for(:sidebar) do %> <%= render 'search_sidebar' %> <% end %> diff --git a/app/views/catalog/show.html.erb b/app/views/catalog/show.html.erb index 39533c8543..c09f46956b 100644 --- a/app/views/catalog/show.html.erb +++ b/app/views/catalog/show.html.erb @@ -1,9 +1,4 @@ -<% if current_search_session %> -
- <%= render 'start_over' %> - <%= link_back_to_catalog class: 'btn btn-outline-secondary' %> -
-<% end %> +<%= render blacklight_config.track_search_session_config.applied_params_component.new if blacklight_config.track_search_session_config.applied_params_component %> <%= render 'show_main_content' %> diff --git a/lib/blacklight/configuration.rb b/lib/blacklight/configuration.rb index 970913764d..65f9d9ac23 100644 --- a/lib/blacklight/configuration.rb +++ b/lib/blacklight/configuration.rb @@ -306,6 +306,11 @@ def default_per_page # @return [Boolean] property :track_search_session, default: true + # @!attribute track_search_session_config + # @since v7.35.0 + # @return [Blacklight::Configuration::SessionTrackingConfig] + property :track_search_session_config, default: nil + # @!attribute advanced_search # @since v7.15.0 # @return [#enabled] @@ -618,6 +623,19 @@ def show_fields_for(document_or_display_types) fields.merge(show_fields) end + def track_search_session=(val) + self.track_search_session_config = Blacklight::Configuration::SessionTrackingConfig.new(storage: val ? 'storage' : false) + super + end + + def track_search_session_config + stored_config = super + + return stored_config if stored_config + + self.track_search_session_config = Blacklight::Configuration::SessionTrackingConfig.new(storage: track_search_session ? 'storage' : false) + end + # @!visibility private def freeze each { |_k, v| v.is_a?(OpenStruct) && v.freeze } diff --git a/lib/blacklight/configuration/session_tracking_config.rb b/lib/blacklight/configuration/session_tracking_config.rb new file mode 100644 index 0000000000..b7d87f7ee1 --- /dev/null +++ b/lib/blacklight/configuration/session_tracking_config.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +class Blacklight::Configuration + class SessionTrackingConfig < Blacklight::OpenStructWithHashAccess + # @!attribute storage + # @return [String, FalseClass] 'server': use server-side tracking; 'client': delegate search tracking and prev/next navigation to client + # @!attribute applied_params_component + # @return [Class] component class used to render a facet group + # @!attribute item_pagination_component + # @return [Class] component class used to render the constraints + + def initialize(property_hash = {}) + super({ storage: 'server' }.merge(property_hash)) + end + + def applied_params_component + super || default_applied_params_component(storage) + end + + def item_pagination_component + super || default_item_pagination_component(storage) + end + + def url_helper + super || default_url_helper(storage) + end + + def default_applied_params_component(storage) + return Blacklight::SearchContext::ServerAppliedParamsComponent if storage == 'server' + + nil + end + + def default_item_pagination_component(storage) + return Blacklight::SearchContextComponent if storage == 'server' + + nil + end + + # extension point for alternative storage types + def default_url_helper(_storage) + nil + end + end +end diff --git a/spec/components/blacklight/document_component_spec.rb b/spec/components/blacklight/document_component_spec.rb index 74dada0616..12713882d0 100644 --- a/spec/components/blacklight/document_component_spec.rb +++ b/spec/components/blacklight/document_component_spec.rb @@ -26,7 +26,7 @@ let(:blacklight_config) do CatalogController.blacklight_config.deep_copy.tap do |config| - config.track_search_session = false + config.track_search_session_config.storage = false config.index.thumbnail_field = 'thumbnail_path_ss' config.index.document_actions[:bookmark].partial = '/catalog/bookmark_control' end diff --git a/spec/components/blacklight/search_context/server_applied_params_component_spec.rb b/spec/components/blacklight/search_context/server_applied_params_component_spec.rb new file mode 100644 index 0000000000..ff21468e7f --- /dev/null +++ b/spec/components/blacklight/search_context/server_applied_params_component_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Blacklight::SearchContext::ServerAppliedParamsComponent, type: :component do + subject(:render) { instance.render_in(view_context) } + + let(:instance) { described_class.new } + let(:current_search_session) { nil } + let(:view_context) { controller.view_context } + + before do + view_context.view_paths.unshift(RSpec::Rails::ViewExampleGroup::StubResolverCache.resolver_for('application/_start_over.html.erb' => 'start over')) + allow(view_context).to receive(:current_search_session).and_return current_search_session + allow(view_context).to receive(:link_back_to_catalog).with(any_args) + end + + it 'is blank without current session' do + expect(render).to be_blank + end + + context 'with current session' do + let(:current_search_session) { double(query_params: { q: 'abc' }) } + + it 'is not blank' do + expect(render).not_to be_blank + end + end +end diff --git a/spec/controllers/bookmarks_controller_spec.rb b/spec/controllers/bookmarks_controller_spec.rb index 05df6104ca..7e4bf8fdd4 100644 --- a/spec/controllers/bookmarks_controller_spec.rb +++ b/spec/controllers/bookmarks_controller_spec.rb @@ -8,6 +8,7 @@ it 'opts out of search session tracking' do expect(@controller.blacklight_config.track_search_session).to eq false + expect(@controller.blacklight_config.track_search_session_config.storage).to be false end end diff --git a/spec/lib/blacklight/configuration/session_tracking_config_spec.rb b/spec/lib/blacklight/configuration/session_tracking_config_spec.rb new file mode 100644 index 0000000000..ad0f648e9d --- /dev/null +++ b/spec/lib/blacklight/configuration/session_tracking_config_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +RSpec.describe Blacklight::Configuration::SessionTrackingConfig do + subject(:config) { described_class.new } + + it "defaults @storage to 'server'" do + expect(config.storage).to eq 'server' + end + + context "@storage is set to 'server'" do + before do + config.storage = 'server' + end + + it "defaults components values" do + expect(config.applied_params_component).to eq Blacklight::SearchContext::ServerAppliedParamsComponent + end + + it "defaults tracking url helper" do + expect(config.url_helper).to be_nil + end + end + + context "@storage is set to false" do + before do + config.storage = false + end + + it "defaults components values" do + expect(config.applied_params_component).to be_nil + end + + it "defaults tracking url helper" do + expect(config.url_helper).to be_nil + end + end +end