From 094a35106008d914a2cc07fde15b2842e256dc74 Mon Sep 17 00:00:00 2001
From: Justin Coyne <jcoyne@justincoyne.com>
Date: Tue, 30 Oct 2018 10:25:01 -0500
Subject: [PATCH] Deprecate passing the label to link_to_document

This is not something we do in the Blacklight code and supporting extra
conditionals makes it harder to reason about.
---
 app/helpers/blacklight/url_helper_behavior.rb       |  1 +
 app/presenters/blacklight/thumbnail_presenter.rb    |  2 +-
 spec/helpers/blacklight/url_helper_behavior_spec.rb | 11 ++++++++++-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/app/helpers/blacklight/url_helper_behavior.rb b/app/helpers/blacklight/url_helper_behavior.rb
index 6de8563ed0..77ae7b169d 100644
--- a/app/helpers/blacklight/url_helper_behavior.rb
+++ b/app/helpers/blacklight/url_helper_behavior.rb
@@ -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
 
diff --git a/app/presenters/blacklight/thumbnail_presenter.rb b/app/presenters/blacklight/thumbnail_presenter.rb
index ea520fffed..55e9d713c7 100644
--- a/app/presenters/blacklight/thumbnail_presenter.rb
+++ b/app/presenters/blacklight/thumbnail_presenter.rb
@@ -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
diff --git a/spec/helpers/blacklight/url_helper_behavior_spec.rb b/spec/helpers/blacklight/url_helper_behavior_spec.rb
index 8cc0462714..608d1c18f0 100644
--- a/spec/helpers/blacklight/url_helper_behavior_spec.rb
+++ b/spec/helpers/blacklight/url_helper_behavior_spec.rb
@@ -201,17 +201,21 @@
 
     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
 
@@ -219,18 +223,23 @@
       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
 
@@ -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