Merge pull request #16909 from grokability/fixes-#16554-category-delete

Fixed #16554 - Added models to deletable check

app/Http/Controllers/Api/CategoriesController.php

@@ -56,7 +56,7 @@ class CategoriesController extends Controller
             'notes',
             ])
             ->with('adminuser')
-            ->withCount('accessories as accessories_count', 'consumables as consumables_count', 'components as components_count', 'licenses as licenses_count');
+            ->withCount('accessories as accessories_count', 'consumables as consumables_count', 'components as components_count', 'licenses as licenses_count', 'models as models_count');
 
 
         /*
@@ -212,7 +212,7 @@ class CategoriesController extends Controller
     public function destroy($id) : JsonResponse
     {
         $this->authorize('delete', Category::class);
-        $category = Category::withCount('assets as assets_count', 'accessories as accessories_count', 'consumables as consumables_count', 'components as components_count', 'licenses as licenses_count')->findOrFail($id);
+        $category = Category::withCount('assets as assets_count', 'accessories as accessories_count', 'consumables as consumables_count', 'components as components_count', 'licenses as licenses_count', 'models as models_count')->findOrFail($id);
 
         if (! $category->isDeletable()) {
             return response()->json(

app/Http/Controllers/CategoriesController.php

@@ -145,7 +145,7 @@ class CategoriesController extends Controller
     {
         $this->authorize('delete', Category::class);
         // Check if the category exists
-        if (is_null($category = Category::findOrFail($categoryId))) {
+        if (is_null($category = Category::withCount('assets as assets_count', 'accessories as accessories_count', 'consumables as consumables_count', 'components as components_count', 'licenses as licenses_count', 'models as models_count')->findOrFail($categoryId))) {
             return redirect()->route('categories.index')->with('error', trans('admin/categories/message.not_found'));
         }
 
@@ -155,7 +155,6 @@ class CategoriesController extends Controller
 
         Storage::disk('public')->delete('categories'.'/'.$category->image);
         $category->delete();
-        // Redirect to the locations management page
         return redirect()->route('categories.index')->with('success', trans('admin/categories/message.delete.success'));
     }
 

app/Models/Category.php

@@ -100,6 +100,14 @@ class Category extends SnipeModel
     public function isDeletable()
     {
 
+        // We have to check for models as well if the category type is asset
+        if ($this->category_type == 'asset') {
+            return Gate::allows('delete', $this)
+                && ($this->itemCount() == 0)
+                && ($this->models_count == 0)
+                && ($this->deleted_at == '');
+        }
+
         return Gate::allows('delete', $this)
                 && ($this->itemCount() == 0)
                 && ($this->deleted_at == '');

tests/Feature/Categories/Api/DeleteCategoriesTest.php

@@ -3,6 +3,7 @@
 namespace Tests\Feature\Categories\Api;
 
 use App\Models\Asset;
+use App\Models\AssetModel;
 use App\Models\Category;
 use App\Models\User;
 use Tests\Concerns\TestsPermissionsRequirement;
@@ -21,7 +22,7 @@ class DeleteCategoriesTest extends TestCase implements TestsPermissionsRequireme
         $this->assertNotSoftDeleted($category);
     }
 
-    public function testCannotDeleteCategoryThatStillHasAssociatedItems()
+    public function testCannotDeleteCategoryThatStillHasAssociatedAssets()
     {
         $asset = Asset::factory()->create();
         $category = $asset->model->category;
@@ -33,6 +34,18 @@ class DeleteCategoriesTest extends TestCase implements TestsPermissionsRequireme
         $this->assertNotSoftDeleted($category);
     }
 
+    public function testCannotDeleteCategoryThatStillHasAssociatedModels()
+    {
+        $model = AssetModel::factory()->create();
+        $category = $model->category;
+
+        $this->actingAsForApi(User::factory()->deleteCategories()->create())
+            ->deleteJson(route('api.categories.destroy', $category))
+            ->assertStatusMessageIs('error');
+
+        $this->assertNotSoftDeleted($category);
+    }
+
     public function testCanDeleteCategory()
     {
         $category = Category::factory()->create();

tests/Feature/Categories/Ui/DeleteCategoriesTest.php

@@ -0,0 +1,60 @@
+<?php
+
+namespace Tests\Feature\Assets\Ui;
+
+use App\Events\CheckoutableCheckedIn;
+use App\Models\Asset;
+use App\Models\AssetModel;
+use App\Models\User;
+use App\Models\Category;
+use Illuminate\Support\Facades\Event;
+use Illuminate\Support\Facades\Storage;
+use Tests\TestCase;
+
+class DeleteCategoriesTest extends TestCase
+{
+    public function testPermissionNeededToDeleteCategory()
+    {
+        $this->actingAs(User::factory()->create())
+            ->delete(route('categories.destroy', Category::factory()->create()))
+            ->assertForbidden();
+    }
+
+    public function testCanDeleteCategory()
+    {
+        $category = Category::factory()->create();
+
+        $this->actingAs(User::factory()->deleteCategories()->create())
+            ->delete(route('categories.destroy', $category))
+            ->assertRedirectToRoute('categories.index')
+            ->assertSessionHas('success');
+
+        $this->assertSoftDeleted($category);
+    }
+
+    public function testCannotDeleteCategoryThatStillHasAssociatedModels()
+    {
+        $model = AssetModel::factory()->create();
+        $category = $model->category;
+
+        $this->actingAs(User::factory()->deleteCategories()->create())
+            ->delete(route('categories.destroy', $category))
+            ->assertRedirectToRoute('categories.index')
+            ->assertSessionHas('error');
+        $this->assertNotSoftDeleted($category);
+    }
+
+    public function testCannotDeleteCategoryThatStillHasAssociatedAssets()
+    {
+        $asset = Asset::factory()->create();
+        $category = $asset->model->category;
+
+        $this->actingAs(User::factory()->deleteCategories()->create())
+            ->delete(route('categories.destroy', $category))
+            ->assertRedirectToRoute('categories.index')
+            ->assertSessionHas('error');
+
+        $this->assertNotSoftDeleted($category);
+    }
+
+}