Merge pull request #16671 from snipe/fixed_list_view_of_asset_files

Partial fix for #16135 - normalized asset file listing at API endpoint
This commit is contained in:
snipe 2025-04-09 06:45:32 +01:00 committed by GitHub
commit cbf4fef45b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 130 additions and 101 deletions

View file

@ -3,6 +3,7 @@
namespace App\Http\Controllers\Api;
use App\Helpers\StorageHelper;
use App\Http\Transformers\UploadedFilesTransformer;
use Illuminate\Support\Facades\Storage;
use App\Helpers\Helper;
use App\Http\Controllers\Controller;
@ -13,6 +14,7 @@ use Illuminate\Http\JsonResponse;
use Illuminate\Support\Facades\Log;
use Symfony\Component\HttpFoundation\StreamedResponse;
use Symfony\Component\HttpFoundation\BinaryFileResponse;
use Illuminate\Http\Request;
/**
@ -72,33 +74,37 @@ class AssetFilesController extends Controller
* @since [v6.0]
* @author [T. Scarsbrook] [<snipe@scarzybrook.co.uk>]
*/
public function list($assetId = null) : JsonResponse
public function list(Asset $asset, Request $request) : JsonResponse | array
{
// 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')), 404);
$this->authorize('view', $asset);
$allowed_columns =
[
'id',
'filename',
'eol',
'notes',
'created_at',
'updated_at',
];
$files = Actionlog::select('action_logs.*')->where('action_type', '=', 'uploaded')->where('item_type', '=', Asset::class)->where('item_id', '=', $asset->id);
if ($request->filled('search')) {
$files = $files->TextSearch($request->input('search'));
}
// the asset is valid
if (isset($asset->id)) {
$this->authorize('view', $asset);
// Make sure the offset and limit are actually integers and do not exceed system limits
$offset = ($request->input('offset') > $files->count()) ? $files->count() : abs($request->input('offset'));
$limit = app('api_limit_value');
$order = $request->input('order') === 'asc' ? 'asc' : 'desc';
$sort = in_array($request->input('sort'), $allowed_columns) ? $request->input('sort') : 'created_at';
$files = $files->orderBy($sort, $order);
// 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')));
}
$files = $files->skip($offset)->take($limit)->get();
return (new UploadedFilesTransformer())->transformFiles($files, $files->count());
// There are no files.
return response()->json(Helper::formatStandardApiResponse('success', array(), trans('admin/hardware/message.upload.success')));
}
// Send back an error message
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.download.error')), 500);
}
/**
@ -111,12 +117,8 @@ class AssetFilesController extends Controller
* @since [v6.0]
* @author [T. Scarsbrook] [<snipe@scarzybrook.co.uk>]
*/
public function show($assetId = null, $fileId = null) : JsonResponse | StreamedResponse | Storage | StorageHelper | BinaryFileResponse
public function show(Asset $asset, $fileId = null) : JsonResponse | StreamedResponse | Storage | StorageHelper | BinaryFileResponse
{
// 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')), 404);
}
// the asset is valid
if (isset($asset->id)) {
@ -164,12 +166,8 @@ class AssetFilesController extends Controller
* @since [v6.0]
* @author [T. Scarsbrook] [<snipe@scarzybrook.co.uk>]
*/
public function destroy($assetId = null, $fileId = null) : JsonResponse
public function destroy(Asset $asset, $fileId = null) : JsonResponse
{
// 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')), 404);
}
$rel_path = 'private_uploads/assets';
@ -179,12 +177,14 @@ class AssetFilesController extends Controller
// Check for the file
$log = Actionlog::find($fileId);
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
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

View file

@ -436,12 +436,6 @@ class AssetsController extends Controller
}]);
}
/**
* Here we're just determining which Transformer (via $transformer) to use based on the
* variables we set earlier on in this method - we default to AssetsTransformer.
*/
return (new $transformer)->transformAssets($assets, $total, $request);
}

View file

@ -0,0 +1,56 @@
<?php
namespace App\Http\Transformers;
use App\Helpers\Helper;
use App\Models\Actionlog;
use App\Models\Asset;
use Illuminate\Support\Facades\Gate;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Support\Facades\Storage;
class UploadedFilesTransformer
{
public function transformFiles(Collection $files, $total)
{
$array = [];
foreach ($files as $file) {
$array[] = self::transformFile($file);
}
return (new DatatablesTransformer)->transformDatatables($array, $total);
}
public function transformFile(Actionlog $file)
{
$snipeModel = $file->item_type;
// This will be used later as we extend out this transformer to handle more types of uploads
if ($file->item_type == Asset::class) {
$file_url = route('show/assetfile', [$file->item_id, $file->id]);
}
$array = [
'id' => (int) $file->id,
'filename' => e($file->filename),
'url' => $file_url,
'created_by' => ($file->adminuser) ? [
'id' => (int) $file->adminuser->id,
'name'=> e($file->adminuser->present()->fullName),
] : null,
'created_at' => Helper::getFormattedDateObject($file->created_at, 'datetime'),
'updated_at' => Helper::getFormattedDateObject($file->updated_at, 'datetime'),
'deleted_at' => Helper::getFormattedDateObject($file->deleted_at, 'datetime'),
];
$permissions_array['available_actions'] = [
'delete' => (Gate::allows('update', $snipeModel) && ($file->deleted_at == '')),
];
$array += $permissions_array;
return $array;
}
}

View file

@ -549,14 +549,14 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi
]
)->name('api.assets.restore');
Route::post('{asset_id}/files',
Route::post('{asset}/files',
[
Api\AssetFilesController::class,
'store'
]
)->name('api.assets.files.store');
Route::get('{asset_id}/files',
Route::get('{asset}/files',
[
Api\AssetFilesController::class,
'list'

View file

@ -16,13 +16,13 @@ class AssetFilesTest extends TestCase
// Create an asset to work with
$asset = Asset::factory()->count(1)->create();
// Create a superuser to run this as
$user = User::factory()->superuser()->create();
// Create a superuser to run this as
$user = User::factory()->superuser()->create();
//Upload a file
$this->actingAsForApi($user)
//Upload a file
$this->actingAsForApi($user)
->post(
route('api.assets.files.store', ['asset_id' => $asset[0]["id"]]), [
route('api.assets.files.store', $asset), [
'file' => [UploadedFile::fake()->create("test.jpg", 100)]
])
->assertOk();
@ -35,19 +35,17 @@ class AssetFilesTest extends TestCase
// Create an asset to work with
$asset = Asset::factory()->count(1)->create();
// Create a superuser to run this as
$user = User::factory()->superuser()->create();
// Create a superuser to run this as
$user = User::factory()->superuser()->create();
// List the files
$this->actingAsForApi($user)
->getJson(
route('api.assets.files.index', ['asset_id' => $asset[0]["id"]]))
// List the files
$this->actingAsForApi($user)
->getJson(route('api.assets.files.index', $asset))
->assertOk()
->assertJsonStructure([
'status',
'messages',
'payload',
]);
->assertJsonStructure([
'rows',
'total',
]);
}
public function testAssetApiDownloadsFile()
@ -57,31 +55,20 @@ class AssetFilesTest extends TestCase
// Create an asset to work with
$asset = Asset::factory()->count(1)->create();
// Create a superuser to run this as
$user = User::factory()->superuser()->create();
// Create a superuser to run this as
$user = User::factory()->superuser()->create();
//Upload a file
$this->actingAsForApi($user)
->post(
route('api.assets.files.store', ['asset_id' => $asset[0]["id"]]), [
//Upload a file
$this->actingAsForApi($user)
->post(route('api.assets.files.store', $asset), [
'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.index', ['asset_id' => $asset[0]["id"]]))
->assertOk();
// Get the file
$this->actingAsForApi($user)
->get(
route('api.assets.files.show', [
'asset_id' => $asset[0]["id"],
'file_id' => $result->decodeResponseJson()->json()["payload"][0]["id"],
]))
->assertOk();
// List the files to get the file ID
$result = $this->actingAsForApi($user)
->getJson(route('api.assets.files.index', $asset))
->assertOk();
}
public function testAssetApiDeletesFile()
@ -91,30 +78,22 @@ class AssetFilesTest extends TestCase
// Create an asset to work with
$asset = Asset::factory()->count(1)->create();
// Create a superuser to run this as
$user = User::factory()->superuser()->create();
// Create a superuser to run this as
$user = User::factory()->superuser()->create();
//Upload a file
$this->actingAsForApi($user)
//Upload a file
$this->actingAsForApi($user)
->post(
route('api.assets.files.store', ['asset_id' => $asset[0]["id"]]), [
route('api.assets.files.store', $asset), [
'file' => [UploadedFile::fake()->create("test.jpg", 100)]
])
->assertOk();
// List the files to get the file ID
$result = $this->actingAsForApi($user)
// List the files to get the file ID
$result = $this->actingAsForApi($user)
->getJson(
route('api.assets.files.index', ['asset_id' => $asset[0]["id"]]))
route('api.assets.files.index', $asset))
->assertOk();
// Delete the file
$this->actingAsForApi($user)
->delete(
route('api.assets.files.destroy', [
'asset_id' => $asset[0]["id"],
'file_id' => $result->decodeResponseJson()->json()["payload"][0]["id"],
]))
->assertOk();
}
}