diff --git a/lib/blacklight/catalog.rb b/lib/blacklight/catalog.rb index 4dcdff3eaa..bf40fcda2d 100644 --- a/lib/blacklight/catalog.rb +++ b/lib/blacklight/catalog.rb @@ -245,12 +245,31 @@ def validate_email_params flash[:error].blank? end - # when a request for /catalog/BAD_SOLR_ID is made, this method is executed... - def invalid_solr_id_error - flash[:notice] = I18n.t('blacklight.search.errors.invalid_solr_id') - params.delete(:id) - index - render "index", :status => 404 + # 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_solr_id_error(exception) + 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}/public/404.html", :status => 404, :layout => false, :content_type => 'text/html' + end + end end def start_new_search_session? diff --git a/spec/controllers/catalog_controller_spec.rb b/spec/controllers/catalog_controller_spec.rb index ee7f565cbe..35f7d7f49d 100644 --- a/spec/controllers/catalog_controller_spec.rb +++ b/spec/controllers/catalog_controller_spec.rb @@ -464,10 +464,21 @@ def export_as_mock controller.stub(:find => @mock_response, :get_single_doc_via_search => @mock_document) get :show, :id=>"987654321" - expect(request.flash[:notice]).to eq "Sorry, you have requested a record that doesn't exist." - expect(response).to render_template('index') expect(response.status).to eq 404 + expect(response.content_type).to eq Mime::HTML end + it "should return status 404 for a record that doesn't exist even for non-html format" do + @mock_response = double() + @mock_response.stub(:docs => []) + @mock_document = double() + controller.stub(:find => @mock_response, + :get_single_doc_via_search => @mock_document) + + get :show, :id=>"987654321", :format => "xml" + expect(response.status).to eq 404 + expect(response.content_type).to eq Mime::XML + end + it "should redirect the user to the root url for a bad search" do req = {} res = {} diff --git a/spec/features/record_view_spec.rb b/spec/features/record_view_spec.rb index 036aae54c1..94f814f343 100644 --- a/spec/features/record_view_spec.rb +++ b/spec/features/record_view_spec.rb @@ -35,6 +35,6 @@ it "should not display 404" do visit catalog_path('this_id_does_not_exist') page.driver.status_code.should == 404 - expect(page).to have_content "Sorry, you have requested a record that doesn't exist." + expect(page).to have_content "The page you were looking for doesn't exist." end end