From 5c08f3a27eb3e9c420a9eebec899181e884f383c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 18 Sep 2025 17:14:33 -0700 Subject: [PATCH 1/4] Add failing test --- .../Checkouts/Ui/BulkAssetCheckoutTest.php | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php b/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php index f05c4389e6..0791b3b422 100644 --- a/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php +++ b/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php @@ -89,4 +89,32 @@ class BulkAssetCheckoutTest extends TestCase $this->fail('Asset checkout email was sent when the entire checkout failed.'); } } + + public function test_prevents_checkouts_checked_out_items() + { + $asset = Asset::factory()->create(); + $checkedOutAsset = Asset::factory()->assignedToUser()->create(); + $existingUserId = $checkedOutAsset->assigned_to; + + $target = User::factory()->create(); + + $response = $this->actingAs(User::factory()->superuser()->create()) + ->post(route('hardware.bulkcheckout.store'), [ + 'selected_assets' => [ + $asset->id, + $checkedOutAsset->id, + ], + 'checkout_to_type' => 'user', + 'assigned_user' => $target->id, + ]); + + $this->assertEquals( + $existingUserId, + $checkedOutAsset->fresh()->assigned_to, + 'Asset was checked out when it should have been prevented.' + ); + + // ensure redirected back + $response->assertRedirectToRoute('hardware.bulkcheckout.show'); + } } From ac8a9e38f0ab7c99dc745b2a56f4bf774b282b2e Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 18 Sep 2025 17:18:27 -0700 Subject: [PATCH 2/4] Implement fix --- app/Http/Controllers/Assets/BulkAssetsController.php | 8 ++++++++ resources/lang/en-US/general.php | 1 + 2 files changed, 9 insertions(+) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 6c75ae06db..8ba3ccf99d 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -647,6 +647,14 @@ class BulkAssetsController extends Controller $assets = Asset::findOrFail($asset_ids); + if ($assets->pluck('assigned_to')->unique()->filter()->isNotEmpty()) { + // re-add the asset ids so the assets select is re-populated + $request->session()->flashInput(['selected_assets' => $asset_ids]); + + return redirect(route('hardware.bulkcheckout.show')) + ->with('error', trans('general.error_assets_already_checked_out')); + } + if (request('checkout_to_type') == 'asset') { foreach ($asset_ids as $asset_id) { if ($target->id == $asset_id) { diff --git a/resources/lang/en-US/general.php b/resources/lang/en-US/general.php index 61b7c9f809..42751c5cf9 100644 --- a/resources/lang/en-US/general.php +++ b/resources/lang/en-US/general.php @@ -520,6 +520,7 @@ return [ 'item_name_var' => ':item Name', 'error_user_company' => 'Checkout target company and asset company do not match', 'error_user_company_accept_view' => 'An Asset assigned to you belongs to a different company so you can\'t accept nor deny it, please check with your manager', + 'error_assets_already_checked_out' => 'One or more of the assets are already checked out', 'importer' => [ 'checked_out_to_fullname' => 'Checked Out to: Full Name', 'checked_out_to_first_name' => 'Checked Out to: First Name', From 7a3596c86d543cd80ea3c557aae658b7c02b2aca Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 18 Sep 2025 17:21:27 -0700 Subject: [PATCH 3/4] Test against other types --- .../Assets/BulkAssetsController.php | 1 + .../Checkouts/Ui/BulkAssetCheckoutTest.php | 43 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 8ba3ccf99d..4b9ccb6753 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -647,6 +647,7 @@ class BulkAssetsController extends Controller $assets = Asset::findOrFail($asset_ids); + // Prevent checking out assets that are already checked out if ($assets->pluck('assigned_to')->unique()->filter()->isNotEmpty()) { // re-add the asset ids so the assets select is re-populated $request->session()->flashInput(['selected_assets' => $asset_ids]); diff --git a/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php b/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php index 0791b3b422..c9619dbd69 100644 --- a/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php +++ b/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php @@ -4,8 +4,10 @@ namespace Tests\Feature\Checkouts\Ui; use App\Mail\CheckoutAssetMail; use App\Models\Asset; +use App\Models\Location; use App\Models\User; use Illuminate\Support\Facades\Mail; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\ExpectationFailedException; use Tests\TestCase; @@ -90,22 +92,53 @@ class BulkAssetCheckoutTest extends TestCase } } - public function test_prevents_checkouts_checked_out_items() + public static function checkoutTargets() { + yield 'Checkout to user' => [ + function () { + return [ + 'type' => 'user', + 'target' => User::factory()->forCompany()->create(), + ]; + } + ]; + + yield 'Checkout to asset' => [ + function () { + return [ + 'type' => 'asset', + 'target' => Asset::factory()->forCompany()->create(), + ]; + } + ]; + + yield 'Checkout to location' => [ + function () { + return [ + 'type' => 'location', + 'target' => Location::factory()->forCompany()->create(), + ]; + } + ]; + } + + #[DataProvider('checkoutTargets')] + public function test_prevents_checkouts_checked_out_items($data) + { + ['type' => $type, 'target' => $target] = $data(); + $asset = Asset::factory()->create(); $checkedOutAsset = Asset::factory()->assignedToUser()->create(); $existingUserId = $checkedOutAsset->assigned_to; - $target = User::factory()->create(); - $response = $this->actingAs(User::factory()->superuser()->create()) ->post(route('hardware.bulkcheckout.store'), [ 'selected_assets' => [ $asset->id, $checkedOutAsset->id, ], - 'checkout_to_type' => 'user', - 'assigned_user' => $target->id, + 'checkout_to_type' => $type, + "assigned_$type" => $target->id, ]); $this->assertEquals( From 52239a88b5f90ea567011fac8a54acf3b3b3c065 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 18 Sep 2025 17:27:17 -0700 Subject: [PATCH 4/4] Improve test name --- tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php b/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php index c9619dbd69..2f325ba68a 100644 --- a/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php +++ b/tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php @@ -123,7 +123,7 @@ class BulkAssetCheckoutTest extends TestCase } #[DataProvider('checkoutTargets')] - public function test_prevents_checkouts_checked_out_items($data) + public function test_prevents_checkouts_of_checked_out_items($data) { ['type' => $type, 'target' => $target] = $data();