Skip to content

Commit

Permalink
Deprecate passing the label to link_to_document
Browse files Browse the repository at this point in the history
This is not something we do in the Blacklight code and supporting extra
conditionals makes it harder to reason about.
  • Loading branch information
jcoyne committed Oct 30, 2018
1 parent 721c3b3 commit 094a351
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 2 deletions.
1 change: 1 addition & 0 deletions app/helpers/blacklight/url_helper_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ def link_to_document(doc, field_or_opts = nil, opts = { counter: nil })
if field_or_opts.is_a? Hash
opts = field_or_opts
else
Deprecation.warn(self, "Passing a field argument to link_to_document is deprecated and will be removed in Blacklight 8")
field = field_or_opts
end

Expand Down
2 changes: 1 addition & 1 deletion app/presenters/blacklight/thumbnail_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def exists?
def thumbnail_tag image_options = {}, url_options = {}
value = thumbnail_value(image_options)
return value if value.nil? || url_options[:suppress_link]
view_context.link_to_document document, value, url_options
view_context.link_to value, view_context.url_for_document(document), view_context.document_link_params(document, url_options)
end

private
Expand Down
11 changes: 10 additions & 1 deletion spec/helpers/blacklight/url_helper_behavior_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,36 +201,45 @@

before do
allow(controller).to receive(:action_name).and_return('index')
allow(Deprecation).to receive(:warn)
end

it "consists of the document title wrapped in a <a>" do
expect(Deprecation).to receive(:warn)
expect(helper.link_to_document(document, :title_tsim)).to have_selector("a", text: '654321', count: 1)
end

it "accepts and returns a string label" do
expect(Deprecation).to receive(:warn)
expect(helper.link_to_document(document, String.new('title_tsim'))).to have_selector("a", text: 'title_tsim', count: 1)
end

it "accepts and returns a Proc" do
expect(Deprecation).to receive(:warn)
expect(helper.link_to_document(document, proc { |doc, _opts| doc[:id] + ": " + doc.first(:title_tsim) })).to have_selector("a", text: '123456: 654321', count: 1)
end

context 'when label is missing' do
let(:data) { { 'id' => id } }

it "returns id" do
expect(Deprecation).to receive(:warn)
expect(helper.link_to_document(document, :title_tsim)).to have_selector("a", text: '123456', count: 1)
end

it "is html safe" do
expect(Deprecation).to receive(:warn)
expect(helper.link_to_document(document, :title_tsim)).to be_html_safe
end

it "passes on the title attribute to the link_to_with_data method" do
expect(Deprecation).to receive(:warn)
expect(helper.link_to_document(document, "Some crazy long label...", title: "Some crazy longer label")).to match(/title=\"Some crazy longer label\"/)
end

it "doesn't add an erroneous title attribute if one isn't provided" do
expect(Deprecation).to receive(:warn)

expect(helper.link_to_document(document, "Some crazy long label...")).not_to match(/title=/)
end

Expand All @@ -245,7 +254,7 @@

it "converts the counter parameter into a data- attribute" do
allow(helper).to receive(:track_test_path).with(hash_including(id: have_attributes(id: '123456'), counter: 5)).and_return('tracking url')

expect(Deprecation).to receive(:warn)
expect(helper.link_to_document(document, :title_tsim, counter: 5)).to include 'data-context-href="tracking url"'
end

Expand Down

0 comments on commit 094a351

Please sign in to comment.