18 Commits

Author SHA1 Message Date
paired-gate
b01eb91493 Merge commit '7e675e7beb25a8ddcb3f6cc6f0101f0ffd668e1e' into HEAD 2026-11-05 04:46:14 -07:00
central-worker-1
7e675e7beb #6168: Help Desk Settings can reach its email connections
The email-connection feature was whole on the server and unreachable in the
product. `ticketing_email_connections` has shipped since 2026_04_08_000008,
the three write routes are live and healthy — an empty POST answers a 422
validation bag — and index() has been computing an `emailConnections` prop on
EVERY settings render. The one thing missing was the segment that lets
somebody look at it: `email` was not in the `{tab}` whitelist, so the only
deep link anybody could guess fell through to the POST-only `/settings/email`
and answered 405 while its five siblings answered 200.

The segment is in the whitelist now, and the shell's Settings.vue grows the
tab that renders the prop (dashboard-shell, same branch).

Two things about the prop had to change before a UI could read it:

  · the stored mailbox PASSWORD no longer leaves the server. It was sitting in
    `config` on a prop that ships to everyone who can open Help Desk Settings;
    nothing rendered it before, so nothing is lost by shaping the rows by hand
    and sending `has_password` instead of the credential.

  · a save with the password box blank KEEPS the stored password. The form
    cannot prefill a password it is never sent, so an empty box has to mean
    "unchanged" — otherwise correcting a host would silently disconnect the
    mailbox, and the row would still read as configured.

Proof: tests/Feature/Ticketing/EmailConnectionsHaveADoorTest.php (shell) —
17 tests, including every tab key read out of the Vue's own TAB_KEYS and
asserted 200.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-11-04 06:55:35 -07:00
central-worker-1
16475ccac1 #6154: priorities 422 on an oversized sort_order instead of 500
`ticketing_priority_levels.sort_order` is an unsigned smallint (0..65535),
the same shape as `ticketing_statuses.sort_order`, but storePriority() and
updatePriority() validated it as `integer|min:0` with no upper bound. A value
past the ceiling passed validation and overflowed the column on INSERT/UPDATE
as an unhandled 500 — reachable by typing into the Sort Order box, which is a
plain `type="number" min="0"`.

#6081 put `max:65535` on storeStatus()/updateStatus() and left this identical
pair untouched. Same rule, same two places.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-11-04 05:53:38 -07:00
central-worker-6
e4d0622361 #6081: statuses store 422s on oversized sort_order instead of 500
sort_order was validated as integer|min:0 with no upper bound, but the
column is an unsigned smallint (0..65535). A client-supplied value
above the ceiling passed validation and overflowed the column on
INSERT/UPDATE, surfacing as an unhandled 500. Added max:65535 to both
storeStatus and updateStatus, which take the same client field through
the same rule.
2026-11-03 06:52:19 -07:00
central-worker-4
abd21f73f2 #5002: the rescue stamp is spent once the appointee uses it
VERIFY #5008 attacked the state #5002's own tests never built: not the rescue
that goes wrong and is undone within the minute, but the one that WORKS. A
group that was orphaned, was rescued, and is now being run by the person the
rescue appointed — with `recovered_by` still sitting on that person's row,
because the only thing that cleared it was the appointee re-saving their own
row, a write nobody makes.

So arm 3 of requireRemoveAccess() stayed open on the sole working manager of a
group that is plainly somebody else's business, which is the line the method's
own docblock says the stamp is narrow enough not to cross: strip the manager,
then "rescue" the group that press just orphaned, and a site admin who never
ran the desk manages it with the manager who staffed it gone. Two presses.

settleRescueStamp() clears the stamp on the actor's own row the moment they
ACT as the manager the rescue made them — staff the group, rename it, archive
it, give it a project or an email connection, remove a row: anything this file
lets a manager do to their own group. It is called from the three gates, not
from each action, because the gates are where "this write was authorised by
the actor's OWN manager row" is already decided, and a read of the page never
comes through them. The undo therefore lasts exactly as long as the mistake it
is for: until somebody takes the appointment up.

removableAgentIds follows without a change — it reads `recovered_by`, so the
button disappears with the gate.

  green: test_a_successful_rescue_does_not_leave_the_rescuer_able_to_take_the_group_over
         test_a_stamp_earned_on_one_group_is_not_a_claim_on_another
         all 17 of OrphanRecoveryIsReversibleTest, 215 Feature/Ticketing,
         3917 tests/Unit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-21 06:58:49 -07:00
central-worker-4
51073fba1e #5002: the orphan recovery goes both ways
#4806 gave a site admin one write on a help desk group nobody is left to
run, and #4926 made the Person field of that form a people search. Neither
closed the other half: `storeAgent()` was gated on
requireManagerAccessOrOrphanRecovery(), `destroyAgent()` on plain
requireManagerAccess(). So the rescue could appoint and never un-appoint —
the row it had just written sat in the rescuer's own Agents list marked
"Not yours to manage", with no Remove control and a 403 behind the DELETE.

A `manager` press was worse: the write ends the orphan state, so the group
left `orphanedGroupIds` and the admin could not remove the row, appoint
anybody else, or appoint themselves. The only person who could undo it was
the person wrongly appointed — #4806's frozen group, reached through
#4806's cure.

destroyAgent() now asks requireRemoveAccess(): a manager of the group, or a
site admin while the group is STILL orphaned (symmetric with the write and
strictly narrower — on such a group they could appoint themselves and then
remove anything anyway), or a site admin undoing a row their own rescue
wrote. That last arm needs provenance, hence `recovered_by`: "the group
would be orphaned without this row" cannot tell a mistaken appointment from
the sole working manager of a healthy group, and admitting the second would
let any admin strip a manager and seize the desk (#4710). The stamp is
cleared the moment a manager of the group writes the row themselves.

`removableAgentIds` is that gate in prop form, so the Remove button and the
DELETE cannot disagree, and the confirmation says when a removal hands the
group back to having nobody.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-21 06:22:24 -07:00
central-worker-3
8b4f788e43 #4932: the picker's two-character floor counts characters, not bytes
VERIFY #4932's named red — test_a_single_non_ascii_letter_is_still_a_one_
character_query — stood: searchUsers() guarded with strlen($q) < 2 under a
comment that calls the floor "two characters". A single Ł, É, ü or a CJK
surname is one letter and two or three bytes, so one keystroke cleared the
byte floor and answered with full chip rows — the directory spill the floor
exists to prevent, on exactly the names a school has the most trouble typing.

mb_strlen, matching admin.settings.users.search (the endpoint this picker's
row shape is keyed against), which answers [] for the same query. The
verifier's test is carried onto this branch so the fix lands with its proof.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-20 07:28:24 -07:00
central-worker-3
a977aedc95 #4926: appointing a help desk agent is a people search, and the receipt names them
The Agents tab's add-an-agent form opened with

    <label class="form-label">User ID</label>
    <input v-model="agentForm.user_id" required type="number"
           placeholder="User ID" class="form-input" />
    <p class="form-hint">Find IDs in Admin → Users</p>

— a primary key asked of an office administrator, on the one screen #4806
documents as the only recovery for a help desk group whose last manager can no
longer act. `exists:users,id` refuses an id nobody holds, so the failure that
mattered was never a typo that errors: 47 typed for 74 is two real accounts,
and the one written is handed `agent` — or `manager` — over a whole queue, with
the person named nowhere before the press or after it. #4781's audit row was
therefore the first place anybody could find out.

Two writes here, both small:

  * `searchUsers()` — `q` over name and email, answered by `PersonChip::rows()`,
    the platform's one serializer for a searchable person (#871), so the
    operator's People setting reaches this picker like every other one. The
    gate is the page's own requireAgentAccess() and not a per-group one: the
    form picks the PERSON before the GROUP, and the write still goes through
    requireManagerAccessOrOrphanRecovery() unchanged.

  * `storeAgent()`'s flash names the appointee, through `PersonNames::display()`
    so it reads the way the chip above it does (#524). The recovery sentence
    keeps every word it had and gains the other half of itself.

The shell half — the picker's markup, its proofs and the chip-surface
registration — is on dashboard-shell card/4926-….

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-20 07:07:46 -07:00
10ef42d78c #4806 bring central-ticketing master into the half before the paired landing
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPh12gFm8cqvAt2aPtPw3q
2026-10-19 23:24:18 -04:00
central-worker-8
9cc2e59b45 #4781: the bulk merged-repair is on the record too
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 <noreply@anthropic.com>
2026-10-19 05:16:23 -07:00
central-worker-5
94fb6ac53c #4806: a help desk group whose last manager can no longer act is recoverable
Every write in TicketingSettingsController is requireManagerAccess() — an
existing manager of that exact group, or 403 — and settings.agents.store, the
only route that writes a `ticketing_agent_access` row, is gated the same way.
storeGroup() auto-appoints a manager only for a group it creates. So the
moment a group's last manager stopped being able to act, the group froze for
good: no rename, no archive, no delete, no project, no agent, and no ticket in
it ever assigned, worked or closed. The page's own answer on the Agents tab
was "Ask a manager of the group to add you", which on such a group names
nobody.

On dev that was every group: all 15 held exactly one agent row, the same
person on every one, and that account had been rewritten into an
admissions-only applicant (#4202/#4203) and could not reach /app/ticketing at
all. Fifteen groups, fourteen tickets, nobody who could administer any of it,
and nothing a super admin could press — the "ticketing lifecycle is DEAD on
dev" observation of 2026-10-18 is this, not a seed gap.

One carve-out, and no more: a site admin may write an agent row for a group
that has NO manager left who could still use this page. "Still" is the
operative word and canStillAdminister() is its definition — the account must
exist, be active, and hold `ticketing.settings`, which is exactly what the
route itself demands, so it answers "could this manager have done this write
themselves". A manager row outlives all three.

#4710's rule is untouched: the carve-out is a property of the GROUP, not of
the caller, so a group somebody is still running remains none of an admin's
business, and the rescue grants the rescuer nothing beyond the row they wrote
— appoint somebody else and the very next request is refused again.

The page gets `orphanedGroupIds` so the Agents tab can offer the recovery,
label those groups "— no manager", and explain what appointing one does
(shell side).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-19 05:15:12 -07:00
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
central-worker-3
7522d48ab9 Merge master (#1706 participants-picker ordering) into card/1710 2026-09-13 13:39:19 -07:00
central-worker-1
b80e200f83 The singular delete-flash agrees with itself: "so it stays usable" (#1710)
deletionFlash() (added by #1567) pluralised the count, the noun, the
auxiliary and the pronoun, but left the verb fixed at "stay" — so the
singular branch, which is the common one, shipped "1 ticket was merged
into it ... so it stay usable".

The verb agrees with the pronoun, so it belongs in the same ternary:
"it stays" / "they stay". Nothing else about the message changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-11 14:15:01 -07:00
8 changed files with 817 additions and 32 deletions

View File

@@ -0,0 +1,43 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
/**
* WHO WROTE THIS ROW AS A RESCUE — [central-ticketing] #5002.
*
* #4806 let a site admin appoint somebody to a help desk group that has no
* manager left who could still act. That write is the only way back into such
* a group, and it ENDS the condition that allowed it: appoint a `manager` and
* the group has a manager again, so the recovery arm shuts behind the press.
* With `destroyAgent()` gated on plain manager rights, a rescuer who named the
* wrong person could not take the row back — the only person who could was the
* person wrongly appointed.
*
* `orphanedGroupIds()` cannot tell that case from a healthy group with one
* working manager: both are "orphaned if you disregard this row", and allowing
* the second would let any site admin strip a manager from a group that is
* none of their business (#4710). So the row has to say where it came from.
*
* Nullable, and null on every row written the ordinary way: this is not
* "created_by", it is "this row exists because <user> rescued the group, and
* it is theirs to undo". No foreign key — `user_id` on this table has none
* either, and a deleted rescuer should leave the row alone, not take it.
*/
return new class extends Migration
{
public function up(): void
{
Schema::table('ticketing_agent_access', function (Blueprint $table) {
$table->unsignedBigInteger('recovered_by')->nullable()->after('role');
});
}
public function down(): void
{
Schema::table('ticketing_agent_access', function (Blueprint $table) {
$table->dropColumn('recovered_by');
});
}
};

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

@@ -776,12 +776,14 @@ class TicketController extends Controller
if ($handled['unmerged'] !== []) { if ($handled['unmerged'] !== []) {
$parts[] = sprintf( $parts[] = sprintf(
'%s %s merged into it and %s been un-merged (status %s) so %s stay usable: %s.', '%s %s merged into it and %s been un-merged (status %s) so %s usable: %s.',
count($handled['unmerged']), count($handled['unmerged']),
count($handled['unmerged']) === 1 ? 'ticket was' : 'tickets were', count($handled['unmerged']) === 1 ? 'ticket was' : 'tickets were',
count($handled['unmerged']) === 1 ? 'has' : 'have', count($handled['unmerged']) === 1 ? 'has' : 'have',
Ticket::STATUS_UNMERGED, Ticket::STATUS_UNMERGED,
count($handled['unmerged']) === 1 ? 'it' : 'they', // The verb agrees with the pronoun, so it lives in the same ternary:
// "so it stays usable" / "so they stay usable".
count($handled['unmerged']) === 1 ? 'it stays' : 'they stay',
implode(', ', $handled['unmerged']), implode(', ', $handled['unmerged']),
); );
} }
@@ -1080,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

@@ -3,6 +3,9 @@
namespace Dashboard\Ticketing\Http\Controllers; namespace Dashboard\Ticketing\Http\Controllers;
use App\Models\Setting; use App\Models\Setting;
use App\Models\User;
use App\Support\PersonChip;
use App\Support\PersonNames;
use Dashboard\Ticketing\Models\EmailConnection; use Dashboard\Ticketing\Models\EmailConnection;
use Dashboard\Ticketing\Models\PriorityLevel; use Dashboard\Ticketing\Models\PriorityLevel;
use Dashboard\Ticketing\Models\TicketingAgentAccess; use Dashboard\Ticketing\Models\TicketingAgentAccess;
@@ -31,8 +34,21 @@ class TicketingSettingsController extends Controller
*/ */
private function isSiteAdmin(): bool private function isSiteAdmin(): bool
{ {
$user = Auth::user(); return $this->isSiteAdminUser(Auth::user());
return $user && ($user->is_super_admin || $user->can('ticketing.settings')); }
/**
* The same question asked about SOMEBODY ELSE — one definition, two callers.
*
* #4806 needs to know whether the manager a group already has could still
* use this page, which is isSiteAdmin()'s test applied to a user who is not
* signed in. Two copies of that rule would drift, and the drift would be
* invisible: the recovery path below opens exactly when this returns false
* for every manager of a group.
*/
private function isSiteAdminUser(?User $user): bool
{
return $user !== null && ($user->is_super_admin || $user->can('ticketing.settings'));
} }
/** /**
@@ -54,15 +70,175 @@ 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.
*
* TWO writes do not come through here, and they are the same write in
* both directions. `storeAgent()` uses
* requireManagerAccessOrOrphanRecovery(), because a group with no manager
* left has nobody this gate could ever admit, and the group and its
* tickets are frozen for good (#4806); `destroyAgent()` uses
* requireRemoveAccess(), so that recovery is not one-way (#5002) — an
* appoint that could never be un-appointed froze the group again through
* its own cure. Every other write on somebody else's group still ends up
* here.
*/
private function requireManagerAccess(int $groupId): void private function requireManagerAccess(int $groupId): void
{ {
$isManager = TicketingAgentAccess::where('user_id', Auth::id()) if (!$this->isManagerOf($groupId)) {
abort(403);
}
$this->settleRescueStamp($groupId);
}
/**
* THE RESCUE STAMP ENDS WHEN THE APPOINTEE USES WHAT IT GAVE THEM — #5008.
*
* `recovered_by` (#5002) exists so a rescuer can take back an appointment
* they got wrong, including a `manager` press that shut the orphan arm
* behind it. It was written as an undo and it has to expire like one. As
* first built, the only thing that cleared it was the appointee re-saving
* their own row — a write nobody makes — so the stamp sat on the row for
* good, and a successful rescue left the rescuer a standing lever over a
* desk somebody else was now running: strip the manager, then "rescue" the
* group that press just orphaned, and a site admin who never ran it
* manages it (#5008). That is #4710's line, which the undo must not cross.
*
* So the stamp settles the moment the appointee ACTS as the manager they
* were made: staffs the group, renames it, archives it, gives it a project
* or an email connection — anything this file lets a manager do to their
* own group. The rescue has been taken up, the group is theirs, and the
* rescuer's claim on it is spent. Until then nothing has happened but the
* press itself, which is exactly the case the undo is for.
*
* Called from the gates and not from each action because the gates are
* where "this write was authorised by the actor's OWN manager row" is
* already decided; a read of the page never comes through them.
*/
private function settleRescueStamp(int $groupId): void
{
TicketingAgentAccess::where('user_id', Auth::id())
->where('group_id', $groupId)
->whereNotNull('recovered_by')
->update(['recovered_by' => null]);
}
/** Does the signed-in user hold a `manager` row on this group? */
private function isManagerOf(int $groupId): bool
{
return TicketingAgentAccess::where('user_id', Auth::id())
->where('group_id', $groupId) ->where('group_id', $groupId)
->where('role', 'manager') ->where('role', 'manager')
->exists(); ->exists();
if (!$isManager) { }
abort(403);
/**
* Could this person still administer a help desk group today?
*
* NOT "do they hold a manager row" — the row is what we are judging. This
* asks whether, if they signed in right now, the Agents tab's own POST
* would work for them: the account has to still exist, still be active,
* and still hold the permission every route in this file sits behind.
*
* A manager row outlives all three. On dev the sole manager of all 15
* groups was a live row pointing at an account that had been rewritten
* into an admissions-only applicant (#4202/#4203) — it could not reach
* /app/ticketing at all, and the Agents tab was still telling site admins
* to go and ask them (#4806).
*/
private function canStillAdminister(?User $user): bool
{
if ($user === null || !($user->is_active ?? true)) {
return false;
} }
return $this->isSiteAdminUser($user);
}
/**
* The groups nobody is left to run — every `manager` row on them points at
* somebody canStillAdminister() says no to (or there is no manager row at
* all).
*
* This is the ONLY condition under which a site admin may write an agent
* row for a group they do not manage. It is deliberately a property of the
* GROUP, not of the caller: an orphaned group cannot be rescued by anyone
* else, and a group with a working manager is still none of an admin's
* business (#4710).
*
* `$onlyGroupIds` narrows both queries for the single-group gate; `null`
* means "every group", which is what the page's prop needs.
*
* @param int[]|null $onlyGroupIds
* @return int[]
*/
private function orphanedGroupIds(?array $onlyGroupIds = null): array
{
$groupIds = TicketingGroup::when($onlyGroupIds !== null,
fn($q) => $q->whereIn('id', $onlyGroupIds))
->pluck('id');
if ($groupIds->isEmpty()) {
return [];
}
$managerRows = TicketingAgentAccess::where('role', 'manager')
->whereIn('group_id', $groupIds)
->get(['group_id', 'user_id']);
// One can() per distinct manager, not per row: the dev case is one
// person holding the manager row on all fifteen groups.
$managerIds = $managerRows->pluck('user_id')->unique();
$actingIds = $managerIds->isEmpty()
? collect()
: User::whereIn('id', $managerIds)->get()
->filter(fn($u) => $this->canStillAdminister($u))
->pluck('id');
$covered = $managerRows
->filter(fn($r) => $actingIds->contains($r->user_id))
->pluck('group_id')
->unique();
return $groupIds->reject(fn($id) => $covered->contains($id))->values()->all();
}
/**
* Manager rights over one group, OR a site admin rescuing an orphaned one.
*
* The second arm is #4806 and nothing wider: the moment a group's last
* usable manager leaves, is deactivated or is diverted elsewhere, every
* write in this file 403s for everybody — the group cannot be renamed,
* archived, given a project or given a single agent, and its tickets can
* never be assigned or closed. The documented answer on the Agents tab was
* "ask a manager of the group", which on such a group names nobody.
*
* So: a site admin may appoint somebody to a group that has no manager
* left. That one write un-orphans the group, and from the next request on
* the ordinary requireManagerAccess() rule governs it again — including
* for the admin, who gets no standing rights over it unless they appointed
* themselves.
*/
private function requireManagerAccessOrOrphanRecovery(int $groupId): void
{
if ($this->isManagerOf($groupId)) {
$this->settleRescueStamp($groupId);
return;
}
if ($this->isSiteAdmin() && $this->orphanedGroupIds([$groupId]) !== []) {
return;
}
abort(403);
} }
public function index(?string $tab = null): Response public function index(?string $tab = null): Response
@@ -71,14 +247,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 +288,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();
@@ -96,19 +299,68 @@ class TicketingSettingsController extends Controller
$agents->each(fn($a) => $a->user = $agentUsers[$a->user_id] ?? null); $agents->each(fn($a) => $a->user = $agentUsers[$a->user_id] ?? null);
} }
// #4806 — the groups with no manager left who could still use this
// page. Computed once: the page names them in the add form, and #5002
// asks the same question again for every Remove button below.
$orphanedGroupIds = $isSiteAdmin && !$isBootstrap ? $this->orphanedGroupIds() : [];
// #5002 — requireRemoveAccess() in prop form, the way myManagedGroupIds
// is requireManagerAccess()'s. It cannot be `canManageGroupId(row.group)`
// any more: a rescuer may also remove a row on a group that is still
// orphaned, and the row their own rescue wrote even after that rescue
// un-orphaned the group. A row an administrator can create has to be a
// row they can see the Remove on — the list said "Not yours to manage"
// over a row the reader had written seconds earlier.
$orphanedSet = array_map('intval', $orphanedGroupIds);
$removableAgentIds = $agents
->filter(fn ($a) => $myManagedGroupIds->contains($a->group_id)
|| ($isSiteAdmin && (in_array((int) $a->group_id, $orphanedSet, true)
|| ($a->recovered_by !== null && (int) $a->recovered_by === (int) $userId))))
->pluck('id')
->values()
->all();
$priorities = $isBootstrap $priorities = $isBootstrap
? collect() ? collect()
: PriorityLevel::orderBy('sort_order')->get(); : PriorityLevel::orderBy('sort_order')->get();
$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 // #6168 — the Email tab's rows. Shaped by hand rather than handed over
// as models for one reason: `config` holds the mailbox PASSWORD, and
// this prop ships on every settings load, so a credential left in it is
// a credential in the page source of everyone who can open Help Desk
// Settings. The form asks for a new password instead and
// updateEmailConnection() keeps the stored one when the box is blank,
// so nothing needs it on the client — `has_password` is all the UI has
// to know to say "a password is set".
$emailConnections = ($isBootstrap
? collect() ? collect()
: EmailConnection::whereIn('group_id', $myGroupIds)->get(); : EmailConnection::when($visibleGroupIds !== null,
fn($q) => $q->whereIn('group_id', $visibleGroupIds))
->orderBy('id')
->get())
->map(function (EmailConnection $connection) {
$config = $connection->config ?? [];
$hasPassword = ($config['password'] ?? '') !== '';
unset($config['password']);
return [
'id' => $connection->id,
'group_id' => $connection->group_id,
'type' => $connection->type,
'active' => (bool) $connection->active,
'config' => $config,
'has_password' => $hasPassword,
'last_polled_at' => $connection->last_polled_at?->toIso8601String(),
];
})
->values();
return Inertia::render('Ticketing/Settings', [ return Inertia::render('Ticketing/Settings', [
'activeTab' => $tab, 'activeTab' => $tab,
@@ -119,8 +371,18 @@ 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,
// #4806 — groups with no manager left who could still use this
// page. A site admin may add an agent to these and nothing else,
// so the Agents tab can offer the recovery instead of printing
// "ask a manager of the group" about a group that has none.
// Empty for a non-admin: the recovery is not theirs to make.
'orphanedGroupIds' => $orphanedGroupIds,
// #5002 — the access rows this user may actually remove. Superset
// of "rows on a group I manage": see requireRemoveAccess().
'removableAgentIds' => $removableAgentIds,
'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
@@ -293,6 +555,71 @@ class TicketingSettingsController extends Controller
return $request->expectsJson(); return $request->expectsJson();
} }
/**
* WHO AM I APPOINTING — the Agents tab's people search ([central-ticketing]
* #4926).
*
* Until this existed the first field of the add-an-agent form was
* `<input type="number">` labelled "User ID", with "Find IDs in Admin →
* Users" under it. That is a primary key asked of an office administrator,
* and on the ONE form the product calls "the only recovery" for a group
* with no manager left (#4806): the workflow was to leave the page, search
* a name somewhere else, read a number off a URL and type it back in.
*
* The cost is not only the detour. `storeAgent()` validates
* `exists:users,id`, so a NON-existent id is refused cleanly — but 47 typed
* for 74 is two real accounts, and the one that gets written is handed
* `manager` over a whole help desk queue: every ticket in it, plus the
* power to rename, archive and staff it. Nothing on that screen named the
* person before the press or after it, so #4781's audit row was the first
* place anybody could have found out, after the fact.
*
* So this is the same search every other Central picker is: `q` over name
* and email, answered by `PersonChip::rows()` — the one serializer for a
* searchable person (#871) — so the operator's People setting reaches this
* picker like the rest, and the page draws `<PersonChip>` rather than a
* hand-written name row.
*
* ── THE GATE IS THE PAGE'S, NOT THE WRITE'S ─────────────────────────────
*
* requireAgentAccess(), the same one index() uses, and deliberately not a
* per-group one: the form picks the PERSON before the GROUP, so there is no
* group to scope to yet, and narrowing it afterwards would mean a search
* that empties when the picker's second field changes. Nothing is widened
* by that — the write itself still goes through
* requireManagerAccessOrOrphanRecovery(), so a person found here can only
* be appointed to a group the caller manages or is rescuing. Reaching this
* route at all already means `permission:ticketing.settings`, which is a
* narrower population than TicketController::searchUsers() (any agent on
* any one ticket's group) already answers the same question for.
*/
public function searchUsers(Request $request)
{
$this->requireAgentAccess();
$q = trim((string) $request->input('q', ''));
// Two characters, like every other picker on the platform: one letter
// is most of the directory, and the dropdown is ten rows. Counted in
// CHARACTERS, like admin.settings.users.search, not bytes: an accented
// letter is two bytes and a CJK surname is three, so a byte floor lets
// a single keystroke answer with the directory on exactly the names a
// school has the most trouble typing.
if (mb_strlen($q) < 2) {
return response()->json([]);
}
return response()->json(PersonChip::rows(
User::where(function ($query) use ($q) {
$query->where('name', 'like', "%{$q}%")
->orWhere('email', 'like', "%{$q}%");
})
->tap(fn ($query) => PersonNames::orderUsers($query))
->limit(10)
->get(PersonChip::columns())
));
}
public function storeAgent(Request $request) public function storeAgent(Request $request)
{ {
$this->requireAgentAccess(); $this->requireAgentAccess();
@@ -305,25 +632,152 @@ class TicketingSettingsController extends Controller
'role' => 'required|in:agent,manager', 'role' => 'required|in:agent,manager',
]); ]);
$this->requireManagerAccess($validated['group_id']); // #4806 — the one way back into a group whose last manager can no
// longer act. Asked BEFORE the write, because the write is what ends
// the condition: appointing yourself makes isManagerOf() true.
$isRecovery = !$this->isManagerOf($validated['group_id'])
&& $this->orphanedGroupIds([$validated['group_id']]) !== [];
$this->requireManagerAccessOrOrphanRecovery($validated['group_id']);
TicketingAgentAccess::updateOrCreate( TicketingAgentAccess::updateOrCreate(
['user_id' => $validated['user_id'], 'group_id' => $validated['group_id']], ['user_id' => $validated['user_id'], 'group_id' => $validated['group_id']],
['role' => $validated['role']] [
'role' => $validated['role'],
// WHO RESCUED THIS GROUP — #5002. A recovery write is the one
// write here that can lock its own author out: appoint a
// `manager` and the group is no longer orphaned, so the arm
// that allowed the press is shut by the press. Stamping the
// row is what lets destroyAgent() hand it back.
//
// Set to null on an ordinary manager's write, including a
// manager editing a row an earlier rescue wrote: once the
// group has somebody of its own touching the row, it is
// theirs and the rescuer has no further claim on it.
'recovered_by' => $isRecovery ? Auth::id() : null,
]
); );
return back()->with('success', 'Agent added.'); /*
* NAME THE PERSON — [central-ticketing] #4926.
*
* The flash already went to the trouble of naming the GROUP and saying
* it had nobody; naming the appointee is the other half of the same
* sentence, and it is the half somebody can check. This write hands a
* person read access to every ticket in a queue — and, as a manager,
* the power to rename, archive and staff it — so "Agent added." left
* the only record of WHO in #4781's audit log, which is somewhere you
* go after you already suspect something.
*
* `PersonNames::display()` and not `$user->name`: the confirmation has
* to read the way the rest of the platform writes a name (#524), and
* the chip the admin picked from in the form is composed the same way,
* so the two agree. A user who vanished between the `exists` rule and
* this line leaves `null` and the sentence falls back to exactly what
* it printed before — a flash is not where a race should surface.
*/
$person = User::find($validated['user_id']);
$who = $person === null ? null : PersonNames::display($person);
$asWhat = $validated['role'] === 'manager' ? 'a manager' : 'an agent';
$name = TicketingGroup::find($validated['group_id'])?->name;
// Say which group was rescued and that it had nobody: a recovery is a
// rare, consequential click and "Agent added." does not describe it.
if ($isRecovery) {
$added = $who === null ? 'Agent added' : "{$who} added as {$asWhat}";
return back()->with('success', $name === null
? "{$added} — that group had no manager who could still act."
: "{$added} to “{$name}”, which had no manager who could still act.");
}
if ($who === null) {
return back()->with('success', 'Agent added.');
}
return back()->with('success', $name === null
? "{$who} added as {$asWhat}."
: "{$who} added to “{$name}” as {$asWhat}.");
} }
/**
* REMOVE IS THE SAME DOOR AS ADD — [central-ticketing] #5002.
*
* This was `requireManagerAccess()` while `storeAgent()` was
* requireManagerAccessOrOrphanRecovery(), so the recovery was ONE-WAY: the
* site admin who rescued a group could appoint and never un-appoint. The
* row they had just written appeared in their own Agents list marked "Not
* yours to manage", with no Remove control, and the DELETE answered 403 —
* the same person, the same second, the same row.
*
* Worse for a `manager` press, which is the one the card about the people
* search (#4926) called out as the expensive mistake: the write ends the
* orphan state, so the admin could not remove the row, could not appoint
* anyone else, and could not appoint themselves. The only person left who
* could undo it was the person wrongly appointed. That is #4806's frozen
* group again, reached through #4806's own cure.
*
* So two arms beyond an ordinary manager, both narrower than the write
* they undo:
*/
public function destroyAgent(TicketingAgentAccess $access) public function destroyAgent(TicketingAgentAccess $access)
{ {
$this->requireManagerAccess($access->group_id); $this->requireRemoveAccess($access);
$access->delete(); $access->delete();
return back()->with('success', 'Agent removed.'); return back()->with('success', 'Agent removed.');
} }
/**
* May the signed-in user take this agent access row away?
*
* 1. A manager of the group — the ordinary rule, unchanged.
* 2. A site admin, while the group is STILL orphaned. Exactly symmetric
* with storeAgent()'s arm and strictly narrower than it: on a group
* with no manager left, an admin can already appoint themselves
* manager and then remove anything, so this grants nothing new — it
* just does not make them take the group over first to clear a
* dangling row.
* 3. A site admin undoing a row THEY wrote as a rescue (`recovered_by`).
* This is the one that reopens the door a mistaken `manager` press
* closed, and it is deliberately not "the group would be orphaned
* without this row": that test cannot tell a mistaken appointment
* from the sole working manager of a healthy group, and admitting the
* second would let any site admin strip a manager from a group that
* is none of their business, then appoint themselves (#4710). A
* rescuer taking back their own appointment claims nothing they did
* not already hold at the moment they made it.
*
* And it lasts only until the appointee USES the access — see
* settleRescueStamp(). Otherwise the undo outlives the mistake it is
* for: a rescue that WORKED leaves the stamp on the row of the person
* now running the desk, and arm 3 becomes a permanent lever for
* stripping them and taking the group over (#5008).
*
* Removing the last manager re-orphans the group, which is the correct
* end state: it is recoverable again, by whoever gets there next.
*/
private function requireRemoveAccess(TicketingAgentAccess $access): void
{
if ($this->isManagerOf($access->group_id)) {
$this->settleRescueStamp($access->group_id);
return;
}
if ($this->isSiteAdmin()) {
if ($this->orphanedGroupIds([$access->group_id]) !== []) {
return;
}
if ($access->recovered_by !== null && (int) $access->recovered_by === (int) Auth::id()) {
return;
}
}
abort(403);
}
public function storePriority(Request $request) public function storePriority(Request $request)
{ {
if (!$this->isSiteAdmin()) { if (!$this->isSiteAdmin()) {
@@ -334,7 +788,10 @@ class TicketingSettingsController extends Controller
'name' => 'required|string|max:100', 'name' => 'required|string|max:100',
'color' => 'required|string|regex:/^#[0-9a-fA-F]{6}$/', 'color' => 'required|string|regex:/^#[0-9a-fA-F]{6}$/',
'description' => 'nullable|string', 'description' => 'nullable|string',
'sort_order' => 'integer|min:0', // unsignedSmallInteger column (0..65535): without the ceiling an
// oversized client value overflows on INSERT as an unhandled 500.
// Same rule #6081 put on storeStatus()/updateStatus().
'sort_order' => 'integer|min:0|max:65535',
]); ]);
PriorityLevel::create($validated); PriorityLevel::create($validated);
@@ -352,7 +809,7 @@ class TicketingSettingsController extends Controller
'name' => 'required|string|max:100', 'name' => 'required|string|max:100',
'color' => 'required|string|regex:/^#[0-9a-fA-F]{6}$/', 'color' => 'required|string|regex:/^#[0-9a-fA-F]{6}$/',
'description' => 'nullable|string', 'description' => 'nullable|string',
'sort_order' => 'required|integer|min:0', 'sort_order' => 'required|integer|min:0|max:65535',
]); ]);
$priority->update($validated); $priority->update($validated);
@@ -481,12 +938,43 @@ class TicketingSettingsController extends Controller
$connection->update([ $connection->update([
'type' => $request->type, 'type' => $request->type,
'active' => $request->boolean('active', true), 'active' => $request->boolean('active', true),
'config' => $request->input('config', $connection->config), 'config' => $this->mergedConnectionConfig($request, $connection),
]); ]);
return back()->with('success', 'Email connection updated.'); return back()->with('success', 'Email connection updated.');
} }
/**
* The config to store for an edited connection.
*
* #6168 — the page is never sent the stored password (see index()), so the
* edit form's password box arrives EMPTY on every save that does not change
* it. Taking the request's config as-is would therefore erase the mailbox
* credential every time somebody renamed a host or flipped the Active
* checkbox. A blank box means "keep the one you have"; typing a new one
* replaces it, and the rest of the config is the request's, so a field
* cleared on purpose still clears.
*/
private function mergedConnectionConfig(Request $request, EmailConnection $connection): array
{
$config = $request->input('config');
$existing = $connection->config ?? [];
if (! is_array($config)) {
return $existing;
}
if (($config['password'] ?? '') === '') {
unset($config['password']);
if (($existing['password'] ?? null) !== null) {
$config['password'] = $existing['password'];
}
}
return $config;
}
public function destroyEmailConnection(EmailConnection $connection) public function destroyEmailConnection(EmailConnection $connection)
{ {
$this->requireAgentAccess(); $this->requireAgentAccess();
@@ -504,7 +992,7 @@ class TicketingSettingsController extends Controller
$validated = $request->validate([ $validated = $request->validate([
'name' => 'required|string|max:100', 'name' => 'required|string|max:100',
'color' => 'required|string|regex:/^#[0-9a-fA-F]{6}$/', 'color' => 'required|string|regex:/^#[0-9a-fA-F]{6}$/',
'sort_order' => 'required|integer|min:0', 'sort_order' => 'required|integer|min:0|max:65535',
'is_closed' => 'boolean', 'is_closed' => 'boolean',
]); ]);
@@ -535,7 +1023,7 @@ class TicketingSettingsController extends Controller
$validated = $request->validate([ $validated = $request->validate([
'name' => 'required|string|max:100', 'name' => 'required|string|max:100',
'color' => 'required|string|regex:/^#[0-9a-fA-F]{6}$/', 'color' => 'required|string|regex:/^#[0-9a-fA-F]{6}$/',
'sort_order' => 'required|integer|min:0', 'sort_order' => 'required|integer|min:0|max:65535',
'is_closed' => 'boolean', 'is_closed' => 'boolean',
]); ]);

View File

@@ -9,7 +9,13 @@ class TicketingAgentAccess extends Model
{ {
protected $table = 'ticketing_agent_access'; protected $table = 'ticketing_agent_access';
protected $fillable = ['user_id', 'group_id', 'role']; /**
* `recovered_by` is #5002: the site admin who wrote this row as an orphan
* recovery (#4806), and may therefore take it back. Null on every row
* written by an ordinary manager — see the migration for why this is not
* a general-purpose `created_by`.
*/
protected $fillable = ['user_id', 'group_id', 'role', 'recovered_by'];
public function group(): BelongsTo public function group(): BelongsTo
{ {

View File

@@ -84,17 +84,22 @@ class MergedTicketRepair
*/ */
public static function repairAll(string $by): int 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."); Log::info("[ticketing] #938 merged-ticket repair ({$by}): nothing to repair.");
return 0; return 0;
} }
$changed = Ticket::query()->inconsistentlyMerged()->update([ // Per model, not one builder update (#4781/#4807): `merged` makes a ticket
'status' => Ticket::STATUS_MERGED, // 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, // The numbers, not just the count. These rows are being made read-only,
// and "which ones" is the question the office will ask afterwards. // and "which ones" is the question the office will ask afterwards.

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.
}
} }
} }

View File

@@ -41,12 +41,25 @@ Route::middleware(['web', 'auth', 'app.access:ticketing'])
// ── Settings (must be before /{ticket} wildcard) ────────────────────── // ── Settings (must be before /{ticket} wildcard) ──────────────────────
Route::middleware('permission:ticketing.settings')->group(function () { Route::middleware('permission:ticketing.settings')->group(function () {
Route::get('/settings', [TicketingSettingsController::class, 'index']) ->name('settings'); Route::get('/settings', [TicketingSettingsController::class, 'index']) ->name('settings');
// #6168 — `email` belongs in this list. It was the one tab segment
// the whitelist omitted while the page shipped an `emailConnections`
// prop on every render and the three write routes below were live, so
// the only deep link an admin could guess fell through to the POST-only
// `/settings/email` and answered 405. The Vue's TAB_KEYS and this list
// are now pinned to each other by
// tests/Unit/TicketingSettingsTabsAreOneVocabularyTest.php.
Route::get('/settings/{tab}', [TicketingSettingsController::class, 'index']) Route::get('/settings/{tab}', [TicketingSettingsController::class, 'index'])
->where('tab', 'groups|agents|priorities|statuses|projects|automation')->name('settings.tab'); ->where('tab', 'groups|agents|priorities|statuses|projects|email|automation')->name('settings.tab');
Route::post('/settings/groups', [TicketingSettingsController::class, 'storeGroup']) ->name('settings.groups.store'); Route::post('/settings/groups', [TicketingSettingsController::class, 'storeGroup']) ->name('settings.groups.store');
Route::put('/settings/groups/{group}', [TicketingSettingsController::class, 'updateGroup']) ->name('settings.groups.update'); Route::put('/settings/groups/{group}', [TicketingSettingsController::class, 'updateGroup']) ->name('settings.groups.update');
Route::delete('/settings/groups/{group}', [TicketingSettingsController::class, 'destroyGroup']) ->name('settings.groups.destroy'); Route::delete('/settings/groups/{group}', [TicketingSettingsController::class, 'destroyGroup']) ->name('settings.groups.destroy');
Route::post('/settings/groups/{group}/restore', [TicketingSettingsController::class, 'restoreGroup']) ->name('settings.groups.restore'); Route::post('/settings/groups/{group}/restore', [TicketingSettingsController::class, 'restoreGroup']) ->name('settings.groups.restore');
// #4926 — the add-an-agent form's people search. Before it the form's
// first field was a raw numeric user id with "Find IDs in Admin → Users"
// under it, on the one screen that is the only recovery for a help desk
// group with no manager left (#4806). Two segments, so it cannot collide
// with the single-segment `/settings/{tab}` above.
Route::get('/settings/agents/search', [TicketingSettingsController::class, 'searchUsers']) ->name('settings.agents.search');
Route::post('/settings/agents', [TicketingSettingsController::class, 'storeAgent']) ->name('settings.agents.store'); Route::post('/settings/agents', [TicketingSettingsController::class, 'storeAgent']) ->name('settings.agents.store');
Route::delete('/settings/agents/{access}', [TicketingSettingsController::class, 'destroyAgent']) ->name('settings.agents.destroy'); Route::delete('/settings/agents/{access}', [TicketingSettingsController::class, 'destroyAgent']) ->name('settings.agents.destroy');
Route::post('/settings/priorities', [TicketingSettingsController::class, 'storePriority']) ->name('settings.priorities.store'); Route::post('/settings/priorities', [TicketingSettingsController::class, 'storePriority']) ->name('settings.priorities.store');