From 22b3ff06d960bc5bd3ea48e3c8951d09e818d2b2 Mon Sep 17 00:00:00 2001 From: Janell-Huyck Date: Wed, 27 Sep 2023 16:47:14 -0400 Subject: [PATCH 1/5] wip, broken rescue_from --- app/controllers/application_controller.rb | 19 +++++ app/controllers/errors_controller.rb | 15 ++++ app/controllers/pages_controller.rb | 2 +- .../404.html => app/views/errors/404.html.erb | 0 .../422.html => app/views/errors/422.html.erb | 0 .../500.html => app/views/errors/500.html.erb | 0 config/routes.rb | 28 +++++-- ...lication_controller_error_handling_spec.rb | 84 +++++++++++++++++++ .../application_controller_spec.rb | 15 +++- spec/controllers/errors_controller_spec.rb | 48 +++++++++++ spec/routing/admin_routing_spec.rb | 35 ++++++++ spec/routing/dynamic_pages_routing_spec.rb | 21 +++++ spec/routing/error_pages_routing_spec.rb | 19 +++++ .../publications_and_submitters_spec.rb | 23 +++++ spec/routing/root_routing_spec.rb | 9 ++ 15 files changed, 310 insertions(+), 8 deletions(-) create mode 100644 app/controllers/errors_controller.rb rename public/404.html => app/views/errors/404.html.erb (100%) rename public/422.html => app/views/errors/422.html.erb (100%) rename public/500.html => app/views/errors/500.html.erb (100%) create mode 100644 spec/controllers/application_controller/application_controller_error_handling_spec.rb rename spec/controllers/{ => application_controller}/application_controller_spec.rb (78%) create mode 100644 spec/controllers/errors_controller_spec.rb create mode 100644 spec/routing/admin_routing_spec.rb create mode 100644 spec/routing/dynamic_pages_routing_spec.rb create mode 100644 spec/routing/error_pages_routing_spec.rb create mode 100644 spec/routing/publications_and_submitters_spec.rb create mode 100644 spec/routing/root_routing_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 743fd779..ecf6ad94 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,8 +2,27 @@ class ApplicationController < ActionController::Base include Pagy::Backend + + rescue_from ActiveRecord::RecordNotFound, with: :render_404 + rescue_from ActiveRecord::RecordInvalid, with: :render_422 + rescue_from StandardError, with: :render_500 + + before_action :check_date + def render_404(exception) + render template: 'errors/404', layout: 'application', status: :not_found + end + + def render_422(exception) + render template: 'errors/422', layout: 'application', status: :unprocessable_entity + end + + def render_500(exception) + logger.error exception + render template: 'errors/500', layout: 'application', status: :internal_server_error + end + private def check_date diff --git a/app/controllers/errors_controller.rb b/app/controllers/errors_controller.rb new file mode 100644 index 00000000..5678bec5 --- /dev/null +++ b/app/controllers/errors_controller.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true +# app/controllers/errors_controller.rb +class ErrorsController < ApplicationController + def not_found + render template: 'errors/404', layout: 'application', status: :not_found + end + + def unprocessable_entity + render template: 'errors/422', layout: 'application', status: :unprocessable_entity + end + + def internal_server_error + render template: 'errors/500', layout: 'application', status: :internal_server_error + end +end \ No newline at end of file diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 5e166180..58ac98a3 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -9,7 +9,7 @@ def show if valid_page? render template: "pages/#{safe_page}" else - render file: 'public/404.html', status: :not_found + render status: :not_found end end diff --git a/public/404.html b/app/views/errors/404.html.erb similarity index 100% rename from public/404.html rename to app/views/errors/404.html.erb diff --git a/public/422.html b/app/views/errors/422.html.erb similarity index 100% rename from public/422.html rename to app/views/errors/422.html.erb diff --git a/public/500.html b/app/views/errors/500.html.erb similarity index 100% rename from public/500.html rename to app/views/errors/500.html.erb diff --git a/config/routes.rb b/config/routes.rb index f450c710..cd54fe4e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,29 +1,45 @@ # frozen_string_literal: true Rails.application.routes.draw do - resources :colleges + # Resource routes resources :artworks - resources :books resources :book_chapters + resources :books + resources :colleges resources :digital_projects resources :editings resources :films resources :journal_articles resources :musical_scores + resources :other_publications resources :photographies resources :physical_media resources :plays resources :public_performances - resources :other_publications resources :submitters + + # Admin-related routes get 'citations', to: 'admin#citations' get 'toggle_links', to: 'admin#toggle_links' - get 'publications', to: 'publications#index' - get 'publications/:id', to: 'publications#index' get 'manage', to: 'admin#login' post 'manage/validate', to: 'admin#validate' + get '/csv/:controller_name', to: 'admin#csv', as: 'controller_name' + + + # Publications and submission routes + get 'publications', to: 'publications#index' + get 'publications/:id', to: 'publications#index' get 'finished', to: 'submitters#finished' + + + # Dynamic pages get '/pages/:page' => 'pages#show' - get '/csv/:controller_name', to: 'admin#csv', as: 'controller_name' + + # Root URL root 'submitters#new' + + # Custom Error Pages + match "/404", to: "errors#not_found", via: :all + match "/500", to: "errors#internal_server_error", via: :all + match "/422", to: "errors#unprocessable_entity", via: :all end diff --git a/spec/controllers/application_controller/application_controller_error_handling_spec.rb b/spec/controllers/application_controller/application_controller_error_handling_spec.rb new file mode 100644 index 00000000..b08e0f88 --- /dev/null +++ b/spec/controllers/application_controller/application_controller_error_handling_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ApplicationController, type: :controller do + include ApplicationHelper + + controller do + + def index + raise ActiveRecord::RecordNotFound + end + + def action_404 + raise ActiveRecord::RecordNotFound + end + + def action_422 + raise ActionController::UnprocessableEntity + end + + def action_500 + raise StandardError, 'This is a standard error' + end + end + + describe 'rescue_from ActiveRecord::RecordNotFound' do + before do + routes.draw { get 'index' => 'anonymous#index', as: :index } + end + + + it 'renders the not_found template' do + puts + puts "response: #{response.inspect}" + puts + expect(controller).to receive(:render_404) + get :index + end + + it 'returns HTTP status 404' do + get :action_404 + expect(response).to have_http_status(404) + end + end + + describe 'rescue_from ActionController::UnprocessableEntity' do + before do + routes.draw { get 'action_422' => 'anonymous#action_422' } + end + + it 'renders the unprocessable_entity template' do + get :action_422 + expect(response).to render_template('errors/422') + end + + it 'returns HTTP status 422' do + get :action_422 + expect(response).to have_http_status(422) + end + end + + describe 'rescue_from StandardError' do + before do + routes.draw { get 'action_500' => 'anonymous#action_500' } + end + + it 'renders the internal_server_error template' do + get :action_500 + expect(response).to render_template('errors/500') + end + + it 'returns HTTP status 500' do + get :action_500 + expect(response).to have_http_status(500) + end + + it 'logs the error' do + allow(controller.logger).to receive(:error) + get :action_500 + expect(controller.logger).to have_received(:error).with(an_instance_of(StandardError).and(having_attributes(message: 'This is a standard error'))) + end + end +end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller/application_controller_spec.rb similarity index 78% rename from spec/controllers/application_controller_spec.rb rename to spec/controllers/application_controller/application_controller_spec.rb index 88b659d0..d31c8910 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller/application_controller_spec.rb @@ -5,13 +5,19 @@ RSpec.describe ApplicationController, type: :controller do include ApplicationHelper - controller do + controller(ApplicationController) do def index render plain: 'Hello, world!' end end describe 'before_action :check_date' do + + it 'should be called before every action' do + expect(controller).to receive(:check_date) + get :index + end + context 'when EXPIRATION_DATE is in the past and user is not admin' do it 'redirects to the closed page' do allow(ENV).to receive(:fetch).with('EXPIRATION_DATE').and_return('2000-01-01') @@ -39,7 +45,14 @@ def index context 'when EXPIRATION_DATE is missing' do it 'raises a KeyError' do + # Stub ENV to simulate KeyError allow(ENV).to receive(:fetch).with('EXPIRATION_DATE').and_raise(KeyError) + + # Temporarily modify the behavior of rescue_from + allow(controller).to receive(:render_500) do |error| + raise error + end + expect { get :index }.to raise_error(KeyError) end end diff --git a/spec/controllers/errors_controller_spec.rb b/spec/controllers/errors_controller_spec.rb new file mode 100644 index 00000000..43c4c019 --- /dev/null +++ b/spec/controllers/errors_controller_spec.rb @@ -0,0 +1,48 @@ +# spec/controllers/errors_controller_spec.rb + +require 'rails_helper' + +RSpec.describe ErrorsController, type: :controller do + describe 'GET #not_found' do + before do + get :not_found + end + it 'renders the not_found template' do + expect(response).to render_template('errors/404') + expect(response).to render_template('layouts/application') + end + + it 'returns HTTP status 404' do + expect(response).to have_http_status(404) + end + end + + describe 'GET #unprocessable_entity' do + before do + get :unprocessable_entity + end + + it 'renders the unprocessable_entity template' do + expect(response).to render_template('errors/422') + expect(response).to render_template('layouts/application') + end + + it 'returns HTTP status 422' do + expect(response).to have_http_status(422) + end + end + + describe 'GET #internal_server_error' do + before do + get :internal_server_error + end + it 'renders the internal_server_error template' do + expect(response).to render_template('errors/500') + expect(response).to render_template('layouts/application') + end + + it 'returns HTTP status 500' do + expect(response).to have_http_status(500) + end + end +end diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb new file mode 100644 index 00000000..95ac80d0 --- /dev/null +++ b/spec/routing/admin_routing_spec.rb @@ -0,0 +1,35 @@ +# spec/routes/admin_routes_spec.rb + +require "rails_helper" + +RSpec.describe "Admin Routes", type: :routing do + describe "GET #citations" do + it "routes to admin#citations" do + expect(get: "/citations").to route_to("admin#citations") + end + end + + describe "GET #toggle_links" do + it "routes to admin#toggle_links" do + expect(get: "/toggle_links").to route_to("admin#toggle_links") + end + end + + describe "GET #manage" do + it "routes to admin#login" do + expect(get: "/manage").to route_to("admin#login") + end + end + + describe "POST #manage/validate" do + it "routes to admin#validate" do + expect(post: "/manage/validate").to route_to("admin#validate") + end + end + + describe "GET #csv/:controller_name" do + it "routes to admin#csv" do + expect(get: "/csv/example_controller").to route_to("admin#csv", controller_name: "example_controller") + end + end +end diff --git a/spec/routing/dynamic_pages_routing_spec.rb b/spec/routing/dynamic_pages_routing_spec.rb new file mode 100644 index 00000000..e11913a1 --- /dev/null +++ b/spec/routing/dynamic_pages_routing_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec.describe PagesController, type: :routing do + describe 'routing' do + it 'routes to #show with about as parameter' do + expect(get: '/pages/about').to route_to('pages#show', page: 'about') + end + + it 'routes to #show with contact as parameter' do + expect(get: '/pages/contact').to route_to('pages#show', page: 'contact') + end + + it 'routes to #show with faq as parameter' do + expect(get: '/pages/faq').to route_to('pages#show', page: 'faq') + end + + it 'does not route to #show without a page parameter' do + expect(get: '/pages/').not_to be_routable + end + end +end diff --git a/spec/routing/error_pages_routing_spec.rb b/spec/routing/error_pages_routing_spec.rb new file mode 100644 index 00000000..9b896d45 --- /dev/null +++ b/spec/routing/error_pages_routing_spec.rb @@ -0,0 +1,19 @@ +# spec/routing/error_pages_routing_spec.rb + +require 'rails_helper' + +RSpec.describe 'Error Pages', type: :routing do + %w[get post put patch delete].each do |http_method| + it "routes /404 to errors#not_found for #{http_method.upcase}" do + expect({ "#{http_method}" => '/404' }).to route_to(controller: 'errors', action: 'not_found') + end + + it "routes /500 to errors#internal_server_error for #{http_method.upcase}" do + expect({ "#{http_method}" => '/500' }).to route_to(controller: 'errors', action: 'internal_server_error') + end + + it "routes /422 to errors#unprocessable_entity for #{http_method.upcase}" do + expect({ "#{http_method}" => '/422' }).to route_to(controller: 'errors', action: 'unprocessable_entity') + end + end +end diff --git a/spec/routing/publications_and_submitters_spec.rb b/spec/routing/publications_and_submitters_spec.rb new file mode 100644 index 00000000..59459ae1 --- /dev/null +++ b/spec/routing/publications_and_submitters_spec.rb @@ -0,0 +1,23 @@ +# spec/routes/publications_and_submitters_routes_spec.rb + +require "rails_helper" + +RSpec.describe "Publications and Submitters Routes", type: :routing do + describe "GET #publications" do + it "routes to publications#index" do + expect(get: "/publications").to route_to("publications#index") + end + end + + describe "GET #publications/:id" do + it "routes to publications#index with an ID" do + expect(get: "/publications/1").to route_to("publications#index", id: "1") + end + end + + describe "GET #finished" do + it "routes to submitters#finished" do + expect(get: "/finished").to route_to("submitters#finished") + end + end +end diff --git a/spec/routing/root_routing_spec.rb b/spec/routing/root_routing_spec.rb new file mode 100644 index 00000000..3dc00a6b --- /dev/null +++ b/spec/routing/root_routing_spec.rb @@ -0,0 +1,9 @@ +# spec/routing/root_routing_spec.rb + +require 'rails_helper' + +RSpec.describe 'Root route', type: :routing do + it 'routes the root URL to submitters#new' do + expect(get: '/').to route_to(controller: 'submitters', action: 'new') + end +end From 9f9a6b5b14bf293fe91f7a51bd6494c44ee06fb5 Mon Sep 17 00:00:00 2001 From: Janell-Huyck Date: Wed, 27 Sep 2023 19:10:44 -0400 Subject: [PATCH 2/5] error pages now being displayed dynamically --- app/controllers/application_controller.rb | 19 ----- app/controllers/errors_controller.rb | 3 +- app/controllers/pages_controller.rb | 2 +- config/application.rb | 1 + config/routes.rb | 8 +- ...application_controller_check_date_spec.rb} | 6 -- ...lication_controller_error_handling_spec.rb | 84 ------------------- spec/controllers/errors_controller_spec.rb | 2 + spec/controllers/pages_controller_spec.rb | 8 +- spec/routing/admin_routing_spec.rb | 36 ++++---- spec/routing/dynamic_pages_routing_spec.rb | 2 + spec/routing/error_pages_routing_spec.rb | 8 +- .../publications_and_submitters_spec.rb | 24 +++--- spec/routing/root_routing_spec.rb | 2 + 14 files changed, 56 insertions(+), 149 deletions(-) rename spec/controllers/application_controller/{application_controller_spec.rb => application_controller_check_date_spec.rb} (90%) delete mode 100644 spec/controllers/application_controller/application_controller_error_handling_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ecf6ad94..743fd779 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,27 +2,8 @@ class ApplicationController < ActionController::Base include Pagy::Backend - - rescue_from ActiveRecord::RecordNotFound, with: :render_404 - rescue_from ActiveRecord::RecordInvalid, with: :render_422 - rescue_from StandardError, with: :render_500 - - before_action :check_date - def render_404(exception) - render template: 'errors/404', layout: 'application', status: :not_found - end - - def render_422(exception) - render template: 'errors/422', layout: 'application', status: :unprocessable_entity - end - - def render_500(exception) - logger.error exception - render template: 'errors/500', layout: 'application', status: :internal_server_error - end - private def check_date diff --git a/app/controllers/errors_controller.rb b/app/controllers/errors_controller.rb index 5678bec5..973879e2 100644 --- a/app/controllers/errors_controller.rb +++ b/app/controllers/errors_controller.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + # app/controllers/errors_controller.rb class ErrorsController < ApplicationController def not_found @@ -12,4 +13,4 @@ def unprocessable_entity def internal_server_error render template: 'errors/500', layout: 'application', status: :internal_server_error end -end \ No newline at end of file +end diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 58ac98a3..619cd704 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -9,7 +9,7 @@ def show if valid_page? render template: "pages/#{safe_page}" else - render status: :not_found + render template: 'errors/404', status: :not_found end end diff --git a/config/application.rb b/config/application.rb index 96b2d9ca..702f9bc3 100644 --- a/config/application.rb +++ b/config/application.rb @@ -13,6 +13,7 @@ module Aaec class Application < Rails::Application # Initialize configuration defaults for originally generated Rails version. config.load_defaults 6.1 + config.exceptions_app = routes # Settings in config/environments/* take precedence over those specified here. # Application configuration can go into files in config/initializers diff --git a/config/routes.rb b/config/routes.rb index cd54fe4e..6b52e844 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -25,13 +25,11 @@ post 'manage/validate', to: 'admin#validate' get '/csv/:controller_name', to: 'admin#csv', as: 'controller_name' - # Publications and submission routes get 'publications', to: 'publications#index' get 'publications/:id', to: 'publications#index' get 'finished', to: 'submitters#finished' - # Dynamic pages get '/pages/:page' => 'pages#show' @@ -39,7 +37,7 @@ root 'submitters#new' # Custom Error Pages - match "/404", to: "errors#not_found", via: :all - match "/500", to: "errors#internal_server_error", via: :all - match "/422", to: "errors#unprocessable_entity", via: :all + match '/404', to: 'errors#not_found', via: :all + match '/500', to: 'errors#internal_server_error', via: :all + match '/422', to: 'errors#unprocessable_entity', via: :all end diff --git a/spec/controllers/application_controller/application_controller_spec.rb b/spec/controllers/application_controller/application_controller_check_date_spec.rb similarity index 90% rename from spec/controllers/application_controller/application_controller_spec.rb rename to spec/controllers/application_controller/application_controller_check_date_spec.rb index d31c8910..19357916 100644 --- a/spec/controllers/application_controller/application_controller_spec.rb +++ b/spec/controllers/application_controller/application_controller_check_date_spec.rb @@ -12,7 +12,6 @@ def index end describe 'before_action :check_date' do - it 'should be called before every action' do expect(controller).to receive(:check_date) get :index @@ -47,11 +46,6 @@ def index it 'raises a KeyError' do # Stub ENV to simulate KeyError allow(ENV).to receive(:fetch).with('EXPIRATION_DATE').and_raise(KeyError) - - # Temporarily modify the behavior of rescue_from - allow(controller).to receive(:render_500) do |error| - raise error - end expect { get :index }.to raise_error(KeyError) end diff --git a/spec/controllers/application_controller/application_controller_error_handling_spec.rb b/spec/controllers/application_controller/application_controller_error_handling_spec.rb deleted file mode 100644 index b08e0f88..00000000 --- a/spec/controllers/application_controller/application_controller_error_handling_spec.rb +++ /dev/null @@ -1,84 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe ApplicationController, type: :controller do - include ApplicationHelper - - controller do - - def index - raise ActiveRecord::RecordNotFound - end - - def action_404 - raise ActiveRecord::RecordNotFound - end - - def action_422 - raise ActionController::UnprocessableEntity - end - - def action_500 - raise StandardError, 'This is a standard error' - end - end - - describe 'rescue_from ActiveRecord::RecordNotFound' do - before do - routes.draw { get 'index' => 'anonymous#index', as: :index } - end - - - it 'renders the not_found template' do - puts - puts "response: #{response.inspect}" - puts - expect(controller).to receive(:render_404) - get :index - end - - it 'returns HTTP status 404' do - get :action_404 - expect(response).to have_http_status(404) - end - end - - describe 'rescue_from ActionController::UnprocessableEntity' do - before do - routes.draw { get 'action_422' => 'anonymous#action_422' } - end - - it 'renders the unprocessable_entity template' do - get :action_422 - expect(response).to render_template('errors/422') - end - - it 'returns HTTP status 422' do - get :action_422 - expect(response).to have_http_status(422) - end - end - - describe 'rescue_from StandardError' do - before do - routes.draw { get 'action_500' => 'anonymous#action_500' } - end - - it 'renders the internal_server_error template' do - get :action_500 - expect(response).to render_template('errors/500') - end - - it 'returns HTTP status 500' do - get :action_500 - expect(response).to have_http_status(500) - end - - it 'logs the error' do - allow(controller.logger).to receive(:error) - get :action_500 - expect(controller.logger).to have_received(:error).with(an_instance_of(StandardError).and(having_attributes(message: 'This is a standard error'))) - end - end -end diff --git a/spec/controllers/errors_controller_spec.rb b/spec/controllers/errors_controller_spec.rb index 43c4c019..35939dde 100644 --- a/spec/controllers/errors_controller_spec.rb +++ b/spec/controllers/errors_controller_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # spec/controllers/errors_controller_spec.rb require 'rails_helper' diff --git a/spec/controllers/pages_controller_spec.rb b/spec/controllers/pages_controller_spec.rb index 2b53348d..5b7c2bba 100644 --- a/spec/controllers/pages_controller_spec.rb +++ b/spec/controllers/pages_controller_spec.rb @@ -15,10 +15,14 @@ end context 'when page is invalid' do - it 'with invalid params' do + it 'returns a 404 status' do get :show, params: { page: 'bad' } expect(response.status).to eq(404) - expect(response.body).to have_text('The page you requested cannot be found') + end + + it 'renders the 404 template' do + get :show, params: { page: 'bad' } + expect(response).to render_template('errors/404') end end end diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb index 95ac80d0..c921116e 100644 --- a/spec/routing/admin_routing_spec.rb +++ b/spec/routing/admin_routing_spec.rb @@ -1,35 +1,37 @@ +# frozen_string_literal: true + # spec/routes/admin_routes_spec.rb -require "rails_helper" +require 'rails_helper' -RSpec.describe "Admin Routes", type: :routing do - describe "GET #citations" do - it "routes to admin#citations" do - expect(get: "/citations").to route_to("admin#citations") +RSpec.describe 'Admin Routes', type: :routing do + describe 'GET #citations' do + it 'routes to admin#citations' do + expect(get: '/citations').to route_to('admin#citations') end end - describe "GET #toggle_links" do - it "routes to admin#toggle_links" do - expect(get: "/toggle_links").to route_to("admin#toggle_links") + describe 'GET #toggle_links' do + it 'routes to admin#toggle_links' do + expect(get: '/toggle_links').to route_to('admin#toggle_links') end end - describe "GET #manage" do - it "routes to admin#login" do - expect(get: "/manage").to route_to("admin#login") + describe 'GET #manage' do + it 'routes to admin#login' do + expect(get: '/manage').to route_to('admin#login') end end - describe "POST #manage/validate" do - it "routes to admin#validate" do - expect(post: "/manage/validate").to route_to("admin#validate") + describe 'POST #manage/validate' do + it 'routes to admin#validate' do + expect(post: '/manage/validate').to route_to('admin#validate') end end - describe "GET #csv/:controller_name" do - it "routes to admin#csv" do - expect(get: "/csv/example_controller").to route_to("admin#csv", controller_name: "example_controller") + describe 'GET #csv/:controller_name' do + it 'routes to admin#csv' do + expect(get: '/csv/example_controller').to route_to('admin#csv', controller_name: 'example_controller') end end end diff --git a/spec/routing/dynamic_pages_routing_spec.rb b/spec/routing/dynamic_pages_routing_spec.rb index e11913a1..2f25620d 100644 --- a/spec/routing/dynamic_pages_routing_spec.rb +++ b/spec/routing/dynamic_pages_routing_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rails_helper' RSpec.describe PagesController, type: :routing do diff --git a/spec/routing/error_pages_routing_spec.rb b/spec/routing/error_pages_routing_spec.rb index 9b896d45..1976de1f 100644 --- a/spec/routing/error_pages_routing_spec.rb +++ b/spec/routing/error_pages_routing_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # spec/routing/error_pages_routing_spec.rb require 'rails_helper' @@ -5,15 +7,15 @@ RSpec.describe 'Error Pages', type: :routing do %w[get post put patch delete].each do |http_method| it "routes /404 to errors#not_found for #{http_method.upcase}" do - expect({ "#{http_method}" => '/404' }).to route_to(controller: 'errors', action: 'not_found') + expect({ http_method.to_s => '/404' }).to route_to(controller: 'errors', action: 'not_found') end it "routes /500 to errors#internal_server_error for #{http_method.upcase}" do - expect({ "#{http_method}" => '/500' }).to route_to(controller: 'errors', action: 'internal_server_error') + expect({ http_method.to_s => '/500' }).to route_to(controller: 'errors', action: 'internal_server_error') end it "routes /422 to errors#unprocessable_entity for #{http_method.upcase}" do - expect({ "#{http_method}" => '/422' }).to route_to(controller: 'errors', action: 'unprocessable_entity') + expect({ http_method.to_s => '/422' }).to route_to(controller: 'errors', action: 'unprocessable_entity') end end end diff --git a/spec/routing/publications_and_submitters_spec.rb b/spec/routing/publications_and_submitters_spec.rb index 59459ae1..13f4360a 100644 --- a/spec/routing/publications_and_submitters_spec.rb +++ b/spec/routing/publications_and_submitters_spec.rb @@ -1,23 +1,25 @@ +# frozen_string_literal: true + # spec/routes/publications_and_submitters_routes_spec.rb -require "rails_helper" +require 'rails_helper' -RSpec.describe "Publications and Submitters Routes", type: :routing do - describe "GET #publications" do - it "routes to publications#index" do - expect(get: "/publications").to route_to("publications#index") +RSpec.describe 'Publications and Submitters Routes', type: :routing do + describe 'GET #publications' do + it 'routes to publications#index' do + expect(get: '/publications').to route_to('publications#index') end end - describe "GET #publications/:id" do - it "routes to publications#index with an ID" do - expect(get: "/publications/1").to route_to("publications#index", id: "1") + describe 'GET #publications/:id' do + it 'routes to publications#index with an ID' do + expect(get: '/publications/1').to route_to('publications#index', id: '1') end end - describe "GET #finished" do - it "routes to submitters#finished" do - expect(get: "/finished").to route_to("submitters#finished") + describe 'GET #finished' do + it 'routes to submitters#finished' do + expect(get: '/finished').to route_to('submitters#finished') end end end diff --git a/spec/routing/root_routing_spec.rb b/spec/routing/root_routing_spec.rb index 3dc00a6b..c4a71c46 100644 --- a/spec/routing/root_routing_spec.rb +++ b/spec/routing/root_routing_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # spec/routing/root_routing_spec.rb require 'rails_helper' From 2fda44ce31e13ca2dd0bc298835c4b7504f11e98 Mon Sep 17 00:00:00 2001 From: Janell-Huyck Date: Fri, 29 Sep 2023 10:46:24 -0400 Subject: [PATCH 3/5] removed 422 and 500 pages --- app/controllers/errors_controller.rb | 9 +-- app/views/errors/422.html.erb | 67 ---------------------- app/views/errors/500.html.erb | 51 ---------------- config/deploy/qa.rb | 2 +- config/routes.rb | 4 +- spec/controllers/errors_controller_spec.rb | 29 ---------- spec/routing/error_pages_routing_spec.rb | 5 +- 7 files changed, 7 insertions(+), 160 deletions(-) delete mode 100644 app/views/errors/422.html.erb delete mode 100644 app/views/errors/500.html.erb diff --git a/app/controllers/errors_controller.rb b/app/controllers/errors_controller.rb index 973879e2..9d44aa4b 100644 --- a/app/controllers/errors_controller.rb +++ b/app/controllers/errors_controller.rb @@ -2,15 +2,8 @@ # app/controllers/errors_controller.rb class ErrorsController < ApplicationController + # Per Infosec, all errors should route to the 404 page, not the 422 or 500 pages. def not_found render template: 'errors/404', layout: 'application', status: :not_found end - - def unprocessable_entity - render template: 'errors/422', layout: 'application', status: :unprocessable_entity - end - - def internal_server_error - render template: 'errors/500', layout: 'application', status: :internal_server_error - end end diff --git a/app/views/errors/422.html.erb b/app/views/errors/422.html.erb deleted file mode 100644 index c08eac0d..00000000 --- a/app/views/errors/422.html.erb +++ /dev/null @@ -1,67 +0,0 @@ - - - - The change you wanted was rejected (422) - - - - - - -
-
-

The change you wanted was rejected.

-

Maybe you tried to change something you didn't have access to.

-
-

If you are the application owner check the logs for more information.

-
- - diff --git a/app/views/errors/500.html.erb b/app/views/errors/500.html.erb deleted file mode 100644 index b448fd07..00000000 --- a/app/views/errors/500.html.erb +++ /dev/null @@ -1,51 +0,0 @@ - - - - Artists, Authors, Editors & Composers | UC Libraries - - - - - - - - -

- The application has encountered an error. You may want to try again. -
 
- If you need assistance, contact Melissa Norris
at melissa.norris@uc.edu or (513) 556-1558 -

- - - - diff --git a/config/deploy/qa.rb b/config/deploy/qa.rb index 1e224bbd..0802684a 100644 --- a/config/deploy/qa.rb +++ b/config/deploy/qa.rb @@ -2,7 +2,7 @@ set :rails_env, :production set :bundle_without, %w[development test].join(' ') -set :branch, 'qa' +set :branch, '218/interactive-deploy-check' set :default_env, path: '$PATH:/usr/local/bin' set :bundle_path, -> { shared_path.join('vendor/bundle') } set :rbenv_prefix, "RBENV_ROOT=#{fetch(:rbenv_path)} RBENV_VERSION=#{fetch(:rbenv_ruby)} #{fetch(:rbenv_path)}/bin/rbenv exec" diff --git a/config/routes.rb b/config/routes.rb index 6b52e844..ac53eef3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -38,6 +38,6 @@ # Custom Error Pages match '/404', to: 'errors#not_found', via: :all - match '/500', to: 'errors#internal_server_error', via: :all - match '/422', to: 'errors#unprocessable_entity', via: :all + match '/500', to: 'errors#not_found', via: :all + match '/422', to: 'errors#not_found', via: :all end diff --git a/spec/controllers/errors_controller_spec.rb b/spec/controllers/errors_controller_spec.rb index 35939dde..89e0c9a9 100644 --- a/spec/controllers/errors_controller_spec.rb +++ b/spec/controllers/errors_controller_spec.rb @@ -18,33 +18,4 @@ expect(response).to have_http_status(404) end end - - describe 'GET #unprocessable_entity' do - before do - get :unprocessable_entity - end - - it 'renders the unprocessable_entity template' do - expect(response).to render_template('errors/422') - expect(response).to render_template('layouts/application') - end - - it 'returns HTTP status 422' do - expect(response).to have_http_status(422) - end - end - - describe 'GET #internal_server_error' do - before do - get :internal_server_error - end - it 'renders the internal_server_error template' do - expect(response).to render_template('errors/500') - expect(response).to render_template('layouts/application') - end - - it 'returns HTTP status 500' do - expect(response).to have_http_status(500) - end - end end diff --git a/spec/routing/error_pages_routing_spec.rb b/spec/routing/error_pages_routing_spec.rb index 1976de1f..0d99ffa1 100644 --- a/spec/routing/error_pages_routing_spec.rb +++ b/spec/routing/error_pages_routing_spec.rb @@ -4,6 +4,7 @@ require 'rails_helper' +# per Infosec, all errors should route to the 404 page, not the 422 or 500 pages RSpec.describe 'Error Pages', type: :routing do %w[get post put patch delete].each do |http_method| it "routes /404 to errors#not_found for #{http_method.upcase}" do @@ -11,11 +12,11 @@ end it "routes /500 to errors#internal_server_error for #{http_method.upcase}" do - expect({ http_method.to_s => '/500' }).to route_to(controller: 'errors', action: 'internal_server_error') + expect({ http_method.to_s => '/500' }).to route_to(controller: 'errors', action: 'not_found') end it "routes /422 to errors#unprocessable_entity for #{http_method.upcase}" do - expect({ http_method.to_s => '/422' }).to route_to(controller: 'errors', action: 'unprocessable_entity') + expect({ http_method.to_s => '/422' }).to route_to(controller: 'errors', action: 'not_found') end end end From 7996c3b5c3409f484d0d3cb91992a2796048110e Mon Sep 17 00:00:00 2001 From: Janell-Huyck Date: Fri, 29 Sep 2023 11:27:46 -0400 Subject: [PATCH 4/5] added test for missing coverage in admincontroller so that code coverage doesn't go down --- .../admin_controller_csv_spec.rb | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/spec/controllers/admin_controller/admin_controller_csv_spec.rb b/spec/controllers/admin_controller/admin_controller_csv_spec.rb index 58f91e6f..17ae1679 100644 --- a/spec/controllers/admin_controller/admin_controller_csv_spec.rb +++ b/spec/controllers/admin_controller/admin_controller_csv_spec.rb @@ -3,20 +3,39 @@ require 'rails_helper' RSpec.describe AdminController, type: :controller do + let(:common_params) { { 'controller_name' => 'other_publications' } } + let(:admin_session) { { 'admin' => true } } + describe 'GET #csv' do - it 'returns a csv when admin' do - get(:csv, params: { :format => 'csv', 'controller_name' => 'other_publications' }, session: { 'admin' => true }) - expect(response.status).to eq 200 - end + context 'when the user is an admin' do + it 'returns a 200 status when requesting a CSV format' do + get(:csv, params: common_params.merge({ format: 'csv' }), session: admin_session) + expect(response.status).to eq(200) + end + + it 'redirects when an invalid format is provided' do + get(:csv, params: common_params.merge({ format: 'html' }), session: admin_session) + expect(response).to redirect_to('/publications') + end + + context 'when a StandardError is raised' do + before do + allow(OtherPublication).to receive(:to_csv).and_raise(StandardError) + end - it 'redirects when invalid format and admin' do - get(:csv, params: { :format => 'html', 'controller_name' => 'other_publications' }, session: { 'admin' => true }) - expect(response).to redirect_to('/publications') + it 'redirects with a notice' do + get(:csv, params: common_params.merge({ format: 'csv' }), session: admin_session) + expect(response).to redirect_to('/publications') + expect(flash[:notice]).to eq('Something went wrong while generating the CSV.') + end + end end - it 'redirects when not admin but valid' do - get(:csv, params: { :format => 'csv', 'controller_name' => 'other_publications' }) - expect(response).to redirect_to('/publications') + context 'when the user is not an admin' do + it 'redirects even if a valid format is provided' do + get(:csv, params: common_params.merge({ format: 'csv' })) + expect(response).to redirect_to('/publications') + end end end end From ead70f8d57e05c28982d8c091fb7bf99f09fe41a Mon Sep 17 00:00:00 2001 From: Janell-Huyck Date: Thu, 5 Oct 2023 09:21:30 -0400 Subject: [PATCH 5/5] returned default qa deploy branch back to qa --- config/deploy/qa.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/deploy/qa.rb b/config/deploy/qa.rb index 0802684a..1e224bbd 100644 --- a/config/deploy/qa.rb +++ b/config/deploy/qa.rb @@ -2,7 +2,7 @@ set :rails_env, :production set :bundle_without, %w[development test].join(' ') -set :branch, '218/interactive-deploy-check' +set :branch, 'qa' set :default_env, path: '$PATH:/usr/local/bin' set :bundle_path, -> { shared_path.join('vendor/bundle') } set :rbenv_prefix, "RBENV_ROOT=#{fetch(:rbenv_path)} RBENV_VERSION=#{fetch(:rbenv_ruby)} #{fetch(:rbenv_path)}/bin/rbenv exec"