3
0
mirror of https://github.com/snipe/snipe-it.git synced 2026-08-18 03:06:23 +00:00

Alert Menu: Switch to Livewire + kill low-inventory N+1

This commit is contained in:
snipe
2026-07-25 13:00:56 +01:00
parent 3e77ac1521
commit 482e8a72b8
8 changed files with 445 additions and 176 deletions

View File

@ -3,7 +3,6 @@
namespace App\Helpers;
use App\Models\Accessory;
use App\Models\Asset;
use App\Models\AssetModel;
use App\Models\Component;
use App\Models\Consumable;
@ -846,113 +845,140 @@ class Helper
*/
public static function checkLowInventory()
{
$alert_threshold = Setting::getSettings()->alert_threshold;
$consumables = Consumable::withCount('consumableAssignments as consumables_users_count')->whereNotNull('min_amt')->get();
$accessories = Accessory::withCount('checkouts as checkouts_count')->whereNotNull('min_amt')->get();
$components = Component::withCount('assets as sum_unconstrained_assets')->whereNotNull('min_amt')->get();
$asset_models = AssetModel::where('min_amt', '>', 0)->withCount(['availableAssets', 'assets'])->get();
$licenses = License::withCount('availCount as licenses_available')->where('min_amt', '>', 0)->get();
$alert_threshold = (int) Setting::getSettings()->alert_threshold;
// Push the "below threshold" filter into SQL via havingRaw on the
// withCount alias, so only rows that will actually alert get
// hydrated. Previous shape loaded every row with min_amt set and
// filtered in PHP — on a 1000-item deployment with 5 low-inventory
// items that meant 200× more rows than needed. Also select only
// the columns the foreach uses (id / name / qty / min_amt),
// avoiding hydration of long text columns like License::serial.
// select() must come BEFORE withCount(): withCount uses addSelect
// under the hood, so a select() after would wipe the count alias.
// GROUP BY primary key satisfies SQLite's strict "HAVING requires
// an aggregated query" check — MariaDB allows bare HAVING but the
// test suite runs SQLite. Grouping by a unique key is a no-op for
// row cardinality (functional dependency), so nothing else shifts.
$consumables = Consumable::select('id', 'name', 'qty', 'min_amt')
->withCount('consumableAssignments as consumables_users_count')
->whereNotNull('min_amt')
->groupBy('consumables.id')
->havingRaw('(qty - consumables_users_count) < (min_amt + ?)', [$alert_threshold])
->get();
$accessories = Accessory::select('id', 'name', 'qty', 'min_amt')
->withCount('checkouts as checkouts_count')
->whereNotNull('min_amt')
->groupBy('accessories.id')
->havingRaw('(qty - checkouts_count) < (min_amt + ?)', [$alert_threshold])
->get();
$components = Component::select('id', 'name', 'qty', 'min_amt')
->withCount('assets as sum_unconstrained_assets')
->whereNotNull('min_amt')
->groupBy('components.id')
->havingRaw('(qty - sum_unconstrained_assets) < (min_amt + ?)', [$alert_threshold])
->get();
$asset_models = AssetModel::select('id', 'name', 'min_amt')
->where('min_amt', '>', 0)
->withCount(['availableAssets', 'assets'])
->groupBy('models.id')
->havingRaw('available_assets_count < (min_amt + ?)', [$alert_threshold])
->get();
// Use the licenses_available withCount alias directly in the
// foreach below rather than $license->remaincount(). remaincount()
// fires two extra queries per row (licenseSeatsCount via
// getLicenseSeatsCountAttribute() and assigned_seats_count via
// getAssignedSeatsCountAttribute()), plus a third
// unReassignableCount() query for non-reassignable licenses —
// classic N+1 on a licenses-with-min_amt list.
$licenses = License::select('id', 'name', 'min_amt')
->withCount('availCount as licenses_available')
->where('min_amt', '>', 0)
->groupBy('licenses.id')
->havingRaw('licenses_available < (min_amt + ?)', [$alert_threshold])
->get();
$items_array = [];
$all_count = 0;
foreach ($consumables as $consumable) {
$avail = $consumable->numRemaining();
if ($avail < ($consumable->min_amt) + $alert_threshold) {
if ($consumable->qty > 0) {
$percent = number_format((($avail / $consumable->qty) * 100), 0);
} else {
$percent = 100;
}
$avail = $consumable->qty - $consumable->consumables_users_count;
$percent = $consumable->qty > 0
? number_format((($avail / $consumable->qty) * 100), 0)
: 100;
$items_array[$all_count]['id'] = $consumable->id;
$items_array[$all_count]['name'] = $consumable->name;
$items_array[$all_count]['type'] = 'consumables';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $consumable->min_amt;
$all_count++;
}
$items_array[$all_count]['id'] = $consumable->id;
$items_array[$all_count]['name'] = $consumable->name;
$items_array[$all_count]['type'] = 'consumables';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $consumable->min_amt;
$all_count++;
}
foreach ($accessories as $accessory) {
$avail = $accessory->qty - $accessory->checkouts_count;
if ($avail < ($accessory->min_amt) + $alert_threshold) {
if ($accessory->qty > 0) {
$percent = number_format((($avail / $accessory->qty) * 100), 0);
} else {
$percent = 100;
}
$percent = $accessory->qty > 0
? number_format((($avail / $accessory->qty) * 100), 0)
: 100;
$items_array[$all_count]['id'] = $accessory->id;
$items_array[$all_count]['name'] = $accessory->name;
$items_array[$all_count]['type'] = 'accessories';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $accessory->min_amt;
$all_count++;
}
$items_array[$all_count]['id'] = $accessory->id;
$items_array[$all_count]['name'] = $accessory->name;
$items_array[$all_count]['type'] = 'accessories';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $accessory->min_amt;
$all_count++;
}
foreach ($components as $component) {
$avail = $component->numRemaining();
if ($avail < ($component->min_amt) + $alert_threshold) {
if ($component->qty > 0) {
$percent = number_format((($avail / $component->qty) * 100), 0);
} else {
$percent = 100;
}
$avail = $component->qty - $component->sum_unconstrained_assets;
$percent = $component->qty > 0
? number_format((($avail / $component->qty) * 100), 0)
: 100;
$items_array[$all_count]['id'] = $component->id;
$items_array[$all_count]['name'] = $component->name;
$items_array[$all_count]['type'] = 'components';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $component->min_amt;
$all_count++;
}
$items_array[$all_count]['id'] = $component->id;
$items_array[$all_count]['name'] = $component->name;
$items_array[$all_count]['type'] = 'components';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $component->min_amt;
$all_count++;
}
foreach ($asset_models as $asset_model) {
$total_owned = $asset_model->assets_count;
$avail = $asset_model->available_assets_count;
$percent = $avail > 0
? number_format((($avail / $total_owned) * 100), 0)
: 100;
$asset = new Asset;
$total_owned = $asset_model->assets_count; // requires the withCount() clause in the initial query!
$avail = $asset_model->available_assets_count; // requires the withCount() clause in the initial query!
if ($avail < ($asset_model->min_amt) + $alert_threshold) {
if ($avail > 0) {
$percent = number_format((($avail / $total_owned) * 100), 0);
} else {
$percent = 100;
}
$items_array[$all_count]['id'] = $asset_model->id;
$items_array[$all_count]['name'] = $asset_model->name;
$items_array[$all_count]['type'] = 'models';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $asset_model->min_amt;
$all_count++;
}
$items_array[$all_count]['id'] = $asset_model->id;
$items_array[$all_count]['name'] = $asset_model->name;
$items_array[$all_count]['type'] = 'models';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $asset_model->min_amt;
$all_count++;
}
foreach ($licenses as $license) {
$avail = $license->remaincount();
if ($avail < ($license->min_amt) + $alert_threshold) {
if ($avail > 0) {
$percent = number_format((($avail / $license->min_amt) * 100), 0);
} else {
$percent = 100;
}
$items_array[$all_count]['id'] = $license->id;
$items_array[$all_count]['name'] = $license->name;
$items_array[$all_count]['type'] = 'licenses';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $license->min_amt;
$all_count++;
}
$avail = $license->licenses_available;
$percent = $avail > 0
? number_format((($avail / $license->min_amt) * 100), 0)
: 100;
$items_array[$all_count]['id'] = $license->id;
$items_array[$all_count]['name'] = $license->name;
$items_array[$all_count]['type'] = 'licenses';
$items_array[$all_count]['percent'] = $percent;
$items_array[$all_count]['remaining'] = $avail;
$items_array[$all_count]['min_amt'] = $license->min_amt;
$all_count++;
}
return $items_array;

View File

@ -0,0 +1,48 @@
<?php
namespace App\Livewire;
use App\Helpers\Helper;
use Illuminate\View\View;
use Livewire\Attributes\Lazy;
use Livewire\Component;
/**
* Top-nav alert bell. Wraps the old blade/alert-menu.blade.php partial so
* the low-inventory + deprecation queries no longer block first-paint.
*
* `#[Lazy]` makes the component render a lightweight placeholder on the
* initial page load, then Livewire fires a second XHR to hydrate the
* real body — so `Helper::checkLowInventory()` and
* `Helper::deprecationCheck()` (both cached at the helper layer with
* observer-driven invalidation) never sit on the critical render path.
*
* The setting-gate lives on the tag in layouts/default.blade.php:
* `@if ($snipeSettings->show_alerts_in_menu == '1') <livewire:alert-menu />`
* so nothing about this component runs when the operator turned the
* bell off.
*/
#[Lazy]
class AlertMenu extends Component
{
public function placeholder(): string
{
// Reserve the same visual footprint as the loaded bell so the
// top-nav doesn't shift when the real component swaps in.
return <<<'HTML'
<li class="dropdown tasks-menu" aria-busy="true">
<a href="#" class="dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
<i class="fa fa-bell" aria-hidden="true"></i>
</a>
</li>
HTML;
}
public function render(): View
{
return view('livewire.alert-menu', [
'alert_items' => Helper::checkLowInventory(),
'deprecations' => Helper::deprecationCheck(),
]);
}
}

View File

@ -1,90 +0,0 @@
@props([
'snipeSettings' => \App\Models\Setting::getSettings(),
])
@if ($snipeSettings->show_alerts_in_menu=='1')
<!-- Tasks: style can be found in dropdown.less -->
<?php
$alert_items = \App\Helpers\Helper::checkLowInventory();
$deprecations = \App\Helpers\Helper::deprecationCheck();
?>
<li class="dropdown tasks-menu">
<a href="#" class="dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
<x-icon type="alerts" />
<span class="sr-only">{{ trans('general.alerts') }}</span>
@if(count($alert_items) + count($deprecations))
<span class="label label-danger">{{ count($alert_items) + count($deprecations)}}</span>
@endif
</a>
<ul class="dropdown-menu">
@if ((count($alert_items) + count($deprecations)) > 0)
@can('superadmin')
@if($deprecations)
@foreach ($deprecations as $key => $deprecation)
@if ($deprecation['check'])
<li class="header alert-warning">{!! $deprecation['message'] !!}</li>
@endif
@endforeach
@endif
@endcan
@if($alert_items)
<li class="header">
{{ trans_choice('general.quantity_minimum', count($alert_items)) }}
</li>
<li>
<!-- inner menu: contains the actual data -->
<ul class="menu">
@if (count($alert_items) <= 50)
@for($i = 0; count($alert_items) > $i; $i++)
<!-- Task item -->
<li>
<a href="{{ route($alert_items[$i]['type'].'.show', $alert_items[$i]['id'])}}">
<h2 class="task_menu">{{ $alert_items[$i]['name'] }}
<small class="pull-right">
{{ $alert_items[$i]['remaining'] }} {{ trans('general.remaining') }}
</small>
</h2>
<div class="progress xs">
<div class="progress-bar progress-bar-yellow"
style="width: {{ $alert_items[$i]['percent'] }}%"
role="progressbar"
aria-valuenow="{{ $alert_items[$i]['percent'] }}"
aria-valuemin="0"
aria-valuemax="100">
<span class="sr-only">
{{ $alert_items[$i]['percent'] }}%
</span>
</div>
</div>
</a>
</li>
<!-- end task item -->
@endfor
@endif
</ul>
</li>
@endif
@else
<li class="header">
{{ trans_choice('general.quantity_minimum', 0) }}
</li>
@endif
{{-- <li class="footer">--}}
{{-- <a href="#">{{ trans('general.tasks_view_all') }}</a>--}}
{{-- </li>--}}
</ul>
</li>
@if (!$slot->isEmpty())
{{ $slot }}
@endif
@endif

View File

@ -1493,7 +1493,9 @@
@endcan
@can('admin')
<x-alert-menu />
@if ($snipeSettings->show_alerts_in_menu == '1')
<livewire:alert-menu/>
@endif
@endcan

View File

@ -0,0 +1,67 @@
{{-- Top-nav alert bell. Hydrated lazily via <livewire:alert-menu /> in
layouts/default.blade.php. The setting-gate lives on the parent
tag this view assumes the operator opted in. --}}
<li class="dropdown tasks-menu">
<a href="#" class="dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
<x-icon type="alerts"/>
<span class="sr-only">{{ trans('general.alerts') }}</span>
@if (count($alert_items) + count($deprecations))
<span class="label label-danger">{{ count($alert_items) + count($deprecations) }}</span>
@endif
</a>
<ul class="dropdown-menu">
@if ((count($alert_items) + count($deprecations)) > 0)
@can('superadmin')
@if ($deprecations)
@foreach ($deprecations as $deprecation)
@if ($deprecation['check'])
<li class="header alert-warning">{!! $deprecation['message'] !!}</li>
@endif
@endforeach
@endif
@endcan
@if ($alert_items)
<li class="header">
{{ trans_choice('general.quantity_minimum', count($alert_items)) }}
</li>
<li>
<ul class="menu">
{{-- Cap the visual list at 50 to keep the dropdown a
sensible height even when a big deployment has
hundreds of low-inventory items simultaneously. --}}
@if (count($alert_items) <= 50)
@foreach ($alert_items as $alert_item)
<li>
<a href="{{ route($alert_item['type'].'.show', $alert_item['id']) }}">
<h2 class="task_menu">{{ $alert_item['name'] }}
<small class="pull-right">
{{ $alert_item['remaining'] }} {{ trans('general.remaining') }}
</small>
</h2>
<div class="progress xs">
<div class="progress-bar progress-bar-yellow"
style="width: {{ $alert_item['percent'] }}%"
role="progressbar"
aria-valuenow="{{ $alert_item['percent'] }}"
aria-valuemin="0"
aria-valuemax="100">
<span class="sr-only">{{ $alert_item['percent'] }}%</span>
</div>
</div>
</a>
</li>
@endforeach
@endif
</ul>
</li>
@endif
@else
<li class="header">
{{ trans_choice('general.quantity_minimum', 0) }}
</li>
@endif
</ul>
</li>

View File

@ -0,0 +1,82 @@
<?php
namespace Tests\Feature\Console;
use App\Models\Consumable;
use Tests\TestCase;
/**
* The snipeit:inventory-alerts artisan command is the daily cron consumer of
* Helper::checkLowInventory(). It had no test coverage before this file was
* added — meaning a refactor to the helper (like the havingRaw / SQL-side
* filter change) could silently break the daily email without failing a
* test. This file locks in the command's end-to-end behavior: sends when
* items are low, skips when they aren't, no-ops when disabled.
*/
class SendInventoryAlertsTest extends TestCase
{
// Assertions target artisan output rather than Mail::/Notification::fake,
// because InventoryAlert routes through Notification::send() with an
// AlertRecipient (Notifiable trait, no Eloquent parent), and the fake
// helpers call ->getKey() on the notifiable during send-time tracking.
// Output-level assertions still catch what matters: the command
// completes without exception, took the correct branch, and reported
// the right thing to cron.
public function test_reports_low_inventory_count_when_items_are_low(): void
{
$this->settings->set([
'alerts_enabled' => 1,
'alert_email' => 'ops@example.test',
'alert_threshold' => 0,
]);
Consumable::factory()->create(['qty' => 0, 'min_amt' => 1]);
$this->artisan('snipeit:inventory-alerts')
->expectsOutputToContain('below minimum inventory')
->assertExitCode(0);
}
public function test_reports_nothing_to_send_when_no_low_inventory(): void
{
$this->settings->set([
'alerts_enabled' => 1,
'alert_email' => 'ops@example.test',
'alert_threshold' => 0,
]);
Consumable::factory()->create(['qty' => 10, 'min_amt' => 1]);
$this->artisan('snipeit:inventory-alerts')
->expectsOutputToContain('No low inventory items found')
->assertExitCode(0);
}
public function test_no_op_when_alerts_disabled_in_settings(): void
{
$this->settings->set([
'alerts_enabled' => 0,
'alert_email' => 'ops@example.test',
]);
Consumable::factory()->create(['qty' => 0, 'min_amt' => 1]);
$this->artisan('snipeit:inventory-alerts')
->expectsOutputToContain('Alerts are disabled')
->assertExitCode(0);
}
public function test_no_op_when_alert_email_is_blank(): void
{
$this->settings->set([
'alerts_enabled' => 1,
'alert_email' => '',
]);
Consumable::factory()->create(['qty' => 0, 'min_amt' => 1]);
$this->artisan('snipeit:inventory-alerts')
->expectsOutputToContain('No alert email configured')
->assertExitCode(0);
}
}

View File

@ -9,6 +9,8 @@ use App\Models\AssetModel;
use App\Models\Component;
use App\Models\Consumable;
use App\Models\License;
use App\Models\User;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;
class CheckLowInventoryTest extends TestCase
@ -209,6 +211,112 @@ class CheckLowInventoryTest extends TestCase
);
}
/**
* Checkouts reduce effective availability: an accessory with qty=5
* and min_amt=3 is above the floor at rest, but with 3 units checked
* out only 2 remain — below the floor — so the alert must fire.
* Guards against the havingRaw filter silently ignoring the
* checkouts_count subquery.
*/
public function test_accessory_with_checkouts_below_min_amt_is_flagged()
{
$this->settings->set(['alert_threshold' => 0]);
$accessory = Accessory::factory()->create(['qty' => 5, 'min_amt' => 3]);
$user = User::factory()->create();
$accessory->checkouts()->createMany([
['assigned_to' => $user->id, 'assigned_type' => User::class, 'created_by' => 1],
['assigned_to' => $user->id, 'assigned_type' => User::class, 'created_by' => 1],
['assigned_to' => $user->id, 'assigned_type' => User::class, 'created_by' => 1],
]);
$this->assertContains(
$accessory->id,
$this->idsForType(Helper::checkLowInventory(), 'accessories'),
);
}
public function test_accessory_with_checkouts_still_above_min_amt_is_not_flagged()
{
$this->settings->set(['alert_threshold' => 0]);
// 5 total, 1 checked out → 4 remaining, min is 3 → above the floor.
$accessory = Accessory::factory()->create(['qty' => 5, 'min_amt' => 3]);
$user = User::factory()->create();
$accessory->checkouts()->create([
'assigned_to' => $user->id,
'assigned_type' => User::class,
'created_by' => 1,
]);
$this->assertNotContains(
$accessory->id,
$this->idsForType(Helper::checkLowInventory(), 'accessories'),
);
}
/**
* License seats are created by the License::created observer, and
* assigning a seat should drop the "available" count. Locks in the
* behaviour of the licenses_available withCount alias — the previous
* code path went through $license->remaincount() which fires
* additional N+1 queries and could drift from the alias.
*/
public function test_license_with_assigned_seats_below_min_amt_is_flagged()
{
$this->settings->set(['alert_threshold' => 0]);
// 5 seats, min 3 → healthy at rest. Assign 3 → 2 available → below min.
$license = License::factory()->create(['seats' => 5, 'min_amt' => 3]);
$seats = $license->licenseseats()->orderBy('id')->take(3)->get();
foreach ($seats as $seat) {
$seat->update(['assigned_to' => User::factory()->create()->id]);
}
$this->assertContains(
$license->id,
$this->idsForType(Helper::checkLowInventory(), 'licenses'),
);
}
/**
* Regression pin for the query-count refactor: the whole checkLowInventory
* call should fire a bounded, small number of queries regardless of how
* many low-inventory rows exist across each category. If someone
* accidentally re-introduces a $model->numRemaining() / remaincount()
* call in the foreach loops this will spike well past the ceiling.
*/
public function test_check_low_inventory_query_count_stays_bounded()
{
$this->settings->set(['alert_threshold' => 0]);
// Populate each category with several low-inventory rows so the
// foreach paths that used to trigger extra per-row queries actually
// execute.
Consumable::factory()->count(3)->create(['qty' => 0, 'min_amt' => 1]);
Accessory::factory()->count(3)->create(['qty' => 0, 'min_amt' => 1]);
Component::factory()->count(3)->create(['qty' => 1, 'min_amt' => 5]);
AssetModel::factory()->count(3)->create(['min_amt' => 5]);
License::factory()->count(3)->create(['seats' => 1, 'min_amt' => 5]);
DB::flushQueryLog();
DB::enableQueryLog();
Helper::checkLowInventory();
// Ceiling covers: 1 settings + 5 category queries + 1 status_labels
// lookup for the RTD() scope in the AssetModel branch = 7. Held to
// 10 to leave a little headroom for a future settings-related
// memoization change without loosening the N+1 guard.
$count = count(DB::getQueryLog());
$this->assertLessThanOrEqual(
10,
$count,
"checkLowInventory fired {$count} queries — expected <=10. Something is doing per-row DB access again.",
);
}
private function idsForType(array $items, string $type): array
{
return collect($items)

View File

@ -0,0 +1,26 @@
<?php
namespace Tests\Feature\Livewire;
use App\Livewire\AlertMenu;
use App\Models\User;
use Livewire\Livewire;
use Tests\TestCase;
class AlertMenuTest extends TestCase
{
public function test_the_component_renders(): void
{
Livewire::actingAs(User::factory()->superuser()->create())
->test(AlertMenu::class)
->assertStatus(200);
}
public function test_placeholder_reserves_bell_footprint(): void
{
$placeholder = (new AlertMenu)->placeholder();
$this->assertStringContainsString('dropdown', $placeholder);
$this->assertStringContainsString('fa-bell', $placeholder);
}
}