From 8a2ea971e16d2c043320cb3cd2f255bbb33820b8 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Tue, 30 Apr 2024 21:30:45 +0100 Subject: [PATCH 01/19] Add an API assets files controller Based heavily on the Assets assets files controller. Added errors related to to the files management. Added the API endpoints for file upload and show, but only upload is currently tested/works. --- .../Controllers/Api/AssetFilesController.php | 171 ++++++++++++++++++ app/Http/Requests/UploadFileRequest.php | 2 + .../lang/en-GB/admin/hardware/message.php | 7 + routes/api.php | 17 +- 4 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 app/Http/Controllers/Api/AssetFilesController.php diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php new file mode 100644 index 0000000000..f94e8afd83 --- /dev/null +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -0,0 +1,171 @@ +] + */ +class AssetFilesController extends Controller +{ + /** + * Accepts a POST to upload a file to the server. + * + * @param \App\Http\Requests\UploadFileRequest $request + * @param int $assetId + * @return \Illuminate\Http\JsonResponse + * @throws \Illuminate\Auth\Access\AuthorizationException + * @since [v6.0] + * @author [T. Scarsbrook] [] + */ + public function store(UploadFileRequest $request, $assetId = null) + { + if (! $asset = Asset::find($assetId)) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); + } + + $this->authorize('update', $asset); + + \Log::warning($request); + #\Log::warning($request['name']); + #\Log::warning($request['filename']); + #\Log::warning($request['contents']); + if ($request->hasFile('file')) { + \Log::warning("So, I am actually getting in here..."); + if (! Storage::exists('private_uploads/assets')) { + Storage::makeDirectory('private_uploads/assets', 775); + } + + \Log::warning("File is"); + \Log::warning($request->file('file')); + \Log::warning("File ends"); + foreach ($request->file('file') as $file) { + \Log::warning("Handling file "); + \Log::warning($file); + $file_name = $request->handleFile('private_uploads/assets/','hardware-'.$asset->id, $file); + + $asset->logUpload($file_name, e($request->get('notes'))); + } + \Log::warning("Done handling"); + #$request->IamAnOrange(); + + return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.upload.success'))); + } + + return response()->json(Helper::formatStandardApiResponse('error', null, "Bad bananas"), 500); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.upload.nofiles')), 500); + } + + /** + * Check for permissions and display the file. + * + * @param int $assetId + * @param int $fileId + * @return \Illuminate\Http\JsonResponse + * @throws \Illuminate\Auth\Access\AuthorizationException + * @since [v6.0] + * @author [T. Scarsbrook] [] + */ + public function show($assetId = null, $fileId = null) + { + $asset = Asset::find($assetId); + // the asset is valid + if (isset($asset->id)) { + $this->authorize('view', $asset); + + if (! $log = Actionlog::whereNotNull('filename')->where('item_id', $asset->id)->find($fileId)) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.no_match', ['id' => $fileId])), 500); + } + + $file = 'private_uploads/assets/'.$log->filename; + \Log::debug('Checking for '.$file); + + if ($log->action_type == 'audit') { + $file = 'private_uploads/audits/'.$log->filename; + } + + if (! Storage::exists($file)) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.does_not_exist', ['id' => $fileId])), 404); + } + + if (request('inline') == 'true') { + + $headers = [ + 'Content-Disposition' => 'inline', + ]; + + return Storage::download($file, $log->filename, $headers); + return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.upload.success'))); + } + + return StorageHelper::downloader($file); + } + + // Send back an error message + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.error', ['id' => $fileId])), 500); + } + + /** + * Delete the associated file + * + * @param int $assetId + * @param int $fileId + * @return \Illuminate\Http\JsonResponse + * @throws \Illuminate\Auth\Access\AuthorizationException + * @since [v6.0] + * @author [T. Scarsbrook] [] + */ + public function destroy($assetId = null, $fileId = null) + { + $asset = Asset::find($assetId); + $this->authorize('update', $asset); + $rel_path = 'private_uploads/assets'; + + // the asset is valid + if (isset($asset->id)) { + $this->authorize('update', $asset); + $log = Actionlog::find($fileId); + if ($log) { + if (Storage::exists($rel_path.'/'.$log->filename)) { + Storage::delete($rel_path.'/'.$log->filename); + } + $log->delete(); + + return redirect()->back()->with('success', trans('admin/hardware/message.deletefile.success')); + } + + return redirect()->back()->with('success', trans('admin/hardware/message.deletefile.success')); + } + + // Redirect to the hardware management page + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.deletefile.error')), 500); + } +} diff --git a/app/Http/Requests/UploadFileRequest.php b/app/Http/Requests/UploadFileRequest.php index d91d0bf0b8..4762e52b75 100644 --- a/app/Http/Requests/UploadFileRequest.php +++ b/app/Http/Requests/UploadFileRequest.php @@ -2,12 +2,14 @@ namespace App\Http\Requests; +use App\Http\Traits\ConvertsBase64ToFiles; use enshrined\svgSanitize\Sanitizer; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Facades\Log; class UploadFileRequest extends Request { + use ConvertsBase64ToFiles; /** * Determine if the user is authorized to make this request. * diff --git a/resources/lang/en-GB/admin/hardware/message.php b/resources/lang/en-GB/admin/hardware/message.php index 3af3dbc6e3..7185cd8dcf 100644 --- a/resources/lang/en-GB/admin/hardware/message.php +++ b/resources/lang/en-GB/admin/hardware/message.php @@ -51,6 +51,13 @@ return [ 'invalidfiles' => 'One or more of your files is too large or is a filetype that is not allowed. Allowed filetypes are png, gif, jpg, doc, docx, pdf, and txt.', ], + 'download' => [ + 'error' => 'File(s) not downloaded. Please try again.', + 'success' => 'File(s) successfully downloaded.', + 'does_not_exist' => 'No file exists', + 'no_match' => 'No matching record for that asset/file', + ], + 'import' => [ 'error' => 'Some items did not import correctly.', 'errorDetail' => 'The following Items were not imported because of errors.', diff --git a/routes/api.php b/routes/api.php index 8adb0af619..0d77aa6b9b 100644 --- a/routes/api.php +++ b/routes/api.php @@ -544,13 +544,22 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi 'restore' ] )->name('api.assets.restore'); + Route::post('{asset_id}/files', + [ + Api\AssetFilesController::class, + 'store' + ] + )->name('api.assets.files'); + + Route::get('{asset_id}/files', + [ + Api\AssetFilesController::class, + 'show' + ] + )->name('api.assets.files'); }); - - - - Route::resource('hardware', Api\AssetsController::class, ['names' => [ From 92670d57119913335e132d384c6ca780e01fb04d Mon Sep 17 00:00:00 2001 From: Scarzy Date: Tue, 30 Apr 2024 22:30:51 +0100 Subject: [PATCH 02/19] Add the ability to list files for an asset --- .../Controllers/Api/AssetFilesController.php | 45 ++++++++++++++----- routes/api.php | 2 +- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index f94e8afd83..6198b42bc9 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -12,6 +12,7 @@ use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Models\Asset; use App\Models\AssetModel; +use App\Models\Actionlog; use \Illuminate\Support\Facades\Auth; use Carbon\Carbon; use DB; @@ -54,28 +55,16 @@ class AssetFilesController extends Controller $this->authorize('update', $asset); - \Log::warning($request); - #\Log::warning($request['name']); - #\Log::warning($request['filename']); - #\Log::warning($request['contents']); if ($request->hasFile('file')) { - \Log::warning("So, I am actually getting in here..."); if (! Storage::exists('private_uploads/assets')) { Storage::makeDirectory('private_uploads/assets', 775); } - \Log::warning("File is"); - \Log::warning($request->file('file')); - \Log::warning("File ends"); foreach ($request->file('file') as $file) { - \Log::warning("Handling file "); - \Log::warning($file); $file_name = $request->handleFile('private_uploads/assets/','hardware-'.$asset->id, $file); $asset->logUpload($file_name, e($request->get('notes'))); } - \Log::warning("Done handling"); - #$request->IamAnOrange(); return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.upload.success'))); } @@ -84,6 +73,38 @@ class AssetFilesController extends Controller return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.upload.nofiles')), 500); } + /** + * List the files for an asset. + * + * @param int $assetId + * @return \Illuminate\Http\JsonResponse + * @throws \Illuminate\Auth\Access\AuthorizationException + * @since [v6.0] + * @author [T. Scarsbrook] [] + */ + public function list($assetId = null) + { + $asset = Asset::find($assetId); + + // the asset is valid + if (isset($asset->id)) { + $this->authorize('view', $asset); + + if ($asset->uploads->count() > 0) { + $files = array(); + foreach ($asset->uploads as $upload) { + array_push($files, $upload); + } + return response()->json(Helper::formatStandardApiResponse('success', $files, trans('admin/hardware/message.upload.success'))); + } + + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.no_match', ['id' => $fileId])), 500); + } + + // Send back an error message + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.error', ['id' => $fileId])), 500); + } + /** * Check for permissions and display the file. * diff --git a/routes/api.php b/routes/api.php index 0d77aa6b9b..a4486dfc6b 100644 --- a/routes/api.php +++ b/routes/api.php @@ -554,7 +554,7 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi Route::get('{asset_id}/files', [ Api\AssetFilesController::class, - 'show' + 'list' ] )->name('api.assets.files'); From 194853d860034e9706f9d08361e5d537f7c5913d Mon Sep 17 00:00:00 2001 From: Scarzy Date: Tue, 30 Apr 2024 23:12:28 +0100 Subject: [PATCH 03/19] Remove a redundant line --- app/Http/Controllers/Api/AssetFilesController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index 6198b42bc9..406848f7f0 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -144,7 +144,6 @@ class AssetFilesController extends Controller ]; return Storage::download($file, $log->filename, $headers); - return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.upload.success'))); } return StorageHelper::downloader($file); From bbb914574491819eb7f62f4f9a0941c906c7dd36 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Tue, 30 Apr 2024 23:12:39 +0100 Subject: [PATCH 04/19] Add an API endpoint to download files --- routes/api.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/routes/api.php b/routes/api.php index a4486dfc6b..bb84f96aeb 100644 --- a/routes/api.php +++ b/routes/api.php @@ -558,6 +558,13 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi ] )->name('api.assets.files'); + Route::get('{asset_id}/file/{file_id}', + [ + Api\AssetFilesController::class, + 'show' + ] + )->name('api.assets.files'); + }); Route::resource('hardware', From f6a1a7609555865851df0edd20b312c1875952f8 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Wed, 1 May 2024 18:46:23 +0100 Subject: [PATCH 05/19] Add an endpoint for deleting a file --- routes/api.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/routes/api.php b/routes/api.php index bb84f96aeb..181c84c1b5 100644 --- a/routes/api.php +++ b/routes/api.php @@ -565,6 +565,13 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi ] )->name('api.assets.files'); + Route::delete('{asset_id}/file/{file_id}', + [ + Api\AssetFilesController::class, + 'destroy' + ] + )->name('api.assets.files'); + }); Route::resource('hardware', From f5791c79a59c1e3cffb07828c864efb014ecf5e0 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Wed, 1 May 2024 18:46:39 +0100 Subject: [PATCH 06/19] Change the returns to be API appropriate --- app/Http/Controllers/Api/AssetFilesController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index 406848f7f0..8c867fe756 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -179,10 +179,10 @@ class AssetFilesController extends Controller } $log->delete(); - return redirect()->back()->with('success', trans('admin/hardware/message.deletefile.success')); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.deletefile.success')), 200); } - return redirect()->back()->with('success', trans('admin/hardware/message.deletefile.success')); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.deletefile.success')), 200); } // Redirect to the hardware management page From 516f766a4443b732e15be73c4e6626b123ed9d90 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Wed, 1 May 2024 18:54:53 +0100 Subject: [PATCH 07/19] Remove some debug code --- app/Http/Controllers/Api/AssetFilesController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index 8c867fe756..6bcac9f679 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -69,7 +69,6 @@ class AssetFilesController extends Controller return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.upload.success'))); } - return response()->json(Helper::formatStandardApiResponse('error', null, "Bad bananas"), 500); return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.upload.nofiles')), 500); } From e817b20840669bd7d3cae84ba91532883ae51cdf Mon Sep 17 00:00:00 2001 From: Scarzy Date: Wed, 1 May 2024 18:56:49 +0100 Subject: [PATCH 08/19] Fix some responses to be more appropriate Error/success was mixed up --- app/Http/Controllers/Api/AssetFilesController.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index 6bcac9f679..cc2f339d3c 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -178,13 +178,12 @@ class AssetFilesController extends Controller } $log->delete(); - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.deletefile.success')), 200); + return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.deletefile.success')), 200); } - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.deletefile.success')), 200); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.deletefile.error')), 500); } - // Redirect to the hardware management page return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.deletefile.error')), 500); } } From f11ea7940687977e4d92259141306ada98a246ee Mon Sep 17 00:00:00 2001 From: Scarzy Date: Wed, 1 May 2024 19:01:33 +0100 Subject: [PATCH 09/19] Add some sanity checks that the asset actually exists --- app/Http/Controllers/Api/AssetFilesController.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index cc2f339d3c..dcacd39535 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -83,7 +83,9 @@ class AssetFilesController extends Controller */ public function list($assetId = null) { - $asset = Asset::find($assetId); + if (! $asset = Asset::find($assetId)) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); + } // the asset is valid if (isset($asset->id)) { @@ -116,7 +118,10 @@ class AssetFilesController extends Controller */ public function show($assetId = null, $fileId = null) { - $asset = Asset::find($assetId); + if (! $asset = Asset::find($assetId)) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); + } + // the asset is valid if (isset($asset->id)) { $this->authorize('view', $asset); @@ -164,8 +169,10 @@ class AssetFilesController extends Controller */ public function destroy($assetId = null, $fileId = null) { - $asset = Asset::find($assetId); - $this->authorize('update', $asset); + if (! $asset = Asset::find($assetId)) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); + } + $rel_path = 'private_uploads/assets'; // the asset is valid From bb0a614c390666197f37cc197839bdc369202eee Mon Sep 17 00:00:00 2001 From: Scarzy Date: Tue, 7 May 2024 20:58:30 +0100 Subject: [PATCH 10/19] Update some comments --- .../Controllers/Api/AssetFilesController.php | 41 +++++++++++++++---- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index dcacd39535..53a68f7244 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -29,11 +29,13 @@ use Route; /** - * This class controls all actions related to assets for - * the Snipe-IT Asset Management application. + * This class controls file related actions related + * to assets for the Snipe-IT Asset Management application. + * + * Based on the Assets/AssetFilesController by A. Gianotto * * @version v1.0 - * @author [A. Gianotto] [] + * @author [T. Scarsbrook] [] */ class AssetFilesController extends Controller { @@ -49,26 +51,32 @@ class AssetFilesController extends Controller */ public function store(UploadFileRequest $request, $assetId = null) { + // Start by checking if the asset being acted upon exists if (! $asset = Asset::find($assetId)) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); } + // Make sure we are allowed to update this asset $this->authorize('update', $asset); - if ($request->hasFile('file')) { + if ($request->hasFile('file')) { + // If the file storage directory doesn't exist; create it if (! Storage::exists('private_uploads/assets')) { Storage::makeDirectory('private_uploads/assets', 775); } + // Loop over the attached files and add them to the asset foreach ($request->file('file') as $file) { $file_name = $request->handleFile('private_uploads/assets/','hardware-'.$asset->id, $file); $asset->logUpload($file_name, e($request->get('notes'))); } + // All done - report success return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.upload.success'))); } + // We only reach here if no files were included in the POST, so tell the user this return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.upload.nofiles')), 500); } @@ -83,6 +91,7 @@ class AssetFilesController extends Controller */ public function list($assetId = null) { + // Start by checking if the asset being acted upon exists if (! $asset = Asset::find($assetId)) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); } @@ -91,18 +100,21 @@ class AssetFilesController extends Controller if (isset($asset->id)) { $this->authorize('view', $asset); + // Check that there are some uploads on this asset that can be listed if ($asset->uploads->count() > 0) { $files = array(); foreach ($asset->uploads as $upload) { array_push($files, $upload); - } + } + // Give the list of files back to the user return response()->json(Helper::formatStandardApiResponse('success', $files, trans('admin/hardware/message.upload.success'))); } - + + // There are no files. This possibly isn't the best response for this, but it does get the point across return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.no_match', ['id' => $fileId])), 500); } - // Send back an error message + // return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.error', ['id' => $fileId])), 500); } @@ -118,6 +130,7 @@ class AssetFilesController extends Controller */ public function show($assetId = null, $fileId = null) { + // Start by checking if the asset being acted upon exists if (! $asset = Asset::find($assetId)) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); } @@ -126,10 +139,12 @@ class AssetFilesController extends Controller if (isset($asset->id)) { $this->authorize('view', $asset); + // Check that the file being requested exists for the asset if (! $log = Actionlog::whereNotNull('filename')->where('item_id', $asset->id)->find($fileId)) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.no_match', ['id' => $fileId])), 500); } + // Form the full filename with path $file = 'private_uploads/assets/'.$log->filename; \Log::debug('Checking for '.$file); @@ -137,6 +152,7 @@ class AssetFilesController extends Controller $file = 'private_uploads/audits/'.$log->filename; } + // Check the file actually exists on the filesystem if (! Storage::exists($file)) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.does_not_exist', ['id' => $fileId])), 404); } @@ -169,6 +185,7 @@ class AssetFilesController extends Controller */ public function destroy($assetId = null, $fileId = null) { + // Start by checking if the asset being acted upon exists if (! $asset = Asset::find($assetId)) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); } @@ -178,16 +195,22 @@ class AssetFilesController extends Controller // the asset is valid if (isset($asset->id)) { $this->authorize('update', $asset); + + // Check for the file $log = Actionlog::find($fileId); - if ($log) { + if ($log) { + // Check the file actually exists, and delete it if (Storage::exists($rel_path.'/'.$log->filename)) { Storage::delete($rel_path.'/'.$log->filename); - } + } + // Delete the record of the file $log->delete(); + // All deleting done - notify the user of success return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.deletefile.success')), 200); } + // The file doesn't seem to really exist, so report an error return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.deletefile.error')), 500); } From 73a87a8ea86ad572ddddf9f54d7c0f9de82358e2 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Sat, 18 May 2024 13:04:34 +0100 Subject: [PATCH 11/19] Add the framework of a test --- tests/Feature/Api/AssetFilesTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 tests/Feature/Api/AssetFilesTest.php diff --git a/tests/Feature/Api/AssetFilesTest.php b/tests/Feature/Api/AssetFilesTest.php new file mode 100644 index 0000000000..8afda7916a --- /dev/null +++ b/tests/Feature/Api/AssetFilesTest.php @@ -0,0 +1,13 @@ + Date: Sun, 26 May 2024 16:47:30 +0100 Subject: [PATCH 12/19] Fix the test file location --- tests/Feature/Api/{ => Assets}/AssetFilesTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/Feature/Api/{ => Assets}/AssetFilesTest.php (78%) diff --git a/tests/Feature/Api/AssetFilesTest.php b/tests/Feature/Api/Assets/AssetFilesTest.php similarity index 78% rename from tests/Feature/Api/AssetFilesTest.php rename to tests/Feature/Api/Assets/AssetFilesTest.php index 8afda7916a..2f69c31885 100644 --- a/tests/Feature/Api/AssetFilesTest.php +++ b/tests/Feature/Api/Assets/AssetFilesTest.php @@ -1,6 +1,6 @@ Date: Sun, 26 May 2024 19:00:22 +0100 Subject: [PATCH 13/19] Fix some routes Names of some of the routes overlapped with others in an incompatible way. This makes the names more distinct --- routes/api.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routes/api.php b/routes/api.php index 181c84c1b5..1f43af7431 100644 --- a/routes/api.php +++ b/routes/api.php @@ -563,14 +563,14 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi Api\AssetFilesController::class, 'show' ] - )->name('api.assets.files'); + )->name('api.assets.file'); Route::delete('{asset_id}/file/{file_id}', [ Api\AssetFilesController::class, 'destroy' ] - )->name('api.assets.files'); + )->name('api.assets.file'); }); From 45329912e6ce2c56cee4dd54529c94200b97bc40 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Sun, 26 May 2024 19:01:24 +0100 Subject: [PATCH 14/19] Give a better response to listing no files on an asset HTTP500 was never a good choice. Now it sends back an empty array --- app/Http/Controllers/Api/AssetFilesController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index 53a68f7244..2c4adfd0e7 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -110,8 +110,8 @@ class AssetFilesController extends Controller return response()->json(Helper::formatStandardApiResponse('success', $files, trans('admin/hardware/message.upload.success'))); } - // There are no files. This possibly isn't the best response for this, but it does get the point across - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.no_match', ['id' => $fileId])), 500); + // There are no files. + return response()->json(Helper::formatStandardApiResponse('success', array(), trans('admin/hardware/message.upload.success'))); } // From 2d4af61e6c2aae011fd549d8668d90833de6faac Mon Sep 17 00:00:00 2001 From: Scarzy Date: Mon, 27 May 2024 11:38:30 +0100 Subject: [PATCH 15/19] Begin to add some tests There is currently only really a test for listing, and only for the empty list response --- tests/Feature/Api/Assets/AssetFilesTest.php | 46 ++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Api/Assets/AssetFilesTest.php b/tests/Feature/Api/Assets/AssetFilesTest.php index 2f69c31885..af00619287 100644 --- a/tests/Feature/Api/Assets/AssetFilesTest.php +++ b/tests/Feature/Api/Assets/AssetFilesTest.php @@ -3,11 +3,55 @@ namespace Tests\Feature\Api\Assets; use Tests\TestCase; +use App\Models\User; +use App\Models\Asset; class AssetFilesTest extends TestCase { - public function testExample() + public function testAssetApiAcceptsFileUpload() { + // Upload a file to an asset + + // Create an asset to work with + $asset = Asset::factory()->count(1)->create(); // + //// Upload a file + //// Create a superuser to run this as + //$this->actingAsForApi(User::factory()->superuser()->create()) + // ->postJson( + // route('api.asset.files', $asset), [ + // 'file[]' => + } + + public function testAssetApiListsFiles() + { + // List all files on an asset + + // Create an asset to work with + $asset = Asset::factory()->count(1)->create(); + + print($asset); + + // Create a superuser to run this as + $user = User::factory()->superuser()->create(); + $this->actingAsForApi($user) + ->getJson( + route('api.assets.files', ['asset_id' => $asset[0]["id"]])) + ->assertOk() + ->assertJsonStructure([ + 'status', + 'messages', + 'payload', + ]); + } + + public function testAssetApiDownloadsFile() + { + // Download a file from an asset + } + + public function testAssetApiDeletesFile() + { + // Delete a file from an asset } } From 633bcbb6c4217084644013130e1eac6640d8e9e9 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Mon, 27 May 2024 12:50:20 +0100 Subject: [PATCH 16/19] Get the file upload test working Added the upload functionality to the get and delete tests, but I currently don't seem to be able to reference the correct file ID --- tests/Feature/Api/Assets/AssetFilesTest.php | 66 ++++++++++++++++++--- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetFilesTest.php b/tests/Feature/Api/Assets/AssetFilesTest.php index af00619287..e799ba1a31 100644 --- a/tests/Feature/Api/Assets/AssetFilesTest.php +++ b/tests/Feature/Api/Assets/AssetFilesTest.php @@ -5,6 +5,7 @@ namespace Tests\Feature\Api\Assets; use Tests\TestCase; use App\Models\User; use App\Models\Asset; +use Illuminate\Http\UploadedFile; class AssetFilesTest extends TestCase { @@ -14,13 +15,17 @@ class AssetFilesTest extends TestCase // Create an asset to work with $asset = Asset::factory()->count(1)->create(); - // - //// Upload a file - //// Create a superuser to run this as - //$this->actingAsForApi(User::factory()->superuser()->create()) - // ->postJson( - // route('api.asset.files', $asset), [ - // 'file[]' => + + // Create a superuser to run this as + $user = User::factory()->superuser()->create(); + + //Upload a file + $this->actingAsForApi($user) + ->post( + route('api.assets.files', ['asset_id' => $asset[0]["id"]]), [ + 'file' => [UploadedFile::fake()->create("test.jpg", 100)] + ]) + ->assertOk(); } public function testAssetApiListsFiles() @@ -30,8 +35,6 @@ class AssetFilesTest extends TestCase // Create an asset to work with $asset = Asset::factory()->count(1)->create(); - print($asset); - // Create a superuser to run this as $user = User::factory()->superuser()->create(); $this->actingAsForApi($user) @@ -48,10 +51,55 @@ class AssetFilesTest extends TestCase public function testAssetApiDownloadsFile() { // Download a file from an asset + + // Create an asset to work with + $asset = Asset::factory()->count(1)->create(); + + // Create a superuser to run this as + $user = User::factory()->superuser()->create(); + + //Upload a file + $this->actingAsForApi($user) + ->post( + route('api.assets.files', ['asset_id' => $asset[0]["id"]]), [ + 'file' => [UploadedFile::fake()->create("test.jpg", 100)] + ])->assertOk(); + + // Get the file + $this->actingAsForApi($user) + ->get( + route('api.assets.file', [ + 'asset_id' => $asset[0]["id"], + 'file_id' => 1, + ])) + ->assertOk(); } public function testAssetApiDeletesFile() { // Delete a file from an asset + + // Create an asset to work with + $asset = Asset::factory()->count(1)->create(); + + // Create a superuser to run this as + $user = User::factory()->superuser()->create(); + + //Upload a file + $this->actingAsForApi($user) + ->post( + route('api.assets.files', ['asset_id' => $asset[0]["id"]]), [ + 'file' => [UploadedFile::fake()->create("test.jpg", 100)] + ]) + ->assertOk(); + + // Delete the file + $this->actingAsForApi($user) + ->delete( + route('api.assets.file', [ + 'asset_id' => $asset[0]["id"], + 'file_id' => 1, + ])) + ->assertOk(); } } From ca9d4e31552b7f3a07543e3cfc6f4df0e2fcc5f8 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Mon, 27 May 2024 13:08:18 +0100 Subject: [PATCH 17/19] Get the tests for download and delete working --- tests/Feature/Api/Assets/AssetFilesTest.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetFilesTest.php b/tests/Feature/Api/Assets/AssetFilesTest.php index e799ba1a31..2c19d5dc77 100644 --- a/tests/Feature/Api/Assets/AssetFilesTest.php +++ b/tests/Feature/Api/Assets/AssetFilesTest.php @@ -37,6 +37,8 @@ class AssetFilesTest extends TestCase // Create a superuser to run this as $user = User::factory()->superuser()->create(); + + // List the files $this->actingAsForApi($user) ->getJson( route('api.assets.files', ['asset_id' => $asset[0]["id"]])) @@ -63,14 +65,21 @@ class AssetFilesTest extends TestCase ->post( route('api.assets.files', ['asset_id' => $asset[0]["id"]]), [ 'file' => [UploadedFile::fake()->create("test.jpg", 100)] - ])->assertOk(); + ]) + ->assertOk(); + + // List the files to get the file ID + $result = $this->actingAsForApi($user) + ->getJson( + route('api.assets.files', ['asset_id' => $asset[0]["id"]])) + ->assertOk(); // Get the file $this->actingAsForApi($user) ->get( route('api.assets.file', [ 'asset_id' => $asset[0]["id"], - 'file_id' => 1, + 'file_id' => $result->decodeResponseJson()->json()["payload"][0]["id"], ])) ->assertOk(); } @@ -93,12 +102,18 @@ class AssetFilesTest extends TestCase ]) ->assertOk(); + // List the files to get the file ID + $result = $this->actingAsForApi($user) + ->getJson( + route('api.assets.files', ['asset_id' => $asset[0]["id"]])) + ->assertOk(); + // Delete the file $this->actingAsForApi($user) ->delete( route('api.assets.file', [ 'asset_id' => $asset[0]["id"], - 'file_id' => 1, + 'file_id' => $result->decodeResponseJson()->json()["payload"][0]["id"], ])) ->assertOk(); } From 98a94dec29b17f804147f9028e1dda250aa43b45 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Wed, 29 May 2024 22:17:36 +0100 Subject: [PATCH 18/19] Change some errors to be 404 The asset or file was not found, so 500 wasn't the best choice of error code --- app/Http/Controllers/Api/AssetFilesController.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index 2c4adfd0e7..c56e0a7a8c 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -53,7 +53,7 @@ class AssetFilesController extends Controller { // Start by checking if the asset being acted upon exists if (! $asset = Asset::find($assetId)) { - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 404); } // Make sure we are allowed to update this asset @@ -93,7 +93,7 @@ class AssetFilesController extends Controller { // Start by checking if the asset being acted upon exists if (! $asset = Asset::find($assetId)) { - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 404); } // the asset is valid @@ -132,7 +132,7 @@ class AssetFilesController extends Controller { // Start by checking if the asset being acted upon exists if (! $asset = Asset::find($assetId)) { - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 404); } // the asset is valid @@ -141,7 +141,7 @@ class AssetFilesController extends Controller // Check that the file being requested exists for the asset if (! $log = Actionlog::whereNotNull('filename')->where('item_id', $asset->id)->find($fileId)) { - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.no_match', ['id' => $fileId])), 500); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.no_match', ['id' => $fileId])), 404); } // Form the full filename with path @@ -187,7 +187,7 @@ class AssetFilesController extends Controller { // Start by checking if the asset being acted upon exists if (! $asset = Asset::find($assetId)) { - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 500); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 404); } $rel_path = 'private_uploads/assets'; From f48c5ee25242a32abe9379479cc304ca16de1f31 Mon Sep 17 00:00:00 2001 From: Scarzy Date: Wed, 29 May 2024 22:32:45 +0100 Subject: [PATCH 19/19] Fix an error message --- app/Http/Controllers/Api/AssetFilesController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Api/AssetFilesController.php b/app/Http/Controllers/Api/AssetFilesController.php index c56e0a7a8c..d8bc5e3841 100644 --- a/app/Http/Controllers/Api/AssetFilesController.php +++ b/app/Http/Controllers/Api/AssetFilesController.php @@ -114,8 +114,8 @@ class AssetFilesController extends Controller return response()->json(Helper::formatStandardApiResponse('success', array(), trans('admin/hardware/message.upload.success'))); } - // - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.error', ['id' => $fileId])), 500); + // Send back an error message + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.error')), 500); } /**