Determine error view and message based on type

...not based on status code.

To simplify this logic, we now use the same error "type" both when
routes are not found and specific models are not found. One exception is
ours, one is from Laravel, but for the purposes of error handling they
should be treated the same.

Fixes flarum/core#1641.
This commit is contained in:
Franz Liedke 2019-08-14 19:43:36 +02:00
parent 40a8068dea
commit 919ebfcc33
5 changed files with 15 additions and 13 deletions

View File

@ -51,26 +51,29 @@ class ViewRenderer implements Formatter
return new HtmlResponse($view->render(), $error->getStatusCode()); return new HtmlResponse($view->render(), $error->getStatusCode());
} }
const ERRORS_WITH_VIEWS = ['csrf_token_mismatch', 'not_found'];
private function determineView(HandledError $error): string private function determineView(HandledError $error): string
{ {
$view = [ $type = $error->getType();
'route_not_found' => '404',
'csrf_token_mismatch' => '419',
][$error->getType()] ?? 'default';
return "flarum.forum::error.$view"; if (in_array($type, self::ERRORS_WITH_VIEWS)) {
return "flarum.forum::error.$type";
} else {
return 'flarum.forum::error.default';
}
} }
private function getMessage(HandledError $error) private function getMessage(HandledError $error)
{ {
return $this->getTranslationIfExists($error->getStatusCode()) return $this->getTranslationIfExists($error->getType())
?? $this->getTranslationIfExists('unknown') ?? $this->getTranslationIfExists('unknown')
?? 'An error occurred while trying to load this page.'; ?? 'An error occurred while trying to load this page.';
} }
private function getTranslationIfExists(string $errorType) private function getTranslationIfExists(string $errorType)
{ {
$key = "core.views.error.${errorType}_message"; $key = "core.views.error.$errorType";
$translation = $this->translator->trans($key, ['{forum}' => $this->settings->get('forum_title')]); $translation = $this->translator->trans($key, ['{forum}' => $this->settings->get('forum_title')]);
return $translation === $key ? null : $translation; return $translation === $key ? null : $translation;

View File

@ -38,8 +38,7 @@ class ErrorServiceProvider extends AbstractServiceProvider
'permission_denied' => 403, 'permission_denied' => 403,
// 404 Not Found // 404 Not Found
'model_not_found' => 404, 'not_found' => 404,
'route_not_found' => 404,
// 405 Method Not Allowed // 405 Method Not Allowed
'method_not_allowed' => 405, 'method_not_allowed' => 405,
@ -52,7 +51,7 @@ class ErrorServiceProvider extends AbstractServiceProvider
$this->app->singleton('flarum.error.classes', function () { $this->app->singleton('flarum.error.classes', function () {
return [ return [
InvalidParameterException::class => 'invalid_parameter', InvalidParameterException::class => 'invalid_parameter',
ModelNotFoundException::class => 'model_not_found', ModelNotFoundException::class => 'not_found',
]; ];
}); });

View File

@ -18,6 +18,6 @@ class RouteNotFoundException extends Exception implements KnownError
{ {
public function getType(): string public function getType(): string
{ {
return 'route_not_found'; return 'not_found';
} }
} }

View File

@ -6,7 +6,7 @@
</p> </p>
<p> <p>
<a href="javascript:history.back()"> <a href="javascript:history.back()">
{{ $translator->trans('core.views.error.419_return_link') }} {{ $translator->trans('core.views.error.csrf_token_mismatch_return_link') }}
</a> </a>
</p> </p>
@endsection @endsection

View File

@ -6,7 +6,7 @@
</p> </p>
<p> <p>
<a href="{{ $app->url() }}"> <a href="{{ $app->url() }}">
{{ $translator->trans('core.views.error.404_return_link', ['{forum}' => $settings->get('forum_title')]) }} {{ $translator->trans('core.views.error.not_found_return_link', ['{forum}' => $settings->get('forum_title')]) }}
</a> </a>
</p> </p>
@endsection @endsection