From 7be7d7d1e74cc8edc1a3106e4691c4883c14079a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 8 May 2021 18:49:58 +0100 Subject: [PATCH] Updated not-found image path handling to have better ux Added test to cover. Started refactoring some of the app error handling in the process of this. Fixes #2696 --- app/Exceptions/Handler.php | 13 ------ app/Exceptions/PrettyException.php | 42 ++++++++++++++++++- .../Controllers/Images/ImageController.php | 6 ++- resources/lang/en/errors.php | 3 ++ resources/views/errors/404.blade.php | 4 +- routes/web.php | 2 +- tests/ErrorTest.php | 7 ++++ 7 files changed, 58 insertions(+), 19 deletions(-) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 196897164..8d3a743fa 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -68,19 +68,6 @@ class Handler extends ExceptionHandler return redirect($e->redirectLocation); } - // Handle pretty exceptions which will show a friendly application-fitting page - // Which will include the basic message to point the user roughly to the cause. - if ($this->isExceptionType($e, PrettyException::class) && !config('app.debug')) { - $message = $this->getOriginalMessage($e); - $code = ($e->getCode() === 0) ? 500 : $e->getCode(); - return response()->view('errors/' . $code, ['message' => $message], $code); - } - - // Handle 404 errors with a loaded session to enable showing user-specific information - if ($this->isExceptionType($e, NotFoundHttpException::class)) { - return \Route::respondWithRoute('fallback'); - } - return parent::render($request, $e); } diff --git a/app/Exceptions/PrettyException.php b/app/Exceptions/PrettyException.php index 7fad7df45..af60c3d06 100644 --- a/app/Exceptions/PrettyException.php +++ b/app/Exceptions/PrettyException.php @@ -1,6 +1,44 @@ getCode() === 0) ? 500 : $this->getCode(); + return response()->view('errors.' . $code, [ + 'message' => $this->getMessage(), + 'subtitle' => $this->subtitle, + 'details' => $this->details, + ], $code); + } + + public function setSubtitle(string $subtitle): self + { + $this->subtitle = $subtitle; + return $this; + } + + public function setDetails(string $details): self + { + $this->details = $details; + return $this; + } } diff --git a/app/Http/Controllers/Images/ImageController.php b/app/Http/Controllers/Images/ImageController.php index ecc36bf67..1eb8917b3 100644 --- a/app/Http/Controllers/Images/ImageController.php +++ b/app/Http/Controllers/Images/ImageController.php @@ -1,6 +1,7 @@ setSubtitle(trans('errors.image_not_found_subtitle')) + ->setDetails(trans('errors.image_not_found_details')); } return response()->file($path); diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index 79024e482..eb8ba54ea 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -83,6 +83,9 @@ return [ '404_page_not_found' => 'Page Not Found', 'sorry_page_not_found' => 'Sorry, The page you were looking for could not be found.', 'sorry_page_not_found_permission_warning' => 'If you expected this page to exist, you might not have permission to view it.', + 'image_not_found' => 'Image Not Found', + 'image_not_found_subtitle' => 'Sorry, The image file you were looking for could not be found.', + 'image_not_found_details' => 'If you expected this image to exist it might have been deleted.', 'return_home' => 'Return to home', 'error_occurred' => 'An Error Occurred', 'app_down' => ':appName is down right now', diff --git a/resources/views/errors/404.blade.php b/resources/views/errors/404.blade.php index 02f97fc54..b3325ba82 100644 --- a/resources/views/errors/404.blade.php +++ b/resources/views/errors/404.blade.php @@ -7,8 +7,8 @@

{{ $message ?? trans('errors.404_page_not_found') }}

-
{{ trans('errors.sorry_page_not_found') }}
-

{{ trans('errors.sorry_page_not_found_permission_warning') }}

+
{{ $subtitle ?? trans('errors.sorry_page_not_found') }}
+

{{ $details ?? trans('errors.sorry_page_not_found_permission_warning') }}

@if(!signedInUser()) diff --git a/routes/web.php b/routes/web.php index 9d482dc41..730c795f8 100644 --- a/routes/web.php +++ b/routes/web.php @@ -254,4 +254,4 @@ Route::post('/password/email', 'Auth\ForgotPasswordController@sendResetLinkEmail Route::get('/password/reset/{token}', 'Auth\ResetPasswordController@showResetForm'); Route::post('/password/reset', 'Auth\ResetPasswordController@reset'); -Route::fallback('HomeController@getNotFound'); \ No newline at end of file +Route::fallback('HomeController@getNotFound')->name('fallback'); \ No newline at end of file diff --git a/tests/ErrorTest.php b/tests/ErrorTest.php index 1558df78d..6b69355fc 100644 --- a/tests/ErrorTest.php +++ b/tests/ErrorTest.php @@ -38,4 +38,11 @@ class ErrorTest extends TestCase $this->assertCount(1, $handler->getRecords()); } + + public function test_access_to_non_existing_image_location_provides_404_response() + { + $resp = $this->actingAs($this->getViewer())->get('/uploads/images/gallery/2021-05/anonexistingimage.png'); + $resp->assertStatus(404); + $resp->assertSeeText('Image Not Found'); + } } \ No newline at end of file