Commit e55fd3b9 authored by azul's avatar azul

handle 404 ourself rather than with exception app

The exceoption app middleware runs after the cookie handler.
Therefore sessions are not available to it
which results in the login form becoming invalid.

In addition exceptions handled by the exception app will still be logged.
This is cluttering our logs quite a bit.
We do not need traces for everything that cannot be found or accessed.

This commit brings back the render_not_found method in Controllers.

For the Dispatch controllers it dispatches to the exceptions controller
directly rather than using it via the exception app.

Keep the assert_not_found helper with a block
even though it is technically not needed anymore.
Before this used assert_raises.
Since we do not raise anymore this is not needed and we now use assert_response.
Keeping the same style will allow us to roll this back more easliy
and reduces the number of places we need to change.
parent 5e5a2487
......@@ -43,7 +43,7 @@ module Common::Application::RescueErrors
# Use the ExceptionApp with ExceptionsController for these:
# ( this is the default for errors that do not inherit from
# one of the above)
rescue_from ErrorNotFound, with: :raise
rescue_from ErrorNotFound, with: :render_not_found
rescue_from AuthenticationRequired, with: :raise
rescue_from PermissionDenied, with: :raise
rescue_from Pundit::NotAuthorizedError, with: :log_and_permission_denied
......@@ -115,6 +115,24 @@ module Common::Application::RescueErrors
end
end
#
# shows a generic not found page or error message, customized
# by any message in the exception.
#
def render_not_found(exception=nil)
# Claim something was not found when people may not access it
# exception = ErrorNotFound.new(:page.t) if exception.is_a? PermissionDenied
respond_to do |format|
format.html do
@exception = exception || ErrorNotFound.new(:page.t)
render 'exceptions/show', status: 404, layout: (!request.xhr? && 'notice')
end
format.js do
render_error_js(exception)
end
end
end
def log_and_permission_denied(exception)
Rails.logger.info exception
Rails.logger.info "User id: #{current_user.id}"
......
......@@ -86,6 +86,8 @@ module Common::Requests
if params[:code] && @req.recipient != current_user
@req.try.redeem_code!(current_user)
end
rescue ActiveRecord::RecordNotFound => e
render_not_found(e)
end
def request_context
......
......@@ -33,7 +33,11 @@ class ContextPagesController < DispatchController
@page = finder.page
@group = finder.group
@user = finder.user
new_controller controller_name
if @page
new_controller @page.controller
else
controller_for_missing_page
end
end
def finder
......@@ -41,21 +45,12 @@ class ContextPagesController < DispatchController
params[:id]
end
def controller_name
if @page
@page.controller
else
controller_for_missing_page
end
end
def controller_for_missing_page
if create_page?
prepare_params_to_create_page
'page/create'
new_controller 'page/create'
else
prepare_params_for_not_found
'exceptions'
not_found
end
end
......@@ -75,10 +70,6 @@ class ContextPagesController < DispatchController
page: { title: new_title }
end
def prepare_params_for_not_found
request.env['action_dispatch.exception'] = ErrorNotFound.new(:page)
end
def new_title
params[:id].sub(/\+\d*/, '').split('+').join(' ').humanize
end
......
......@@ -13,22 +13,24 @@ class ContextsController < DispatchController
def find_controller
name = params[:id]
controller_for_group(name)
rescue ActiveRecord::RecordNotFound
controller_for_person(name)
controller_for_group(name) ||
controller_for_person(name) ||
not_found
end
def controller_for_group(name)
# we are dealing with a committee!
name.sub!(' ', '+') if name =~ /\ /
@group = Group.where(name: name).first!
@group = Group.where(name: name).first
return unless @group
params[:group_id] = params[:id]
new_controller 'group/home'
end
def controller_for_person(login)
@user = User.where(login: login).first!
@user = User.where(login: login).first
return unless @user
params[:person_id] = params[:id]
new_controller 'person/home'
end
......
......@@ -30,6 +30,15 @@ class DispatchController < ApplicationController
end
end
def not_found
prepare_params_for_not_found
new_controller 'exceptions'
end
def prepare_params_for_not_found
request.env['action_dispatch.exception'] = ErrorNotFound.new(:page)
end
# We want the modification to also apply to the newly instantiated controller.
# So we have to modify the request - not just the Parameters instance.
def modify_params(options = {})
......
......@@ -8,8 +8,8 @@ class Person::BaseController < ApplicationController
def fetch_person
# person might be preloaded by DispatchController
@user ||= User.where(login: (params[:person_id] || params[:id])).first!
raise_not_found unless policy(@user).show?
@user ||= User.where(login: (params[:person_id] || params[:id])).first
render_not_found unless @user && policy(@user).show?
end
def setup_context
......
......@@ -111,7 +111,7 @@ define_navigation do
global_section :people do
label { :people.t }
url controller: 'person/directory'
active { controller?('person/') or (context?(:user) && visible_context?) }
active { controller?('person/directory') || (context?(:user) && visible_context?)}
html partial: '/layouts/global/nav/people_menu'
context_section :no_context do
......@@ -163,7 +163,7 @@ define_navigation do
global_section :group do
label { :groups.t }
url { groups_directory_path }
active { controller?('group/') or (context?(:group) && visible_context?) }
active { controller?('group/directory') || ( context?(:group) && visible_context? ) }
html partial: '/layouts/global/nav/groups_menu'
context_section :directory do
......
......@@ -21,13 +21,8 @@ module FunctionalTestHelper
].freeze
def assert_not_found
if block_given?
assert_raises(*NOT_FOUND_ERRORS) do
yield
end
else
assert_response :not_found
end
yield if block_given?
assert_response :not_found
end
# can pass either a regexp of the flash error string,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment