Merge pull request #16277 from Godmartinz/bulk_delete_asset_bug

Fixes deletion of assigned assets through bulk delete

Author: snipe

app/Http/Controllers/Assets/BulkAssetsController.php

@@ -525,21 +525,31 @@ public function destroy(Request $request) : RedirectResponse
         $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');
 
-        if ($request->filled('ids')) {
-            $assets = Asset::find($request->get('ids'));
-            foreach ($assets as $asset) {
+        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($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) {
                 $asset->delete();
-            } // endforeach
+            }
 
-            return redirect($bulk_back_url)->with('success', trans('admin/hardware/message.delete.success'));
+        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'));
     }
 
     /**

resources/lang/en-US/admin/hardware/message.php

@@ -72,6 +72,7 @@
     '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.',
     ],

tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php

@@ -162,5 +162,28 @@ public function testActionLogCreatedUponBulkRestore()
         );
     }
 
+     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);
+     }
 
 }