Skip to content

Commit

Permalink
Merge pull request #3015 from DMPRoadmap/bug_561_api_error_should_ret…
Browse files Browse the repository at this point in the history
…urn_json_not_html_page

Issue #561 - Fix for ensuring Json format rendering errors no longer
  • Loading branch information
briri committed Aug 24, 2021
2 parents 2b7cf07 + e770d28 commit 2e747a5
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 6 deletions.
37 changes: 34 additions & 3 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
6 changes: 3 additions & 3 deletions spec/controllers/super_admin/orgs_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 2e747a5

Please sign in to comment.