diff --git a/app/presenters/blacklight/index_presenter.rb b/app/presenters/blacklight/index_presenter.rb index 1f8a5ae486..bc498035ea 100644 --- a/app/presenters/blacklight/index_presenter.rb +++ b/app/presenters/blacklight/index_presenter.rb @@ -48,12 +48,20 @@ def render_document_index_label(*args) # # Allow an extention point where information in the document # may drive the value of the field - # @param [String] field + # @param [Configuration::Field, String] field_or_name # @param [Hash] options # @option options [String] :value - def field_value field, options = {} - field_config = field_config(field) - field_values(field_config, options) + def field_value field_or_name, options = {} + field = case field_or_name + when String + Deprecation.warn(self, "You provided a String value to IndexPresenter#field_value " \ + "Provide a Blacklight::Configuration::Field instead. field_value() will not accept " \ + "strings in Blacklight 7") + field_config(field_or_name) + else + field_or_name + end + field_values(field, options) end # @deprecated diff --git a/app/presenters/blacklight/show_presenter.rb b/app/presenters/blacklight/show_presenter.rb index 0640a4e719..aa48d4038b 100644 --- a/app/presenters/blacklight/show_presenter.rb +++ b/app/presenters/blacklight/show_presenter.rb @@ -74,10 +74,19 @@ def document_heading # # Allow an extention point where information in the document # may drive the value of the field - # @param [String] field + # @param [Configuration::Field, String] field_or_name # @param [Hash] options # @option options [String] :value - def field_value field, options={} + def field_value field_or_name, options={} + field = case field_or_name + when String + Deprecation.warn(self, "You provided a String value to ShowPresenter#field_value " \ + "Provide a Blacklight::Configuration::Field instead. field_value() will not accept " \ + "strings in Blacklight 7") + field_config(field_or_name) + else + field_or_name + end field_values(field_config(field), options) end diff --git a/app/views/catalog/_index_default.html.erb b/app/views/catalog/_index_default.html.erb index 44f12b5149..8e417a1308 100644 --- a/app/views/catalog/_index_default.html.erb +++ b/app/views/catalog/_index_default.html.erb @@ -1,11 +1,11 @@ -<% doc_presenter = index_presenter(document) %> +<% doc_presenter = index_presenter(document) %> <%# default partial to display solr document fields in catalog index view -%>
- + <% index_fields(document).each do |field_name, field| -%> <% if should_render_index_field? document, field %>
<%= render_index_field_label document, field: field_name %>
-
<%= doc_presenter.field_value field_name %>
+
<%= doc_presenter.field_value field %>
<% end -%> <% end -%> diff --git a/app/views/catalog/_show_default.html.erb b/app/views/catalog/_show_default.html.erb index b1f70e5344..2190933e0a 100644 --- a/app/views/catalog/_show_default.html.erb +++ b/app/views/catalog/_show_default.html.erb @@ -4,7 +4,7 @@ <% document_show_fields(document).each do |field_name, field| -%> <% if should_render_show_field? document, field %>
<%= render_document_show_field_label document, field: field_name %>
-
<%= doc_presenter.field_value field_name %>
+
<%= doc_presenter.field_value field %>
<% end -%> <% end -%>
diff --git a/spec/presenters/index_presenter_spec.rb b/spec/presenters/index_presenter_spec.rb index 17f8e117df..8ca04bdffd 100644 --- a/spec/presenters/index_presenter_spec.rb +++ b/spec/presenters/index_presenter_spec.rb @@ -16,103 +16,136 @@ SolrDocument.new(id: 1, 'link_to_search_true' => 'x', 'link_to_search_named' => 'x', - 'qwer' => 'document qwer value', - 'mnbv' => 'document mnbv value') + 'qwer' => 'document qwer value') end before do allow(request_context).to receive(:search_state).and_return(search_state) end - describe "field_value" do + describe '#field_value' do + subject { presenter.field_value field } + + let(:field) { config.index_fields[field_name] } + let(:field_name) { 'asdf' } 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 '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 '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' + context 'when a name is provided' do + subject { presenter.field_value 'qwer' } + + it 'raises a deprecation' do + expect(Deprecation).to receive(:warn) + expect(subject).to eq 'document qwer value' + end 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' + context 'when an explicit value is provided' do + subject { presenter.field_value field, value: 'asdf' } + + it { is_expected.to eq 'asdf' } 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' + context 'when field has a helper method' do + before do + allow(request_context).to receive(:render_asdf_index_field).and_return('custom asdf value') + end + + it { is_expected.to eq 'custom asdf value' } 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 + context 'when field has link_to_search with true' do + before 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') + end + + let(:field_name) { 'link_to_search_true' } + + it { is_expected.to eq 'bar' } 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(['highlight'.html_safe]) - value = subject.field_value 'highlight' - expect(value).to eq 'highlight' + context 'when field has link_to_search with a field name' do + before 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') + end + + let(:field_name) { 'link_to_search_named' } + + it { is_expected.to eq 'bar' } end - it "checks the document field value" do - value = subject.field_value 'qwer' - expect(value).to eq 'document qwer value' + context 'when no highlight field is available' do + before do + allow(document).to receive(:has_highlight_field?).and_return(false) + end + + let(:field_name) { 'highlight' } + + it { is_expected.to be_blank } end - it "works with index fields that aren't explicitly defined" do - value = subject.field_value 'mnbv' - expect(value).to eq 'document mnbv value' + context 'when highlight field is available' do + before do + allow(document).to receive(:has_highlight_field?).and_return(true) + allow(document).to receive(:highlight_field).with('highlight').and_return(['highlight'.html_safe]) + end + + let(:field_name) { 'highlight' } + + it { is_expected.to eq 'highlight' } 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" + context 'when no options are provided' do + let(:field_name) { 'qwer' } + + it "checks the document field value" do + expect(subject).to eq 'document qwer value' + end 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" + context 'when accessor is true' do + before do + allow(document).to receive_messages(solr_doc_accessor: "123") + end + + let(:field_name) { 'solr_doc_accessor' } + + it { is_expected.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" + context 'when accessor is set to a value' do + let(:field_name) { 'explicit_accessor' } + + it 'calls the accessor with the field_name as the argument' do + expect(document).to receive(:solr_doc_accessor).with('explicit_accessor').and_return("123") + + expect(subject).to eq '123' + end end - it "supports solr field configuration" do - value = subject.field_value 'alias' - expect(value).to eq "document qwer value" + context 'when the field is an alias' do + let(:field_name) { 'alias' } + + it { is_expected.to eq 'document qwer value' } end - it "supports default values in the field configuration" do - value = subject.field_value 'with_default' - expect(value).to eq "value" + context 'when the field has a default' do + let(:field_name) { 'with_default' } + + it { is_expected.to eq 'value' } end end @@ -170,4 +203,3 @@ end end end - diff --git a/spec/presenters/show_presenter_spec.rb b/spec/presenters/show_presenter_spec.rb index b96fe54dc8..b477d2b1b3 100644 --- a/spec/presenters/show_presenter_spec.rb +++ b/spec/presenters/show_presenter_spec.rb @@ -16,8 +16,7 @@ SolrDocument.new(id: 1, 'link_to_search_true' => 'x', 'link_to_search_named' => 'x', - 'qwer' => 'document qwer value', - 'mnbv' => 'document mnbv value') + 'qwer' => 'document qwer value') end before do @@ -97,118 +96,155 @@ def mock_document_app_helper_url *args end end - describe "field_value" do + describe '#field_value' do + subject { presenter.field_value field } + + let(:field) { config.show_fields[field_name] } + let(:field_name) { 'asdf' } 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 + 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] end end - it 'html-escapes values' do - value = subject.field_value 'asdf', value: 'val1' - expect(value).to eq '<b>val1</b>' - end + context 'when a name is provided' do + subject { presenter.field_value 'qwer' } - it 'joins multivalued valued fields' do - value = subject.field_value 'asdf', value: ['val1' } - it "checks 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' + it { is_expected.to eq '<b>val1</b>' } end - it "checks 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' + context 'when an explicit array value with unsafe characters is provided' do + subject { presenter.field_value field, value: [' { 'link_to_search_true' => ['x'] }).and_return('/foo') + allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar') end + + let(:field_name) { 'link_to_search_true' } + + it { is_expected.to eq 'bar' } 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(['highlight'.html_safe]) - value = subject.field_value 'highlight' - expect(value).to eq 'highlight' + context 'when field has link_to_search with a field name' do + before 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') + end + + let(:field_name) { 'link_to_search_named' } + + it { is_expected.to eq 'bar' } 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(['highlight'.html_safe, 'other highlight'.html_safe]) - value = subject.field_value 'highlight' - expect(value).to eq 'highlight and other highlight' + context 'when no highlight field is available' do + before do + allow(document).to receive(:has_highlight_field?).and_return(false) + end + + let(:field_name) { 'highlight' } + + it { is_expected.to be_blank } end - it "checks the document field value" do - value = subject.field_value 'qwer' - expect(value).to eq 'document qwer value' + context 'when highlight field is available' do + before do + allow(document).to receive(:has_highlight_field?).and_return(true) + allow(document).to receive(:highlight_field).with('highlight').and_return(['highlight'.html_safe]) + end + + let(:field_name) { 'highlight' } + + it { is_expected.to eq 'highlight' } end - it "works with show fields that aren't explicitly defined" do - value = subject.field_value 'mnbv' - expect(value).to eq 'document mnbv value' + context 'when highlight returns multiple values' do + before do + allow(document).to receive(:has_highlight_field?).and_return(true) + allow(document).to receive(:highlight_field).with('highlight').and_return(['highlight'.html_safe, 'other highlight'.html_safe]) + end + + let(:field_name) { 'highlight' } + + it { is_expected.to eq 'highlight and other highlight' } 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" + context 'when no options are provided' do + let(:field_name) { 'qwer' } + + it "checks the document field value" do + expect(subject).to eq 'document qwer value' + end 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" + context 'when accessor is true' do + before do + allow(document).to receive_messages(solr_doc_accessor: "123") + end + + let(:field_name) { 'solr_doc_accessor' } + + it { is_expected.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" + context 'when accessor is set to a value' do + let(:field_name) { 'explicit_accessor' } + + it 'calls the accessor with the field_name as the argument' do + expect(document).to receive(:solr_doc_accessor).with('explicit_accessor').and_return("123") + + expect(subject).to eq '123' + end 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" + context 'when accessor is set to an array' do + let(:field_name) { 'explicit_array_accessor' } + + it 'calls the accessors on the return of the preceeding' do + allow(document).to receive_message_chain(:solr_doc_accessor, some_method: "123") + + expect(subject).to eq '123' + end end end @@ -310,4 +346,3 @@ def mock_document_app_helper_url *args end end end -