#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.
This commit is contained in:
@@ -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.');
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user