Trim comment text before persisting, not just for validation

add_comment.php computed a trimmed copy of comment_text only to check for empty input, then passed the original untrimmed $data through to CommentModel::addComment(), so any leading/trailing whitespace the user typed (or pasted) was written to ticket_comments.comment_text as-is. update_comment.php already trims before saving edits, so a comment could pass through this endpoint once with untrimmed text (creation) and be silently corrected the moment it was next edited — inconsistent storage that, combined with the markdown parser's line-anchored regexes (headings, tables, lists all match on ^), could make a markdown-enabled comment mis-render after a reload depending on whether its first line carried leading whitespace. Also trims in the "Load more comments" pagination re-render path in TicketView.php, matching the two on-load renderers in markdown.js so all three code paths that call parseMarkdown() on stored comment text treat leading whitespace consistently. Closes #18 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Merge #22 chart click-to-filter into main
2026-08-31 21:03:00 -04:00 · 2026-08-07 23:15:59 -04:00 · 2026-08-07 23:15:51 -04:00 · 2026-08-07 23:07:14 -04:00 · 2026-08-07 23:07:06 -04:00 · 2026-08-07 23:06:36 -04:00
13 changed files with 421 additions and 45 deletions
@@ -99,6 +99,11 @@ try {
        exit;
    }
    // Persist the trimmed text (not the raw client value) — matches update_comment.php
    // and keeps stored comment_text free of leading whitespace that could shift a
    // markdown-enabled comment's first line out of column 0 on reload.
    $data['comment_text'] = $commentTextRaw;
    // Never trust a client-supplied display name — always attribute the comment to
    // the authenticated session user.
    $data['user_name'] = $currentUser['display_name'] ?? $currentUser['username'] ?? 'User';
@@ -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';
@@ -66,6 +66,7 @@
  --accent-green-bright: #33FFAA;
  --accent-green-dim:    rgba(0,255,136,0.10);
  --accent-green-border: rgba(0,255,136,0.22);
  --shadow-color:        rgba(0,0,0,0.5);
  /* --- Error / Critical --- */
  --accent-red:     #FF2D55;
@@ -3640,6 +3641,8 @@ html[data-theme="light"] {
  --accent-red-dim:      rgba(181,0,31,0.10);
  --accent-amber-dim:    rgba(138,90,0,0.10);
  --accent-cyan-border:  rgba(0,98,184,0.28);
  --accent-green-border: rgba(0,109,53,0.28);
  --shadow-color:        rgba(50,80,130,0.18);
  /* — Glows become subtle drop shadows in light mode — */
  --glow-orange:         0 0 0 1px rgba(196,78,0,0.25),  0 1px 6px rgba(196,78,0,0.18);
@@ -3737,6 +3740,25 @@ html[data-theme="light"] .lt-textarea:focus-visible {
  border-color: var(--accent-cyan);
  box-shadow: var(--box-glow-cyan);
 }
 /* Native <select> popup in light mode.
   `.lt-select` sets `color-scheme: dark` on the element itself, which beats the
   `color-scheme: light` declared on <html>, so the browser drew the dropdown with
   dark chrome even in light mode. Reset it per element, and re-tint the option
   list, which is otherwise hardcoded to #0d1117 for the dark theme. */
 html[data-theme="light"] .lt-select { color-scheme: light; }
 html[data-theme="light"] .lt-select option,
 html[data-theme="light"] select option {
  background: var(--bg-input);
  color: var(--text-primary);
 }
 html[data-theme="light"] .lt-select option:hover,
 html[data-theme="light"] .lt-select option:focus,
 html[data-theme="light"] .lt-select option:checked,
 html[data-theme="light"] select option:checked {
  background: var(--accent-orange-dim);
  color: var(--accent-orange);
 }
 html[data-theme="light"] .lt-label { color: var(--text-muted); }
 /* — Buttons — */
@@ -338,17 +338,22 @@ kbd {
 }
 /* ── Ticket preview popup ────────────────────────────────────── */
 /* --lt-surface is not defined anywhere, so the background always fell through to
   the hardcoded #0a0e14 — a near-black panel in light mode, with no colour set at
   all, so the inherited near-black body text was invisible on it. These tokens
   are redefined for light mode in base.css. */
 .ticket-preview-popup {
  position: fixed;
  z-index: 9999;
-  background: var(--lt-surface, #0a0e14);
+  background: var(--bg-card);
-  border: 1px solid rgba(0, 255, 65, 0.4);
+  color: var(--text-primary);
  border: 1px solid var(--accent-green-border);
  padding: 0.75rem;
  min-width: 280px;
  max-width: 360px;
  font-size: 0.75rem;
  pointer-events: auto;
-  box-shadow: 0 4px 20px rgba(0,0,0,0.5);
+  box-shadow: 0 4px 20px var(--shadow-color);
 }
 .ticket-preview-popup .preview-header {
  display: flex;
@@ -356,7 +361,7 @@ kbd {
  align-items: center;
  margin-bottom: 0.4rem;
 }
-.ticket-preview-popup .preview-id   { color: var(--lt-cyan, #00ffff); font-weight: 700; }
+.ticket-preview-popup .preview-id   { color: var(--accent-cyan); font-weight: 700; }
 .ticket-preview-popup .preview-title { font-weight: 600; margin-bottom: 0.4rem; }
 .ticket-preview-popup .preview-meta { opacity: 0.7; display: flex; flex-direction: column; gap: 0.1rem; }
 .ticket-preview-popup .preview-footer { margin-top: 0.4rem; opacity: 0.5; font-size: 0.65rem; }
@@ -54,10 +54,18 @@ body.edit-mode .editable-metadata {
  text-transform: uppercase;
  letter-spacing: 0.05em;
 }
-.lt-status-select.lt-status-open      { color: var(--lt-success, #00ff41); border-color: var(--lt-success, #00ff41); }
+/* --lt-success / --lt-amber / --lt-cyan / --lt-danger are not defined anywhere,
-.lt-status-select.lt-status-pending   { color: var(--lt-amber,   #ffb000); border-color: var(--lt-amber,   #ffb000); }
+   so these always fell through to the hardcoded neon fallbacks — unreadable on
-.lt-status-select.lt-status-in-progress { color: var(--lt-cyan,  #00ffff); border-color: var(--lt-cyan,   #00ffff); }
+   the light theme's white input background. The --accent-* tokens carry the same
-.lt-status-select.lt-status-closed    { color: var(--lt-danger,  #ff4d4d); border-color: var(--lt-danger, #ff4d4d); }
+   hues and are redefined for light mode in base.css.
   The leading .lt-select is needed for specificity: base.css's
   `html[data-theme="light"] .lt-select` (0,2,1) would otherwise outrank a plain
   two-class selector and repaint every status the same near-black. */
 .lt-select.lt-status-select.lt-status-open        { color: var(--accent-green); border-color: var(--accent-green); }
 .lt-select.lt-status-select.lt-status-pending     { color: var(--accent-amber); border-color: var(--accent-amber); }
 .lt-select.lt-status-select.lt-status-in-progress { color: var(--accent-cyan);  border-color: var(--accent-cyan); }
 .lt-select.lt-status-select.lt-status-closed      { color: var(--accent-red);   border-color: var(--accent-red); }
 /* ── Ticket meta KV grid ─────────────────────────────────────── */
 .ticket-meta-grid {
@@ -241,6 +241,11 @@
        trigger.focus();
      }
    }
    // Announce the close so whoever opened the modal can undo optimistic UI or
    // clean up a dynamically-inserted overlay. A modal can be dismissed four
    // ways — the ✕ button, a Cancel button, a backdrop click, and Escape — and
    // the last two are handled globally here, so button-only listeners miss them.
    el.dispatchEvent(new CustomEvent('lt:modalclose', { bubbles: true }));
  }
  function closeAllModals() {
@@ -2774,6 +2779,9 @@
        setTimeout(() => { if (modalEl && modalEl.parentNode) modalEl.remove(); }, 300);
        resolve(value);
      };
      // Any dismissal counts as "no comment given", including a backdrop click or
      // Escape, which close the overlay through the global handlers above.
      modalEl.addEventListener('lt:modalclose', () => finish(null));
      modalEl.querySelector('[data-modal-close]').addEventListener('click', () => finish(null));
      document.getElementById(modalId + '_cancel').addEventListener('click', () => finish(null));
      document.getElementById(modalId + '_confirm').addEventListener('click', () => {
@@ -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(
+    // Closing needs a reason: the default workflow marks every → Closed transition
-        `Close ${ticketIds.length} Ticket(s)?`,
+    // requires_comment, so collect it here instead of failing server-side.
-        'Are you sure you want to close these tickets?',
+    const modalHtml = `
-        'warning',
+        <div class="lt-modal-overlay" id="bulkCloseModal" aria-hidden="true" role="dialog" aria-modal="true" aria-labelledby="bulkCloseModalTitle">
-        () => performBulkCloseAction(ticketIds)
+            <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);
    openModalWithDismiss('bulkCloseModal', closeBulkCloseModal);
 }
 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);
    });
 }
@@ -584,7 +633,7 @@ function showBulkAssignModal() {
    `;
    document.body.insertAdjacentHTML('beforeend', modalHtml);
-    lt.modal.open('bulkAssignModal');
+    openModalWithDismiss('bulkAssignModal', closeBulkAssignModal);
    setTimeout(() => { const inp = document.getElementById('bulkAssignUserInput'); if (inp) inp.focus(); }, 120);
    lt.api.get('/api/get_users.php')
@@ -682,7 +731,7 @@ function showBulkPriorityModal() {
    `;
    document.body.insertAdjacentHTML('beforeend', modalHtml);
-    lt.modal.open('bulkPriorityModal');
+    openModalWithDismiss('bulkPriorityModal', closeBulkPriorityModal);
 }
 function closeBulkPriorityModal() {
@@ -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>
@@ -787,7 +845,7 @@ function showBulkStatusModal() {
    `;
    document.body.insertAdjacentHTML('beforeend', modalHtml);
-    lt.modal.open('bulkStatusModal');
+    openModalWithDismiss('bulkStatusModal', closeBulkStatusModal);
 }
 function closeBulkStatusModal() {
@@ -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();
@@ -859,7 +942,7 @@ function showBulkDeleteModal() {
    `;
    document.body.insertAdjacentHTML('beforeend', modalHtml);
-    lt.modal.open('bulkDeleteModal');
+    openModalWithDismiss('bulkDeleteModal', closeBulkDeleteModal);
 }
 function closeBulkDeleteModal() {
@@ -949,6 +1032,22 @@ function showInputModal(title, label, placeholder = '', onSubmit, onCancel = nul
    input.addEventListener('keypress', (e) => { if (e.key === 'Enter') handleSubmit(); });
    document.getElementById(`${modalId}_cancel`).addEventListener('click', () => cleanup(onCancel));
    modal.querySelector('[data-modal-close]').addEventListener('click', () => cleanup(onCancel));
    // Backdrop click / Escape close the overlay via base.js's global handlers.
    modal.addEventListener('lt:modalclose', () => cleanup(onCancel));
 }
 /**
 * Open a dynamically-inserted modal and make sure it tears itself down however it
 * is dismissed. base.js handles backdrop clicks and Escape globally, so wiring
 * only the ✕/Cancel buttons leaves the overlay in the DOM — and the next open
 * inserts a second element with the same id, which then shadows the live one.
 */
 function openModalWithDismiss(modalId, onDismiss) {
    lt.modal.open(modalId);
    const el = document.getElementById(modalId);
    // lt.modal.close() early-returns once .is-open is gone, so the close call
    // inside onDismiss cannot re-enter this listener.
    if (el) el.addEventListener('lt:modalclose', onDismiss);
 }
 // ========================================
@@ -986,7 +1085,7 @@ function quickStatusChange(ticketId, currentStatus) {
    `;
    document.body.insertAdjacentHTML('beforeend', modalHtml);
-    lt.modal.open('quickStatusModal');
+    openModalWithDismiss('quickStatusModal', closeQuickStatusModal);
 }
 function closeQuickStatusModal() {
@@ -1053,7 +1152,7 @@ function quickAssign(ticketId) {
    `;
    document.body.insertAdjacentHTML('beforeend', modalHtml);
-    lt.modal.open('quickAssignModal');
+    openModalWithDismiss('quickAssignModal', closeQuickAssignModal);
    lt.api.get('/api/get_users.php')
        .then(data => {
@@ -284,7 +284,15 @@ function addComment() {
            // Clear the comment box
            const nc = document.getElementById('newComment');
            if (nc) nc.value = '';
-            
+
            // Clear the live preview — clearing the textarea programmatically
            // does not fire 'input', so updatePreview() never runs
            const previewDiv = document.getElementById('markdownPreview');
            if (previewDiv) {
                previewDiv.innerHTML = '';
                previewDiv.classList.add('is-hidden');
            }
            // Format the comment text for display
            let displayText;
            if (isMarkdownEnabled) {
@@ -521,7 +529,19 @@ function updateTicketStatus() {
        `);
        const modal = document.getElementById(modalId);
        lt.modal.open(modalId);
-        const cleanup = (ok) => { lt.modal.close(modalId); setTimeout(() => modal.remove(), 300); if (!ok) statusSelect.selectedIndex = 0; };
+        let settled = false;
        const cleanup = (ok) => {
            if (settled) return;   // lt.modal.close() below re-enters via lt:modalclose
            settled = true;
            lt.modal.close(modalId);
            setTimeout(() => modal.remove(), 300);
            if (!ok) statusSelect.selectedIndex = 0;
        };
        // Backdrop click and Escape close the overlay through base.js's global
        // handlers. Without this the dropdown kept displaying the new status
        // while the server was never called, so the ticket looked closed until
        // a reload revealed it was still open.
        modal.addEventListener('lt:modalclose', () => cleanup(false));
        modal.querySelector('[data-modal-close]').addEventListener('click', () => cleanup(false));
        document.getElementById(`${modalId}_cancel`).addEventListener('click', () => cleanup(false));
        document.getElementById(`${modalId}_confirm`).addEventListener('click', () => {
@@ -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,
@@ -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';
@@ -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
+                    // bulk_status / bulk_close enforce the same Workflow Designer
-                    // WorkflowModel::isTransitionAllowed(). Bulk operations are an
+                    // rules as the single-ticket path: a transition the designer
-                    // admin-only escape hatch for forcing ticket states (e.g. mass
+                    // doesn't define is refused, and requires_comment is honoured
-                    // re-opening), so they bypass the workflow transition rules that
+                    // (checked up front, above). requires_admin is satisfied because
-                    // the single-ticket update path enforces. This is by design.
+                    // 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
+        // Update operation status. This is bookkeeping only and runs after the
-        $status = $failed > 0 ? 'completed_with_errors' : 'completed';
+        // ticket changes are committed, so a failure here (e.g. the status column
-        $sql = "UPDATE bulk_operations SET status = ?, processed_tickets = ?, failed_tickets = ?,
+        // not yet widened by 001_widen_bulk_operations_status.sql on an instance
-                completed_at = NOW() WHERE operation_id = ?";
+        // deployed ahead of its migrations) must not turn a completed operation
-        $stmt = $this->conn->prepare($sql);
+        // into an error response.
-        $stmt->bind_param("siii", $status, $processed, $failed, $operationId);
+        try {
-        $stmt->execute();
+            $status = $failed > 0 ? 'completed_with_errors' : 'completed';
-        $stmt->close();
+            $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.
     */
@@ -277,9 +277,64 @@ include __DIR__ . '/layout_header.php';
      array_values($stats['by_category'] ?? [])
  ))) ?>;
  // ── Click-to-filter ────────────────────────────────────────────────────────
  // Charts navigate to the same URL filters the stat cards use.
  //
  // The status the click filters on has to be explicit rather than left to the
  // default: with no `status` param the controller falls back to the viewer's
  // default_status_filters preference, which can be anything, so the resulting
  // list would not necessarily match what the chart counted. StatsModel builds
  // by_priority and by_category with `status != 'Closed'`, while by_status spans
  // every status — so only the priority and category charts pin the open set.
  function openStatuses() {
    var all = window.TICKET_STATUSES || ['Open', 'Pending', 'In Progress', 'Closed'];
    return all.filter(function(s) { return s !== 'Closed'; }).join(',');
  }
  function gotoFilter(params) {
    var qs = new URLSearchParams();
    Object.keys(params).forEach(function(k) {
      if (params[k] !== null && params[k] !== undefined && params[k] !== '') qs.set(k, params[k]);
    });
    window.location.href = '/?' + qs.toString();
  }
  // Each chart maps a clicked label to a filter. Returns null when the label
  // can't be mapped, so the click is simply ignored.
  var CHART_FILTERS = {
    chartPriority: function(label) {
      var m = /^P(\d+)$/.exec(label);
      return m ? { priority: m[1], status: openStatuses() } : null;
    },
    chartStatus: function(label) {
      return label ? { status: label } : null;
    },
    chartCategory: function(label) {
      return label ? { category: label, status: openStatuses() } : null;
    }
  };
  function filterOnClick(canvasId) {
    return function(evt, elements, chart) {
      if (!elements || !elements.length) return;
      var label = chart.data.labels[elements[0].index];
      var mapper = CHART_FILTERS[canvasId];
      var params = mapper && mapper(label);
      if (params) gotoFilter(params);
    };
  }
  // Pointer cursor over clickable segments so the affordance is visible.
  function filterOnHover(evt, elements) {
    if (evt && evt.native && evt.native.target) {
      evt.native.target.style.cursor = (elements && elements.length) ? 'pointer' : 'default';
    }
  }
  function makeDonut(canvasId, data, colorMap) {
    var ctx = document.getElementById(canvasId);
    if (!ctx || !data.length) return;
    ctx.title = 'Click a segment to filter the ticket list';
    return new Chart(ctx, {
      type: 'doughnut',
      data: {
@@ -295,12 +350,14 @@ include __DIR__ . '/layout_header.php';
      },
      options: {
        responsive: true, maintainAspectRatio: false,
        onClick: filterOnClick(canvasId),
        onHover: filterOnHover,
        plugins: {
          legend: {
            position: 'bottom',
            labels: { color: '#8fa3b1', font: { family: 'monospace', size: 10 }, padding: 8, boxWidth: 10 }
          },
-          tooltip: { callbacks: { label: function(ctx) { return ' ' + ctx.label + ': ' + ctx.parsed; } } }
+          tooltip: { callbacks: { label: function(ctx) { return ' ' + ctx.label + ': ' + ctx.parsed + ' — click to filter'; } } }
        },
        cutout: '68%'
      }
@@ -310,6 +367,7 @@ include __DIR__ . '/layout_header.php';
  function makeBar(canvasId, data) {
    var ctx = document.getElementById(canvasId);
    if (!ctx || !data.length) return;
    ctx.title = 'Click a bar to filter the ticket list';
    return new Chart(ctx, {
      type: 'bar',
      data: {
@@ -323,7 +381,12 @@ include __DIR__ . '/layout_header.php';
      },
      options: {
        indexAxis: 'y', responsive: true, maintainAspectRatio: false,
-        plugins: { legend: { display: false } },
+        onClick: filterOnClick(canvasId),
        onHover: filterOnHover,
        plugins: {
          legend: { display: false },
          tooltip: { callbacks: { label: function(ctx) { return ' ' + ctx.parsed.x + ' — click to filter'; } } }
        },
        scales: {
          x: { ticks: { color: '#8fa3b1', font: { size: 10 } }, grid: { color: 'rgba(0,255,65,0.06)' } },
          y: { ticks: { color: '#8fa3b1', font: { family: 'monospace', size: 10 } }, grid: { display: false } }
@@ -1219,7 +1219,7 @@ document.addEventListener('DOMContentLoaded', function () {
                if (typeof parseMarkdown === 'function') {
                    list.querySelectorAll('.comment-text[data-markdown]').forEach(function (el) {
                        if (!el.dataset.rendered) {
-                            el.innerHTML = parseMarkdown(el.textContent);
+                            el.innerHTML = parseMarkdown(el.textContent.trim());
                            el.dataset.rendered = '1';
                        }
                    });
Author	SHA1	Message	Date
jaredandClaude Sonnet 5	6e0863449f	Trim comment text before persisting, not just for validation Security / PHP Security (semgrep) (push) Successful in 2m6s Details Lint / Deploy (push) Successful in 3s Details Lint / PHP (phpcs PSR-12) (push) Successful in 26s Details Lint / JS (eslint) (push) Successful in 11s Details Lint / PHP requirements (version + extensions) (push) Successful in 29s Details Lint / Notify on failure (push) Skipped Details add_comment.php computed a trimmed copy of comment_text only to check for empty input, then passed the original untrimmed $data through to CommentModel::addComment(), so any leading/trailing whitespace the user typed (or pasted) was written to ticket_comments.comment_text as-is. update_comment.php already trims before saving edits, so a comment could pass through this endpoint once with untrimmed text (creation) and be silently corrected the moment it was next edited — inconsistent storage that, combined with the markdown parser's line-anchored regexes (headings, tables, lists all match on ^), could make a markdown-enabled comment mis-render after a reload depending on whether its first line carried leading whitespace. Also trims in the "Load more comments" pagination re-render path in TicketView.php, matching the two on-load renderers in markdown.js so all three code paths that call parseMarkdown() on stored comment text treat leading whitespace consistently. Closes #18 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>	2026-08-31 21:03:00 -04:00
jared	1fb984e352	Merge #22 chart click-to-filter into main Lint / JS (eslint) (push) Successful in 19s Details Lint / PHP requirements (version + extensions) (push) Successful in 56s Details Lint / PHP (phpcs PSR-12) (push) Successful in 30s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 2m3s Details Lint / Deploy (push) Successful in 19s Details	2026-08-07 23:15:59 -04:00
jared	ce0ea66994	Charts: click a segment to filter the dashboard (#22 ) Lint / PHP (phpcs PSR-12) (push) Successful in 31s Details Lint / JS (eslint) (push) Successful in 21s Details Lint / PHP requirements (version + extensions) (push) Successful in 1m7s Details Lint / Notify on failure (push) Skipped Details Lint / Deploy (push) Successful in 3s Details Security / PHP Security (semgrep) (push) Successful in 2m0s Details All three charts (priority donut, status donut, category bar) now navigate to the same URL filters the stat cards already use, with a pointer cursor on hover, a title hint, and "click to filter" in the tooltip. The status each click applies is explicit rather than left to the default. With no `status` param the controller falls back to the viewer's default_status_filters preference, which can be anything, so the list would not necessarily match what the chart counted. StatsModel builds by_priority and by_category with `status != 'Closed'` while by_status spans every status, so only the priority and category charts pin the open set; the status chart filters on the clicked status alone (which is how clicking "Closed" works at all). Verified two ways: - 17/17 in headless chromium, driving the real chart script from this view with the Chart constructor stubbed, asserting the exact query each click produces and that a click hitting no segment navigates nowhere. - Against the live database, every segment's count equals the number of tickets its filter returns — 12/12 across all three charts — so the list you land on matches the number you clicked.	2026-08-07 23:15:51 -04:00
jared	4fd2c7ce7d	Merge #20 modal dismissal fix into main Lint / PHP (phpcs PSR-12) (push) Successful in 18s Details Lint / JS (eslint) (push) Successful in 9s Details Lint / PHP requirements (version + extensions) (push) Successful in 39s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 1m32s Details Lint / Deploy (push) Successful in 3s Details	2026-08-07 23:07:14 -04:00
jared	2ff7345a73	Dismissing the required-comment modal no longer looks like a close (#20 ) Lint / JS (eslint) (push) Successful in 15s Details Lint / PHP requirements (version + extensions) (push) Successful in 41s Details Security / PHP Security (semgrep) (push) Successful in 1m8s Details Lint / PHP (phpcs PSR-12) (push) Successful in 18s Details Lint / Notify on failure (push) Skipped Details Lint / Deploy (push) Successful in 2s Details A modal can be dismissed four ways: the ✕ button, Cancel, a backdrop click, or Escape. base.js handles the last two globally (a document click handler and registerKey('escape', closeAllModals)), so the status-change modal — which wired only the two buttons — never learned it had been dismissed. The status dropdown kept displaying the new status even though update_ticket.php was never called, so the ticket looked closed with no comment until a reload showed it still open. The same gap left every dynamically-inserted modal in the DOM when dismissed that way, so the next open inserted a duplicate id that shadowed the live one. - base.js closeModal now dispatches a bubbling lt:modalclose event (synced to web_template as bbec859), and _statusCommentModal treats it as "no comment". - ticket.js reverts the dropdown on any dismissal, guarded against the re-entry its own lt.modal.close() would otherwise cause. - dashboard.js gains openModalWithDismiss() so all seven dynamic modals plus the generic prompt modal tear down however they are dismissed. Verified in headless chromium against all four dismissal routes plus a confirm-with-comment control: 22/22. Against the pre-fix files the same test fails 6 assertions — backdrop and Escape leave the dropdown on "Closed *" with an orphaned overlay — so it reproduces the reported behaviour exactly.	2026-08-07 23:07:06 -04:00
jared	1de04d4908	Clear the markdown live preview after posting a comment Setting the textarea's .value programmatically does not fire an 'input' event, so updatePreview() never ran and the preview kept showing the just-posted comment's rendered markdown underneath an empty composer. (This change was already present in the working tree at the start of the session; committing it on its own rather than folding it into an unrelated fix.)	2026-08-07 23:06:36 -04:00
jared	153f9a7cef	Merge #23 light-mode ticket preview fix into main Lint / PHP (phpcs PSR-12) (push) Successful in 1m9s Details Lint / JS (eslint) (push) Successful in 9s Details Lint / PHP requirements (version + extensions) (push) Successful in 21s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 2m54s Details Lint / Deploy (push) Successful in 6s Details	2026-08-07 22:56:41 -04:00
jared	0a7201d754	Light mode: ticket-ID hover preview follows the theme (#23 ) Lint / PHP (phpcs PSR-12) (push) Successful in 17s Details Lint / JS (eslint) (push) Successful in 9s Details Lint / PHP requirements (version + extensions) (push) Successful in 22s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 1m15s Details Lint / Deploy (push) Successful in 2s Details .ticket-preview-popup used var(--lt-surface), which is not defined anywhere, so the background always fell through to the hardcoded #0a0e14. In light mode that left a near-black panel — and since the rule set no `color`, the inherited near-black body text was effectively invisible on it. The border was hardcoded neon green and the shadow a heavy rgba(0,0,0,0.5). Now uses --bg-card / --text-primary / --accent-green-border / --shadow-color, and .preview-id uses --accent-cyan instead of the undefined --lt-cyan. base.css gains the two tokens the light theme was missing (--accent-green-border and --shadow-color), synced from web_template 0d633bd. Verified with computed styles in headless chromium: light body-text contrast on the panel goes from invisible to 17.7:1, dark stays at 13.2:1, and the ID accent clears 3:1 in both themes.	2026-08-07 22:54:51 -04:00
jared	12ffd217bb	Merge #19 light-mode status dropdown fix into main Lint / PHP (phpcs PSR-12) (push) Successful in 29s Details Lint / JS (eslint) (push) Successful in 17s Details Lint / PHP requirements (version + extensions) (push) Successful in 40s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 1m14s Details Lint / Deploy (push) Successful in 3s Details	2026-08-07 22:51:22 -04:00
jared	a5b0655623	Light mode: status dropdown no longer renders dark (#19 ) Lint / PHP (phpcs PSR-12) (push) Successful in 23s Details Lint / JS (eslint) (push) Successful in 11s Details Lint / PHP requirements (version + extensions) (push) Successful in 22s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 1m11s Details Lint / Deploy (push) Successful in 2s Details Two separate causes, both light-mode-only: 1. base.css `.lt-select` sets `color-scheme: dark` on the element itself, which outranks the `color-scheme: light` the light theme sets on <html>, so the native dropdown popup kept dark chrome. The option list is also hardcoded #0d1117/#c9d1d9 with no light override. Fixed with light overrides for both (synced from web_template, where the same fix landed as 378a8cd). 2. ticket.css coloured the status select with var(--lt-success), --lt-amber, --lt-cyan and --lt-danger — none of which are defined anywhere in the project, so all four always fell through to hardcoded neon fallbacks. Now uses the --accent-* tokens, which carry the same hues and are redefined for light mode. The selectors also lead with .lt-select: at two classes they lost to base.css's `html[data-theme="light"] .lt-select` (0,2,1) and every status was repainted near-black in light mode. Verified with computed styles in headless chromium — all four statuses in both themes (8/8), plus the popup colour-scheme and option colours.	2026-08-07 22:51:16 -04:00
jared	fa5f347c08	Merge #21 workflow enforcement for bulk operations into main Lint / PHP (phpcs PSR-12) (push) Successful in 31s Details Lint / JS (eslint) (push) Successful in 12s Details Lint / PHP requirements (version + extensions) (push) Successful in 40s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 1m13s Details Lint / Deploy (push) Successful in 3s Details	2026-08-07 22:45:34 -04:00
jared	1d03800ab2	Widen bulk_operations.status so partial bulk results can be recorded (#21 ) Lint / PHP (phpcs PSR-12) (push) Successful in 26s Details Lint / JS (eslint) (push) Successful in 9s Details Lint / PHP requirements (version + extensions) (push) Successful in 22s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 1m8s Details Lint / Deploy (push) Successful in 3s Details Found while verifying #21 against the live schema: the model writes 'completed_with_errors' (21 chars) when a bulk operation finishes with per-ticket failures, but bulk_operations.status was varchar(20), so the write failed with "Data too long for column 'status'". This was latent — bulk status changes previously forced every transition through, so failed was always 0. Now that they honour the Workflow Designer, a partially-skipped batch is a normal outcome and hits it. - migrations/001 widens the column to varchar(32) (idempotent). - The baseline is updated to match, for fresh installs. - The bookkeeping UPDATE is wrapped in a try/catch: it runs after the ticket changes are committed, so an instance deployed ahead of its migrations must not turn a completed operation into an error response. Verified against the live database with a disposable-ticket harness: comment-required rejection changes nothing, undefined transitions are refused per ticket with a reason, allowed transitions still work, mixed batches apply the valid half, and an already-Closed ticket is a no-op.	2026-08-07 22:40:48 -04:00
jared	9d982ab73f	Bulk status/close: enforce Workflow Designer rules (#21 ) Lint / PHP (phpcs PSR-12) (push) Successful in 23s Details Lint / JS (eslint) (push) Successful in 9s Details Lint / PHP requirements (version + extensions) (push) Successful in 30s Details Lint / Notify on failure (push) Skipped Details Security / PHP Security (semgrep) (push) Successful in 1m15s Details Lint / Deploy (push) Successful in 2s Details 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.	2026-08-07 22:35:36 -04:00