From 6d68af40e7d8f06df476a948fb06948cdafde948 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 14:28:12 -0400 Subject: [PATCH 1/4] Merge into current URL params instead of replacing them in Advanced Search (#58) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same root pattern as the earlier chart click-to-filter bug (#29): performAdvancedSearch() built a brand-new URLSearchParams from only the form's own fields and navigated to it, silently dropping any active filter the form doesn't represent (e.g. a category/type filter applied via a dashboard quick-filter pill or stats-widget click). populateCurrentFilters() also only read search/status back out of the URL into the form, not the date ranges/priority range/user fields the form does control. Fixed performAdvancedSearch() to start from the current URL's params and only set/clear the ones this form actually represents, leaving everything else untouched. Also fixed populateCurrentFilters() to restore all of those fields, not just search/status — without that, reopening the modal and submitting without touching anything would now silently wipe date/priority/user filters that were active but shown blank in the form (a new foot-gun the first fix alone would have introduced). Verified via jsdom: category/type/sort params not represented in the form survive a search submission; page resets to 1; and reopening the modal with an active created_from filter correctly restores it into the form and preserves it on a no-op resubmit. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- assets/js/advanced-search.js | 43 +++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/assets/js/advanced-search.js b/assets/js/advanced-search.js index 09c42f9..4572437 100644 --- a/assets/js/advanced-search.js +++ b/assets/js/advanced-search.js @@ -61,25 +61,46 @@ function populateCurrentFilters() { const urlParams = new URLSearchParams(window.location.search); // Search text - if (urlParams.has('search')) { - document.getElementById('adv-search-text').value = urlParams.get('search'); - } + document.getElementById('adv-search-text').value = urlParams.get('search') || ''; // Status - if (urlParams.has('status')) { - const statuses = urlParams.get('status').split(','); - const statusSelect = document.getElementById('adv-status'); - Array.from(statusSelect.options).forEach(option => { - option.selected = statuses.includes(option.value); - }); - } + const statuses = urlParams.has('status') ? urlParams.get('status').split(',') : []; + const statusSelect = document.getElementById('adv-status'); + Array.from(statusSelect.options).forEach(option => { + option.selected = statuses.includes(option.value); + }); + + // Date ranges + document.getElementById('adv-created-from').value = urlParams.get('created_from') || ''; + document.getElementById('adv-created-to').value = urlParams.get('created_to') || ''; + document.getElementById('adv-updated-from').value = urlParams.get('updated_from') || ''; + document.getElementById('adv-updated-to').value = urlParams.get('updated_to') || ''; + + // Priority range + document.getElementById('adv-priority-min').value = urlParams.get('priority_min') || ''; + document.getElementById('adv-priority-max').value = urlParams.get('priority_max') || ''; + + // Users + document.getElementById('adv-created-by').value = urlParams.get('created_by') || ''; + document.getElementById('adv-assigned-to').value = urlParams.get('assigned_to') || ''; } // Perform advanced search function performAdvancedSearch(event) { event.preventDefault(); - const params = new URLSearchParams(); + // Start from the CURRENT URL's params, not a fresh set, so a filter this + // form doesn't represent (e.g. a category/type filter applied via a + // dashboard quick-filter pill or stats-widget click) isn't silently + // dropped on submit. Only the params this form actually controls are + // set/cleared below; everything else passes through untouched. + const params = new URLSearchParams(window.location.search); + const advParams = [ + 'search', 'created_from', 'created_to', 'updated_from', 'updated_to', + 'status', 'priority_min', 'priority_max', 'created_by', 'assigned_to', + ]; + advParams.forEach(key => params.delete(key)); + params.delete('page'); // filters changed — reset to page 1 // Search text const searchText = document.getElementById('adv-search-text').value.trim(); From 86ef91abcbbadf0b4ae780d50cfddfb8578a1347 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 14:28:21 -0400 Subject: [PATCH 2/4] Reject duplicate workflow transitions with a clear error (#62) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit status_transitions already has a DB-level UNIQUE KEY on (from_status, to_status), so a genuine duplicate pair was never actually possible to insert — but hitting that constraint raw surfaced as an opaque "An internal error occurred" to the admin instead of a clear message, since manage_workflows.php only validated from_status !== to_status before attempting the insert/update. Added an explicit existence check before insert/update in both the POST and PUT handlers (excluding the row's own ID on update), so the common case — an admin re-adding or renaming into a pair that already exists — gets a specific 409 with the conflicting pair named, instead of a generic 500. Also added ORDER BY transition_id to WorkflowModel::getAllTransitions() as a defense-in-depth backstop: since it collapses rows into a PHP array keyed by [from_status][to_status] with no defined winner otherwise, if the DB constraint were ever weakened or bypassed, this at least makes which row wins deterministic (most recently created). Verified against a real running server + real MariaDB: creating a duplicate active pair, a duplicate inactive pair, and updating a different row into an existing pair are all correctly rejected with the friendly message; updating a row to keep its own existing pair succeeds; and a genuinely different pair still creates normally. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- api/manage_workflows.php | 51 ++++++++++++++++++++++++++++++++++++++-- models/WorkflowModel.php | 8 ++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/api/manage_workflows.php b/api/manage_workflows.php index 90ba6dd..9ce5c6e 100644 --- a/api/manage_workflows.php +++ b/api/manage_workflows.php @@ -97,13 +97,39 @@ try { exit; } + $wf_active = (int)($data['is_active'] ?? 1); + + // status_transitions already has a DB-level UNIQUE KEY on + // (from_status, to_status) (regardless of is_active), so a + // duplicate pair can't actually be inserted — but hitting that + // constraint raw surfaces as an opaque "internal error occurred" + // to the admin instead of a clear message. Check first so the + // common case (an admin re-adding a pair that already exists) + // gets a friendly, specific error. + $dupCheck = $conn->prepare( + "SELECT transition_id FROM status_transitions WHERE from_status = ? AND to_status = ?" + ); + $dupCheck->bind_param('ss', $data['from_status'], $data['to_status']); + $dupCheck->execute(); + if ($dupCheck->get_result()->fetch_assoc()) { + $dupCheck->close(); + http_response_code(409); + echo json_encode([ + 'success' => false, + 'error' => 'A transition already exists for ' + . $data['from_status'] . ' → ' . $data['to_status'] + . ' — edit that row instead of creating a duplicate.', + ]); + exit; + } + $dupCheck->close(); + $stmt = $conn->prepare("INSERT INTO status_transitions (from_status, to_status, requires_comment, requires_admin, is_active) VALUES (?, ?, ?, ?, ?)"); $wf_from = $data['from_status']; $wf_to = $data['to_status']; $wf_comment = (int)($data['requires_comment'] ?? 0); $wf_admin = (int)($data['requires_admin'] ?? 0); - $wf_active = (int)($data['is_active'] ?? 1); $stmt->bind_param('ssiii', $wf_from, $wf_to, $wf_comment, $wf_admin, $wf_active); if ($stmt->execute()) { @@ -149,6 +175,28 @@ try { exit; } + $wf_active = (int)($data['is_active'] ?? 1); + + // Same duplicate-pair guard as create, excluding this row itself. + $dupCheck = $conn->prepare( + "SELECT transition_id FROM status_transitions + WHERE from_status = ? AND to_status = ? AND transition_id != ?" + ); + $dupCheck->bind_param('ssi', $data['from_status'], $data['to_status'], $id); + $dupCheck->execute(); + if ($dupCheck->get_result()->fetch_assoc()) { + $dupCheck->close(); + http_response_code(409); + echo json_encode([ + 'success' => false, + 'error' => 'A transition already exists for ' + . $data['from_status'] . ' → ' . $data['to_status'] + . ' — edit that row instead of creating a duplicate.', + ]); + exit; + } + $dupCheck->close(); + $stmt = $conn->prepare("UPDATE status_transitions SET from_status = ?, to_status = ?, requires_comment = ?, requires_admin = ?, is_active = ? WHERE transition_id = ?"); @@ -156,7 +204,6 @@ try { $wf_to = $data['to_status']; $wf_comment = (int)($data['requires_comment'] ?? 0); $wf_admin = (int)($data['requires_admin'] ?? 0); - $wf_active = (int)($data['is_active'] ?? 1); $stmt->bind_param('ssiiii', $wf_from, $wf_to, $wf_comment, $wf_admin, $wf_active, $id); $success = $stmt->execute(); diff --git a/models/WorkflowModel.php b/models/WorkflowModel.php index 473bfed..7928871 100644 --- a/models/WorkflowModel.php +++ b/models/WorkflowModel.php @@ -31,9 +31,15 @@ class WorkflowModel return $cached; } + // ORDER BY makes which row wins deterministic (most recently created, + // by transition_id) in the pathological case where two active rows + // exist for the same (from_status, to_status) pair — manage_workflows.php + // now rejects creating that duplicate going forward, but this is a + // defense-in-depth backstop against any duplicate already in the DB. $sql = "SELECT from_status, to_status, requires_comment, requires_admin FROM status_transitions - WHERE is_active = TRUE"; + WHERE is_active = TRUE + ORDER BY transition_id ASC"; $result = $this->conn->query($sql); if (!$result) { From a4828c1b7b3554c8e392e8e0a8cac3fb501205b5 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 14:28:31 -0400 Subject: [PATCH 3/4] Alert and record a lost recurring-ticket occurrence on creation failure (#88) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RecurringTicketModel::claimForRun() deliberately advances next_run_at before TicketModel::createTicket() runs, to prevent duplicate-ticket floods if creation fails partway and the cron retries. The tradeoff: if createTicket() then fails, that specific occurrence is gone forever with no record anywhere an admin would normally look — the catch block only wrote a line to stdout/the cron log. Added recordMissedOccurrence(), called from both the "createTicket() returned success:false" branch and the exception catch, which writes an audit_log entry (entity_type='recurring_ticket', action_type='error') and fires a new NotificationHelper::sendSystemAlert() — a generic operational alert (unlike the ticket-specific notification methods, it has no associated ticket) sent to the shared MATRIX_NOTIFY_USERS list regardless of any per-event toggle, so a silently-skipped recurring ticket surfaces immediately instead of requiring someone to grep cron logs. Verified against real MariaDB and a real webhook-capturing server: calling the recorder writes the audit_log row with the failure reason and schedule details, and fires the Matrix alert with the same information. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- cron/create_recurring_tickets.php | 40 ++++++++++++++++++++++++++++++- helpers/NotificationHelper.php | 17 +++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/cron/create_recurring_tickets.php b/cron/create_recurring_tickets.php index f2b851d..49ab526 100644 --- a/cron/create_recurring_tickets.php +++ b/cron/create_recurring_tickets.php @@ -29,6 +29,38 @@ function logMessage($message) echo "[" . date('Y-m-d H:i:s') . "] " . $message . "\n"; } +/** + * Record a recurring-ticket occurrence that was claimed (next_run_at already + * advanced to the next future run) but then failed to actually produce a + * ticket. That claim-then-fail ordering is deliberate — it stops a failing + * creation from re-firing and flooding duplicates on every subsequent cron + * tick — but means this specific occurrence has no other record anywhere an + * admin would normally look: no audit_log entry (nothing was created), no + * Matrix "ticket created" alert, no failure table. Without this, it's simply + * gone, silently, forever. + */ +function recordMissedOccurrence($auditLog, $recurring, $reason) +{ + $auditLog->log( + $recurring['created_by'], + 'error', + 'recurring_ticket', + (string)$recurring['recurring_id'], + [ + 'reason' => $reason, + 'title_template' => $recurring['title_template'], + 'schedule_type' => $recurring['schedule_type'], + ] + ); + + NotificationHelper::sendSystemAlert( + "Recurring ticket occurrence lost: schedule #{$recurring['recurring_id']} " + . "(\"{$recurring['title_template']}\") was claimed for this run but ticket " + . "creation failed, so this occurrence will not be created or retried.", + ['reason' => $reason, 'recurring_id' => $recurring['recurring_id']] + ); +} + logMessage("Starting recurring tickets cron job"); try { @@ -100,11 +132,17 @@ try { $created++; } else { - logMessage("ERROR: Failed to create ticket - " . ($result['error'] ?? 'Unknown error')); + $reason = $result['error'] ?? 'Unknown error'; + logMessage("ERROR: Failed to create ticket - " . $reason); + recordMissedOccurrence($auditLog, $recurring, $reason); $errors++; } } catch (Exception $e) { logMessage("ERROR: Exception processing recurring ticket - " . $e->getMessage()); + // claimForRun() already advanced next_run_at before this point, so + // this occurrence is permanently gone unless recorded somewhere an + // admin would actually look — a cron log line alone doesn't count. + recordMissedOccurrence($auditLog, $recurring, $e->getMessage()); $errors++; } } diff --git a/helpers/NotificationHelper.php b/helpers/NotificationHelper.php index 4ffa01a..f4590e5 100644 --- a/helpers/NotificationHelper.php +++ b/helpers/NotificationHelper.php @@ -59,6 +59,23 @@ class NotificationHelper // ─── Public event methods ───────────────────────────────────────────────── + /** + * Generic operational alert with no associated ticket (e.g. a recurring + * schedule whose ticket creation failed after its next_run_at was + * already advanced, so the missed occurrence has no other record an + * admin would normally see). Always sent to the shared + * MATRIX_NOTIFY_USERS list, regardless of any per-event notify toggle. + */ + public static function sendSystemAlert(string $message, array $context = []): void + { + self::fire(array_merge([ + 'event' => 'system_alert', + 'message' => $message, + ], $context, [ + 'notify_users' => self::notifyUsers(), + ])); + } + /** * New ticket created (manual or automated/API). * From 6bd1bb082aca9c8d5d68bae8b2e24c049d24ee3c Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 14:28:39 -0400 Subject: [PATCH 4/4] Debounce the live markdown preview (#108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updatePreview() was bound directly to the comment textarea's 'input' event with no debounce, re-running the full markdown parser (regex passes for headings, tables, links, footnotes, etc.) on every single keystroke. Wrapped it with the existing lt.debounce() helper (150ms) — the initial preview render on enabling the toggle still happens immediately; only the per-keystroke live updates are debounced. Verified via jsdom with real timers: 10 rapid keystrokes within the debounce window produce exactly one parse call instead of ten. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- assets/js/ticket.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/assets/js/ticket.js b/assets/js/ticket.js index bc0aea0..f40cab2 100644 --- a/assets/js/ticket.js +++ b/assets/js/ticket.js @@ -357,12 +357,17 @@ function togglePreview() { if (isPreviewEnabled) { preview.innerHTML = parseMarkdown(textarea.value); - textarea.addEventListener('input', updatePreview); + textarea.addEventListener('input', debouncedUpdatePreview); } else { - textarea.removeEventListener('input', updatePreview); + textarea.removeEventListener('input', debouncedUpdatePreview); } } +// Re-running the full markdown parser on every single keystroke is wasted +// work while the user is still mid-word; 150ms debounce keeps the preview +// feeling live without re-parsing on every keystroke. +const debouncedUpdatePreview = window.lt ? lt.debounce(updatePreview, 150) : updatePreview; + function updatePreview() { const textarea = document.getElementById('newComment'); const previewDiv = document.getElementById('markdownPreview');