#4710: Help Desk Settings shows the help desk that exists
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
private function requireManagerAccess(int $groupId): void
|
||||||
{
|
{
|
||||||
$isManager = TicketingAgentAccess::where('user_id', Auth::id())
|
$isManager = TicketingAgentAccess::where('user_id', Auth::id())
|
||||||
@@ -71,14 +80,38 @@ class TicketingSettingsController extends Controller
|
|||||||
|
|
||||||
$userId = Auth::id();
|
$userId = Auth::id();
|
||||||
$isBootstrap = $this->isBootstrapState();
|
$isBootstrap = $this->isBootstrapState();
|
||||||
|
$isSiteAdmin = $this->isSiteAdmin();
|
||||||
$myGroupIds = TicketingAgentAccess::where('user_id', $userId)->pluck('group_id');
|
$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
|
// 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.
|
// remain visible, so a manager can see what happened and restore it.
|
||||||
// Actives first, then archived, each alphabetical.
|
// Actives first, then archived, each alphabetical.
|
||||||
$groups = $isBootstrap
|
$groups = $isBootstrap
|
||||||
? collect()
|
? 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
|
// tickets_count decides archive-vs-delete, so the UI can warn
|
||||||
// the operator which one their click is about to do.
|
// the operator which one their click is about to do.
|
||||||
->withCount('tickets')
|
->withCount('tickets')
|
||||||
@@ -88,7 +121,10 @@ class TicketingSettingsController extends Controller
|
|||||||
|
|
||||||
$agents = $isBootstrap
|
$agents = $isBootstrap
|
||||||
? collect()
|
? 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()) {
|
if ($agents->isNotEmpty()) {
|
||||||
$agentUserIds = $agents->pluck('user_id')->unique();
|
$agentUserIds = $agents->pluck('user_id')->unique();
|
||||||
@@ -102,13 +138,16 @@ class TicketingSettingsController extends Controller
|
|||||||
|
|
||||||
$projects = $isBootstrap
|
$projects = $isBootstrap
|
||||||
? collect()
|
? collect()
|
||||||
: TicketingProject::whereIn('group_id', $myGroupIds)
|
: TicketingProject::when($visibleGroupIds !== null,
|
||||||
|
fn($q) => $q->whereIn('group_id', $visibleGroupIds))
|
||||||
->orderBy('name')
|
->orderBy('name')
|
||||||
->get();
|
->get();
|
||||||
|
|
||||||
$emailConnections = $isBootstrap
|
$emailConnections = $isBootstrap
|
||||||
? collect()
|
? collect()
|
||||||
: EmailConnection::whereIn('group_id', $myGroupIds)->get();
|
: EmailConnection::when($visibleGroupIds !== null,
|
||||||
|
fn($q) => $q->whereIn('group_id', $visibleGroupIds))
|
||||||
|
->get();
|
||||||
|
|
||||||
return Inertia::render('Ticketing/Settings', [
|
return Inertia::render('Ticketing/Settings', [
|
||||||
'activeTab' => $tab,
|
'activeTab' => $tab,
|
||||||
@@ -119,8 +158,9 @@ class TicketingSettingsController extends Controller
|
|||||||
'ticketStatuses' => TicketStatus::orderBy('sort_order')->get(),
|
'ticketStatuses' => TicketStatus::orderBy('sort_order')->get(),
|
||||||
'emailConnections' => $emailConnections,
|
'emailConnections' => $emailConnections,
|
||||||
'myGroupIds' => $myGroupIds,
|
'myGroupIds' => $myGroupIds,
|
||||||
|
'myManagedGroupIds' => $myManagedGroupIds,
|
||||||
'isBootstrap' => $isBootstrap,
|
'isBootstrap' => $isBootstrap,
|
||||||
'isSiteAdmin' => $this->isSiteAdmin(),
|
'isSiteAdmin' => $isSiteAdmin,
|
||||||
'autoCloseDays' => (int) Setting::get('ticketing.auto_close_days', 0),
|
'autoCloseDays' => (int) Setting::get('ticketing.auto_close_days', 0),
|
||||||
// #938 — the deploy-time repair, and how many rows it would touch. The
|
// #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
|
// count is what turns an abstract toggle into a decision somebody can
|
||||||
|
|||||||
Reference in New Issue
Block a user