Merge development into main: search filter merge, workflow dup rejection, recurring-ticket loss alert, preview debounce (#58, #62, #88, #108)
- Merge into current URL params instead of replacing them in Advanced Search (#58) - Reject duplicate workflow transitions with a clear error (#62) - Alert and record a lost recurring-ticket occurrence on creation failure (#88) - Debounce the live markdown preview (#108)
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
+7
-2
@@ -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');
|
||||
|
||||
@@ -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++;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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).
|
||||
*
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user