From 751e3620c489a202c9c487f0b1355b4de5d22069 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Fri, 25 Aug 2017 10:31:05 -0500 Subject: [PATCH] Remove option of passing false as the second argument to thumbnail_tag 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) ``` --- .../blacklight/thumbnail_presenter.rb | 7 +-- spec/helpers/catalog_helper_spec.rb | 61 ++++--------------- spec/presenters/thumbnail_presenter_spec.rb | 8 --- 3 files changed, 13 insertions(+), 63 deletions(-) diff --git a/app/presenters/blacklight/thumbnail_presenter.rb b/app/presenters/blacklight/thumbnail_presenter.rb index cfbfa0a405..6962188054 100644 --- a/app/presenters/blacklight/thumbnail_presenter.rb +++ b/app/presenters/blacklight/thumbnail_presenter.rb @@ -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 diff --git a/spec/helpers/catalog_helper_spec.rb b/spec/helpers/catalog_helper_spec.rb index 9547f3942a..375eb9e18a 100644 --- a/spec/helpers/catalog_helper_spec.rb +++ b/spec/helpers/catalog_helper_spec.rb @@ -226,65 +226,26 @@ 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 @@ -292,7 +253,7 @@ def render_grouped_response? 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) diff --git a/spec/presenters/thumbnail_presenter_spec.rb b/spec/presenters/thumbnail_presenter_spec.rb index 10dbde97e5..62742ad41a 100644 --- a/spec/presenters/thumbnail_presenter_spec.rb +++ b/spec/presenters/thumbnail_presenter_spec.rb @@ -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) }