Commit 4fdde4a4 authored by azul's avatar azul

cleanup: use raise_not_found consistently

* do not use render_not_found directly

* do not use `raise ErrorNotFound, :file`
* do not rescue from ErrorNotFound yourself.
  We do have an application wide handler for that.
parent e55fd3b9
......@@ -11,7 +11,7 @@ class AssetsController < ApplicationController
type: file.content_type,
disposition: disposition(file)
rescue ActionController::MissingFile
raise_not_found
raise_not_found :file
end
def destroy
......@@ -33,7 +33,7 @@ class AssetsController < ApplicationController
def fetch_asset
@asset = Asset.find(params[:id]).version_or_self(params[:version])
raise ErrorNotFound, :file unless @asset
raise_not_found :file unless @asset
@page = @asset.page
true
end
......@@ -55,7 +55,7 @@ class AssetsController < ApplicationController
def file_to_send
if thumb_name
thumb = @asset.thumbnail(thumb_name)
raise ErrorNotFound, :file unless thumb
raise_not_found :file unless thumb
thumb.generate
thumb
else
......@@ -63,7 +63,7 @@ class AssetsController < ApplicationController
end
rescue Errno::ENOENT => e
Rails.logger.warn "WARNING: Asset not found: #{thumb_name}"
raise ErrorNotFound, :file
raise_not_found :file
end
#
......
......@@ -96,17 +96,17 @@ module Common::Application::AlertMessages
end
#
# We use the default rails error to trigger the ErrorApp middleware.
# This will then in turn call ExceptionsController#show as defined in
# config.exceptions_app
# Use this function with a symbol for the thing you are missing:
# `raise_not_found :file`
#
# Why?
# Because we need to get rid of all Controller state.
# * Instance variables may leak information.
# * Controller functions like setup_navigation may crash.
# It will wrap the thing in an exception.
# `Common::Application::RescueErrors` will `rescue_from` this exception
# with `render_not_found`.
#
# `render_not_found` in turn will use `translate_exception`
# as defined below to find the right translation for the error
# message.
#
# At the same time redirect would alter the url in the users browser.
# Maybe they just typed it wrong. So we better leave it there.
#
def raise_not_found(thing = nil)
raise ErrorNotFound.new(thing)
......
......@@ -119,16 +119,20 @@ module Common::Application::RescueErrors
# shows a generic not found page or error message, customized
# by any message in the exception.
#
# Please do not call this directly but rather use
# `raise_not_found :file`
#
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)
@exception = exception || ErrorNotFound.new(:page)
render 'exceptions/show', status: 404, layout: (!request.xhr? && 'notice')
end
format.js do
render_error_js(exception)
render_error_js exception, status: 404
end
format.any do
render status: 404, body: nil
end
end
end
......
......@@ -87,7 +87,7 @@ module Common::Requests
@req.try.redeem_code!(current_user)
end
rescue ActiveRecord::RecordNotFound => e
render_not_found(e)
raise_not_found
end
def request_context
......
......@@ -17,7 +17,7 @@ module Page::BeforeFilters
#
def default_fetch_data
@page ||= Page.find(params[:page_id] || params[:id])
raise ErrorNotFound, :page unless @page && policy(@page).show?
raise_not_found unless @page && policy(@page).show?
if logged_in?
# grab the current user's participation from memory
......@@ -28,11 +28,6 @@ module Page::BeforeFilters
fetch_data
true
rescue ErrorNotFound => e
@exception = e
render 'exceptions/show',
status: 404,
layout: request.xhr? ? nil : 'notice'
end
def default_setup_options
......
......@@ -9,7 +9,7 @@ class Person::BaseController < ApplicationController
def fetch_person
# person might be preloaded by DispatchController
@user ||= User.where(login: (params[:person_id] || params[:id])).first
render_not_found unless @user && policy(@user).show?
raise_not_found unless @user && policy(@user).show?
end
def setup_context
......
......@@ -7,18 +7,13 @@ class GalleryImageController < Page::BaseController
def show
authorize @page
@showing = @page.try.showings.includes(:asset).find_by_asset_id(params[:id])
raise ErrorNotFound, :file unless @showing.try.asset
raise_not_found :file unless @showing.try.asset
@image = @showing.asset
# position sometimes starts at 0 and sometimes at 1?
@image_index = @page.images.index(@image).next
@image_count = @page.showings.count
@next = @showing.lower_item
@previous = @showing.higher_item
rescue ErrorNotFound => e
@exception = e
render 'exceptions/show',
status: 404,
layout: request.xhr? ? nil : 'notice'
end
# removed an non ajax fallback, azul
......
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