Skip to content

Commit

Permalink
Merge pull request #15221 from marcusmoore/fixes/bulk-checkin-logging
Browse files Browse the repository at this point in the history 
Fixed accessories and consumables not being logged correctly during bulk check-in
  • Loading branch information
@snipe
snipe committed Aug 8, 2024
2 parents cc1e356 + 4a2d2f7 commit f76fbe7
Show file tree
Hide file tree
Showing 2 changed files with 295 additions and 13 deletions.
48 changes: 35 additions & 13 deletions app/Http/Controllers/Users/BulkUsersController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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')),
Expand All @@ -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');
Expand All @@ -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;
Expand All @@ -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
*
Expand Down
260 changes: 260 additions & 0 deletions tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
<?php

namespace Tests\Feature\Users\Ui\BulkActions;

use App\Models\Accessory;
use App\Models\Asset;
use App\Models\Consumable;
use App\Models\License;
use App\Models\LicenseSeat;
use App\Models\Statuslabel;
use App\Models\User;
use Illuminate\Database\Eloquent\Model;
use Tests\TestCase;

class BulkDeleteUsersTest extends TestCase
{
public function test_requires_correct_permission()
{
$this->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,
]);
}
}

0 comments on commit f76fbe7

Please sign in to comment.