Guard against duplicate checkout requests

app/Actions/CheckoutRequests/CancelCheckoutRequestAction.php

@@ -2,6 +2,7 @@
 
 namespace App\Actions\CheckoutRequests;
 
+use App\Exceptions\NoActiveCheckoutRequest;
 use App\Models\Actionlog;
 use App\Models\Asset;
 use App\Models\Company;
@@ -9,18 +10,39 @@ use App\Models\Setting;
 use App\Models\User;
 use App\Notifications\RequestAssetCancelation;
 use Illuminate\Auth\Access\AuthorizationException;
+use Illuminate\Support\Facades\DB;
 
 class CancelCheckoutRequestAction
 {
+    /**
+     * @throws AuthorizationException
+     * @throws NoActiveCheckoutRequest
+     */
     public static function run(Asset $asset, User $user)
     {
         if (! Company::isCurrentUserHasAccess($asset)) {
             throw new AuthorizationException;
         }
 
-        $asset->cancelRequest();
+        // Cancel + counter decrement share a transaction, and the
+        // decrement is gated on the actual affected row count.
+        // Previously this method unconditionally decremented by 1
+        // regardless of whether the caller had an active request,
+        // driving the shared requests_counter negative on no-op calls
+        // and letting a duplicate-request cancel decrement by less
+        // than the number of rows it actually canceled.
+        $affected = DB::transaction(function () use ($asset) {
+            $affected = $asset->cancelRequest();
+            if ($affected > 0) {
+                $asset->decrement('requests_counter', $affected);
+            }
 
-        $asset->decrement('requests_counter', 1);
+            return $affected;
+        });
+
+        if ($affected === 0) {
+            throw new NoActiveCheckoutRequest;
+        }
 
         $data['item'] = $asset;
         $data['target'] = $user;

app/Actions/CheckoutRequests/CreateCheckoutRequestAction.php

@@ -3,6 +3,7 @@
 namespace App\Actions\CheckoutRequests;
 
 use App\Exceptions\AssetNotRequestable;
+use App\Exceptions\DuplicateCheckoutRequest;
 use App\Models\Actionlog;
 use App\Models\Asset;
 use App\Models\Company;
@@ -10,6 +11,7 @@ use App\Models\Setting;
 use App\Models\User;
 use App\Notifications\RequestAssetNotification;
 use Illuminate\Auth\Access\AuthorizationException;
+use Illuminate\Support\Facades\DB;
 use Log;
 
 class CreateCheckoutRequestAction
@@ -17,6 +19,7 @@ class CreateCheckoutRequestAction
     /**
      * @throws AssetNotRequestable
      * @throws AuthorizationException
+     * @throws DuplicateCheckoutRequest
      */
     public static function run(Asset $asset, User $user): string
     {
@@ -27,6 +30,15 @@ class CreateCheckoutRequestAction
             throw new AuthorizationException;
         }
 
+        // Enforce single-active-request-per-user-per-asset. Without this
+        // gate the same POST fires repeatedly, each firing adds an
+        // active CheckoutRequest row AND bumps requests_counter, but a
+        // single cancellation only decrements the counter by 1, so the
+        // counter and admin queue drift apart.
+        if ($asset->isRequestedBy($user)) {
+            throw new DuplicateCheckoutRequest;
+        }
+
         $data['item'] = $asset;
         $data['target'] = $user;
         $data['item_quantity'] = 1;
@@ -41,8 +53,14 @@ class CreateCheckoutRequestAction
         $logaction->location_id = $user->location_id ?? null;
         $logaction->logaction('requested');
 
-        $asset->request();
-        $asset->increment('requests_counter', 1);
+        // Row write + counter increment share one transaction so a
+        // partial failure can't leave the counter incremented without a
+        // matching row (or vice versa).
+        DB::transaction(function () use ($asset) {
+            $asset->request();
+            $asset->increment('requests_counter', 1);
+        });
+
         try {
             $settings->notify((new RequestAssetNotification($data))->locale($settings->locale));
         } catch (\Exception $e) {

app/Exceptions/DuplicateCheckoutRequest.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+
+/**
+ * Thrown by CreateCheckoutRequestAction when the caller already has an
+ * active (not-yet-canceled) CheckoutRequest for the same requestable.
+ * Enforces the one-active-request-per-user-per-asset invariant so the
+ * shared requests_counter and admin queue stay 1:1 with real active
+ * rows.
+ */
+class DuplicateCheckoutRequest extends Exception {}

app/Exceptions/NoActiveCheckoutRequest.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+
+/**
+ * Thrown by CancelCheckoutRequestAction when the caller has no active
+ * (not-yet-canceled) CheckoutRequest to cancel. Prevents the shared
+ * requests_counter from being decremented on no-op cancellations,
+ * which used to drive it negative and out of sync with the admin
+ * queue.
+ */
+class NoActiveCheckoutRequest extends Exception {}

app/Http/Controllers/Api/CheckoutRequest.php

@@ -5,6 +5,8 @@ namespace App\Http\Controllers\Api;
 use App\Actions\CheckoutRequests\CancelCheckoutRequestAction;
 use App\Actions\CheckoutRequests\CreateCheckoutRequestAction;
 use App\Exceptions\AssetNotRequestable;
+use App\Exceptions\DuplicateCheckoutRequest;
+use App\Exceptions\NoActiveCheckoutRequest;
 use App\Helpers\Helper;
 use App\Http\Controllers\Controller;
 use App\Models\Asset;
@@ -22,6 +24,11 @@ class CheckoutRequest extends Controller
             return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.requests.success')));
         } catch (AssetNotRequestable $e) {
             return response()->json(Helper::formatStandardApiResponse('error', 'Asset is not requestable'));
+        } catch (DuplicateCheckoutRequest $e) {
+            return response()->json(
+                Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.requests.duplicate')),
+                409,
+            );
         } catch (AuthorizationException $e) {
             return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.insufficient_permissions')));
         } catch (Exception $e) {
@@ -37,6 +44,11 @@ class CheckoutRequest extends Controller
             CancelCheckoutRequestAction::run($asset, auth()->user());
 
             return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.requests.canceled')));
+        } catch (NoActiveCheckoutRequest $e) {
+            return response()->json(
+                Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.requests.no_active')),
+                404,
+            );
         } catch (AuthorizationException $e) {
             return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.insufficient_permissions')));
         } catch (Exception $e) {

app/Http/Controllers/ViewAssetsController.php

@@ -268,6 +268,8 @@ class ViewAssetsController extends Controller
             return redirect()->route('requestable-assets')->with('success')->with('success', trans('admin/hardware/message.requests.success'));
         } catch (AssetNotRequestable $e) {
             return redirect()->back()->with('error', 'Asset is not requestable');
+        } catch (\App\Exceptions\DuplicateCheckoutRequest $e) {
+            return redirect()->back()->with('error', trans('admin/hardware/message.requests.duplicate'));
         } catch (AuthorizationException $e) {
             return redirect()->back()->with('error', trans('admin/hardware/message.requests.error'));
         } catch (Exception $e) {
@@ -283,6 +285,8 @@ class ViewAssetsController extends Controller
             CancelCheckoutRequestAction::run($asset, auth()->user());
 
             return redirect()->route('requestable-assets')->with('success')->with('success', trans('admin/hardware/message.requests.canceled'));
+        } catch (\App\Exceptions\NoActiveCheckoutRequest $e) {
+            return redirect()->back()->with('error', trans('admin/hardware/message.requests.no_active'));
         } catch (Exception $e) {
             report($e);
 

app/Models/Traits/Requestable.php

@@ -18,7 +18,11 @@ trait Requestable
 
     public function isRequestedBy(User $user)
     {
-        return $this->requests->where('canceled_at', null)->where('user_id', $user->id)->first();
+        // Fresh query rather than filtering the loaded ->requests
+        // collection so a same-request-cycle check-then-cancel sees
+        // current DB state instead of a possibly stale eager-loaded
+        // snapshot.
+        return $this->requests()->whereNull('canceled_at')->where('user_id', $user->id)->first();
     }
 
     public function scopeRequestedBy($query, User $user)
@@ -42,12 +46,24 @@ trait Requestable
         $this->requests()->where('user_id', auth()->id())->delete();
     }
 
-    public function cancelRequest($user_id = null)
+    /**
+     * Mark every active CheckoutRequest for $user_id (or the current
+     * auth user) as canceled. Returns the number of rows actually
+     * flipped so callers can gate side effects (counter decrement,
+     * notifications, log entries) on real work happening. The
+     * whereNull filter prevents already-canceled rows from getting
+     * their canceled_at bumped, and prevents no-op cancellations from
+     * looking like real events downstream.
+     */
+    public function cancelRequest($user_id = null): int
     {
         if (! $user_id) {
             $user_id = auth()->id();
         }
 
-        $this->requests()->where('user_id', $user_id)->update(['canceled_at' => Carbon::now()]);
+        return $this->requests()
+            ->where('user_id', $user_id)
+            ->whereNull('canceled_at')
+            ->update(['canceled_at' => Carbon::now()]);
     }
 }

resources/lang/en-US/admin/hardware/message.php

@@ -173,6 +173,8 @@ return [
         'success' => 'Request successfully submitted.',
         'canceled' => 'Request successfully canceled.',
         'cancel' => 'Cancel this item request',
+        'duplicate' => 'You already have an active request for this item.',
+        'no_active' => 'You have no active request to cancel for this item.',
     ],
 
 ];