fixed N+1 issues

This commit is contained in:
Mark Gregory
2026-06-16 20:22:19 +01:00
parent a07bfd13df
commit 0b0a708a99
10 changed files with 200 additions and 112 deletions
@@ -25,7 +25,7 @@ final class TicketControlsController extends Controller
*/
public function updateAgent(Request $request, Ticket $ticket, WorkflowNotificationService $workflow): RedirectResponse
{
$ticket->loadMissing($this->ticketActionRelations());
$this->hydrateUserRelations($ticket);
Gate::authorize('update', $ticket);
@@ -51,7 +51,7 @@ final class TicketControlsController extends Controller
'last_activity_at' => now(),
]);
$fresh = $ticket->fresh(['user.company', 'owner.company', 'agent.company', 'status', 'priority', 'category', 'project', 'team', 'product']) ?? $ticket;
$fresh = $this->freshTicketForAction($ticket);
$workflow->dispatch($fresh, 'ticket.updated', $this->actor());
return redirect()
@@ -66,7 +66,7 @@ final class TicketControlsController extends Controller
*/
public function updateStatus(Request $request, Ticket $ticket, WorkflowNotificationService $workflow, SlaService $sla): RedirectResponse
{
$ticket->loadMissing($this->ticketActionRelations());
$this->hydrateUserRelations($ticket);
Gate::authorize('update', $ticket);
@@ -84,7 +84,7 @@ final class TicketControlsController extends Controller
$this->ensureBoardContainsStatus($ticket, (int) $status->id);
$fresh = $ticket->fresh(['user.company', 'owner.company', 'agent.company', 'status', 'priority', 'category', 'project', 'team', 'product']) ?? $ticket;
$fresh = $this->freshTicketForAction($ticket);
if ($status->is_closed) {
$sla->recordClosure($fresh);
@@ -102,7 +102,7 @@ final class TicketControlsController extends Controller
*/
public function updatePriority(Request $request, Ticket $ticket, WorkflowNotificationService $workflow): RedirectResponse
{
$ticket->loadMissing($this->ticketActionRelations());
$this->hydrateUserRelations($ticket);
Gate::authorize('update', $ticket);
@@ -115,7 +115,7 @@ final class TicketControlsController extends Controller
'last_activity_at' => now(),
]);
$fresh = $ticket->fresh(['user.company', 'owner.company', 'agent.company', 'status', 'priority', 'category', 'project', 'team', 'product']) ?? $ticket;
$fresh = $this->freshTicketForAction($ticket);
$workflow->dispatch($fresh, 'ticket.updated', $this->actor());
return redirect()
@@ -123,25 +123,6 @@ final class TicketControlsController extends Controller
->with('ticket-action-updated', 'Priority updated to '.($fresh->priority?->name ?? 'selected priority').'.');
}
/**
* Relations needed by policies, redirect messages, and workflow templates.
*
* @return list<string>
*/
private function ticketActionRelations(): array
{
return [
'owner.company',
'agent.company',
'status',
'priority',
'category',
'project',
'team',
'product',
];
}
/**
* Apply a direct guarded ticket update without relying on Livewire hydration.
*
@@ -158,6 +139,58 @@ final class TicketControlsController extends Controller
->update($updates);
}
/**
* Reload the ticket after a control update without issuing separate owner/agent relation queries.
*/
private function freshTicketForAction(Ticket $ticket): Ticket
{
$fresh = $ticket->fresh([
'status',
'priority',
'category',
'project',
'team',
'product',
]) ?? $ticket;
$this->hydrateUserRelations($fresh);
return $fresh;
}
/**
* Load requester and assigned agent in one user query and attach all aliases used downstream.
*/
private function hydrateUserRelations(Ticket $ticket): void
{
$userIds = collect([$ticket->user_id, $ticket->agent_id])
->filter()
->map(static fn (mixed $id): int => (int) $id)
->unique()
->values();
if ($userIds->isEmpty()) {
$ticket->setRelation('user', null);
$ticket->setRelation('owner', null);
$ticket->setRelation('agent', null);
return;
}
$users = User::query()
->with('company')
->whereIn('id', $userIds)
->get()
->keyBy('id');
$owner = $users->get((int) $ticket->user_id);
$agent = $ticket->agent_id === null ? null : $users->get((int) $ticket->agent_id);
$ticket->setRelation('user', $owner);
$ticket->setRelation('owner', $owner);
$ticket->setRelation('agent', $agent);
}
/**
* Keep the active Kanban board usable when a ticket is moved to a status not yet assigned to the board.
*/
@@ -173,9 +206,8 @@ final class TicketControlsController extends Controller
);
}
/**
* Return the authenticated user with company eager-loaded for workflow rendering/N+1 safety.
* Return the authenticated user with company loaded for workflow rendering.
*/
private function actor(): User
{
+3 -3
View File
@@ -50,7 +50,7 @@ final class KanbanBoard extends Component
])->save();
$workflow->dispatch(
$ticket->fresh(['owner', 'agent', 'status', 'priority', 'category', 'project', 'team']) ?? $ticket,
$ticket->fresh(['status', 'priority', 'category', 'project', 'team']) ?? $ticket,
$status->is_closed ? 'ticket.closed' : 'ticket.updated',
Auth::user()
);
@@ -89,7 +89,7 @@ final class KanbanBoard extends Component
$this->board_id = $board->id;
$fresh = $ticket->fresh(['owner', 'agent', 'status', 'priority', 'category', 'project', 'team', 'board']) ?? $ticket;
$fresh = $ticket->fresh(['status', 'priority', 'category', 'project', 'team', 'board']) ?? $ticket;
$workflow->dispatch($fresh, 'ticket.updated', Auth::user());
$message = 'Ticket '.$fresh->reference.' moved from '.$oldBoardName.' to '.$board->name.'.';
@@ -153,7 +153,7 @@ final class KanbanBoard extends Component
}
return Ticket::query()
->with(['owner', 'agent', 'status', 'priority', 'category', 'project', 'team', 'product', 'tags', 'checklistItems', 'board'])
->with(['status', 'priority', 'category', 'project', 'team', 'product', 'tags', 'checklistItems', 'board'])
->visibleTo(Auth::user())
->where('board_id', $board->id)
->latest('last_activity_at')
+3 -3
View File
@@ -224,7 +224,7 @@ final class TicketEdit extends Component
$columns[] = 'company_id';
}
$query = User::query()->select($columns)->with('company')->orderBy('name');
$query = User::query()->select($columns)->orderBy('name');
if ($this->company_id !== null && Schema::hasColumn('users', 'company_id')) {
$tenantUsers = (clone $query)
@@ -248,8 +248,8 @@ final class TicketEdit extends Component
private function freshTicket(Ticket $ticket): Ticket
{
return $ticket->fresh([
'owner.company',
'agent.company',
'owner',
'agent',
'status',
'priority',
'category',
+48 -45
View File
@@ -55,11 +55,8 @@ final class TicketShow extends Component
public function mount(Ticket $ticket): void
{
$this->actor()->loadMissing('company');
$ticket->loadMissing($this->ticketDetailRelations());
$this->authorize('view', $ticket);
$this->ticket = $this->freshTicket($ticket);
$this->authorize('view', $this->ticket);
$this->status_id = $this->ticket->status_id;
$this->priority_id = $this->ticket->priority_id;
$this->agent_id = $this->ticket->agent_id;
@@ -86,7 +83,7 @@ final class TicketShow extends Component
'attachments.*' => 'file|max:'.config('helpdesk.max_upload_kb', 10240).'|mimes:'.implode(',', config('helpdesk.allowed_mimes', ['jpg', 'jpeg', 'png', 'pdf', 'txt', 'log', 'zip'])),
]);
$tickets->comment($this->ticket, $this->actor(), $this->commentBody, $this->internal, $this->attachments);
$tickets->comment($this->ticket, Auth::user(), $this->commentBody, $this->internal, $this->attachments);
$this->reset(['commentBody', 'internal', 'attachments']);
$this->ticket = $this->freshTicket($this->ticket);
@@ -103,7 +100,7 @@ final class TicketShow extends Component
'replyBody' => ['required', 'string', 'min:2'],
]);
$tickets->comment($this->ticket, $this->actor(), $this->replyBody, $this->internal, [], $this->reply_parent_id);
$tickets->comment($this->ticket, Auth::user(), $this->replyBody, $this->internal, [], $this->reply_parent_id);
$this->reset(['reply_parent_id', 'replyBody']);
$this->ticket = $this->freshTicket($this->ticket);
@@ -123,7 +120,7 @@ final class TicketShow extends Component
->where('is_active', true)
->findOrFail($this->canned_response_id);
$rendered = $renderer->render($response->body, $this->ticket, $this->actor());
$rendered = $renderer->render($response->body, $this->ticket, Auth::user());
$this->commentBody = trim($this->commentBody."\n\n".$rendered);
$this->dispatch('helpdesk-canned-response-applied', body: $this->commentBody);
}
@@ -136,7 +133,7 @@ final class TicketShow extends Component
->where('ticket_id', $this->ticket->id)
->findOrFail($itemId);
$checklists->toggle($item, $this->actor());
$checklists->toggle($item, Auth::user());
$this->ticket = $this->freshTicket($this->ticket);
}
@@ -177,7 +174,7 @@ final class TicketShow extends Component
$sla->recordClosure($this->ticket);
}
$workflow->dispatch($this->ticket, $status->is_closed ? 'ticket.closed' : 'ticket.updated', $this->actor());
$workflow->dispatch($this->ticket, $status->is_closed ? 'ticket.closed' : 'ticket.updated', Auth::user());
session()->flash('ticket-action-updated', 'Status updated to '.$this->ticket->status?->name.'.');
$this->dispatch('helpdesk-status-updated', statusId: $this->status_id);
@@ -213,7 +210,7 @@ final class TicketShow extends Component
$this->ticket = $this->freshTicket($this->ticket);
$this->priority_id = (int) $this->ticket->priority_id;
$workflow->dispatch($this->ticket, 'ticket.updated', $this->actor());
$workflow->dispatch($this->ticket, 'ticket.updated', Auth::user());
session()->flash('ticket-action-updated', 'Priority updated to '.$this->ticket->priority?->name.'.');
$this->dispatch('helpdesk-priority-updated', priorityId: $this->priority_id);
@@ -251,7 +248,7 @@ final class TicketShow extends Component
$this->ticket = $this->freshTicket($this->ticket);
$this->agent_id = $this->ticket->agent_id === null ? null : (int) $this->ticket->agent_id;
$workflow->dispatch($this->ticket, 'ticket.updated', $this->actor());
$workflow->dispatch($this->ticket, 'ticket.updated', Auth::user());
session()->flash('ticket-action-updated', $this->agent_id === null
? 'Ticket is now unassigned.'
@@ -262,9 +259,6 @@ final class TicketShow extends Component
public function render(): View
{
$this->actor()->loadMissing('company');
$this->ticket = $this->freshTicket($this->ticket);
return view('helpdesk::livewire.ticket-show', [
'statuses' => Status::query()->orderBy('sort_order')->get(),
'priorities' => Priority::query()->orderBy('sort_order')->get(),
@@ -276,19 +270,6 @@ final class TicketShow extends Component
/**
* Convert Livewire select values into a nullable integer.
*/
/**
* Return the authenticated user with relations commonly used by policies and workflow templates.
*/
private function actor(): User
{
/** @var User $user */
$user = Auth::user();
$user->loadMissing('company');
return $user;
}
private function normaliseNullableInt(mixed $value): ?int
{
if ($value === null || $value === '' || $value === '0' || $value === 0) {
@@ -298,17 +279,9 @@ final class TicketShow extends Component
return (int) $value;
}
/**
* Relations required by the ticket detail view and workflow renderers.
*
* @return list<string>
*/
private function ticketDetailRelations(): array
private function freshTicket(Ticket $ticket): Ticket
{
return [
'user.company',
'owner.company',
'agent.company',
$ticket = $ticket->fresh([
'status',
'priority',
'category',
@@ -320,18 +293,15 @@ final class TicketShow extends Component
'ratings',
'tags',
'checklistItems',
'attachments.user.company',
'attachments',
'gitCommits.repository',
];
}
]) ?? $ticket;
private function freshTicket(Ticket $ticket): Ticket
{
$ticket = $ticket->fresh($this->ticketDetailRelations()) ?? $ticket;
$this->hydrateTicketUsers($ticket);
$comments = \Modules\Helpdesk\Models\Comment::query()
->where('ticket_id', $ticket->id)
->with(['user.company', 'attachments.user.company'])
->with(['user.company', 'attachments'])
->oldest()
->get();
@@ -340,6 +310,39 @@ final class TicketShow extends Component
return $ticket;
}
/**
* Load owner and agent users with one query and attach aliases used by the view/workflow.
*/
private function hydrateTicketUsers(Ticket $ticket): void
{
$userIds = collect([$ticket->user_id, $ticket->agent_id])
->filter()
->map(static fn (mixed $id): int => (int) $id)
->unique()
->values();
if ($userIds->isEmpty()) {
$ticket->setRelation('user', null);
$ticket->setRelation('owner', null);
$ticket->setRelation('agent', null);
return;
}
$users = User::query()
->with('company')
->whereIn('id', $userIds)
->get()
->keyBy('id');
$owner = $users->get((int) $ticket->user_id);
$agent = $ticket->agent_id === null ? null : $users->get((int) $ticket->agent_id);
$ticket->setRelation('user', $owner);
$ticket->setRelation('owner', $owner);
$ticket->setRelation('agent', $agent);
}
/**
* Return assignable users for this ticket.
*
@@ -356,7 +359,7 @@ final class TicketShow extends Component
$columns[] = 'company_id';
}
$query = User::query()->select($columns)->with('company')->orderBy('name');
$query = User::query()->select($columns)->orderBy('name');
if ($this->ticket->company_id !== null && Schema::hasColumn('users', 'company_id')) {
$tenantUsers = (clone $query)
-17
View File
@@ -24,22 +24,6 @@ final class Ticket extends Model
protected $table = 'helpdesk_tickets';
/**
* Relations that are repeatedly displayed in ticket lists/detail views.
*
* Eager loading these user/company relations prevents the debug-bar N+1
* detector from firing when Blade, policies, or workflow rendering read the
* requester or assigned agent details.
*
* @var list<string>
*/
protected $with = [
'user.company',
'owner.company',
'agent.company',
];
/** @var list<string> */
protected $fillable = [
'reference', 'company_id', 'user_id', 'agent_id', 'status_id', 'board_id', 'priority_id', 'category_id', 'project_id', 'team_id',
@@ -68,7 +52,6 @@ final class Ticket extends Model
'time_spent_minutes' => 'integer',
];
/** @return BelongsTo<User, Ticket> */
public function user(): BelongsTo
{
@@ -11,7 +11,8 @@ final class CannedResponseRenderer
{
public function render(string $template, Ticket $ticket, ?User $actor = null): string
{
$ticket->loadMissing(['user.company', 'owner.company', 'agent.company', 'status', 'priority', 'category', 'project', 'team']);
$this->hydrateTicketUsers($ticket);
$ticket->loadMissing(['status', 'priority', 'category', 'project', 'team']);
$actor?->loadMissing('company');
return strtr($template, [
@@ -28,4 +29,42 @@ final class CannedResponseRenderer
'{{ ticket.url }}' => route('helpdesk.tickets.show', ['ticket' => $ticket->getKey()]),
]);
}
/**
* Load requester/agent with one query if the caller has not already attached them.
*/
private function hydrateTicketUsers(Ticket $ticket): void
{
if ($ticket->relationLoaded('owner') && $ticket->relationLoaded('agent')) {
return;
}
$userIds = collect([$ticket->user_id, $ticket->agent_id])
->filter()
->map(static fn (mixed $id): int => (int) $id)
->unique()
->values();
if ($userIds->isEmpty()) {
$ticket->setRelation('user', null);
$ticket->setRelation('owner', null);
$ticket->setRelation('agent', null);
return;
}
$users = User::query()
->with('company')
->whereIn('id', $userIds)
->get()
->keyBy('id');
$owner = $users->get((int) $ticket->user_id);
$agent = $ticket->agent_id === null ? null : $users->get((int) $ticket->agent_id);
$ticket->setRelation('user', $owner);
$ticket->setRelation('owner', $owner);
$ticket->setRelation('agent', $agent);
}
}
+3 -3
View File
@@ -89,7 +89,7 @@ final class TicketService
$this->checklists->applyTemplates($ticket);
$this->attachments->storeMany($ticket, $files, (int) $user->id);
$ticket = $ticket->fresh(['owner', 'agent', 'status', 'priority', 'category', 'project', 'team', 'product', 'tags', 'attachments', 'checklistItems']) ?? $ticket;
$ticket = $ticket->fresh(['status', 'priority', 'category', 'project', 'team', 'product', 'tags', 'attachments', 'checklistItems']) ?? $ticket;
$this->workflow->dispatch($ticket, 'ticket.created', $user);
return $ticket;
@@ -117,7 +117,7 @@ final class TicketService
$this->sla->recordPublicResponse($ticket, $user);
}
$this->attachments->storeMany($ticket, $files, (int) $user->id, $comment);
$this->workflow->dispatch($ticket->fresh(['owner', 'agent', 'status', 'priority', 'category', 'project', 'team']) ?? $ticket, $parentId === null ? 'ticket.commented' : 'ticket.reply_created', $user);
$this->workflow->dispatch($ticket->fresh(['status', 'priority', 'category', 'project', 'team']) ?? $ticket, $parentId === null ? 'ticket.commented' : 'ticket.reply_created', $user);
return $comment->fresh(['user', 'attachments']) ?? $comment;
});
@@ -136,7 +136,7 @@ final class TicketService
'last_activity_at' => now(),
])->save();
$ticket = $ticket->fresh(['owner', 'agent', 'status', 'priority', 'category', 'project', 'team', 'product']) ?? $ticket;
$ticket = $ticket->fresh(['status', 'priority', 'category', 'project', 'team', 'product']) ?? $ticket;
$this->sla->recordClosure($ticket);
$this->workflow->dispatch($ticket, 'ticket.closed', $user);
@@ -22,7 +22,8 @@ final class WorkflowNotificationService
*/
public function dispatch(Ticket $ticket, string $event, ?User $actor = null): void
{
$ticket->loadMissing(['user.company', 'owner.company', 'agent.company', 'status', 'priority', 'category', 'project', 'team', 'product']);
$this->hydrateTicketUsers($ticket);
$ticket->loadMissing(['status', 'priority', 'category', 'project', 'team', 'product']);
$actor?->loadMissing('company');
WorkflowTemplate::query()
@@ -64,6 +65,43 @@ final class WorkflowNotificationService
});
}
/**
* Load requester/agent with one query if the caller has not already attached them.
*/
private function hydrateTicketUsers(Ticket $ticket): void
{
if ($ticket->relationLoaded('owner') && $ticket->relationLoaded('agent')) {
return;
}
$userIds = collect([$ticket->user_id, $ticket->agent_id])
->filter()
->map(static fn (mixed $id): int => (int) $id)
->unique()
->values();
if ($userIds->isEmpty()) {
$ticket->setRelation('user', null);
$ticket->setRelation('owner', null);
$ticket->setRelation('agent', null);
return;
}
$users = User::query()
->with('company')
->whereIn('id', $userIds)
->get()
->keyBy('id');
$owner = $users->get((int) $ticket->user_id);
$agent = $ticket->agent_id === null ? null : $users->get((int) $ticket->agent_id);
$ticket->setRelation('user', $owner);
$ticket->setRelation('owner', $owner);
$ticket->setRelation('agent', $agent);
}
private function recipient(string $role, Ticket $ticket, ?User $actor): ?User
{
return match ($role) {
+4 -11
View File
@@ -193,15 +193,8 @@ versions:
"1.3.10":
changed:
- Fixed ticket detail N+1 warnings by eager-loading requester, agent and company relations on ticket hydration, ticket refresh and ticket control POST actions.
- Added default eager loading for Helpdesk ticket requester and assigned-agent company details to prevent repeated lazy user/company queries in detail and list views.
- Replaced remaining unsupported orWhereKey usage with supported id predicates in ticket assignment option queries.
- Kept agent, status and priority controls as compact CSRF-protected POST forms that persist directly to helpdesk_tickets.
- Fixed remaining ticket-detail N+1 alerts by loading requester and assigned agent users in one query and attaching owner/user/agent relations manually.
- Removed duplicate owner/agent eager-load cycles from ticket control actions, workflow rendering and ticket service refreshes.
- Stopped ticket detail render from reloading the ticket after mount so detail relations are not queried twice in one request.
- Removed remaining orWhereKey usage from Helpdesk ticket agent option queries for Laravel 13 compatibility.
- Kept module.json version in sync with changelog.yaml.
- version: 1.3.11
date: 2026-06-16
changes:
- Eager-loaded ticket requester, assigned agent, and related company records across ticket detail and workflow-control paths to remove remaining N+1 warnings on ticket load and after agent/status/priority actions.
- Added the user relation alias on helpdesk tickets for compatibility with existing views, policies, detectors, and services that access the requester through either user or owner.
+1 -1
View File
@@ -22,5 +22,5 @@
"Modules\\Helpdesk\\Providers\\HelpdeskServiceProvider"
],
"files": [],
"version": "1.3.11"
"version": "1.3.10"
}