diff --git a/app/presenters/blacklight/document_presenter.rb b/app/presenters/blacklight/document_presenter.rb index 0173cb49e6..77bc0e4d12 100644 --- a/app/presenters/blacklight/document_presenter.rb +++ b/app/presenters/blacklight/document_presenter.rb @@ -126,10 +126,10 @@ def field_values(field_config, options={}) deprecation_deprecate field_values: 'Use ShowPresenter or IndexPresenter field_values instead' # @deprecated - def render_field_value(values, field_config = nil) - ValueRenderer.new(Array.wrap(values), field_config).render + def render_field_value(values, field_config = Configuration::NullField.new) + FieldPresenter.new(@controller, @document, field_config, value: values).render end - deprecation_deprecate render_field_value: 'Use ValueRenderer instead' + deprecation_deprecate render_field_value: 'Use FieldPresenter instead' private diff --git a/app/presenters/blacklight/field_presenter.rb b/app/presenters/blacklight/field_presenter.rb index fff76180e3..08b5baa574 100644 --- a/app/presenters/blacklight/field_presenter.rb +++ b/app/presenters/blacklight/field_presenter.rb @@ -9,49 +9,23 @@ def initialize(controller, document, field_config, options) end attr_reader :controller, :document, :field_config, :options - delegate :field, to: :field_config def render - # TODO: move the itemprop stuff here - case - when field_config.helper_method - render_helper - when field_config.link_to_search - link_to_search - else - ValueRenderer.new(retrieve_values, field_config).render + if options[:value] + # This prevents helper methods from drawing. + config = Configuration::NullField.new(field_config.to_h.except(:helper_method)) + values = Array.wrap(options[:value]) + else + config = field_config + values = retrieve_values end + Rendering::Pipeline.render(values, config, document, controller, options) end private - def render_helper - controller.send(field_config.helper_method, - options.merge(document: document, - field: field, - config: field_config, - value: retrieve_values)) - end - - # This allows the link to wrap an itemprop - def link_to_search - return unless field - link_field = if field_config.link_to_search === true - field_config.key - else - field_config.link_to_search - end - - links = retrieve_values.map do |v| - 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 - end - def retrieve_values FieldRetriever.new(document, field_config).fetch end - end end diff --git a/app/presenters/blacklight/index_presenter.rb b/app/presenters/blacklight/index_presenter.rb index 8f16430438..357235dc02 100644 --- a/app/presenters/blacklight/index_presenter.rb +++ b/app/presenters/blacklight/index_presenter.rb @@ -16,18 +16,20 @@ def initialize(document, controller, configuration = controller.blacklight_confi # @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 + def label(field_or_string_or_proc, opts = {}) + config = Configuration::NullField.new + value = case field_or_string_or_proc when Symbol - @document[field] + config = field_config(field_or_string_or_proc) + @document[field_or_string_or_proc] when Proc - field.call(@document, opts) + field_or_string_or_proc.call(@document, opts) when String - field + field_or_string_or_proc end - label ||= @document.id - ValueRenderer.new(Array.wrap(label)).render + value ||= @document.id + field_values(config, value: value) end ## @@ -40,12 +42,7 @@ def label(field, 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 + field_values(field_config, options) end private @@ -70,4 +67,3 @@ def field_config(field) end end end - diff --git a/app/presenters/blacklight/rendering/abstract_step.rb b/app/presenters/blacklight/rendering/abstract_step.rb new file mode 100644 index 0000000000..43097c3037 --- /dev/null +++ b/app/presenters/blacklight/rendering/abstract_step.rb @@ -0,0 +1,24 @@ +module Blacklight + module Rendering + class AbstractStep + def initialize(values, config, document, context, options, stack) + @values = values + @config = config + @document = document + @context = context + @options = options + @stack = stack + end + + attr_reader :values, :config, :document, :context, :options, :stack + + protected + + def next_step(output_values) + first, *rest = *stack + first.new(output_values, config, document, context, options, rest).render + end + end + end +end + diff --git a/app/presenters/blacklight/rendering/helper_method.rb b/app/presenters/blacklight/rendering/helper_method.rb new file mode 100644 index 0000000000..9e91f3deb7 --- /dev/null +++ b/app/presenters/blacklight/rendering/helper_method.rb @@ -0,0 +1,23 @@ +module Blacklight + module Rendering + class HelperMethod < AbstractStep + def render + return next_step(values) unless config.helper_method + return render_helper # short circut the rest of the steps + end + + private + + def render_helper + context.send(config.helper_method, + options.merge(document: document, + field: config.field, + config: config, + value: values)) + end + end + end +end + + + diff --git a/app/presenters/blacklight/rendering/join.rb b/app/presenters/blacklight/rendering/join.rb new file mode 100644 index 0000000000..b04215ca79 --- /dev/null +++ b/app/presenters/blacklight/rendering/join.rb @@ -0,0 +1,16 @@ +module Blacklight + module Rendering + class Join < AbstractStep + def render + options = config.separator_options || {} + next_step(values.map { |x| html_escape(x) }.to_sentence(options).html_safe) + end + + private + + def html_escape(*args) + ERB::Util.html_escape(*args) + end + end + end +end diff --git a/app/presenters/blacklight/rendering/link_to_facet.rb b/app/presenters/blacklight/rendering/link_to_facet.rb new file mode 100644 index 0000000000..3fb4574df0 --- /dev/null +++ b/app/presenters/blacklight/rendering/link_to_facet.rb @@ -0,0 +1,35 @@ +module Blacklight + module Rendering + class LinkToFacet < AbstractStep + def render + # TODO: We should rename the config variable, because it creates a link to a facet. + return next_step(values) unless config.link_to_search + next_step(render_link) + end + + private + + # This allows the link to wrap an itemprop + def render_link + values.map { |v| link(link_field, v) } + end + + def link_field + return config.key if config.link_to_search === true + config.link_to_search + end + + def link(field, v) + context.link_to v, search_path(field, v) + end + + def search_path(field, v) + context.search_action_path(facet_params(field, v)) + end + + def facet_params(field, v) + context.search_state.reset.add_facet_params(field, v) + end + end + end +end diff --git a/app/presenters/blacklight/rendering/microdata.rb b/app/presenters/blacklight/rendering/microdata.rb new file mode 100644 index 0000000000..1cce3a28cc --- /dev/null +++ b/app/presenters/blacklight/rendering/microdata.rb @@ -0,0 +1,17 @@ +module Blacklight + module Rendering + class Microdata < AbstractStep + include ActionView::Helpers::TagHelper + def render + return next_step(values) unless config.itemprop + next_step(values.map { |x| itemprop(x, config.itemprop) }) + end + + private + + def itemprop(val, itemprop) + content_tag :span, val, itemprop: itemprop + end + end + end +end diff --git a/app/presenters/blacklight/rendering/pipeline.rb b/app/presenters/blacklight/rendering/pipeline.rb new file mode 100644 index 0000000000..3f99d11b6e --- /dev/null +++ b/app/presenters/blacklight/rendering/pipeline.rb @@ -0,0 +1,32 @@ +module Blacklight + module Rendering + # The field rendering pipeline + class Pipeline + def initialize(values, config, document, context, options) + @values = values + @config = config + @document = document + @context = context + @options = options + end + + attr_reader :values, :config, :document, :context, :options + + def self.render(values, config, document, context, options) + new(values, config, document, context, options).render + end + + def render + first, *rest = *stack + first.new(values, config, document, context, options, rest).render + end + + protected + + # Ordered list of operations, Terminator must be at the end. + def stack + [HelperMethod, LinkToFacet, Microdata, Join, Terminator] + end + end + end +end diff --git a/app/presenters/blacklight/rendering/terminator.rb b/app/presenters/blacklight/rendering/terminator.rb new file mode 100644 index 0000000000..9a05747585 --- /dev/null +++ b/app/presenters/blacklight/rendering/terminator.rb @@ -0,0 +1,9 @@ +module Blacklight + module Rendering + class Terminator < AbstractStep + def render + values + end + end + end +end diff --git a/app/presenters/blacklight/show_presenter.rb b/app/presenters/blacklight/show_presenter.rb index 48644f35c6..f888802448 100644 --- a/app/presenters/blacklight/show_presenter.rb +++ b/app/presenters/blacklight/show_presenter.rb @@ -49,9 +49,8 @@ def html_title 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 + f ||= @configuration.document_model.unique_key + field_values(field_config(f), value: @document[f]) end ## @@ -63,13 +62,7 @@ def heading # @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 + field_values(field_config(field), options) end private diff --git a/app/presenters/blacklight/value_renderer.rb b/app/presenters/blacklight/value_renderer.rb deleted file mode 100644 index 22a18a37b6..0000000000 --- a/app/presenters/blacklight/value_renderer.rb +++ /dev/null @@ -1,55 +0,0 @@ -module Blacklight - # Render a value (or array of values) from a field - # Renders itemprop and joins multiple values with separator_options - class ValueRenderer - include ActionView::Helpers::TagHelper - # @param [Array] values list of values to display - # @param [Blacklight::Configuration::Field] field_config field configuration - def initialize(values, field_config = nil) - @values = recode_values(values) - @field_config = field_config - end - - attr_reader :values, :field_config - - # @return [String] - # TODO: Move this out to field_presenter, so that ValueRenderer doesn't need TagHelper . - def render - if field_config and field_config.itemprop - @values = values.map { |x| content_tag :span, x, :itemprop => field_config.itemprop } - end - - render_values(@values, field_config) - end - - private - - ## - # Render a fields values as a string - # @param [Array] values to display - # @return [String] - def render_values(values, field_config = nil) - options = {} - options = field_config.separator_options if field_config && field_config.separator_options - - values.map { |x| html_escape(x) }.to_sentence(options).html_safe - end - - # @param [Array] values the values to display - # @return [Array] an array with all strings converted to UTF-8 - def recode_values(values) - 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') - else - value - end - end - end - - def html_escape(*args) - ERB::Util.html_escape(*args) - end - end -end diff --git a/lib/blacklight/configuration/null_field.rb b/lib/blacklight/configuration/null_field.rb index e32777f5eb..500498eb75 100644 --- a/lib/blacklight/configuration/null_field.rb +++ b/lib/blacklight/configuration/null_field.rb @@ -1,8 +1,13 @@ 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) + def initialize(field_or_hash = nil) + case field_or_hash + when String, Symbol + super(field: field_or_hash) + else + super + end end end end diff --git a/spec/presenters/document_presenter_spec.rb b/spec/presenters/document_presenter_spec.rb index 5d5933184e..42126d62d8 100644 --- a/spec/presenters/document_presenter_spec.rb +++ b/spec/presenters/document_presenter_spec.rb @@ -318,12 +318,18 @@ def mock_document_app_helper_url *args expect(subject.render_field_value(['a', 'b'])).to eq "a and b" end - it "should use the field_config.separator_options from the Blacklight field configuration" do - expect(subject.render_field_value(['c', 'd'], double(separator: nil, itemprop: nil, separator_options: { two_words_connector: '; '}))).to eq "c; d" + context "with separator_options" do + let(:field_config) { double(to_h: { field: 'foo', separator: nil, itemprop: nil, separator_options: { two_words_connector: '; '}}) } + it "uses the field_config.separator_options" do + expect(subject.render_field_value(['c', 'd'], field_config)).to eq "c; d" + end end - it "should include schema.org itemprop attributes" do - expect(subject.render_field_value('a', double(separator: nil, itemprop: 'some-prop', separator_options: nil))).to have_selector("span[@itemprop='some-prop']", :text => "a") + context "with itemprop attributes" do + let(:field_config) { double(to_h: { field: 'bar', separator: nil, itemprop: 'some-prop', separator_options: nil }) } + it "includes schema.org itemprop attributes" do + expect(subject.render_field_value('a', field_config)).to have_selector("span[@itemprop='some-prop']", :text => "a") + end end end @@ -332,7 +338,7 @@ def mock_document_app_helper_url *args 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" + allow(document).to receive(:[]).with('id').and_return "xyz" expect(subject.document_heading).to eq document.id end @@ -357,7 +363,7 @@ def mock_document_app_helper_url *args 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" + allow(document).to receive(:[]).with('id').and_return "xyz" expect(subject.document_show_html_title).to eq document.id end diff --git a/spec/presenters/pipeline_spec.rb b/spec/presenters/pipeline_spec.rb new file mode 100644 index 0000000000..f5e1a1e40d --- /dev/null +++ b/spec/presenters/pipeline_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Blacklight::Rendering::Pipeline do + include Capybara::RSpecMatchers + let(:document) { double } + let(:context) { double } + let(:options) { double } + let(:presenter) { described_class.new(values, field_config, document, context, options) } + describe "render" do + subject { presenter.render } + let(:values) { ['a', 'b'] } + let(:field_config) { Blacklight::Configuration::NullField.new } + it { is_expected.to eq "a and b" } + + context "when separator_options are in the config" do + let(:values) { ['c', 'd'] } + let(:field_config) { Blacklight::Configuration::NullField.new(separator: nil, itemprop: nil, separator_options: { two_words_connector: '; '}) } + it { is_expected.to eq "c; d" } + end + + context "when itemprop is in the config" do + let(:values) { ['a'] } + let(:field_config) { Blacklight::Configuration::NullField.new(separator: nil, itemprop: 'some-prop', separator_options: nil) } + it { is_expected.to have_selector("span[@itemprop='some-prop']", :text => "a") } + end + end +end diff --git a/spec/presenters/show_presenter_spec.rb b/spec/presenters/show_presenter_spec.rb index 17dbda335a..48362ab3ec 100644 --- a/spec/presenters/show_presenter_spec.rb +++ b/spec/presenters/show_presenter_spec.rb @@ -210,7 +210,7 @@ def mock_document_app_helper_url *args describe "#heading" do it "falls back to an id" do - allow(document).to receive(:id).and_return "xyz" + allow(document).to receive(:[]).with('id').and_return "xyz" expect(subject.heading).to eq document.id end @@ -232,7 +232,7 @@ def mock_document_app_helper_url *args describe "#html_title" do it "falls back to an id" do - allow(document).to receive(:id).and_return "xyz" + allow(document).to receive(:[]).with('id').and_return "xyz" expect(subject.html_title).to eq document.id end diff --git a/spec/presenters/value_renderer_spec.rb b/spec/presenters/value_renderer_spec.rb deleted file mode 100644 index 45eb83da73..0000000000 --- a/spec/presenters/value_renderer_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true -require 'spec_helper' - -describe Blacklight::ValueRenderer do - include Capybara::RSpecMatchers - let(:presenter) { Blacklight::ValueRenderer.new(values, field_config) } - - describe "render" do - subject { presenter.render } - let(:values) { ['a', 'b'] } - let(:field_config) { nil } - it { is_expected.to eq "a and b" } - - context "when separator_options are in the config" do - let(:values) { ['c', 'd'] } - let(:field_config) { double(separator: nil, itemprop: nil, separator_options: { two_words_connector: '; '}) } - it { is_expected.to eq "c; d" } - end - - context "when itemprop is in the config" do - let(:values) { ['a'] } - let(:field_config) { double(separator: nil, itemprop: 'some-prop', separator_options: nil) } - it { is_expected.to have_selector("span[@itemprop='some-prop']", :text => "a") } - end - end -end