diff --git a/api/notifications.php b/api/notifications.php index e5c0586..ff22d70 100644 --- a/api/notifications.php +++ b/api/notifications.php @@ -138,7 +138,7 @@ $statusSql = "SELECT DISTINCT COALESCE(u.display_name, u.username, 'System') AS actor_name FROM audit_log al LEFT JOIN users u ON al.user_id = u.user_id - INNER JOIN ticket_watchers tw ON tw.ticket_id = CAST(al.entity_id AS UNSIGNED) AND tw.user_id = ? + INNER JOIN ticket_watchers tw ON tw.ticket_id = al.entity_id AND tw.user_id = ? WHERE al.action_type = 'update' AND al.entity_type = 'ticket' AND al.user_id != ? diff --git a/api/watch_ticket.php b/api/watch_ticket.php index 10c7f3e..40e9867 100644 --- a/api/watch_ticket.php +++ b/api/watch_ticket.php @@ -12,40 +12,43 @@ require_once dirname(__DIR__) . '/models/TicketModel.php'; $data = json_decode(file_get_contents('php://input'), true) ?? []; -$ticketId = isset($_GET['ticket_id']) - ? (int)$_GET['ticket_id'] - : (int)($data['ticket_id'] ?? 0); +$ticketIdRaw = isset($_GET['ticket_id']) ? $_GET['ticket_id'] : ($data['ticket_id'] ?? ''); if ($_SERVER['REQUEST_METHOD'] === 'POST') { - $ticketId = (int)($data['ticket_id'] ?? 0); - $action = $data['action'] ?? ''; + $ticketIdRaw = $data['ticket_id'] ?? ''; + $action = $data['action'] ?? ''; - if ($ticketId <= 0 || !in_array($action, ['watch', 'unwatch'], true)) { + if ($ticketIdRaw === '' || !in_array($action, ['watch', 'unwatch'], true)) { http_response_code(400); echo json_encode(['success' => false, 'error' => 'Invalid parameters']); exit; } $ticketModel = new TicketModel($conn); - $ticket = $ticketModel->getTicketById($ticketId); + $ticket = $ticketModel->getTicketById((string)$ticketIdRaw); if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $currentUser)) { http_response_code(404); echo json_encode(['success' => false, 'error' => 'Ticket not found']); exit; } + // Use the canonical ticket_id string from the fetched ticket row, not the + // raw request value, so ticket_watchers always stores exactly what's in + // tickets.ticket_id. + $ticketId = $ticket['ticket_id']; + if ($action === 'watch') { $stmt = $conn->prepare( "INSERT IGNORE INTO ticket_watchers (ticket_id, user_id) VALUES (?, ?)" ); - $stmt->bind_param("ii", $ticketId, $userId); + $stmt->bind_param("si", $ticketId, $userId); $stmt->execute(); $stmt->close(); } else { $stmt = $conn->prepare( "DELETE FROM ticket_watchers WHERE ticket_id = ? AND user_id = ?" ); - $stmt->bind_param("ii", $ticketId, $userId); + $stmt->bind_param("si", $ticketId, $userId); $stmt->execute(); $stmt->close(); } @@ -54,7 +57,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { $countStmt = $conn->prepare( "SELECT COUNT(*) as cnt FROM ticket_watchers WHERE ticket_id = ?" ); - $countStmt->bind_param("i", $ticketId); + $countStmt->bind_param("s", $ticketId); $countStmt->execute(); $count = (int)$countStmt->get_result()->fetch_assoc()['cnt']; $countStmt->close(); @@ -73,7 +76,7 @@ if ($_SERVER['REQUEST_METHOD'] !== 'GET') { exit; } -if ($ticketId <= 0) { +if ($ticketIdRaw === '') { http_response_code(400); echo json_encode(['success' => false, 'error' => 'ticket_id required']); exit; @@ -83,17 +86,19 @@ if ($ticketId <= 0) { // restricted ticket's watcher list and count aren't disclosed (the POST path // already checks this). $ticketModel = new TicketModel($conn); -$ticket = $ticketModel->getTicketById($ticketId); +$ticket = $ticketModel->getTicketById((string)$ticketIdRaw); if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $currentUser)) { http_response_code(404); echo json_encode(['success' => false, 'error' => 'Ticket not found']); exit; } +$ticketId = $ticket['ticket_id']; + $watchingStmt = $conn->prepare( "SELECT COUNT(*) as cnt FROM ticket_watchers WHERE ticket_id = ? AND user_id = ?" ); -$watchingStmt->bind_param("ii", $ticketId, $userId); +$watchingStmt->bind_param("si", $ticketId, $userId); $watchingStmt->execute(); $watching = (bool)$watchingStmt->get_result()->fetch_assoc()['cnt']; $watchingStmt->close(); @@ -107,7 +112,7 @@ $watchersStmt = $conn->prepare( ORDER BY tw.created_at ASC LIMIT 6" ); -$watchersStmt->bind_param("i", $ticketId); +$watchersStmt->bind_param("s", $ticketId); $watchersStmt->execute(); $watchersResult = $watchersStmt->get_result(); $watchers = []; @@ -118,7 +123,7 @@ $watchersStmt->close(); // True watcher count (the list above is capped at 6 for the avatar group) $countStmt = $conn->prepare("SELECT COUNT(*) AS cnt FROM ticket_watchers WHERE ticket_id = ?"); -$countStmt->bind_param("i", $ticketId); +$countStmt->bind_param("s", $ticketId); $countStmt->execute(); $count = (int)$countStmt->get_result()->fetch_assoc()['cnt']; $countStmt->close(); diff --git a/create_ticket_api.php b/create_ticket_api.php index 62b6fcc..e4a85a9 100644 --- a/create_ticket_api.php +++ b/create_ticket_api.php @@ -5,6 +5,9 @@ header('Content-Type: application/json'); error_reporting(E_ALL); ini_set('display_errors', 0); +require_once __DIR__ . '/middleware/RateLimitMiddleware.php'; +RateLimitMiddleware::apply('api'); + // Load environment variables with error check $envFile = __DIR__ . '/.env'; if (!file_exists($envFile)) { diff --git a/helpers/NotificationHelper.php b/helpers/NotificationHelper.php index 1afceac..af4cf91 100644 --- a/helpers/NotificationHelper.php +++ b/helpers/NotificationHelper.php @@ -204,9 +204,9 @@ class NotificationHelper return; } if ($excludeUserId !== null) { - $stmt->bind_param("ii", $ticketId, $excludeUserId); + $stmt->bind_param("si", $ticketId, $excludeUserId); } else { - $stmt->bind_param("i", $ticketId); + $stmt->bind_param("s", $ticketId); } $stmt->execute(); $result = $stmt->get_result(); diff --git a/migrations/000_baseline.sql b/migrations/000_baseline.sql index f5ffec9..02781a9 100644 --- a/migrations/000_baseline.sql +++ b/migrations/000_baseline.sql @@ -243,11 +243,12 @@ CREATE TABLE IF NOT EXISTS `ticket_templates` ( -- ============ ticket_watchers ============ CREATE TABLE IF NOT EXISTS `ticket_watchers` ( - `ticket_id` int(11) NOT NULL, + `ticket_id` varchar(9) NOT NULL, `user_id` int(11) NOT NULL, `created_at` timestamp NOT NULL DEFAULT current_timestamp(), PRIMARY KEY (`ticket_id`,`user_id`), - KEY `idx_watcher_user` (`user_id`) + KEY `idx_watcher_user` (`user_id`), + CONSTRAINT `fk_watchers_ticket_id` FOREIGN KEY (`ticket_id`) REFERENCES `tickets` (`ticket_id`) ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; -- ============ tickets ============ diff --git a/migrations/004_fix_ticket_watchers_type.sql b/migrations/004_fix_ticket_watchers_type.sql new file mode 100644 index 0000000..c3d4d7e --- /dev/null +++ b/migrations/004_fix_ticket_watchers_type.sql @@ -0,0 +1,28 @@ +-- Fix ticket_watchers.ticket_id type mismatch and missing FK to tickets +-- +-- ticket_watchers.ticket_id was int(11), while every other satellite table +-- (ticket_comments, ticket_attachments, ticket_dependencies, +-- custom_field_values) stores it as varchar(9)/varchar(10) matching +-- tickets.ticket_id. There was also no FK constraint at all, unlike every +-- other satellite table, so orphaned watcher rows could never be caught by +-- referential integrity. Ticket IDs are always 9-digit numeric strings +-- (see TicketModel::create's sprintf('%09d', ...)), so the int -> varchar(9) +-- conversion below is lossless for real data. +-- +-- Safe to re-run. + +-- Remove any watcher rows that no longer point at a real ticket (possible +-- today precisely because there was no FK to prevent it) before adding the +-- constraint, since orphans would make the ADD CONSTRAINT below fail. +DELETE tw FROM `ticket_watchers` tw + LEFT JOIN `tickets` t ON tw.`ticket_id` = t.`ticket_id` + WHERE t.`ticket_id` IS NULL; + +ALTER TABLE `ticket_watchers` + MODIFY COLUMN `ticket_id` varchar(9) NOT NULL; + +ALTER TABLE `ticket_watchers` + DROP FOREIGN KEY IF EXISTS `fk_watchers_ticket_id`; + +ALTER TABLE `ticket_watchers` + ADD CONSTRAINT `fk_watchers_ticket_id` FOREIGN KEY (`ticket_id`) REFERENCES `tickets` (`ticket_id`) ON DELETE CASCADE; diff --git a/migrations/migrate.php b/migrations/migrate.php index a725242..b34a8b4 100644 --- a/migrations/migrate.php +++ b/migrations/migrate.php @@ -46,6 +46,23 @@ if (!$conn->query($createTable)) { exit(1); } +// Tracks per-statement progress within a migration file. MySQL DDL statements +// (ALTER/CREATE TABLE, etc.) cause an implicit commit, so begin_transaction()/ +// rollback() around a whole file can't actually undo DDL already executed +// earlier in that same file. This table lets a re-run after a partial failure +// resume from the statement after the last one that succeeded, instead of +// re-executing already-applied DDL and wedging on "already exists" errors. +$createProgressTable = "CREATE TABLE IF NOT EXISTS migration_progress ( + filename VARCHAR(255) NOT NULL PRIMARY KEY, + last_statement_index INT NOT NULL, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP +)"; + +if (!$conn->query($createProgressTable)) { + echo "Error: Could not create migration_progress table: " . $conn->error . "\n"; + exit(1); +} + // Get list of completed migrations $completed = []; $result = $conn->query("SELECT filename FROM migrations ORDER BY id"); @@ -114,47 +131,87 @@ foreach ($pending as $file) { continue; } - // Execute migration - handle multiple statements - $conn->begin_transaction(); - + // Execute migration statement-by-statement, tracking progress as we go. + // No begin_transaction()/rollback() here: DDL statements auto-commit in + // MySQL/MariaDB regardless, so a transaction wrapper around the whole + // file would only create the illusion of atomicity while giving no real + // protection. Instead, each statement commits immediately (autocommit), + // and its index is durably recorded so a later re-run can resume exactly + // where a previous run left off rather than re-executing already-applied + // DDL. try { // Split by semicolon but respect statements properly // Note: This doesn't handle semicolons in strings, but our migrations are simple - $statements = array_filter( + $statements = array_values(array_filter( array_map('trim', explode(';', $sql)), function($stmt) { // Remove comments and check if there's actual SQL $cleaned = preg_replace('/--.*$/m', '', $stmt); return !empty(trim($cleaned)); } - ); + )); + + $resumeFrom = 0; + $progressStmt = $conn->prepare( + "SELECT last_statement_index FROM migration_progress WHERE filename = ?" + ); + $progressStmt->bind_param('s', $filename); + $progressStmt->execute(); + $progressRow = $progressStmt->get_result()->fetch_assoc(); + $progressStmt->close(); + if ($progressRow) { + $resumeFrom = (int)$progressRow['last_statement_index'] + 1; + echo "\n Resuming from statement " . ($resumeFrom + 1) . " of " . count($statements) + . " after a previous partial failure... "; + } + + foreach ($statements as $index => $statement) { + if ($index < $resumeFrom) { + continue; + } - foreach ($statements as $statement) { if (!$conn->query($statement)) { // Some "errors" are acceptable (like "index already exists") $error = $conn->error; if (strpos($error, 'Duplicate key name') !== false || strpos($error, 'already exists') !== false) { // Index already exists, that's fine - continue; + } else { + throw new Exception($error); } - throw new Exception($error); } + + // Record progress after every statement so a later run can + // resume from here even if a subsequent statement fails. + $upsert = $conn->prepare( + "INSERT INTO migration_progress (filename, last_statement_index) VALUES (?, ?) + ON DUPLICATE KEY UPDATE last_statement_index = VALUES(last_statement_index)" + ); + $upsert->bind_param('si', $filename, $index); + $upsert->execute(); + $upsert->close(); } - // Record the migration + // Record the migration as fully complete and clear its progress marker $stmt = $conn->prepare("INSERT INTO migrations (filename) VALUES (?)"); $stmt->bind_param('s', $filename); if (!$stmt->execute()) { throw new Exception("Could not record migration: " . $conn->error); } - $conn->commit(); + $clearProgress = $conn->prepare("DELETE FROM migration_progress WHERE filename = ?"); + $clearProgress->bind_param('s', $filename); + $clearProgress->execute(); + $clearProgress->close(); + echo "OK\n"; $success++; } catch (Exception $e) { - $conn->rollback(); + // Nothing to roll back: every statement up to the failure already + // committed (DDL implicitly, everything else via autocommit). The + // progress marker recorded above reflects exactly how far this file + // got, so the next run will resume right after the last success. echo "FAILED (" . $e->getMessage() . ")\n"; $failed++; } diff --git a/models/TicketModel.php b/models/TicketModel.php index b64c837..7e05e42 100644 --- a/models/TicketModel.php +++ b/models/TicketModel.php @@ -726,7 +726,10 @@ class TicketModel $groupConditions = []; foreach ($userGroups as $group) { $groupConditions[] = "FIND_IN_SET(?, REPLACE(t.visibility_groups, ' ', ''))"; - $params[] = $group; + // Strip spaces from the bound value too, matching the REPLACE() + // applied to the column, so a group name like "IT Support" is + // normalized the same way on both sides of the comparison. + $params[] = str_replace(' ', '', $group); $types .= 's'; } $conditions[] = "(t.visibility = 'internal' AND (" . implode(' OR ', $groupConditions) . "))";