From be6caf936e4e4fbb288b99d7cb31ddc1aa367fb6 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 12 Mar 2025 11:48:14 -0700 Subject: [PATCH 1/2] Avoid logging consumable checkins --- .../Controllers/Users/BulkUsersController.php | 15 ------------- .../Ui/BulkActions/BulkDeleteUsersTest.php | 21 ++++++++++++++----- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 1909dd821..0269f3fd0 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -298,7 +298,6 @@ class BulkUsersController extends Controller $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 @@ class BulkUsersController extends Controller } } - 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 * diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index 46ba023c1..60b41ce61 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -146,11 +146,10 @@ class BulkDeleteUsersTest extends TestCase $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 @@ class BulkDeleteUsersTest extends TestCase '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, + ]); + } } From d5bc5caacd09833497fb2d7455bb4f1f654761df Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 12 Mar 2025 11:57:18 -0700 Subject: [PATCH 2/2] Purge activity log of consumable bulk checkins --- ...gs_table_of_consumable_checkin_entries.php | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 database/migrations/2025_03_12_184851_purge_action_logs_table_of_consumable_checkin_entries.php diff --git a/database/migrations/2025_03_12_184851_purge_action_logs_table_of_consumable_checkin_entries.php b/database/migrations/2025_03_12_184851_purge_action_logs_table_of_consumable_checkin_entries.php new file mode 100644 index 000000000..6f2b1324b --- /dev/null +++ b/database/migrations/2025_03_12_184851_purge_action_logs_table_of_consumable_checkin_entries.php @@ -0,0 +1,30 @@ +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... + } +};