Merge pull request #16305 from snipe/bug/sc-28425

Fixed #16262 - Check for quantity before allowing component deletion

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')));

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'));

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;
 

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
      *

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(

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');
+    }
 }

tests/Feature/Components/Ui/DeleteComponentTest.php

@@ -40,6 +40,16 @@ 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'));
+    }
+
     public function testDeletingComponentRemovesComponentImage()
     {
         Storage::fake('public');