Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added tests around asset check in and added missing actions to the api controller action #14260

Merged
merged 32 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
c81bc1d
Scaffold tests around asset check in
marcusmoore Feb 13, 2024
307b39b
Implement tests around asset check in
marcusmoore Feb 13, 2024
852b0b3
Scaffold additional tests
marcusmoore Feb 13, 2024
0506f3b
Scaffold additional tests
marcusmoore Feb 13, 2024
31a75bd
Add some assertions
marcusmoore Feb 13, 2024
b653d19
Implement test
marcusmoore Feb 13, 2024
391b832
Implement test
marcusmoore Feb 13, 2024
f708b8b
Implement test
marcusmoore Feb 13, 2024
9ab56fe
Implement tests
marcusmoore Feb 14, 2024
ad1846f
Implement tests
marcusmoore Feb 14, 2024
7bfd020
Remove duplicate authorization check
marcusmoore Feb 14, 2024
af51394
Implement test
marcusmoore Feb 14, 2024
d7aed2e
Remove unneeded code
marcusmoore Feb 14, 2024
02f3947
Remove duplicate test
marcusmoore Feb 14, 2024
4354e12
Scaffold tests
marcusmoore Feb 14, 2024
3cc7202
Move notification test to notifications test suite
marcusmoore Feb 14, 2024
bacfdc5
Scaffold additional tests
marcusmoore Feb 14, 2024
905df5e
Consolidate test cases
marcusmoore Feb 14, 2024
aec59f2
Update assertion to be more correct
marcusmoore Feb 14, 2024
aa2632f
Merge branch 'develop' into chore/sc-24808
marcusmoore Feb 21, 2024
3ae8adf
Remove incomplete flag on test case
marcusmoore Feb 21, 2024
2df026b
Allow updating asset default location when checking in asset via api
marcusmoore Feb 22, 2024
714fc63
Have legacy locations updated upon api asset checkin
marcusmoore Feb 22, 2024
dba837b
Move location migration logic to trait
marcusmoore Feb 22, 2024
4caadcf
Clear pending checkout acceptances when checking in asset via api
marcusmoore Feb 22, 2024
b55a19c
Add assertion event is dispatched with correct timestamp
marcusmoore Feb 22, 2024
c401c88
Scope event fake
marcusmoore Feb 23, 2024
29d7291
Align test with actual values passed from the web
marcusmoore Feb 26, 2024
69022bb
Implement test
marcusmoore Feb 27, 2024
0e460ba
Improve readability
marcusmoore Feb 27, 2024
a5516e3
Improve trait name
marcusmoore Feb 27, 2024
5084e5d
Improve variable type
marcusmoore Feb 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions app/Http/Controllers/Api/AssetsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

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;
Expand Down Expand Up @@ -45,6 +49,8 @@
*/
class AssetsController extends Controller
{
use MigratesLegacyAssetLocations;

/**
* Returns JSON listing of all assets
*
Expand Down Expand Up @@ -864,11 +870,9 @@ public function checkout(AssetCheckoutRequest $request, $asset_id)
*/
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', [
Expand All @@ -881,18 +885,23 @@ public function checkin(Request $request, $asset_id)
$asset->expected_checkin = null;
$asset->last_checkout = null;
$asset->last_checkin = now();
$asset->assigned_to = null;
$asset->assignedTo()->disassociate($asset);
$asset->accepted = null;

if ($request->has('name')) {
$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')) {
Expand All @@ -906,12 +915,22 @@ public function checkin(Request $request, $asset_id)
$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));
Expand Down
34 changes: 8 additions & 26 deletions app/Http/Controllers/Assets/AssetCheckinController.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@
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;
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.
*
Expand Down Expand Up @@ -69,34 +73,15 @@ public function store(AssetCheckinRequest $request, $assetId = null, $backto = n
$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');

if ($request->filled('status_id')) {
$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;

Expand All @@ -117,12 +102,9 @@ public function store(AssetCheckinRequest $request, $assetId = null, $backto = n
$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',
Expand Down
33 changes: 33 additions & 0 deletions app/Http/Traits/MigratesLegacyAssetLocations.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
23 changes: 19 additions & 4 deletions database/factories/AssetFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use App\Models\Statuslabel;
use App\Models\Supplier;
use App\Models\User;
use Carbon\Carbon;
use Carbon\CarbonImmutable;
use Illuminate\Database\Eloquent\Factories\Factory;

Expand Down Expand Up @@ -289,12 +288,13 @@ public function ultrasharp()
});
}

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(),
];
});
}
Expand Down Expand Up @@ -352,4 +352,19 @@ public function nonrequestable()
{
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);
});
}
}
10 changes: 10 additions & 0 deletions database/factories/LicenseSeatFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -13,4 +14,13 @@ public function definition()
'license_id' => License::factory(),
];
}

public function assignedToUser(User $user = null)
{
return $this->state(function () use ($user) {
return [
'assigned_to' => $user->id ?? User::factory(),
];
});
}
}
Loading
Loading