diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 489b2309bf77..1a8f84b7a1af 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -16,6 +16,7 @@ use App\Models\User; use Carbon\Carbon; use Illuminate\Http\Request; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Password; @@ -219,20 +220,18 @@ public function destroy(Request $request) $users = User::whereIn('id', $user_raw_array)->get(); $assets = Asset::whereIn('assigned_to', $user_raw_array)->where('assigned_type', User::class)->get(); - $accessories = DB::table('accessories_checkout')->where('assigned_type', User::class)->whereIn('assigned_to', $user_raw_array)->get(); + $accessoryUserRows = DB::table('accessories_checkout')->where('assigned_type', User::class)->whereIn('assigned_to', $user_raw_array)->get(); $licenses = DB::table('license_seats')->whereIn('assigned_to', $user_raw_array)->get(); - $consumables = DB::table('consumables_users')->whereIn('assigned_to', $user_raw_array)->get(); + $consumableUserRows = DB::table('consumables_users')->whereIn('assigned_to', $user_raw_array)->get(); if ((($assets->count() > 0) && ((!$request->filled('status_id')) || ($request->input('status_id') == '')))) { return redirect()->route('users.index')->with('error', 'No status selected'); } - $this->logItemCheckinAndDelete($assets, Asset::class); - $this->logItemCheckinAndDelete($accessories, Accessory::class); + $this->logAccessoriesCheckin($accessoryUserRows); $this->logItemCheckinAndDelete($licenses, License::class); - $this->logItemCheckinAndDelete($consumables, Consumable::class); - + $this->logConsumablesCheckin($consumableUserRows); Asset::whereIn('id', $assets->pluck('id'))->update([ 'status_id' => e(request('status_id')), @@ -241,19 +240,14 @@ public function destroy(Request $request) 'expected_checkin' => null, ]); - LicenseSeat::whereIn('id', $licenses->pluck('id'))->update(['assigned_to' => null]); - ConsumableAssignment::whereIn('id', $consumables->pluck('id'))->delete(); - + ConsumableAssignment::whereIn('id', $consumableUserRows->pluck('id'))->delete(); foreach ($users as $user) { - - $user->consumables()->sync([]); $user->accessories()->sync([]); if ($request->input('delete_user')=='1') { $user->delete(); } - } $msg = trans('general.bulk_checkin_success'); @@ -279,7 +273,7 @@ protected function logItemCheckinAndDelete($items, $itemType) if ($itemType == License::class){ $item_id = $item->license_id; } - + $logAction->item_id = $item_id; // We can't rely on get_class here because the licenses/accessories fetched above are not eloquent models, but simply arrays. $logAction->item_type = $itemType; @@ -291,6 +285,34 @@ protected function logItemCheckinAndDelete($items, $itemType) } } + private function logAccessoriesCheckin(Collection $accessoryUserRows): void + { + foreach ($accessoryUserRows as $accessoryUserRow) { + $logAction = new Actionlog(); + $logAction->item_id = $accessoryUserRow->accessory_id; + $logAction->item_type = Accessory::class; + $logAction->target_id = $accessoryUserRow->assigned_to; + $logAction->target_type = User::class; + $logAction->user_id = Auth::id(); + $logAction->note = 'Bulk checkin items'; + $logAction->logaction('checkin from'); + } + } + + 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->user_id = 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 new file mode 100644 index 000000000000..46ba023c1bef --- /dev/null +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -0,0 +1,260 @@ +actingAs(User::factory()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + User::factory()->create()->id, + ], + 'status_id' => Statuslabel::factory()->create()->id, + ]) + ->assertForbidden(); + } + + public function test_validation() + { + $user = User::factory()->create(); + Asset::factory()->assignedToUser($user)->create(); + + $actor = $this->actingAs(User::factory()->editUsers()->create()); + + // "ids" required + $actor->post(route('users/bulksave'), [ + // 'ids' => [ + // $user->id, + // ], + 'status_id' => Statuslabel::factory()->create()->id, + ])->assertSessionHas('error')->assertRedirect(); + + // "status_id" needed when provided users have assets associated + $actor->post(route('users/bulksave'), [ + 'ids' => [ + $user->id, + ], + // 'status_id' => Statuslabel::factory()->create()->id, + ])->assertSessionHas('error')->assertRedirect(); + } + + public function test_cannot_perform_bulk_actions_on_self() + { + $actor = User::factory()->editUsers()->create(); + + $this->actingAs($actor) + ->post(route('users/bulksave'), [ + 'ids' => [ + $actor->id, + ], + 'delete_user' => '1', + ]) + ->assertRedirect(route('users.index')) + ->assertSessionHas('success', trans('general.bulk_checkin_delete_success')); + + $this->assertNotSoftDeleted($actor); + } + + public function test_accessories_can_be_bulk_checked_in() + { + [$accessoryA, $accessoryB] = Accessory::factory()->count(2)->create(); + [$userA, $userB, $userC] = User::factory()->count(3)->create(); + + // Add checkouts for multiple accessories to multiple users to get different ids in the mix + $this->attachAccessoryToUsers($accessoryA, [$userA, $userB, $userC]); + $this->attachAccessoryToUsers($accessoryB, [$userA, $userB]); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + $userC->id, + ], + 'status_id' => Statuslabel::factory()->create()->id, + ]) + ->assertRedirect(route('users.index')); + + $this->assertTrue($userA->fresh()->accessories->isEmpty()); + $this->assertTrue($userB->fresh()->accessories->isNotEmpty()); + $this->assertTrue($userC->fresh()->accessories->isEmpty()); + + // These assertions check against a bug where the wrong value from + // accessories_users was being populated in action_logs.item_id. + $this->assertActionLogCheckInEntryFor($userA, $accessoryA); + $this->assertActionLogCheckInEntryFor($userA, $accessoryB); + $this->assertActionLogCheckInEntryFor($userC, $accessoryA); + } + + public function test_assets_can_be_bulk_checked_in() + { + [$userA, $userB, $userC] = User::factory()->count(3)->create(); + + $assetForUserA = $this->assignAssetToUser($userA); + $lonelyAsset = $this->assignAssetToUser($userB); + $assetForUserC = $this->assignAssetToUser($userC); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + $userC->id, + ], + 'status_id' => Statuslabel::factory()->create()->id, + ]) + ->assertRedirect(route('users.index')); + + $this->assertTrue($userA->fresh()->assets->isEmpty()); + $this->assertTrue($userB->fresh()->assets->isNotEmpty()); + $this->assertTrue($userC->fresh()->assets->isEmpty()); + + $this->assertActionLogCheckInEntryFor($userA, $assetForUserA); + $this->assertActionLogCheckInEntryFor($userC, $assetForUserC); + } + + public function test_consumables_can_be_bulk_checked_in() + { + [$consumableA, $consumableB] = Consumable::factory()->count(2)->create(); + [$userA, $userB, $userC] = User::factory()->count(3)->create(); + + // Add checkouts for multiple consumables to multiple users to get different ids in the mix + $this->attachConsumableToUsers($consumableA, [$userA, $userB, $userC]); + $this->attachConsumableToUsers($consumableB, [$userA, $userB]); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + $userC->id, + ], + 'status_id' => Statuslabel::factory()->create()->id, + ]) + ->assertRedirect(route('users.index')); + + $this->assertTrue($userA->fresh()->consumables->isEmpty()); + $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); + } + + public function test_license_seats_can_be_bulk_checked_in() + { + [$userA, $userB, $userC] = User::factory()->count(3)->create(); + + $licenseSeatForUserA = LicenseSeat::factory()->assignedToUser($userA)->create(); + $lonelyLicenseSeat = LicenseSeat::factory()->assignedToUser($userB)->create(); + $licenseSeatForUserC = LicenseSeat::factory()->assignedToUser($userC)->create(); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + $userC->id, + ], + ]) + ->assertRedirect(route('users.index')) + ->assertSessionHas('success', trans('general.bulk_checkin_success')); + + $this->assertDatabaseMissing('license_seats', [ + 'license_id' => $licenseSeatForUserA->license->id, + 'assigned_to' => $userA->id, + ]); + + $this->assertDatabaseMissing('license_seats', [ + 'license_id' => $licenseSeatForUserC->license->id, + 'assigned_to' => $userC->id, + ]); + + // Slightly different from the other assertions since we use + // the license and not the license seat in this case. + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userA->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => License::class, + 'item_id' => $licenseSeatForUserA->license->id, + ]); + + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userC->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => License::class, + 'item_id' => $licenseSeatForUserC->license->id, + ]); + } + + public function test_users_can_be_deleted_in_bulk() + { + [$userA, $userB, $userC] = User::factory()->count(3)->create(); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + $userC->id, + ], + 'delete_user' => '1', + ]) + ->assertRedirect(route('users.index')) + ->assertSessionHas('success', trans('general.bulk_checkin_delete_success')); + + $this->assertSoftDeleted($userA); + $this->assertNotSoftDeleted($userB); + $this->assertSoftDeleted($userC); + } + + private function assignAssetToUser(User $user): Asset + { + return Asset::factory()->assignedToUser($user)->create(); + } + + private function attachAccessoryToUsers(Accessory $accessory, array $users): void + { + foreach ($users as $user) { + $accessoryCheckout = $accessory->checkouts()->make(); + $accessoryCheckout->assignedTo()->associate($user); + $accessoryCheckout->save(); + } + } + + private function attachConsumableToUsers(Consumable $consumable, array $users): void + { + foreach ($users as $user) { + $consumable->users()->attach($consumable->id, [ + 'consumable_id' => $consumable->id, + 'assigned_to' => $user->id, + ]); + } + } + + private function assertActionLogCheckInEntryFor(User $user, Model $model): void + { + $this->assertDatabaseHas('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, + ]); + } +}