Compare commits
8 Commits
card/4781-
...
card/6168-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7e675e7beb | ||
|
|
e4d0622361 | ||
|
|
abd21f73f2 | ||
|
|
51073fba1e | ||
|
|
8b4f788e43 | ||
|
|
a977aedc95 | ||
| 10ef42d78c | |||
|
|
94fb6ac53c |
@@ -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');
|
||||||
|
});
|
||||||
|
}
|
||||||
|
};
|
||||||
@@ -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()) {
|
||||||
@@ -521,12 +935,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 +989,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 +1020,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',
|
||||||
]);
|
]);
|
||||||
|
|
||||||
|
|||||||
@@ -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
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -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');
|
||||||
|
|||||||
Reference in New Issue
Block a user