11 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
5 changed files with 538 additions and 23 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

@@ -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'));
} }
/** /**
@@ -62,16 +78,167 @@ class TicketingSettingsController extends Controller
* visibility bug, not a licence to hand every `ticketing.settings` holder * 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 * 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. * 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
@@ -132,6 +299,27 @@ 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();
@@ -143,11 +331,36 @@ class TicketingSettingsController extends Controller
->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::when($visibleGroupIds !== null, : EmailConnection::when($visibleGroupIds !== null,
fn($q) => $q->whereIn('group_id', $visibleGroupIds)) fn($q) => $q->whereIn('group_id', $visibleGroupIds))
->get(); ->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,
@@ -159,6 +372,15 @@ class TicketingSettingsController extends Controller
'emailConnections' => $emailConnections, 'emailConnections' => $emailConnections,
'myGroupIds' => $myGroupIds, 'myGroupIds' => $myGroupIds,
'myManagedGroupIds' => $myManagedGroupIds, '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' => $isSiteAdmin, 'isSiteAdmin' => $isSiteAdmin,
'autoCloseDays' => (int) Setting::get('ticketing.auto_close_days', 0), 'autoCloseDays' => (int) Setting::get('ticketing.auto_close_days', 0),
@@ -333,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();
@@ -345,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()) {
@@ -374,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);
@@ -392,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);
@@ -521,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();
@@ -544,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',
]); ]);
@@ -575,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

@@ -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');