Skip to content

Commit

Permalink
Deprecate render_index_field_value and just use the presenter
Browse files Browse the repository at this point in the history
  • Loading branch information
jcoyne committed Jul 7, 2016
1 parent 24c5241 commit c08a263
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 4 deletions.
9 changes: 7 additions & 2 deletions app/helpers/blacklight/blacklight_helper_behavior.rb
Expand Up @@ -160,18 +160,21 @@ def render_index_field_label *args
# @param [String] field
# @param [Hash] opts
# @options opts [String] :value
# TODO: deprecate and use render_field_value
# @deprecated use IndexPresenter#field_value
def render_index_field_value *args
render_field_value(*args)
end
deprecation_deprecate render_index_field_value: 'replaced by IndexPresenter#field_value'

# @deprecated use IndexPresenter#field_value
def render_field_value(*args)
options = args.extract_options!
document = args.shift || options[:document]

field = args.shift || options[:field]
presenter(document).field_value field, options.except(:document, :field)
end
deprecation_deprecate render_field_value: 'replaced by IndexPresenter#field_value'

##
# Render the show field label for a document
Expand Down Expand Up @@ -218,10 +221,12 @@ def render_document_show_field_label *args
# @param [String] field
# @param [Hash] opts
# @options opts [String] :value
# TODO: deprecate and use render_field_value
# @deprecated use ShowPresenter#field_value
def render_document_show_field_value *args
render_field_value(*args)
end
deprecation_deprecate render_document_show_field_value: 'replaced by ShowPresenter#field_value'


##
# Get the value of the document's "title" field, or a placeholder
Expand Down
3 changes: 2 additions & 1 deletion app/views/catalog/_index_default.html.erb
@@ -1,10 +1,11 @@
<% doc_presenter = index_presenter(document) %>

This comment has been minimized.

Copy link
@cbeer

cbeer Jul 7, 2016

Member

What do you think about the present(&block) pattern @flyingzumwalt suggested (not saying it's necessarily a blocker to merging now..): #1329

This comment has been minimized.

Copy link
@jcoyne

jcoyne Jul 7, 2016

Author Member

I don't have a problem with that pattern for right now. I don't think it's the ideal way to do it in the long term. I'd prefer (for 7.x) for the controller to initialize presenters rather than having any logic like this in the view.

This comment has been minimized.

Copy link
@cbeer

cbeer Jul 7, 2016

Member

👍

<%# default partial to display solr document fields in catalog index view -%>
<dl class="document-metadata dl-horizontal dl-invert">

<% index_fields(document).each do |field_name, field| -%>
<% if should_render_index_field? document, field %>
<dt class="blacklight-<%= field_name.parameterize %>"><%= render_index_field_label document, field: field_name %></dt>
<dd class="blacklight-<%= field_name.parameterize %>"><%= render_index_field_value document, field: field_name %></dd>
<dd class="blacklight-<%= field_name.parameterize %>"><%= doc_presenter.field_value field_name %></dd>
<% end -%>
<% end -%>

Expand Down
3 changes: 2 additions & 1 deletion app/views/catalog/_show_default.html.erb
@@ -1,9 +1,10 @@
<% doc_presenter = show_presenter(document) %>
<%# default partial to display solr document fields in catalog show view -%>
<dl class="dl-horizontal dl-invert">
<% document_show_fields(document).each do |field_name, field| -%>
<% if should_render_show_field? document, field %>
<dt class="blacklight-<%= field_name.parameterize %>"><%= render_document_show_field_label document, field: field_name %></dt>
<dd class="blacklight-<%= field_name.parameterize %>"><%= render_document_show_field_value document, field: field_name %></dd>
<dd class="blacklight-<%= field_name.parameterize %>"><%= doc_presenter.field_value field_name %></dd>
<% end -%>
<% end -%>
</dl>
2 changes: 2 additions & 0 deletions spec/helpers/blacklight_helper_spec.rb
Expand Up @@ -165,6 +165,7 @@ def current_search_session
describe "#render_index_field_value" do
let(:presenter) { double }
before do
allow(Deprecation).to receive(:warn)
allow(helper).to receive(:presenter).with(doc).and_return(presenter)
end

Expand All @@ -191,6 +192,7 @@ def current_search_session
let(:presenter) { double }
before do
allow(helper).to receive(:presenter).with(doc).and_return(presenter)
allow(Deprecation).to receive(:warn)
end

let(:doc) { double }
Expand Down

0 comments on commit c08a263

Please sign in to comment.