Merge pull request #16494 from marcusmoore/bug/sc-28675

Avoid logging consumable checkins and purge action log of bad entries

Author: snipe

app/Http/Controllers/Users/BulkUsersController.php

@@ -298,7 +298,6 @@ public function destroy(Request $request)
         $this->logItemCheckinAndDelete($assets, Asset::class);
         $this->logAccessoriesCheckin($accessoryUserRows);
         $this->logItemCheckinAndDelete($licenses, License::class);
-        $this->logConsumablesCheckin($consumableUserRows);
 
         Asset::whereIn('id', $assets->pluck('id'))->update([
             'status_id'     => e(request('status_id')),
@@ -366,20 +365,6 @@ private function logAccessoriesCheckin(Collection $accessoryUserRows): void
         }
     }
 
-    private function logConsumablesCheckin(Collection $consumableUserRows): void
-    {
-        foreach ($consumableUserRows as $consumableUserRow) {
-            $logAction = new Actionlog();
-            $logAction->item_id = $consumableUserRow->consumable_id;
-            $logAction->item_type = Consumable::class;
-            $logAction->target_id = $consumableUserRow->assigned_to;
-            $logAction->target_type = User::class;
-            $logAction->created_by = auth()->id();
-            $logAction->note = 'Bulk checkin items';
-            $logAction->logaction('checkin from');
-        }
-    }
-
     /**
      * Save bulk-edited users
      *

database/migrations/2025_03_12_184851_purge_action_logs_table_of_consumable_checkin_entries.php

@@ -0,0 +1,30 @@
+<?php
+
+use App\Models\Consumable;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Support\Facades\DB;
+
+return new class extends Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        DB::table('action_logs')
+            ->where([
+                'action_type' => 'checkin from',
+                'note' => 'Bulk checkin items',
+                'item_type' => Consumable::class,
+            ])
+            ->delete();
+    }
+
+    /**
+     * Reverse the migrations.
+     */
+    public function down(): void
+    {
+        // nothing to do here...
+    }
+};

tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php

@@ -146,11 +146,10 @@ public function test_consumables_can_be_bulk_checked_in()
         $this->assertTrue($userB->fresh()->consumables->isNotEmpty());
         $this->assertTrue($userC->fresh()->consumables->isEmpty());
 
-        // These assertions check against a bug where the wrong value from
-        // consumables_users was being populated in action_logs.item_id.
-        $this->assertActionLogCheckInEntryFor($userA, $consumableA);
-        $this->assertActionLogCheckInEntryFor($userA, $consumableB);
-        $this->assertActionLogCheckInEntryFor($userC, $consumableA);
+        // Consumable checkin should not be logged.
+        $this->assertNoActionLogCheckInEntryFor($userA, $consumableA);
+        $this->assertNoActionLogCheckInEntryFor($userA, $consumableB);
+        $this->assertNoActionLogCheckInEntryFor($userC, $consumableA);
     }
 
     public function test_license_seats_can_be_bulk_checked_in()
@@ -257,4 +256,16 @@ private function assertActionLogCheckInEntryFor(User $user, Model $model): void
             'item_id' => $model->id,
         ]);
     }
+
+    private function assertNoActionLogCheckInEntryFor(User $user, Model $model): void
+    {
+        $this->assertDatabaseMissing('action_logs', [
+            'action_type' => 'checkin from',
+            'target_id' => $user->id,
+            'target_type' => User::class,
+            'note' => 'Bulk checkin items',
+            'item_type' => get_class($model),
+            'item_id' => $model->id,
+        ]);
+    }
 }