A system note is a ticket message source
Merge and Split both 500ed. The audit note each writes uses `source = 'system'`
and the column is `enum('web','email')`, so the insert threw inside the
transaction and nothing committed — which is why the operator saw no state change
at all rather than a half-done merge.
Widened to an enum rather than a varchar, and the reasoning is the same test that
made `tickets.status` a varchar in `2026_04_17_000002`: statuses became
operator-definable rows, so their domain is open. `source` is the opposite — all
three values are set by code, an operator can never add a fourth, and the enum is
what keeps a typo'd source loud instead of a silently unfilterable row.
`TicketMessage::system()` is now the only place a system note is written. Three
scattered literals is how this reopens.
A SECOND 500 ON THE SAME ACTION, unreported and one longer title away. `split()`
appended ' (split)' to a title the product validates at max:255, so splitting a
full-length title produced 263 characters and the identical hard failure. Bounded
at the write rather than widened, and the distinction is written down: widen when
the data is right and the column is wrong; bound when the column expresses a real
product rule. 255 is the product's own limit, so a 263-character title would be
un-editable forever — the edit form would bounce it every time.
And `split()` now honours its own comment. Line 682 has always said "exclude
system notes" while the code copied them; it never mattered because no system row
could exist. The moment the enum accepts them, splitting a merged ticket would
carry "Merged from ticket QAX-0005." onto a ticket that was never merged.
This commit is contained in:
@@ -0,0 +1,66 @@
|
||||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
/**
|
||||
* MERGE AND SPLIT WROTE A `source` THE COLUMN DID NOT ACCEPT. [central-ticketing] #415
|
||||
*
|
||||
* Both actions write an audit note into `ticket_messages` with `source = 'system'`,
|
||||
* and the column was created as `enum('web','email')`. Under strict mode the insert
|
||||
* throws `Data truncated for column 'source'` INSIDE the transaction, so nothing
|
||||
* commits: no merge, no split, no partial state, and a generic 500 page. Every
|
||||
* guard around them worked — a missing reference, a self-merge and an absent
|
||||
* `source_number` were all handled correctly — so only the happy path was broken,
|
||||
* which is why it survived.
|
||||
*
|
||||
* ── THE WHOLE DOMAIN, NOT THE ONE IN THE TRACE ──────────────────────────────
|
||||
*
|
||||
* The trace named `'system'`. Widening to accept only that would be a guess about
|
||||
* how many more there are, so every value written to this column anywhere in the
|
||||
* platform was enumerated first:
|
||||
*
|
||||
* 'web' TicketMessageController::store(), the demo seeder
|
||||
* 'email' the demo seeder (no inbound-mail ingester exists yet)
|
||||
* 'system' TicketController::merge() and ::split(), the three notes
|
||||
*
|
||||
* `split()` also copies `$msg->source` verbatim when duplicating a thread, which
|
||||
* introduces no fourth value. So the set is closed at three and this runs once.
|
||||
*
|
||||
* ── WHY AN ENUM AND NOT A VARCHAR ───────────────────────────────────────────
|
||||
*
|
||||
* `tickets.status` went the other way — `2026_04_17_000002` deliberately widened it
|
||||
* from an enum to `varchar(50)` — and the difference is the test worth writing down:
|
||||
* statuses became OPERATOR-DEFINABLE rows in `ticketing_statuses`, so no enum could
|
||||
* ever hold them. `source` is the opposite: all three values are set by code, an
|
||||
* operator has no way to add a fourth, and the enum is what makes a typo'd source a
|
||||
* loud failure rather than a silently unfilterable row.
|
||||
*
|
||||
* The write side is bounded too — `TicketMessage::system()` is now the only place a
|
||||
* system note is created, so the three scattered `'source' => 'system'` literals that
|
||||
* caused this cannot reappear one at a time.
|
||||
*/
|
||||
return new class extends Migration
|
||||
{
|
||||
public function up(): void
|
||||
{
|
||||
DB::statement(
|
||||
"ALTER TABLE ticket_messages
|
||||
MODIFY COLUMN source ENUM('web','email','system') NOT NULL DEFAULT 'web'"
|
||||
);
|
||||
}
|
||||
|
||||
public function down(): void
|
||||
{
|
||||
// Rewrite the rows first: narrowing an enum that still holds 'system' turns
|
||||
// those rows into the empty string rather than failing, which would leave the
|
||||
// merge and split notes present but unattributable. 'web' is the honest
|
||||
// fallback — they were written by a person pressing a button in the app.
|
||||
DB::table('ticket_messages')->where('source', 'system')->update(['source' => 'web']);
|
||||
|
||||
DB::statement(
|
||||
"ALTER TABLE ticket_messages
|
||||
MODIFY COLUMN source ENUM('web','email') NOT NULL DEFAULT 'web'"
|
||||
);
|
||||
}
|
||||
};
|
||||
@@ -626,13 +626,7 @@ class TicketController extends Controller
|
||||
->update(['ticket_id' => $ticket->id]);
|
||||
|
||||
// Add a system note marking the merge
|
||||
TicketMessage::create([
|
||||
'ticket_id' => $ticket->id,
|
||||
'user_id' => Auth::id(),
|
||||
'body' => "Merged from ticket {$source->number}.",
|
||||
'is_internal' => true,
|
||||
'source' => 'system',
|
||||
]);
|
||||
TicketMessage::system($ticket, "Merged from ticket {$source->number}.");
|
||||
|
||||
// Add source's submitter as a participant (if not already the submitter of target)
|
||||
if ($source->submitter_id !== $ticket->submitter_id) {
|
||||
@@ -672,16 +666,31 @@ class TicketController extends Controller
|
||||
'submitter_id' => $ticket->submitter_id,
|
||||
'assigned_to' => $ticket->assigned_to,
|
||||
'project_id' => $ticket->project_id,
|
||||
'title' => $ticket->title . ' (split)',
|
||||
'title' => static::splitTitle($ticket->title),
|
||||
'description' => $ticket->description,
|
||||
'status' => 'open',
|
||||
'priority_id' => $ticket->priority_id,
|
||||
'due_date' => $ticket->due_date,
|
||||
]);
|
||||
|
||||
// Duplicate messages (exclude system notes)
|
||||
/*
|
||||
* Duplicate messages, EXCLUDING system notes.
|
||||
*
|
||||
* This comment has always said "exclude" and the code never did. It cost
|
||||
* nothing while the enum rejected 'system', because no system row could
|
||||
* exist to copy — #415 makes them writable, so without this a split of a
|
||||
* previously-merged ticket would carry "Merged from ticket QAX-0005." into
|
||||
* a brand-new ticket that was never merged with anything.
|
||||
*
|
||||
* A system note is an audit record of something that happened to THIS
|
||||
* ticket. The copy gets its own, written below.
|
||||
*/
|
||||
$ticket->load('messages');
|
||||
foreach ($ticket->messages as $msg) {
|
||||
if ($msg->isSystem()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
TicketMessage::create([
|
||||
'ticket_id' => $newTicket->id,
|
||||
'user_id' => $msg->user_id,
|
||||
@@ -693,20 +702,8 @@ class TicketController extends Controller
|
||||
}
|
||||
|
||||
// Cross-reference notes on both tickets
|
||||
TicketMessage::create([
|
||||
'ticket_id' => $ticket->id,
|
||||
'user_id' => Auth::id(),
|
||||
'body' => "Split: new ticket {$newTicket->number} created from this ticket.",
|
||||
'is_internal' => true,
|
||||
'source' => 'system',
|
||||
]);
|
||||
TicketMessage::create([
|
||||
'ticket_id' => $newTicket->id,
|
||||
'user_id' => Auth::id(),
|
||||
'body' => "Split from ticket {$ticket->number}.",
|
||||
'is_internal' => true,
|
||||
'source' => 'system',
|
||||
]);
|
||||
TicketMessage::system($ticket, "Split: new ticket {$newTicket->number} created from this ticket.");
|
||||
TicketMessage::system($newTicket, "Split from ticket {$ticket->number}.");
|
||||
|
||||
return $newTicket;
|
||||
});
|
||||
@@ -715,6 +712,39 @@ class TicketController extends Controller
|
||||
->with('success', "Ticket {$newTicket->number} created from split.");
|
||||
}
|
||||
|
||||
/**
|
||||
* The split ticket's title: the original plus a suffix, guaranteed to fit.
|
||||
*
|
||||
* `tickets.title` is varchar(255) and both store() and update() validate the
|
||||
* title at `max:255`. Appending " (split)" to a title already at the limit
|
||||
* produced 263 characters and the same hard 500 as #415 — a second, unreported
|
||||
* instance of it on the very action being fixed, needing only a long enough
|
||||
* title to trigger.
|
||||
*
|
||||
* ── WHY THIS IS BOUNDED AND NOT WIDENED ─────────────────────────────────
|
||||
*
|
||||
* The sibling defect in central-radius (#410) was fixed the other way: a
|
||||
* 510-character skip reason was widened into the column rather than shortened,
|
||||
* because the reason was the point and the column was simply too small.
|
||||
*
|
||||
* The rule that separates them is what the limit MEANS. 255 here is not an
|
||||
* incidental column width — it is the product's own declared, user-facing title
|
||||
* limit, enforced by two validation rules. Widening the column to fit 263 would
|
||||
* mint rows the edit form then refuses to save, so the ticket could never be
|
||||
* corrected by anyone: a row the product will not let you fix is worse than a
|
||||
* truncated title. So: widen when the data is right and the column is wrong;
|
||||
* bound when the column expresses a real product rule.
|
||||
*
|
||||
* Multibyte-safe by character, because varchar(255) counts characters.
|
||||
*/
|
||||
private static function splitTitle(string $title): string
|
||||
{
|
||||
$suffix = ' (split)';
|
||||
$room = 255 - mb_strlen($suffix);
|
||||
|
||||
return (mb_strlen($title) > $room ? mb_substr($title, 0, $room) : $title).$suffix;
|
||||
}
|
||||
|
||||
/**
|
||||
* Add a participant to a ticket.
|
||||
*/
|
||||
|
||||
@@ -5,9 +5,20 @@ namespace Dashboard\Ticketing\Models;
|
||||
use Illuminate\Database\Eloquent\Model;
|
||||
use Illuminate\Database\Eloquent\Relations\BelongsTo;
|
||||
use Illuminate\Database\Eloquent\Relations\HasMany;
|
||||
use Illuminate\Support\Facades\Auth;
|
||||
|
||||
class TicketMessage extends Model
|
||||
{
|
||||
/**
|
||||
* Every value `ticket_messages.source` accepts, written down once.
|
||||
*
|
||||
* The column is an enum and this is the same list, so the two can be compared
|
||||
* rather than remembered. #415 was three `'source' => 'system'` literals against
|
||||
* a column that only knew 'web' and 'email' — the value existed in the code for
|
||||
* as long as merge and split have, and nowhere in the schema.
|
||||
*/
|
||||
public const SOURCES = ['web', 'email', 'system'];
|
||||
|
||||
protected $fillable = [
|
||||
'ticket_id', 'user_id', 'author_email', 'body',
|
||||
'is_internal', 'source', 'email_message_id',
|
||||
@@ -17,6 +28,33 @@ class TicketMessage extends Model
|
||||
'is_internal' => 'boolean',
|
||||
];
|
||||
|
||||
/**
|
||||
* THE ONLY WAY A SYSTEM NOTE IS WRITTEN.
|
||||
*
|
||||
* Merge writes one and split writes two, and before this they were three
|
||||
* separate literals in one controller — which is how all three carried a
|
||||
* `source` the column rejected, and how a fourth would have joined them.
|
||||
*
|
||||
* A system note is always internal: it is the audit trail of an agent action,
|
||||
* addressed to whoever works the ticket next, and never to the submitter.
|
||||
*/
|
||||
public static function system(Ticket|int $ticket, string $body): self
|
||||
{
|
||||
return static::create([
|
||||
'ticket_id' => $ticket instanceof Ticket ? $ticket->id : $ticket,
|
||||
'user_id' => Auth::id(),
|
||||
'body' => $body,
|
||||
'is_internal' => true,
|
||||
'source' => 'system',
|
||||
]);
|
||||
}
|
||||
|
||||
/** A note this class wrote, rather than a person or a mailbox. */
|
||||
public function isSystem(): bool
|
||||
{
|
||||
return $this->source === 'system';
|
||||
}
|
||||
|
||||
public function ticket(): BelongsTo
|
||||
{
|
||||
return $this->belongsTo(Ticket::class, 'ticket_id');
|
||||
|
||||
Reference in New Issue
Block a user