Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing resources should redirect and error. #2548

Merged
merged 1 commit into from
Feb 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions spec/resources/ephemera_box/ephemera_boxes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion spec/resources/file_set/file_sets_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions spec/resources/scanned_map/scanned_maps_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions spec/shared_specs/controllers/base_resource_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions spec/support/matchers/redirect_to_not_found.rb
Original file line number Diff line number Diff line change
@@ -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