Bulk status/close: enforce Workflow Designer rules (#21)

Bulk status changes previously bypassed the workflow entirely — the model
carried an explicit "admin-only escape hatch" note — so bulk edit could
drive tickets through transitions the designer forbids and skip comments
the designer requires.

BulkOperationsModel now applies the same rules as the single-ticket path:

- Transitions absent from status_transitions are refused per ticket and
  reported with a reason, instead of being forced through.
- requires_comment is checked up front across the whole selection, so a
  batch is rejected before any ticket is mutated rather than half-applied.
- The reason is persisted as a comment on each ticket changed, matching
  what a single-ticket close records.
- Tickets already in the target status are a no-op success, not a failure.

requires_admin needs no extra check: api/bulk_operation.php already gates
the endpoint on admin.

Client: both bulk modals now collect a reason, the close path gets a real
modal instead of a bare confirm, and per-ticket skip reasons surface in
the result toast instead of a bare failure count.

models/BulkOperationsModel.php

@@ -7,11 +7,47 @@ class BulkOperationsModel
 {
     private $conn;
 
+    /** @var WorkflowModel|null Lazily created; only needed by status-changing operations */
+    private $workflowModel = null;
+
+    /** @var CommentModel|null Lazily created; only needed when a status change carries a comment */
+    private $commentModel = null;
+
+    /** @var array<int,string> user_id → display name, resolved once per request */
+    private $userNames = [];
+
     public function __construct($conn)
     {
         $this->conn = $conn;
     }
 
+    /**
+     * Workflow model, created on first use.
+     */
+    private function workflow(): WorkflowModel
+    {
+        if ($this->workflowModel === null) {
+            require_once dirname(__DIR__) . '/models/WorkflowModel.php';
+            $this->workflowModel = new WorkflowModel($this->conn);
+        }
+        return $this->workflowModel;
+    }
+
+    /**
+     * The status a bulk operation is trying to move tickets into, or null for
+     * operations that don't change status.
+     */
+    private function targetStatusFor(string $operationType, array $parameters): ?string
+    {
+        if ($operationType === 'bulk_close') {
+            return 'Closed';
+        }
+        if ($operationType === 'bulk_status') {
+            return isset($parameters['status']) ? (string)$parameters['status'] : null;
+        }
+        return null;
+    }
+
     /**
      * Create a new bulk operation record
      *
@@ -100,6 +136,30 @@ class BulkOperationsModel
         // Batch load all tickets in one query to eliminate N+1 problem
         $ticketsById = $ticketModel->getTicketsByIds($ticketIds);
 
+        // Status-changing operations honour the Workflow Designer. If any ticket in
+        // the selection needs a comment for its transition, reject the whole batch
+        // before mutating anything so the client can collect one — a partially
+        // applied batch is worse than none.
+        $targetStatus = $this->targetStatusFor($operation['operation_type'], is_array($parameters) ? $parameters : []);
+        $bulkComment = trim((string)($parameters['comment'] ?? ''));
+        if ($targetStatus !== null && $bulkComment === '') {
+            foreach ($ticketIds as $tid) {
+                $t = $ticketsById[trim($tid)] ?? null;
+                if (!$t || $t['status'] === $targetStatus) {
+                    continue;
+                }
+                if ($this->workflow()->transitionRequiresComment($t['status'], $targetStatus)) {
+                    return [
+                        'processed' => 0,
+                        'failed' => count($ticketIds),
+                        'error' => 'A comment is required to change status from '
+                            . $t['status'] . ' → ' . $targetStatus,
+                        'requires_comment' => true,
+                    ];
+                }
+            }
+        }
+
         // Start transaction for data consistency
         $this->conn->begin_transaction();
 
@@ -113,11 +173,32 @@ class BulkOperationsModel
                 $success = false;
 
                 try {
-                    // NOTE: bulk_status / bulk_close intentionally do NOT run
-                    // WorkflowModel::isTransitionAllowed(). Bulk operations are an
-                    // admin-only escape hatch for forcing ticket states (e.g. mass
-                    // re-opening), so they bypass the workflow transition rules that
-                    // the single-ticket update path enforces. This is by design.
+                    // bulk_status / bulk_close enforce the same Workflow Designer
+                    // rules as the single-ticket path: a transition the designer
+                    // doesn't define is refused, and requires_comment is honoured
+                    // (checked up front, above). requires_admin is satisfied because
+                    // api/bulk_operation.php already gates the endpoint on admin.
+                    if ($targetStatus !== null) {
+                        $currentTicket = $ticketsById[$ticketId] ?? null;
+                        if ($currentTicket && $currentTicket['status'] === $targetStatus) {
+                            // Already in the requested state — nothing to do, and
+                            // reporting a no-op as a failure would just confuse.
+                            $processed++;
+                            continue;
+                        }
+                        $allowed = $currentTicket === null || $this->workflow()->isTransitionAllowed(
+                            $currentTicket['status'],
+                            $targetStatus,
+                            true
+                        );
+                        if (!$allowed) {
+                            $failed++;
+                            $errors[] = "Ticket $ticketId: transition not allowed ("
+                                . $currentTicket['status'] . ' → ' . $targetStatus . ')';
+                            continue;
+                        }
+                    }
+
                     switch ($operation['operation_type']) {
                         case 'bulk_close':
                             // Get current ticket from pre-loaded batch
@@ -232,6 +313,12 @@ class BulkOperationsModel
                     }
 
                     if ($success) {
+                        // Persist the status-change reason as a real comment, so a
+                        // bulk close is as auditable on the ticket as a single close
+                        // (where the client posts the comment before updating).
+                        if ($targetStatus !== null && $bulkComment !== '') {
+                            $this->postBulkComment($ticketId, (int)$operation['performed_by'], $bulkComment);
+                        }
                         $processed++;
                     } else {
                         $failed++;
@@ -350,6 +437,37 @@ class BulkOperationsModel
         return null;
     }
 
+    /**
+     * Post the bulk status-change reason as a comment on one ticket.
+     *
+     * Runs inside the caller's transaction, so a rollback drops the comment along
+     * with the status change.
+     */
+    private function postBulkComment(string $ticketId, int $userId, string $text): void
+    {
+        require_once dirname(__DIR__) . '/models/CommentModel.php';
+
+        if ($this->commentModel === null) {
+            $this->commentModel = new CommentModel($this->conn);
+        }
+        if (!isset($this->userNames[$userId])) {
+            $stmt = $this->conn->prepare(
+                "SELECT COALESCE(NULLIF(display_name, ''), username) AS name FROM users WHERE user_id = ? LIMIT 1"
+            );
+            $stmt->bind_param("i", $userId);
+            $stmt->execute();
+            $row = $stmt->get_result()->fetch_assoc();
+            $stmt->close();
+            $this->userNames[$userId] = $row['name'] ?? 'User';
+        }
+
+        $this->commentModel->addComment($ticketId, [
+            'user_name' => $this->userNames[$userId],
+            'comment_text' => $text,
+            'markdown_enabled' => 0,
+        ], $userId);
+    }
+
     /**
      * Check whether a user ID exists.
      */