Refactor DocumentPresenter#fields to use generic logic applicable for different types of field displays

app/presenters/blacklight/document_presenter.rb

@@ -79,10 +79,8 @@ def display_type(base_name = nil, default: nil)
 
       fields += ['format'] if fields.empty? # backwards compatibility with the old default value for display_type_field
 
-      display_type = fields.lazy.map { |field| field_presenter(field_config(field)) }.detect(&:any?)&.values
-      display_type ||= Array(default) if default
-
-      display_type || []
+      display_type = fields.lazy.map { |field| field_presenter(display_fields[field] || Configuration::NullDisplayField.new(field)) }.detect(&:any?)&.values
+      display_type || Array(default)
     end
 
     ##
@@ -118,7 +116,7 @@ def link_rel_alternates(options = {})
     end
 
     def view_config
-      @view_config ||= show_view_config
+      show_view_config
     end
 
     def show_view_config
@@ -132,8 +130,23 @@ def inspect
 
     private
 
+    # @return [Hash<String,Configuration::Field>]
+    def fields
+      @fields ||= Array(display_type).inject(display_fields) do |fields, display_type|
+        fields.merge(display_fields(configuration.for_display_type(display_type)))
+      end
+    end
+
+    def display_fields(config = configuration)
+      config[view_config.document_fields_key || :index_fields]
+    end
+
+    def field_config(field)
+      fields.fetch(field) { Configuration::NullDisplayField.new(field) }
+    end
+
     def field_presenter(field_config, options = {})
-      field_config.presenter.new(view_context, document, field_config, options.merge(field_presenter_options))
+      field_config.presenter.new(view_context, document, field_config, field_presenter_options.merge(options))
     end
 
     def field_presenter_options

app/presenters/blacklight/index_presenter.rb

@@ -3,18 +3,7 @@
 module Blacklight
   class IndexPresenter < DocumentPresenter
     def view_config
-      @view_config ||= configuration.view_config(view_context.document_index_view_type)
-    end
-
-    private
-
-    # @return [Hash<String,Configuration::Field>] all the fields for this index view
-    def fields
-      configuration.index_fields_for(display_type)
-    end
-
-    def field_config(field)
-      configuration.index_fields.fetch(field) { Configuration::NullDisplayField.new(field) }
+      configuration.view_config(view_context.document_index_view_type)
     end
   end
 end

app/presenters/blacklight/show_presenter.rb

@@ -4,15 +4,6 @@ module Blacklight
   class ShowPresenter < DocumentPresenter
     private
 
-    # @return [Hash<String,Configuration::Field>]
-    def fields
-      configuration.show_fields_for(display_type)
-    end
-
-    def field_config(field)
-      configuration.show_fields.fetch(field) { Configuration::NullDisplayField.new(field) }
-    end
-
     def field_presenter_options
       { context: 'show' }
     end

lib/blacklight/configuration.rb

@@ -146,6 +146,8 @@ def initialized_default_configuration?
         title_field: nil,
         # solr field to use to render format-specific partials
         display_type_field: nil,
+        # the "field access" key to use to look up the document display fields
+        document_fields_key: :index_fields,
         # partials to render for each document(see #render_document_partials)
         partials: [],
         document_actions: NestedOpenStructWithHashAccess.new(ToolConfig),
@@ -173,6 +175,8 @@ def initialized_default_configuration?
         document_component: Blacklight::DocumentComponent,
         sidebar_component: Blacklight::Document::SidebarComponent,
         display_type_field: nil,
+        # the "field access" key to use to look up the document display fields
+        document_fields_key: :show_fields,
         # 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 current controller.
@@ -296,6 +300,14 @@ def initialized_default_configuration?
       # @since v8.0.0
       # @return [Boolean]
       property :filter_search_state_fields, default: true
+
+      # Additional Blacklight configuration setting for document-type specific
+      # configuration.
+      # @!attribute fields_for_type
+      # @since v8.0.0
+      # @return [Hash{Symbol => Blacklight::Configuration}]
+      # @see [#for_display_type]
+      property :fields_for_type, default: {}.with_indifferent_access
     end
     # rubocop:enable Metrics/BlockLength
 
@@ -538,37 +550,31 @@ def add_nav_action(name, opts = {})
     ##
     # Add a section of config that only applies to documents with a matching display type
     def for_display_type display_type, &_block
-      self.fields_for_type ||= {}
+      fields_for_type[display_type] ||= self.class.new
 
-      (fields_for_type[display_type] ||= self.class.new).tap do |conf|
+      fields_for_type[display_type].tap do |conf|
         yield(conf) if block_given?
       end
     end
 
     ##
     # Return a list of fields for the index display that should be used for the
     # provided document.  This respects any configuration made using for_display_type
+    # @deprecated
     def index_fields_for(display_types)
-      fields = {}.with_indifferent_access
-
-      display_types.each do |display_type|
-        fields = fields.merge(for_display_type(display_type).index_fields)
+      Array(display_types).inject(index_fields) do |fields, display_type|
+        fields.merge(for_display_type(display_type).index_fields)
       end
-
-      fields.merge(index_fields)
     end
 
     ##
     # Return a list of fields for the show page that should be used for the
     # provided document.  This respects any configuration made using for_display_type
+    # @deprecated
     def show_fields_for(display_types)
-      fields = {}.with_indifferent_access
-
-      display_types.each do |display_type|
-        fields = fields.merge(for_display_type(display_type).show_fields)
+      Array(display_types).inject(show_fields) do |fields, display_type|
+        fields.merge(for_display_type(display_type).show_fields)
       end
-
-      fields.merge(show_fields)
     end
 
     # @!visibility private

spec/presenters/blacklight/index_presenter_spec.rb

@@ -25,14 +25,12 @@
   end
 
   describe '#fields' do
-    let(:field) { instance_double(Blacklight::Configuration::Field) }
-
     before do
-      allow(config).to receive(:index_fields_for).and_return(title: field)
+      config.add_index_field 'title'
     end
 
     it 'returns the list from the configs' do
-      expect(subject.send(:fields)).to eq(title: field)
+      expect(subject.send(:fields).keys).to eq ['title']
     end
   end
 

spec/presenters/blacklight/show_presenter_spec.rb

@@ -14,10 +14,7 @@
   let(:search_state) { Blacklight::SearchState.new(params, config, controller) }
 
   let(:document) do
-    SolrDocument.new(id: 1,
-                     'link_to_facet_true' => 'x',
-                     'link_to_facet_named' => 'x',
-                     'qwer' => 'document qwer value')
+    SolrDocument.new(id: 'xyz', some_field: 'value')
   end
 
   before do
@@ -108,34 +105,27 @@ def mock_document_app_helper_url *args
   end
 
   describe '#fields' do
-    let(:field) { instance_double(Blacklight::Configuration::Field) }
-
     before do
-      allow(config).to receive(:show_fields_for).and_return(title: field)
+      config.add_show_field 'title'
     end
 
     it 'returns the list from the configs' do
-      expect(subject.send(:fields)).to eq(title: field)
+      expect(subject.send(:fields).keys).to eq ['title']
     end
   end
 
   describe "#heading" do
     it "falls back to an id" do
-      allow(document).to receive(:[]).with('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(:fetch).with('x', nil).and_return("value")
+      config.show.title_field = 'some_field'
       expect(subject.heading).to eq "value"
     end
 
     it "returns the first present value" do
-      config.show.title_field = %w[x y]
-      allow(document).to receive(:fetch).with('x', nil).and_return(nil)
-      allow(document).to receive(:fetch).with('y', nil).and_return("value")
+      config.show.title_field = %w[a_field_that_doesnt_exist some_field]
       expect(subject.heading).to eq "value"
     end
 
@@ -144,7 +134,7 @@ def mock_document_app_helper_url *args
       expect(subject.heading).to eq 'hardcoded'
     end
 
-    context "when empty document" do
+    context "with an empty document" do
       let(:document) { SolrDocument.new({}) }
 
       it "returns an empty string as the heading" do
@@ -155,21 +145,16 @@ def mock_document_app_helper_url *args
 
   describe "#html_title" do
     it "falls back to an id" do
-      allow(document).to receive(:[]).with('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")
+      config.show.html_title_field = 'some_field'
       expect(subject.html_title).to eq "value"
     end
 
     it "returns the first present value" do
-      config.show.html_title_field = %w[x y]
-      allow(document).to receive(:fetch).with('x', nil).and_return(nil)
-      allow(document).to receive(:fetch).with('y', nil).and_return("value")
+      config.show.html_title_field = %w[a_field_that_doesnt_exist some_field]
       expect(subject.html_title).to eq "value"
     end