From 2266298b6c1341f894f347439650c9866c77dab3 Mon Sep 17 00:00:00 2001 From: Mark Gregory Date: Tue, 23 Jun 2026 11:05:48 +0100 Subject: [PATCH] pases 4 ticket updates --- .../Services/TicketReferenceService.php | 39 ++++- Modules/Helpdesk/Services/TicketService.php | 136 +++++++++++------- .../Tests/Unit/TicketReferenceServiceTest.php | 11 ++ Modules/Helpdesk/changelog.yaml | 11 ++ Modules/Helpdesk/module.json | 2 +- 5 files changed, 142 insertions(+), 57 deletions(-) diff --git a/Modules/Helpdesk/Services/TicketReferenceService.php b/Modules/Helpdesk/Services/TicketReferenceService.php index 499045ef..bc790c4d 100644 --- a/Modules/Helpdesk/Services/TicketReferenceService.php +++ b/Modules/Helpdesk/Services/TicketReferenceService.php @@ -1,27 +1,58 @@ format('Y'); $prefix = 'HD-'.$year.'-'; - $last = Ticket::query() + + $this->lockReferenceSeries($prefix); + + $lastReference = Ticket::query() ->where('reference', 'like', $prefix.'%') - ->latest('id') + ->orderByDesc('reference') ->value('reference'); - $next = $last ? ((int) substr((string) $last, -5)) + 1 : 1; + $next = $lastReference === null + ? 1 + : ((int) substr((string) $lastReference, -5)) + 1; return $prefix.str_pad((string) $next, 5, '0', STR_PAD_LEFT); } + + /** + * Lock the current ticket reference series for the active transaction. + */ + private function lockReferenceSeries(string $prefix): void + { + if (DB::connection()->getDriverName() !== 'pgsql') { + return; + } + + DB::select('select pg_advisory_xact_lock(hashtext(?))', [ + 'helpdesk-ticket-reference:'.$prefix, + ]); + } } diff --git a/Modules/Helpdesk/Services/TicketService.php b/Modules/Helpdesk/Services/TicketService.php index 5e2ea44b..e8f5c15b 100644 --- a/Modules/Helpdesk/Services/TicketService.php +++ b/Modules/Helpdesk/Services/TicketService.php @@ -6,6 +6,7 @@ namespace Modules\Helpdesk\Services; use App\Models\User; use Illuminate\Http\UploadedFile; +use Illuminate\Database\QueryException; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Schema; use Illuminate\Support\Str; @@ -34,67 +35,98 @@ final class TicketService */ public function create(User $user, array $data, array $files = []): Ticket { - return DB::transaction(function () use ($user, $data, $files): Ticket { - $defaultStatusSlug = (string) config('helpdesk.default_status_slug', 'new'); - $status = Schema::hasTable('helpdesk_statuses') && Schema::hasColumn('helpdesk_statuses', 'is_default') - ? Status::query()->where('is_default', true)->orderBy('sort_order')->first() - : null; - - if (! $status) { - $status = Status::query()->where('slug', $defaultStatusSlug)->first(); + for ($attempt = 1; $attempt <= 5; $attempt++) { + try { + return DB::transaction(fn (): Ticket => $this->createInsideTransaction($user, $data, $files)); + } catch (QueryException $exception) { + if (! $this->isReferenceCollision($exception) || $attempt === 5) { + throw $exception; + } } + } - if (! $status) { - $status = Status::query()->create([ - 'name' => 'New', - 'slug' => $defaultStatusSlug, - 'color' => 'sky', - 'sort_order' => 1, - 'is_closed' => false, - ]); - } + throw new \RuntimeException('Unable to create a unique Helpdesk ticket reference.'); + } - $boardId = $data['board_id'] ?? null; - if (blank($boardId) && Schema::hasTable('helpdesk_kanban_boards')) { - $boardId = KanbanBoard::query() - ->where('is_default', true) - ->orderBy('sort_order') - ->value('id') ?: KanbanBoard::query()->orderBy('sort_order')->value('id'); - } + /** + * Create the ticket while the caller owns the database transaction. + * + * @param array $data + * @param array $files + */ + private function createInsideTransaction(User $user, array $data, array $files = []): Ticket + { + $defaultStatusSlug = (string) config('helpdesk.default_status_slug', 'new'); + $status = Schema::hasTable('helpdesk_statuses') && Schema::hasColumn('helpdesk_statuses', 'is_default') + ? Status::query()->where('is_default', true)->orderBy('sort_order')->first() + : null; - /** @var Ticket $ticket */ - $ticket = Ticket::query()->create([ - 'reference' => $this->references->next(), - 'company_id' => $data['company_id'] ?? ($user->company_id ?? null), - 'user_id' => $user->id, - 'agent_id' => $this->assignment->selectAgentId($data['category_id'] ?? null), - 'status_id' => $status->id, - 'board_id' => $boardId, - 'priority_id' => $data['priority_id'], - 'category_id' => $data['category_id'] ?? null, - 'project_id' => $data['project_id'] ?? null, - 'product_id' => $data['product_id'] ?? null, - 'team_id' => $data['team_id'] ?? null, - 'subject' => $data['subject'], - 'body' => $data['body'], - 'due_at' => $data['due_at'] ?? null, - 'last_activity_at' => now(), + if (! $status) { + $status = Status::query()->where('slug', $defaultStatusSlug)->first(); + } + + if (! $status) { + $status = Status::query()->create([ + 'name' => 'New', + 'slug' => $defaultStatusSlug, + 'color' => 'sky', + 'sort_order' => 1, + 'is_closed' => false, ]); + } - if (! empty($data['tag_ids']) && is_array($data['tag_ids'])) { - $ticket->tags()->sync(array_values(array_unique(array_map('intval', $data['tag_ids'])))); - } + $boardId = $data['board_id'] ?? null; + if (blank($boardId) && Schema::hasTable('helpdesk_kanban_boards')) { + $boardId = KanbanBoard::query() + ->where('is_default', true) + ->orderBy('sort_order') + ->value('id') ?: KanbanBoard::query()->orderBy('sort_order')->value('id'); + } - $ticket = $this->automation->apply($ticket, $data); - $ticket = $this->sla->applyPolicy($ticket); - $this->checklists->applyTemplates($ticket); - $this->attachments->storeMany($ticket, $files, (int) $user->id); + /** @var Ticket $ticket */ + $ticket = Ticket::query()->create([ + 'reference' => $this->references->next(), + 'company_id' => $data['company_id'] ?? ($user->company_id ?? null), + 'user_id' => $user->id, + 'agent_id' => $this->assignment->selectAgentId($data['category_id'] ?? null), + 'status_id' => $status->id, + 'board_id' => $boardId, + 'priority_id' => $data['priority_id'], + 'category_id' => $data['category_id'] ?? null, + 'project_id' => $data['project_id'] ?? null, + 'product_id' => $data['product_id'] ?? null, + 'team_id' => $data['team_id'] ?? null, + 'subject' => $data['subject'], + 'body' => $data['body'], + 'due_at' => $data['due_at'] ?? null, + 'last_activity_at' => now(), + ]); - $ticket = $ticket->fresh(['status', 'priority', 'category', 'project', 'team', 'product', 'tags', 'attachments', 'checklistItems']) ?? $ticket; - $this->workflow->dispatch($ticket, 'ticket.created', $user); + if (! empty($data['tag_ids']) && is_array($data['tag_ids'])) { + $ticket->tags()->sync(array_values(array_unique(array_map('intval', $data['tag_ids'])))); + } - return $ticket; - }); + $ticket = $this->automation->apply($ticket, $data); + $ticket = $this->sla->applyPolicy($ticket); + $this->checklists->applyTemplates($ticket); + $this->attachments->storeMany($ticket, $files, (int) $user->id); + + $ticket = $ticket->fresh(['status', 'priority', 'category', 'project', 'team', 'product', 'tags', 'attachments', 'checklistItems']) ?? $ticket; + $this->workflow->dispatch($ticket, 'ticket.created', $user); + + return $ticket; + } + + /** + * Determine whether a database error was caused by the ticket reference unique index. + */ + private function isReferenceCollision(QueryException $exception): bool + { + $message = $exception->getMessage(); + + return str_contains($message, 'helpdesk_tickets_reference_unique') + || str_contains($message, 'Duplicate entry') && str_contains($message, 'reference') + || str_contains($message, 'UNIQUE constraint failed') && str_contains($message, 'helpdesk_tickets.reference'); } /** diff --git a/Modules/Helpdesk/Tests/Unit/TicketReferenceServiceTest.php b/Modules/Helpdesk/Tests/Unit/TicketReferenceServiceTest.php index 372921e3..e1e4b5c5 100644 --- a/Modules/Helpdesk/Tests/Unit/TicketReferenceServiceTest.php +++ b/Modules/Helpdesk/Tests/Unit/TicketReferenceServiceTest.php @@ -29,4 +29,15 @@ final class TicketReferenceServiceTest extends TestCase self::assertSame('HD-'.now()->format('Y').'-00042', $reference); } + public function test_it_uses_the_highest_reference_suffix_instead_of_latest_database_id(): void + { + $year = now()->format('Y'); + + $this->ticket(['reference' => 'HD-'.$year.'-00042']); + $this->ticket(['reference' => 'HD-'.$year.'-00002']); + + $reference = app(TicketReferenceService::class)->next(); + + self::assertSame('HD-'.$year.'-00043', $reference); + } } diff --git a/Modules/Helpdesk/changelog.yaml b/Modules/Helpdesk/changelog.yaml index 439d560b..1d079397 100644 --- a/Modules/Helpdesk/changelog.yaml +++ b/Modules/Helpdesk/changelog.yaml @@ -105,3 +105,14 @@ - Added input, keyup, paste, blur, and submit-capture synchronisation to prevent false "body field is required" validation failures. - Changed the hidden body field to live Livewire binding so the server receives the latest editor content without requiring a page reload. - Preserved append-only changelog history. + +- version: 1.9.40 + date: 2026-06-23 + type: bugfix + summary: Prevent duplicate Helpdesk ticket references during ticket creation. + changes: + - Updated Helpdesk ticket reference generation to use the highest existing yearly reference suffix instead of the latest database row id. + - Added a PostgreSQL transaction-level advisory lock around reference generation to reduce concurrent ticket creation collisions. + - Added retry handling around ticket creation so reference unique-index collisions do not produce a user-facing 500 error. + - Added regression coverage for out-of-order ticket references so lower references cannot be reused accidentally. + - Preserved append-only changelog history. diff --git a/Modules/Helpdesk/module.json b/Modules/Helpdesk/module.json index 86234d07..6af09ad3 100644 --- a/Modules/Helpdesk/module.json +++ b/Modules/Helpdesk/module.json @@ -22,5 +22,5 @@ "Modules\\Helpdesk\\Providers\\HelpdeskServiceProvider" ], "files": [], - "version": "1.9.39" + "version": "1.9.40" }