Merge pull request #14260 from marcusmoore/chore/sc-24808

Added tests around asset check in and added missing actions to the api controller action
This commit is contained in:
snipe 2024-02-28 13:34:12 +00:00 committed by GitHub
commit a4941031cb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 460 additions and 81 deletions

View file

@ -4,6 +4,10 @@ namespace App\Http\Controllers\Api;
use App\Events\CheckoutableCheckedIn;
use App\Http\Requests\StoreAssetRequest;
use App\Http\Traits\MigratesLegacyAssetLocations;
use App\Models\CheckoutAcceptance;
use App\Models\LicenseSeat;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Http\JsonResponse;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Facades\Gate;
@ -45,6 +49,8 @@ use Route;
*/
class AssetsController extends Controller
{
use MigratesLegacyAssetLocations;
/**
* Returns JSON listing of all assets
*
@ -864,11 +870,9 @@ class AssetsController extends Controller
*/
public function checkin(Request $request, $asset_id)
{
$this->authorize('checkin', Asset::class);
$asset = Asset::with('model')->findOrFail($asset_id);
$this->authorize('checkin', $asset);
$target = $asset->assignedTo;
if (is_null($target)) {
return response()->json(Helper::formatStandardApiResponse('error', [
@ -881,7 +885,6 @@ class AssetsController extends Controller
$asset->expected_checkin = null;
//$asset->last_checkout = null;
$asset->last_checkin = now();
$asset->assigned_to = null;
$asset->assignedTo()->disassociate($asset);
$asset->accepted = null;
@ -889,10 +892,16 @@ class AssetsController extends Controller
$asset->name = $request->input('name');
}
$this->migrateLegacyLocations($asset);
$asset->location_id = $asset->rtd_location_id;
if ($request->filled('location_id')) {
$asset->location_id = $request->input('location_id');
if ($request->input('update_default_location')){
$asset->rtd_location_id = $request->input('location_id');
}
}
if ($request->has('status_id')) {
@ -906,12 +915,22 @@ class AssetsController extends Controller
$originalValues['action_date'] = $checkin_at;
}
if(!empty($asset->licenseseats->all())){
foreach ($asset->licenseseats as $seat){
$seat->assigned_to = null;
$seat->save();
}
}
$asset->licenseseats->each(function (LicenseSeat $seat) {
$seat->update(['assigned_to' => null]);
});
// Get all pending Acceptances for this asset and delete them
CheckoutAcceptance::pending()
->whereHasMorph(
'checkoutable',
[Asset::class],
function (Builder $query) use ($asset) {
$query->where('id', $asset->id);
})
->get()
->map(function ($acceptance) {
$acceptance->delete();
});
if ($asset->save()) {
event(new CheckoutableCheckedIn($asset, $target, Auth::user(), $request->input('note'), $checkin_at, $originalValues));

View file

@ -6,8 +6,10 @@ use App\Events\CheckoutableCheckedIn;
use App\Helpers\Helper;
use App\Http\Controllers\Controller;
use App\Http\Requests\AssetCheckinRequest;
use App\Http\Traits\MigratesLegacyAssetLocations;
use App\Models\Asset;
use App\Models\CheckoutAcceptance;
use App\Models\LicenseSeat;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Redirect;
@ -15,6 +17,8 @@ use Illuminate\Support\Facades\View;
class AssetCheckinController extends Controller
{
use MigratesLegacyAssetLocations;
/**
* Returns a view that presents a form to check an asset back into inventory.
*
@ -69,9 +73,7 @@ class AssetCheckinController extends Controller
$asset->expected_checkin = null;
//$asset->last_checkout = null;
$asset->last_checkin = now();
$asset->assigned_to = null;
$asset->assignedTo()->disassociate($asset);
$asset->assigned_type = null;
$asset->accepted = null;
$asset->name = $request->get('name');
@ -79,24 +81,7 @@ class AssetCheckinController extends Controller
$asset->status_id = e($request->get('status_id'));
}
// This is just meant to correct legacy issues where some user data would have 0
// as a location ID, which isn't valid. Later versions of Snipe-IT have stricter validation
// rules, so it's necessary to fix this for long-time users. It's kinda gross, but will help
// people (and their data) in the long run
if ($asset->rtd_location_id == '0') {
\Log::debug('Manually override the RTD location IDs');
\Log::debug('Original RTD Location ID: '.$asset->rtd_location_id);
$asset->rtd_location_id = '';
\Log::debug('New RTD Location ID: '.$asset->rtd_location_id);
}
if ($asset->location_id == '0') {
\Log::debug('Manually override the location IDs');
\Log::debug('Original Location ID: '.$asset->location_id);
$asset->location_id = '';
\Log::debug('New Location ID: '.$asset->location_id);
}
$this->migrateLegacyLocations($asset);
$asset->location_id = $asset->rtd_location_id;
@ -117,12 +102,9 @@ class AssetCheckinController extends Controller
$checkin_at = $request->get('checkin_at');
}
if(!empty($asset->licenseseats->all())){
foreach ($asset->licenseseats as $seat){
$seat->assigned_to = null;
$seat->save();
}
}
$asset->licenseseats->each(function (LicenseSeat $seat) {
$seat->update(['assigned_to' => null]);
});
// Get all pending Acceptances for this asset and delete them
$acceptances = CheckoutAcceptance::pending()->whereHasMorph('checkoutable',

View file

@ -0,0 +1,33 @@
<?php
namespace App\Http\Traits;
use App\Models\Asset;
trait MigratesLegacyAssetLocations
{
/**
* This is just meant to correct legacy issues where some user data would have 0
* as a location ID, which isn't valid. Later versions of Snipe-IT have stricter validation
* rules, so it's necessary to fix this for long-time users. It's kinda gross, but will help
* people (and their data) in the long run
* @param Asset $asset
* @return void
*/
private function migrateLegacyLocations(Asset $asset): void
{
if ($asset->rtd_location_id == '0') {
\Log::debug('Manually override the RTD location IDs');
\Log::debug('Original RTD Location ID: ' . $asset->rtd_location_id);
$asset->rtd_location_id = '';
\Log::debug('New RTD Location ID: ' . $asset->rtd_location_id);
}
if ($asset->location_id == '0') {
\Log::debug('Manually override the location IDs');
\Log::debug('Original Location ID: ' . $asset->location_id);
$asset->location_id = '';
\Log::debug('New Location ID: ' . $asset->location_id);
}
}
}

View file

@ -8,7 +8,6 @@ use App\Models\Location;
use App\Models\Statuslabel;
use App\Models\Supplier;
use App\Models\User;
use Carbon\Carbon;
use Carbon\CarbonImmutable;
use Illuminate\Database\Eloquent\Factories\Factory;
@ -289,12 +288,13 @@ class AssetFactory extends Factory
});
}
public function assignedToUser()
public function assignedToUser(User $user = null)
{
return $this->state(function () {
return $this->state(function () use ($user) {
return [
'assigned_to' => User::factory(),
'assigned_to' => $user->id ?? User::factory(),
'assigned_type' => User::class,
'last_checkout' => now()->subDay(),
];
});
}
@ -352,4 +352,19 @@ class AssetFactory extends Factory
{
return $this->state(['requestable' => false]);
}
/**
* This allows bypassing model level validation if you want to purposefully
* create an asset in an invalid state. Validation is turned back on
* after the model is created via the factory.
* @return AssetFactory
*/
public function canBeInvalidUponCreation()
{
return $this->afterMaking(function (Asset $asset) {
$asset->setValidating(false);
})->afterCreating(function (Asset $asset) {
$asset->setValidating(true);
});
}
}

View file

@ -3,6 +3,7 @@
namespace Database\Factories;
use App\Models\License;
use App\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
class LicenseSeatFactory extends Factory
@ -13,4 +14,13 @@ class LicenseSeatFactory extends Factory
'license_id' => License::factory(),
];
}
public function assignedToUser(User $user = null)
{
return $this->state(function () use ($user) {
return [
'assigned_to' => $user->id ?? User::factory(),
];
});
}
}

View file

@ -2,8 +2,15 @@
namespace Tests\Feature\Api\Assets;
use App\Events\CheckoutableCheckedIn;
use App\Models\Asset;
use App\Models\CheckoutAcceptance;
use App\Models\LicenseSeat;
use App\Models\Location;
use App\Models\Statuslabel;
use App\Models\User;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Event;
use Tests\Support\InteractsWithSettings;
use Tests\TestCase;
@ -11,20 +18,150 @@ class AssetCheckinTest extends TestCase
{
use InteractsWithSettings;
public function testLastCheckInFieldIsSetOnCheckin()
public function testCheckingInAssetRequiresCorrectPermission()
{
$admin = User::factory()->superuser()->create();
$asset = Asset::factory()->create(['last_checkin' => null]);
$this->actingAsForApi(User::factory()->create())
->postJson(route('api.asset.checkin', Asset::factory()->assignedToUser()->create()))
->assertForbidden();
}
$asset->checkOut(User::factory()->create(), $admin, now());
public function testCannotCheckInNonExistentAsset()
{
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', ['id' => 'does-not-exist']))
->assertStatusMessageIs('error');
}
$this->actingAsForApi($admin)
->postJson(route('api.asset.checkin', $asset))
public function testCannotCheckInAssetThatIsNotCheckedOut()
{
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', Asset::factory()->create()->id))
->assertStatusMessageIs('error');
}
public function testAssetCanBeCheckedIn()
{
Event::fake([CheckoutableCheckedIn::class]);
$user = User::factory()->create();
$location = Location::factory()->create();
$status = Statuslabel::factory()->create();
$asset = Asset::factory()->assignedToUser($user)->create([
'expected_checkin' => now()->addDay(),
'last_checkin' => null,
'accepted' => 'accepted',
]);
$this->assertTrue($asset->assignedTo->is($user));
$currentTimestamp = now();
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', $asset), [
'name' => 'Changed Name',
'status_id' => $status->id,
'location_id' => $location->id,
])
->assertOk();
$this->assertNotNull(
$asset->fresh()->last_checkin,
'last_checkin field should be set on checkin'
);
$this->assertNull($asset->refresh()->assignedTo);
$this->assertNull($asset->expected_checkin);
$this->assertNull($asset->last_checkout);
$this->assertNotNull($asset->last_checkin);
$this->assertNull($asset->assignedTo);
$this->assertNull($asset->assigned_type);
$this->assertNull($asset->accepted);
$this->assertEquals('Changed Name', $asset->name);
$this->assertEquals($status->id, $asset->status_id);
$this->assertTrue($asset->location()->is($location));
Event::assertDispatched(function (CheckoutableCheckedIn $event) use ($currentTimestamp) {
// this could be better mocked but is ok for now.
return Carbon::parse($event->action_date)->diffInSeconds($currentTimestamp) < 2;
}, 1);
}
public function testLocationIsSetToRTDLocationByDefaultUponCheckin()
{
$rtdLocation = Location::factory()->create();
$asset = Asset::factory()->assignedToUser()->create([
'location_id' => Location::factory()->create()->id,
'rtd_location_id' => $rtdLocation->id,
]);
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', $asset->id));
$this->assertTrue($asset->refresh()->location()->is($rtdLocation));
}
public function testDefaultLocationCanBeUpdatedUponCheckin()
{
$location = Location::factory()->create();
$asset = Asset::factory()->assignedToUser()->create();
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', $asset), [
'location_id' => $location->id,
'update_default_location' => true,
]);
$this->assertTrue($asset->refresh()->defaultLoc()->is($location));
}
public function testAssetsLicenseSeatsAreClearedUponCheckin()
{
$asset = Asset::factory()->assignedToUser()->create();
LicenseSeat::factory()->assignedToUser()->for($asset)->create();
$this->assertNotNull($asset->licenseseats->first()->assigned_to);
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', $asset));
$this->assertNull($asset->refresh()->licenseseats->first()->assigned_to);
}
public function testLegacyLocationValuesSetToZeroAreUpdated()
{
$asset = Asset::factory()->canBeInvalidUponCreation()->assignedToUser()->create([
'rtd_location_id' => 0,
'location_id' => 0,
]);
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', $asset));
$this->assertNull($asset->refresh()->rtd_location_id);
$this->assertEquals($asset->location_id, $asset->rtd_location_id);
}
public function testPendingCheckoutAcceptancesAreClearedUponCheckin()
{
$asset = Asset::factory()->assignedToUser()->create();
$acceptance = CheckoutAcceptance::factory()->for($asset, 'checkoutable')->pending()->create();
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', $asset));
$this->assertFalse($acceptance->exists(), 'Acceptance was not deleted');
}
public function testCheckinTimeAndActionLogNoteCanBeSet()
{
Event::fake();
$this->actingAsForApi(User::factory()->checkinAssets()->create())
->postJson(route('api.asset.checkin', Asset::factory()->assignedToUser()->create()), [
// time is appended to the provided date in controller
'checkin_at' => '2023-01-02',
'note' => 'hi there',
]);
Event::assertDispatched(function (CheckoutableCheckedIn $event) {
return Carbon::parse('2023-01-02')->isSameDay(Carbon::parse($event->action_date))
&& $event->note === 'hi there';
}, 1);
}
}

View file

@ -1,32 +0,0 @@
<?php
namespace Tests\Feature\Assets;
use App\Models\Asset;
use App\Models\User;
use Tests\Support\InteractsWithSettings;
use Tests\TestCase;
class AssetCheckinTest extends TestCase
{
use InteractsWithSettings;
public function testLastCheckInFieldIsSetOnCheckin()
{
$admin = User::factory()->superuser()->create();
$asset = Asset::factory()->create(['last_checkin' => null]);
$asset->checkOut(User::factory()->create(), $admin, now());
$this->actingAs($admin)
->post(route('hardware.checkin.store', [
'assetId' => $asset->id,
]))
->assertRedirect();
$this->assertNotNull(
$asset->fresh()->last_checkin,
'last_checkin field should be set on checkin'
);
}
}

View file

@ -0,0 +1,167 @@
<?php
namespace Tests\Feature\Checkins;
use App\Events\CheckoutableCheckedIn;
use App\Models\Asset;
use App\Models\CheckoutAcceptance;
use App\Models\LicenseSeat;
use App\Models\Location;
use App\Models\Statuslabel;
use App\Models\User;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Event;
use Tests\Support\InteractsWithSettings;
use Tests\TestCase;
class AssetCheckinTest extends TestCase
{
use InteractsWithSettings;
public function testCheckingInAssetRequiresCorrectPermission()
{
$this->actingAs(User::factory()->create())
->post(route('hardware.checkin.store', [
'assetId' => Asset::factory()->assignedToUser()->create()->id,
]))
->assertForbidden();
}
public function testCannotCheckInAssetThatIsNotCheckedOut()
{
$this->actingAs(User::factory()->checkinAssets()->create())
->post(route('hardware.checkin.store', ['assetId' => Asset::factory()->create()->id]))
->assertSessionHas('error')
->assertRedirect(route('hardware.index'));
}
public function testAssetCanBeCheckedIn()
{
Event::fake([CheckoutableCheckedIn::class]);
$user = User::factory()->create();
$location = Location::factory()->create();
$status = Statuslabel::first() ?? Statuslabel::factory()->create();
$asset = Asset::factory()->assignedToUser($user)->create([
'expected_checkin' => now()->addDay(),
'last_checkin' => null,
'accepted' => 'accepted',
]);
$this->assertTrue($asset->assignedTo->is($user));
$currentTimestamp = now();
$this->actingAs(User::factory()->checkinAssets()->create())
->post(
route('hardware.checkin.store', ['assetId' => $asset->id, 'backto' => 'user']),
[
'name' => 'Changed Name',
'status_id' => $status->id,
'location_id' => $location->id,
],
)
->assertRedirect(route('users.show', $user));
$this->assertNull($asset->refresh()->assignedTo);
$this->assertNull($asset->expected_checkin);
$this->assertNull($asset->last_checkout);
$this->assertNotNull($asset->last_checkin);
$this->assertNull($asset->assignedTo);
$this->assertNull($asset->assigned_type);
$this->assertNull($asset->accepted);
$this->assertEquals('Changed Name', $asset->name);
$this->assertEquals($status->id, $asset->status_id);
$this->assertTrue($asset->location()->is($location));
Event::assertDispatched(function (CheckoutableCheckedIn $event) use ($currentTimestamp) {
// this could be better mocked but is ok for now.
return Carbon::parse($event->action_date)->diffInSeconds($currentTimestamp) < 2;
}, 1);
}
public function testLocationIsSetToRTDLocationByDefaultUponCheckin()
{
$rtdLocation = Location::factory()->create();
$asset = Asset::factory()->assignedToUser()->create([
'location_id' => Location::factory()->create()->id,
'rtd_location_id' => $rtdLocation->id,
]);
$this->actingAs(User::factory()->checkinAssets()->create())
->post(route('hardware.checkin.store', ['assetId' => $asset->id]));
$this->assertTrue($asset->refresh()->location()->is($rtdLocation));
}
public function testDefaultLocationCanBeUpdatedUponCheckin()
{
$location = Location::factory()->create();
$asset = Asset::factory()->assignedToUser()->create();
$this->actingAs(User::factory()->checkinAssets()->create())
->post(route('hardware.checkin.store', ['assetId' => $asset->id]), [
'location_id' => $location->id,
'update_default_location' => 0
]);
$this->assertTrue($asset->refresh()->defaultLoc()->is($location));
}
public function testAssetsLicenseSeatsAreClearedUponCheckin()
{
$asset = Asset::factory()->assignedToUser()->create();
LicenseSeat::factory()->assignedToUser()->for($asset)->create();
$this->assertNotNull($asset->licenseseats->first()->assigned_to);
$this->actingAs(User::factory()->checkinAssets()->create())
->post(route('hardware.checkin.store', ['assetId' => $asset->id]));
$this->assertNull($asset->refresh()->licenseseats->first()->assigned_to);
}
public function testLegacyLocationValuesSetToZeroAreUpdated()
{
$asset = Asset::factory()->canBeInvalidUponCreation()->assignedToUser()->create([
'rtd_location_id' => 0,
'location_id' => 0,
]);
$this->actingAs(User::factory()->checkinAssets()->create())
->post(route('hardware.checkin.store', ['assetId' => $asset->id]));
$this->assertNull($asset->refresh()->rtd_location_id);
$this->assertEquals($asset->location_id, $asset->rtd_location_id);
}
public function testPendingCheckoutAcceptancesAreClearedUponCheckin()
{
$asset = Asset::factory()->assignedToUser()->create();
$acceptance = CheckoutAcceptance::factory()->for($asset, 'checkoutable')->pending()->create();
$this->actingAs(User::factory()->checkinAssets()->create())
->post(route('hardware.checkin.store', ['assetId' => $asset->id]));
$this->assertFalse($acceptance->exists(), 'Acceptance was not deleted');
}
public function testCheckinTimeAndActionLogNoteCanBeSet()
{
Event::fake([CheckoutableCheckedIn::class]);
$this->actingAs(User::factory()->checkinAssets()->create())
->post(route(
'hardware.checkin.store',
['assetId' => Asset::factory()->assignedToUser()->create()->id]
), [
'checkin_at' => '2023-01-02',
'note' => 'hello'
]);
Event::assertDispatched(function (CheckoutableCheckedIn $event) {
return $event->action_date === '2023-01-02' && $event->note === 'hello';
}, 1);
}
}

View file

@ -108,6 +108,54 @@ class AssetWebhookTest extends TestCase
Notification::assertNotSentTo(new AnonymousNotifiable, CheckinAssetNotification::class);
}
public function testCheckInEmailSentToUserIfSettingEnabled()
{
Notification::fake();
$user = User::factory()->create();
$asset = Asset::factory()->assignedToUser($user)->create();
$asset->model->category->update(['checkin_email' => true]);
event(new CheckoutableCheckedIn(
$asset,
$user,
User::factory()->checkinAssets()->create(),
''
));
Notification::assertSentTo(
[$user],
function (CheckinAssetNotification $notification, $channels) {
return in_array('mail', $channels);
},
);
}
public function testCheckInEmailNotSentToUserIfSettingDisabled()
{
Notification::fake();
$user = User::factory()->create();
$asset = Asset::factory()->assignedToUser($user)->create();
$asset->model->category->update(['checkin_email' => false]);
event(new CheckoutableCheckedIn(
$asset,
$user,
User::factory()->checkinAssets()->create(),
''
));
Notification::assertNotSentTo(
[$user],
function (CheckinAssetNotification $notification, $channels) {
return in_array('mail', $channels);
}
);
}
private function createAsset()
{
return Asset::factory()->laptopMbp()->create();