Missing resources should redirect and error.

This should prevent a whole suite of Honeybadger errors, and avoid
displaying 500 errors to users when they can't do anything about it.

Closes #2534

app/controllers/application_controller.rb

@@ -32,6 +32,16 @@ def deny_resource_access(exception)
     end
   end
 
+  rescue_from Valkyrie::Persistence::ObjectNotFoundError, with: :resource_not_found
+  def resource_not_found(_exception)
+    respond_to do |format|
+      format.json { head :not_found }
+      format.html do
+        redirect_to root_url, alert: "The requested resource does not exist."
+      end
+    end
+  end
+
   # Figgy has no use cases for having unique shared searches, and this prevents
   # the user list from growing out of control.
   def guest_user

spec/resources/ephemera_box/ephemera_boxes_controller_spec.rb

@@ -154,7 +154,8 @@
     end
     context "when a ephemera box doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -193,7 +194,8 @@
     end
     context "when a ephemera box doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     it_behaves_like "a workflow controller", :ephemera_box

spec/resources/ephemera_field/ephemera_fields_controller_spec.rb

@@ -111,7 +111,8 @@
     end
     context "when a ephemera field doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -151,7 +152,8 @@
     end
     context "when a ephemera field doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/resources/ephemera_folder/ephemera_folders_controller_spec.rb

@@ -254,7 +254,8 @@
     end
     context "when a ephemera folder doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -382,7 +383,8 @@
     end
     context "when a ephemera folder doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/resources/ephemera_project/ephemera_projects_controller_spec.rb

@@ -148,7 +148,8 @@
     end
     context "when a ephemera project doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -184,7 +185,8 @@
     end
     context "when a ephemera project doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/resources/ephemera_term/ephemera_terms_controller_spec.rb

@@ -108,7 +108,8 @@
     end
     context "when a ephemera term doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -132,7 +133,8 @@
     end
     context "when a ephemera term doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/resources/ephemera_vocabulary/ephemera_vocabularies_controller_spec.rb

@@ -125,7 +125,8 @@
     end
     context "when a ephemera vocabulary doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -149,7 +150,8 @@
     end
     context "when a ephemera vocabulary doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/resources/file_set/file_sets_controller_spec.rb

@@ -26,7 +26,8 @@
 
     context "with an invalid FileSet ID" do
       it "displays an error" do
-        expect { patch :update, params: { id: "no-exist", file_set: { title: ["Second"] } } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "no-exist", file_set: { title: ["Second"] } }
+        expect(response).to redirect_to_not_found
       end
     end
 

spec/resources/raster_resource/raster_resources_controller_spec.rb

@@ -118,7 +118,8 @@
     end
     context "when a raster resource doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -156,7 +157,8 @@
     end
     context "when a raster resource doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/resources/scanned_map/scanned_maps_controller_spec.rb

@@ -177,7 +177,8 @@
     end
     context "when a map image doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -201,7 +202,8 @@
     end
     context "when a map image doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -239,7 +241,8 @@
     end
     context "when a map image doesn't exist" do
       it "raises an error" do
-        expect { get :structure, params: { id: "banana" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :structure, params: { id: "banana" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/resources/scanned_resource/scanned_resources_controller_spec.rb

@@ -203,7 +203,8 @@
     end
     context "when a scanned resource doesn't exist" do
       it "raises an error" do
-        expect { get :structure, params: { id: "banana" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :structure, params: { id: "banana" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/resources/vector_resource/vector_resources_controller_spec.rb

@@ -118,7 +118,8 @@
     end
     context "when a vector resource doesn't exist" do
       it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -155,7 +156,8 @@
     end
     context "when a vector resource doesn't exist" do
       it "raises an error" do
-        expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        patch :update, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do

spec/shared_specs/controllers/base_resource_controller.rb

@@ -4,7 +4,6 @@
 RSpec.shared_examples "a BaseResourceController" do |*flags|
   include ActionDispatch::Routing::PolymorphicRoutes
   include Rails.application.routes.url_helpers
-
   with_queue_adapter :inline
   let(:user) { nil }
   let(:adapter) { Valkyrie::MetadataAdapter.find(:indexing_persister) }
@@ -157,6 +156,11 @@
       expect(response).to redirect_to root_path
       expect { query_service.find_by(id: resource.id) }.to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
     end
+    it "returns a 404 when given a bad resource ID" do
+      delete :destroy, params: { id: SecureRandom.uuid }
+
+      expect(response).to redirect_to_not_found
+    end
   end
 
   describe "edit" do
@@ -168,8 +172,9 @@
       it_behaves_like "an access controlled edit request"
     end
     context "when a resource doesn't exist" do
-      it "raises an error" do
-        expect { get :edit, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+      it "redirects" do
+        get :edit, params: { id: "test" }
+        expect(response).to redirect_to_not_found
       end
     end
     context "when it does exist" do
@@ -194,7 +199,8 @@
       end
       context "when a resource doesn't exist" do
         it "raises an error" do
-          expect { patch :update, params: { id: "test" } }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+          patch :update, params: { id: "test" }
+          expect(response).to redirect_to_not_found
         end
       end
       context "when it does exist" do

spec/support/matchers/redirect_to_not_found.rb

@@ -0,0 +1,6 @@
+RSpec::Matchers.define :redirect_to_not_found do
+  match do |actual|
+    expect(actual).to redirect_to '/'
+    expect(actual.flash["alert"]).to eq "The requested resource does not exist."
+  end
+end