Merge #21 workflow enforcement for bulk operations into main

api/bulk_operation.php

@@ -107,10 +107,17 @@ $result = $bulkOpsModel->processBulkOperation($operationId);
 
 if (isset($result['error'])) {
     $conn->close();
-    echo json_encode([
+    $response = [
         'success' => false,
         'error' => $result['error']
-    ]);
+    ];
+    // Let the client know it should collect a comment and retry, rather than
+    // showing the failure as a dead end.
+    if (!empty($result['requires_comment'])) {
+        $response['requires_comment'] = true;
+        http_response_code(400);
+    }
+    echo json_encode($response);
 } else {
     // Invalidate stats cache so dashboard tiles reflect changes immediately
     require_once dirname(__DIR__) . '/models/StatsModel.php';

assets/js/dashboard.js

@@ -157,6 +157,12 @@ document.addEventListener('DOMContentLoaded', function() {
             case 'close-bulk-status-modal':
                 closeBulkStatusModal();
                 break;
+            case 'perform-bulk-close':
+                performBulkCloseAction();
+                break;
+            case 'close-bulk-close-modal':
+                closeBulkCloseModal();
+                break;
             case 'perform-bulk-delete':
                 performBulkDelete();
                 break;
@@ -515,24 +521,59 @@ function bulkClose() {
         return;
     }
 
-    showConfirmModal(
-        `Close ${ticketIds.length} Ticket(s)?`,
-        'Are you sure you want to close these tickets?',
-        'warning',
-        () => performBulkCloseAction(ticketIds)
-    );
+    // Closing needs a reason: the default workflow marks every → Closed transition
+    // requires_comment, so collect it here instead of failing server-side.
+    const modalHtml = `
+        <div class="lt-modal-overlay" id="bulkCloseModal" aria-hidden="true" role="dialog" aria-modal="true" aria-labelledby="bulkCloseModalTitle">
+            <div class="lt-modal">
+                <div class="lt-modal-header" style="color:var(--terminal-amber)">
+                    <span class="lt-modal-title" id="bulkCloseModalTitle">[ ! ] Close ${ticketIds.length} Ticket(s)</span>
+                    <button class="lt-modal-close" data-modal-close aria-label="Close">✕</button>
+                </div>
+                <div class="lt-modal-body">
+                    <label for="bulkCloseComment">Close Reason:</label>
+                    <textarea id="bulkCloseComment" class="lt-input lt-w-full" rows="3"
+                              placeholder="Why are these tickets being closed?…"
+                              style="resize:vertical;font-family:inherit;font-size:0.8rem"
+                              aria-label="Reason for closing the tickets"></textarea>
+                    <p class="lt-text-xs lt-text-muted" style="margin-top:0.35rem">
+                        Posted as a comment on every ticket closed. Tickets whose workflow
+                        forbids closing from their current status are skipped.
+                    </p>
+                </div>
+                <div class="lt-modal-footer">
+                    <button data-action="perform-bulk-close" class="lt-btn lt-btn-primary">CLOSE TICKETS</button>
+                    <button data-action="close-bulk-close-modal" class="lt-btn lt-btn-ghost">CANCEL</button>
+                </div>
+            </div>
+        </div>
+    `;
+
+    document.body.insertAdjacentHTML('beforeend', modalHtml);
+    lt.modal.open('bulkCloseModal');
+}
+
+function closeBulkCloseModal() {
+    lt.modal.close('bulkCloseModal');
+    const modal = document.getElementById('bulkCloseModal');
+    if (modal) setTimeout(() => modal.remove(), 300);
 }
 
 function performBulkCloseAction(ticketIds) {
+    ticketIds = ticketIds || getSelectedTicketIds();
+    const commentEl = document.getElementById('bulkCloseComment');
+    const comment = commentEl ? commentEl.value.trim() : '';
 
     lt.api.post('/api/bulk_operation.php', {
         operation_type: 'bulk_close',
-        ticket_ids: ticketIds
+        ticket_ids: ticketIds,
+        parameters: { comment: comment }
     })
     .then(data => {
+        closeBulkCloseModal();
         if (data.success) {
             if (data.failed > 0) {
-                lt.toast.warning(`Bulk close: ${data.processed} succeeded, ${data.failed} failed`, 5000);
+                lt.toast.warning(bulkResultMessage('Bulk close', data), 6000);
             } else {
                 lt.toast.success(`Successfully closed ${data.processed} ticket(s)`, 4000);
             }
@@ -542,6 +583,14 @@ function performBulkCloseAction(ticketIds) {
         }
     })
     .catch(error => {
+        // Missing required comment — keep the modal open so it can be entered.
+        if (error && error.data && error.data.requires_comment) {
+            lt.toast.warning(error.data.error || 'A close reason is required', 6000);
+            const ta = document.getElementById('bulkCloseComment');
+            if (ta) ta.focus();
+            return;
+        }
+        closeBulkCloseModal();
         lt.toast.error('Bulk close failed: ' + error.message, 5000);
     });
 }
@@ -777,6 +826,15 @@ function showBulkStatusModal() {
                         <option value="">Select Status...</option>
                         ${(window.TICKET_STATUSES || ['Open','Pending','In Progress','Closed']).map(s => `<option value="${s}">${s}</option>`).join('')}
                     </select>
+                    <label for="bulkStatusComment" style="margin-top:0.75rem">Reason / Comment:</label>
+                    <textarea id="bulkStatusComment" class="lt-input lt-w-full" rows="3"
+                              placeholder="Reason for the status change…"
+                              style="resize:vertical;font-family:inherit;font-size:0.8rem"
+                              aria-label="Reason for the bulk status change"></textarea>
+                    <p class="lt-text-xs lt-text-muted" style="margin-top:0.35rem">
+                        Required for transitions the Workflow Designer marks as needing a comment
+                        (e.g. closing a ticket). Posted as a comment on every ticket changed.
+                    </p>
                 </div>
                 <div class="lt-modal-footer">
                     <button data-action="perform-bulk-status" class="lt-btn lt-btn-primary">UPDATE</button>
@@ -807,16 +865,19 @@ function performBulkStatusChange() {
         return;
     }
 
+    const commentEl = document.getElementById('bulkStatusComment');
+    const comment = commentEl ? commentEl.value.trim() : '';
+
     lt.api.post('/api/bulk_operation.php', {
         operation_type: 'bulk_status',
         ticket_ids: ticketIds,
-        parameters: { status: status }
+        parameters: { status: status, comment: comment }
     })
     .then(data => {
         closeBulkStatusModal();
         if (data.success) {
             if (data.failed > 0) {
-                lt.toast.warning(`Status update: ${data.processed} succeeded, ${data.failed} failed`, 5000);
+                lt.toast.warning(bulkResultMessage('Status update', data), 6000);
             } else {
                 lt.toast.success(`Successfully updated status for ${data.processed} ticket(s)`, 4000);
             }
@@ -826,10 +887,32 @@ function performBulkStatusChange() {
         }
     })
     .catch(error => {
+        // Workflow needs a comment for at least one selected ticket — keep the
+        // modal open so the reason can be typed in without re-selecting.
+        if (error && error.data && error.data.requires_comment) {
+            lt.toast.warning(error.data.error || 'A comment is required for this status change', 6000);
+            const ta = document.getElementById('bulkStatusComment');
+            if (ta) ta.focus();
+            return;
+        }
+        closeBulkStatusModal();
         lt.toast.error('Bulk status change failed: ' + error.message, 5000);
     });
 }
 
+/**
+ * Build a result message for a partially-successful bulk operation, surfacing the
+ * per-ticket reasons (e.g. "transition not allowed") instead of a bare count.
+ */
+function bulkResultMessage(label, data) {
+    let msg = `${label}: ${data.processed} succeeded, ${data.failed} failed`;
+    if (Array.isArray(data.errors) && data.errors.length) {
+        msg += ' — ' + data.errors.slice(0, 3).join('; ');
+        if (data.errors.length > 3) msg += ` (+${data.errors.length - 3} more)`;
+    }
+    return msg;
+}
+
 // Bulk Delete
 function showBulkDeleteModal() {
     const ticketIds = getSelectedTicketIds();

migrations/000_baseline.sql

@@ -59,7 +59,8 @@ CREATE TABLE IF NOT EXISTS `bulk_operations` (
   `ticket_ids` text NOT NULL,
   `performed_by` int(11) NOT NULL,
   `parameters` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL CHECK (json_valid(`parameters`)),
-  `status` varchar(20) DEFAULT 'pending',
+  -- 32, not 20: 'completed_with_errors' is 21 chars (see 001_widen_bulk_operations_status.sql)
+  `status` varchar(32) DEFAULT 'pending',
   `total_tickets` int(11) DEFAULT NULL,
   `processed_tickets` int(11) DEFAULT 0,
   `failed_tickets` int(11) DEFAULT 0,

migrations/001_widen_bulk_operations_status.sql

@@ -0,0 +1,12 @@
+-- Widen bulk_operations.status
+--
+-- The code writes 'completed_with_errors' (21 chars) when a bulk operation
+-- finishes with per-ticket failures, but the column was varchar(20), so the
+-- write failed with "Data too long for column 'status'". This was unreachable
+-- while bulk status changes forced every transition through; now that they
+-- honour the Workflow Designer, partial failures are a normal outcome.
+--
+-- Safe to re-run.
+
+ALTER TABLE `bulk_operations`
+    MODIFY COLUMN `status` varchar(32) DEFAULT 'pending';

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++;
@@ -290,14 +377,22 @@ class BulkOperationsModel
             ];
         }
 
-        // Update operation status
-        $status = $failed > 0 ? 'completed_with_errors' : 'completed';
-        $sql = "UPDATE bulk_operations SET status = ?, processed_tickets = ?, failed_tickets = ?,
-                completed_at = NOW() WHERE operation_id = ?";
-        $stmt = $this->conn->prepare($sql);
-        $stmt->bind_param("siii", $status, $processed, $failed, $operationId);
-        $stmt->execute();
-        $stmt->close();
+        // Update operation status. This is bookkeeping only and runs after the
+        // ticket changes are committed, so a failure here (e.g. the status column
+        // not yet widened by 001_widen_bulk_operations_status.sql on an instance
+        // deployed ahead of its migrations) must not turn a completed operation
+        // into an error response.
+        try {
+            $status = $failed > 0 ? 'completed_with_errors' : 'completed';
+            $sql = "UPDATE bulk_operations SET status = ?, processed_tickets = ?, failed_tickets = ?,
+                    completed_at = NOW() WHERE operation_id = ?";
+            $stmt = $this->conn->prepare($sql);
+            $stmt->bind_param("siii", $status, $processed, $failed, $operationId);
+            $stmt->execute();
+            $stmt->close();
+        } catch (Throwable $e) {
+            error_log("Bulk operation $operationId completed but status bookkeeping failed: " . $e->getMessage());
+        }
 
         $result = ['processed' => $processed, 'failed' => $failed];
         if (!empty($errors)) {
@@ -350,6 +445,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.
      */