Ticketing groups can be retired (e2e dashboard-ticketing#7)
There was no destroy route at all — only settings.groups.store and .update — so
the e2e CRUD spec, which creates an ephemeral group as its documented setup step,
could never clean up (a DELETE-spoofed POST returned 405). Every sweep left litter
behind, and an operator who mistyped a group was stuck with it forever.
Deleting a group cannot be unconditional: tickets are the audit record of real
correspondence, and tickets.group_id is a RESTRICT FK. So destroyGroup counts the
tickets and picks:
0 tickets -> real delete (agent access / projects / email connections are
cascadeOnDelete, so nothing dangles). This is the teardown and
undo-a-typo path.
>=1 ticket -> archived_at = now(). Tickets untouched and still readable.
The caller is TOLD which happened rather than inferring it from a 302: an
expectsJson caller gets {"outcome":"deleted"|"archived","ticket_count":N}, the UI
gets distinct flash wording, and the confirm modal says which it will be BEFORE
you click ("This group has 1 ticket and will be archived, not deleted").
Archiving is idempotent; restore is a separate route.
Archived groups leave the PICKERS but nothing else: index() and create() use
scopeActive, store() validation adds whereNull('archived_at') so a stale id cannot
bypass the picker, and update() rejects transfers INTO an archived group while
always allowing the ticket's own group — otherwise a ticket stranded in an
archived group would become uneditable. Index.vue re-adds that own group to the
transfer select labelled "(archived)" so the select is never blank. Deliberately
NOT filtered: the Settings groups list (archived rows show a badge + Restore), and
agentGroupIds()/isAgent()/isManager(), which read ticketing_agent_access — so
existing agents keep working the tickets that remain. No global scope; exclusion
is per call site.
Gating is byte-identical to updateGroup: inside the existing
permission:ticketing.settings group plus requireManagerAccess(), so a super admin
with no manager row on that group is still refused (asserted).
Migration is additive — one nullable timestamp. down() is a deliberate NO-OP,
matching the myedpuller-sync warnings-column precedent: DashboardDoctor greps the
whole file for drop/rename patterns, so a dropColumn even in down() would raise a
"pending destructive migration" advisory on every host where it is still pending,
which is a confusing warning for a purely additive change. Verified against
doctor's exact regex, not a looser grep.
Tests: 9 (69 assertions). 10 mutations, all 10 detected — including forcing a hard
delete on a group with tickets, which throws a QueryException from the RESTRICT FK,
i.e. the database backs up the application check.
Browser-verified on dev: the confirm modal wording for both cases, the ARCHIVED
badge and Restore, the archived group vanishing from both the create picker and the
All-Tickets switcher while its ticket stays listed, and ZERO native
confirm/alert dialogs. doctor exit 0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -17,6 +17,7 @@ use Illuminate\Http\Request;
|
||||
use Illuminate\Routing\Controller;
|
||||
use Illuminate\Support\Facades\Auth;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Validation\Rule;
|
||||
use Inertia\Inertia;
|
||||
use Inertia\Response;
|
||||
|
||||
@@ -172,7 +173,14 @@ class TicketController extends Controller
|
||||
return $ticket;
|
||||
});
|
||||
|
||||
$groups = TicketingGroup::when($isAgent, fn ($q) => $q->whereIn('id', $agentGroupIds))->get();
|
||||
// Picker data: the group switcher/filter and the detail pane's transfer
|
||||
// select both read this, so archived groups are excluded. The tickets
|
||||
// themselves are NOT filtered — an archived group's tickets stay in the
|
||||
// list and stay workable; Index.vue re-adds the current group to the
|
||||
// transfer options when it happens to be archived.
|
||||
$groups = TicketingGroup::active()
|
||||
->when($isAgent, fn ($q) => $q->whereIn('id', $agentGroupIds))
|
||||
->get();
|
||||
$priorities = PriorityLevel::orderBy('sort_order')->get();
|
||||
$projects = TicketingProject::when($isAgent, fn ($q) => $q->whereIn('group_id', $agentGroupIds))
|
||||
->where('status', 'active')
|
||||
@@ -269,7 +277,10 @@ class TicketController extends Controller
|
||||
$agentGroupIds = $this->agentGroupIds();
|
||||
$isAgent = count($agentGroupIds) > 0;
|
||||
|
||||
$groups = TicketingGroup::when($isAgent, fn($q) => $q->whereIn('id', $agentGroupIds))->get();
|
||||
// New tickets can only be filed against a live group.
|
||||
$groups = TicketingGroup::active()
|
||||
->when($isAgent, fn($q) => $q->whereIn('id', $agentGroupIds))
|
||||
->get();
|
||||
|
||||
// No groups at all? Show first-run state.
|
||||
if ($groups->isEmpty() && TicketingGroup::count() === 0) {
|
||||
@@ -296,7 +307,9 @@ class TicketController extends Controller
|
||||
$validated = $request->validate([
|
||||
'title' => 'required|string|max:255',
|
||||
'description' => 'required|string',
|
||||
'group_id' => 'required|exists:ticketing_groups,id',
|
||||
// Archived groups are gone from the picker; they must also be
|
||||
// unreachable by posting a stale id.
|
||||
'group_id' => ['required', Rule::exists('ticketing_groups', 'id')->whereNull('archived_at')],
|
||||
'priority_id' => 'nullable|exists:ticketing_priority_levels,id',
|
||||
'due_date' => 'nullable|date',
|
||||
'attachments' => 'nullable|array|max:5',
|
||||
@@ -442,7 +455,19 @@ class TicketController extends Controller
|
||||
'priority_id' => 'nullable|exists:ticketing_priority_levels,id',
|
||||
'due_date' => 'nullable|date',
|
||||
'project_id' => 'nullable|exists:ticketing_projects,id',
|
||||
'group_id' => 'nullable|exists:ticketing_groups,id',
|
||||
// A ticket may not be transferred INTO an archived group, but a
|
||||
// ticket already sitting in one must still be editable — so its own
|
||||
// group id always validates.
|
||||
'group_id' => ['nullable', 'exists:ticketing_groups,id',
|
||||
function ($attribute, $value, $fail) use ($ticket) {
|
||||
if (! $value || (int) $value === (int) $ticket->group_id) {
|
||||
return;
|
||||
}
|
||||
if (TicketingGroup::whereKey($value)->archived()->exists()) {
|
||||
$fail('That group is archived — choose an active group.');
|
||||
}
|
||||
},
|
||||
],
|
||||
'assigned_to' => 'nullable|in:' . implode(',', $validAssigneeIds ?: [0]),
|
||||
];
|
||||
|
||||
|
||||
@@ -72,9 +72,18 @@ class TicketingSettingsController extends Controller
|
||||
$isBootstrap = $this->isBootstrapState();
|
||||
$myGroupIds = TicketingAgentAccess::where('user_id', $userId)->pluck('group_id');
|
||||
|
||||
// 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.
|
||||
// Actives first, then archived, each alphabetical.
|
||||
$groups = $isBootstrap
|
||||
? collect()
|
||||
: TicketingGroup::whereIn('id', $myGroupIds)->get();
|
||||
: TicketingGroup::whereIn('id', $myGroupIds)
|
||||
// tickets_count decides archive-vs-delete, so the UI can warn
|
||||
// the operator which one their click is about to do.
|
||||
->withCount('tickets')
|
||||
->orderByRaw('archived_at IS NOT NULL')
|
||||
->orderBy('name')
|
||||
->get();
|
||||
|
||||
$agents = $isBootstrap
|
||||
? collect()
|
||||
@@ -180,6 +189,104 @@ class TicketingSettingsController extends Controller
|
||||
return back()->with('success', 'Group updated.');
|
||||
}
|
||||
|
||||
/**
|
||||
* Retire a group — hard delete when it is empty, archive when it is not.
|
||||
*
|
||||
* Tickets are the audit record of real correspondence, so a group that owns
|
||||
* any ticket can never be deleted: `tickets.group_id` is a RESTRICT foreign
|
||||
* key, which is the backstop, and this method is the polite front door. It
|
||||
* archives instead, leaving every ticket byte-identical and readable.
|
||||
*
|
||||
* An empty group has nothing to protect, so it really is deleted — which is
|
||||
* what lets an operator undo a typo and lets the e2e CRUD spec clean up the
|
||||
* throwaway group it creates as its setup step (issue #7).
|
||||
*
|
||||
* The caller is told which of the two happened, explicitly: an `outcome` of
|
||||
* `deleted` or `archived` in the JSON body for API callers, and distinct
|
||||
* flash wording for the Inertia UI. Nobody has to guess from a 302.
|
||||
*/
|
||||
public function destroyGroup(Request $request, TicketingGroup $group)
|
||||
{
|
||||
$this->requireManagerAccess($group->id);
|
||||
|
||||
$ticketCount = $group->tickets()->count();
|
||||
|
||||
if ($ticketCount > 0) {
|
||||
// Idempotent: archiving an already-archived group is not an error,
|
||||
// it just re-reports the same outcome.
|
||||
if ($group->archived_at === null) {
|
||||
$group->archived_at = now();
|
||||
$group->save();
|
||||
}
|
||||
|
||||
return $this->groupRetirementResponse($request, 'archived', $group, $ticketCount);
|
||||
}
|
||||
|
||||
// Empty group: agent access, projects and email connections all cascade
|
||||
// on delete (see their migrations), so nothing is left dangling.
|
||||
$group->delete();
|
||||
|
||||
return $this->groupRetirementResponse($request, 'deleted', $group, 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Un-archive a group so it can take new tickets again.
|
||||
*/
|
||||
public function restoreGroup(Request $request, TicketingGroup $group)
|
||||
{
|
||||
$this->requireManagerAccess($group->id);
|
||||
|
||||
if ($group->archived_at !== null) {
|
||||
$group->archived_at = null;
|
||||
$group->save();
|
||||
}
|
||||
|
||||
if ($this->wantsJsonBody($request)) {
|
||||
return response()->json([
|
||||
'outcome' => 'restored',
|
||||
'group_id' => $group->id,
|
||||
]);
|
||||
}
|
||||
|
||||
return back()->with('success', "Group “{$group->name}” restored.");
|
||||
}
|
||||
|
||||
/**
|
||||
* One shape for "what happened to that group", for both kinds of caller.
|
||||
*/
|
||||
private function groupRetirementResponse(Request $request, string $outcome, TicketingGroup $group, int $ticketCount)
|
||||
{
|
||||
if ($this->wantsJsonBody($request)) {
|
||||
return response()->json([
|
||||
'outcome' => $outcome,
|
||||
'group_id' => $group->id,
|
||||
'ticket_count' => $ticketCount,
|
||||
]);
|
||||
}
|
||||
|
||||
$message = $outcome === 'deleted'
|
||||
? "Group “{$group->name}” deleted — it had no tickets."
|
||||
: "Group “{$group->name}” archived — it has {$ticketCount} ticket"
|
||||
. ($ticketCount === 1 ? '' : 's') . ', which stay readable.';
|
||||
|
||||
return back()->with('success', $message);
|
||||
}
|
||||
|
||||
/**
|
||||
* True for an API/script caller that wants a JSON body.
|
||||
*
|
||||
* Inertia sends `Accept: text/html` but also XHR headers, so
|
||||
* expectsJson() alone would hijack the UI's redirect-back flow.
|
||||
*/
|
||||
private function wantsJsonBody(Request $request): bool
|
||||
{
|
||||
if ($request->header('X-Inertia')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $request->expectsJson();
|
||||
}
|
||||
|
||||
public function storeAgent(Request $request)
|
||||
{
|
||||
$this->requireAgentAccess();
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
namespace Dashboard\Ticketing\Models;
|
||||
|
||||
use Illuminate\Database\Eloquent\Builder;
|
||||
use Illuminate\Database\Eloquent\Model;
|
||||
use Illuminate\Database\Eloquent\Relations\HasMany;
|
||||
|
||||
@@ -9,6 +10,36 @@ class TicketingGroup extends Model
|
||||
{
|
||||
protected $fillable = ['name', 'email_address', 'color', 'prefix'];
|
||||
|
||||
protected $casts = [
|
||||
'archived_at' => 'datetime',
|
||||
];
|
||||
|
||||
protected $appends = ['is_archived'];
|
||||
|
||||
/**
|
||||
* Archived groups are deliberately NOT hidden by a global scope.
|
||||
*
|
||||
* The tickets in an archived group are still live correspondence: their
|
||||
* agents must keep working them, `$ticket->group` must still resolve, and
|
||||
* Settings must still list the group so a manager can restore it. Only the
|
||||
* *pickers* — "which group should this new ticket go to", "filter by
|
||||
* group" — exclude archived ones, via this scope, one call site at a time.
|
||||
*/
|
||||
public function scopeActive(Builder $query): Builder
|
||||
{
|
||||
return $query->whereNull('archived_at');
|
||||
}
|
||||
|
||||
public function scopeArchived(Builder $query): Builder
|
||||
{
|
||||
return $query->whereNotNull('archived_at');
|
||||
}
|
||||
|
||||
public function getIsArchivedAttribute(): bool
|
||||
{
|
||||
return $this->archived_at !== null;
|
||||
}
|
||||
|
||||
public function tickets(): HasMany
|
||||
{
|
||||
return $this->hasMany(Ticket::class, 'group_id');
|
||||
|
||||
@@ -33,6 +33,8 @@ Route::middleware(['web', 'auth', 'app.access:ticketing'])
|
||||
->where('tab', 'groups|agents|priorities|statuses|projects|automation')->name('settings.tab');
|
||||
Route::post('/settings/groups', [TicketingSettingsController::class, 'storeGroup']) ->name('settings.groups.store');
|
||||
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::post('/settings/groups/{group}/restore', [TicketingSettingsController::class, 'restoreGroup']) ->name('settings.groups.restore');
|
||||
Route::post('/settings/agents', [TicketingSettingsController::class, 'storeAgent']) ->name('settings.agents.store');
|
||||
Route::delete('/settings/agents/{access}', [TicketingSettingsController::class, 'destroyAgent']) ->name('settings.agents.destroy');
|
||||
Route::post('/settings/priorities', [TicketingSettingsController::class, 'storePriority']) ->name('settings.priorities.store');
|
||||
|
||||
Reference in New Issue
Block a user