Fixed n+1 query, changed checkout behavior to just ask for a license ID

We’re offloading the freeSeat() to the checkout page now

app/Http/Controllers/Api/LicensesController.php

@@ -21,7 +21,7 @@ class LicensesController extends Controller
     public function index(Request $request)
     {
         $this->authorize('view', License::class);
-        $licenses = Company::scopeCompanyables(License::with('company', 'licenseseats', 'manufacturer', 'supplier')->withCount('licenseseats'));
+        $licenses = Company::scopeCompanyables(License::with('company', 'manufacturer', 'freeSeats', 'supplier')->withCount('freeSeats'));
 
         if ($request->has('search')) {
             $licenses = $licenses->TextSearch($request->input('search'));

app/Http/Controllers/LicensesController.php

@@ -239,16 +239,19 @@ class LicensesController extends Controller
     * @param int $seatId
     * @return \Illuminate\Contracts\View\View
      */
-    public function getCheckout($seatId)
+    public function getCheckout($licenceId)
     {
-        // Check if the license seat exists
+        // Check that the license is valid
-        if (is_null($licenseSeat = LicenseSeat::find($seatId))) {
+        if ($license = License::where('id',$licenceId)->first()) {
-            // Redirect to the asset management page with error
+
-            return redirect()->route('licenses.index')->with('error', trans('admin/licenses/message.not_found'));
+            // If the license is valid, check that there is an available seat
+            if ($license->getAvailSeatsCountAttribute() < 1) {
+                return redirect()->route('licenses.index')->with('error', 'There are no available seats for this license');
+            }
         }
 
-        $this->authorize('checkout', $licenseSeat);
+        $this->authorize('checkout', $license);
-        return view('licenses/checkout', compact('licenseSeat'));
+        return view('licenses/checkout', compact('license'));
     }
 
 
@@ -262,77 +265,94 @@ class LicensesController extends Controller
      * @param int $seatId
      * @return \Illuminate\Http\RedirectResponse
      */
-    public function postCheckout(Request $request, $seatId)
+    public function postCheckout(Request $request, $licenseId)
     {
-        $licenseSeat = LicenseSeat::find($seatId);
-        $assigned_to = e($request->input('assigned_to'));
-        $asset_id = e($request->input('asset_id'));
 
-        $this->authorize('checkout', $licenseSeat);
+        // Check that the license is valid
+        if ($license = License::where('id',$licenseId)->first()) {
 
-        // Declare the rules for the form validation
+            // If the license is valid, check that there is an available seat
-        $rules = [
+            if ($license->getAvailSeatsCountAttribute() < 1) {
-            'note'   => 'string|nullable',
+                return redirect()->route('licenses.index')->with('error', 'There are no available seats for this license');
-            'asset_id'  => 'required_without:assigned_to',
+            }
-        ];
+            $next = $license->freeSeat();
 
-        // Create a new validator instance from our validation rules
-        $validator = Validator::make(Input::all(), $rules);
 
-        // If validation fails, we'll exit the operation now.
+            $licenseSeat = LicenseSeat::where('license_id',$license->id)->find($next)->first();
-        if ($validator->fails()) {
+            $assigned_to = $request->input('assigned_to');
-            // Ooops.. something went wrong
+            $asset_id = $request->input('asset_id');
-            return redirect()->back()->withInput()->withErrors($validator);
+
-        }
+            $this->authorize('checkout', $licenseSeat);
-        $target = null;
+
-        if ($assigned_to!='') {
+            // Declare the rules for the form validation
-        // Check if the user exists
+            $rules = [
-            if (is_null($target = User::find($assigned_to))) {
+                'note'   => 'string|nullable',
+                'asset_id'  => 'required_without:assigned_to',
+            ];
+
+            // Create a new validator instance from our validation rules
+            $validator = Validator::make(Input::all(), $rules);
+
+            // If validation fails, we'll exit the operation now.
+            if ($validator->fails()) {
+                // Ooops.. something went wrong
+                return redirect()->back()->withInput()->withErrors($validator);
+            }
+            $target = null;
+            if ($assigned_to!='') {
+                // Check if the user exists
+                if (is_null($target = User::find($assigned_to))) {
+                    // Redirect to the asset management page with error
+                    return redirect()->route('licenses.index')->with('error', trans('admin/licenses/message.user_does_not_exist'));
+                }
+            }
+
+            if ($asset_id!='') {
+                if (is_null($target = Asset::find($asset_id))) {
+                    // Redirect to the asset management page with error
+                    return redirect()->route('licenses.index')->with('error', trans('admin/licenses/message.asset_does_not_exist'));
+                }
+
+                if (($request->has('assigned_to')) && ($request->has('asset_id'))) {
+                    return redirect()->back()->withInput()->with('error', trans('admin/licenses/message.select_asset_or_person'));
+                }
+            }
+
+            // Check if the asset exists
+            if (is_null($licenseSeat)) {
                 // Redirect to the asset management page with error
-                return redirect()->route('licenses.index')->with('error', trans('admin/licenses/message.user_does_not_exist'));
+                return redirect()->route('licenses.index')->with('error', trans('admin/licenses/message.not_found'));
-            }
-        }
-
-        if ($asset_id!='') {
-            if (is_null($target = Asset::find($asset_id))) {
-                // Redirect to the asset management page with error
-                return redirect()->route('licenses.index')->with('error', trans('admin/licenses/message.asset_does_not_exist'));
             }
 
-            if (($request->has('assigned_to')) && ($request->has('asset_id'))) {
+            if ($request->input('asset_id') == '') {
-                return redirect()->back()->withInput()->with('error', trans('admin/licenses/message.select_asset_or_person'));
+                $licenseSeat->asset_id = null;
+            } else {
+                $licenseSeat->asset_id = $request->input('asset_id');
             }
-        }
 
-        // Check if the asset exists
+            // Update the asset data
-        if (is_null($licenseSeat)) {
+            if ($request->input('assigned_to') == '') {
-            // Redirect to the asset management page with error
-            return redirect()->route('licenses.index')->with('error', trans('admin/licenses/message.not_found'));
-        }
-
-        if ($request->input('asset_id') == '') {
-            $licenseSeat->asset_id = null;
-        } else {
-            $licenseSeat->asset_id = $request->input('asset_id');
-        }
-
-        // Update the asset data
-        if ($request->input('assigned_to') == '') {
                 $licenseSeat->assigned_to =  null;
-        } else {
+            } else {
                 $licenseSeat->assigned_to = $request->input('assigned_to');
+            }
+
+            // Was the asset updated?
+            if ($licenseSeat->save()) {
+                $licenseSeat->logCheckout($request->input('note'), $target);
+
+                $data['license_id'] = $licenseSeat->license_id;
+                $data['note'] = $request->input('note');
+
+                // Redirect to the new asset page
+                return redirect()->route("licenses.index")->with('success', trans('admin/licenses/message.checkout.success'));
+            }
+
         }
+        return redirect()->route('licenses.index')->with('error', trans('admin/licenses/message.not_found'));
 
-        // Was the asset updated?
-        if ($licenseSeat->save()) {
-            $licenseSeat->logCheckout($request->input('note'), $target);
 
-            $data['license_id'] = $licenseSeat->license_id;
-            $data['note'] = $request->input('note');
 
-            // Redirect to the new asset page
-            return redirect()->route("licenses.index")->with('success', trans('admin/licenses/message.checkout.success'));
-        }
 
         return redirect()->route("licenses.index")->with('error', trans('admin/licenses/message.checkout.error'));
     }

app/Http/Transformers/LicensesTransformer.php

@@ -33,14 +33,14 @@ class LicensesTransformer
             'notes' => e($license->notes),
             'expiration_date' => Helper::getFormattedDateObject($license->expiration_date, 'date'),
             'total_seats' => (int) $license->seats,
-            'remaining_qty' => (int) ($license->seats - $license->licenseseats_count),
+            'remaining_qty' => $license->free_seats_count,
             'license_name' =>  e($license->license_name),
             'license_email' => e($license->license_email),
             'maintained' => ($license->maintained == 1) ? true : false,
             'supplier' =>  ($license->supplier) ? ['id' => (int)  $license->supplier->id,'name'=> e($license->supplier->name)] : null,
             'created_at' => Helper::getFormattedDateObject($license->created_at, 'datetime'),
             'updated_at' => Helper::getFormattedDateObject($license->updated_at, 'datetime'),
-            'user_can_checkout' => (bool) (($license->seats - $license->licenseseats_count) > 0),
+            'user_can_checkout' => (bool) ($license->free_seats_count > 0),
         ];
 
         $permissions_array['available_actions'] = [

app/Models/License.php

@@ -266,7 +266,9 @@ class License extends Depreciable
     public function availCount()
     {
         return $this->licenseSeatsRelation()
-            ->whereNull('asset_id');
+            ->whereNull('asset_id')
+            ->whereNull('assigned_to')
+            ->whereNull('user_id');
     }
 
     public function getAvailSeatsCountAttribute()
@@ -345,6 +347,15 @@ class License extends Depreciable
                     ->first();
     }
 
+    /*
+   * Get the next available free seat - used by
+   * the API to populate next_seat
+   */
+    public function freeSeats()
+    {
+        return $this->hasMany('\App\Models\LicenseSeat')->whereNull('assigned_to')->whereNull('deleted_at')->whereNull('asset_id');
+    }
+
     public static function getExpiringLicenses($days = 60)
     {
 

resources/views/licenses/checkout.blade.php

@@ -21,7 +21,7 @@
 
             <div class="box box-default">
                 <div class="box-header with-border">
-                    <h3 class="box-title"> {{ $licenseSeat->license->name }}</h3>
+                    <h3 class="box-title"> {{ $license->name }}</h3>
                 </div>
                 <div class="box-body">
 
@@ -29,7 +29,7 @@
                     <div class="form-group">
                         <label class="col-sm-3 control-label">{{ trans('admin/hardware/form.name') }}</label>
                         <div class="col-md-6">
-                            <p class="form-control-static">{{ $licenseSeat->license->name }}</p>
+                            <p class="form-control-static">{{ $license->name }}</p>
                         </div>
                     </div>
 
@@ -37,7 +37,7 @@
                     <div class="form-group">
                         <label class="col-sm-3 control-label">{{ trans('admin/hardware/form.serial') }}</label>
                         <div class="col-md-9">
-                            <p class="form-control-static" style="word-wrap: break-word;">{{ $licenseSeat->license->serial }}</p>
+                            <p class="form-control-static" style="word-wrap: break-word;">{{ $license->serial }}</p>
                         </div>
                     </div>
 
@@ -54,7 +54,7 @@
                     <div class="form-group {{ $errors->has('note') ? 'error' : '' }}">
                         <label for="note" class="col-md-3 control-label">{{ trans('admin/hardware/form.notes') }}</label>
                         <div class="col-md-7">
-                            <textarea class="col-md-6 form-control" id="note" name="note">{{ Input::old('note', $licenseSeat->note) }}</textarea>
+                            <textarea class="col-md-6 form-control" id="note" name="note">{{ Input::old('note') }}</textarea>
                             {!! $errors->first('note', '<span class="alert-msg"><i class="fa fa-times"></i> :message</span>') !!}
                         </div>
                     </div>