Merge pull request #1623 from projectblacklight/action_dispatch

Leverage ActionDispatch to handle missing document errors

app/controllers/concerns/blacklight/catalog.rb

@@ -13,10 +13,6 @@ module Blacklight::Catalog
 
     helper Blacklight::Facet
 
-    # When an action raises Blacklight::Exceptions::RecordNotFound, handle
-    # the exception appropriately.
-    rescue_from Blacklight::Exceptions::RecordNotFound, with: :invalid_document_id_error
-
     # The index action will more than likely throw this one.
     # Example: when the standard query parser is used, and a user submits a "bad" query.
     rescue_from Blacklight::Exceptions::InvalidRequest, with: :handle_request_error
@@ -246,36 +242,6 @@ def validate_email_params
       flash[:error].blank?
     end
 
-    ##
-    # when a request for /catalog/BAD_SOLR_ID is made, this method is executed.
-    # Just returns a 404 response, but you can override locally in your own
-    # CatalogController to do something else -- older BL displayed a Catalog#inde
-    # page with a flash message and a 404 status.
-    def invalid_document_id_error(exception)
-      raise exception unless Rails.root.join('public', '404.html').exist?
-
-      error_info = {
-        "status" => "404",
-        "error"  => "#{exception.class}: #{exception.message}"
-      }
-
-      respond_to do |format|
-        format.xml  { render :xml  => error_info, :status => 404 }
-        format.json { render :json => error_info, :status => 404 }
-
-        # default to HTML response, even for other non-HTML formats we don't
-        # neccesarily know about, seems to be consistent with what Rails4 does
-        # by default with uncaught ActiveRecord::RecordNotFound in production
-        format.any do
-          # use standard, possibly locally overridden, 404.html file. Even for
-          # possibly non-html formats, this is consistent with what Rails does
-          # on raising an ActiveRecord::RecordNotFound. Rails.root IS needed
-          # for it to work under testing, without worrying about CWD.
-          render file: Rails.root.join('public', '404.html'), status: 404, layout: false, content_type: 'text/html'
-        end
-      end
-    end
-
     def start_new_search_session?
       action_name == "index"
     end

lib/blacklight/engine.rb

@@ -45,5 +45,9 @@ class Engine < Rails::Engine
     config.bookmarks_http_method = :post
 
     config.email_regexp = defined?(Devise) ? Devise.email_regexp : /\A[^@\s]+@[^@\s]+\z/
+
+    config.action_dispatch.rescue_responses.merge!(
+      "Blacklight::Exceptions::RecordNotFound" => :not_found
+    )
   end
 end

spec/controllers/catalog_controller_spec.rb

@@ -463,16 +463,9 @@ def export_as_mock
   describe "errors" do
     it "returns status 404 for a record that doesn't exist" do
       allow(controller).to receive_messages(:find => double(documents: []))
-      get :show, params: { id: "987654321" }
-      expect(response.status).to eq 404
-      expect(response.content_type).to eq Mime[:html]
-    end
-
-    it "returns status 404 for a record that doesn't exist even for non-html format" do
-      allow(controller).to receive_messages(:find => double(documents: []))
-      get :show, params: { id: "987654321", format: "xml" }
-      expect(response.status).to eq 404
-      expect(response.content_type).to eq Mime[:xml]
+      expect do
+        get :show, params: { id: "987654321" }
+      end.to raise_error Blacklight::Exceptions::RecordNotFound
     end
 
     it "redirects the user to the root url for a bad search" do

spec/features/record_view_spec.rb

@@ -31,9 +31,4 @@
     expect(page).to have_content "林行止" 
     expect(page).to have_content "臺北縣板橋市" 
   end
-  it "does not display 404" do
-    visit solr_document_path('this_id_does_not_exist')
-    expect(page.driver.status_code).to eq 404
-    expect(page).to have_content "The page you were looking for doesn't exist." 
-  end
 end