From a534b488b27d39279e29f22c96cf5eefdb962030 Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Wed, 19 Feb 2025 10:09:02 -0800 Subject: [PATCH 1/3] prevents deletion of assigned assets in bulk deletion --- .../Assets/BulkAssetsController.php | 21 ++++++++++++------- .../lang/en-US/admin/hardware/message.php | 1 + 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 93f7255c0b..d38350f5b2 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -525,20 +525,25 @@ class BulkAssetsController extends Controller $this->authorize('delete', Asset::class); $bulk_back_url = route('hardware.index'); + if ($request->session()->has('bulk_back_url')) { $bulk_back_url = $request->session()->pull('bulk_back_url'); } + $assetIds = $request->get('ids'); + $assignedAssets = Asset::whereIn('id', $assetIds)->whereNotNull('assigned_to')->get(); - if ($request->filled('ids')) { - $assets = Asset::find($request->get('ids')); - foreach ($assets as $asset) { - $asset->delete(); - } // endforeach - - return redirect($bulk_back_url)->with('success', trans('admin/hardware/message.delete.success')); - // no values given, nothing to update + if($assignedAssets->isNotEmpty()) { + //if assets are checked out, return a list of asset tags that would need to be checked in first. + $assetTags = $assignedAssets->pluck('asset_tag')->implode(', '); + return redirect()->route('hardware.index')->with('error', trans_choice('admin/hardware/message.delete.assigned_to_error', $assignedAssets->count(), ['asset_tag' => $assetTags] )); } + if($assetIds) { + Asset::wherein('id', $assetIds)->delete(); + return redirect($bulk_back_url)->with('success', trans('admin/hardware/message.delete.success')); + } + // no values given, nothing to update + return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.delete.nothing_updated')); } diff --git a/resources/lang/en-US/admin/hardware/message.php b/resources/lang/en-US/admin/hardware/message.php index 222cbc439e..605c1fe230 100644 --- a/resources/lang/en-US/admin/hardware/message.php +++ b/resources/lang/en-US/admin/hardware/message.php @@ -72,6 +72,7 @@ return [ 'delete' => [ 'confirm' => 'Are you sure you wish to delete this asset?', 'error' => 'There was an issue deleting the asset. Please try again.', + 'assigned_to_error' => '{1}Asset Tag: :asset_tag is currently checked out. Check in this device before deletion.|[2,*]Asset Tags: :asset_tag are currently checked out. Check in these devices before deletion.', 'nothing_updated' => 'No assets were selected, so nothing was deleted.', 'success' => 'The asset was deleted successfully.', ], From eaacf29d57ece6b82b3df273265fd2f7d0db9fe1 Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Wed, 19 Feb 2025 15:11:16 -0800 Subject: [PATCH 2/3] fix tests --- .../Assets/BulkAssetsController.php | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index d38350f5b2..6ad5ae20d2 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -529,22 +529,27 @@ class BulkAssetsController extends Controller if ($request->session()->has('bulk_back_url')) { $bulk_back_url = $request->session()->pull('bulk_back_url'); } - $assetIds = $request->get('ids'); + $assetIds = $request->get('ids') ?? []; + + if(empty($assetIds)) { + return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.delete.nothing_updated')); + } + $assignedAssets = Asset::whereIn('id', $assetIds)->whereNotNull('assigned_to')->get(); if($assignedAssets->isNotEmpty()) { //if assets are checked out, return a list of asset tags that would need to be checked in first. $assetTags = $assignedAssets->pluck('asset_tag')->implode(', '); - return redirect()->route('hardware.index')->with('error', trans_choice('admin/hardware/message.delete.assigned_to_error', $assignedAssets->count(), ['asset_tag' => $assetTags] )); + return redirect()->route($bulk_back_url)->with('error', trans_choice('admin/hardware/message.delete.assigned_to_error', $assignedAssets->count(), ['asset_tag' => $assetTags] )); } - if($assetIds) { - Asset::wherein('id', $assetIds)->delete(); - return redirect($bulk_back_url)->with('success', trans('admin/hardware/message.delete.success')); - } + foreach (Asset::wherein('id', $assetIds)->get() as $asset) { + $asset->delete(); + } + + return redirect($bulk_back_url)->with('success', trans('admin/hardware/message.delete.success')); // no values given, nothing to update - return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.delete.nothing_updated')); } /** From a27539155754c78093cc2990b505a38ba6d33bb2 Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Wed, 19 Feb 2025 15:59:57 -0800 Subject: [PATCH 3/3] adds test for deleting assigned asset --- .../Assets/BulkAssetsController.php | 6 ++--- .../Assets/Ui/BulkDeleteAssetsTest.php | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 6ad5ae20d2..14e5463ef9 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -529,18 +529,18 @@ class BulkAssetsController extends Controller if ($request->session()->has('bulk_back_url')) { $bulk_back_url = $request->session()->pull('bulk_back_url'); } - $assetIds = $request->get('ids') ?? []; + $assetIds = $request->get('ids'); if(empty($assetIds)) { return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.delete.nothing_updated')); } $assignedAssets = Asset::whereIn('id', $assetIds)->whereNotNull('assigned_to')->get(); - if($assignedAssets->isNotEmpty()) { + //if assets are checked out, return a list of asset tags that would need to be checked in first. $assetTags = $assignedAssets->pluck('asset_tag')->implode(', '); - return redirect()->route($bulk_back_url)->with('error', trans_choice('admin/hardware/message.delete.assigned_to_error', $assignedAssets->count(), ['asset_tag' => $assetTags] )); + return redirect($bulk_back_url)->with('error', trans_choice('admin/hardware/message.delete.assigned_to_error', $assignedAssets->count(), ['asset_tag' => $assetTags] )); } foreach (Asset::wherein('id', $assetIds)->get() as $asset) { diff --git a/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php b/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php index d1375c5393..38c69f3b99 100644 --- a/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php +++ b/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php @@ -162,5 +162,28 @@ class BulkDeleteAssetsTest extends TestCase ); } + public function testBulkDeleteAssignedAssetTriggersError(){ + $user = User::factory()->viewAssets()->deleteAssets()->editAssets()->create(); + $asset = Asset::factory()->create([ + 'id' => 5, + 'assigned_to' => $user->id, + 'asset_tag' => '12345', + ]); + + $response = $this->actingAs($user) + ->from(route('hardware/bulkedit')) + ->post('/hardware/bulkdelete', [ + 'ids' => [$asset->id], + 'bulk_actions' => 'delete', + ]); + + $this->assertEquals(302, $response->getStatusCode()); + $this->assertEquals(route('hardware.index'), $response->headers->get('Location')); + + + $errorMessage = session('error'); + $expectedMessage = trans_choice('admin/hardware/message.delete.assigned_to_error',1, ['asset_tag' => $asset->asset_tag]); + $this->assertEquals($expectedMessage, $errorMessage); + } }