Merge pull request #1036 from projectblacklight/deprecation-warnings

Deprecation warnings

Author: jcoyne

app/views/catalog/email_sent.html.erb

@@ -0,0 +1,2 @@
+<% Deprecation.warn controller, "The template catalog/email_sent.html.erb is deprecated; use email_success instead" %>
+<%= render template: "catalog/email_success" %>

app/views/catalog/sms_sent.html.erb

@@ -0,0 +1,2 @@
+<% Deprecation.warn controller, "The template catalog/sms_sent.html.erb is deprecated; use sms_success instead" %>
+<%= render template: "catalog/sms_success" %>

lib/blacklight/catalog/component_configuration.rb

@@ -34,8 +34,10 @@ module ClassMethods
       #                             The proc will receive the action configuration and the document or documents for the action.
       def add_show_tools_partial name, opts = {}
         blacklight_config.add_show_tools_partial(name, opts)
+
         define_method name do
           @response, @documents = action_documents
+
           if request.post? and
               opts[:callback] and
               (opts[:validator].blank? || self.send(opts[:validator]))
@@ -54,7 +56,7 @@ def add_show_tools_partial name, opts = {}
               format.js { render :layout => false }
             end
           end
-        end
+        end unless method_defined? name
       end
 
       ##

lib/blacklight/solr_helper.rb

@@ -67,19 +67,7 @@ def find *args
 
   # returns a params hash for finding a single solr document (CatalogController #show action)
   def solr_doc_params(id=nil)
-    id ||= params[:id]
-
-    # add our document id to the document_unique_id_param query parameter
-    p = blacklight_config.default_document_solr_params.merge({
-      # this assumes the request handler will map the unique id param
-      # to the unique key field using either solr local params, the
-      # real-time get handler, etc.
-      blacklight_config.document_unique_id_param => id
-    })
-
-    p[:qt] ||= blacklight_config.document_solr_request_handler
-
-    p
+    default_solr_doc_params(id)
   end
   deprecation_deprecate :solr_doc_params
 
@@ -116,6 +104,20 @@ def query_solr(user_params = params || {}, extra_controller_params = {})
   # retrieve a solr document, given the doc id
   # @return [Blacklight::SolrResponse, Blacklight::SolrDocument] the solr response object and the first document
   def get_solr_response_for_doc_id(id=nil, extra_controller_params={})
+    if id.nil?
+      Deprecation.warn Blacklight::SolrHelper, "Calling #get_solr_response_for_doc_id without an explicit id argument is deprecated"
+      id ||= params[:id]
+    end
+
+    old_solr_doc_params = Deprecation.silence(Blacklight::SolrHelper) do
+      solr_doc_params(id)
+    end
+
+    if default_solr_doc_params(id) != old_solr_doc_params
+      Deprecation.warn Blacklight::SolrHelper, "The #solr_doc_params method is deprecated. Instead, you should provide a custom SolrRepository implementation for the additional behavior you're offering"
+      extra_controller_params = extra_controller_params.merge(old_solr_doc_params)
+    end
+
     solr_response = solr_repository.find id, extra_controller_params
     [solr_response, solr_response.documents.first]
   end
@@ -234,4 +236,24 @@ def solr_repository
     @solr_repository ||= Blacklight::SolrRepository.new(blacklight_config)
   end
 
+  private
+
+  ##
+  # @deprecated
+  def default_solr_doc_params(id=nil)
+    id ||= params[:id]
+
+    # add our document id to the document_unique_id_param query parameter
+    p = blacklight_config.default_document_solr_params.merge({
+      # this assumes the request handler will map the unique id param
+      # to the unique key field using either solr local params, the
+      # real-time get handler, etc.
+      blacklight_config.document_unique_id_param => id
+    })
+
+    p[:qt] ||= blacklight_config.document_solr_request_handler
+
+    p
+  end
+
 end

spec/lib/blacklight/catalog/component_configuration_spec.rb

@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe Blacklight::Catalog::ComponentConfiguration do
+  subject do
+    Class.new do
+      include Blacklight::Configurable
+      include Blacklight::Catalog::ComponentConfiguration
+
+      def some_existing_action
+        1
+      end
+    end
+  end
+
+  describe ".add_show_tools_partial" do
+    it "should define an action method" do
+      subject.add_show_tools_partial :xyz
+      expect(subject.new).to respond_to :xyz
+    end
+
+    it "should not replace an existing method" do
+      subject.add_show_tools_partial :some_existing_action
+      expect(subject.new.some_existing_action).to eq 1
+    end
+  end
+
+  
+
+end