Merge pull request #15264 from snipe/fixes/fixed_admin_ordering_on_report

Fixed #15252 - admin ordering on activity report
This commit is contained in:
snipe 2024-08-10 18:26:15 +01:00 committed by GitHub
commit 60eb602156
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 107 additions and 12 deletions

View file

@ -83,11 +83,19 @@ class ReportsController extends Controller
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
$limit = app('api_limit_value');
$sort = in_array($request->input('sort'), $allowed_columns) ? e($request->input('sort')) : 'created_at';
$order = ($request->input('order') == 'asc') ? 'asc' : 'desc';
switch ($request->input('sort')) {
case 'admin':
$actionlogs->OrderAdmin($order);
break;
default:
$sort = in_array($request->input('sort'), $allowed_columns) ? e($request->input('sort')) : 'created_at';
$actionlogs = $actionlogs->orderBy($sort, $order);
break;
}
$actionlogs = $actionlogs->orderBy($sort, $order)->skip($offset)->take($limit)->get();
$actionlogs = $actionlogs->skip($offset)->take($limit)->get();
return response()->json((new ActionlogsTransformer)->transformActionlogs($actionlogs, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
}

View file

@ -7,7 +7,6 @@ use App\Presenters\Presentable;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\SoftDeletes;
use Illuminate\Support\Facades\Auth;
/**
* Model for the Actionlog (the table that keeps a historical log of
@ -17,10 +16,12 @@ use Illuminate\Support\Facades\Auth;
*/
class Actionlog extends SnipeModel
{
use CompanyableTrait;
use HasFactory;
// This is to manually set the source (via setActionSource()) for determineActionSource()
protected ?string $source = null;
protected $with = ['admin'];
protected $presenter = \App\Presenters\ActionlogPresenter::class;
use SoftDeletes;
@ -66,7 +67,7 @@ class Actionlog extends SnipeModel
'company' => ['name'],
'admin' => ['first_name','last_name','username', 'email'],
'user' => ['first_name','last_name','username', 'email'],
'assets' => ['asset_tag','name'],
'assets' => ['asset_tag','name','model', 'model_number'],
];
/**
@ -372,4 +373,9 @@ class Actionlog extends SnipeModel
{
$this->source = $source;
}
public function scopeOrderAdmin($query, $order)
{
return $query->leftJoin('users as admin_sort', 'action_logs.user_id', '=', 'admin_sort.id')->select('action_logs.*')->orderBy('admin_sort.first_name', $order)->orderBy('admin_sort.last_name', $order);
}
}

View file

@ -2,7 +2,6 @@
namespace App\Models;
use App\Events\AssetCheckedOut;
use App\Events\CheckoutableCheckedOut;
use App\Exceptions\CheckoutNotAllowed;
use App\Helpers\Helper;
@ -31,6 +30,7 @@ class Asset extends Depreciable
{
protected $presenter = AssetPresenter::class;
protected $with = ['model', 'admin'];
use CompanyableTrait;
use HasFactory, Loggable, Requestable, Presentable, SoftDeletes, ValidatingTrait, UniqueUndeletedTrait;
@ -716,7 +716,7 @@ class Asset extends Depreciable
* @since [v1.0]
* @return \Illuminate\Database\Eloquent\Relations\Relation
*/
public function adminuser()
public function admin()
{
return $this->belongsTo(\App\Models\User::class, 'user_id');
}

View file

@ -52,7 +52,7 @@
<th class="col-sm-3" data-field="item.serial" data-visible="false">{{ trans('admin/hardware/table.serial') }}</th>
<th class="col-sm-3" data-field="item" data-formatter="polymorphicItemFormatter">{{ trans('general.item') }}</th>
<th class="col-sm-2" data-field="target" data-formatter="polymorphicItemFormatter">{{ trans('general.to') }}</th>
<th class="col-sm-1" data-field="note">{{ trans('general.notes') }}</th>
<th class="col-sm-1" data-field="note" data-sortable="true">{{ trans('general.notes') }}</th>
<th class="col-sm-2" data-field="log_meta" data-visible="false" data-formatter="changeLogFormatter">{{ trans('general.changed') }}</th>
<th data-field="remote_ip" data-visible="false" data-sortable="true">{{ trans('admin/settings/general.login_ip') }}</th>
<th data-field="user_agent" data-visible="false" data-sortable="true">{{ trans('admin/settings/general.login_user_agent') }}</th>

View file

@ -61,7 +61,7 @@ class StoreAssetTest extends TestCase
$asset = Asset::find($response['payload']['id']);
$this->assertTrue($asset->adminuser->is($user));
$this->assertTrue($asset->admin->is($user));
$this->assertEquals('2024-06-02', $asset->asset_eol_date);
$this->assertEquals('random_string', $asset->asset_tag);
@ -456,7 +456,7 @@ class StoreAssetTest extends TestCase
$asset = Asset::find($response['payload']['id']);
$this->assertTrue($asset->adminuser->is($user));
$this->assertTrue($asset->admin->is($user));
$this->assertTrue($asset->checkedOutToUser());
$this->assertTrue($asset->assignedTo->is($userAssigned));
}
@ -482,7 +482,7 @@ class StoreAssetTest extends TestCase
$asset = Asset::find($response['payload']['id']);
$this->assertTrue($asset->adminuser->is($user));
$this->assertTrue($asset->admin->is($user));
$this->assertTrue($asset->checkedOutToLocation());
$this->assertTrue($asset->location->is($location));
}
@ -508,7 +508,7 @@ class StoreAssetTest extends TestCase
$apiAsset = Asset::find($response['payload']['id']);
$this->assertTrue($apiAsset->adminuser->is($user));
$this->assertTrue($apiAsset->admin->is($user));
$this->assertTrue($apiAsset->checkedOutToAsset());
// I think this makes sense, but open to a sanity check
$this->assertTrue($asset->assignedAssets()->find($response['payload']['id'])->is($apiAsset));

View file

@ -165,7 +165,7 @@ class BulkEditAssetsTest extends TestCase
});
}
public function testBulkEditAssetsRequiresAdminUserToUpdateEncryptedCustomFields()
public function testBulkEditAssetsRequiresadminToUpdateEncryptedCustomFields()
{
$this->markIncompleteIfMySQL('Custom Fields tests do not work on mysql');
$edit_user = User::factory()->editAssets()->create();

View file

@ -0,0 +1,81 @@
<?php
namespace Tests\Feature\Reporting;
use App\Models\Asset;
use App\Models\Company;
use App\Models\User;
use App\Models\Actionlog;
use Database\Factories\ActionlogFactory;
use Illuminate\Testing\Fluent\AssertableJson;
use Illuminate\Testing\TestResponse;
use League\Csv\Reader;
use PHPUnit\Framework\Assert;
use Tests\TestCase;
class ActivityReportTest extends TestCase
{
public function testRequiresPermissionToViewActivity()
{
$this->actingAsForApi(User::factory()->create())
->getJson(route('api.activity.index'))
->assertForbidden();
}
public function testRecordsAreScopedToCompanyWhenMultipleCompanySupportEnabled()
{
$this->markTestIncomplete('This test returns strange results. Need to figure out why.');
$this->settings->enableMultipleFullCompanySupport();
$companyA = Company::factory()->create();
$companyB = Company::factory()->create();
$superUser = User::factory()->superuser()->make();
$userInCompanyA = User::factory()
->viewUsers()
->viewAssets()
->canViewReports()
->create(['company_id' => $companyA->id]);
$userInCompanyB = User::factory()
->viewUsers()
->viewAssets()
->canViewReports()
->create(['company_id' => $companyB->id]);
Asset::factory()->count(5)->create(['company_id' => $companyA->id]);
Asset::factory()->count(4)->create(['company_id' => $companyB->id]);
Asset::factory()->count(3)->create();
Actionlog::factory()->userUpdated()->count(5)->create(['company_id' => $companyA->id]);
Actionlog::factory()->userUpdated()->count(4)->create(['company_id' => $companyB->id]);
Actionlog::factory()->userUpdated()->count(3)->create(['company_id' => $companyB->id]);
// I don't love this, since it doesn't test that we're actually storing the company ID appropriately
// but it's better than what we had
$response = $this->actingAsForApi($userInCompanyA)
->getJson(route('api.activity.index'))
->assertOk()
->assertJsonStructure([
'rows',
])
->assertJson(fn(AssertableJson $json) => $json->has('rows', 5)->etc());
$this->actingAsForApi($userInCompanyB)
->getJson(
route('api.activity.index'))
->assertOk()
->assertJsonStructure([
'rows',
])
->assertJson(fn(AssertableJson $json) => $json->has('rows', 7)->etc());
}
}