Merge pull request #14707 from spencerrlongg/bug/sc-24912

Bulk Edit Tests and Tweaks
This commit is contained in:
snipe 2024-05-15 11:22:31 +01:00 committed by GitHub
commit eb09a99eb0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 270 additions and 22 deletions

View file

@ -13,7 +13,9 @@ use App\Models\Setting;
use App\View\Label;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Gate;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Session;
use App\Http\Requests\AssetCheckoutRequest;
@ -189,7 +191,6 @@ class BulkAssetsController extends Controller
* Save bulk edits
*
* @author [A. Gianotto] [<snipe@snipe.net>]
* @return Redirect
* @internal param array $assets
* @since [v2.0]
*/
@ -214,7 +215,7 @@ class BulkAssetsController extends Controller
}
$assets = Asset::whereIn('id', array_keys($request->input('ids')))->get();
$assets = Asset::whereIn('id', $request->input('ids'))->get();
@ -379,28 +380,30 @@ class BulkAssetsController extends Controller
foreach ($asset->model->fieldset->fields as $field) {
if ((array_key_exists($field->db_column, $this->update_array)) && ($field->field_encrypted == '1')) {
$decrypted_old = Helper::gracefulDecrypt($field, $asset->{$field->db_column});
if (Gate::allows('admin')) {
$decrypted_old = Helper::gracefulDecrypt($field, $asset->{$field->db_column});
/*
* Check if the decrypted existing value is different from one we just submitted
* and if not, pull it out of the object since it shouldn't really be updating at all.
* If we don't do this, it will try to re-encrypt it, and the same value encrypted two
* different times will have different values, so it will *look* like it was updated
* but it wasn't.
*/
if ($decrypted_old != $this->update_array[$field->db_column]) {
$asset->{$field->db_column} = \Crypt::encrypt($this->update_array[$field->db_column]);
} else {
/*
* Remove the encrypted custom field from the update_array, since nothing changed
* Check if the decrypted existing value is different from one we just submitted
* and if not, pull it out of the object since it shouldn't really be updating at all.
* If we don't do this, it will try to re-encrypt it, and the same value encrypted two
* different times will have different values, so it will *look* like it was updated
* but it wasn't.
*/
unset($this->update_array[$field->db_column]);
unset($asset->{$field->db_column});
}
if ($decrypted_old != $this->update_array[$field->db_column]) {
$asset->{$field->db_column} = Crypt::encrypt($this->update_array[$field->db_column]);
} else {
/*
* Remove the encrypted custom field from the update_array, since nothing changed
*/
unset($this->update_array[$field->db_column]);
unset($asset->{$field->db_column});
}
/*
* These custom fields aren't encrypted, just carry on as usual
*/
/*
* These custom fields aren't encrypted, just carry on as usual
*/
}
} else {
if ((array_key_exists($field->db_column, $this->update_array)) && ($asset->{$field->db_column} != $this->update_array[$field->db_column])) {

View file

@ -363,6 +363,15 @@ class AssetFactory extends Factory
});
}
public function hasMultipleCustomFields(array $fields = null): self
{
return $this->state(function () use ($fields) {
return [
'model_id' => AssetModel::factory()->hasMultipleCustomFields($fields),
];
});
}
/**
* This allows bypassing model level validation if you want to purposefully

View file

@ -439,4 +439,13 @@ class AssetModelFactory extends Factory
];
});
}
public function hasMultipleCustomFields(array $fields = null)
{
return $this->state(function () use ($fields) {
return [
'fieldset_id' => CustomFieldset::factory()->hasMultipleCustomFields($fields),
];
});
}
}

View file

@ -53,4 +53,23 @@ class CustomFieldsetFactory extends Factory
$fieldset->fields()->attach($field, ['order' => '1', 'required' => false]);
});
}
public function hasMultipleCustomFields(array $fields = null): self
{
return $this->afterCreating(function (CustomFieldset $fieldset) use ($fields) {
if (empty($fields)) {
$mac_address = CustomField::factory()->macAddress()->create();
$ram = CustomField::factory()->ram()->create();
$cpu = CustomField::factory()->cpu()->create();
$fieldset->fields()->attach($mac_address, ['order' => '1', 'required' => false]);
$fieldset->fields()->attach($ram, ['order' => '2', 'required' => false]);
$fieldset->fields()->attach($cpu, ['order' => '3', 'required' => false]);
} else {
foreach ($fields as $field) {
$fieldset->fields()->attach($field, ['order' => '1', 'required' => false]);
}
}
});
}
}

View file

@ -196,8 +196,8 @@
@include("models/custom_fields_form_bulk_edit",["models" => $models])
@foreach ($assets as $key => $value)
<input type="hidden" name="ids[{{ $value }}]" value="1">
@foreach($assets as $asset)
<input type="hidden" name="ids[]" value="{{ $asset }}">
@endforeach
</div> <!--/.box-body-->

View file

@ -0,0 +1,208 @@
<?php
namespace Tests\Feature\Assets;
use App\Models\Asset;
use App\Models\AssetModel;
use App\Models\Company;
use App\Models\CustomField;
use App\Models\Statuslabel;
use App\Models\Supplier;
use App\Models\User;
use Illuminate\Support\Facades\Crypt;
use Tests\TestCase;
class AssetsBulkEditTest extends TestCase
{
public function testUserWithPermissionsCanAccessPage()
{
$user = User::factory()->viewAssets()->editAssets()->create();
$assets = Asset::factory()->count(2)->create();
$id_array = $assets->pluck('id')->toArray();
$this->actingAs($user)->post('/hardware/bulkedit', [
'ids' => $id_array,
'order' => 'asc',
'bulk_actions' => 'edit',
'sort' => 'id'
])->assertStatus(200);
}
public function testStandardUserCannotAccessPage()
{
$user = User::factory()->create();
$assets = Asset::factory()->count(2)->create();
$id_array = $assets->pluck('id')->toArray();
$this->actingAs($user)->post('/hardware/bulkedit', [
'ids' => $id_array,
'order' => 'asc',
'bulk_actions' => 'edit',
'sort' => 'id'
])->assertStatus(403);
}
public function testBulkEditAssetsAcceptsAllPossibleAttributes()
{
// sets up all needed models and attributes on the assets
// this test does not deal with custom fields - will be dealt with in separate cases
$status1 = Statuslabel::factory()->create();
$status2 = Statuslabel::factory()->create();
$model1 = AssetModel::factory()->create();
$model2 = AssetModel::factory()->create();
$supplier1 = Supplier::factory()->create();
$supplier2 = Supplier::factory()->create();
$company1 = Company::factory()->create();
$company2 = Company::factory()->create();
$assets = Asset::factory()->count(10)->create([
'purchase_date' => '2023-01-01',
'expected_checkin' => '2023-01-01',
'status_id' => $status1->id,
'model_id' => $model1->id,
// skipping locations on this test, it deserves it's own test
'purchase_cost' => 1234.90,
'supplier_id' => $supplier1->id,
'company_id' => $company1->id,
'order_number' => '123456',
'warranty_months' => 24,
'next_audit_date' => '2024-06-01',
'requestable' => false
]);
// gets the ids together to submit to the endpoint
$id_array = $assets->pluck('id')->toArray();
// submits the ids and new values for each attribute
$this->actingAs(User::factory()->editAssets()->create())->post(route('hardware/bulksave'), [
'ids' => $id_array,
'purchase_date' => '2024-01-01',
'expected_checkin' => '2024-01-01',
'status_id' => $status2->id,
'model_id' => $model2->id,
'purchase_cost' => 5678.92,
'supplier_id' => $supplier2->id,
'company_id' => $company2->id,
'order_number' => '7890',
'warranty_months' => 36,
'next_audit_date' => '2025-01-01',
'requestable' => true
])
->assertStatus(302)
->assertSessionHasNoErrors();
// asserts that each asset has the updated values
Asset::findMany($id_array)->each(function (Asset $asset) use ($status2, $model2, $supplier2, $company2) {
$this->assertEquals('2024-01-01', $asset->purchase_date->format('Y-m-d'));
$this->assertEquals('2024-01-01', $asset->expected_checkin->format('Y-m-d'));
$this->assertEquals($status2->id, $asset->status_id);
$this->assertEquals($model2->id, $asset->model_id);
$this->assertEquals(5678.92, $asset->purchase_cost);
$this->assertEquals($supplier2->id, $asset->supplier_id);
$this->assertEquals($company2->id, $asset->company_id);
$this->assertEquals(7890, $asset->order_number);
$this->assertEquals(36, $asset->warranty_months);
$this->assertEquals('2025-01-01', $asset->next_audit_date->format('Y-m-d'));
// shouldn't requestable be cast as a boolean??? it's not.
$this->assertEquals(1, $asset->requestable);
});
}
public function testBulkEditAssetsAcceptsAndUpdatesUnencryptedCustomFields()
{
$this->markIncompleteIfMySQL('Custom Fields tests do not work on MySQL');
CustomField::factory()->ram()->create();
CustomField::factory()->cpu()->create();
// when getting the custom field directly from the factory the field has not been fully created yet
// so we have to do a query afterwards to get the actual model :shrug:
$ram = CustomField::where('name', 'RAM')->first();
$cpu = CustomField::where('name', 'CPU')->first();
$assets = Asset::factory()->count(10)->hasMultipleCustomFields([$ram, $cpu])->create([
$ram->db_column => 8,
$cpu->db_column => '2.1',
]);
$id_array = $assets->pluck('id')->toArray();
$this->actingAs(User::factory()->editAssets()->create())->post(route('hardware/bulksave'), [
'ids' => $id_array,
$ram->db_column => 16,
$cpu->db_column => '4.1',
])->assertStatus(302);
Asset::findMany($id_array)->each(function (Asset $asset) use ($ram, $cpu) {
$this->assertEquals(16, $asset->{$ram->db_column});
$this->assertEquals('4.1', $asset->{$cpu->db_column});
});
}
public function testBulkEditAssetsAcceptsAndUpdatesEncryptedCustomFields()
{
$this->markIncompleteIfMySQL('Custom Fields tests do not work on MySQL');
CustomField::factory()->testEncrypted()->create();
$encrypted = CustomField::where('name', 'Test Encrypted')->first();
$assets = Asset::factory()->count(10)->hasEncryptedCustomField($encrypted)->create([
$encrypted->db_column => Crypt::encrypt('Original Encrypted Text'),
]);
$id_array = $assets->pluck('id')->toArray();
$this->actingAs(User::factory()->admin()->create())->post(route('hardware/bulksave'), [
'ids' => $id_array,
$encrypted->db_column => 'New Encrypted Text',
])->assertStatus(302);
Asset::findMany($id_array)->each(function (Asset $asset) use ($encrypted) {
$this->assertEquals('New Encrypted Text', Crypt::decrypt($asset->{$encrypted->db_column}));
});
}
public function testBulkEditAssetsRequiresAdminUserToUpdateEncryptedCustomFields()
{
$this->markIncompleteIfMySQL('Custom Fields tests do not work on mysql');
$edit_user = User::factory()->editAssets()->create();
$admin_user = User::factory()->admin()->create();
CustomField::factory()->testEncrypted()->create();
$encrypted = CustomField::where('name', 'Test Encrypted')->first();
$admin_assets = Asset::factory()->count(5)->hasEncryptedCustomField($encrypted)->create([
$encrypted->db_column => Crypt::encrypt('Original Encrypted Text'),
]);
$standard_assets = Asset::factory()->count(5)->hasEncryptedCustomField($encrypted)->create([
$encrypted->db_column => Crypt::encrypt('Original Encrypted Text'),
]);
$admin_id_array = $admin_assets->pluck('id')->toArray();
$standard_id_array = $standard_assets->pluck('id')->toArray();
$this->actingAs($admin_user)->post(route('hardware/bulksave'), [
'ids' => $admin_id_array,
$encrypted->db_column => 'New Encrypted Text',
])->assertStatus(302);
// do we want to return an error when this happens???
$this->actingAs($edit_user)->post(route('hardware/bulksave'), [
'ids' => $standard_id_array,
$encrypted->db_column => 'New Encrypted Text',
])->assertStatus(302);
Asset::findMany($admin_id_array)->each(function (Asset $asset) use ($encrypted) {
$this->assertEquals('New Encrypted Text', Crypt::decrypt($asset->{$encrypted->db_column}));
});
Asset::findMany($standard_id_array)->each(function (Asset $asset) use ($encrypted) {
$this->assertEquals('Original Encrypted Text', Crypt::decrypt($asset->{$encrypted->db_column}));
});
}
}