Merge remote-tracking branch 'origin/master' into develop

This commit is contained in:
snipe
2026-05-29 10:58:33 +01:00
38 changed files with 1097 additions and 285 deletions
+26 -12
View File
@@ -609,6 +609,11 @@ class AssetsController extends Controller
])->with('model', 'status', 'assignedTo')
->NotArchived();
// When FMCS is enabled, automatically scope to companies the acting user belongs to.
// scopeCompanyables is a no-op for superusers and when FMCS is disabled.
$assets = Company::scopeCompanyables($assets);
// Allow further narrowing to a specific company passed via data-company-id on the select.
if ((Setting::getSettings()->full_multiple_companies_support == '1') && $request->filled('companyId')) {
$companyIds = array_values(array_filter(array_map('intval', explode(',', $request->input('companyId')))));
if (! empty($companyIds)) {
@@ -616,6 +621,10 @@ class AssetsController extends Controller
}
}
if ($request->filled('excludeId')) {
$assets->where('assets.id', '!=', (int) $request->input('excludeId'));
}
if ($request->filled('statusType') && $request->input('statusType') === 'RTD') {
$assets = $assets->RTD();
}
@@ -904,11 +913,21 @@ class AssetsController extends Controller
private function checkoutCompanyMismatchResponse(Asset $asset, User|Asset|Location $target): ?JsonResponse
{
if ((Setting::getSettings()->full_multiple_companies_support == '1')
&& (! is_null($asset->company_id))
&& (! is_null($target->company_id))
&& ((int) $asset->company_id !== (int) $target->company_id)
) {
if (Setting::getSettings()->full_multiple_companies_support != '1' || is_null($asset->company_id)) {
return null;
}
// For users with multiple companies, check all their associated companies,
// not just the primary company_id column.
if ($target instanceof User) {
if (! $target->canReceiveFromCompany((int) $asset->company_id)) {
return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_user_company')));
}
return null;
}
if (! is_null($target->company_id) && (int) $asset->company_id !== (int) $target->company_id) {
return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_user_company')));
}
@@ -1062,13 +1081,8 @@ class AssetsController extends Controller
}
// In FMCS mode, enforce explicit same-company target checks before mutating checkout state.
$targetCompanyId = data_get($target, 'company_id');
if ((Setting::getSettings()->full_multiple_companies_support == '1')
&& (! is_null($asset->company_id))
&& (! is_null($targetCompanyId))
&& ((int) $asset->company_id !== (int) $targetCompanyId)
) {
return response()->json(Helper::formatStandardApiResponse('error', $error_payload, trans('general.error_user_company')));
if ($mismatch = $this->checkoutCompanyMismatchResponse($asset, $target)) {
return $mismatch;
}
$checkout_at = request('checkout_at', date('Y-m-d H:i:s'));
@@ -9,6 +9,7 @@ use App\Http\Requests\ImageUploadRequest;
use App\Http\Transformers\CompaniesTransformer;
use App\Http\Transformers\SelectlistTransformer;
use App\Models\Company;
use App\Models\Setting;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Storage;
@@ -206,6 +207,16 @@ class CompaniesController extends Controller
'companies.tag_color',
]);
// When FMCS is enabled and the user is not a superuser, restrict the list to
// companies they belong to (primary company_id + pivot companies). This lets
// non-superusers select a company from their own set when creating assets, etc.
if (Setting::getSettings()->full_multiple_companies_support == '1' && ! auth()->user()->isSuperUser()) {
$userCompanyIds = auth()->user()->allCompanies()->pluck('id');
if ($userCompanyIds->isNotEmpty()) {
$companies->whereIn('companies.id', $userCompanyIds);
}
}
if ($request->filled('search')) {
$companies = $companies->where('companies.name', 'LIKE', '%'.$request->input('search').'%');
}
@@ -67,7 +67,18 @@ class LocationsController extends Controller
'notes',
];
$locations = Location::with('parent', 'manager', 'children')->select([
$locations = Location::with([
'parent',
'children',
'manager' => fn ($q) => $q->withCount([
'assets as assets_count',
'accessories as accessories_count',
'licenses as licenses_count',
'consumables as consumables_count',
'managesUsers as manages_users_count',
'managedLocations as manages_locations_count',
]),
])->select([
'locations.id',
'locations.name',
'locations.address',
@@ -103,7 +114,9 @@ class LocationsController extends Controller
->withCount('components as components_count')
->with('adminuser');
// Only scope locations if the setting is enabled
// scope_locations_fmcs is required for location-level company scoping (locations may not
// have company_id assigned unless the compatibility check has been completed in Settings).
// Without it, locations are visible to all authenticated users regardless of FMCS state.
if (Setting::getSettings()->scope_locations_fmcs) {
$locations = Company::scopeCompanyables($locations);
}
@@ -157,8 +170,6 @@ class LocationsController extends Controller
$locations->where('tag_color', '=', $request->input('locations.tag_color'));
}
// Make sure the offset and limit are actually integers and do not exceed system limits
$offset = ($request->input('offset') > $locations->count()) ? $locations->count() : app('api_offset_value');
$limit = app('api_limit_value');
$order = $request->input('order') === 'asc' ? 'asc' : 'desc';
@@ -180,6 +191,7 @@ class LocationsController extends Controller
}
$total = $locations->count();
$offset = ($request->input('offset') > $total) ? $total : app('api_offset_value');
$locations = $locations->skip($offset)->take($limit)->get();
return (new LocationsTransformer)->transformLocations($locations, $total);
@@ -227,7 +239,19 @@ class LocationsController extends Controller
public function show($id): JsonResponse|array
{
$this->authorize('view', Location::class);
$location = Location::with('parent', 'manager', 'children', 'company')
$location = Location::with([
'parent',
'children',
'company',
'manager' => fn ($q) => $q->withCount([
'assets as assets_count',
'accessories as accessories_count',
'licenses as licenses_count',
'consumables as consumables_count',
'managesUsers as manages_users_count',
'managedLocations as manages_locations_count',
]),
])
->select([
'locations.id',
'locations.name',
@@ -422,15 +446,6 @@ class LocationsController extends Controller
'locations.tag_color',
]);
// Only scope locations if the setting is enabled
if (Setting::getSettings()->scope_locations_fmcs) {
$locations = Company::scopeCompanyables($locations);
}
if ((Setting::getSettings()->full_multiple_companies_support == '1') && $request->filled('companyId')) {
$locations->where('locations.company_id', $request->input('companyId'));
}
$page = 1;
if ($request->filled('page')) {
$page = $request->input('page');
@@ -440,6 +455,10 @@ class LocationsController extends Controller
$locations = $locations->where('locations.name', 'LIKE', '%'.$request->input('search').'%');
}
if ($request->filled('excludeId')) {
$locations->where('locations.id', '!=', (int) $request->input('excludeId'));
}
$locations = $locations->orderBy('name', 'ASC')->get();
$locations_with_children = [];
@@ -396,6 +396,11 @@ class UsersController extends Controller
]
)->where('show_in_list', '=', '1');
// When FMCS is enabled, automatically scope to companies the acting user belongs to.
// scopeCompanyables is a no-op for superusers and when FMCS is disabled.
$users = Company::scopeCompanyables($users, 'company_id', 'users');
// Allow further narrowing to a specific company passed via data-company-ids on the select.
if ((Setting::getSettings()->full_multiple_companies_support == '1') && $request->filled('companyId')) {
$companyIds = array_values(array_filter(array_map('intval', explode(',', $request->input('companyId')))));
if (! empty($companyIds)) {
@@ -403,6 +408,10 @@ class UsersController extends Controller
}
}
if ($request->filled('excludeId')) {
$users->where('users.id', '!=', (int) $request->input('excludeId'));
}
if ($request->filled('search')) {
$users = $users->where(function ($query) use ($request) {
$query->SimpleNameSearch($request->input('search'))
@@ -121,9 +121,14 @@ class AssetCheckoutController extends Controller
$settings = Setting::getSettings();
// We have to check whether $target->company_id is null here since locations don't have a company yet
if (($settings->full_multiple_companies_support) && ((! is_null($target->company_id)) && (! is_null($asset->company_id)))) {
if ($target->company_id != $asset->company_id) {
// Locations have no company, so we only enforce FMCS when both sides have a company_id.
// For users with multiple companies, check all their associated companies via the pivot.
if ($settings->full_multiple_companies_support && ! is_null($asset->company_id)) {
$mismatch = $target instanceof User
? ! $target->canReceiveFromCompany((int) $asset->company_id)
: (! is_null($target->company_id) && (int) $target->company_id !== (int) $asset->company_id);
if ($mismatch) {
return redirect()->route('hardware.checkout.create', $asset)->with('error', trans('general.error_user_company'));
}
}
@@ -16,6 +16,7 @@ use App\Models\CustomField;
use App\Models\LicenseSeat;
use App\Models\Setting;
use App\Models\Statuslabel;
use App\Models\User;
use App\View\Label;
use Carbon\Carbon;
use Illuminate\Contracts\View\View;
@@ -687,18 +688,25 @@ class BulkAssetsController extends Controller
->with('error', trans('general.error_assets_already_checked_out'));
}
// Prevent checking out assets across companies if FMCS enabled
if (Setting::getSettings()->full_multiple_companies_support && $target->company_id) {
$company_ids = $assets->pluck('company_id')->unique();
// Prevent checking out assets across companies if FMCS enabled.
// For users with multiple companies, check all their associated companies via the pivot.
if (Setting::getSettings()->full_multiple_companies_support) {
$company_ids = $assets->pluck('company_id')->filter()->unique();
// if there is more than one unique company id or the singular company id does not match
// then the checkout is invalid
if ($company_ids->count() > 1 || $company_ids->first() != $target->company_id) {
// re-add the asset ids so the assets select is re-populated
$request->session()->flashInput(['selected_assets' => $asset_ids]);
if ($company_ids->isNotEmpty()) {
$assetCompanyId = (int) $company_ids->first();
return redirect(route('hardware.bulkcheckout.show'))
->with('error', trans('general.error_user_company_multiple'));
$mismatch = $company_ids->count() > 1
|| ($target instanceof User
? ! $target->canReceiveFromCompany($assetCompanyId)
: (! is_null($target->company_id) && (int) $target->company_id !== $assetCompanyId));
if ($mismatch) {
$request->session()->flashInput(['selected_assets' => $asset_ids]);
return redirect(route('hardware.bulkcheckout.show'))
->with('error', trans('general.error_user_company_multiple'));
}
}
}
-2
View File
@@ -2,7 +2,6 @@
namespace App\Http;
use App\Http\Middleware\AssetCountForSidebar;
use App\Http\Middleware\CheckColorSettings;
use App\Http\Middleware\CheckForDebug;
use App\Http\Middleware\CheckForSetup;
@@ -75,7 +74,6 @@ class Kernel extends HttpKernel
CheckUserIsActivated::class,
CheckForTwoFactor::class,
CreateFreshApiToken::class,
AssetCountForSidebar::class,
CheckColorSettings::class,
AuthenticateSession::class,
SubstituteBindings::class,
@@ -1,119 +0,0 @@
<?php
namespace App\Http\Middleware;
use App\Models\Asset;
use App\Models\Setting;
use Closure;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Log;
class AssetCountForSidebar
{
/**
* Handle an incoming request.
*
* @param Request $request
* @return mixed
*/
public function handle($request, Closure $next)
{
/**
* This needs to be set for the /setup process, since the tables might not exist yet
*/
$total_assets = 0;
$total_due_for_checkin = 0;
$total_overdue_for_checkin = 0;
$total_due_for_audit = 0;
$total_overdue_for_audit = 0;
try {
$settings = Setting::getSettings();
view()->share('settings', $settings);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_assets = Asset::AssetsForShow()->count();
view()->share('total_assets', $total_assets);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_rtd_sidebar = Asset::RTD()->count();
view()->share('total_rtd_sidebar', $total_rtd_sidebar);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_deployed_sidebar = Asset::Deployed()->count();
view()->share('total_deployed_sidebar', $total_deployed_sidebar);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_archived_sidebar = Asset::Archived()->count();
view()->share('total_archived_sidebar', $total_archived_sidebar);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_pending_sidebar = Asset::Pending()->count();
view()->share('total_pending_sidebar', $total_pending_sidebar);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_undeployable_sidebar = Asset::Undeployable()->count();
view()->share('total_undeployable_sidebar', $total_undeployable_sidebar);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_byod_sidebar = Asset::where('byod', '=', '1')->count();
view()->share('total_byod_sidebar', $total_byod_sidebar);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_due_for_audit = Asset::DueForAudit($settings)->count();
view()->share('total_due_for_audit', $total_due_for_audit);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_overdue_for_audit = Asset::OverdueForAudit()->count();
view()->share('total_overdue_for_audit', $total_overdue_for_audit);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_due_for_checkin = Asset::DueForCheckin($settings)->count();
view()->share('total_due_for_checkin', $total_due_for_checkin);
} catch (\Exception $e) {
Log::debug($e);
}
try {
$total_overdue_for_checkin = Asset::OverdueForCheckin()->count();
view()->share('total_overdue_for_checkin', $total_overdue_for_checkin);
} catch (\Exception $e) {
Log::debug($e);
}
view()->share('total_due_and_overdue_for_checkin', ($total_due_for_checkin + $total_overdue_for_checkin));
view()->share('total_due_and_overdue_for_audit', ($total_due_for_audit + $total_overdue_for_audit));
return $next($request);
}
}
+1 -1
View File
@@ -75,7 +75,7 @@ class UserImporter extends ItemImporter
// Pull the records from the CSV to determine their values
$this->item['id'] = trim($this->findCsvMatch($row, 'id'));
$this->item['username'] = trim($this->findCsvMatch($row, 'username'));
$this->item['display_name'] = trim($this->findCsvMatch($row, 'display_name'));
$this->item['display_name'] = trim($this->findCsvMatch($row, 'display_name')) ?: null;
$this->item['first_name'] = trim($this->findCsvMatch($row, 'first_name'));
$this->item['last_name'] = trim($this->findCsvMatch($row, 'last_name'));
$this->item['email'] = trim($this->findCsvMatch($row, 'email'));
+28 -42
View File
@@ -34,7 +34,7 @@ class Asset extends Depreciable
{
protected $presenter = AssetPresenter::class;
protected $with = ['model', 'adminuser', 'location', 'company'];
// protected $with = ['model', 'adminuser', 'location', 'company'];
use CompanyableTrait;
use HasFactory;
@@ -1482,13 +1482,10 @@ class Asset extends Depreciable
*/
public function scopePending($query)
{
return $query->whereHas(
'status', function ($query) {
$query->where('deployable', '=', 0)
->where('pending', '=', 1)
->where('archived', '=', 0);
}
);
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('deployable', 0)->where('pending', 1)->where('archived', 0)->whereNull('deleted_at')->pluck('id');
return $query->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
/**
@@ -1538,14 +1535,11 @@ class Asset extends Depreciable
*/
public function scopeRTD($query)
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('deployable', 1)->where('pending', 0)->where('archived', 0)->whereNull('deleted_at')->pluck('id');
return $query->whereNull('assets.assigned_to')
->whereHas(
'status', function ($query) {
$query->where('deployable', '=', 1)
->where('pending', '=', 0)
->where('archived', '=', 0);
}
);
->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
/**
@@ -1556,13 +1550,10 @@ class Asset extends Depreciable
*/
public function scopeUndeployable($query)
{
return $query->whereHas(
'status', function ($query) {
$query->where('deployable', '=', 0)
->where('pending', '=', 0)
->where('archived', '=', 0);
}
);
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('deployable', 0)->where('pending', 0)->where('archived', 0)->whereNull('deleted_at')->pluck('id');
return $query->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
/**
@@ -1573,11 +1564,10 @@ class Asset extends Depreciable
*/
public function scopeNotArchived($query)
{
return $query->whereHas(
'status', function ($query) {
$query->where('archived', '=', 0);
}
);
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('archived', 0)->whereNull('deleted_at')->pluck('id');
return $query->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
/**
@@ -1740,17 +1730,16 @@ class Asset extends Depreciable
*/
public function scopeAssetsForShow($query)
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
if (Setting::getSettings()->show_archived_in_list != 1) {
return $query->whereHas(
'status', function ($query) {
$query->where('archived', '=', 0);
}
);
} else {
return $query;
$validStatusIds = Statuslabel::where('archived', 0)
->whereNull('deleted_at')
->pluck('id');
return $query->whereIn('assets.status_id', $validStatusIds->isEmpty() ? [0] : $validStatusIds);
}
return $query;
}
/**
@@ -1761,13 +1750,10 @@ class Asset extends Depreciable
*/
public function scopeArchived($query)
{
return $query->whereHas(
'status', function ($query) {
$query->where('deployable', '=', 0)
->where('pending', '=', 0)
->where('archived', '=', 1);
}
);
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('deployable', 0)->where('pending', 0)->where('archived', 1)->whereNull('deleted_at')->pluck('id');
return $query->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
/**
+1 -1
View File
@@ -269,7 +269,7 @@ final class Company extends SnipeModel
{
return ! self::isFullMultipleCompanySupportEnabled()
|| auth()->user()->isSuperUser()
|| empty(self::getCurrentUserCompanyIds());
|| ! empty(self::getCurrentUserCompanyIds());
}
/**
+6 -7
View File
@@ -170,14 +170,13 @@ class Location extends SnipeModel
*/
public function assets()
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where(function ($q) {
$q->where('deployable', 1)->orWhere('pending', 1)->orWhere('archived', 0);
})->whereNull('deleted_at')->pluck('id');
return $this->hasMany(Asset::class, 'location_id')
->whereHas(
'status', function ($query) {
$query->where('status_labels.deployable', '=', 1)
->orWhere('status_labels.pending', '=', 1)
->orWhere('status_labels.archived', '=', 0);
}
);
->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
public function countAllTheThings()
+1 -10
View File
@@ -2,9 +2,7 @@
namespace App\Models\Traits;
use App\Models\Company\Company;
use App\Models\CompanyableScope;
use App\Models\Setting;
trait CompanyableTrait
{
@@ -18,13 +16,6 @@ trait CompanyableTrait
*/
public static function bootCompanyableTrait()
{
// In Version 7.0 and before locations weren't scoped by companies, so add a check for the backward compatibility setting
if (__CLASS__ != 'App\Models\Location') {
static::addGlobalScope(new CompanyableScope);
} else {
if (Setting::getSettings()?->scope_locations_fmcs == 1) {
static::addGlobalScope(new CompanyableScope);
}
}
static::addGlobalScope(new CompanyableScope);
}
}
+42 -2
View File
@@ -323,7 +323,7 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
protected function displayName(): Attribute
{
return Attribute::make(
get: fn (mixed $value) => $value ?? $this->getFullNameAttribute(),
get: fn (mixed $value) => ($value !== null && $value !== '') ? $value : $this->getFullNameAttribute(),
);
}
@@ -601,7 +601,6 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
&& (($this->accessories_count ?? $this->accessories()->count()) === 0)
&& (($this->licenses_count ?? $this->licenses()->count()) === 0)
&& (($this->consumables_count ?? $this->consumables()->count()) === 0)
&& (($this->accessories_count ?? $this->accessories()->count()) === 0)
&& (($this->manages_users_count ?? $this->managesUsers()->count()) === 0)
&& (($this->manages_locations_count ?? $this->managedLocations()->count()) === 0)
&& ($this->deleted_at == '');
@@ -626,6 +625,47 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
return $this->belongsToMany(Company::class, 'company_user');
}
/**
* Returns whether an FMCS company check should block this user from receiving
* an asset that belongs to the given company.
*
* - If the user has no company associations at all: returns true (no restriction).
* - If the user has associations: returns true only when $companyId is among them.
*
* Checks both the primary company_id column and the many-to-many pivot table so
* that users assigned to multiple companies can receive assets from any of them.
*/
public function canReceiveFromCompany(int $companyId): bool
{
// Primary company matches
if (! is_null($this->company_id) && (int) $this->company_id === $companyId) {
return true;
}
// Pivot company matches
if ($this->companies()->where('companies.id', $companyId)->exists()) {
return true;
}
// User has no company associations — don't enforce (mirrors legacy behaviour
// where a null company_id on the user skipped the FMCS check entirely).
if (is_null($this->company_id) && ! $this->companies()->exists()) {
return true;
}
return false;
}
/**
* Returns all companies this user belongs to union of the primary company_id
* column and the many-to-many pivot as a deduplicated Collection.
* Used to scope FMCS dropdowns to companies the user is allowed to work with.
*/
public function allCompanies(): Collection
{
return $this->companies->push($this->company)->filter()->unique('id')->values();
}
/**
* Sync company pivot membership and log the change if the set of companies changed.
*
+4
View File
@@ -23,11 +23,13 @@ use App\Observers\LocationObserver;
use App\Observers\MaintenanceObserver;
use App\Observers\SettingObserver;
use App\Observers\UserObserver;
use App\View\Composers\SidebarComposer;
use Illuminate\Pagination\Paginator;
use Illuminate\Routing\UrlGenerator;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Schema;
use Illuminate\Support\Facades\URL;
use Illuminate\Support\Facades\View;
use Illuminate\Support\ServiceProvider;
use Rollbar\Laravel\RollbarServiceProvider;
@@ -75,6 +77,8 @@ class AppServiceProvider extends ServiceProvider
Paginator::useBootstrap();
View::composer('layouts.default', SidebarComposer::class);
Schema::defaultStringLength(191);
Accessory::observe(AccessoryObserver::class);
Asset::observe(AssetObserver::class);
@@ -30,6 +30,7 @@ class SettingsServiceProvider extends ServiceProvider
// Share common setting variables with all views.
view()->composer('*', function ($view) {
$view->with('snipeSettings', Setting::getSettings());
$view->with('settings', Setting::getSettings());
});
/**
+54
View File
@@ -0,0 +1,54 @@
<?php
// A View Composer is a callback Laravel runs right before a specific view renders.
// It's registered in AppServiceProvider bound to 'layouts.default', so it only fires
// when a full page is rendered — not on modal AJAX responses, select2 searches, or
// API requests. This replaces the old AssetCountForSidebar middleware, which ran on
// every web request regardless of what was returned.
namespace App\View\Composers;
use App\Models\Asset;
use App\Models\Setting;
use Illuminate\Support\Facades\Log;
use Illuminate\View\View;
class SidebarComposer
{
public function compose(View $view): void
{
// Guard against the setup wizard, where DB tables may not exist yet
try {
$settings = Setting::getSettings();
} catch (\Exception $e) {
Log::debug($e);
return;
}
try {
$due_for_checkin = Asset::DueForCheckin($settings)->count();
$overdue_for_checkin = Asset::OverdueForCheckin()->count();
$due_for_audit = Asset::DueForAudit($settings)->count();
$overdue_for_audit = Asset::OverdueForAudit()->count();
$view->with([
'total_assets' => Asset::AssetsForShow()->count(),
'total_rtd_sidebar' => Asset::RTD()->count(),
'total_deployed_sidebar' => Asset::Deployed()->count(),
'total_archived_sidebar' => Asset::Archived()->count(),
'total_pending_sidebar' => Asset::Pending()->count(),
'total_undeployable_sidebar' => Asset::Undeployable()->count(),
'total_byod_sidebar' => Asset::where('byod', 1)->count(),
'total_due_for_audit' => $due_for_audit,
'total_overdue_for_audit' => $overdue_for_audit,
'total_due_for_checkin' => $due_for_checkin,
'total_overdue_for_checkin' => $overdue_for_checkin,
'total_due_and_overdue_for_checkin' => $due_for_checkin + $overdue_for_checkin,
'total_due_and_overdue_for_audit' => $due_for_audit + $overdue_for_audit,
]);
} catch (\Exception $e) {
Log::debug($e);
}
}
}
+5 -5
View File
@@ -2,10 +2,10 @@
return [
'app_version' => 'v8.6.1',
'full_app_version' => 'v8.6.1 - build 22962-g4edf40acaf',
'build_version' => '22962',
'full_app_version' => 'v8.6.1 - build 22854-gcfa8069953',
'build_version' => '22854',
'prerelease_version' => '',
'hash_version' => 'g4edf40acaf',
'full_hash' => 'v8.6.1-149-g4edf40acaf',
'branch' => 'develop',
'hash_version' => 'gcfa8069953',
'full_hash' => 'v8.6.1-195-gcfa8069953',
'branch' => 'master',
];
+11 -3
View File
@@ -52318,7 +52318,8 @@ $(function () {
search: params.term,
page: params.page || 1,
statusType: link.data("asset-status-type"),
companyId: link.data("company-ids") || link.data("company-id")
companyId: link.data("company-ids") || link.data("company-id"),
excludeId: link.data("exclude-id")
};
return data;
},
@@ -52541,8 +52542,15 @@ $(function () {
syncCheckoutToTypeUi(true);
});
// Apply the current radio selection on initial render.
syncCheckoutToTypeUi(false);
// Apply the current radio selection on initial render, but only when the
// selector row itself is already visible. On the asset create page the selector
// starts hidden (display:none) and user_add() reveals it after a deployability
// AJAX check — running here would prematurely show a panel before the radio
// group is visible. On the standalone checkout page the selector is visible
// from the start, so the sync runs normally there.
if ($('#assignto_selector').is(':visible')) {
syncCheckoutToTypeUi(false);
}
});
// ------------------------------------------------
+1 -1
View File
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -1,5 +1,5 @@
{
"/js/dist/all.js": "/js/dist/all.js?id=4619b48bfce17ad41fc5a2e9ee578988",
"/js/dist/all.js": "/js/dist/all.js?id=b7b86fa704f44a30a4593132fd496fee",
"/css/build/overrides.css": "/css/build/overrides.css?id=c173dd71d56c1089bf560a849586d93e",
"/css/build/app.css": "/css/build/app.css?id=63ef76491d01db361ad53cf1c8c7114f",
"/css/build/AdminLTE.css": "/css/build/AdminLTE.css?id=ee0ed88465dd878588ed044eefb67723",
+10 -2
View File
@@ -211,6 +211,7 @@ $(function () {
page: params.page || 1,
statusType: link.data("asset-status-type"),
companyId: link.data("company-ids") || link.data("company-id"),
excludeId: link.data("exclude-id"),
};
return data;
},
@@ -468,8 +469,15 @@ $(function () {
syncCheckoutToTypeUi(true);
});
// Apply the current radio selection on initial render.
syncCheckoutToTypeUi(false);
// Apply the current radio selection on initial render, but only when the
// selector row itself is already visible. On the asset create page the selector
// starts hidden (display:none) and user_add() reveals it after a deployability
// AJAX check — running here would prematurely show a panel before the radio
// group is visible. On the standalone checkout page the selector is visible
// from the start, so the sync runs normally there.
if ($('#assignto_selector').is(':visible')) {
syncCheckoutToTypeUi(false);
}
});
+1 -1
View File
@@ -121,7 +121,7 @@
@include ('partials.forms.checkout-selector', ['user_select' => 'true','asset_select' => 'true', 'location_select' => 'true'])
@include ('partials.forms.edit.user-select', ['translated_name' => trans('general.user'), 'fieldname' => 'assigned_user', 'company_id' => $asset->company_id, 'style' => (session('checkout_to_type') ?: 'user') == 'user' ? '' : 'display: none;'])
<!-- We have to pass unselect here so that we don't default to the asset that's being checked out. We want that asset to be pre-selected everywhere else. -->
@include ('partials.forms.edit.asset-select', ['translated_name' => trans('general.select_asset'), 'fieldname' => 'assigned_asset', 'company_id' => $asset->company_id, 'unselect' => 'true', 'style' => session('checkout_to_type') == 'asset' ? '' : 'display: none;'])
@include ('partials.forms.edit.asset-select', ['translated_name' => trans('general.select_asset'), 'fieldname' => 'assigned_asset', 'company_id' => $asset->company_id, 'unselect' => 'true', 'exclude_id' => $asset->id, 'style' => session('checkout_to_type') == 'asset' ? '' : 'display: none;'])
@include ('partials.forms.edit.location-select', ['translated_name' => trans('general.location'), 'fieldname' => 'assigned_location', 'company_id' => $asset->company_id, 'style' => session('checkout_to_type') == 'location' ? '' : 'display: none;'])
+10 -5
View File
@@ -100,9 +100,9 @@
@include ('partials.forms.edit.status', [ 'required' => 'true'])
@if (!$item->id)
@include ('partials.forms.checkout-selector', ['user_select' => 'true','asset_select' => 'true', 'location_select' => 'true', 'style' => 'display:none;'])
@include ('partials.forms.edit.user-select', ['translated_name' => trans('admin/hardware/form.checkout_to'), 'fieldname' => 'assigned_user', 'style' => 'display:none;', 'required' => 'false'])
@include ('partials.forms.edit.asset-select', ['translated_name' => trans('admin/hardware/form.checkout_to'), 'fieldname' => 'assigned_asset', 'style' => 'display:none;', 'required' => 'false'])
@include ('partials.forms.edit.location-select', ['translated_name' => trans('admin/hardware/form.checkout_to'), 'fieldname' => 'assigned_location', 'style' => 'display:none;', 'required' => 'false'])
@include ('partials.forms.edit.user-select', ['translated_name' => trans('general.user'), 'fieldname' => 'assigned_user', 'style' => 'display:none;', 'required' => 'false'])
@include ('partials.forms.edit.asset-select', ['translated_name' => trans('general.asset'), 'fieldname' => 'assigned_asset', 'style' => 'display:none;', 'required' => 'false'])
@include ('partials.forms.edit.location-select', ['translated_name' => trans('general.location'), 'fieldname' => 'assigned_location', 'style' => 'display:none;', 'required' => 'false'])
@endif
@include ('partials.forms.edit.notes')
@@ -282,16 +282,21 @@
$("#selected_status_status").fadeIn();
if (data == true) {
var checkoutType = $('input[name=checkout_to_type]:checked').val() || 'user';
$("#assignto_selector").show();
$("#assigned_user").show();
$("#assigned_user").toggle(checkoutType === 'user');
$("#assigned_asset").toggle(checkoutType === 'asset');
$("#assigned_location").toggle(checkoutType === 'location');
$("#selected_status_status").removeClass('text-danger');
$("#selected_status_status").addClass('text-success');
$("#selected_status_status").html('<x-icon type="checkmark" /> {{ trans_choice('admin/hardware/form.asset_deployable', 1)}}');
} else {
$("#assignto_selector").hide();
$("#assigned_user").hide();
$("#assigned_asset").hide();
$("#assigned_location").hide();
$("#selected_status_status").removeClass('text-success');
$("#selected_status_status").addClass('text-danger');
$("#selected_status_status").html('<x-icon type="warning" /> {{ (($item->assigned_to!='') && ($item->assigned_type!='') && ($item->deleted_at == '')) ? trans_choice('admin/hardware/form.asset_not_deployable_checkin', 1) : trans('admin/hardware/form.asset_not_deployable') }} ');
+1 -1
View File
@@ -10,7 +10,7 @@
@include ('partials.forms.edit.name', ['translated_name' => trans('admin/locations/table.name')])
<!-- parent -->
@include ('partials.forms.edit.location-select', ['translated_name' => trans('admin/locations/table.parent'), 'fieldname' => 'parent_id'])
@include ('partials.forms.edit.location-select', ['translated_name' => trans('admin/locations/table.parent'), 'fieldname' => 'parent_id', 'exclude_id' => isset($item) ? $item->id : null])
<!-- Manager-->
@include ('partials.forms.edit.user-select', ['translated_name' => trans('admin/users/table.manager'), 'fieldname' => 'manager_id'])
@@ -13,6 +13,7 @@
{{ ((isset($multiple)) && ($multiple === true)) ? ' multiple' : '' }}
{!! (!empty($asset_status_type)) ? ' data-asset-status-type="' . $asset_status_type . '"' : '' !!}
{!! (!empty($company_id)) ? ' data-company-id="' .$company_id.'"' : '' !!}
{!! (!empty($exclude_id)) ? ' data-exclude-id="'.e($exclude_id).'"' : '' !!}
{{ ((isset($required) && ($required =='true'))) ? ' required' : '' }}
>
@@ -1,47 +1,27 @@
<!-- Company -->
@if (($snipeSettings->full_multiple_companies_support=='1') && (!Auth::user()->isSuperUser()))
<!-- full company support is enabled and this user isn't a superadmin -->
<div class="form-group">
<label for="{{ $fieldname }}" class="col-md-3 control-label">{{ $translated_name }}</label>
<div class="col-md-6">
<select class="js-data-ajax" disabled data-endpoint="companies"
data-placeholder="{{ trans('general.select_company') }}" name="{{ $fieldname }}{{ (isset($multiple) && ($multiple=='true')) ? '[]' : '' }}" style="width: 100%"
aria-label="{{ $fieldname }}"{{ (isset($multiple) && ($multiple=='true')) ? " multiple='multiple'" : '' }}>
<!-- When FMCS is enabled the companies selectlist API automatically scopes results to
the current user's companies (primary + pivot), so no separate disabled branch is needed. -->
<div id="{{ $fieldname }}" class="form-group{{ $errors->has($fieldname) ? ' has-error' : '' }}">
<label for="{{ $fieldname }}" class="col-md-3 control-label">{{ $translated_name }}</label>
<div class="col-md-6">
<select class="js-data-ajax" data-endpoint="companies" data-placeholder="{{ trans('general.select_company') }}" name="{{ $fieldname }}{{ (isset($multiple) && ($multiple=='true')) ? '[]' : '' }}" style="width: 100%"{{ (isset($multiple) && ($multiple=='true')) ? " multiple='multiple'" : '' }}>
@isset ($selected)
@foreach ($selected as $company_id)
<option value="{{ $company_id }}" selected="selected" role="option" aria-selected="true">
{{ \App\Models\Company::find($company_id)->name }}
</option>
@endforeach
@endisset
@if (!isset($multiple) || $multiple !== 'true')
@if ($company_id = old($fieldname, (isset($item)) ? $item->{$fieldname} : ''))
<option value="{{ $company_id }}" selected="selected" role="option" aria-selected="true" role="option">
<option value="{{ $company_id }}" selected="selected">
{{ (\App\Models\Company::find($company_id)) ? \App\Models\Company::find($company_id)->name : '' }}
</option>
@else
{!! (!isset($multiple) || ($multiple=='false')) ? '<option value="" role="option">'.trans('general.select_company').'</option>' : '' !!}
<option value="" role="option">{{ trans('general.select_company') }}</option>
@endif
</select>
</div>
@endif
</select>
</div>
@else
<!-- full company support is enabled or this user is a superadmin -->
<div id="{{ $fieldname }}" class="form-group{{ $errors->has($fieldname) ? ' has-error' : '' }}">
<label for="{{ $fieldname }}" class="col-md-3 control-label">{{ $translated_name }}</label>
<div class="col-md-6">
<select class="js-data-ajax" data-endpoint="companies" data-placeholder="{{ trans('general.select_company') }}" name="{{ $fieldname }}{{ (isset($multiple) && ($multiple=='true')) ? '[]' : '' }}" style="width: 100%"{{ (isset($multiple) && ($multiple=='true')) ? " multiple='multiple'" : '' }}>
@isset ($selected)
@foreach ($selected as $company_id)
<option value="{{ $company_id }}" selected="selected" role="option" aria-selected="true">
{{ \App\Models\Company::find($company_id)->name }}
</option>
@endforeach
@endisset
@if (!isset($multiple) || $multiple !== 'true')
@if ($company_id = old($fieldname, (isset($item)) ? $item->{$fieldname} : ''))
<option value="{{ $company_id }}" selected="selected">
{{ (\App\Models\Company::find($company_id)) ? \App\Models\Company::find($company_id)->name : '' }}
</option>
@else
<option value="" role="option">{{ trans('general.select_company') }}</option>
@endif
@endif
</select>
</div>
{!! $errors->first($fieldname, '<div class="col-md-8 col-md-offset-3"><span class="alert-msg"><i class="fas fa-times" aria-hidden="true"></i> :message</span></div>') !!}
</div>
@endif
{!! $errors->first($fieldname, '<div class="col-md-8 col-md-offset-3"><span class="alert-msg"><i class="fas fa-times" aria-hidden="true"></i> :message</span></div>') !!}
</div>
@@ -3,7 +3,7 @@
<label for="{{ $fieldname }}" class="col-md-3 control-label">{{ $translated_name }}</label>
<div class="col-md-7">
<select class="js-data-ajax" data-endpoint="locations" data-placeholder="{{ trans('general.select_location') }}" name="{{ $fieldname }}" style="width: 100%" id="{{ $fieldname }}_location_select" aria-label="{{ $fieldname }}"{{ (isset($multiple) && ($multiple=='true')) ? " multiple='multiple'" : '' }}{!! ((isset($item)) && (Helper::checkIfRequired($item, $fieldname))) ? ' required ' : '' !!}{!! (!empty($company_id)) ? ' data-company-id="'.e($company_id).'"' : '' !!}>
<select class="js-data-ajax" data-endpoint="locations" data-placeholder="{{ trans('general.select_location') }}" name="{{ $fieldname }}" style="width: 100%" id="{{ $fieldname }}_location_select" aria-label="{{ $fieldname }}"{{ (isset($multiple) && ($multiple=='true')) ? " multiple='multiple'" : '' }}{!! ((isset($item)) && (Helper::checkIfRequired($item, $fieldname))) ? ' required ' : '' !!}{!! (!empty($company_id)) ? ' data-company-id="'.e($company_id).'"' : '' !!}{!! (!empty($exclude_id)) ? ' data-exclude-id="'.e($exclude_id).'"' : '' !!}>
@isset($selected)
@foreach($selected as $location_id)
<option value="{{ $location_id }}" selected="selected" role="option" aria-selected="true" role="option">
@@ -3,7 +3,7 @@
<label for="{{ $fieldname }}" class="col-md-3 control-label">{{ $translated_name }}</label>
<div class="col-md-7">
<select class="js-data-ajax" data-endpoint="users" data-placeholder="{{ trans('general.select_user') }}" name="{{ $fieldname }}" style="width: 100%" id="assigned_user_select" aria-label="{{ $fieldname }}"{{ ((isset($required)) && ($required=='true')) ? ' required' : '' }}{!! (!empty($company_id)) ? ' data-company-ids="'.e($company_id).'"' : '' !!}>
<select class="js-data-ajax" data-endpoint="users" data-placeholder="{{ trans('general.select_user') }}" name="{{ $fieldname }}" style="width: 100%" id="assigned_user_select" aria-label="{{ $fieldname }}"{{ ((isset($required)) && ($required=='true')) ? ' required' : '' }}{!! (!empty($company_id)) ? ' data-company-ids="'.e($company_id).'"' : '' !!}{!! (!empty($exclude_id)) ? ' data-exclude-id="'.e($exclude_id).'"' : '' !!}>
@if ($user_id = old($fieldname, (isset($item)) ? $item->{$fieldname} : ''))
<option value="{{ $user_id }}" selected="selected" role="option" aria-selected="true" role="option">
{{ (\App\Models\User::find($user_id)) ? \App\Models\User::find($user_id)->present()->fullName : '' }}
+1 -1
View File
@@ -413,7 +413,7 @@
<!-- Manager -->
@include ('partials.forms.edit.user-select', ['translated_name' => trans('admin/users/table.manager'), 'fieldname' => 'manager_id'])
@include ('partials.forms.edit.user-select', ['translated_name' => trans('admin/users/table.manager'), 'fieldname' => 'manager_id', 'exclude_id' => isset($item) ? $item->id : null])
<!-- Department -->
@include ('partials.forms.edit.department-select', ['translated_name' => trans('general.department'), 'fieldname' => 'department_id'])
@@ -78,6 +78,18 @@ class AssetsForSelectListTest extends TestCase
->assertResponseContainsInResults($assetB);
}
public function test_asset_is_excluded_from_selectlist_when_exclude_id_matches()
{
[$assetA, $assetB] = Asset::factory()->count(2)->create();
$actor = User::factory()->createAssets()->create();
$this->actingAsForApi($actor)
->getJson(route('assets.selectlist', ['excludeId' => $assetA->id]))
->assertResponseDoesNotContainInResults($assetA)
->assertResponseContainsInResults($assetB);
}
public function test_assets_are_filtered_by_multiple_comma_separated_company_ids_when_full_company_support_is_enabled()
{
$this->settings->enableMultipleFullCompanySupport();
@@ -277,6 +277,87 @@ class AssetCheckoutTest extends TestCase
});
}
public function test_asset_can_be_checked_out_to_user_in_same_company_via_pivot_when_fmcs_enabled()
{
// Regression: company check used to compare asset company_id to user's primary company_id only.
// Users assigned to multiple companies via the pivot table must be able to receive assets
// from any of their companies — not just their first/primary one.
$this->settings->enableMultipleFullCompanySupport();
[$companyA, $companyB, $companyC] = Company::factory()->count(3)->create();
// Actor is in companyC (same as the asset) so FMCS scoping lets them see and checkout it.
$actor = User::factory()->checkoutAssets()->for($companyC)->create();
$assetInCompanyC = Asset::factory()->for($companyC)->create();
// Target user's primary company is A, but they also belong to C via pivot.
$target = User::factory()->for($companyA)->create();
$target->companies()->sync([$companyA->id, $companyB->id, $companyC->id]);
$this->actingAsForApi($actor)
->postJson(route('api.asset.checkout', $assetInCompanyC), [
'checkout_to_type' => 'user',
'assigned_user' => $target->id,
])
->assertOk()
->assertStatusMessageIs('success');
$assetInCompanyC->refresh();
$this->assertEquals($target->id, $assetInCompanyC->assigned_to);
}
public function test_asset_cannot_be_checked_out_to_user_whose_companies_exclude_asset_company_when_fmcs_enabled()
{
$this->settings->enableMultipleFullCompanySupport();
[$companyA, $companyB, $companyC] = Company::factory()->count(3)->create();
// Actor is in companyC (same as the asset).
$actor = User::factory()->checkoutAssets()->for($companyC)->create();
$assetInCompanyC = Asset::factory()->for($companyC)->create();
// Target belongs to A and B — not C. Checkout to them should be blocked.
$target = User::factory()->for($companyA)->create();
$target->companies()->sync([$companyA->id, $companyB->id]);
$this->actingAsForApi($actor)
->postJson(route('api.asset.checkout', $assetInCompanyC), [
'checkout_to_type' => 'user',
'assigned_user' => $target->id,
])
->assertOk()
->assertStatusMessageIs('error')
->assertMessagesAre(trans('general.error_user_company'));
$assetInCompanyC->refresh();
$this->assertNull($assetInCompanyC->assigned_to);
}
public function test_asset_can_be_checked_out_to_user_with_no_company_when_fmcs_enabled()
{
// Users with no company associations should not be blocked — they're unrestricted.
$this->settings->enableMultipleFullCompanySupport();
$company = Company::factory()->create();
// Actor is in the same company as the asset.
$actor = User::factory()->checkoutAssets()->for($company)->create();
$assetInCompany = Asset::factory()->for($company)->create();
$target = User::factory()->create(['company_id' => null]);
$target->companies()->sync([]);
$this->actingAsForApi($actor)
->postJson(route('api.asset.checkout', $assetInCompany), [
'checkout_to_type' => 'user',
'assigned_user' => $target->id,
])
->assertOk()
->assertStatusMessageIs('success');
$assetInCompany->refresh();
$this->assertEquals($target->id, $assetInCompany->assigned_to);
}
public function test_license_seats_are_assigned_to_user_upon_checkout()
{
$this->markTestIncomplete('This is not implemented');
@@ -434,4 +434,34 @@ class ImportUsersTest extends ImportDataTestCase implements TestsPermissionsRequ
$this->assertEquals('updated@example.com', $target->refresh()->email);
}
#[Test]
public function display_name_falls_back_to_full_name_when_column_is_missing_from_csv(): void
{
$importFileBuilder = ImportFileBuilder::new()->forget('displayName');
$row = $importFileBuilder->firstRow();
$import = Import::factory()->users()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]);
$this->actingAsForApi(User::factory()->superuser()->create());
$this->importFileResponse(['import' => $import->id])->assertOk();
$newUser = User::query()->where('username', $row['username'])->sole();
$this->assertEquals("{$row['firstName']} {$row['lastName']}", $newUser->display_name);
}
#[Test]
public function display_name_falls_back_to_full_name_when_column_is_empty_in_csv(): void
{
$importFileBuilder = ImportFileBuilder::new(['displayName' => '']);
$row = $importFileBuilder->firstRow();
$import = Import::factory()->users()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]);
$this->actingAsForApi(User::factory()->superuser()->create());
$this->importFileResponse(['import' => $import->id])->assertOk();
$newUser = User::query()->where('username', $row['username'])->sole();
$this->assertEquals("{$row['firstName']} {$row['lastName']}", $newUser->display_name);
}
}
@@ -0,0 +1,276 @@
<?php
namespace Tests\Feature\Locations\Api;
use App\Models\Company;
use App\Models\Location;
use App\Models\User;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;
/**
* Verifies FMCS scoping rules for the location index and selectlist endpoints.
*
* Rules under test:
* 1. FMCS OFF all locations visible to any authorized user regardless of company
* 2. FMCS ON, user has companies only locations whose company_id matches one of the user's companies
* 3. FMCS ON, user has companies locations with NULL company_id are NOT visible
* 4. FMCS ON, user has companies locations in OTHER companies are NOT visible
* 5. FMCS ON, user has NO companies only locations with NULL company_id are visible
* 6. FMCS ON, user has NO companies locations with a company_id are NOT visible
* 7. scope_locations_fmcs does not change visibility; rules 2-6 hold with or without it
*/
class LocationsFmcsScopingTest extends TestCase
{
// -----------------------------------------------------------------------
// Helpers
// -----------------------------------------------------------------------
private function userInCompany(Company $company): User
{
$user = User::factory()->viewLocationHistory()->createUsers()->create();
DB::table('company_user')->insert([
'company_id' => $company->id,
'user_id' => $user->id,
'created_at' => now(),
'updated_at' => now(),
]);
return $user;
}
private function userWithNoCompany(): User
{
return User::factory()->viewLocationHistory()->createUsers()->create(['company_id' => null]);
}
private function indexIds(User $user): array
{
return collect(
$this->actingAsForApi($user)
->getJson(route('api.locations.index', ['limit' => 500]))
->assertOk()
->json('rows')
)->pluck('id')->all();
}
private function selectlistIds(User $user): array
{
return collect(
$this->actingAsForApi($user)
->getJson(route('api.locations.selectlist', ['limit' => 500]))
->assertOk()
->json('results')
)->pluck('id')->all();
}
// -----------------------------------------------------------------------
// FMCS OFF
// -----------------------------------------------------------------------
public function test_fmcs_off_user_sees_all_locations_on_index()
{
$this->settings->disableMultipleFullCompanySupport();
$companyA = Company::factory()->create();
$companyB = Company::factory()->create();
$locationA = Location::factory()->create(['company_id' => $companyA->id]);
$locationB = Location::factory()->create(['company_id' => $companyB->id]);
$locationNull = Location::factory()->create(['company_id' => null]);
$user = $this->userInCompany($companyA);
$ids = $this->indexIds($user);
$this->assertContains($locationA->id, $ids, 'Own-company location should be visible');
$this->assertContains($locationB->id, $ids, 'Other-company location should be visible when FMCS off');
$this->assertContains($locationNull->id, $ids, 'Null-company location should be visible when FMCS off');
}
public function test_fmcs_off_user_sees_all_locations_on_selectlist()
{
$this->settings->disableMultipleFullCompanySupport();
$companyA = Company::factory()->create();
$companyB = Company::factory()->create();
$locationA = Location::factory()->create(['company_id' => $companyA->id]);
$locationB = Location::factory()->create(['company_id' => $companyB->id]);
$locationNull = Location::factory()->create(['company_id' => null]);
$user = $this->userInCompany($companyA);
$ids = $this->selectlistIds($user);
$this->assertContains($locationA->id, $ids, 'Own-company location should be in selectlist');
$this->assertContains($locationB->id, $ids, 'Other-company location should be in selectlist when FMCS off');
$this->assertContains($locationNull->id, $ids, 'Null-company location should be in selectlist when FMCS off');
}
// -----------------------------------------------------------------------
// FMCS ON — user WITH companies
// -----------------------------------------------------------------------
public function test_fmcs_on_user_with_company_sees_own_company_location_on_index()
{
$this->settings->enableMultipleFullCompanySupport();
$company = Company::factory()->create();
$location = Location::factory()->create(['company_id' => $company->id]);
$user = $this->userInCompany($company);
$this->assertContains($location->id, $this->indexIds($user),
'Location in same company should be visible');
}
public function test_fmcs_on_user_with_company_cannot_see_other_company_location_on_index()
{
$this->settings->enableMultipleFullCompanySupport();
$companyA = Company::factory()->create();
$companyB = Company::factory()->create();
$locationB = Location::factory()->create(['company_id' => $companyB->id]);
$user = $this->userInCompany($companyA);
$this->assertNotContains($locationB->id, $this->indexIds($user),
'Location in a different company should not be visible');
}
public function test_fmcs_on_user_with_company_cannot_see_null_company_location_on_index()
{
$this->settings->enableMultipleFullCompanySupport();
$company = Company::factory()->create();
$locationNull = Location::factory()->create(['company_id' => null]);
$user = $this->userInCompany($company);
$this->assertNotContains($locationNull->id, $this->indexIds($user),
'Location with no company should not be visible to company-scoped user');
}
public function test_fmcs_on_user_with_company_sees_own_company_location_on_selectlist()
{
$this->settings->enableMultipleFullCompanySupport();
$company = Company::factory()->create();
$location = Location::factory()->create(['company_id' => $company->id]);
$user = $this->userInCompany($company);
$this->assertContains($location->id, $this->selectlistIds($user),
'Location in same company should appear in selectlist');
}
public function test_fmcs_on_user_with_company_cannot_see_other_company_location_on_selectlist()
{
$this->settings->enableMultipleFullCompanySupport();
$companyA = Company::factory()->create();
$companyB = Company::factory()->create();
$locationB = Location::factory()->create(['company_id' => $companyB->id]);
$user = $this->userInCompany($companyA);
$this->assertNotContains($locationB->id, $this->selectlistIds($user),
'Location in a different company should not appear in selectlist');
}
public function test_fmcs_on_user_with_company_cannot_see_null_company_location_on_selectlist()
{
$this->settings->enableMultipleFullCompanySupport();
$company = Company::factory()->create();
$locationNull = Location::factory()->create(['company_id' => null]);
$user = $this->userInCompany($company);
$this->assertNotContains($locationNull->id, $this->selectlistIds($user),
'Location with no company should not appear in selectlist for company-scoped user');
}
// -----------------------------------------------------------------------
// FMCS ON — user with NO companies
// -----------------------------------------------------------------------
public function test_fmcs_on_user_with_no_company_sees_null_company_locations_on_index()
{
$this->settings->enableMultipleFullCompanySupport();
$locationNull = Location::factory()->create(['company_id' => null]);
$user = $this->userWithNoCompany();
$this->assertContains($locationNull->id, $this->indexIds($user),
'Location with no company should be visible to user with no company');
}
public function test_fmcs_on_user_with_no_company_cannot_see_company_locations_on_index()
{
$this->settings->enableMultipleFullCompanySupport();
$company = Company::factory()->create();
$location = Location::factory()->create(['company_id' => $company->id]);
$user = $this->userWithNoCompany();
$this->assertNotContains($location->id, $this->indexIds($user),
'Location with a company should not be visible to user with no company');
}
public function test_fmcs_on_user_with_no_company_sees_null_company_locations_on_selectlist()
{
$this->settings->enableMultipleFullCompanySupport();
$locationNull = Location::factory()->create(['company_id' => null]);
$user = $this->userWithNoCompany();
$this->assertContains($locationNull->id, $this->selectlistIds($user),
'Location with no company should appear in selectlist for user with no company');
}
public function test_fmcs_on_user_with_no_company_cannot_see_company_locations_on_selectlist()
{
$this->settings->enableMultipleFullCompanySupport();
$company = Company::factory()->create();
$location = Location::factory()->create(['company_id' => $company->id]);
$user = $this->userWithNoCompany();
$this->assertNotContains($location->id, $this->selectlistIds($user),
'Location with a company should not appear in selectlist for user with no company');
}
// -----------------------------------------------------------------------
// scope_locations_fmcs does not change visibility rules
// -----------------------------------------------------------------------
public function test_scope_locations_fmcs_does_not_change_visibility_for_user_with_company()
{
$this->settings->enableScopedLocationsWithFullMultipleCompanySupport();
$companyA = Company::factory()->create();
$companyB = Company::factory()->create();
$locationA = Location::factory()->create(['company_id' => $companyA->id]);
$locationB = Location::factory()->create(['company_id' => $companyB->id]);
$locationNull = Location::factory()->create(['company_id' => null]);
$user = $this->userInCompany($companyA);
$ids = $this->indexIds($user);
$this->assertContains($locationA->id, $ids, 'Own-company location should still be visible');
$this->assertNotContains($locationB->id, $ids, 'Other-company location should still be hidden');
$this->assertNotContains($locationNull->id, $ids, 'Null-company location should still be hidden from company-scoped user');
}
public function test_scope_locations_fmcs_does_not_change_visibility_for_user_with_no_company()
{
$this->settings->enableScopedLocationsWithFullMultipleCompanySupport();
$company = Company::factory()->create();
$locationNull = Location::factory()->create(['company_id' => null]);
$locationA = Location::factory()->create(['company_id' => $company->id]);
$user = $this->userWithNoCompany();
$ids = $this->indexIds($user);
$this->assertContains($locationNull->id, $ids, 'Null-company location should still be visible to no-company user');
$this->assertNotContains($locationA->id, $ids, 'Company location should still be hidden from no-company user');
}
}
@@ -35,6 +35,19 @@ class LocationsForSelectListTest extends TestCase
->assertJson(fn (AssertableJson $json) => $json->has('results', 1)->etc());
}
public function test_location_is_excluded_from_selectlist_when_exclude_id_matches()
{
[$locationA, $locationB] = Location::factory()->count(2)->create();
$this->actingAsForApi(User::factory()->createUsers()->create())
->getJson(route('api.locations.selectlist', ['excludeId' => $locationA->id]))
->assertOk()
->assertJson(fn (AssertableJson $json) => $json->where('results', fn ($results) => collect($results)->doesntContain('id', $locationA->id) &&
collect($results)->contains('id', $locationB->id)
)->etc()
);
}
public function test_locations_are_returned_when_user_is_updating_their_profile_and_has_permission_to_update_location()
{
$this->actingAsForApi(User::factory()->canEditOwnLocation()->create())
+251
View File
@@ -0,0 +1,251 @@
<?php
/**
* Query-count regression guards for the performance optimisations shipped in this PR.
*
* Three classes of regressions are guarded against:
*
* 1. **Locations API N+1** LocationsTransformer calls transformUser($location->manager),
* which calls isDeletable(), which fires 6 count queries per manager unless the counts
* are eagerly loaded. The fix eager-loads manager withCount in the controller.
*
* 2. **Asset-scope correlated subqueries** scopeRTD / scopePending / scopeArchived /
* scopeAssetsForShow all used whereHas('status', ). In a withCount context MySQL
* evaluates the EXISTS subquery once per outer row, producing O(n) query work.
* The fix plucks status-label IDs to PHP first so MySQL sees a flat IN (1, 2, 3).
*
* 3. **Sidebar count on every web request** the old AssetCountForSidebar middleware
* fired on every web request including modals and select2 AJAX calls, adding ~14
* count queries each time. The fix moves the counts into a View Composer bound to
* layouts.default, so they only fire when a full page renders.
*
* Each test asserts a ceiling; the ceilings are intentionally a bit generous so minor
* schema additions don't break CI, but they are tight enough to catch a regression back
* to the old O(n) behaviour.
*
* Run with:
* php artisan test tests/Feature/QueryCountBenchmarkTest.php --verbose
*/
namespace Tests\Feature;
use App\Models\Asset;
use App\Models\AssetModel;
use App\Models\Category;
use App\Models\Location;
use App\Models\Statuslabel;
use App\Models\User;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;
class QueryCountBenchmarkTest extends TestCase
{
// ---------------------------------------------------------------------------
// Locations API N+1 on manager counts
// ---------------------------------------------------------------------------
/**
* With one shared manager across N locations, the old code fired 6 queries per
* location (one for each count in isDeletable). With eager-loaded manager counts
* the total should be roughly constant regardless of N.
*/
public function test_locations_api_index_does_not_n_plus_1_on_manager_counts(): void
{
$manager = User::factory()->superuser()->create();
// 10 locations all sharing one manager — the worst case for N+1 fan-out.
Location::factory()->count(10)->create(['manager_id' => $manager->id]);
$actor = User::factory()->superuser()->create();
DB::flushQueryLog();
DB::enableQueryLog();
$this->actingAsForApi($actor)
->getJson(route('api.locations.index', ['limit' => 50, 'offset' => 0]))
->assertOk();
$queryCount = count(DB::getQueryLog());
DB::disableQueryLog();
// Without eager loading this would be ≥ 10×6 = 60 extra queries just for manager counts.
// With the fix the manager is loaded once and counts are embedded in that query.
$this->assertLessThan(25, $queryCount,
"Locations index query count ({$queryCount}) suggests N+1 on manager counts. "
.'Ensure LocationsController eager-loads manager withCount.');
}
/**
* Scaling check: doubling the location count should not double the query count.
*/
public function test_locations_api_index_query_count_does_not_scale_with_location_count(): void
{
$manager = User::factory()->superuser()->create();
$actor = User::factory()->superuser()->create();
// Measure with 5 locations
Location::factory()->count(5)->create(['manager_id' => $manager->id]);
DB::flushQueryLog();
DB::enableQueryLog();
$this->actingAsForApi($actor)
->getJson(route('api.locations.index', ['limit' => 50, 'offset' => 0]))
->assertOk();
$countWith5 = count(DB::getQueryLog());
DB::disableQueryLog();
// Add 15 more (total 20)
Location::factory()->count(15)->create(['manager_id' => $manager->id]);
DB::flushQueryLog();
DB::enableQueryLog();
$this->actingAsForApi($actor)
->getJson(route('api.locations.index', ['limit' => 50, 'offset' => 0]))
->assertOk();
$countWith20 = count(DB::getQueryLog());
DB::disableQueryLog();
// If N+1 is present, going from 5→20 locations (4×) would add ~90 queries.
// With the fix the increase should be negligible (within 5 queries).
$this->assertLessThan($countWith5 + 5, $countWith20,
"Locations index fired {$countWith5} queries for 5 locations and {$countWith20} for 20. "
.'This looks like N+1 on manager counts.');
}
// ---------------------------------------------------------------------------
// Asset model API correlated-subquery scopes used in withCount
// ---------------------------------------------------------------------------
/**
* AssetModel::withCount(['assets', 'availableAssets', 'archivedAssets']) uses
* the RTD / Archived scopes. Without the pluck+whereIn fix each row generates
* correlated EXISTS subqueries evaluated per row by MySQL invisible in query
* count but catastrophic for runtime. We can't measure execution time reliably
* in tests, so instead we assert that the SQL contains no EXISTS subquery shape
* and that the query count is flat.
*/
public function test_asset_model_index_query_count_is_flat(): void
{
$category = Category::factory()->create(['category_type' => 'asset']);
AssetModel::factory()->count(5)->create(['category_id' => $category->id]);
$actor = User::factory()->superuser()->create();
DB::flushQueryLog();
DB::enableQueryLog();
$this->actingAsForApi($actor)
->getJson(route('api.models.index', ['limit' => 50, 'offset' => 0]))
->assertOk();
$queryCount = count(DB::getQueryLog());
DB::disableQueryLog();
$this->assertLessThan(25, $queryCount,
"Asset model index fired {$queryCount} queries; expected < 25. "
.'Check that availableAssets/archivedAssets scopes use pluck+whereIn, not whereHas.');
}
// ---------------------------------------------------------------------------
// RTD / Pending / Archived scope SQL shapes
// ---------------------------------------------------------------------------
/**
* These four tests verify the SQL produced by each scope does NOT contain a
* correlated EXISTS (which would indicate a regression back to whereHas).
* They also confirm the IN clause IS present.
*/
public function test_scope_rtd_uses_where_in_not_exists(): void
{
$sql = Asset::RTD()->toSql();
$this->assertStringNotContainsString('exists (select', strtolower($sql),
'scopeRTD must not use a correlated EXISTS subquery.');
$this->assertStringContainsString('in (', strtolower($sql),
'scopeRTD should use a flat IN clause from plucked IDs.');
}
public function test_scope_pending_uses_where_in_not_exists(): void
{
$sql = Asset::Pending()->toSql();
$this->assertStringNotContainsString('exists (select', strtolower($sql),
'scopePending must not use a correlated EXISTS subquery.');
$this->assertStringContainsString('in (', strtolower($sql),
'scopePending should use a flat IN clause from plucked IDs.');
}
public function test_scope_archived_uses_where_in_not_exists(): void
{
$sql = Asset::Archived()->toSql();
$this->assertStringNotContainsString('exists (select', strtolower($sql),
'scopeArchived must not use a correlated EXISTS subquery.');
$this->assertStringContainsString('in (', strtolower($sql),
'scopeArchived should use a flat IN clause from plucked IDs.');
}
public function test_scope_undeployable_uses_where_in_not_exists(): void
{
$sql = Asset::Undeployable()->toSql();
$this->assertStringNotContainsString('exists (select', strtolower($sql),
'scopeUndeployable must not use a correlated EXISTS subquery.');
$this->assertStringContainsString('in (', strtolower($sql),
'scopeUndeployable should use a flat IN clause from plucked IDs.');
}
public function test_scope_assets_for_show_uses_where_in_not_exists(): void
{
$sql = Asset::AssetsForShow()->toSql();
// When show_archived_in_list is off (default), the scope adds a whereIn.
// When it's on, no filter is added at all — both are fine, neither should have EXISTS.
$this->assertStringNotContainsString('exists (select', strtolower($sql),
'scopeAssetsForShow must not use a correlated EXISTS subquery.');
}
// ---------------------------------------------------------------------------
// Sidebar composer skips non-full-page requests
// ---------------------------------------------------------------------------
/**
* A modal or select2 AJAX endpoint should not trigger the sidebar asset counts.
* We verify this indirectly by checking that hitting a selectlist endpoint
* does not fire the ~14 count queries that the old middleware would have fired.
*
* The selectlist endpoints are purely JSON, never render layouts.default,
* so the SidebarComposer must not fire for them.
*/
public function test_selectlist_endpoint_does_not_fire_sidebar_counts(): void
{
Statuslabel::factory()->count(3)->create();
$actor = User::factory()->superuser()->create();
DB::flushQueryLog();
DB::enableQueryLog();
$this->actingAsForApi($actor)
->getJson(route('api.locations.selectlist'))
->assertOk();
$queries = DB::getQueryLog();
DB::disableQueryLog();
$queryCount = count($queries);
// The old middleware would fire ~14 asset count queries on every request.
// A selectlist endpoint needs only a handful of queries (auth + the list query).
// We assert fewer than 15 to confirm the sidebar counts are not being run.
$this->assertLessThan(15, $queryCount,
"Selectlist endpoint fired {$queryCount} queries. "
.'This may indicate the sidebar composer (or old middleware) is firing on AJAX requests. '
.'The SidebarComposer should only fire when layouts.default renders.');
// Confirm none of the queries counted RTD/Deployed/Archived/Pending — those
// are the tell-tale sidebar count queries.
$sidebarKeywords = ['rtd_assets', 'byod', 'overdue_for_checkin', 'due_for_audit'];
foreach ($queries as $query) {
$sql = strtolower($query['query']);
foreach ($sidebarKeywords as $keyword) {
$this->assertStringNotContainsString($keyword, $sql,
"Selectlist endpoint ran a sidebar-related query containing '{$keyword}'. "
.'The SidebarComposer should only fire when layouts.default renders.');
}
}
}
}
+115
View File
@@ -0,0 +1,115 @@
<?php
namespace Tests\Feature;
use App\Models\Asset;
use App\Models\AssetModel;
use App\Models\Category;
use App\Models\Location;
use Tests\TestCase;
/**
* Guards against the correlated-subquery footgun.
*
* The problem: using whereHas() to filter by status label inside a withCount() relationship
* generates a nested EXISTS subquery that MySQL re-evaluates for every row. On large datasets
* (thousands of locations, hundreds of thousands of assets) this causes 30+ second timeouts.
*
* The fix: pluck the matching status IDs once in PHP, then use whereIn() with the flat list.
* MySQL can use the index on assets.status_id directly rather than running a subquery per row.
*
* If any test here fails, DO NOT restore whereHas() you will reintroduce the timeout.
* See the comment blocks in Asset::scopeRTD(), scopeArchived(), scopePending(),
* scopeUndeployable(), scopeNotArchived(), scopeAssetsForShow(), and Location::assets().
*/
class QueryShapeTest extends TestCase
{
private function assertNoCorrelatedExists(string $sql, string $context): void
{
$this->assertStringNotContainsString(
'exists (select',
strtolower($sql),
"Correlated EXISTS detected in {$context}. Replace whereHas() with a Statuslabel::pluck()+whereIn() — see comments in the affected scope/relationship."
);
}
// ----- Direct scope SQL shape -----
public function test_rtd_scope_uses_where_in_not_correlated_exists(): void
{
$sql = Asset::RTD()->toSql();
$this->assertNoCorrelatedExists($sql, 'Asset::RTD()');
$this->assertStringContainsString('in (', strtolower($sql));
}
public function test_pending_scope_uses_where_in_not_correlated_exists(): void
{
$sql = Asset::Pending()->toSql();
$this->assertNoCorrelatedExists($sql, 'Asset::Pending()');
$this->assertStringContainsString('in (', strtolower($sql));
}
public function test_archived_scope_uses_where_in_not_correlated_exists(): void
{
$sql = Asset::Archived()->toSql();
$this->assertNoCorrelatedExists($sql, 'Asset::Archived()');
$this->assertStringContainsString('in (', strtolower($sql));
}
public function test_undeployable_scope_uses_where_in_not_correlated_exists(): void
{
$sql = Asset::Undeployable()->toSql();
$this->assertNoCorrelatedExists($sql, 'Asset::Undeployable()');
$this->assertStringContainsString('in (', strtolower($sql));
}
public function test_not_archived_scope_uses_where_in_not_correlated_exists(): void
{
$sql = Asset::NotArchived()->toSql();
$this->assertNoCorrelatedExists($sql, 'Asset::NotArchived()');
$this->assertStringContainsString('in (', strtolower($sql));
}
public function test_assets_for_show_scope_uses_where_in_not_correlated_exists(): void
{
// show_archived_in_list defaults to 0, so the whereIn filter is always applied in tests
$sql = Asset::AssetsForShow()->toSql();
$this->assertNoCorrelatedExists($sql, 'Asset::AssetsForShow()');
$this->assertStringContainsString('in (', strtolower($sql));
}
// ----- withCount SQL shape (the real danger zone) -----
// These test the queries that actually timed out in production.
// withCount() embeds the relationship query as a correlated subquery —
// any EXISTS inside it runs once per outer row, not once total.
public function test_asset_model_available_assets_withcount_uses_where_in_not_correlated_exists(): void
{
$sql = AssetModel::withCount('availableAssets as remaining')->toSql();
$this->assertNoCorrelatedExists($sql, 'AssetModel::withCount(availableAssets)');
}
public function test_asset_model_archived_assets_withcount_uses_where_in_not_correlated_exists(): void
{
$sql = AssetModel::withCount('archivedAssets as assets_archived_count')->toSql();
$this->assertNoCorrelatedExists($sql, 'AssetModel::withCount(archivedAssets)');
}
public function test_location_assets_withcount_uses_where_in_not_correlated_exists(): void
{
$sql = Location::withCount('assets as assets_count')->toSql();
$this->assertNoCorrelatedExists($sql, 'Location::withCount(assets)');
}
public function test_location_assigned_assets_withcount_uses_where_in_not_correlated_exists(): void
{
$sql = Location::withCount('assignedAssets as assigned_assets_count')->toSql();
$this->assertNoCorrelatedExists($sql, 'Location::withCount(assignedAssets)');
}
public function test_category_showable_assets_withcount_uses_where_in_not_correlated_exists(): void
{
$sql = Category::withCount('showableAssets as assets_count')->toSql();
$this->assertNoCorrelatedExists($sql, 'Category::withCount(showableAssets)');
}
}
@@ -115,6 +115,18 @@ class UsersForSelectListTest extends TestCase
$this->assertEquals(0, collect($response->json('results'))->count());
}
public function test_user_is_excluded_from_selectlist_when_exclude_id_matches()
{
[$userA, $userB] = User::factory()->count(2)->create();
Passport::actingAs(User::factory()->superuser()->create());
$response = $this->getJson(route('api.users.selectlist', ['excludeId' => $userA->id]))->assertOk();
$results = collect($response->json('results'));
$this->assertFalse($results->contains('id', $userA->id), 'Excluded user should not appear');
$this->assertTrue($results->contains('id', $userB->id), 'Other user should still appear');
}
public function test_users_are_filtered_by_company_id_parameter_when_full_company_support_is_enabled()
{
$this->settings->enableMultipleFullCompanySupport();