Redirect to main_app.root_url with error message when folder not found

[jiaola]

Hi,

We found a problem a user tries to access a folder that doesn't exist or has been deleted. It throws an error: ActiveRecord::RecordNotFound in Blacklight::Folders::FoldersController#show

We'd like to catch this error and provide a helpful message. This fix does that.

Thanks

David Jiao Indiana University Enterprise Library Systems

[jcoyne]

This rescue clause is too broad. You are asserting any missing ActiveRecord is a missing folder. Can you scope it more narrowly? I'd suggest actually doing a rescue block just in the method that is raising the error.

When you run an application in production mode, Rails already rescues ActiveRecord::RecordNotFound and renders a 404 page using the ExceptionWrapper middleware. Many people have come to count on that behavior and this PR would disable that for them.

See: https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb

[jiaola]

The general 404 error is not good enough and our library are asking for a similar error message like the one displayed for missing catalog record.

Since the rescue_from is in the scope of the FoldersController it won't catch any error in other controller.I don't know if there is a way to put the rescue_from in a method ('show' in this case), but I can add a line to check if params[:action]=='show'. Would that be acceptable?

[jcoyne]

Okay, I didn't notice at first that this was only in the FoldersController. That is probably tightly enough scoped. I think it's easier to understand if you just put that in the show action though. Add :show to the exclude list of load_and_authorize_resources then load the object in the show method.

def show
  begin
    @folder = Folder.find(params[:id])
  rescue  ActiveRecord::RecordNotFound
     ...
  end
  authorize! :show, @folder
   ...
end

This pull request will also need a test to show that the added code is running correctly.

[jiaola]

Thanks! I'll put the code in the method and write a test. I'll close this pull request and start a new one so it's cleaner.