diff --git a/app/builders/blacklight/action_builder.rb b/app/builders/blacklight/action_builder.rb new file mode 100644 index 0000000000..c9c022e10a --- /dev/null +++ b/app/builders/blacklight/action_builder.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Blacklight + # Dynamically creates methods on the given controller (typically CatalogController) + # for handling configured show tools + class ActionBuilder + def initialize(klass, name, opts) + @klass = klass + @name = name + @opts = opts + end + + attr_reader :klass, :name, :opts + + # Define a simple action handler for the tool as long as the method + # doesn't already exist or the `:define_method` option is not `false` + def build + return if skip? + callback = opts.fetch(:callback, nil).inspect + validator = opts.fetch(:validator, nil).inspect + klass.class_eval < # + def render_sms_action?(_config, _options) + sms_mappings.present? + end + def search_service search_service_class.new(blacklight_config, search_state.to_h) end diff --git a/app/controllers/concerns/blacklight/default_component_configuration.rb b/app/controllers/concerns/blacklight/default_component_configuration.rb index 39d4cbd2ed..0ee064797d 100644 --- a/app/controllers/concerns/blacklight/default_component_configuration.rb +++ b/app/controllers/concerns/blacklight/default_component_configuration.rb @@ -4,6 +4,9 @@ module DefaultComponentConfiguration extend ActiveSupport::Concern included do + Deprecation.warn(self, "Blacklight::DefaultComponentConfiguration is deprecated and will be removed in the next release." \ + "this means you must call add_results_document_tool, add_results_collection_tool, " \ + "add_show_tools_partial and add_nav_action manually in your config") add_results_document_tool(:bookmark, partial: 'bookmark_control', if: :render_bookmarks_control?) add_results_collection_tool(:sort_widget) @@ -19,10 +22,6 @@ module DefaultComponentConfiguration add_nav_action(:search_history, partial: 'blacklight/nav/search_history') end - def render_sms_action?(_config, _options) - sms_mappings.present? - end - module ClassMethods # YARD will include inline disabling as docs, cannot do multiline inside @!macro. AND this must be separate from doc block. # rubocop:disable Metrics/LineLength @@ -40,53 +39,26 @@ module ClassMethods # @option opts [Symbol] :callback method for further processing of documents, receives Array of documents def add_show_tools_partial(name, opts = {}) blacklight_config.add_show_tools_partial(name, opts) - create_action_handler(name, opts) - end - - # Define a simple action handler for the tool as long as the method - # doesn't already exist or the `:define_method` option is not `false` - def create_action_handler(name, opts) - return if method_defined?(name) || opts[:define_method] == false - - define_method name do - @response, @documents = action_documents - - if request.post? && opts[:callback] && - (opts[:validator].blank? || send(opts[:validator])) - - send(opts[:callback], @documents) - - flash[:success] ||= I18n.t("blacklight.#{name}.success", default: nil) - - respond_to do |format| - format.html do - return render "#{name}_success" if request.xhr? - redirect_to action_success_redirect_path - end - end - else - respond_to do |format| - format.html do - return render layout: false if request.xhr? - # Otherwise draw the full page - end - end - end - end + ActionBuilder.new(self, name, opts).build end # rubocop:enable Metrics/LineLength + deprecation_deprecate add_show_tools_partial: 'use blacklight_config.add_show_tools_partial instead' + # Add a tool to be displayed for each document in the search results. # @!macro partial_if_unless delegate :add_results_document_tool, to: :blacklight_config + deprecation_deprecate add_results_document_tool: 'use blacklight_config.add_results_document_tool instead' # Add a tool to be displayed for the list of search results themselves. # @!macro partial_if_unless delegate :add_results_collection_tool, to: :blacklight_config + deprecation_deprecate add_results_collection_tool: 'use blacklight_config.add_results_collection_tool instead' # Add a partial to the header navbar. # @!macro partial_if_unless delegate :add_nav_action, to: :blacklight_config + deprecation_deprecate add_nav_action: 'use blacklight_config.add_nav_action instead' end end end diff --git a/app/models/concerns/blacklight/configurable.rb b/app/models/concerns/blacklight/configurable.rb index 06d23a00d5..093d6fe2ff 100644 --- a/app/models/concerns/blacklight/configurable.rb +++ b/app/models/concerns/blacklight/configurable.rb @@ -14,7 +14,7 @@ def blacklight_config module ClassMethods def copy_blacklight_config_from(other_class) - self.blacklight_config = other_class.blacklight_config.inheritable_copy + self.blacklight_config = other_class.blacklight_config.inheritable_copy(self) end # lazy load a deep_copy of superclass if present, else @@ -24,7 +24,7 @@ def copy_blacklight_config_from(other_class) # we lazy load to 'inherit' how we want. def blacklight_config @blacklight_config ||= if superclass.respond_to?(:blacklight_config) - superclass.blacklight_config.deep_copy + superclass.blacklight_config.build(self) else default_configuration end @@ -39,7 +39,7 @@ def configure_blacklight(*args, &block) ## # The default configuration object def default_configuration - Blacklight::Configurable.default_configuration.inheritable_copy + Blacklight::Configurable.default_configuration.inheritable_copy(self) end end diff --git a/lib/blacklight/configuration.rb b/lib/blacklight/configuration.rb index 2a2695a846..8d31dc9bdd 100644 --- a/lib/blacklight/configuration.rb +++ b/lib/blacklight/configuration.rb @@ -281,7 +281,14 @@ def deep_copy end end end - alias_method :inheritable_copy, :deep_copy + + # builds a copy for the provided controller class + def build(klass) + deep_copy.tap do |conf| + conf.klass = klass + end + end + alias_method :inheritable_copy, :build # Get a view configuration for the given view type # including default values from the index configuration @@ -304,6 +311,7 @@ def view_config(view_type) def add_show_tools_partial(name, opts = {}) opts[:partial] ||= 'document_action' add_action(show.document_actions, name, opts) + klass && ActionBuilder.new(klass, name, opts).build end # rubocop:enable Metrics/LineLength diff --git a/lib/generators/blacklight/templates/catalog_controller.rb b/lib/generators/blacklight/templates/catalog_controller.rb index 18443f4be3..fce7e20889 100644 --- a/lib/generators/blacklight/templates/catalog_controller.rb +++ b/lib/generators/blacklight/templates/catalog_controller.rb @@ -30,6 +30,20 @@ class <%= controller_name.classify %>Controller < ApplicationController config.index.display_type_field = 'format' #config.index.thumbnail_field = 'thumbnail_path_ss' + config.add_results_document_tool(:bookmark, partial: 'bookmark_control', if: :render_bookmarks_control?) + + config.add_results_collection_tool(:sort_widget) + config.add_results_collection_tool(:per_page_widget) + config.add_results_collection_tool(:view_type_group) + + config.add_show_tools_partial(:bookmark, partial: 'bookmark_control', if: :render_bookmarks_control?) + config.add_show_tools_partial(:email, callback: :email_action, validator: :validate_email_params) + config.add_show_tools_partial(:sms, if: :render_sms_action?, callback: :sms_action, validator: :validate_sms_params) + config.add_show_tools_partial(:citation) + + config.add_nav_action(:bookmark, partial: 'blacklight/nav/bookmark', if: :render_bookmarks_control?) + config.add_nav_action(:search_history, partial: 'blacklight/nav/search_history') + # solr field configuration for document/show views #config.show.title_field = 'title_display' #config.show.display_type_field = 'format' diff --git a/spec/controllers/blacklight/catalog/component_configuration_spec.rb b/spec/controllers/blacklight/catalog/component_configuration_spec.rb index 7d87381218..c2fec99bdb 100644 --- a/spec/controllers/blacklight/catalog/component_configuration_spec.rb +++ b/spec/controllers/blacklight/catalog/component_configuration_spec.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true RSpec.describe Blacklight::DefaultComponentConfiguration do + before do + allow(Deprecation).to receive(:warn) + end + subject do Class.new do include Blacklight::Configurable @@ -13,18 +17,18 @@ def some_existing_action end describe ".add_show_tools_partial" do - it "should define an action method" do - subject.add_show_tools_partial :xyz + it "defines an action method" do + subject.blacklight_config.add_show_tools_partial :xyz expect(subject.new).to respond_to :xyz end - it "should not replace an existing method" do - subject.add_show_tools_partial :some_existing_action + it "does not replace an existing method" do + subject.blacklight_config.add_show_tools_partial :some_existing_action expect(subject.new.some_existing_action).to eq 1 end - it "should allow the configuration to opt out of creating a method" do - subject.add_show_tools_partial :some_missing_action, define_method: false + it "allows the configuration to opt out of creating a method" do + subject.blacklight_config.add_show_tools_partial :some_missing_action, define_method: false expect(subject.new).not_to respond_to :some_missing_action end end diff --git a/spec/controllers/catalog_controller_spec.rb b/spec/controllers/catalog_controller_spec.rb index 2d86465e68..56526ab7dd 100644 --- a/spec/controllers/catalog_controller_spec.rb +++ b/spec/controllers/catalog_controller_spec.rb @@ -639,7 +639,7 @@ def export_as_mock describe "#add_show_tools_partial" do before do - described_class.add_show_tools_partial(:like, callback: :perform_like, validator: :validate_like_params) + described_class.blacklight_config.add_show_tools_partial(:like, callback: :perform_like, validator: :validate_like_params) allow(controller).to receive(:solr_document_url).and_return('catalog/1') allow(controller).to receive(:action_documents).and_return(1) Rails.application.routes.draw do diff --git a/spec/models/blacklight/configurable_spec.rb b/spec/models/blacklight/configurable_spec.rb index f65a87a1e0..92e1efdc07 100644 --- a/spec/models/blacklight/configurable_spec.rb +++ b/spec/models/blacklight/configurable_spec.rb @@ -17,19 +17,19 @@ class Child < Parent end end end - it "inherits the configuration when subclassed" do + it "inherits the configuration when subclassed" do expect(TestCaseInheritence::Child.blacklight_config.list).to include(1,2,3) end - - it "inherited version should be a deep copy, not original" do + + it "inherited version should be a deep copy, not original" do expect(TestCaseInheritence::Child.blacklight_config).to_not be(TestCaseInheritence::Parent.blacklight_config) - + TestCaseInheritence::Child.blacklight_config.list << "child_only" - expect(TestCaseInheritence::Child.blacklight_config.list).to include("child_only") + expect(TestCaseInheritence::Child.blacklight_config.list).to include("child_only") expect(TestCaseInheritence::Parent.blacklight_config.list).to_not include("child_only") - end + end end describe "default configuration" do @@ -56,15 +56,15 @@ class Child < Parent a.send(:include, Blacklight::Configurable) expect(a.blacklight_config.a).to eq 1 end - + it "has configure_blacklight convenience method" do klass = Class.new klass.send(:include, Blacklight::Configurable) - + klass.configure_blacklight do |config| config.my_key = 'value' - end - + end + expect(klass.blacklight_config.my_key).to eq 'value' end @@ -75,11 +75,11 @@ class Child < Parent instance = klass.new instance.blacklight_config = Blacklight::Configuration.new - + expect(instance.blacklight_config).to_not eq klass.blacklight_config expect(instance.blacklight_config.foo).to be_nil end - + it "allows instance to set it's own config seperate from class" do # this is built into class_attribute; we spec it both to document it, # and to ensure we preserve this feature if we change implementation @@ -89,7 +89,7 @@ class Child < Parent klass.blacklight_config.foo = "bar" klass.blacklight_config.bar = [] klass.blacklight_config.bar << "asd" - + instance = klass.new instance.blacklight_config.bar << "123" expect(instance.blacklight_config).to_not eq klass.blacklight_config @@ -111,7 +111,6 @@ class Child < Parent expect(klass.blacklight_config.foo).to eq "bar" expect(klass2.blacklight_config.foo).to eq "asdf" end - + end end - diff --git a/spec/models/blacklight/configuration_spec.rb b/spec/models/blacklight/configuration_spec.rb index adf3b5f6d6..5f63e856bf 100644 --- a/spec/models/blacklight/configuration_spec.rb +++ b/spec/models/blacklight/configuration_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe "Blacklight::Configuration" do - + before(:each) do @config = Blacklight::Configuration.new end @@ -78,15 +78,18 @@ it "defaults to 5" do expect(Blacklight::Configuration.new.spell_max).to eq 5 end - + it "accepts config'd value" do expect(Blacklight::Configuration.new(:spell_max => 10).spell_max).to eq 10 end end describe "inheritable_copy" do + let(:klass) { Class.new } + let(:config_copy) { @config.inheritable_copy(klass) } + + it "provides a deep copy of the configuration" do - config_copy = @config.inheritable_copy config_copy.a = 1 @mock_facet = Blacklight::Configuration::FacetField.new @@ -97,12 +100,12 @@ end context "when model classes are customised" do - it "does not dup response_model or document_model" do + before do @config.response_model = Hash @config.document_model = Array + end - config_copy = @config.inheritable_copy - + it "does not dup response_model or document_model" do expect(config_copy.response_model).to eq Hash expect(config_copy.document_model).to eq Array end @@ -110,33 +113,31 @@ context "when model classes are not set" do it "leaves response_model and document_model empty" do - config_copy = @config.inheritable_copy - expect(config_copy.fetch(:response_model, nil)).to be_nil expect(config_copy.fetch(:document_model, nil)).to be_nil end it "returns default classes" do - config_copy = @config.inheritable_copy - expect(config_copy.response_model).to eq Blacklight::Solr::Response expect(config_copy.document_model).to eq SolrDocument end end - it "provides cloned copies of mutable data structures" do - @config.a = { value: 1 } - @config.b = [1,2,3] - - config_copy = @config.inheritable_copy - - config_copy.a[:value] = 2 - config_copy.b << 5 - - expect(@config.a[:value]).to eq 1 - expect(config_copy.a[:value]).to eq 2 - expect(@config.b).to match_array [1,2,3] - expect(config_copy.b).to match_array [1,2,3,5] + context "when the config has mutable datastructures" do + before do + @config.a = { value: 1 } + @config.b = [1,2,3] + end + + it "provides cloned copies of mutable data structures" do + config_copy.a[:value] = 2 + config_copy.b << 5 + + expect(@config.a[:value]).to eq 1 + expect(config_copy.a[:value]).to eq 2 + expect(@config.b).to match_array [1,2,3] + expect(config_copy.b).to match_array [1,2,3,5] + end end end @@ -148,7 +149,7 @@ config.add_my_custom_field 'qwerty', :label => "asdf" end - + expect(config.my_custom_fields.keys).to include('qwerty') end @@ -161,17 +162,17 @@ config.add_my_custom_facet_field 'qwerty', :label => "asdf" end - + expect(config.my_custom_facet_fields['qwerty']).to be_a_kind_of(Blacklight::Configuration::FacetField) end end - + describe "add_facet_field" do it "accepts field name and hash form arg" do @config.add_facet_field('format', :label => "Format", :limit => true) - + expect(@config.facet_fields["format"]).to_not be_nil expect(@config.facet_fields["format"]["label"]).to eq "Format" expect(@config.facet_fields["format"]["limit"]).to be true @@ -179,17 +180,17 @@ it "accepts FacetField obj arg" do @config.add_facet_field("format", Blacklight::Configuration::FacetField.new( :label => "Format")) - + expect(@config.facet_fields["format"]).to_not be_nil expect(@config.facet_fields["format"]["label"]).to eq "Format" end - + it "accepts field name and block form" do - @config.add_facet_field("format") do |facet| + @config.add_facet_field("format") do |facet| facet.label = "Format" facet.limit = true end - + expect(@config.facet_fields["format"]).to_not be_nil expect(@config.facet_fields["format"].limit).to be true end @@ -223,23 +224,23 @@ it "creates default label from titleized solr field" do @config.add_facet_field("publication_date") - + expect(@config.facet_fields["publication_date"].label).to eq "Publication Date" end it "allows you to not show the facet in the facet bar" do @config.add_facet_field("publication_date", :show=>false) - + expect(@config.facet_fields["publication_date"]['show']).to be false end - + it "raises on nil solr field name" do expect { @config.add_facet_field(nil) }.to raise_error ArgumentError end it "looks up and match field names" do - allow(@config).to receive_messages(luke_fields: { - "some_field_facet" => {}, + allow(@config).to receive_messages(luke_fields: { + "some_field_facet" => {}, "another_field_facet" => {}, "a_facet_field" => {}, }) @@ -275,41 +276,41 @@ end end end - + describe "add_index_field" do it "takes hash form" do @config.add_index_field("title_display", :label => "Title") - + expect(@config.index_fields["title_display"]).to_not be_nil expect(@config.index_fields["title_display"].label).to eq "Title" end it "takes IndexField param" do @config.add_index_field("title_display", Blacklight::Configuration::IndexField.new(:field => "title_display", :label => "Title")) - + expect(@config.index_fields["title_display"]).to_not be_nil expect(@config.index_fields["title_display"].label).to eq "Title" end it "takes block form" do - @config.add_index_field("title_display") do |field| + @config.add_index_field("title_display") do |field| field.label = "Title" end expect(@config.index_fields["title_display"]).to_not be_nil expect(@config.index_fields["title_display"].label).to eq "Title" end - + it "creates default label from titleized field" do @config.add_index_field("title_display") - + expect(@config.index_fields["title_display"].label).to eq "Title Display" end - + it "raises on nil solr field name" do expect { @config.add_index_field(nil) }.to raise_error ArgumentError end it "takes wild-carded field names and dereference them to solr fields" do - allow(@config).to receive_messages(luke_fields: { - "some_field_display" => {}, + allow(@config).to receive_messages(luke_fields: { + "some_field_display" => {}, "another_field_display" => {}, "a_facet_field" => {}, }) @@ -323,42 +324,42 @@ expect(@config.index_fields.keys).to include "subtitle_display", "subtitle_vern_display", "title_display", "title_vern_display" end end - + describe "add_show_field" do it "takes hash form" do @config.add_show_field("title_display", :label => "Title") - + expect(@config.show_fields["title_display"]).to_not be_nil expect(@config.show_fields["title_display"].label).to eq "Title" end it "takes ShowField argument" do @config.add_show_field("title_display", Blacklight::Configuration::ShowField.new(:field => "title_display", :label => "Title")) - + expect(@config.show_fields["title_display"]).to_not be_nil expect(@config.show_fields["title_display"].label).to eq "Title" end it "takes block form" do - @config.add_show_field("title_display") do |f| + @config.add_show_field("title_display") do |f| f.label = "Title" end - + expect(@config.show_fields["title_display"]).to_not be_nil expect(@config.show_fields["title_display"].label).to eq "Title" end - + it "creates default label humanized from field" do @config.add_show_field("my_custom_field") - + expect(@config.show_fields["my_custom_field"].label).to eq "My Custom Field" end - + it "raises on nil solr field name" do expect { @config.add_show_field(nil) }.to raise_error ArgumentError end - + it "takes wild-carded field names and dereference them to solr fields" do - allow(@config).to receive_messages(luke_fields: { - "some_field_display" => {}, + allow(@config).to receive_messages(luke_fields: { + "some_field_display" => {}, "another_field_display" => {}, "a_facet_field" => {}, }) @@ -368,8 +369,8 @@ end end - - + + describe "add_search_field" do it "accepts hash form" do c = Blacklight::Configuration.new @@ -379,55 +380,55 @@ it "accepts two-arg hash form" do c = Blacklight::Configuration.new - + c.add_search_field("my_search_type", :key => "my_search_type", - :solr_parameters => { :qf => "my_field_qf^10" }, + :solr_parameters => { :qf => "my_field_qf^10" }, :solr_local_parameters => { :pf=>"$my_field_pf"}) - + field = c.search_fields["my_search_type"] - + expect(field).to_not be_nil - - + + expect(field.solr_parameters).to_not be_nil - expect(field.solr_local_parameters).to_not be_nil - - + expect(field.solr_local_parameters).to_not be_nil + + end - + it "accepts block form" do c = Blacklight::Configuration.new - - c.add_search_field("some_field") do |field| + + c.add_search_field("some_field") do |field| field.solr_parameters = {:qf => "solr_field^10"} field.solr_local_parameters = {:pf => "$some_field_pf"} end - + f = c.search_fields["some_field"] - + expect(f).to_not be_nil expect(f.solr_parameters).to_not be_nil - expect(f.solr_local_parameters).to_not be_nil + expect(f.solr_local_parameters).to_not be_nil end - + it "accepts SearchField object" do c = Blacklight::Configuration.new - + f = Blacklight::Configuration::SearchField.new( :foo => "bar") - + c.add_search_field("foo", f) - + expect(c.search_fields["foo"]).to_not be_nil end - + it "raises on nil key" do expect {@config.add_search_field(nil, :foo => "bar")}.to raise_error ArgumentError end - + it "creates default label from titleized field key" do @config.add_search_field("author_name") - + expect(@config.search_fields["author_name"].label).to eq "Author Name" end @@ -447,7 +448,7 @@ end end end - + describe "add_sort_field" do it "takes a hash" do c = Blacklight::Configuration.new @@ -456,29 +457,29 @@ end it "takes a two-arg form with a hash" do - @config.add_sort_field("score desc, pub_date_sort desc, title_sort asc", :label => "relevance") + @config.add_sort_field("score desc, pub_date_sort desc, title_sort asc", :label => "relevance") + - expect(@config.sort_fields.values.find{|f| f.label == "relevance"}).to_not be_nil end - + it "takes a SortField object" do @config.add_sort_field(Blacklight::Configuration::SortField.new(:label => "relevance", :sort => "score desc, pub_date_sort desc, title_sort asc" -)) +)) expect(@config.sort_fields.values.find{|f| f.label == "relevance"}).to_not be_nil end - + it "takes block form" do @config.add_sort_field do |field| field.label = "relevance" field.sort = "score desc, pub_date_sort desc, title_sort asc" end - + expect(@config.sort_fields.values.find{|f| f.label == "relevance"}).to_not be_nil end end - + describe "#default_search_field" do it "uses the field with a :default key" do @config.add_search_field('search_field_1')