diff --git a/app/helpers/blacklight/blacklight_helper_behavior.rb b/app/helpers/blacklight/blacklight_helper_behavior.rb index d734d787fa..3fe8706210 100644 --- a/app/helpers/blacklight/blacklight_helper_behavior.rb +++ b/app/helpers/blacklight/blacklight_helper_behavior.rb @@ -160,12 +160,17 @@ def render_index_field_label *args # @param [String] field # @param [Hash] opts # @options opts [String] :value + # TODO: deprecate and use render_field_value def render_index_field_value *args + render_field_value(*args) + end + + def render_field_value(*args) options = args.extract_options! document = args.shift || options[:document] field = args.shift || options[:field] - presenter(document).render_index_field_value field, options.except(:document, :field) + presenter(document).field_value field, options.except(:document, :field) end ## @@ -213,12 +218,9 @@ def render_document_show_field_label *args # @param [String] field # @param [Hash] opts # @options opts [String] :value + # TODO: deprecate and use render_field_value def render_document_show_field_value *args - options = args.extract_options! - document = args.shift || options[:document] - - field = args.shift || options[:field] - presenter(document).render_document_show_field_value field, options.except(:document, :field) + render_field_value(*args) end ## @@ -229,7 +231,7 @@ def render_document_show_field_value *args # @return [String] def document_heading document=nil document ||= @document - presenter(document).document_heading + presenter(document).heading end ## @@ -242,7 +244,7 @@ def document_heading document=nil def document_show_html_title document=nil document ||= @document - presenter(document).document_show_html_title + presenter(document).html_title end ## @@ -260,7 +262,7 @@ def render_document_heading(*args) tag = options.fetch(:tag, :h4) document ||= @document - content_tag(tag, presenter(document).document_heading, itemprop: "name") + content_tag(tag, presenter(document).heading, itemprop: "name") end ## @@ -322,15 +324,40 @@ def render_grouped_response? response = @response ## # Returns a document presenter for the given document + # TODO: Move this to the controller. It can just pass a presenter or set of presenters. def presenter(document) - presenter_class.new(document, self) + case action_name + when 'show', 'citation' + show_presenter(document) + when 'index' + index_presenter(document) + else + raise "Unable to determine presenter type for #{action_name} on #{controller_name}" + end + end + + def show_presenter(document) + show_presenter_class(document).new(document, self) + end + + def index_presenter(document) + index_presenter_class(document).new(document, self) end - ## - # Override this method if you want to use a different presenter class def presenter_class blacklight_config.document_presenter_class end + deprecation_deprecate presenter_class: "replaced by show_presenter_class and index_presenter_class" + + ## + # Override this method if you want to use a different presenter class + def show_presenter_class(_document) + blacklight_config.show.document_presenter_class + end + + def index_presenter_class(_document) + blacklight_config.index.document_presenter_class + end ## # Open Search discovery tag for HTML links diff --git a/app/helpers/blacklight/url_helper_behavior.rb b/app/helpers/blacklight/url_helper_behavior.rb index e3346bbb1e..6229007e73 100644 --- a/app/helpers/blacklight/url_helper_behavior.rb +++ b/app/helpers/blacklight/url_helper_behavior.rb @@ -16,6 +16,7 @@ def url_for_document(doc, options = {}) # Use the catalog_path RESTful route to create a link to the show page for a specific item. # catalog_path accepts a hash. The solr query params are stored in the session, # so we only need the +counter+ param here. We also need to know if we are viewing to document as part of search results. + # TODO: move this to the IndexPresenter def link_to_document(doc, field_or_opts = nil, opts={:counter => nil}) if field_or_opts.is_a? Hash opts = field_or_opts @@ -24,7 +25,7 @@ def link_to_document(doc, field_or_opts = nil, opts={:counter => nil}) end field ||= document_show_link_field(doc) - label = presenter(doc).render_document_index_label field, opts + label = index_presenter(doc).label field, opts link_to label, url_for_document(doc), document_link_params(doc, opts) end diff --git a/app/presenters/blacklight/document_presenter.rb b/app/presenters/blacklight/document_presenter.rb index 68a3e9775c..0173cb49e6 100644 --- a/app/presenters/blacklight/document_presenter.rb +++ b/app/presenters/blacklight/document_presenter.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true module Blacklight + # @deprecated class DocumentPresenter extend Deprecation self.deprecation_horizon = 'Blacklight version 7.0.0' @@ -19,13 +20,11 @@ def initialize(document, controller, configuration = controller.blacklight_confi # # @param [SolrDocument] document # @return [String] + # @deprecated use ShowPresenter#heading instead def document_heading - fields = Array(@configuration.view_config(:show).title_field) - f = fields.find { |field| @document.has? field } - - value = f.nil? ? @document.id : @document[f] - ValueRenderer.new(Array.wrap(value)).render + show_presenter.heading end + deprecation_deprecate document_heading: "use ShowPresenter#heading instead" ## # Create links from a documents dynamically @@ -35,9 +34,11 @@ def document_heading # @option options [Boolean] :unique ensures only one link is output for every # content type, e.g. as required by atom # @option options [Array] :exclude array of format shortnames to not include in the output + # @deprecated moved to ShowPresenter#link_rel_alternates def link_rel_alternates(options = {}) - LinkAlternatePresenter.new(@controller, @document, options).render + show_presenter.link_rel_alternates(options) end + deprecation_deprecate link_rel_alternates: "use ShowPresenter#link_rel_alternates instead" ## # Get the document's "title" to display in the element. @@ -45,53 +46,36 @@ def link_rel_alternates(options = {}) # # @see #document_heading # @return [String] + # @deprecated use ShowPresenter#html_title instead def document_show_html_title - if @configuration.view_config(:show).html_title_field - fields = Array.wrap(@configuration.view_config(:show).html_title_field) - f = fields.find { |field| @document.has? field } - f ||= 'id' - field_values(show_field_config(f)) - else - document_heading - end + show_presenter.html_title end + deprecation_deprecate document_show_html_title: "use ShowPresenter#html_title instead" ## # Render the document index heading # # @param [Symbol, Proc, String] field Render the given field or evaluate the proc or render the given string # @param [Hash] opts - def render_document_index_label(field, opts = {}) - label = case field - when Symbol - @document[field] - when Proc - field.call(@document, opts) - when String - field - end - - label ||= @document.id - ValueRenderer.new(Array.wrap(label)).render + # @deprecated use IndexPresenter#label instead + def render_document_index_label(*args) + index_presenter.label(*args) end + deprecation_deprecate render_document_index_label: "use IndexPresenter#label instead" ## # Render the index field label for a document # - # Allow an extention point where information in the document - # may drive the value of the field - # @param [String] field - # @param [Hash] opts - # @options opts [String] :value - def render_index_field_value field, options = {} - field_config = index_field_config(field) - if options[:value] - # TODO: Fold this into field_values - ValueRenderer.new(Array.wrap(options[:value]), field_config).render - else - field_values(field_config, options) - end + # Allow an extention point where information in the document + # may drive the value of the field + # @param [String] field + # @param [Hash] opts + # @options opts [String] :value + # @deprecated use IndexPresenter#field_value instead + def render_index_field_value *args + index_presenter.field_value(*args) end + deprecation_deprecate render_index_field_value: "use IndexPresenter#field_value instead" ## # Render the show field value for a document @@ -101,16 +85,11 @@ def render_index_field_value field, options = {} # @param [String] field # @param [Hash] options # @options opts [String] :value - def render_document_show_field_value field, options={} - field_config = show_field_config(field) - if options[:value] - # TODO: Fold this into field_values - ValueRenderer.new(Array.wrap(options[:value]), field_config).render - else - field_values(field_config, options) - end - + # @deprecated use ShowPresenter#field_value + def render_document_show_field_value *args + show_presenter.field_value(*args) end + deprecation_deprecate render_document_show_field_value: "use ShowPresenter#field_value instead" ## # Get the value for a document's field, and prepare to render it. @@ -144,6 +123,7 @@ def get_field_values _field, field_config, options = {} def field_values(field_config, options={}) FieldPresenter.new(@controller, @document, field_config, options).render end + deprecation_deprecate field_values: 'Use ShowPresenter or IndexPresenter field_values instead' # @deprecated def render_field_value(values, field_config = nil) @@ -153,23 +133,12 @@ def render_field_value(values, field_config = nil) private - def show_field_config(field) - field_config(@configuration.show_fields, field) - end - - def index_field_config(field) - field_config(@configuration.index_fields, field) - end - - def field_config(conf, field) - conf.fetch(field) { NilFieldConfig.new(field) } + def index_presenter + @controller.index_presenter(@document) end - # Returned if no config is defined for the field in the Blacklight::Configuration - class NilFieldConfig < Blacklight::Configuration::Field - def initialize(field) - super(field: field) - end + def show_presenter + @controller.show_presenter(@document) end end end diff --git a/app/presenters/blacklight/field_presenter.rb b/app/presenters/blacklight/field_presenter.rb index 7bffd480e2..fff76180e3 100644 --- a/app/presenters/blacklight/field_presenter.rb +++ b/app/presenters/blacklight/field_presenter.rb @@ -9,10 +9,7 @@ def initialize(controller, document, field_config, options) end attr_reader :controller, :document, :field_config, :options - - def field - field_config.field - end + delegate :field, to: :field_config def render # TODO: move the itemprop stuff here @@ -49,7 +46,7 @@ def link_to_search controller.link_to ValueRenderer.new([v], field_config).render, controller.search_action_path(controller.search_state.reset.add_facet_params(link_field, v)) end - links.to_sentence().html_safe + links.to_sentence.html_safe end def retrieve_values diff --git a/app/presenters/blacklight/index_presenter.rb b/app/presenters/blacklight/index_presenter.rb new file mode 100644 index 0000000000..8f16430438 --- /dev/null +++ b/app/presenters/blacklight/index_presenter.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true +module Blacklight + class IndexPresenter + # @param [SolrDocument] document + # @param [ActionController::Base] controller scope for linking and generating urls + # @param [Blacklight::Configuration] configuration + def initialize(document, controller, configuration = controller.blacklight_config) + @document = document + @configuration = configuration + @controller = controller + end + + ## + # Render the document index heading + # + # @param [Symbol, Proc, String] field Render the given field or evaluate the proc or render the given string + # @param [Hash] opts + # TODO: the default field should be `document_show_link_field(doc)' + def label(field, opts = {}) + label = case field + when Symbol + @document[field] + when Proc + field.call(@document, opts) + when String + field + end + + label ||= @document.id + ValueRenderer.new(Array.wrap(label)).render + end + + ## + # Render the index field label for a document + # + # Allow an extention point where information in the document + # may drive the value of the field + # @param [String] field + # @param [Hash] opts + # @options opts [String] :value + def field_value field, options = {} + field_config = field_config(field) + if options[:value] + # TODO: Fold this into field_values + ValueRenderer.new(Array.wrap(options[:value]), field_config).render + else + field_values(field_config, options) + end + end + + private + + ## + # Get the value for a document's field, and prepare to render it. + # - highlight_field + # - accessor + # - solr field + # + # Rendering: + # - helper_method + # - link_to_search + # @param [Blacklight::Configuration::Field] solr field configuration + # @param [Hash] options additional options to pass to the rendering helpers + def field_values(field_config, options={}) + FieldPresenter.new(@controller, @document, field_config, options).render + end + + def field_config(field) + @configuration.index_fields.fetch(field) { Configuration::NullField.new(field) } + end + end +end + diff --git a/app/presenters/blacklight/show_presenter.rb b/app/presenters/blacklight/show_presenter.rb new file mode 100644 index 0000000000..48644f35c6 --- /dev/null +++ b/app/presenters/blacklight/show_presenter.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true +module Blacklight + class ShowPresenter + # @param [SolrDocument] document + # @param [ActionController::Base] controller scope for linking and generating urls + # @param [Blacklight::Configuration] configuration + def initialize(document, controller, configuration = controller.blacklight_config) + @document = document + @configuration = configuration + @controller = controller + end + + ## + # Create <link rel="alternate"> links from a documents dynamically + # provided export formats. Returns empty string if no links available. + # + # @params [Hash] options + # @option options [Boolean] :unique ensures only one link is output for every + # content type, e.g. as required by atom + # @option options [Array<String>] :exclude array of format shortnames to not include in the output + # @deprecated moved to ShowPresenter#link_rel_alternates + def link_rel_alternates(options = {}) + LinkAlternatePresenter.new(@controller, @document, options).render + end + + ## + # Get the document's "title" to display in the <title> element. + # (by default, use the #document_heading) + # + # @see #document_heading + # @return [String] + def html_title + if view_config.html_title_field + fields = Array.wrap(view_config.html_title_field) + f = fields.detect { |field| @document.has? field } + f ||= 'id' + field_values(field_config(f)) + else + heading + end + end + + ## + # Get the value of the document's "title" field, or a placeholder + # value (if empty) + # + # @param [SolrDocument] document + # @return [String] + def heading + fields = Array.wrap(view_config.title_field) + f = fields.detect { |field| @document.has? field } + + value = f.nil? ? @document.id : @document[f] + ValueRenderer.new(Array.wrap(value)).render + end + + ## + # Render the show field value for a document + # + # Allow an extention point where information in the document + # may drive the value of the field + # @param [String] field + # @param [Hash] options + # @options opts [String] :value + def field_value field, options={} + field_config = field_config(field) + if options[:value] + # TODO: Fold this into field_values + ValueRenderer.new(Array.wrap(options[:value]), field_config).render + else + field_values(field_config, options) + end + end + + private + + ## + # Get the value for a document's field, and prepare to render it. + # - highlight_field + # - accessor + # - solr field + # + # Rendering: + # - helper_method + # - link_to_search + # @param [Blacklight::Configuration::Field] solr field configuration + # @param [Hash] options additional options to pass to the rendering helpers + def field_values(field_config, options={}) + FieldPresenter.new(@controller, @document, field_config, options).render + end + + def view_config + @configuration.view_config(:show) + end + + def field_config(field) + @configuration.show_fields.fetch(field) { Configuration::NullField.new(field) } + end + end +end diff --git a/app/presenters/blacklight/value_renderer.rb b/app/presenters/blacklight/value_renderer.rb index 6a8b00fb93..22a18a37b6 100644 --- a/app/presenters/blacklight/value_renderer.rb +++ b/app/presenters/blacklight/value_renderer.rb @@ -38,7 +38,7 @@ def render_values(values, field_config = nil) # @param [Array] values the values to display # @return [Array] an array with all strings converted to UTF-8 def recode_values(values) - values.collect do |value| + values.map do |value| if value.respond_to?(:encoding) && value.encoding != Encoding::UTF_8 Rails.logger.warn "Found a non utf-8 value in Blacklight::DocumentPresenter. \"#{value}\" Encoding is #{value.encoding}" value.dup.force_encoding('UTF-8') diff --git a/app/services/blacklight/field_retriever.rb b/app/services/blacklight/field_retriever.rb index f7c33b5751..c0ad10cfa2 100644 --- a/app/services/blacklight/field_retriever.rb +++ b/app/services/blacklight/field_retriever.rb @@ -8,10 +8,7 @@ def initialize(document, field_config) end attr_reader :document, :field_config - - def field - field_config.field - end + delegate :field, to: :field_config # @return [Array] def fetch diff --git a/app/views/catalog/_document_default.atom.builder b/app/views/catalog/_document_default.atom.builder index dae31c8d30..e724736666 100644 --- a/app/views/catalog/_document_default.atom.builder +++ b/app/views/catalog/_document_default.atom.builder @@ -1,6 +1,5 @@ xml.entry do - - xml.title presenter(document).render_document_index_label(document_show_link_field(document)) + xml.title index_presenter(document).label(document_show_link_field(document)) # updated is required, for now we'll just set it to now, sorry xml.updated Time.current.iso8601 @@ -8,7 +7,7 @@ xml.entry do xml.link "rel" => "alternate", "type" => "text/html", "href" => polymorphic_url(url_for_document(document)) # add other doc-specific formats, atom only lets us have one per # content type, so the first one in the list wins. - xml << render_link_rel_alternates(document, :unique => true) + xml << show_presenter(document).link_rel_alternates(unique: true) xml.id polymorphic_url(url_for_document(document)) diff --git a/app/views/catalog/_document_default.rss.builder b/app/views/catalog/_document_default.rss.builder index b6a99df923..e040a95b69 100644 --- a/app/views/catalog/_document_default.rss.builder +++ b/app/views/catalog/_document_default.rss.builder @@ -1,5 +1,5 @@ xml.item do - xml.title(presenter(document).render_document_index_label(document_show_link_field(document)) || (document.to_semantic_values[:title].first if document.to_semantic_values.key?(:title))) + xml.title(index_presenter(document).label(document_show_link_field(document)) || (document.to_semantic_values[:title].first if document.to_semantic_values.key?(:title))) xml.link(polymorphic_url(url_for_document(document))) xml.author( document.to_semantic_values[:author].first ) if document.to_semantic_values.key? :author -end \ No newline at end of file +end diff --git a/lib/blacklight/configuration.rb b/lib/blacklight/configuration.rb index 98763889f3..031c017165 100644 --- a/lib/blacklight/configuration.rb +++ b/lib/blacklight/configuration.rb @@ -16,6 +16,7 @@ class Configuration < OpenStructWithHashAccess # XXX this isn't very pretty, but it works. require_dependency 'blacklight/configuration/fields' require_dependency 'blacklight/configuration/field' + require_dependency 'blacklight/configuration/null_field' require_dependency 'blacklight/configuration/search_field' require_dependency 'blacklight/configuration/facet_field' require_dependency 'blacklight/configuration/sort_field' @@ -69,7 +70,7 @@ def default_values # the model to use for each response document document_model: nil, # document presenter class used by helpers and views - document_presenter_class: nil, + document_presenter_class: nil, # deprecated # Class for paginating long lists of facet fields facet_paginator_class: nil, # repository connection configuration @@ -80,6 +81,8 @@ def default_values navbar: OpenStructWithHashAccess.new(partials: { }), # General configuration for all views index: ViewConfig::Index.new( + # document presenter class used by helpers and views + document_presenter_class: nil, # solr field to use to render a document title title_field: nil, # solr field to use to render format-specific partials @@ -95,6 +98,8 @@ def default_values ), # Additional configuration when displaying a single document show: ViewConfig::Show.new( + # document presenter class used by helpers and views + document_presenter_class: nil, # default route parameters for 'show' requests # set this to a hash with additional arguments to merge into # the route, or set `controller: :current` to route to the @@ -186,13 +191,20 @@ def document_model= *args super end + def response_model + super || Blacklight::Solr::Response + end + + # @deprecated def document_presenter_class super || Blacklight::DocumentPresenter end - def response_model - super || Blacklight::Solr::Response + # @deprecated + def document_presenter_class=(klass) + super end + deprecation_deprecate :document_presenter_class= => "replaced by show.presenter_class and index.presenter_class" def response_model= *args super diff --git a/lib/blacklight/configuration/null_field.rb b/lib/blacklight/configuration/null_field.rb new file mode 100644 index 0000000000..e32777f5eb --- /dev/null +++ b/lib/blacklight/configuration/null_field.rb @@ -0,0 +1,8 @@ +module Blacklight + # Returned if no config is defined for the field in the Blacklight::Configuration + class Configuration::NullField < Blacklight::Configuration::Field + def initialize(field) + super(field: field) + end + end +end diff --git a/lib/blacklight/configuration/view_config.rb b/lib/blacklight/configuration/view_config.rb index aff29931d9..21e6204469 100644 --- a/lib/blacklight/configuration/view_config.rb +++ b/lib/blacklight/configuration/view_config.rb @@ -2,9 +2,15 @@ class Blacklight::Configuration class ViewConfig < Blacklight::OpenStructWithHashAccess class Show < ViewConfig + def document_presenter_class + super || Blacklight::ShowPresenter + end end class Index < ViewConfig + def document_presenter_class + super || Blacklight::IndexPresenter + end end end end diff --git a/spec/helpers/blacklight_helper_spec.rb b/spec/helpers/blacklight_helper_spec.rb index a8c9a479f9..f08e4bc2d7 100644 --- a/spec/helpers/blacklight_helper_spec.rb +++ b/spec/helpers/blacklight_helper_spec.rb @@ -172,17 +172,17 @@ def current_search_session let(:field) { "some_field" } it "should pass the document and field through to the presenter" do - expect(presenter).to receive(:render_index_field_value).with(field, {}) + expect(presenter).to receive(:field_value).with(field, {}) helper.render_index_field_value(doc, field) end it "should allow the document and field to be passed as hash arguments" do - expect(presenter).to receive(:render_index_field_value).with(field, {}) + expect(presenter).to receive(:field_value).with(field, {}) helper.render_index_field_value(document: doc, field: field) end it "should allow additional options to be passed to the presenter" do - expect(presenter).to receive(:render_index_field_value).with(field, x: 1) + expect(presenter).to receive(:field_value).with(field, x: 1) helper.render_index_field_value(document: doc, field: field, x: 1) end end @@ -197,17 +197,17 @@ def current_search_session let(:field) { "some_field" } it "should pass the document and field through to the presenter" do - expect(presenter).to receive(:render_document_show_field_value).with(field, {}) + expect(presenter).to receive(:field_value).with(field, {}) helper.render_document_show_field_value(doc, field) end it "should allow the document and field to be passed as hash arguments" do - expect(presenter).to receive(:render_document_show_field_value).with(field, {}) + expect(presenter).to receive(:field_value).with(field, {}) helper.render_document_show_field_value(document: doc, field: field) end it "should allow additional options to be passed to the presenter" do - expect(presenter).to receive(:render_document_show_field_value).with(field, x: 1) + expect(presenter).to receive(:field_value).with(field, x: 1) helper.render_document_show_field_value(document: doc, field: field, x: 1) end end @@ -446,6 +446,23 @@ def current_search_session allow(helper).to receive(:blacklight_config).and_return(blacklight_config) end + it "uses the value defined in the blacklight configuration" do + expect(Deprecation).to receive(:warn).exactly(4).times + blacklight_config.document_presenter_class = presenter_class + expect(helper.presenter_class).to eq presenter_class + end + + it "defaults to Blacklight::DocumentPresenter" do + expect(Deprecation).to receive(:warn) + expect(helper.presenter_class).to eq Blacklight::DocumentPresenter + end + end + + describe "#index_presenter_class" do + before do + allow(helper).to receive(:blacklight_config).and_return(blacklight_config) + end + let :blacklight_config do Blacklight::Configuration.new end @@ -454,19 +471,42 @@ def current_search_session double end - it "should use the value defined in the blacklight configuration" do - blacklight_config.document_presenter_class = presenter_class - expect(helper.presenter_class).to eq presenter_class + it "uses the value defined in the blacklight configuration" do + blacklight_config.index.document_presenter_class = presenter_class + expect(helper.index_presenter_class(nil)).to eq presenter_class end - it "should default to Blacklight::DocumentPresenter" do - expect(helper.presenter_class).to eq Blacklight::DocumentPresenter + it "defaults to Blacklight::IndexPresenter" do + expect(helper.index_presenter_class(nil)).to eq Blacklight::IndexPresenter + end + end + + describe "#show_presenter_class" do + before do + allow(helper).to receive(:blacklight_config).and_return(blacklight_config) + end + + let :blacklight_config do + Blacklight::Configuration.new + end + + let :presenter_class do + double + end + + it "uses the value defined in the blacklight configuration" do + blacklight_config.show.document_presenter_class = presenter_class + expect(helper.show_presenter_class(nil)).to eq presenter_class + end + + it "defaults to Blacklight::DocumentPresenter" do + expect(helper.show_presenter_class(nil)).to eq Blacklight::ShowPresenter end end describe "#render_document_heading" do before do - allow(helper).to receive(:presenter).and_return(double(document_heading: "Heading")) + allow(helper).to receive(:presenter).and_return(double(heading: "Heading")) end let(:document) { double } @@ -480,12 +520,12 @@ def current_search_session end it "should accept an explicit document argument" do - allow(helper).to receive(:presenter).with(document).and_return(double(document_heading: "Document Heading")) + allow(helper).to receive(:presenter).with(document).and_return(double(heading: "Document Heading")) expect(helper.render_document_heading(document)).to have_selector "h4", text: "Document Heading" end it "should accept the document with a tag option" do - allow(helper).to receive(:presenter).with(document).and_return(double(document_heading: "Document Heading")) + allow(helper).to receive(:presenter).with(document).and_return(double(heading: "Document Heading")) expect(helper.render_document_heading(document, tag: "h3")).to have_selector "h3", text: "Document Heading" end end diff --git a/spec/helpers/url_helper_spec.rb b/spec/helpers/url_helper_spec.rb index f28eaa7736..f7121e4474 100644 --- a/spec/helpers/url_helper_spec.rb +++ b/spec/helpers/url_helper_spec.rb @@ -199,6 +199,9 @@ let(:id) { '123456' } let(:data) { { 'id' => id, 'title_display' => [title_display] } } let(:document) { SolrDocument.new(data) } + before do + allow(controller).to receive(:action_name).and_return('index') + end it "consists of the document title wrapped in a <a>" do expect(helper.link_to_document(document, :title_display)).to have_selector("a", :text => '654321', :count => 1) diff --git a/spec/presenters/document_presenter_spec.rb b/spec/presenters/document_presenter_spec.rb index 613393b6d4..5d5933184e 100644 --- a/spec/presenters/document_presenter_spec.rb +++ b/spec/presenters/document_presenter_spec.rb @@ -3,11 +3,13 @@ describe Blacklight::DocumentPresenter do include Capybara::RSpecMatchers + let(:show_presenter) { Blacklight::ShowPresenter.new(document, request_context, config) } + let(:index_presenter) { Blacklight::IndexPresenter.new(document, request_context, config) } let(:request_context) { double } let(:config) { Blacklight::Configuration.new } subject { presenter } - let(:presenter) { Blacklight::DocumentPresenter.new(document, request_context, config) } + let(:presenter) { described_class.new(document, request_context, config) } let(:parameter_class) { ActionController::Parameters } let(:params) { parameter_class.new } let(:search_state) { Blacklight::SearchState.new(params, config) } @@ -22,6 +24,7 @@ before do allow(request_context).to receive(:search_state).and_return(search_state) + allow(Deprecation).to receive(:warn) end describe "link_rel_alternates" do @@ -50,6 +53,7 @@ def mock_document_app_helper_url *args allow(request_context).to receive(:polymorphic_url) do |_, opts| "url.#{opts[:format]}" end + allow(request_context).to receive(:show_presenter).and_return(show_presenter) end let(:document) { MockDocument.new(id: "MOCK_ID1") } @@ -112,6 +116,9 @@ def mock_document_app_helper_url *args config.add_index_field 'with_default', default: 'value' end end + before do + allow(request_context).to receive(:index_presenter).and_return(index_presenter) + end it "checks for an explicit value" do value = subject.render_index_field_value 'asdf', :value => 'asdf' expect(value).to eq 'asdf' @@ -204,6 +211,10 @@ def mock_document_app_helper_url *args end end + before do + allow(request_context).to receive(:show_presenter).and_return(show_presenter) + end + it 'html-escapes values' do value = subject.render_document_show_field_value 'asdf', value: '<b>val1</b>' expect(value).to eq '<b>val1</b>' @@ -317,6 +328,9 @@ def mock_document_app_helper_url *args end describe "#document_heading" do + before do + allow(request_context).to receive(:show_presenter).and_return(show_presenter) + end it "should fallback to an id" do allow(document).to receive(:id).and_return "xyz" expect(subject.document_heading).to eq document.id @@ -339,6 +353,9 @@ def mock_document_app_helper_url *args end describe "#document_show_html_title" do + before do + allow(request_context).to receive(:show_presenter).and_return(show_presenter) + end it "falls back to an id" do allow(document).to receive(:id).and_return "xyz" expect(subject.document_show_html_title).to eq document.id diff --git a/spec/presenters/index_presenter_spec.rb b/spec/presenters/index_presenter_spec.rb new file mode 100644 index 0000000000..7c8d31da45 --- /dev/null +++ b/spec/presenters/index_presenter_spec.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Blacklight::IndexPresenter do + include Capybara::RSpecMatchers + let(:request_context) { double } + let(:config) { Blacklight::Configuration.new } + + subject { presenter } + let(:presenter) { described_class.new(document, request_context, config) } + let(:parameter_class) { ActionController::Parameters } + let(:params) { parameter_class.new } + let(:search_state) { Blacklight::SearchState.new(params, config) } + + let(:document) do + SolrDocument.new(id: 1, + 'link_to_search_true' => 'x', + 'link_to_search_named' => 'x', + 'qwer' => 'document qwer value', + 'mnbv' => 'document mnbv value') + end + + before do + allow(request_context).to receive(:search_state).and_return(search_state) + end + + describe "field_value" do + let(:config) do + Blacklight::Configuration.new.configure do |config| + config.add_index_field 'qwer' + config.add_index_field 'asdf', :helper_method => :render_asdf_index_field + config.add_index_field 'link_to_search_true', :link_to_search => true + config.add_index_field 'link_to_search_named', :link_to_search => :some_field + config.add_index_field 'highlight', :highlight => true + config.add_index_field 'solr_doc_accessor', :accessor => true + config.add_index_field 'explicit_accessor', :accessor => :solr_doc_accessor + config.add_index_field 'explicit_accessor_with_arg', :accessor => :solr_doc_accessor_with_arg + config.add_index_field 'alias', field: 'qwer' + config.add_index_field 'with_default', default: 'value' + end + end + it "checks for an explicit value" do + value = subject.field_value 'asdf', :value => 'asdf' + expect(value).to eq 'asdf' + end + + it "checks for a helper method to call" do + allow(request_context).to receive(:render_asdf_index_field).and_return('custom asdf value') + value = subject.field_value 'asdf' + expect(value).to eq 'custom asdf value' + end + + it "checks for a link_to_search" do + allow(request_context).to receive(:search_action_path).with('f' => { 'link_to_search_true' => ['x'] }).and_return('/foo') + allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar') + value = subject.field_value 'link_to_search_true' + expect(value).to eq 'bar' + end + + it "checks for a link_to_search with a field name" do + allow(request_context).to receive(:search_action_path).with('f' => { 'some_field' => ['x'] }).and_return('/foo') + allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar') + value = subject.field_value 'link_to_search_named' + expect(value).to eq 'bar' + end + + it "gracefully handles when no highlight field is available" do + allow(document).to receive(:has_highlight_field?).and_return(false) + value = subject.field_value 'highlight' + expect(value).to be_blank + end + + it "checks for a highlighted field" do + allow(document).to receive(:has_highlight_field?).and_return(true) + allow(document).to receive(:highlight_field).with('highlight').and_return(['<em>highlight</em>'.html_safe]) + value = subject.field_value 'highlight' + expect(value).to eq '<em>highlight</em>' + end + + it "checks the document field value" do + value = subject.field_value 'qwer' + expect(value).to eq 'document qwer value' + end + + it "should work with index fields that aren't explicitly defined" do + value = subject.field_value 'mnbv' + expect(value).to eq 'document mnbv value' + end + + it "should call an accessor on the solr document" do + allow(document).to receive_messages(solr_doc_accessor: "123") + value = subject.field_value 'solr_doc_accessor' + expect(value).to eq "123" + end + + it "should call an explicit accessor on the solr document" do + allow(document).to receive_messages(solr_doc_accessor: "123") + value = subject.field_value 'explicit_accessor' + expect(value).to eq "123" + end + + it "should call an accessor on the solr document with the field as an argument" do + allow(document).to receive(:solr_doc_accessor_with_arg).with('explicit_accessor_with_arg').and_return("123") + value = subject.field_value 'explicit_accessor_with_arg' + expect(value).to eq "123" + end + + it "should support solr field configuration" do + value = subject.field_value 'alias' + expect(value).to eq "document qwer value" + end + + it "should support default values in the field configuration" do + value = subject.field_value 'with_default' + expect(value).to eq "value" + end + end + + describe '#field_values' do + context 'for a field with the helper_method option' do + let(:field_name) { 'field_with_helper' } + let(:field_config) { config.add_facet_field 'field_with_helper', helper_method: 'render_field_with_helper' } + + before do + document['field_with_helper'] = 'value' + end + + it "should check call the helper method with arguments" do + allow(request_context).to receive(:render_field_with_helper) do |*args| + args.first + end + + render_options = { a: 1 } + + options = subject.send(:field_values, field_config, a: 1) + + expect(options).to include :document, :field, :value, :config, :a + expect(options[:document]).to eq document + expect(options[:field]).to eq 'field_with_helper' + expect(options[:value]).to eq ['value'] + expect(options[:config]).to eq field_config + expect(options[:a]).to eq 1 + end + end + end +end + diff --git a/spec/presenters/show_presenter_spec.rb b/spec/presenters/show_presenter_spec.rb new file mode 100644 index 0000000000..17dbda335a --- /dev/null +++ b/spec/presenters/show_presenter_spec.rb @@ -0,0 +1,283 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Blacklight::ShowPresenter do + include Capybara::RSpecMatchers + let(:request_context) { double } + let(:config) { Blacklight::Configuration.new } + + subject { presenter } + let(:presenter) { described_class.new(document, request_context, config) } + let(:parameter_class) { ActionController::Parameters } + let(:params) { parameter_class.new } + let(:search_state) { Blacklight::SearchState.new(params, config) } + + let(:document) do + SolrDocument.new(id: 1, + 'link_to_search_true' => 'x', + 'link_to_search_named' => 'x', + 'qwer' => 'document qwer value', + 'mnbv' => 'document mnbv value') + end + + before do + allow(request_context).to receive(:search_state).and_return(search_state) + end + + describe "link_rel_alternates" do + before do + class MockDocument + include Blacklight::Solr::Document + end + + module MockExtension + def self.extended(document) + document.will_export_as(:weird, "application/weird") + document.will_export_as(:weirder, "application/weirder") + document.will_export_as(:weird_dup, "application/weird") + end + def export_as_weird ; "weird" ; end + def export_as_weirder ; "weirder" ; end + def export_as_weird_dup ; "weird_dup" ; end + end + + MockDocument.use_extension(MockExtension) + + def mock_document_app_helper_url *args + solr_document_url(*args) + end + + allow(request_context).to receive(:polymorphic_url) do |_, opts| + "url.#{opts[:format]}" + end + end + + let(:document) { MockDocument.new(id: "MOCK_ID1") } + + context "with no arguments" do + subject { presenter.link_rel_alternates } + + it "generates <link rel=alternate> tags" do + tmp_value = Capybara.ignore_hidden_elements + Capybara.ignore_hidden_elements = false + document.export_formats.each_pair do |format, spec| + expect(subject).to have_selector("link[href$='.#{ format }']") do |matches| + expect(matches).to have(1).match + tag = matches[0] + expect(tag.attributes["rel"].value).to eq "alternate" + expect(tag.attributes["title"].value).to eq format.to_s + expect(tag.attributes["href"].value).to eq mock_document_app_helper_url(document, format: format) + end + end + Capybara.ignore_hidden_elements = tmp_value + end + + it { is_expected.to be_html_safe } + end + + context "with unique: true" do + subject { presenter.link_rel_alternates(unique: true) } + + it "respects unique: true" do + tmp_value = Capybara.ignore_hidden_elements + Capybara.ignore_hidden_elements = false + expect(subject).to have_selector("link[type='application/weird']", count: 1) + Capybara.ignore_hidden_elements = tmp_value + end + end + + context "with exclude" do + subject { presenter.link_rel_alternates(unique: true) } + it "excludes formats from :exclude" do + tmp_value = Capybara.ignore_hidden_elements + Capybara.ignore_hidden_elements = false + expect(subject).to_not have_selector("link[href$='.weird_dup']") + Capybara.ignore_hidden_elements = tmp_value + end + end + end + + describe "field_value" do + let(:config) do + Blacklight::Configuration.new.configure do |config| + config.add_show_field 'qwer' + config.add_show_field 'asdf', :helper_method => :render_asdf_document_show_field + config.add_show_field 'link_to_search_true', :link_to_search => true + config.add_show_field 'link_to_search_named', :link_to_search => :some_field + config.add_show_field 'highlight', :highlight => true + config.add_show_field 'solr_doc_accessor', :accessor => true + config.add_show_field 'explicit_accessor', :accessor => :solr_doc_accessor + config.add_show_field 'explicit_array_accessor', :accessor => [:solr_doc_accessor, :some_method] + config.add_show_field 'explicit_accessor_with_arg', :accessor => :solr_doc_accessor_with_arg + end + end + + it 'html-escapes values' do + value = subject.field_value 'asdf', value: '<b>val1</b>' + expect(value).to eq '<b>val1</b>' + end + + it 'joins multivalued valued fields' do + value = subject.field_value 'asdf', value: ['<a', 'b'] + expect(value).to eq '<a and b' + end + + it 'joins multivalued valued fields' do + value = subject.field_value 'asdf', value: ['a', 'b', 'c'] + expect(value).to eq 'a, b, and c' + end + + it "should check for an explicit value" do + expect(request_context).to_not receive(:render_asdf_document_show_field) + value = subject.field_value 'asdf', :value => 'val1' + expect(value).to eq 'val1' + end + + it "should check for a helper method to call" do + allow(request_context).to receive(:render_asdf_document_show_field).and_return('custom asdf value') + value = subject.field_value 'asdf' + expect(value).to eq 'custom asdf value' + end + + it "should check for a link_to_search" do + allow(request_context).to receive(:search_action_path).and_return('/foo') + allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar') + value = subject.field_value 'link_to_search_true' + expect(value).to eq 'bar' + end + + it "should check for a link_to_search with a field name" do + allow(request_context).to receive(:search_action_path).and_return('/foo') + allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar') + value = subject.field_value 'link_to_search_named' + expect(value).to eq 'bar' + end + + it "should gracefully handle when no highlight field is available" do + allow(document).to receive(:has_highlight_field?).and_return(false) + value = subject.field_value 'highlight' + expect(value).to be_blank + end + + it "should check for a highlighted field" do + allow(document).to receive(:has_highlight_field?).and_return(true) + allow(document).to receive(:highlight_field).with('highlight').and_return(['<em>highlight</em>'.html_safe]) + value = subject.field_value 'highlight' + expect(value).to eq '<em>highlight</em>' + end + + it 'respects the HTML-safeness of multivalued highlight fields' do + allow(document).to receive(:has_highlight_field?).and_return(true) + allow(document).to receive(:highlight_field).with('highlight').and_return(['<em>highlight</em>'.html_safe, '<em>other highlight</em>'.html_safe]) + value = subject.field_value 'highlight' + expect(value).to eq '<em>highlight</em> and <em>other highlight</em>' + end + + it "checks the document field value" do + value = subject.field_value 'qwer' + expect(value).to eq 'document qwer value' + end + + it "works with show fields that aren't explicitly defined" do + value = subject.field_value 'mnbv' + expect(value).to eq 'document mnbv value' + end + + it "calls an accessor on the solr document" do + allow(document).to receive_messages(solr_doc_accessor: "123") + value = subject.field_value 'solr_doc_accessor' + expect(value).to eq "123" + end + + it "calls an explicit accessor on the solr document" do + allow(document).to receive_messages(solr_doc_accessor: "123") + value = subject.field_value 'explicit_accessor' + expect(value).to eq "123" + end + + it "calls an explicit array-style accessor on the solr document" do + allow(document).to receive_message_chain(:solr_doc_accessor, some_method: "123") + value = subject.field_value 'explicit_array_accessor' + expect(value).to eq "123" + end + + it "calls an accessor on the solr document with the field as an argument" do + allow(document).to receive(:solr_doc_accessor_with_arg).with('explicit_accessor_with_arg').and_return("123") + value = subject.field_value 'explicit_accessor_with_arg' + expect(value).to eq "123" + end + end + + describe "#heading" do + it "falls back to an id" do + allow(document).to receive(:id).and_return "xyz" + expect(subject.heading).to eq document.id + end + + it "returns the value of the field" do + config.show.title_field = :x + allow(document).to receive(:has?).with(:x).and_return(true) + allow(document).to receive(:[]).with(:x).and_return("value") + expect(subject.heading).to eq "value" + end + + it "returns the first present value" do + config.show.title_field = [:x, :y] + allow(document).to receive(:has?).with(:x).and_return(false) + allow(document).to receive(:has?).with(:y).and_return(true) + allow(document).to receive(:[]).with(:y).and_return("value") + expect(subject.heading).to eq "value" + end + end + + describe "#html_title" do + it "falls back to an id" do + allow(document).to receive(:id).and_return "xyz" + expect(subject.html_title).to eq document.id + end + + it "returns the value of the field" do + config.show.html_title_field = :x + allow(document).to receive(:has?).with(:x).and_return(true) + allow(document).to receive(:fetch).with(:x, nil).and_return("value") + expect(subject.html_title).to eq "value" + end + + it "returns the first present value" do + config.show.html_title_field = [:x, :y] + allow(document).to receive(:has?).with(:x).and_return(false) + allow(document).to receive(:has?).with(:y).and_return(true) + allow(document).to receive(:fetch).with(:y, nil).and_return("value") + expect(subject.html_title).to eq "value" + end + end + + describe '#field_values' do + context 'for a field with the helper_method option' do + let(:field_name) { 'field_with_helper' } + let(:field_config) { config.add_facet_field 'field_with_helper', helper_method: 'render_field_with_helper' } + + before do + document['field_with_helper'] = 'value' + end + + it "should check call the helper method with arguments" do + allow(request_context).to receive(:render_field_with_helper) do |*args| + args.first + end + + render_options = { a: 1 } + + options = subject.send(:field_values, field_config, a: 1) + + expect(options).to include :document, :field, :value, :config, :a + expect(options[:document]).to eq document + expect(options[:field]).to eq 'field_with_helper' + expect(options[:value]).to eq ['value'] + expect(options[:config]).to eq field_config + expect(options[:a]).to eq 1 + end + end + end +end + diff --git a/spec/views/catalog/_index_default.erb_spec.rb b/spec/views/catalog/_index_default.erb_spec.rb index b3f9589e44..db365b7241 100644 --- a/spec/views/catalog/_index_default.erb_spec.rb +++ b/spec/views/catalog/_index_default.erb_spec.rb @@ -9,6 +9,7 @@ include CatalogHelper before(:each) do + allow(view).to receive(:action_name).and_return('index') @config = Blacklight::Configuration.new do |config| config.show.display_type_field = 'asdf' config.add_index_field 'one_field', :label => 'One:' diff --git a/spec/views/catalog/_index_header_default.html.erb_spec.rb b/spec/views/catalog/_index_header_default.html.erb_spec.rb index 552bd7f5d3..4e4176f563 100644 --- a/spec/views/catalog/_index_header_default.html.erb_spec.rb +++ b/spec/views/catalog/_index_header_default.html.erb_spec.rb @@ -11,6 +11,7 @@ end before do + allow(controller).to receive(:action_name).and_return('index') assign :response, double(start: 0) allow(view).to receive(:render_grouped_response?).and_return false allow(view).to receive(:blacklight_config).and_return(blacklight_config) diff --git a/spec/views/catalog/_show_default.erb_spec.rb b/spec/views/catalog/_show_default.erb_spec.rb index 7e772949f9..cc9826b7bf 100644 --- a/spec/views/catalog/_show_default.erb_spec.rb +++ b/spec/views/catalog/_show_default.erb_spec.rb @@ -11,7 +11,8 @@ before(:each) do - @config = Blacklight::Configuration.new do |config| + allow(controller).to receive(:action_name).and_return('show') + @config = Blacklight::Configuration.new do |config| config.show.display_type_field = 'asdf' config.add_show_field 'one_field', :label => 'One:' config.add_show_field 'empty_field', :label => 'Three:' diff --git a/spec/views/catalog/_show_sidebar.erb_spec.rb b/spec/views/catalog/_show_sidebar.erb_spec.rb index dd1f32591b..1aea1f2d03 100644 --- a/spec/views/catalog/_show_sidebar.erb_spec.rb +++ b/spec/views/catalog/_show_sidebar.erb_spec.rb @@ -12,6 +12,7 @@ end before(:each) do + allow(controller).to receive(:action_name).and_return('show') allow(view).to receive(:blacklight_config).and_return(blacklight_config) allow(view).to receive(:has_user_authentication_provider?).and_return(false) allow(view).to receive(:current_search_session).and_return nil diff --git a/spec/views/catalog/_thumbnail_default.erb_spec.rb b/spec/views/catalog/_thumbnail_default.erb_spec.rb index f839bc208e..43845b2203 100644 --- a/spec/views/catalog/_thumbnail_default.erb_spec.rb +++ b/spec/views/catalog/_thumbnail_default.erb_spec.rb @@ -18,6 +18,7 @@ end before do + allow(controller).to receive(:action_name).and_return('index') assign :response, double(start: 0) allow(view).to receive(:render_grouped_response?).and_return false allow(view).to receive(:blacklight_config).and_return(blacklight_config) diff --git a/spec/views/catalog/index.atom.builder_spec.rb b/spec/views/catalog/index.atom.builder_spec.rb index 8c532e7513..680038bfd8 100644 --- a/spec/views/catalog/index.atom.builder_spec.rb +++ b/spec/views/catalog/index.atom.builder_spec.rb @@ -14,6 +14,7 @@ end before do + allow(view).to receive(:action_name).and_return('index') params['content_format'] = 'some_format' @document_list = document_list allow_any_instance_of(SolrDocument).to receive(:export_as_some_format).and_return("") diff --git a/spec/views/catalog/show.html.erb_spec.rb b/spec/views/catalog/show.html.erb_spec.rb index 0f09be76f9..1a4f1587d3 100644 --- a/spec/views/catalog/show.html.erb_spec.rb +++ b/spec/views/catalog/show.html.erb_spec.rb @@ -12,6 +12,7 @@ end before :each do + allow(view).to receive(:action_name).and_return('show') allow(view).to receive_messages(:has_user_authentication_provider? => false) allow(view).to receive_messages(:render_document_sidebar_partial => "Sidebar") allow(view).to receive_messages(current_search_session: nil)