Merge remote-tracking branch 'origin/master' into develop
This commit is contained in:
@@ -405,11 +405,11 @@ class AccessoriesController extends Controller
|
||||
public function history(Request $request, Accessory $accessory): JsonResponse|array
|
||||
{
|
||||
$this->authorize('history', $accessory);
|
||||
$history = $accessory->getHistory($request);
|
||||
$total = $accessory->getHistory($request)->count();
|
||||
$historyQuery = $accessory->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -343,11 +343,11 @@ class AssetModelsController extends Controller
|
||||
public function history(Request $request, AssetModel $model): JsonResponse|array
|
||||
{
|
||||
$this->authorize('history', $model);
|
||||
$history = $model->getHistory($request);
|
||||
$total = $model->getHistory($request)->count();
|
||||
$historyQuery = $model->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -1468,11 +1468,11 @@ class AssetsController extends Controller
|
||||
public function history(Request $request, Asset $asset): JsonResponse|array
|
||||
{
|
||||
$this->authorize('history', $asset);
|
||||
$history = $asset->getHistory($request);
|
||||
$total = $asset->getHistory($request)->count();
|
||||
$historyQuery = $asset->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -391,11 +391,11 @@ class ComponentsController extends Controller
|
||||
public function history(Request $request, Component $component): JsonResponse|array
|
||||
{
|
||||
$this->authorize('history', $component);
|
||||
$history = $component->getHistory($request);
|
||||
$total = $component->getHistory($request)->count();
|
||||
$historyQuery = $component->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -361,11 +361,11 @@ class ConsumablesController extends Controller
|
||||
public function history(Request $request, Consumable $consumable): JsonResponse|array
|
||||
{
|
||||
$this->authorize('history', $consumable);
|
||||
$history = $consumable->getHistory($request);
|
||||
$total = $consumable->getHistory($request)->count();
|
||||
$historyQuery = $consumable->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -282,11 +282,11 @@ class LicensesController extends Controller
|
||||
public function history(Request $request, License $license): JsonResponse|array
|
||||
{
|
||||
$this->authorize('history', $license);
|
||||
$history = $license->getHistory($request);
|
||||
$total = $license->getHistory($request)->count();
|
||||
$historyQuery = $license->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -462,11 +462,11 @@ class LocationsController extends Controller
|
||||
public function history(Request $request, Location $location): JsonResponse|array
|
||||
{
|
||||
$this->authorize('history', $location);
|
||||
$history = $location->getHistory($request);
|
||||
$total = $location->getHistory($request)->count();
|
||||
$historyQuery = $location->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -260,11 +260,11 @@ class MaintenancesController extends Controller
|
||||
$this->authorize('view', Asset::class);
|
||||
$asset = $maintenance->asset;
|
||||
$this->authorize('history', $asset);
|
||||
$history = $maintenance->getHistory($request);
|
||||
$total = $maintenance->getHistory($request)->count();
|
||||
$historyQuery = $maintenance->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -939,11 +939,11 @@ class UsersController extends Controller
|
||||
public function history(Request $request, User $user): JsonResponse|array
|
||||
{
|
||||
$this->authorize('history', $user);
|
||||
$history = $user->getHistory($request);
|
||||
$total = $user->getHistory($request)->count();
|
||||
$historyQuery = $user->getHistory($request);
|
||||
$total = (clone $historyQuery)->count();
|
||||
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
|
||||
$limit = app('api_limit_value');
|
||||
$history = $history->skip($offset)->take($limit)->get();
|
||||
$history = (clone $historyQuery)->skip($offset)->take($limit)->get();
|
||||
|
||||
return response()->json((new ActionlogsTransformer)->transformActionlogs($history, $total), 200, ['Content-Type' => 'application/json;charset=utf8'], JSON_UNESCAPED_UNICODE);
|
||||
}
|
||||
|
||||
@@ -917,7 +917,7 @@ class ReportsController extends Controller
|
||||
|
||||
if ($request->filled('user_company')) {
|
||||
if ($asset->checkedOutToUser()) {
|
||||
$row[] = ($asset->assignedto->company) ? $asset->assignedto->company->display_name : '';
|
||||
$row[] = ($asset->assignedto?->company) ? $asset->assignedto->company->display_name : '';
|
||||
} else {
|
||||
$row[] = ''; // Empty string if unassigned
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use App\Presenters\ActionlogPresenter;
|
||||
use App\Presenters\Presentable;
|
||||
use Carbon\Carbon;
|
||||
use Illuminate\Database\Eloquent\Factories\HasFactory;
|
||||
use Illuminate\Database\Eloquent\Relations\MorphTo;
|
||||
use Illuminate\Database\Eloquent\Relations\Relation;
|
||||
use Illuminate\Database\Eloquent\SoftDeletes;
|
||||
use Illuminate\Support\Str;
|
||||
@@ -328,6 +329,27 @@ class Actionlog extends SnipeModel
|
||||
return $this->morphTo('target')->withTrashed();
|
||||
}
|
||||
|
||||
/**
|
||||
* Eager load history relations used by the API transformer to avoid N+1 queries.
|
||||
*/
|
||||
public function scopeForApiHistory($query)
|
||||
{
|
||||
return $query->with([
|
||||
'adminuser',
|
||||
'location',
|
||||
'item' => function (MorphTo $morphTo) {
|
||||
$morphTo->morphWith([
|
||||
Asset::class => ['model'],
|
||||
]);
|
||||
},
|
||||
'target' => function (MorphTo $morphTo) {
|
||||
$morphTo->morphWith([
|
||||
Asset::class => ['model'],
|
||||
]);
|
||||
},
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Establishes the actionlog -> location relationship
|
||||
*
|
||||
|
||||
@@ -107,7 +107,7 @@ trait Loggable
|
||||
break;
|
||||
}
|
||||
|
||||
return $history;
|
||||
return $history->forApiHistory();
|
||||
|
||||
}
|
||||
|
||||
|
||||
@@ -22,6 +22,14 @@ class AddEolDateOnAssetsTable extends Migration
|
||||
$table->date('asset_eol_date')->after('purchase_date')->nullable()->default(null);
|
||||
}
|
||||
|
||||
// This is a back in time migration to fix restores from very old versions of Snipe-IT where
|
||||
// companies were not soft-deletable.
|
||||
Schema::table('companies', function (Blueprint $table) {
|
||||
if (! Schema::hasColumn('companies', 'deleted_at')) {
|
||||
$table->softDeletes();
|
||||
}
|
||||
});
|
||||
|
||||
// This is a temporary shim so we don't have to modify the asset observer for migrations where
|
||||
// there is a large version difference. (See the AssetObserver notes). This column gets created
|
||||
// later in 2023_07_13_052204_denormalized_eol_and_add_column_for_explicit_date_to_assets.php
|
||||
|
||||
+1
@@ -54,6 +54,7 @@ class DenormalizedEolAndAddColumnForExplicitDateToAssets extends Migration
|
||||
->update([
|
||||
'asset_eol_date' => $this->eolUpdateExpression(),
|
||||
]);
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -11,8 +11,11 @@ return new class extends Migration
|
||||
*/
|
||||
public function up(): void
|
||||
{
|
||||
|
||||
Schema::table('companies', function (Blueprint $table) {
|
||||
$table->softDeletes();
|
||||
if (! Schema::hasColumn('companies', 'deleted_at')) {
|
||||
$table->softDeletes();
|
||||
}
|
||||
});
|
||||
|
||||
}
|
||||
|
||||
@@ -77,7 +77,7 @@
|
||||
{{ trans('admin/hardware/form.name') }}
|
||||
</label>
|
||||
|
||||
<div class="col-md-8">
|
||||
<div class="col-md-7">
|
||||
<input class="form-control" type="text" name="name" id="name"
|
||||
value="{{ old('name', $asset->name) }}" tabindex="1">
|
||||
{!! $errors->first('name', '<span class="alert-msg" aria-hidden="true"><i class="fas fa-times" aria-hidden="true"></i> :message</span>') !!}
|
||||
@@ -137,6 +137,7 @@
|
||||
<div class="col-md-8">
|
||||
<x-input.datepicker
|
||||
name="expected_checkin"
|
||||
col_size_class="col-md-7"
|
||||
:value="old('expected_checkin', $item->expected_checkin)"
|
||||
placeholder="{{ trans('general.select_date') }}"
|
||||
required="{{ Helper::checkIfRequired($item, 'expected_checkin') }}"
|
||||
|
||||
@@ -11,6 +11,7 @@ use App\Models\License;
|
||||
use App\Models\Location;
|
||||
use App\Models\Maintenance;
|
||||
use App\Models\User;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Tests\TestCase;
|
||||
|
||||
class IndexHistoryTest extends TestCase
|
||||
@@ -440,4 +441,61 @@ class IndexHistoryTest extends TestCase
|
||||
->assertJsonCount(1, 'rows')
|
||||
->assertJsonPath('rows.0.id', $second->id);
|
||||
}
|
||||
|
||||
public function test_viewing_user_history_avoids_n_plus_one_queries_for_polymorphic_relations()
|
||||
{
|
||||
$subject = User::factory()->create();
|
||||
$actor = User::factory()->viewUserHistory()->create();
|
||||
$uniqueNote = 'history-polymorphic-n-plus-one-'.uniqid();
|
||||
|
||||
$locations = Location::factory()->count(10)->create();
|
||||
$assets = Asset::factory()->count(10)->create();
|
||||
$users = User::factory()->count(10)->create();
|
||||
|
||||
for ($index = 0; $index < 30; $index++) {
|
||||
$itemType = $index % 3;
|
||||
|
||||
if ($itemType === 0) {
|
||||
$item = $assets[$index % $assets->count()];
|
||||
$itemTypeClass = Asset::class;
|
||||
} elseif ($itemType === 1) {
|
||||
$item = $locations[$index % $locations->count()];
|
||||
$itemTypeClass = Location::class;
|
||||
} else {
|
||||
$item = $users[$index % $users->count()];
|
||||
$itemTypeClass = User::class;
|
||||
}
|
||||
|
||||
Actionlog::factory()->create([
|
||||
'item_id' => $item->id,
|
||||
'item_type' => $itemTypeClass,
|
||||
'target_id' => $subject->id,
|
||||
'target_type' => User::class,
|
||||
'location_id' => $locations[$index % $locations->count()]->id,
|
||||
'created_by' => $actor->id,
|
||||
'action_type' => 'update',
|
||||
'note' => $uniqueNote,
|
||||
]);
|
||||
}
|
||||
|
||||
DB::flushQueryLog();
|
||||
DB::enableQueryLog();
|
||||
|
||||
$response = $this->actingAsForApi($actor)
|
||||
->getJson(route('api.users.history', [
|
||||
'user' => $subject,
|
||||
'limit' => 30,
|
||||
'offset' => 0,
|
||||
'search' => $uniqueNote,
|
||||
]))
|
||||
->assertOk()
|
||||
->assertJsonPath('total', 30);
|
||||
|
||||
$queryCount = count(DB::getQueryLog());
|
||||
DB::disableQueryLog();
|
||||
|
||||
// This threshold is intentionally generous but prevents N+1 regressions.
|
||||
$this->assertLessThan(45, $queryCount, 'History endpoint query count regressed and may have reintroduced N+1 behavior.');
|
||||
$this->assertCount(30, $response->json('rows'));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user