diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 0b172c792b..f1f6027558 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -229,6 +229,16 @@ def current_ability end end + def handle_routing_error + if request.format == :json + render json: { errors: ["API action does not exist. Check that your URL and HTTP request method are correct."] }, status: :not_found + elsif request.format == :html + render file: Rails.root.join('public', '404.html').to_s, status: :not_found, layout: false + else + head :not_found + end + end + def configure_permitted_parameters devise_parameter_sanitizer.permit(:sign_up) do |user_params| user_params.permit(:username, :email, :password, :password_confirmation) diff --git a/config/routes.rb b/config/routes.rb index 33b2b59afb..4c83b693e1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -274,4 +274,11 @@ end end end + + # Routing errors are triggered in the middleware stack so can't be handled by + # a regular rescue_from block. Constraint is necessary to not break retrieval + # of things from active storage. + match '*unmatched', to: 'application#handle_routing_error', via: :all, constraints: lambda { |req| + req.path.exclude? 'rails/active_storage' + } end diff --git a/spec/controllers/admin_collections_controller_spec.rb b/spec/controllers/admin_collections_controller_spec.rb index 0ac38ebe0c..0c57ce0b53 100644 --- a/spec/controllers/admin_collections_controller_spec.rb +++ b/spec/controllers/admin_collections_controller_spec.rb @@ -417,13 +417,20 @@ collection = Admin::Collection.find(JSON.parse(response.body)['id']) expect(collection.managers).to eq([]) end - it "should return 422 if collection creation failed" do + it "should return 422 if unit not provided" do post 'create', params: { format:'json', admin_collection: { name: collection.name, description: collection.description } } expect(response.status).to eq(422) expect(JSON.parse(response.body)).to include('errors') expect(JSON.parse(response.body)["errors"].class).to eq Array expect(JSON.parse(response.body)["errors"].first.class).to eq String end + it "should return 422 if name not provided" do + post 'create', params: { format:'json', admin_collection: { description: collection.description, unit_id: collection.unit.id } } + expect(response.status).to eq(422) + expect(JSON.parse(response.body)).to include('errors') + expect(JSON.parse(response.body)["errors"].class).to eq Array + expect(JSON.parse(response.body)["errors"].first.class).to eq String + end end context 'when user does not have permissions for unit' do diff --git a/spec/controllers/admin_units_controller_spec.rb b/spec/controllers/admin_units_controller_spec.rb index 3821716d58..7e473941af 100644 --- a/spec/controllers/admin_units_controller_spec.rb +++ b/spec/controllers/admin_units_controller_spec.rb @@ -332,7 +332,7 @@ unit = Admin::Unit.find(JSON.parse(response.body)['id']) expect(unit.unit_admins).to eq([administrator.username]) end - it "should return 422 if unit creation failed" do + it "should return 422 if name not provided" do post 'create', params: { format: 'json', admin_unit: { description: unit.description } } expect(response.status).to eq(422) expect(JSON.parse(response.body)).to include('errors') diff --git a/spec/controllers/media_objects_controller_spec.rb b/spec/controllers/media_objects_controller_spec.rb index fd8b321883..48afc0182f 100644 --- a/spec/controllers/media_objects_controller_spec.rb +++ b/spec/controllers/media_objects_controller_spec.rb @@ -302,6 +302,17 @@ expect(JSON.parse(response.body)["errors"].class).to eq Array expect(JSON.parse(response.body)["errors"].first.class).to eq String end + it "should return 422 if required fields are missing" do + media_object = FactoryBot.create(:published_media_object) + fields = {} + descMetadata_fields.each { |f| fields[f] = media_object.send(f) } + fields[:title] = nil + post 'create', params: { format: 'json', fields: fields, files: [master_file], collection_id: collection.id, publish: true } + expect(response.status).to eq(422) + expect(JSON.parse(response.body)).to include('errors') + expect(JSON.parse(response.body)["errors"].class).to eq Array + expect(JSON.parse(response.body)["errors"].first.class).to eq String + end it "should create a new media_object" do # master_file_obj = FactoryBot.create(:master_file, master_file.slice(:files)) media_object = FactoryBot.create(:media_object)#, sections: [master_file_obj]) @@ -569,6 +580,17 @@ expect(JSON.parse(response.body)["errors"].class).to eq Array expect(JSON.parse(response.body)["errors"].first.class).to eq String end + it "should return 422 if required fields are missing" do + media_object = FactoryBot.create(:published_media_object) + fields = {} + descMetadata_fields.each { |f| fields[f] = media_object.send(f) } + fields[:title] = nil + put 'json_update', params: { format: 'json', id: media_object.id, fields: { title: nil }, files: [master_file], collection_id: collection.id } + expect(response.status).to eq(422) + expect(JSON.parse(response.body)).to include('errors') + expect(JSON.parse(response.body)["errors"].class).to eq Array + expect(JSON.parse(response.body)["errors"].first.class).to eq String + end context "as an authorized non-admin user" do let(:user) { FactoryBot.create(:user) } diff --git a/spec/requests/routing_error_spec.rb b/spec/requests/routing_error_spec.rb new file mode 100644 index 0000000000..8c6ef00559 --- /dev/null +++ b/spec/requests/routing_error_spec.rb @@ -0,0 +1,35 @@ +# Copyright 2011-2026, The Trustees of Indiana University and Northwestern +# University. Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed +# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +# CONDITIONS OF ANY KIND, either express or implied. See the License for the +# specific language governing permissions and limitations under the License. +# --- END LICENSE_HEADER BLOCK --- + +require 'rails_helper.rb' + +describe 'Routing Error Handling', type: :request do + it 'responds with 404 page for html request' do + get '/fake/route', params: { format: :html } + expect(response.status).to eq(404) + expect(response.body).to include('The page you were looking for doesn’t exist (404 Not found)') + end + + it 'responds with json error for json request' do + get '/fake/route', params: { format: :json } + expect(response.status).to eq(404) + expect(JSON.parse(response.body)).to eq("errors" => ["API action does not exist. Check that your URL and HTTP request method are correct."]) + end + + it 'responds with 404 status for other request' do + get '/fake/route', params: { format: :atom } + expect(response.status).to eq(404) + expect(response.body).to be_empty + end +end diff --git a/spec/routing/media_objects_routing_spec.rb b/spec/routing/media_objects_routing_spec.rb index 85e74773b2..d0e2d556f3 100644 --- a/spec/routing/media_objects_routing_spec.rb +++ b/spec/routing/media_objects_routing_spec.rb @@ -19,5 +19,8 @@ it "routes to #move_preview.json" do expect(:get => "/media_objects/abc1234/move_preview.json").to route_to("media_objects#move_preview", id: 'abc1234', format: 'json') end + it "handles routing errors" do + expect(post: "/media_objects/abc1234").to route_to("application#handle_routing_error", unmatched: "media_objects/abc1234") + end end end