WIP: Fixed stale pending acceptances on checkin and transfer, and reconcile accessory quantities

app/Http/Controllers/Users/UserItemTransferController.php

@@ -10,7 +10,6 @@
 use App\Models\AccessoryCheckout;
 use App\Models\Asset;
 use App\Models\CheckoutAcceptance;
-use App\Models\License;
 use App\Models\LicenseSeat;
 use App\Models\User;
 use Illuminate\Contracts\View\View;
@@ -224,13 +223,9 @@ private function checkInAccessory(AccessoryCheckout $checkout, Accessory $access
     {
         $source = $checkout->assignedTo;
 
-        CheckoutAcceptance::pending()
-            ->where('checkoutable_type', Accessory::class)
-            ->where('checkoutable_id', $accessory->id)
-            ->where('assigned_to_id', $checkout->assigned_to)
-            ->get()
-            ->each(fn ($a) => $a->delete());
-
+        // You might think you need to clean up acceptances here but that
+        // is going to be handled in the CheckoutableListener that
+        // is run when the event below is fired.
         $checkout->delete();
 
         event(new CheckoutableCheckedIn($accessory, $source, auth()->user(), $note, date('Y-m-d H:i:s')));
@@ -253,8 +248,8 @@ private function checkOutAccessory(Accessory $accessory, User $target, ?string $
     private function transferLicenseSeat(LicenseSeat $seat, User $source, User $target, ?string $note): void
     {
         CheckoutAcceptance::pending()
-            ->where('checkoutable_type', License::class)
-            ->where('checkoutable_id', $seat->license_id)
+            ->where('checkoutable_type', LicenseSeat::class)
+            ->where('checkoutable_id', $seat->id)
             ->where('assigned_to_id', $source->id)
             ->get()
             ->each(fn ($a) => $a->delete());

app/Listeners/CheckoutableListener.php

@@ -34,6 +34,7 @@
 use App\Notifications\CheckoutLicenseSeatNotification;
 use Exception;
 use GuzzleHttp\Exception\ClientException;
+use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Notifications\Notification as BaseNotification;
 use Illuminate\Support\Facades\Context;
@@ -172,6 +173,10 @@ public function onCheckedIn($event)
     {
         Log::debug('onCheckedIn in the Checkoutable listener fired');
 
+        if ($event->checkedOutTo instanceof User && $event->checkoutable) {
+            $this->retirePendingAcceptances($event->checkoutable, $event->checkedOutTo);
+        }
+
         if ($this->shouldNotSendAnyNotifications($event->checkoutable)) {
             return;
         }
@@ -187,18 +192,6 @@ public function onCheckedIn($event)
             /**
              * Send the appropriate notification
              */
-            if ($event->checkedOutTo && $event->checkoutable) {
-                $acceptances = CheckoutAcceptance::where('checkoutable_id', $event->checkoutable->id)
-                    ->where('assigned_to_id', $event->checkedOutTo->id)
-                    ->get();
-
-                foreach ($acceptances as $acceptance) {
-                    if ($acceptance->isPending()) {
-                        $acceptance->delete();
-                    }
-                }
-            }
-
             $mailable = $this->getCheckinMailType($event);
             $notifiable = $this->getNotifiableUser($event);
 
@@ -270,6 +263,63 @@ public function onCheckedIn($event)
         }
     }
 
+    /**
+     * Clear the holder's outstanding acceptance requests for checked-in item.
+     *
+     * Assets and license seats are 1:1 with their acceptance rows. Accessories
+     * are not: accessories_checkout holds one row per unit while an acceptance
+     * row covers a whole checkout action and carries its qty, so checking one
+     * unit in retires one unit rather than a row that may be worth three.
+     *
+     * Only ever called for a User holder. Acceptances are created for users
+     * alone, so assigned_to_id holds a user id — matching a Location or Asset
+     * id against it would clear a different holder's rows by collision.
+     */
+    private function retirePendingAcceptances(Model $checkoutable, User $checkedOutTo): void
+    {
+        $acceptances = CheckoutAcceptance::pending()
+            ->where('checkoutable_type', $checkoutable->getMorphClass())
+            ->where('checkoutable_id', $checkoutable->getKey())
+            ->where('assigned_to_id', $checkedOutTo->id)
+            ->orderBy('id')
+            ->get();
+
+        if ($checkoutable instanceof Accessory) {
+            $this->retireOneUnitOfPendingQty($acceptances);
+
+            return;
+        }
+
+        $acceptances->each(fn (CheckoutAcceptance $acceptance) => $acceptance->delete());
+    }
+
+    /**
+     * Retire one unit from the oldest pending row, deleting it at zero.
+     *
+     * Accessory units are fungible — no serial, no tag — so there is no fact
+     * about which unit came back; a checkin is defined to retire an unaccepted
+     * one, and to do nothing when none are left.
+     *
+     * @param  Collection<int, CheckoutAcceptance>  $acceptances
+     */
+    private function retireOneUnitOfPendingQty($acceptances): void
+    {
+        $acceptance = $acceptances->first();
+
+        if (! $acceptance) {
+            return;
+        }
+
+        // Null qty means one unit, as in AcceptanceController and LogListener.
+        if (($acceptance->qty ?? 1) <= 1) {
+            $acceptance->delete();
+
+            return;
+        }
+
+        $acceptance->decrement('qty');
+    }
+
     /**
      * Generates a checkout acceptance
      *

database/factories/CategoryFactory.php

@@ -207,6 +207,13 @@ public function forConsumables()
         ]);
     }
 
+    public function requiresAcceptance()
+    {
+        return $this->state([
+            'require_acceptance' => true,
+        ]);
+    }
+
     public function doesNotRequireAcceptance()
     {
         return $this->state([

database/factories/CheckoutAcceptanceFactory.php

@@ -5,6 +5,7 @@
 use App\Models\Accessory;
 use App\Models\Asset;
 use App\Models\CheckoutAcceptance;
+use App\Models\LicenseSeat;
 use App\Models\User;
 use Illuminate\Database\Eloquent\Factories\Factory;
 
@@ -64,6 +65,14 @@ public function forAccessory()
         ]);
     }
 
+    public function forLicenseSeat()
+    {
+        return $this->state([
+            'checkoutable_type' => LicenseSeat::class,
+            'checkoutable_id' => LicenseSeat::factory(),
+        ]);
+    }
+
     public function pending()
     {
         return $this->state([
@@ -80,6 +89,14 @@ public function accepted()
         ]);
     }
 
+    public function declined()
+    {
+        return $this->state([
+            'accepted_at' => null,
+            'declined_at' => now()->subDay(),
+        ]);
+    }
+
     public function withoutAlerting()
     {
         return $this->state(function () {

database/migrations/2026_08_24_234955_clean_stale_pending_acceptances.php

@@ -0,0 +1,178 @@
+<?php
+
+use App\Models\Accessory;
+use App\Models\Asset;
+use App\Models\Consumable;
+use App\Models\LicenseSeat;
+use App\Models\User;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Query\Builder;
+use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\DB;
+
+/**
+ * Soft-deletes pending acceptance requests that no longer correspond to
+ * anything the holder has, cleaning up after the checkin/transfer bugs fixed
+ * alongside this. Pendings for units still held are left alone.
+ */
+return new class extends Migration
+{
+    public function up(): void
+    {
+        $clearedAt = now();
+
+        // Assets and license seats are 1:1 with their acceptance rows, so a
+        // boolean "still holds it" test is enough. A trashed item does not
+        // count: a restored asset comes back unassigned and licenses have no
+        // restore path, so the pending could never become actionable.
+        $this->clean(Asset::class, $clearedAt, function (Builder $query) {
+            $query->from('assets')
+                ->whereColumn('assets.id', 'checkout_acceptances.checkoutable_id')
+                ->whereColumn('assets.assigned_to', 'checkout_acceptances.assigned_to_id')
+                ->where('assets.assigned_type', User::class)
+                ->whereNull('assets.deleted_at');
+        });
+
+        $this->clean(LicenseSeat::class, $clearedAt, function (Builder $query) {
+            $query->from('license_seats')
+                ->whereColumn('license_seats.id', 'checkout_acceptances.checkoutable_id')
+                ->whereColumn('license_seats.assigned_to', 'checkout_acceptances.assigned_to_id')
+                ->whereNull('license_seats.deleted_at');
+        });
+
+        // Accessories and consumables are not 1:1: their pivots hold one row
+        // per unit, so a held/not-held test cannot see "holds one unit, has
+        // pendings worth three".
+        $this->reconcile(Accessory::class, $clearedAt, 'accessories_checkout', 'accessory_id', pivotHasAssignedType: true);
+        $this->reconcile(Consumable::class, $clearedAt, 'consumables_users', 'consumable_id', pivotHasAssignedType: false);
+    }
+
+    public function down(): void
+    {
+        // No-op. These rows were already meaningless when this migration ran,
+        // and restoring them is not possible anyway: there is no way to tell
+        // them apart from acceptances soft-deleted by a normal checkin.
+    }
+
+    /**
+     * Soft-delete every pending acceptance of one 1:1 type whose holder no
+     * longer has the item. `$stillHolds` receives a subquery that should match
+     * a row proving the pair still holds it; anything unmatched gets cleared.
+     *
+     * The `deleted_at` filter is what makes re-running this a no-op.
+     */
+    private function clean(string $checkoutableType, DateTimeInterface $clearedAt, Closure $stillHolds): void
+    {
+        DB::table('checkout_acceptances')
+            ->select('id')
+            ->where('checkoutable_type', $checkoutableType)
+            ->whereNull('deleted_at')
+            ->whereNull('accepted_at')
+            ->whereNull('declined_at')
+            ->whereNotExists($stillHolds)
+            ->chunkById(500, function (Collection $acceptances) use ($clearedAt) {
+                DB::table('checkout_acceptances')
+                    ->whereIn('id', $acceptances->pluck('id'))
+                    ->update(['deleted_at' => $clearedAt]);
+            });
+    }
+
+    /**
+     * Bring each pair's total pending qty down to the units it actually holds.
+     *
+     * Excess is retired oldest row first: a row is soft-deleted when all of it
+     * is excess, decremented when only part is. A pair holding nothing loses
+     * every row, so this is a superset of the boolean pass, not a departure.
+     * Re-running computes zero excess and writes nothing.
+     */
+    private function reconcile(
+        string $checkoutableType,
+        DateTimeInterface $clearedAt,
+        string $pivotTable,
+        string $itemColumn,
+        bool $pivotHasAssignedType,
+    ): void {
+        foreach ($this->pairsWithPendingAcceptances($checkoutableType) as $pair) {
+            $acceptances = $this->pendingAcceptancesFor($checkoutableType, $pair);
+
+            $unitsHeld = DB::table($pivotTable)
+                ->where($itemColumn, $pair->checkoutable_id)
+                ->where('assigned_to', $pair->assigned_to_id)
+                ->when($pivotHasAssignedType, fn ($query) => $query->where('assigned_type', User::class))
+                ->count();
+
+            $excess = $this->totalQty($acceptances) - $unitsHeld;
+
+            foreach ($acceptances as $acceptance) {
+                if ($excess <= 0) {
+                    break;
+                }
+
+                $qty = $acceptance->qty ?? 1;
+
+                if ($qty <= $excess) {
+                    DB::table('checkout_acceptances')
+                        ->where('id', $acceptance->id)
+                        ->update(['deleted_at' => $clearedAt]);
+
+                    $excess -= $qty;
+
+                    continue;
+                }
+
+                DB::table('checkout_acceptances')
+                    ->where('id', $acceptance->id)
+                    ->update(['qty' => $qty - $excess]);
+
+                $excess = 0;
+            }
+        }
+    }
+
+    /**
+     * The distinct (item, holder) pairs with anything pending. Grouping keeps
+     * this to one row per pair rather than loading the whole table.
+     *
+     * @return Collection<int, stdClass>
+     */
+    private function pairsWithPendingAcceptances(string $checkoutableType): Collection
+    {
+        return DB::table('checkout_acceptances')
+            ->select('checkoutable_id', 'assigned_to_id')
+            ->where('checkoutable_type', $checkoutableType)
+            ->whereNull('deleted_at')
+            ->whereNull('accepted_at')
+            ->whereNull('declined_at')
+            ->groupBy('checkoutable_id', 'assigned_to_id')
+            ->get();
+    }
+
+    /**
+     * @return Collection<int, stdClass>
+     */
+    private function pendingAcceptancesFor(string $checkoutableType, stdClass $pair): Collection
+    {
+        return DB::table('checkout_acceptances')
+            ->select('id', 'qty')
+            ->where('checkoutable_type', $checkoutableType)
+            ->where('checkoutable_id', $pair->checkoutable_id)
+            ->where('assigned_to_id', $pair->assigned_to_id)
+            ->whereNull('deleted_at')
+            ->whereNull('accepted_at')
+            ->whereNull('declined_at')
+            ->orderBy('id')
+            ->get();
+    }
+
+    /**
+     * Null qty means one unit, as in AcceptanceController and LogListener.
+     * Rows predating the qty column, and every asset and seat acceptance,
+     * carry null.
+     *
+     * @param  Collection<int, stdClass>  $acceptances
+     */
+    private function totalQty(Collection $acceptances): int
+    {
+        return (int) $acceptances->sum(fn (stdClass $acceptance) => $acceptance->qty ?? 1);
+    }
+};

tests/Feature/Checkins/Api/AccessoryCheckinTest.php

@@ -134,11 +134,17 @@ public function test_checkin_sends_checkin_email_to_user_when_category_enables_i
         );
     }
 
-    public function test_checkin_clears_pending_acceptance()
+    public function test_checkin_clears_pending_acceptance_when_notifications_are_fully_disabled()
     {
+        $this->settings->disableAdminCC()->disableAdminCCAlways()->disableSlackWebhook();
+
         $user = User::factory()->create();
         $accessory = Accessory::factory()->checkedOutToUser($user)->create();
-        $accessory->category->update(['require_acceptance' => true, 'checkin_email' => true]);
+
+        $accessory->category->update([
+            'require_acceptance' => true,
+            'checkin_email' => false,
+        ]);
 
         $checkout = $accessory->checkouts()
             ->where('assigned_type', User::class)