From 428a9e42c6e3c896880dc88cd7236ef1fb556c85 Mon Sep 17 00:00:00 2001 From: central-worker-6 Date: Mon, 19 Oct 2026 04:53:23 -0700 Subject: [PATCH 1/3] #4781: Help Desk writes to the shell's Changes log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- src/Http/Controllers/TicketController.php | 10 +- src/TicketingServiceProvider.php | 142 ++++++++++++++++++++++ 2 files changed, 151 insertions(+), 1 deletion(-) diff --git a/src/Http/Controllers/TicketController.php b/src/Http/Controllers/TicketController.php index 411bb6c..21a4d84 100644 --- a/src/Http/Controllers/TicketController.php +++ b/src/Http/Controllers/TicketController.php @@ -1082,9 +1082,17 @@ class TicketController extends Controller 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) ->where('user_id', $userId) - ->delete(); + ->get() + ->each->delete(); return back()->with('success', 'Participant removed.'); } diff --git a/src/TicketingServiceProvider.php b/src/TicketingServiceProvider.php index 1f52c25..a90d51c 100644 --- a/src/TicketingServiceProvider.php +++ b/src/TicketingServiceProvider.php @@ -24,6 +24,11 @@ class TicketingServiceProvider extends ServiceProvider // Notifications is absent or dark — the notifier no-ops. \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()) { $this->commands([ \Dashboard\Ticketing\Console\Commands\AutoCloseTickets::class, @@ -78,4 +83,141 @@ class TicketingServiceProvider extends ServiceProvider ->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. + } + } } From adcb02863d674fbcfc3d8fe13e0b8816970bc856 Mon Sep 17 00:00:00 2001 From: central-worker-6 Date: Mon, 19 Oct 2026 04:57:38 -0700 Subject: [PATCH 2/3] #4781: the nightly auto-close is on the record too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- src/Console/Commands/AutoCloseTickets.php | 34 +++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Console/Commands/AutoCloseTickets.php b/src/Console/Commands/AutoCloseTickets.php index 4f48099..fd1d529 100644 --- a/src/Console/Commands/AutoCloseTickets.php +++ b/src/Console/Commands/AutoCloseTickets.php @@ -22,9 +22,39 @@ class AutoCloseTickets extends Command $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) - ->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."); From 9cc2e59b4517c50cfb9484096eeba0a15c50a23e Mon Sep 17 00:00:00 2001 From: central-worker-8 Date: Mon, 19 Oct 2026 05:16:23 -0700 Subject: [PATCH 3/3] #4781: the bulk merged-repair is on the record too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MergedTicketRepair::repairAll() re-marks tickets as merged (permanently read-only) per model instead of with one builder update, so each frozen ticket gets its own Changes-log entry — the same as the per-ticket button. Co-Authored-By: Claude Opus 5.5 --- src/Support/MergedTicketRepair.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Support/MergedTicketRepair.php b/src/Support/MergedTicketRepair.php index 8621f33..d669fee 100644 --- a/src/Support/MergedTicketRepair.php +++ b/src/Support/MergedTicketRepair.php @@ -84,17 +84,22 @@ class MergedTicketRepair */ public static function repairAll(string $by): int { - $numbers = Ticket::query()->inconsistentlyMerged()->pluck('number')->all(); + $tickets = Ticket::query()->inconsistentlyMerged()->orderBy('id')->get(); - if ($numbers === []) { + if ($tickets->isEmpty()) { Log::info("[ticketing] #938 merged-ticket repair ({$by}): nothing to repair."); return 0; } - $changed = Ticket::query()->inconsistentlyMerged()->update([ - 'status' => Ticket::STATUS_MERGED, - ]); + // Per model, not one builder update (#4781/#4807): `merged` makes a ticket + // permanently read-only, and a bulk statement would freeze it with nothing + // on the Changes log — while repair() below, the same change one ticket at + // a time, is recorded. + $tickets->each(fn (Ticket $ticket) => $ticket->forceFill(['status' => Ticket::STATUS_MERGED])->save()); + + $numbers = $tickets->pluck('number')->all(); + $changed = $tickets->count(); // The numbers, not just the count. These rows are being made read-only, // and "which ones" is the question the office will ask afterwards.