Skip to content

Commit

Permalink
Remove option of passing false as the second argument to thumbnail_tag
Browse files Browse the repository at this point in the history
It's much more clear to the reader what the intention of this code is:

```ruby
thumbnail_tag({}, suppress_link: true)
```
as compared to the more obtuse:
```ruby
thumbnail_tag({}, false)
```
  • Loading branch information
jcoyne committed Aug 25, 2017
1 parent daa693c commit 751e362
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 63 deletions.
7 changes: 2 additions & 5 deletions app/presenters/blacklight/thumbnail_presenter.rb
Expand Up @@ -31,11 +31,8 @@ def exists?
# rubocop:disable Lint/AssignmentInCondition
def thumbnail_tag image_options = {}, url_options = {}
return unless value = thumbnail_value(image_options)
if url_options == false || url_options[:suppress_link]
value
else
view_context.link_to_document document, value, url_options
end
return value if url_options[:suppress_link]
view_context.link_to_document document, value, url_options
end
# rubocop:enable Lint/AssignmentInCondition

Expand Down
61 changes: 11 additions & 50 deletions spec/helpers/catalog_helper_spec.rb
Expand Up @@ -226,73 +226,34 @@ def render_grouped_response?
end

describe "render_thumbnail_tag" do
let(:index_presenter) do
instance_double(Blacklight::IndexPresenter, thumbnail: thumbnail_presenter)
end
let(:thumbnail_presenter){ instance_double(Blacklight::ThumbnailPresenter) }

before do
expect(Deprecation).to receive(:warn)
allow(helper).to receive(:index_presenter).with(document).and_return(index_presenter)
end

let(:document) { instance_double(SolrDocument) }

it "calls the provided thumbnail method" do
allow(helper).to receive_messages(:blacklight_config => Blacklight::Configuration.new(:index => Blacklight::OpenStructWithHashAccess.new(:thumbnail_method => :xyz, document_presenter_class: Blacklight::IndexPresenter) ))
expect(helper).to receive_messages(:xyz => "some-thumbnail")

allow(helper).to receive(:link_to_document).with(document, "some-thumbnail", {})
helper.render_thumbnail_tag document
end

it "creates an image tag from the given field" do
allow(helper).to receive_messages(:blacklight_config => Blacklight::Configuration.new(:index => Blacklight::OpenStructWithHashAccess.new(:thumbnail_field => :xyz, document_presenter_class: Blacklight::IndexPresenter) ))

allow(document).to receive(:has?).with(:xyz).and_return(true)
allow(document).to receive(:first).with(:xyz).and_return("http://example.com/some.jpg")

expect(helper).to receive(:link_to_document).with(document, image_tag("http://example.com/some.jpg"), {})
it "calls thumbnail presenter with default values" do
expect(thumbnail_presenter).to receive(:thumbnail_tag).with({}, {})
helper.render_thumbnail_tag document
end

it "does not link to the document if the url options are false" do
allow(helper).to receive_messages(:blacklight_config => Blacklight::Configuration.new(:index => Blacklight::OpenStructWithHashAccess.new(:thumbnail_method => :xyz, document_presenter_class: Blacklight::IndexPresenter) ))
allow(helper).to receive_messages(:xyz => "some-thumbnail")

result = helper.render_thumbnail_tag document, {}, false
expect(result).to eq "some-thumbnail"
end

it "does not link to the document if the url options have :suppress_link" do
allow(helper).to receive_messages(:blacklight_config => Blacklight::Configuration.new(:index => Blacklight::OpenStructWithHashAccess.new(:thumbnail_method => :xyz, document_presenter_class: Blacklight::IndexPresenter) ))
allow(helper).to receive_messages(:xyz => "some-thumbnail")

it "calls thumbnail presenter with provided values" do
expect(thumbnail_presenter).to receive(:thumbnail_tag).with({}, suppress_link: true)
result = helper.render_thumbnail_tag document, {}, suppress_link: true
expect(result).to eq "some-thumbnail"
end


it "returns nil if no thumbnail is available" do
allow(helper).to receive_messages(:blacklight_config => Blacklight::Configuration.new(:index => Blacklight::OpenStructWithHashAccess.new(document_presenter_class: Blacklight::IndexPresenter) ))
expect(helper.render_thumbnail_tag document).to be_nil
end

it "returns nil if no thumbnail is returned from the thumbnail method" do
allow(helper).to receive_messages(:blacklight_config => Blacklight::Configuration.new(:index => Blacklight::OpenStructWithHashAccess.new(:thumbnail_method => :xyz, document_presenter_class: Blacklight::IndexPresenter) ))
allow(helper).to receive_messages(:xyz => nil)

expect(helper.render_thumbnail_tag document).to be_nil
end

it "returns nil if no thumbnail is in the document" do
allow(helper).to receive_messages(:blacklight_config => Blacklight::Configuration.new(:index => Blacklight::OpenStructWithHashAccess.new(:thumbnail_field => :xyz, document_presenter_class: Blacklight::IndexPresenter) ))

allow(document).to receive(:first).with(:xyz).and_return(nil)

expect(helper.render_thumbnail_tag document).to be_nil
end
end

describe "thumbnail_url" do
before do
expect(Deprecation).to receive(:warn)
end

it "pulls the configured thumbnail field out of the document" do
allow(helper).to receive_messages(:blacklight_config => Blacklight::Configuration.new(:index => Blacklight::OpenStructWithHashAccess.new(:thumbnail_field => :xyz) ))
document = instance_double(SolrDocument)
Expand Down
8 changes: 0 additions & 8 deletions spec/presenters/thumbnail_presenter_spec.rb
Expand Up @@ -57,14 +57,6 @@
expect(subject).to eq "link"
end

context "and url options are false" do
subject { presenter.thumbnail_tag({}, false) }

it "does not link to the document" do
expect(subject).to eq "some-thumbnail"
end
end

context "and url options have :suppress_link" do
subject { presenter.thumbnail_tag({}, suppress_link: true) }

Expand Down

0 comments on commit 751e362

Please sign in to comment.