5 Commits

Author SHA1 Message Date
central-worker-6
adcb02863d #4781: the nightly auto-close is on the record too
`ticketing:auto-close` closed tickets with a single bulk UPDATE, which
fires no model events. With the audit declaration in place that left one
way of closing a ticket invisible: the Changes log would show a ticket
going open → resolved and then nothing, while the row itself read
`closed`. "Who closed this" is the question the trail exists to answer and
"the nightly sweep did" is a real answer — but only if the sweep writes it.

Closed one at a time through the model, chunked by id so a first sweep
over a backlog does not load every resolved ticket at once. Paging by id
is what makes that safe while the filtered column is being written.
2026-10-19 04:57:38 -07:00
central-worker-6
428a9e42c6 #4781: Help Desk writes to the shell's Changes log
`ticketing` was not one of the apps `audit_events` had ever seen — 4,378
rows, fifteen apps, none of them this one, since the table was created. A
super admin created a ticket and replied to it and the only new row
afterwards was the shell recording their own visit to the log page.

Nothing in core was wrong: AuditRegistry's docblock is explicit that
snap-ins declare their own models from their own providers and that core
never references a snap-in. Ticketing simply never declared.

TicketingServiceProvider::boot() now declares four models, in the shape
FormsServiceProvider uses for FormCollaborator — the exact precedent, for
the exact reason:

  - TicketingAgentAccess — the grant. `settings.agents.store`/`.destroy`
    hand out and take back access to a whole queue of other people's
    correspondence and left no trace at all. This is the one that matters.
  - TicketingGroup — name, prefix, email_address, archived_at, so the one
    button that archives-or-deletes a desk says which it did.
  - Ticket — created, deleted, moved, re-assigned, re-prioritised, closed,
    merged.
  - TicketParticipant — the narrower way somebody comes to see a ticket.

Nothing is declared `sensitive`: that buys read-audit, which on Ticket
would record every agent opening every ticket all day and bury the writes
this card is about. TicketMessage is deliberately not declared either —
it can only ever be created (one controller method, no edit, no delete
route), every inbound email writes one under `system`, and the thread is
already the permanent record of what was said.

removeParticipant deleted through the query builder, which fires no model
events, so it would have recorded every add and no removal — a trail that
reads as though nobody is ever taken off a ticket. It now deletes through
the model.
2026-10-19 04:53:23 -07:00
central-worker-3
eb198ab05a #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>
2026-10-17 04:58:55 -07:00
central-worker-5
dbe3677093 #3144: this app's scheduled jobs run on the school's clock, not the server's
`dailyAt()`/`weeklyOn()` with no `->timezone()` are read against
`config('app.timezone')`, which is UTC on every box this platform runs on, so
every line here that pinned an hour was running seven or eight hours early.
Each now names \App\Support\Timezone::organisation(), the one place the
school's own zone is decided.

organisation() and not current(): there is no signed-in user on a scheduler
tick, so the two agreed only by falling through, and stop agreeing the moment
anything resolves the schedule inside a request. It reads a setting and cannot
throw — it catches its own failure and falls back to America/Vancouver.

The shell half ([dashboard-shell] #3144) carries the guard that keeps this
true: it walks the REAL schedule, so a snapins.lock bump that brings back an
unclocked line reddens the shell's run.
2026-09-28 19:27:56 -07:00
central-worker-6
0dbc81684b #2980: ticketing:auto-close is scheduled by this snap-in, not by the shell
A spawn of [dashboard-shell] #2965. The shell's routes/console.php carried

    Schedule::command('ticketing:auto-close')
        ->dailyAt(ScheduleStagger::dailyAt('ticketing:auto-close'));

for a command only this repository defines, and two repositories deploy at
different moments. On 2026-09-26 01:10 PDT production met the other end of
that: `Command "admissions:remind-unconfirmed-links" is not defined`, exit 1,
every tick for nine and a half hours, with nothing an operator could press —
the code the schedule named was not on the box yet.

TicketingServiceProvider::boot() now registers the event itself, beside the
$this->commands() call that defines AutoCloseTickets. A box has both or
neither, because they arrive in the same commit; a shell without this snap-in
gets no row in `schedule:list` rather than a nightly failure about one.

WHAT DID NOT MOVE. The time: ScheduleStagger is the shell's one place a start
time is chosen — a bare ->daily() is 00:00:00, where the hourly and
quarter-hourly maps already meet ([dashboard-shell] #1910) — so the provider
asks dailyAt() and the job stays at 00:12 exactly. And the absence of a lock:
the shell's line carried no withoutOverlapping() and this adds none, because a
mutex grown in passing is a behaviour change wearing a move's clothes.

PAIRED with the shell half on card/2980: the console line, the
TheShellSchedulesNoSnapInCommandTest::BASELINE entry and the
ScheduledJobCatalog::NOT_A_REGISTERED_JOB excuse all go in the same round as
this lands, and tests/Feature/Ops/TicketingSchedulesItsOwnAutoCloseTest
asserts both ends at once.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-27 04:29:21 -07:00
4 changed files with 276 additions and 8 deletions

View File

@@ -22,9 +22,39 @@ class AutoCloseTickets extends Command
$cutoff = now()->subDays($days); $cutoff = now()->subDays($days);
$count = Ticket::where('status', 'resolved') /*
* CLOSED ONE AT A TIME, THROUGH THE MODEL — [central-ticketing] #4781.
*
* This was a single `->update(['status' => 'closed'])`, which is one
* statement and no model events. Since #4781 a ticket's status change is
* an audited fact, and a bulk update would have left exactly one way of
* closing a ticket invisible: the Changes log would show a ticket going
* open → resolved and then nothing, while the row itself said `closed`.
* "Who closed this" is the question the audit trail exists to answer,
* and "the nightly sweep did, on this date" is a real answer — but only
* if the sweep writes it.
*
* Per-ticket rather than one summary event because the answer has to be
* on THE TICKET: the per-record history panel reads rows whose subject
* is that ticket, and an aggregate "closed 14 tickets" row would not
* appear in any of their histories.
*
* `chunkById`, so a first sweep over a long backlog does not load every
* resolved ticket into memory at once. Paging by id is what makes this
* safe while the filtered column is being written: each page continues
* from the last id seen rather than from an offset, so rows dropping out
* of `status = 'resolved'` cannot cause a page to be skipped.
*/
$count = 0;
Ticket::where('status', 'resolved')
->where('updated_at', '<=', $cutoff) ->where('updated_at', '<=', $cutoff)
->update(['status' => 'closed']); ->chunkById(200, function ($tickets) use (&$count) {
foreach ($tickets as $ticket) {
$ticket->forceFill(['status' => 'closed'])->save();
$count++;
}
});
$this->info("Auto-closed {$count} ticket(s) resolved more than {$days} day(s) ago."); $this->info("Auto-closed {$count} ticket(s) resolved more than {$days} day(s) ago.");

View File

@@ -1082,9 +1082,17 @@ class TicketController extends Controller
return $refusal; return $refusal;
} }
// Fetched and deleted one model at a time rather than as a query-builder
// delete, because a builder delete fires no model events — and since
// #4781 a TicketParticipant row IS an audited fact (who can see this
// ticket's correspondence). Deleting in bulk here would have recorded
// every add and no removal, which is a worse trail than none: it reads
// as though nobody is ever taken off a ticket. ->get() rather than
// ->first(), so a duplicated pair is still fully removed.
TicketParticipant::where('ticket_id', $ticket->id) TicketParticipant::where('ticket_id', $ticket->id)
->where('user_id', $userId) ->where('user_id', $userId)
->delete(); ->get()
->each->delete();
return back()->with('success', 'Participant removed.'); return back()->with('success', 'Participant removed.');
} }

View File

@@ -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

View File

@@ -2,6 +2,7 @@
namespace Dashboard\Ticketing; namespace Dashboard\Ticketing;
use Illuminate\Console\Scheduling\Schedule;
use Illuminate\Support\ServiceProvider; use Illuminate\Support\ServiceProvider;
class TicketingServiceProvider extends ServiceProvider class TicketingServiceProvider extends ServiceProvider
@@ -23,11 +24,200 @@ class TicketingServiceProvider extends ServiceProvider
// Notifications is absent or dark — the notifier no-ops. // Notifications is absent or dark — the notifier no-ops.
\Dashboard\Ticketing\Support\TicketingNotifier::registerCategories(); \Dashboard\Ticketing\Support\TicketingNotifier::registerCategories();
// Declare this app's models to the shell's audit trail (#4781), so a
// ticket, a group retirement and above all an AGENT GRANT are on the
// Changes log. Guarded inside — central-logs is optional on a host.
$this->registerAuditedModels();
if ($this->app->runningInConsole()) { if ($this->app->runningInConsole()) {
$this->commands([ $this->commands([
\Dashboard\Ticketing\Console\Commands\AutoCloseTickets::class, \Dashboard\Ticketing\Console\Commands\AutoCloseTickets::class,
\Dashboard\Ticketing\Console\Commands\RepairMergedTickets::class, \Dashboard\Ticketing\Console\Commands\RepairMergedTickets::class,
]); ]);
} }
$this->scheduleAutoClose();
}
/**
* THE NIGHTLY AUTO-CLOSE IS SCHEDULED HERE, BESIDE THE COMMAND IT NAMES
* ([central-ticketing] #2980, a spawn of [dashboard-shell] #2965).
*
* It used to be one line in the shell's routes/console.php — a file that
* names commands other repositories define. On 2026-09-26 01:10 PDT that
* arrangement cost production a php process a day whose entire output was
* *Command "admissions:remind-unconfirmed-links" is not defined*, exit 1: a
* schedule had reached a box nine and a half hours ahead of the code it
* scheduled, and nothing an operator could press would have fixed it,
* because the code was not there to fix.
*
* Registered from this provider, that is not expressible. The event exists
* on exactly the boxes where `AutoCloseTickets` is registered above,
* because the same `boot()` does both and both arrive in the same commit.
* A shell without central-ticketing simply has no such row in
* `schedule:list`, rather than a nightly failure about one.
*
* ── THE MINUTE IS STILL THE SHELL'S TO DECIDE ──────────────────────────
*
* `ScheduleStagger` is the one place a start time is chosen, and that is
* the whole point of it: a bare `->daily()` is 00:00:00, where every hourly
* and every quarter-hourly task on the platform already boots its own PHP
* inside the scheduler's one cgroup ([dashboard-shell] #1910). A time typed
* in this repository could not know that. Asking `dailyAt()` keeps this job
* at 00:12 — exactly where routes/console.php had it — and keeps the shell's
* map the single account of the night even for a job it no longer schedules.
*
* No `withoutOverlapping()`, and deliberately not added here: the shell's
* line carried none, and a lock is a behaviour change rather than a move.
*/
private function scheduleAutoClose(): void
{
$this->callAfterResolving(Schedule::class, function (Schedule $schedule) {
$schedule->command('ticketing:auto-close')
// …AND THE CLOCK IS THE SCHOOL'S ([dashboard-shell] #3144).
// ScheduleStagger decides the MINUTE; `dailyAt()` on its own is
// read against config('app.timezone'), which is UTC on every box
// here — so 00:12 was landing at 17:12 the previous afternoon,
// i.e. the nightly sweep ran in the middle of the school day.
->timezone(\App\Support\Timezone::organisation())
->dailyAt(\App\Support\Ops\ScheduleStagger::dailyAt('ticketing:auto-close'));
});
}
/**
* HELP DESK WRITES TO THE SHELL'S CHANGES LOG — [central-ticketing] #4781.
*
* Until this method existed, `ticketing` was not one of the apps
* `audit_events` had ever seen: 4,378 rows, fifteen distinct apps, not one
* of them this one. A QA sweep signed in as a super admin, created a ticket,
* replied to it, and found that the newest row in the table afterwards was
* the shell recording the sweep's own visit to the log page. So "who closed
* this", "who deleted that group" and — the one that actually matters —
* "who gave this person the keys to the HR desk's queue" had no answer
* anywhere in Central.
*
* Nothing in core was wrong. `AuditRegistry`'s own docblock is explicit that
* snap-ins register their own models from their own providers and that core
* never references a snap-in. Ticketing simply never declared. This is the
* declaration, in the shape `FormsServiceProvider::registerAuditedGrants()`
* uses for `FormCollaborator` and `FormSharedGroup` — the precedent is exact
* and so is the reason: granting somebody access to other people's
* correspondence is a permission change, and a permission change belongs on
* the record.
*
* Declaring a model is the whole job. The registry attaches `AuditObserver`
* itself, so every create / declared-field update / delete on these four
* classes lands in the Changes tab under app `ticketing` (derived from the
* namespace by `Audit::appForClass()`), with the actor and the subject's
* label frozen into the row at write time.
*
* ── THE FIELD LISTS ARE THE VOLUME CONTROL ───────────────────────────────
*
* Only declared fields are diffed and an update touching none of them
* records nothing at all, so these lists — not the tables' width — decide
* what the log costs. Each is the set of columns somebody would ask an
* auditor about: where a ticket lives, who is on the hook for it, what state
* it is in, and who may see it. `color` on a group and `description` on a
* ticket are left out deliberately: a palette tweak is not an audit event,
* and the body of a complaint is correspondence we are not copying into a
* second table to be kept for seven years.
*
* ── NOTHING HERE IS `sensitive`, AND THAT IS A DECISION ──────────────────
*
* `sensitive: true` buys exactly one thing: read-audit. It makes
* `RecordSensitiveViews` record a `viewed` row every time a route implicitly
* binds the model on a successful GET. For `Ticket` that is every agent
* opening every ticket all day — volume that would bury the writes this card
* is about, to answer a question nobody asked. It changes no retention
* window either (writes are kept on the operator's Changes window
* regardless; `sensitive` only lengthens what READ rows get). So: writes,
* declared; reads, not. If ticketing ever needs "who read this ticket", that
* is its own card and its own conversation about volume.
*
* ── WHAT IS DELIBERATELY NOT DECLARED ────────────────────────────────────
*
* `TicketMessage`. The reporter expected a row for the reply they posted,
* and it is the right instinct on the wrong model: a message can only ever
* be created (there is no edit route and no delete route — see
* `TicketMessageController`, which has one method), so auditing it could
* only ever mean one row per message. Every inbound email through an
* `EmailConnection` writes one, under `system`, and a merge rewrites whole
* runs of them. That is a stream of "a message exists" against the record of
* who CHANGED something, which is what the Changes tab is for — and the
* messages themselves are already the permanent, readable record of what was
* said. The ticket's own row says who opened it and who closed it; the
* thread says what was written in between.
*
* A group hard-delete cascades its agent-access rows in the DATABASE (see
* the migration's foreign key), so those grants end without a `deleted` row
* of their own. The group's deletion is recorded, which is the question
* being asked — but it is worth knowing that the two are one event here and
* not several.
*
* Guarded on the class and wrapped in a catch for the same reason Forms and
* Files are: central-logs is optional on any given host, and a declaration
* must never be the reason a box fails to boot.
*/
private function registerAuditedModels(): void
{
if (! class_exists(\Central\Logs\Support\AuditRegistry::class)) {
return;
}
try {
/*
* THE GRANT. The reason this card was filed and the first thing
* declared: `ticketing.settings.agents.store` / `.destroy` hand out
* and take back a person's access to a help desk group's entire
* queue, and did so leaving no trace at all.
*
* The label is resolved the long way round because this model has no
* `user()` relation and must not grow one — `TicketingSettingsController`
* HYDRATES `$access->user` as a plain attribute for the Settings page,
* and a relation of the same name would fight it.
*/
\Central\Logs\Support\AuditRegistry::audit(
\Dashboard\Ticketing\Models\TicketingAgentAccess::class,
fields: ['user_id', 'group_id', 'role'],
label: fn ($access) => (\App\Models\User::find($access->user_id)?->name ?? 'Deleted user')
.' on '.($access->group?->name ?? 'a help desk group'),
);
// The desk itself: renamed, re-prefixed, pointed at a different
// inbox, archived, restored, or deleted outright. `archived_at` is
// what tells archive from delete — one button does both, chosen by
// whether the group still owns tickets.
\Central\Logs\Support\AuditRegistry::audit(
\Dashboard\Ticketing\Models\TicketingGroup::class,
fields: ['name', 'prefix', 'email_address', 'archived_at'],
label: fn ($group) => ($group->name ?: 'Help desk group')
.($group->prefix ? ' ('.$group->prefix.')' : ''),
);
// The ticket: created, deleted, moved between desks, re-assigned,
// re-prioritised, closed, merged. `merged_into_id` is declared
// beside `status` because a merge is a pair of columns moving
// together and reading either one alone misreports it.
\Central\Logs\Support\AuditRegistry::audit(
\Dashboard\Ticketing\Models\Ticket::class,
fields: [
'group_id', 'status', 'assigned_to', 'priority_id',
'project_id', 'due_date', 'merged_into_id', 'title',
],
label: fn ($ticket) => trim(($ticket->number ?: 'Ticket').' '.($ticket->title ?? '')),
);
// Participants are the OTHER way somebody comes to see a ticket's
// correspondence — narrower than an agent grant (one ticket, not a
// whole queue) but the same kind of fact.
\Central\Logs\Support\AuditRegistry::audit(
\Dashboard\Ticketing\Models\TicketParticipant::class,
fields: ['ticket_id', 'user_id', 'added_by'],
label: fn ($participant) => (\App\Models\User::find($participant->user_id)?->name ?? 'Deleted user')
.' on ticket '.($participant->ticket?->number ?? '#'.$participant->ticket_id),
);
} catch (\Throwable) {
// Never let a declaration break boot.
}
} }
} }