From 7b151cf6920e12801bfc463498fc6a0d3b544672 Mon Sep 17 00:00:00 2001 From: snipe Date: Sun, 23 Feb 2025 13:17:20 +0000 Subject: [PATCH 1/5] Added test Signed-off-by: snipe --- tests/Feature/Components/Ui/DeleteComponentTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Feature/Components/Ui/DeleteComponentTest.php b/tests/Feature/Components/Ui/DeleteComponentTest.php index a0b8131532..56950cab7b 100644 --- a/tests/Feature/Components/Ui/DeleteComponentTest.php +++ b/tests/Feature/Components/Ui/DeleteComponentTest.php @@ -40,6 +40,18 @@ class DeleteComponentTest extends TestCase implements TestsFullMultipleCompanies $this->assertSoftDeleted($component); } + public function testCannotDeleteComponentIfCheckedOut() + { + $component = Component::factory()->checkedOutToAsset()->create(); + + $this->actingAs(User::factory()->deleteComponents()->create()) + ->delete(route('components.destroy', $component->id)) + ->assertSessionHas('error') + ->assertRedirect(route('components.index')); + + $this->assertSoftDeleted($component); + } + public function testDeletingComponentRemovesComponentImage() { Storage::fake('public'); From 157801242d654c02c5812d7b99960cda7cc24af8 Mon Sep 17 00:00:00 2001 From: snipe Date: Sun, 23 Feb 2025 13:21:36 +0000 Subject: [PATCH 2/5] Added API test, renamed test to match filter Signed-off-by: snipe --- ...leteComponentsTest.php => DeleteComponentTest.php} | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) rename tests/Feature/Components/Api/{DeleteComponentsTest.php => DeleteComponentTest.php} (83%) diff --git a/tests/Feature/Components/Api/DeleteComponentsTest.php b/tests/Feature/Components/Api/DeleteComponentTest.php similarity index 83% rename from tests/Feature/Components/Api/DeleteComponentsTest.php rename to tests/Feature/Components/Api/DeleteComponentTest.php index e95fe34559..80d22d389e 100644 --- a/tests/Feature/Components/Api/DeleteComponentsTest.php +++ b/tests/Feature/Components/Api/DeleteComponentTest.php @@ -9,7 +9,7 @@ use Tests\Concerns\TestsFullMultipleCompaniesSupport; use Tests\Concerns\TestsPermissionsRequirement; use Tests\TestCase; -class DeleteComponentsTest extends TestCase implements TestsFullMultipleCompaniesSupport, TestsPermissionsRequirement +class DeleteComponentTest extends TestCase implements TestsFullMultipleCompaniesSupport, TestsPermissionsRequirement { public function testRequiresPermission() { @@ -63,4 +63,13 @@ class DeleteComponentsTest extends TestCase implements TestsFullMultipleCompanie $this->assertSoftDeleted($component); } + + public function testCannotDeleteComponentIfCheckedOut() + { + $component = Component::factory()->checkedOutToAsset()->create(); + + $this->actingAsForApi(User::factory()->deleteComponents()->create()) + ->deleteJson(route('api.components.destroy', $component)) + ->assertStatusMessageIs('error'); + } } From 96248e00232eb03102d17f07f45c4e4139e370fa Mon Sep 17 00:00:00 2001 From: snipe Date: Sun, 23 Feb 2025 13:21:49 +0000 Subject: [PATCH 3/5] Removed soft delete from test Signed-off-by: snipe --- tests/Feature/Components/Ui/DeleteComponentTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Feature/Components/Ui/DeleteComponentTest.php b/tests/Feature/Components/Ui/DeleteComponentTest.php index 56950cab7b..31d62f65dd 100644 --- a/tests/Feature/Components/Ui/DeleteComponentTest.php +++ b/tests/Feature/Components/Ui/DeleteComponentTest.php @@ -48,8 +48,6 @@ class DeleteComponentTest extends TestCase implements TestsFullMultipleCompanies ->delete(route('components.destroy', $component->id)) ->assertSessionHas('error') ->assertRedirect(route('components.index')); - - $this->assertSoftDeleted($component); } public function testDeletingComponentRemovesComponentImage() From 2bee4532ecad33e3a7bb1e8c673c925194b41365 Mon Sep 17 00:00:00 2001 From: snipe Date: Sun, 23 Feb 2025 13:21:58 +0000 Subject: [PATCH 4/5] Added qty error string Signed-off-by: snipe --- resources/lang/en-US/admin/components/message.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/resources/lang/en-US/admin/components/message.php b/resources/lang/en-US/admin/components/message.php index 0a7dd8d954..ec39dc4178 100644 --- a/resources/lang/en-US/admin/components/message.php +++ b/resources/lang/en-US/admin/components/message.php @@ -17,7 +17,8 @@ return array( 'delete' => array( 'confirm' => 'Are you sure you wish to delete this component?', 'error' => 'There was an issue deleting the component. Please try again.', - 'success' => 'The component was deleted successfully.' + 'success' => 'The component was deleted successfully.', + 'error_qty' => 'Some components of this type are still checked out. Please check them in and try again.', ), 'checkout' => array( From dd2b570db5846c94ea00dfb0bb89a7fd9e39b40a Mon Sep 17 00:00:00 2001 From: snipe Date: Sun, 23 Feb 2025 14:11:39 +0000 Subject: [PATCH 5/5] Added tighter constraints on deleting components Signed-off-by: snipe --- .../Controllers/Api/ComponentsController.php | 8 ++++- .../Components/ComponentsController.php | 4 +++ .../Transformers/ComponentsTransformer.php | 2 +- app/Models/Component.php | 29 +++++++++++++++---- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/Api/ComponentsController.php b/app/Http/Controllers/Api/ComponentsController.php index 0f594f5e75..2009518862 100644 --- a/app/Http/Controllers/Api/ComponentsController.php +++ b/app/Http/Controllers/Api/ComponentsController.php @@ -48,7 +48,8 @@ class ComponentsController extends Controller ]; $components = Component::select('components.*') - ->with('company', 'location', 'category', 'assets', 'supplier', 'adminuser', 'manufacturer'); + ->with('company', 'location', 'category', 'assets', 'supplier', 'adminuser', 'manufacturer', 'uncontrainedAssets') + ->withSum('uncontrainedAssets', 'components_assets.assigned_qty'); if ($request->filled('search')) { $components = $components->TextSearch($request->input('search')); @@ -197,6 +198,11 @@ class ComponentsController extends Controller $this->authorize('delete', Component::class); $component = Component::findOrFail($id); $this->authorize('delete', $component); + + if ($component->numCheckedOut() > 0) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/components/message.delete.error_qty'))); + } + $component->delete(); return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/components/message.delete.success'))); diff --git a/app/Http/Controllers/Components/ComponentsController.php b/app/Http/Controllers/Components/ComponentsController.php index d5883c05f6..74594d312b 100644 --- a/app/Http/Controllers/Components/ComponentsController.php +++ b/app/Http/Controllers/Components/ComponentsController.php @@ -196,6 +196,10 @@ class ComponentsController extends Controller } } + if ($component->numCheckedOut() > 0) { + return redirect()->route('components.index')->with('error', trans('admin/components/message.delete.error_qty')); + } + $component->delete(); return redirect()->route('components.index')->with('success', trans('admin/components/message.delete.success')); diff --git a/app/Http/Transformers/ComponentsTransformer.php b/app/Http/Transformers/ComponentsTransformer.php index f98edd6e3f..90d10ba9a5 100644 --- a/app/Http/Transformers/ComponentsTransformer.php +++ b/app/Http/Transformers/ComponentsTransformer.php @@ -62,7 +62,7 @@ class ComponentsTransformer 'checkout' => Gate::allows('checkout', Component::class), 'checkin' => Gate::allows('checkin', Component::class), 'update' => Gate::allows('update', Component::class), - 'delete' => Gate::allows('delete', Component::class), + 'delete' => $component->isDeletable(), ]; $array += $permissions_array; diff --git a/app/Models/Component.php b/app/Models/Component.php index fb77bf0824..d9277d7da7 100644 --- a/app/Models/Component.php +++ b/app/Models/Component.php @@ -6,6 +6,7 @@ use App\Models\Traits\Searchable; use App\Presenters\Presentable; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\SoftDeletes; +use Illuminate\Support\Facades\Gate; use Watson\Validating\ValidatingTrait; /** @@ -104,6 +105,13 @@ class Component extends SnipeModel ]; + public function isDeletable() + { + return Gate::allows('delete', $this) + && ($this->numCheckedOut() === 0) + && ($this->deleted_at == ''); + } + /** * Establishes the components -> action logs -> uploads relationship * @@ -234,13 +242,24 @@ class Component extends SnipeModel // In case there are elements checked out to assets that belong to a different company // than this asset and full multiple company support is on we'll remove the global scope, // so they are included in the count. - foreach ($this->assets()->withoutGlobalScope(new CompanyableScope)->get() as $checkout) { - $checkedout += $checkout->pivot->assigned_qty; - } - - return $checkedout; + return $this->uncontrainedAssets->sum('pivot.assigned_qty'); } + + /** + * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany + * + * This allows us to get the assets with assigned components without the company restriction + */ + public function uncontrainedAssets() { + + return $this->belongsToMany(\App\Models\Asset::class, 'components_assets') + ->withPivot('id', 'assigned_qty', 'created_at', 'created_by', 'note') + ->withoutGlobalScope(new CompanyableScope); + + } + + /** * Check how many items within a component are remaining *