From eb198ab05a9038d905821e2f2854d16c52f3a767 Mon Sep 17 00:00:00 2001 From: central-worker-3 Date: Sat, 17 Oct 2026 04:58:55 -0700 Subject: [PATCH] #4710: Help Desk Settings shows the help desk that exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Groups, Agents and Projects tabs were scoped to the viewer's `ticketing_agent_access` rows. A site admin holds none of those, so the lists came back empty and the Vue — which decided between "first run" and "nothing for you" on list length alone — rendered the first-run copy: "No groups yet. Create your first group above.", on a page whose own props said `isBootstrap: false` and `isSiteAdmin: true`. The two empty states on that page contradicted each other and the one that won was the one inviting a duplicate of a group that already existed. On dev that was the normal case, not a corner: every pre-existing group had zero agent-access rows, so the whole configuration was invisible to the one role meant to own it. The list is now what this page is for: a site admin sees every group (archived ones included, as before), agent, project and email connection. What they may DO is unchanged — requireManagerAccess() still gates every write per group, which is what GroupRetirementTest already demanded of exactly this user. So the page also ships `myManagedGroupIds`: the rows a viewer cannot act on say "Not yours to manage" instead of offering an Edit that 403s, and the add-agent / add-project group pickers offer only groups the viewer manages. Known and deliberately left: a group with zero agent-access rows is still frozen — adding its first agent needs a manager row on it, which nobody holds. It is now visible and labelled rather than invisible, but thawing it is a separate decision (operator notified). Co-Authored-By: Claude Opus 5 (1M context) --- .../TicketingSettingsController.php | 50 +++++++++++++++++-- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/src/Http/Controllers/TicketingSettingsController.php b/src/Http/Controllers/TicketingSettingsController.php index cd5720f..87dbd87 100644 --- a/src/Http/Controllers/TicketingSettingsController.php +++ b/src/Http/Controllers/TicketingSettingsController.php @@ -54,6 +54,15 @@ class TicketingSettingsController extends Controller } } + /** + * Require manager rights over one group. + * + * Deliberately NOT widened to site admins: GroupRetirementTest pins that a + * super admin with no agent access is refused here, and #4710 is a + * visibility bug, not a licence to hand every `ticketing.settings` holder + * manager rights over every group. Seeing a group you do not manage is the + * fix; acting on it is a separate decision for a human to make. + */ private function requireManagerAccess(int $groupId): void { $isManager = TicketingAgentAccess::where('user_id', Auth::id()) @@ -71,14 +80,38 @@ class TicketingSettingsController extends Controller $userId = Auth::id(); $isBootstrap = $this->isBootstrapState(); + $isSiteAdmin = $this->isSiteAdmin(); $myGroupIds = TicketingAgentAccess::where('user_id', $userId)->pluck('group_id'); + // The groups this user may actually ACT on — requireManagerAccess() in + // prop form. A site admin now sees groups they do not manage (#4710), + // so the page has to say which rows its buttons will work on instead + // of offering an Edit that 403s. + $myManagedGroupIds = TicketingAgentAccess::where('user_id', $userId) + ->where('role', 'manager') + ->pluck('group_id'); + + // What this page is allowed to SHOW — separate from what it lets you + // DO, which stays requireManagerAccess() per group. A site admin sees + // every group: scoping the list to agent-access rows left an admin + // staring at "No groups yet. Create your first group above." with + // twelve groups in the database, inviting a duplicate (#4710). + // `null` means "no restriction". + // + // Every one of these routes sits behind `permission:ticketing.settings` + // and isSiteAdmin() is `is_super_admin || can('ticketing.settings')`, + // so today this is true for anyone who reaches the page at all. The + // scoped branch is kept because it is the correct behaviour if that + // route gate is ever loosened — it is not load-bearing right now. + $visibleGroupIds = $isSiteAdmin ? null : $myGroupIds; + // Archived groups stay in this list — this is the one place they must // remain visible, so a manager can see what happened and restore it. // Actives first, then archived, each alphabetical. $groups = $isBootstrap ? collect() - : TicketingGroup::whereIn('id', $myGroupIds) + : TicketingGroup::when($visibleGroupIds !== null, + fn($q) => $q->whereIn('id', $visibleGroupIds)) // tickets_count decides archive-vs-delete, so the UI can warn // the operator which one their click is about to do. ->withCount('tickets') @@ -88,7 +121,10 @@ class TicketingSettingsController extends Controller $agents = $isBootstrap ? collect() - : TicketingAgentAccess::whereIn('group_id', $myGroupIds)->with('group')->get(); + : TicketingAgentAccess::when($visibleGroupIds !== null, + fn($q) => $q->whereIn('group_id', $visibleGroupIds)) + ->with('group') + ->get(); if ($agents->isNotEmpty()) { $agentUserIds = $agents->pluck('user_id')->unique(); @@ -102,13 +138,16 @@ class TicketingSettingsController extends Controller $projects = $isBootstrap ? collect() - : TicketingProject::whereIn('group_id', $myGroupIds) + : TicketingProject::when($visibleGroupIds !== null, + fn($q) => $q->whereIn('group_id', $visibleGroupIds)) ->orderBy('name') ->get(); $emailConnections = $isBootstrap ? collect() - : EmailConnection::whereIn('group_id', $myGroupIds)->get(); + : EmailConnection::when($visibleGroupIds !== null, + fn($q) => $q->whereIn('group_id', $visibleGroupIds)) + ->get(); return Inertia::render('Ticketing/Settings', [ 'activeTab' => $tab, @@ -119,8 +158,9 @@ class TicketingSettingsController extends Controller 'ticketStatuses' => TicketStatus::orderBy('sort_order')->get(), 'emailConnections' => $emailConnections, 'myGroupIds' => $myGroupIds, + 'myManagedGroupIds' => $myManagedGroupIds, 'isBootstrap' => $isBootstrap, - 'isSiteAdmin' => $this->isSiteAdmin(), + 'isSiteAdmin' => $isSiteAdmin, 'autoCloseDays' => (int) Setting::get('ticketing.auto_close_days', 0), // #938 — the deploy-time repair, and how many rows it would touch. The // count is what turns an abstract toggle into a decision somebody can