Put filterMessagesForRole's docblock back on filterMessagesForRole

The #920 guard landed between the comment and the function it describes.

src/Http/Controllers/TicketAttachmentController.php

@@ -29,6 +29,11 @@ class TicketAttachmentController extends Controller
             abort(403);
         }
 
+        // Same read-only rule as a reply — see TicketMessageController::store(). #920
+        if ($ticket->isMerged()) {
+            return back()->withErrors(['ticket' => Ticket::READ_ONLY_REASON]);
+        }
+
         $request->validate([
             'file' => 'required|file|max:' . (self::MAX_SIZE_MB * 1024),
             // Bug #5: message_id must belong to THIS ticket, not any arbitrary message

src/Http/Controllers/TicketController.php

@@ -47,6 +47,40 @@ class TicketController extends Controller
             ->exists();
     }
 
+    /**
+     * REFUSE A WRITE TO A MERGED TICKET — [central-ticketing] #920.
+     *
+     * The Show page has always announced "This ticket has been merged and is
+     * read-only." and hidden the reply composer and the Merge button. Nothing
+     * enforced it: the Edit form was still linked, still rendered (with its Status
+     * select blank, because `merged` is not one of its options), and `update()`
+     * happily wrote `status = open` while leaving `merged_into_id` pointing at the
+     * target. That produced a ticket that was live and assignable in the All Open
+     * queue and simultaneously merged into another one, with none of its own
+     * messages — an empty duplicate of the ticket it had been folded into.
+     *
+     * So the check lives HERE, on the server, once, and every write path that the
+     * page presents as unavailable calls it. Returning a refusal rather than
+     * `abort(403)` on purpose: the realistic way to arrive here is a page that was
+     * already open when somebody else merged the ticket, and that operator is
+     * better served by the reason on the ticket than by an error screen.
+     * `FormErrors` renders any key it is handed, so the sentence shows up.
+     *
+     * @return \Illuminate\Http\RedirectResponse|null  null when the write may proceed
+     */
+    private function refuseIfMerged(Ticket $ticket, bool $stayPut = false)
+    {
+        if (! $ticket->isMerged()) {
+            return null;
+        }
+
+        $errors = ['ticket' => Ticket::READ_ONLY_REASON];
+
+        return $stayPut
+            ? back()->withErrors($errors)
+            : redirect()->route('ticketing.show', $ticket)->withErrors($errors);
+    }
+
     /**
      * Filter messages for submitters — strip internal notes.
      */
@@ -417,12 +451,17 @@ class TicketController extends Controller
         ]);
     }
 
-    public function edit(Ticket $ticket): Response
+    /** @return Response|\Illuminate\Http\RedirectResponse — merged tickets are sent back to Show. */
+    public function edit(Ticket $ticket)
     {
         if (!$this->isAgent($ticket->group_id)) {
             abort(403);
         }
 
+        if ($refusal = $this->refuseIfMerged($ticket)) {
+            return $refusal;
+        }
+
         $ticket->load(['group', 'priority', 'project']);
         $priorities = PriorityLevel::orderBy('sort_order')->get();
         $agentIds = TicketingAgentAccess::where('group_id', $ticket->group_id)->pluck('user_id');
@@ -434,6 +473,16 @@ class TicketController extends Controller
             'priorities' => $priorities,
             'agents' => $agents,
             'projects' => $projects,
+            /*
+             * The form's Status select used to hard-code five slugs, so any ticket
+             * whose status was not one of them — `merged`, or any status added on
+             * the Statuses settings tab — rendered with nothing selected and the
+             * first save silently rewrote the status to whatever the operator
+             * happened to leave showing. Same defect as #920, one layer up: drive
+             * the options from the same table `update()` validates against.
+             */
+            'ticketStatuses' => TicketStatus::where('slug', '!=', Ticket::STATUS_MERGED)
+                ->orderBy('sort_order')->get(),
         ]);
     }
 
@@ -443,6 +492,10 @@ class TicketController extends Controller
             abort(403);
         }
 
+        if ($refusal = $this->refuseIfMerged($ticket)) {
+            return $refusal;
+        }
+
         // Determine effective group (may be changing via a transfer)
         $targetGroupId = $request->filled('group_id')
             ? (int) $request->input('group_id')
@@ -454,7 +507,7 @@ class TicketController extends Controller
             ->toArray();
 
         // Build valid status list from DB (exclude 'merged' — set only by the merge operation)
-        $validStatusSlugs = TicketStatus::where('slug', '!=', 'merged')->pluck('slug')->implode(',');
+        $validStatusSlugs = TicketStatus::where('slug', '!=', Ticket::STATUS_MERGED)->pluck('slug')->implode(',');
 
         $rules = [
             'title'       => 'sometimes|required|string|max:255',
@@ -609,7 +662,7 @@ class TicketController extends Controller
         if ($source->id === $ticket->id) {
             return back()->withErrors(['source_number' => 'Cannot merge a ticket into itself.']);
         }
-        if ($source->status === 'merged') {
+        if ($source->isMerged()) {
             return back()->withErrors(['source_number' => 'That ticket has already been merged.']);
         }
         if (! $this->isAgent($source->group_id)) {
@@ -638,7 +691,7 @@ class TicketController extends Controller
 
             // Mark source as merged
             $source->update([
-                'status'         => 'merged',
+                'status'         => Ticket::STATUS_MERGED,
                 'merged_into_id' => $ticket->id,
             ]);
         });
@@ -656,6 +709,10 @@ class TicketController extends Controller
             abort(403);
         }
 
+        if ($refusal = $this->refuseIfMerged($ticket)) {
+            return $refusal;
+        }
+
         $newTicket = DB::transaction(function () use ($ticket) {
             $group  = TicketingGroup::findOrFail($ticket->group_id);
             $number = $group->nextTicketNumber();
@@ -754,6 +811,10 @@ class TicketController extends Controller
             abort(403);
         }
 
+        if ($refusal = $this->refuseIfMerged($ticket, stayPut: true)) {
+            return $refusal;
+        }
+
         $validated = $request->validate([
             'user_id' => 'required|integer|exists:users,id',
         ]);
@@ -775,6 +836,10 @@ class TicketController extends Controller
             abort(403);
         }
 
+        if ($refusal = $this->refuseIfMerged($ticket, stayPut: true)) {
+            return $refusal;
+        }
+
         TicketParticipant::where('ticket_id', $ticket->id)
             ->where('user_id', $userId)
             ->delete();

src/Http/Controllers/TicketMessageController.php

@@ -29,6 +29,19 @@ class TicketMessageController extends Controller
             abort(403);
         }
 
+        /*
+         * A merged ticket takes no new messages — [central-ticketing] #920.
+         *
+         * Show.vue replaces the composer with "This ticket has been merged and is
+         * read-only.", and that was the whole of the guard: a POST straight to this
+         * route (or from a page that was already open when the merge happened) still
+         * wrote a message onto a ticket whose thread has moved to the merge target,
+         * where nobody working the ticket would ever see it.
+         */
+        if ($ticket->isMerged()) {
+            return back()->withErrors(['ticket' => Ticket::READ_ONLY_REASON]);
+        }
+
         $validated = $request->validate([
             'body' => 'required|string',
             'is_internal' => 'boolean',

src/Models/Ticket.php

@@ -8,6 +8,16 @@ use Illuminate\Database\Eloquent\Relations\HasMany;
 
 class Ticket extends Model
 {
+    /** Set only by TicketController::merge(); never offered by any form. */
+    public const STATUS_MERGED = 'merged';
+
+    /**
+     * What every surface says when a write to a merged ticket is refused. The Show
+     * page has displayed this sentence since merge shipped; #920 was that it was
+     * only ever a sentence, with no server-side guard behind it.
+     */
+    public const READ_ONLY_REASON = 'This ticket has been merged and is read-only.';
+
     protected $fillable = [
         'number', 'group_id', 'submitter_id', 'assigned_to', 'project_id',
         'title', 'description', 'status', 'priority_id', 'due_date', 'merged_into_id',
@@ -70,4 +80,20 @@ class Ticket extends Model
     {
         return $this->belongsTo(Ticket::class, 'merged_into_id');
     }
+
+    /**
+     * A merged ticket is read-only — its messages and attachments now live on the
+     * ticket it was merged into, so it is a stub kept for its reference number and
+     * the audit trail, not a ticket anyone can still work.
+     *
+     * `status === 'merged'` and NOT `merged_into_id !== null`, deliberately. #920
+     * minted rows with `status = 'open', merged_into_id = 56` — the very rows this
+     * guard exists to prevent — and reading the id would make each of those
+     * permanently uneditable, i.e. a row the product will not let anyone correct.
+     * The status is the state; the id is the pointer that goes with it.
+     */
+    public function isMerged(): bool
+    {
+        return $this->status === self::STATUS_MERGED;
+    }
 }