Skip to content

Commit

Permalink
Move helper methods to ShowPresenter
Browse files Browse the repository at this point in the history
  • Loading branch information
jcoyne committed Sep 1, 2017
1 parent cf3d1d1 commit c496583
Show file tree
Hide file tree
Showing 12 changed files with 47 additions and 24 deletions.
4 changes: 2 additions & 2 deletions app/controllers/concerns/blacklight/catalog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ def show

respond_to do |format|
format.html do
@presenter = show_presenter_class(@document).new(@document, view_context)
@search_context = setup_next_and_previous_documents
search_context = setup_next_and_previous_documents
@presenter = show_presenter_class(@document).new(@document, view_context, blacklight_config, search_context)
end
format.json { render json: { response: { document: @document } } }
additional_export_formats(@document, format)
Expand Down
3 changes: 3 additions & 0 deletions app/helpers/blacklight/blacklight_helper_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def render_page_title
def render_link_rel_alternates(presenter = @presenter, options = {})
presenter.link_rel_alternates(options)
end
deprecation_deprecate render_link_rel_alternates: 'use ShowPresenter#link_rel_alternates instead'

##
# Render OpenSearch headers for this search
Expand Down Expand Up @@ -112,6 +113,7 @@ def document_heading presenter = nil
def document_show_html_title
@presenter.html_title
end
deprecation_deprecate document_show_html_title: 'use ShowPresenter#html_title instead'

##
# Render the document "heading" (title) in a content tag
Expand All @@ -130,6 +132,7 @@ def render_document_heading(*args)

content_tag(tag, presenter.heading, itemprop: "name")
end
deprecation_deprecate render_document_heading: 'use ShowPresenter#render_document_heading'

##
# Get the current "view type" (and ensure it is a valid type)
Expand Down
1 change: 1 addition & 0 deletions app/helpers/blacklight/catalog_helper_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ def document_class_prefix
def render_document_main_content_partial(_document = @document)
render partial: 'show_main_content'
end
deprecation_deprecate render_document_main_content_partial: 'use ShowPresenter#render_content_partial instead'

##
# Should we display the sort and per page widget?
Expand Down
23 changes: 21 additions & 2 deletions app/presenters/blacklight/show_presenter.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
# frozen_string_literal: true
module Blacklight
class ShowPresenter
attr_reader :document, :configuration, :view_context
attr_reader :document, :configuration, :view_context, :search_context

# @param [SolrDocument] document
# @param [ActionView::Base] view_context scope for linking and generating urls
# @param [Blacklight::Configuration] configuration
def initialize(document, view_context, configuration = view_context.blacklight_config)
# @param [Hash] search_context contains the next and previous documents in the current search
def initialize(document, view_context, configuration = view_context.blacklight_config, search_context = {})
@document = document
@view_context = view_context
@configuration = configuration
@search_context = search_context
end

##
Expand All @@ -24,6 +26,23 @@ def link_rel_alternates(options = {})
LinkAlternatePresenter.new(view_context, document, options).render
end

##
# Render the main content partial for a document
#
# @return [String]
def render_content_partial
view_context.render 'show_main_content', presenter: self
end

##
# Render the document "heading" (title) in a content tag
# @param [Hash] options
# @option options [Symbol] :tag
def render_document_heading(options = {})
tag = options.fetch(:tag, :h4)
view_context.content_tag(tag, heading, itemprop: "name")
end

##
# Get the document's "title" to display in the <title> element.
# (by default, use the #document_heading)
Expand Down
6 changes: 3 additions & 3 deletions app/views/catalog/_previous_next_doc.html.erb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<% # Using the Bootstrap Pagination class -%>
<div class='pagination-search-widgets'>
<% if @search_context[:prev] || @search_context[:next] %>
<% if search_context[:prev] || search_context[:next] %>
<div class="page-links">
<%= link_to_previous_document @search_context[:prev] %> |
<%= link_to_previous_document search_context[:prev] %> |
<%= item_page_entry_info %> |
<%= link_to_next_document @search_context[:next] %>
<%= link_to_next_document search_context[:next] %>
</div>
<% end %>
</div>
2 changes: 1 addition & 1 deletion app/views/catalog/_show.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%# default partial to display solr document fields in catalog show view %>
<dl class="row dl-invert document-metadata">
<% @presenter.fields do |field_presenter| %>
<% presenter.fields do |field_presenter| %>
<dt class="blacklight-<%= field_presenter.to_param %> col-md-3"><%= field_presenter.label %></dt>
<dd class="blacklight-<%= field_presenter.to_param %> col-md-9"><%= field_presenter.value %></dd>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/catalog/_show_header.html.erb
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<% # bookmark/folder functions -%>
<%= render_document_heading(@presenter, tag: :h1) %>
<%= presenter.render_document_heading(tag: :h1) %>
8 changes: 3 additions & 5 deletions app/views/catalog/_show_main_content.html.erb
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
<%= render 'previous_next_doc' if @search_context %>
<%= render('previous_next_doc', search_context: presenter.search_context) if presenter.search_context %>
<% @page_title = t('blacklight.search.show.title',
document_title: document_show_html_title,
document_title: presenter.html_title,
application_name: application_name
).html_safe %>
<% content_for(:head) { render_link_rel_alternates } %>

<div id="document" class="document <%= render_document_class %>" itemscope itemtype="<%= @document.itemtype %>">
<div id="doc_<%= @document.id.to_s.parameterize %>">
<%= render_document_partials @document, blacklight_config.view_config(:show).partials %>
<%= render_document_partials @document, blacklight_config.view_config(:show).partials, presenter: presenter %>
</div>
</div>

Expand Down
4 changes: 3 additions & 1 deletion app/views/catalog/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
<% content_for(:head) { @presenter.link_rel_alternates } %>
<% if current_search_session %>
<div id="appliedParams" class="clearfix constraints-container">
<%= link_to t('blacklight.search.start_over'), start_over_path, class: "catalog_startOverLink btn btn-primary" %>
<%= link_back_to_catalog class: 'btn btn-secondary' %>
</div>
<% end %>
<%= render_document_main_content_partial %>
<%= @presenter.render_content_partial %>
<% content_for(:sidebar) do %>
<%= render 'show_sidebar' %>
Expand Down
10 changes: 5 additions & 5 deletions spec/controllers/catalog_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -276,32 +276,32 @@
it "sets previous document if counter present in session" do
session[:search] = search_session.merge('counter' => 2)
get :show, params: { id: doc_id }
expect(assigns[:search_context][:prev]).to_not be_nil
expect(assigns[:presenter].search_context[:prev]).to_not be_nil
end

it "does not set previous or next document if session is blank" do
get :show, params: { id: doc_id }
expect(assigns[:search_context]).to be_nil
expect(assigns[:presenter].search_context).to be_nil
end

it "does not set previous or next document if session[:search]['counter'] is nil" do
session[:search] = {}
get :show, params: { id: doc_id }
expect(assigns[:search_context]).to be_nil
expect(assigns[:presenter].search_context).to be_nil
end

it "sets next document if counter present in session" do
session[:search] = search_session.merge('counter' => 2)
get :show, params: { id: doc_id }
expect(assigns[:search_context][:next]).to_not be_nil
expect(assigns[:presenter].search_context[:next]).to_not be_nil
end

it "does not break if solr returns an exception" do
allow(controller.send(:search_service)).to receive(:previous_and_next_documents_for_search) {
raise Blacklight::Exceptions::InvalidRequest.new "Error"
}
get :show, params: { id: doc_id }
expect(assigns[:search_context]).to be_nil
expect(assigns[:presenter].search_context).to be_nil
end
end

Expand Down
5 changes: 3 additions & 2 deletions spec/views/catalog/_show.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@
end
end

let(:presenter) { Blacklight::ShowPresenter.new(document, view, config) }

before do
allow(controller).to receive(:action_name).and_return('show')
allow(view).to receive(:blacklight_config).and_return(config)
view.instance_variable_set :@presenter, Blacklight::ShowPresenter.new(document, view, config)
end

# this is in RenderPartialsHelperBehavior
subject { view.render_document_partial document, :show }
subject { view.render_document_partial document, :show, presenter: presenter }

it "only displays fields listed in the initializer" do
expect(subject).to_not include("val_2")
Expand Down
3 changes: 1 addition & 2 deletions spec/views/catalog/show.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,14 @@
end

it "sets the @page_title" do
allow(view).to receive(:document_show_html_title).and_return("Heading")
allow(presenter).to receive(:html_title).and_return("Heading")
render
page_title = view.instance_variable_get(:@page_title)
expect(page_title).to eq "Heading - Blacklight"
expect(page_title).to be_html_safe
end

it "includes schema.org itemscope/type properties" do
allow(view).to receive(:document_show_html_title).and_return("Heading")
allow(document).to receive_messages(:itemtype => 'some-item-type-uri')
render
expect(rendered).to have_selector('div#document[@itemscope]')
Expand Down

0 comments on commit c496583

Please sign in to comment.