From 8c21d625fc29f8f0db7b260fe487b5b5da0fbdda Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Mar 2025 15:56:01 -0800 Subject: [PATCH 1/5] Prevent deleting accessory that has checkouts via UI --- .../Accessories/AccessoriesController.php | 6 ++--- database/factories/AccessoryFactory.php | 27 +++++++++++++++++++ .../lang/en-US/admin/accessories/message.php | 1 + .../Accessories/Ui/DeleteAccessoryTest.php | 13 +++++++-- 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/app/Http/Controllers/Accessories/AccessoriesController.php b/app/Http/Controllers/Accessories/AccessoriesController.php index eb004d5594..3bebf895c1 100755 --- a/app/Http/Controllers/Accessories/AccessoriesController.php +++ b/app/Http/Controllers/Accessories/AccessoriesController.php @@ -184,15 +184,15 @@ class AccessoriesController extends Controller */ public function destroy($accessoryId) : RedirectResponse { - if (is_null($accessory = Accessory::find($accessoryId))) { + if (is_null($accessory = Accessory::withCount('checkouts as checkouts_count')->find($accessoryId))) { return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.not_found')); } $this->authorize($accessory); - if ($accessory->hasUsers() > 0) { - return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.assoc_users', ['count'=> $accessory->hasUsers()])); + if ($accessory->checkouts_count > 0) { + return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.assoc_checkouts', ['count' => $accessory->checkouts_count])); } if ($accessory->image) { diff --git a/database/factories/AccessoryFactory.php b/database/factories/AccessoryFactory.php index bdcffd50e9..f2c01dc8fb 100644 --- a/database/factories/AccessoryFactory.php +++ b/database/factories/AccessoryFactory.php @@ -3,6 +3,7 @@ namespace Database\Factories; use App\Models\Accessory; +use App\Models\Asset; use App\Models\Category; use App\Models\Location; use App\Models\Manufacturer; @@ -170,4 +171,30 @@ class AccessoryFactory extends Factory } }); } + + public function checkedOutToAsset(Asset $asset = null) + { + return $this->afterCreating(function (Accessory $accessory) use ($asset) { + $accessory->checkouts()->create([ + 'accessory_id' => $accessory->id, + 'created_at' => Carbon::now(), + 'created_by' => 1, + 'assigned_to' => $asset->id ?? Asset::factory()->create()->id, + 'assigned_type' => Asset::class, + ]); + }); + } + + public function checkedOutToLocation(Location $location = null) + { + return $this->afterCreating(function (Accessory $accessory) use ($location) { + $accessory->checkouts()->create([ + 'accessory_id' => $accessory->id, + 'created_at' => Carbon::now(), + 'created_by' => 1, + 'assigned_to' => $location->id ?? Location::factory()->create()->id, + 'assigned_type' => Location::class, + ]); + }); + } } diff --git a/resources/lang/en-US/admin/accessories/message.php b/resources/lang/en-US/admin/accessories/message.php index f60d41957b..693203c21c 100644 --- a/resources/lang/en-US/admin/accessories/message.php +++ b/resources/lang/en-US/admin/accessories/message.php @@ -5,6 +5,7 @@ return array( 'does_not_exist' => 'The accessory [:id] does not exist.', 'not_found' => 'That accessory was not found.', 'assoc_users' => 'This accessory currently has :count items checked out to users. Please check in the accessories and and try again. ', + 'assoc_checkouts' => 'This accessory currently has :count items checked out. Please check in the accessories and and try again.', 'create' => array( 'error' => 'The accessory was not created, please try again.', diff --git a/tests/Feature/Accessories/Ui/DeleteAccessoryTest.php b/tests/Feature/Accessories/Ui/DeleteAccessoryTest.php index 80cf960911..45ebbf71f7 100644 --- a/tests/Feature/Accessories/Ui/DeleteAccessoryTest.php +++ b/tests/Feature/Accessories/Ui/DeleteAccessoryTest.php @@ -5,6 +5,7 @@ namespace Tests\Feature\Accessories\Ui; use App\Models\Accessory; use App\Models\Company; use App\Models\User; +use PHPUnit\Framework\Attributes\DataProvider; use Tests\TestCase; class DeleteAccessoryTest extends TestCase @@ -29,9 +30,17 @@ class DeleteAccessoryTest extends TestCase $this->assertFalse($accessoryForCompanyA->refresh()->trashed(), 'Accessory should not be deleted'); } - public function testCannotDeleteAccessoryThatHasCheckouts() + public static function checkedOutAccessories() { - $accessory = Accessory::factory()->checkedOutToUser()->create(); + yield 'checked out to user' => [fn() => Accessory::factory()->checkedOutToUser()->create()]; + yield 'checked out to asset' => [fn() => Accessory::factory()->checkedOutToAsset()->create()]; + yield 'checked out to location' => [fn() => Accessory::factory()->checkedOutToLocation()->create()]; + } + + #[DataProvider('checkedOutAccessories')] + public function testCannotDeleteAccessoryThatHasCheckouts($data) + { + $accessory = $data(); $this->actingAs(User::factory()->deleteAccessories()->create()) ->delete(route('accessories.destroy', $accessory->id)) From 00cbebd1e37f0856678b52b15acf3cad35f860df Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Mar 2025 15:57:18 -0800 Subject: [PATCH 2/5] Add failing test for api --- .../Accessories/Api/DeleteAccessoriesTest.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Accessories/Api/DeleteAccessoriesTest.php b/tests/Feature/Accessories/Api/DeleteAccessoriesTest.php index f6a15118f5..e82d0de488 100644 --- a/tests/Feature/Accessories/Api/DeleteAccessoriesTest.php +++ b/tests/Feature/Accessories/Api/DeleteAccessoriesTest.php @@ -5,6 +5,7 @@ namespace Tests\Feature\Accessories\Api; use App\Models\Accessory; use App\Models\Company; use App\Models\User; +use PHPUnit\Framework\Attributes\DataProvider; use Tests\Concerns\TestsFullMultipleCompaniesSupport; use Tests\Concerns\TestsPermissionsRequirement; use Tests\TestCase; @@ -53,9 +54,17 @@ class DeleteAccessoriesTest extends TestCase implements TestsFullMultipleCompani $this->assertSoftDeleted($accessoryC); } - public function testCannotDeleteAccessoryThatHasCheckouts() + public static function checkedOutAccessories() { - $accessory = Accessory::factory()->checkedOutToUser()->create(); + yield 'checked out to user' => [fn() => Accessory::factory()->checkedOutToUser()->create()]; + yield 'checked out to asset' => [fn() => Accessory::factory()->checkedOutToAsset()->create()]; + yield 'checked out to location' => [fn() => Accessory::factory()->checkedOutToLocation()->create()]; + } + + #[DataProvider('checkedOutAccessories')] + public function testCannotDeleteAccessoryThatHasCheckouts($data) + { + $accessory = $data(); $this->actingAsForApi(User::factory()->deleteAccessories()->create()) ->deleteJson(route('api.accessories.destroy', $accessory)) From a19582a5f3f3cdfafe99c8347bff970295e1bbab Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Mar 2025 15:58:34 -0800 Subject: [PATCH 3/5] Prevent deleting accessory that has checkouts via api --- app/Http/Controllers/Api/AccessoriesController.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Api/AccessoriesController.php b/app/Http/Controllers/Api/AccessoriesController.php index 76e69591d4..e859110f8e 100644 --- a/app/Http/Controllers/Api/AccessoriesController.php +++ b/app/Http/Controllers/Api/AccessoriesController.php @@ -249,11 +249,11 @@ class AccessoriesController extends Controller public function destroy($id) { $this->authorize('delete', Accessory::class); - $accessory = Accessory::findOrFail($id); + $accessory = Accessory::withCount('checkouts as checkouts_count')->findOrFail($id); $this->authorize($accessory); - if ($accessory->hasUsers() > 0) { - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/accessories/message.assoc_users', ['count'=> $accessory->hasUsers()]))); + if ($accessory->checkouts_count > 0) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/accessories/message.assoc_checkouts', ['count' => $accessory->checkouts_count]))); } $accessory->delete(); From f038254038e68addc7d1f85a95573fae2d703916 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Mar 2025 16:02:23 -0800 Subject: [PATCH 4/5] Have UI reflect not being able to delete accessory --- app/Http/Transformers/AccessoriesTransformer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/Transformers/AccessoriesTransformer.php b/app/Http/Transformers/AccessoriesTransformer.php index 58858cf897..dde38ccd80 100644 --- a/app/Http/Transformers/AccessoriesTransformer.php +++ b/app/Http/Transformers/AccessoriesTransformer.php @@ -53,7 +53,7 @@ class AccessoriesTransformer 'checkout' => Gate::allows('checkout', Accessory::class), 'checkin' => false, 'update' => Gate::allows('update', Accessory::class), - 'delete' => Gate::allows('delete', Accessory::class), + 'delete' => $accessory->checkouts_count === 0 && Gate::allows('delete', Accessory::class), 'clone' => Gate::allows('create', Accessory::class), ]; From d1683d1c653f70c95153fcad1934b6adba1f368f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 5 Mar 2025 16:10:36 -0800 Subject: [PATCH 5/5] Use existing translation string --- app/Http/Controllers/Accessories/AccessoriesController.php | 2 +- app/Http/Controllers/Api/AccessoriesController.php | 2 +- resources/lang/en-US/admin/accessories/message.php | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Accessories/AccessoriesController.php b/app/Http/Controllers/Accessories/AccessoriesController.php index 3bebf895c1..737f972c4f 100755 --- a/app/Http/Controllers/Accessories/AccessoriesController.php +++ b/app/Http/Controllers/Accessories/AccessoriesController.php @@ -192,7 +192,7 @@ class AccessoriesController extends Controller if ($accessory->checkouts_count > 0) { - return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.assoc_checkouts', ['count' => $accessory->checkouts_count])); + return redirect()->route('accessories.index')->with('error', trans('admin/accessories/general.delete_disabled')); } if ($accessory->image) { diff --git a/app/Http/Controllers/Api/AccessoriesController.php b/app/Http/Controllers/Api/AccessoriesController.php index e859110f8e..53ad49cd8d 100644 --- a/app/Http/Controllers/Api/AccessoriesController.php +++ b/app/Http/Controllers/Api/AccessoriesController.php @@ -253,7 +253,7 @@ class AccessoriesController extends Controller $this->authorize($accessory); if ($accessory->checkouts_count > 0) { - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/accessories/message.assoc_checkouts', ['count' => $accessory->checkouts_count]))); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/accessories/general.delete_disabled'))); } $accessory->delete(); diff --git a/resources/lang/en-US/admin/accessories/message.php b/resources/lang/en-US/admin/accessories/message.php index 693203c21c..f60d41957b 100644 --- a/resources/lang/en-US/admin/accessories/message.php +++ b/resources/lang/en-US/admin/accessories/message.php @@ -5,7 +5,6 @@ return array( 'does_not_exist' => 'The accessory [:id] does not exist.', 'not_found' => 'That accessory was not found.', 'assoc_users' => 'This accessory currently has :count items checked out to users. Please check in the accessories and and try again. ', - 'assoc_checkouts' => 'This accessory currently has :count items checked out. Please check in the accessories and and try again.', 'create' => array( 'error' => 'The accessory was not created, please try again.',