Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate render_document_partials. #2546

Merged
merged 1 commit into from
Dec 9, 2021
Merged

Deprecate render_document_partials. #2546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/components/blacklight/facet_field_list_component.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
module Blacklight
class FacetFieldListComponent < ::ViewComponent::Base
extend Deprecation
self.deprecation_horizon = 'blacklight 9.0'

def initialize(facet_field:, layout: nil)
@facet_field = facet_field
Expand Down
4 changes: 4 additions & 0 deletions app/helpers/blacklight/render_partials_helper_behavior.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# frozen_string_literal: true

module Blacklight::RenderPartialsHelperBehavior
extend Deprecation
self.deprecation_horizon = 'blacklight 9.0'

##
# Render the document index view
#
Expand All @@ -22,6 +25,7 @@ def render_document_partials(doc, partials = [], locals = {})
render_document_partial(doc, action_name, locals)
end, "\n")
end
deprecation_deprecate render_document_partials: 'Render the document_component instead'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean? If I get this error I'm not sure what to do. Can we expand this deprecation error? Or if I just put in render_document_component instead of render_document_partials it'll be good?

Copy link
Member Author

@jcoyne jcoyne Nov 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would be a rather long message to stick here:

Replace this call with:
"document_component = blacklight_config.view_config(:atom).summary_component
render document_component.new(presenter: document_presenter(document), component: :div, show: true)" instead.

This assumes that the current context has a document_presenter and blacklight_config helpers and that :atom is the current config context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be in the minority here, but I'm 👍 to that long message. Changing it to "Replace this call with a document component render, e.g: allthat" might be nice to be more clear?


##
# Return the list of xml for a given solr document. Doesn't safely escape for HTML.
Expand Down
5 changes: 2 additions & 3 deletions app/views/catalog/_document.atom.builder
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ xml.entry do

with_format(:html) do
xml.summary "type" => "html" do
xml.text! render_document_partials(document,
blacklight_config.view_config(:atom).summary_partials,
document_counter: document_counter)
document_component = blacklight_config.view_config(:atom).summary_component
xml.text! render document_component.new(presenter: document_presenter(document), component: :div, show: true)
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/blacklight/configuration.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def default_values
atom: {
if: false, # by default, atom should not show up as an alternative view
partials: [:document],
summary_partials: [:index]
summary_component: Blacklight::DocumentComponent
},
rss: {
if: false, # by default, rss should not show up as an alternative view
Expand Down
27 changes: 17 additions & 10 deletions spec/views/catalog/index.atom.builder_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
RSpec.describe "catalog/index" do
let(:document_list) do
10.times.map do |i|
doc = SolrDocument.new(id: i)
allow(doc).to receive(:export_as_some_format).and_return("")
allow(doc).to receive(:to_semantic_values).and_return(author: ['xyz']) if i.zero?
doc.will_export_as(:some_format, "application/some-format") if i == 1
doc
SolrDocument.new(id: i, title_tsim: "Title #{i}").tap do |doc|
allow(doc).to receive(:export_as_some_format).and_return("")
allow(doc).to receive(:to_semantic_values).and_return(author: ['xyz']) if i.zero?
doc.will_export_as(:some_format, "application/some-format") if i == 1
end
end
end

Expand Down Expand Up @@ -86,15 +86,22 @@
end

it "has a summary" do
stub_template "catalog/_index.html.erb" => "partial content"
render template: 'catalog/index', formats: [:atom]
expect(rendered).to have_selector("entry > summary", text: 'partial content')
expect(rendered).to have_selector("entry > summary", text: 'Title 0')
end

context 'with a custom HTML partial' do
context 'with a custom template' do
before do
blacklight_config.view.atom.summary_partials = ['whatever']
stub_template 'catalog/_whatever_default.html.erb' => 'whatever content'
my_template = Class.new(ViewComponent::Base) do
def call
'whatever content'
end

def self.name
'TestComponent'
end
end
blacklight_config.view.atom.summary_component = my_template
end

it "has the customized summary" do
Expand Down