Merge pull request #12336 from snipe/features/category_query_optimization

Refactor category API for fewer queries

app/Http/Controllers/Api/CategoriesController.php

@@ -10,6 +10,7 @@ use App\Models\Category;
 use Illuminate\Http\Request;
 use App\Http\Requests\ImageUploadRequest;
 use Illuminate\Support\Facades\Storage;
+use Illuminate\Support\Facades\Validator;
 
 class CategoriesController extends Controller
 {
@@ -107,7 +108,7 @@ class CategoriesController extends Controller
     public function show($id)
     {
         $this->authorize('view', Category::class);
-        $category = Category::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')->findOrFail($id);
         return (new CategoriesTransformer)->transformCategory($category);
 
     }
@@ -126,8 +127,14 @@ class CategoriesController extends Controller
     {
         $this->authorize('update', Category::class);
         $category = Category::findOrFail($id);
+
+        // Don't allow the user to change the category_type once it's been created
+        if (($request->filled('category_type')) && ($category->category_type != $request->input('category_type'))) {
+            return response()->json(
+                Helper::formatStandardApiResponse('error', null,  trans('admin/categories/message.update.cannot_change_category_type'))
+            );
+        }
         $category->fill($request->all());
-        $category->category_type = strtolower($request->input('category_type'));
         $category = $request->handleImages($category);
 
         if ($category->save()) {
@@ -148,7 +155,7 @@ class CategoriesController extends Controller
     public function destroy($id)
     {
         $this->authorize('delete', Category::class);
-        $category = Category::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')->findOrFail($id);
 
         if (! $category->isDeletable()) {
             return response()->json(

app/Http/Transformers/CategoriesTransformer.php

@@ -22,6 +22,26 @@ class CategoriesTransformer
 
     public function transformCategory(Category $category = null)
     {
+
+        // We only ever use item_count for categories in this transformer, so it makes sense to keep it
+        // simple and do this switch here.
+        switch ($category->category_type) {
+            case 'asset':
+                $category->item_count = $category->assets_count;
+                break;
+            case 'accessory':
+                $category->item_count = $category->accessories_count;
+                break;
+            case 'consumable':
+                $category->item_count = $category->consumables_count;
+                break;
+            case 'component':
+                $category->item_count = $category->components_count;
+                break;
+            default:
+                $category->item_count = 0;
+        }
+
         if ($category) {
             $array = [
                 'id' => (int) $category->id,
@@ -33,7 +53,7 @@ class CategoriesTransformer
                 'eula' => ($category->getEula()),
                 'checkin_email' => ($category->checkin_email == '1'),
                 'require_acceptance' => ($category->require_acceptance == '1'),
-                'item_count' => (int) $category->itemCount(),
+                'item_count' => (int) $category->item_count,
                 'assets_count' => (int) $category->assets_count,
                 'accessories_count' => (int) $category->accessories_count,
                 'consumables_count' => (int) $category->consumables_count,

app/Models/Category.php

@@ -10,6 +10,7 @@ use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Support\Facades\Gate;
 use Watson\Validating\ValidatingTrait;
 use App\Helpers\Helper;
+use Illuminate\Support\Str;
 
 /**
  * Model for Categories. Categories are a higher-level group
@@ -97,6 +98,7 @@ class Category extends SnipeModel
      */
     public function isDeletable()
     {
+
         return Gate::allows('delete', $this)
                 && ($this->itemCount() == 0);
     }
@@ -150,7 +152,10 @@ class Category extends SnipeModel
     }
 
     /**
-     * Get the number of items in the category
+     * Get the number of items in the category. This should NEVER be used in
+     * a collection of categories, as you'll end up with an n+1 query problem.
+     *
+     * It should only be used in a single category context.
      *
      * @author [A. Gianotto] [<snipe@snipe.net>]
      * @since [v2.0]
@@ -158,6 +163,11 @@ class Category extends SnipeModel
      */
     public function itemCount()
     {
+
+        if (isset($this->{Str::plural($this->category_type).'_count'})) {
+            return $this->{Str::plural($this->category_type).'_count'};
+        }
+
         switch ($this->category_type) {
             case 'asset':
                 return $this->assets()->count();
@@ -169,9 +179,10 @@ class Category extends SnipeModel
                 return $this->consumables()->count();
             case 'license':
                 return $this->licenses()->count();
+            default:
+                return 0;
         }
 
-        return '0';
     }
 
     /**

resources/lang/en/admin/categories/message.php

@@ -13,7 +13,8 @@ return array(
 
     'update' => array(
         'error'   => 'Category was not updated, please try again',
-        'success' => 'Category updated successfully.'
+        'success' => 'Category updated successfully.',
+        'cannot_change_category_type'   => 'You cannot change the category type once it has been created',
     ),
 
     'delete' => array(

resources/views/categories/edit.blade.php

@@ -15,11 +15,16 @@
 <div class="form-group {{ $errors->has('category_type') ? ' has-error' : '' }}">
     <label for="category_type" class="col-md-3 control-label">{{ trans('general.type') }}</label>
     <div class="col-md-7 required">
-        {{ Form::select('category_type', $category_types , old('category_type', $item->category_type), array('class'=>'select2', 'style'=>'min-width:350px', 'aria-label'=>'category_type', $item->itemCount() > 0 ? 'disabled' : '')) }}
+        {{ Form::select('category_type', $category_types , old('category_type', $item->category_type), array('class'=>'select2', 'style'=>'min-width:350px', 'aria-label'=>'category_type', ($item->category_type!='') || ($item->itemCount() > 0) ? 'disabled' : '')) }}
         {!! $errors->first('category_type', '<span class="alert-msg" aria-hidden="true"><i class="fas fa-times" aria-hidden="true"></i> :message</span>') !!}
     </div>
+    <div class="col-md-7 col-md-offset-3">
+        <p class="help-block">{!! trans('admin/categories/message.update.cannot_change_category_type') !!} </p>
+    </div>
 </div>
 
+
+
 <!-- EULA text -->
 <div class="form-group {{ $errors->has('eula_text') ? 'error' : '' }}">
     <label for="eula_text" class="col-md-3 control-label">{{ trans('admin/categories/general.eula_text') }}</label>