ouyang/snipe-it
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #16546 from marcusmoore/bug/sc-28024

Browse Source 
Fixed potential bad method call and premature email sending in bulk asset checkout
This commit is contained in:
snipe
2025-03-31 12:32:50 +01:00
committed by GitHub
parent 6870698cdc 02f109c3b5
commit 13940071a9
2 changed files with 96 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
app/Http/Controllers/Assets/BulkAssetsController.php
View File

@@ -570,7 +570,6 @@ class BulkAssetsController extends Controller
*/
public function storeCheckout(AssetCheckoutRequest $request) : RedirectResponse | ModelNotFoundException
{
$this->authorize('checkout', Asset::class);
try {
@@ -584,6 +583,8 @@ class BulkAssetsController extends Controller
$asset_ids = array_filter($request->get('selected_assets'));
$assets = Asset::findOrFail($asset_ids);
if (request('checkout_to_type') == 'asset') {
foreach ($asset_ids as $asset_id) {
if ($target->id == $asset_id) {
@@ -603,9 +604,8 @@ class BulkAssetsController extends Controller
}
$errors = [];
DB::transaction(function () use ($target, $admin, $checkout_at, $expected_checkin, &$errors, $asset_ids, $request) { //NOTE: $errors is passsed by reference!
foreach ($asset_ids as $asset_id) {
$asset = Asset::findOrFail($asset_id);
DB::transaction(function () use ($target, $admin, $checkout_at, $expected_checkin, &$errors, $assets, $request) { //NOTE: $errors is passsed by reference!
foreach ($assets as $asset) {
$this->authorize('checkout', $asset);
$checkout_success = $asset->checkOut($target, $admin, $checkout_at, $expected_checkin, e($request->get('note')), $asset->name, null);
@@ -632,7 +632,7 @@ class BulkAssetsController extends Controller
// Redirect to the asset management page with error
return redirect()->route('hardware.bulkcheckout.show')->withInput()->with('error', trans_choice('admin/hardware/message.multi-checkout.error', $asset_ids))->withErrors($errors);
} catch (ModelNotFoundException $e) {
return redirect()->route('hardware.bulkcheckout.show')->with('error', $e->getErrors());
return redirect()->route('hardware.bulkcheckout.show')->withInput()->with('error', trans_choice('admin/hardware/message.multi-checkout.error', $request->input('selected_assets')));
}
}

91
tests/Feature/Checkouts/Ui/BulkAssetCheckoutTest.php Normal file
View File

@@ -0,0 +1,91 @@
<?php
namespace Tests\Feature\Checkouts\Ui;
use App\Mail\CheckoutAssetMail;
use App\Models\Asset;
use App\Models\User;
use Illuminate\Support\Facades\Mail;
use PHPUnit\Framework\ExpectationFailedException;
use Tests\TestCase;
class BulkAssetCheckoutTest extends TestCase
{
public function testRequiresPermission()
{
$this->actingAs(User::factory()->create())
->post(route('hardware.bulkcheckout.store'), [
'selected_assets' => [1],
'checkout_to_type' => 'user',
'assigned_user' => 1,
'assigned_asset' => null,
'checkout_at' => null,
'expected_checkin' => null,
'note' => null,
])
->assertForbidden();
}
public function testCanBulkCheckoutAssets()
{
Mail::fake();
$assets = Asset::factory()->requiresAcceptance()->count(2)->create();
$user = User::factory()->create(['email' => 'someone@example.com']);
$checkoutAt = now()->subWeek()->format('Y-m-d');
$expectedCheckin = now()->addWeek()->format('Y-m-d');
$this->actingAs(User::factory()->checkoutAssets()->viewAssets()->create())
->followingRedirects()
->post(route('hardware.bulkcheckout.store'), [
'selected_assets' => $assets->pluck('id')->toArray(),
'checkout_to_type' => 'user',
'assigned_user' => $user->id,
'assigned_asset' => null,
'checkout_at' => $checkoutAt,
'expected_checkin' => $expectedCheckin,
'note' => null,
])
->assertOk();
$assets = $assets->fresh();
$assets->each(function ($asset) use ($expectedCheckin, $checkoutAt, $user) {
$asset->assignedTo()->is($user);
$asset->last_checkout = $checkoutAt;
$asset->expected_checkin = $expectedCheckin;
});
Mail::assertSent(CheckoutAssetMail::class, 2);
Mail::assertSent(CheckoutAssetMail::class, function (CheckoutAssetMail $mail) {
return $mail->hasTo('someone@example.com');
});
}
public function testHandleMissingModelBeingIncluded()
{
Mail::fake();
$this->actingAs(User::factory()->checkoutAssets()->create())
->post(route('hardware.bulkcheckout.store'), [
'selected_assets' => [
Asset::factory()->requiresAcceptance()->create()->id,
9999999,
],
'checkout_to_type' => 'user',
'assigned_user' => User::factory()->create(['email' => 'someone@example.com'])->id,
'assigned_asset' => null,
'checkout_at' => null,
'expected_checkin' => null,
'note' => null,
])
->assertSessionHas('error', trans_choice('admin/hardware/message.multi-checkout.error', 2));
try {
Mail::assertNotSent(CheckoutAssetMail::class);
} catch (ExpectationFailedException $e) {
$this->fail('Asset checkout email was sent when the entire checkout failed.');
}
}
}