Fix data-layer bugs: bind_param fatals, ticket_id bindings, cache poisoning
- CustomFieldModel: assign ?? fallbacks to variables before bind_param
(by-reference args cannot be ?? expressions; fatal on PHP 8.2, custom
fields were uncreatable/uneditable)
- RecurringTicketModel::create: fix swapped bind type for schedule_type
(enum bound as int coerced 'daily' to 0, breaking the cron)
- TicketModel/CommentModel: bind varchar ticket_id as string not int so
the unique index is usable and leading-zero IDs match; ticket_watchers
(int column) left as integer
- TicketModel::deleteTicket: delete from custom_field_values (real table)
not the nonexistent ticket_custom_fields
- TicketModel search: honor literal '0'; never emit AGAINST('*') on
all-special-char input (fall back to LIKE)
- TicketModel::updateTicket: disambiguate not-found vs no-op vs genuine
optimistic-lock conflict on zero affected rows
- WorkflowModel: do not cache transitions/statuses on DB failure (a
transient error no longer blocks all status changes for the TTL)
- DependencyModel: filter linked tickets by visibility (new optional user
context params) to stop confidential metadata leaking via dependencies
- BulkOperationsModel: validate status/priority/assignee before mutating
- AuditLogModel: gate getClientIP forwarded headers on trusted proxies;
add missing action/entity types so audit-log filters work
- WorkflowModel: add transitionRequiresComment() accessor for enforcement
- CommentModel: stop leaking raw DB errors to clients (log instead)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+39
-17
@@ -19,13 +19,15 @@ class AuditLogModel
|
||||
/** @var array Allowed action types for filtering */
|
||||
private const VALID_ACTION_TYPES = [
|
||||
'create', 'update', 'delete', 'view', 'security_event',
|
||||
'login', 'logout', 'assign', 'comment', 'bulk_update'
|
||||
'login', 'logout', 'assign', 'unassign', 'comment', 'mention',
|
||||
'revoke', 'attachment_upload', 'attachment_delete', 'bulk_update'
|
||||
];
|
||||
|
||||
/** @var array Allowed entity types for filtering */
|
||||
private const VALID_ENTITY_TYPES = [
|
||||
'ticket', 'comment', 'user', 'api_key', 'security',
|
||||
'template', 'attachment', 'group'
|
||||
'template', 'attachment', 'ticket_attachments', 'group',
|
||||
'dependency', 'workflow_transition'
|
||||
];
|
||||
|
||||
public function __construct($conn)
|
||||
@@ -327,24 +329,44 @@ class AuditLogModel
|
||||
*/
|
||||
private function getClientIP()
|
||||
{
|
||||
$ipAddress = '';
|
||||
$remoteAddr = $_SERVER['REMOTE_ADDR'] ?? '';
|
||||
|
||||
// Check for proxy headers
|
||||
if (!empty($_SERVER['HTTP_CF_CONNECTING_IP'])) {
|
||||
// Cloudflare
|
||||
$ipAddress = $_SERVER['HTTP_CF_CONNECTING_IP'];
|
||||
} elseif (!empty($_SERVER['HTTP_X_REAL_IP'])) {
|
||||
// Nginx proxy
|
||||
$ipAddress = $_SERVER['HTTP_X_REAL_IP'];
|
||||
} elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
|
||||
// Standard proxy header
|
||||
$ipAddress = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR'])[0];
|
||||
} elseif (!empty($_SERVER['REMOTE_ADDR'])) {
|
||||
// Direct connection
|
||||
$ipAddress = $_SERVER['REMOTE_ADDR'];
|
||||
// Forwarded/proxy headers are client-controlled, so only believe them when
|
||||
// the request actually came from a trusted reverse proxy (same rule as
|
||||
// RateLimitMiddleware). Otherwise a client could forge its audit-log IP.
|
||||
$trusted = $GLOBALS['config']['TRUSTED_PROXIES'] ?? [];
|
||||
if (empty($trusted) || !in_array($remoteAddr, $trusted, true)) {
|
||||
return trim($remoteAddr);
|
||||
}
|
||||
|
||||
return trim($ipAddress);
|
||||
// Cloudflare sets CF-Connecting-IP to the real client.
|
||||
if (
|
||||
!empty($_SERVER['HTTP_CF_CONNECTING_IP'])
|
||||
&& filter_var($_SERVER['HTTP_CF_CONNECTING_IP'], FILTER_VALIDATE_IP)
|
||||
) {
|
||||
return trim($_SERVER['HTTP_CF_CONNECTING_IP']);
|
||||
}
|
||||
|
||||
// The trusted proxy appends the connecting client to X-Forwarded-For, so
|
||||
// the RIGHTMOST entry is the IP it observed (any client-supplied prefix is
|
||||
// not trustworthy).
|
||||
if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
|
||||
$ips = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']);
|
||||
$ip = trim(end($ips));
|
||||
if (filter_var($ip, FILTER_VALIDATE_IP)) {
|
||||
return $ip;
|
||||
}
|
||||
}
|
||||
|
||||
// X-Real-IP is set by the proxy itself.
|
||||
if (
|
||||
!empty($_SERVER['HTTP_X_REAL_IP'])
|
||||
&& filter_var($_SERVER['HTTP_X_REAL_IP'], FILTER_VALIDATE_IP)
|
||||
) {
|
||||
return trim($_SERVER['HTTP_X_REAL_IP']);
|
||||
}
|
||||
|
||||
return trim($remoteAddr);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -77,6 +77,15 @@ class BulkOperationsModel
|
||||
|
||||
$ticketIds = explode(',', $operation['ticket_ids']);
|
||||
$parameters = $operation['parameters'] ? json_decode($operation['parameters'], true) : [];
|
||||
|
||||
// Validate operation parameters up front so invalid values (out-of-range
|
||||
// priority, unknown status, nonexistent assignee) are rejected cleanly
|
||||
// instead of corrupting tickets or throwing mid-transaction.
|
||||
$paramError = $this->validateOperationParameters($operation['operation_type'], is_array($parameters) ? $parameters : []);
|
||||
if ($paramError !== null) {
|
||||
return ['processed' => 0, 'failed' => count($ticketIds), 'error' => $paramError];
|
||||
}
|
||||
|
||||
$processed = 0;
|
||||
$failed = 0;
|
||||
$errors = [];
|
||||
@@ -297,6 +306,63 @@ class BulkOperationsModel
|
||||
return $result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate the parameters for a bulk operation before any ticket is mutated.
|
||||
*
|
||||
* @return string|null Error message, or null if the parameters are valid
|
||||
*/
|
||||
private function validateOperationParameters(string $type, array $parameters): ?string
|
||||
{
|
||||
switch ($type) {
|
||||
case 'bulk_priority':
|
||||
if (!isset($parameters['priority'])) {
|
||||
return 'Missing priority parameter';
|
||||
}
|
||||
$priority = $parameters['priority'];
|
||||
// tickets.priority has a CHECK constraint (between 1 and 6).
|
||||
if (!is_numeric($priority) || (int)$priority < 1 || (int)$priority > 6) {
|
||||
return 'Invalid priority: must be between 1 and 6';
|
||||
}
|
||||
break;
|
||||
|
||||
case 'bulk_status':
|
||||
if (!isset($parameters['status'])) {
|
||||
return 'Missing status parameter';
|
||||
}
|
||||
$validStatuses = $GLOBALS['config']['TICKET_STATUSES']
|
||||
?? ['Open', 'Pending', 'In Progress', 'Closed'];
|
||||
if (!in_array($parameters['status'], $validStatuses, true)) {
|
||||
return 'Invalid status value';
|
||||
}
|
||||
break;
|
||||
|
||||
case 'bulk_assign':
|
||||
if (!isset($parameters['assigned_to'])) {
|
||||
return 'Missing assigned_to parameter';
|
||||
}
|
||||
$assignedTo = $parameters['assigned_to'];
|
||||
if (!is_numeric($assignedTo) || (int)$assignedTo <= 0 || !$this->userExists((int)$assignedTo)) {
|
||||
return 'Invalid assigned_to: user does not exist';
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a user ID exists.
|
||||
*/
|
||||
private function userExists(int $userId): bool
|
||||
{
|
||||
$stmt = $this->conn->prepare("SELECT 1 FROM users WHERE user_id = ? LIMIT 1");
|
||||
$stmt->bind_param("i", $userId);
|
||||
$stmt->execute();
|
||||
$exists = $stmt->get_result()->num_rows > 0;
|
||||
$stmt->close();
|
||||
return $exists;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get bulk operation by ID
|
||||
*
|
||||
|
||||
+11
-9
@@ -58,12 +58,12 @@ class CommentModel
|
||||
/**
|
||||
* Get total comment count for a ticket
|
||||
*/
|
||||
public function getCommentCount(int $ticketId): int
|
||||
public function getCommentCount(string $ticketId): int
|
||||
{
|
||||
$stmt = $this->conn->prepare(
|
||||
"SELECT COUNT(*) as total FROM ticket_comments WHERE ticket_id = ?"
|
||||
);
|
||||
$stmt->bind_param("i", $ticketId);
|
||||
$stmt->bind_param("s", $ticketId);
|
||||
$stmt->execute();
|
||||
$row = $stmt->get_result()->fetch_assoc();
|
||||
$stmt->close();
|
||||
@@ -108,9 +108,9 @@ class CommentModel
|
||||
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
if ($limit > 0) {
|
||||
$stmt->bind_param("iii", $ticketId, $limit, $offset);
|
||||
$stmt->bind_param("sii", $ticketId, $limit, $offset);
|
||||
} else {
|
||||
$stmt->bind_param("i", $ticketId);
|
||||
$stmt->bind_param("s", $ticketId);
|
||||
}
|
||||
$stmt->execute();
|
||||
$result = $stmt->get_result();
|
||||
@@ -146,7 +146,7 @@ class CommentModel
|
||||
/**
|
||||
* Paginated threaded comments: fetch one page of root comments + all their replies.
|
||||
*/
|
||||
private function getThreadedCommentsPaged(int $ticketId, int $limit, int $offset): array
|
||||
private function getThreadedCommentsPaged(string $ticketId, int $limit, int $offset): array
|
||||
{
|
||||
// Page of root comments
|
||||
$rootSql = "SELECT tc.*, u.display_name, u.username
|
||||
@@ -156,7 +156,7 @@ class CommentModel
|
||||
ORDER BY tc.created_at DESC
|
||||
LIMIT ? OFFSET ?";
|
||||
$stmt = $this->conn->prepare($rootSql);
|
||||
$stmt->bind_param("iii", $ticketId, $limit, $offset);
|
||||
$stmt->bind_param("sii", $ticketId, $limit, $offset);
|
||||
$stmt->execute();
|
||||
$rootResult = $stmt->get_result();
|
||||
$stmt->close();
|
||||
@@ -192,7 +192,7 @@ class CommentModel
|
||||
AND tc.parent_comment_id IN ($placeholders)
|
||||
ORDER BY tc.created_at ASC";
|
||||
$replyStmt = $this->conn->prepare($replySql);
|
||||
$types = 'i' . str_repeat('i', count($parentIds));
|
||||
$types = 's' . str_repeat('i', count($parentIds));
|
||||
$replyStmt->bind_param($types, $ticketId, ...$parentIds);
|
||||
$replyStmt->execute();
|
||||
$replyResult = $replyStmt->get_result();
|
||||
@@ -394,7 +394,8 @@ class CommentModel
|
||||
'updated_at' => $hasUpdatedAt ? date('M d, Y H:i') : null
|
||||
];
|
||||
} else {
|
||||
return ['success' => false, 'error' => $this->conn->error];
|
||||
error_log('CommentModel::updateComment failed: ' . $this->conn->error);
|
||||
return ['success' => false, 'error' => 'Failed to update comment'];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -428,7 +429,8 @@ class CommentModel
|
||||
'ticket_id' => $ticketId
|
||||
];
|
||||
} else {
|
||||
return ['success' => false, 'error' => $this->conn->error];
|
||||
error_log('CommentModel::deleteComment failed: ' . $this->conn->error);
|
||||
return ['success' => false, 'error' => 'Failed to delete comment'];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -96,6 +96,10 @@ class CustomFieldModel
|
||||
(field_name, field_label, field_type, field_options, category, is_required, display_order, is_active)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?)";
|
||||
|
||||
$isRequired = $data['is_required'] ?? 0;
|
||||
$displayOrder = $data['display_order'] ?? 0;
|
||||
$isActive = $data['is_active'] ?? 1;
|
||||
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param(
|
||||
'sssssiii',
|
||||
@@ -104,9 +108,9 @@ class CustomFieldModel
|
||||
$data['field_type'],
|
||||
$options,
|
||||
$data['category'],
|
||||
$data['is_required'] ?? 0,
|
||||
$data['display_order'] ?? 0,
|
||||
$data['is_active'] ?? 1
|
||||
$isRequired,
|
||||
$displayOrder,
|
||||
$isActive
|
||||
);
|
||||
|
||||
if ($stmt->execute()) {
|
||||
@@ -135,6 +139,10 @@ class CustomFieldModel
|
||||
category = ?, is_required = ?, display_order = ?, is_active = ?
|
||||
WHERE field_id = ?";
|
||||
|
||||
$isRequired = $data['is_required'] ?? 0;
|
||||
$displayOrder = $data['display_order'] ?? 0;
|
||||
$isActive = $data['is_active'] ?? 1;
|
||||
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param(
|
||||
'sssssiiii',
|
||||
@@ -143,9 +151,9 @@ class CustomFieldModel
|
||||
$data['field_type'],
|
||||
$options,
|
||||
$data['category'],
|
||||
$data['is_required'] ?? 0,
|
||||
$data['display_order'] ?? 0,
|
||||
$data['is_active'] ?? 1,
|
||||
$isRequired,
|
||||
$displayOrder,
|
||||
$isActive,
|
||||
$fieldId
|
||||
);
|
||||
|
||||
|
||||
@@ -12,25 +12,67 @@ class DependencyModel
|
||||
$this->conn = $conn;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the extra WHERE fragment (and bound params) that restricts the joined
|
||||
* ticket alias `t` to tickets the requesting user may see. Reuses
|
||||
* TicketModel::getVisibilityFilter so the rules stay in one place.
|
||||
*
|
||||
* @return array{sql:string,types:string,params:array}
|
||||
*/
|
||||
private function buildVisibilityClause($userId, array $userGroups, $isAdmin): array
|
||||
{
|
||||
if ($isAdmin) {
|
||||
return ['sql' => '', 'types' => '', 'params' => []];
|
||||
}
|
||||
|
||||
require_once dirname(__DIR__) . '/models/TicketModel.php';
|
||||
$ticketModel = new TicketModel($this->conn);
|
||||
$filter = $ticketModel->getVisibilityFilter([
|
||||
'user_id' => (int)$userId,
|
||||
'groups' => implode(',', $userGroups),
|
||||
'is_admin' => false,
|
||||
]);
|
||||
|
||||
if ($filter['sql'] === '1=1' || $filter['sql'] === '') {
|
||||
return ['sql' => '', 'types' => '', 'params' => []];
|
||||
}
|
||||
|
||||
return [
|
||||
'sql' => ' AND ' . $filter['sql'],
|
||||
'types' => $filter['types'],
|
||||
'params' => $filter['params'],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all dependencies for a ticket
|
||||
*
|
||||
* The linked ticket's title/status/priority are only returned for tickets the
|
||||
* requesting user is allowed to see (same rules as TicketModel::getVisibilityFilter).
|
||||
* With the default (null user, non-admin) only public tickets are exposed.
|
||||
*
|
||||
* @param string $ticketId Ticket ID
|
||||
* @param int|null $userId Requesting user's ID (null = anonymous)
|
||||
* @param array $userGroups Requesting user's group names
|
||||
* @param bool $isAdmin Whether the requesting user is an admin (bypasses filtering)
|
||||
* @return array Dependencies grouped by type
|
||||
*/
|
||||
public function getDependencies($ticketId)
|
||||
public function getDependencies($ticketId, $userId = null, array $userGroups = [], $isAdmin = false)
|
||||
{
|
||||
$visibility = $this->buildVisibilityClause($userId, $userGroups, $isAdmin);
|
||||
|
||||
$sql = "SELECT d.*, t.title, t.status, t.priority
|
||||
FROM ticket_dependencies d
|
||||
LEFT JOIN tickets t ON d.depends_on_id = t.ticket_id
|
||||
WHERE d.ticket_id = ?
|
||||
WHERE d.ticket_id = ?" . $visibility['sql'] . "
|
||||
ORDER BY d.dependency_type, d.created_at DESC";
|
||||
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
if (!$stmt) {
|
||||
throw new Exception('Prepare failed: ' . $this->conn->error);
|
||||
}
|
||||
$stmt->bind_param("s", $ticketId);
|
||||
$types = 's' . $visibility['types'];
|
||||
$stmt->bind_param($types, $ticketId, ...$visibility['params']);
|
||||
if (!$stmt->execute()) {
|
||||
throw new Exception('Execute failed: ' . $stmt->error);
|
||||
}
|
||||
@@ -54,22 +96,32 @@ class DependencyModel
|
||||
/**
|
||||
* Get tickets that depend on this ticket
|
||||
*
|
||||
* The linked ticket's title/status/priority are only returned for tickets the
|
||||
* requesting user is allowed to see (same rules as TicketModel::getVisibilityFilter).
|
||||
* With the default (null user, non-admin) only public tickets are exposed.
|
||||
*
|
||||
* @param string $ticketId Ticket ID
|
||||
* @param int|null $userId Requesting user's ID (null = anonymous)
|
||||
* @param array $userGroups Requesting user's group names
|
||||
* @param bool $isAdmin Whether the requesting user is an admin (bypasses filtering)
|
||||
* @return array Dependent tickets
|
||||
*/
|
||||
public function getDependentTickets($ticketId)
|
||||
public function getDependentTickets($ticketId, $userId = null, array $userGroups = [], $isAdmin = false)
|
||||
{
|
||||
$visibility = $this->buildVisibilityClause($userId, $userGroups, $isAdmin);
|
||||
|
||||
$sql = "SELECT d.*, t.title, t.status, t.priority
|
||||
FROM ticket_dependencies d
|
||||
LEFT JOIN tickets t ON d.ticket_id = t.ticket_id
|
||||
WHERE d.depends_on_id = ?
|
||||
WHERE d.depends_on_id = ?" . $visibility['sql'] . "
|
||||
ORDER BY d.dependency_type, d.created_at DESC";
|
||||
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
if (!$stmt) {
|
||||
throw new Exception('Prepare failed: ' . $this->conn->error);
|
||||
}
|
||||
$stmt->bind_param("s", $ticketId);
|
||||
$types = 's' . $visibility['types'];
|
||||
$stmt->bind_param($types, $ticketId, ...$visibility['params']);
|
||||
if (!$stmt->execute()) {
|
||||
throw new Exception('Execute failed: ' . $stmt->error);
|
||||
}
|
||||
|
||||
@@ -65,7 +65,7 @@ class RecurringTicketModel
|
||||
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param(
|
||||
'ssssiiisssii',
|
||||
'ssssiissssii',
|
||||
$data['title_template'],
|
||||
$data['description_template'],
|
||||
$data['category'],
|
||||
|
||||
+46
-31
@@ -9,7 +9,7 @@ class TicketModel
|
||||
$this->conn = $conn;
|
||||
}
|
||||
|
||||
public function getTicketById(int $id): ?array
|
||||
public function getTicketById(string $id): ?array
|
||||
{
|
||||
$sql = "SELECT t.*,
|
||||
u_created.username as creator_username,
|
||||
@@ -24,7 +24,7 @@ class TicketModel
|
||||
LEFT JOIN users u_assigned ON t.assigned_to = u_assigned.user_id
|
||||
WHERE t.ticket_id = ?";
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param("i", $id);
|
||||
$stmt->bind_param("s", $id);
|
||||
$stmt->execute();
|
||||
$result = $stmt->get_result();
|
||||
|
||||
@@ -82,18 +82,22 @@ class TicketModel
|
||||
$paramTypes .= str_repeat('s', count($types));
|
||||
}
|
||||
|
||||
// Search Functionality — use FULLTEXT when available, fall back to LIKE
|
||||
if ($search && !empty($search)) {
|
||||
if ($this->hasFulltextIndex()) {
|
||||
// Search Functionality — use FULLTEXT when available, fall back to LIKE.
|
||||
// Use a strict emptiness check so a literal "0" search is honored.
|
||||
if ($search !== null && $search !== '') {
|
||||
// Strip MySQL boolean mode special chars to prevent parse errors on user input
|
||||
$ftSearch = trim(preg_replace('/\s+/', ' ', preg_replace('/[+\-><()\~*"@]+/', ' ', $search)));
|
||||
if ($this->hasFulltextIndex() && $ftSearch !== '') {
|
||||
// MATCH...AGAINST for indexed full-text search (much faster at scale)
|
||||
// Strip MySQL boolean mode special chars to prevent parse errors on user input
|
||||
$ftSearch = preg_replace('/[+\-><()\~*"@]+/', ' ', $search);
|
||||
$ftSearch = trim(preg_replace('/\s+/', ' ', $ftSearch)) . '*';
|
||||
$ftSearch .= '*';
|
||||
$whereConditions[] = "(MATCH(t.title, t.description) AGAINST (? IN BOOLEAN MODE) OR t.ticket_id LIKE ? OR t.category LIKE ? OR t.type LIKE ?)";
|
||||
$searchTerm = "%$search%";
|
||||
$params = array_merge($params, [$ftSearch, $searchTerm, $searchTerm, $searchTerm]);
|
||||
$paramTypes .= 'ssss';
|
||||
} else {
|
||||
// No FULLTEXT index, or the sanitized boolean query is empty (search was
|
||||
// only special chars) — fall back to LIKE instead of emitting invalid
|
||||
// AGAINST('*' ...) syntax.
|
||||
$whereConditions[] = "(t.title LIKE ? OR t.description LIKE ? OR t.ticket_id LIKE ? OR t.category LIKE ? OR t.type LIKE ?)";
|
||||
$searchTerm = "%$search%";
|
||||
$params = array_merge($params, [$searchTerm, $searchTerm, $searchTerm, $searchTerm, $searchTerm]);
|
||||
@@ -308,7 +312,7 @@ class TicketModel
|
||||
|
||||
if ($expectedUpdatedAt !== null) {
|
||||
$stmt->bind_param(
|
||||
"sissssisis",
|
||||
"sissssisss",
|
||||
$ticketData['title'],
|
||||
$ticketData['priority'],
|
||||
$ticketData['status'],
|
||||
@@ -322,7 +326,7 @@ class TicketModel
|
||||
);
|
||||
} else {
|
||||
$stmt->bind_param(
|
||||
"sissssisi",
|
||||
"sissssiss",
|
||||
$ticketData['title'],
|
||||
$ticketData['priority'],
|
||||
$ticketData['status'],
|
||||
@@ -343,20 +347,31 @@ class TicketModel
|
||||
return ['success' => false, 'error' => 'Database error: ' . $this->conn->error, 'conflict' => false];
|
||||
}
|
||||
|
||||
// Check for optimistic locking conflict
|
||||
if ($expectedUpdatedAt !== null && $affectedRows === 0) {
|
||||
// Either ticket doesn't exist or was modified by someone else
|
||||
// Zero affected rows is ambiguous: the ticket may not exist, an optimistic
|
||||
// lock may have failed, or the row simply matched with no column changes
|
||||
// (identical resubmit). Disambiguate so we neither report a false conflict
|
||||
// nor silently "succeed" on a non-existent ticket.
|
||||
if ($affectedRows === 0) {
|
||||
$ticket = $this->getTicketById($ticketData['ticket_id']);
|
||||
if ($ticket) {
|
||||
return [
|
||||
'success' => false,
|
||||
'error' => 'This ticket was modified by another user. Please refresh and try again.',
|
||||
'conflict' => true,
|
||||
'current_updated_at' => $ticket['updated_at']
|
||||
];
|
||||
} else {
|
||||
if (!$ticket) {
|
||||
return ['success' => false, 'error' => 'Ticket not found', 'conflict' => false];
|
||||
}
|
||||
|
||||
if ($expectedUpdatedAt !== null) {
|
||||
// Only a genuine concurrent modification changes updated_at. If it
|
||||
// still equals the expected value the WHERE matched but nothing
|
||||
// changed (e.g. identical data resubmitted within the same second),
|
||||
// which is not a conflict.
|
||||
if ($ticket['updated_at'] !== $expectedUpdatedAt) {
|
||||
return [
|
||||
'success' => false,
|
||||
'error' => 'This ticket was modified by another user. Please refresh and try again.',
|
||||
'conflict' => true,
|
||||
'current_updated_at' => $ticket['updated_at']
|
||||
];
|
||||
}
|
||||
}
|
||||
// Ticket exists and no conflict: treat no-op update as success.
|
||||
}
|
||||
|
||||
return ['success' => true, 'error' => null, 'conflict' => false];
|
||||
@@ -516,9 +531,9 @@ class TicketModel
|
||||
}
|
||||
}
|
||||
|
||||
public function addComment(int $ticketId, array $commentData): array
|
||||
public function addComment(string $ticketId, array $commentData): array
|
||||
{
|
||||
$sql = "INSERT INTO ticket_comments (ticket_id, user_name, comment_text, markdown_enabled)
|
||||
$sql = "INSERT INTO ticket_comments (ticket_id, user_name, comment_text, markdown_enabled)
|
||||
VALUES (?, ?, ?, ?)";
|
||||
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
@@ -528,7 +543,7 @@ class TicketModel
|
||||
$markdownEnabled = $commentData['markdown_enabled'] ? 1 : 0;
|
||||
|
||||
$stmt->bind_param(
|
||||
"issi",
|
||||
"sssi",
|
||||
$ticketId,
|
||||
$username,
|
||||
$commentData['comment_text'],
|
||||
@@ -557,11 +572,11 @@ class TicketModel
|
||||
* @param int $assignedBy User ID performing the assignment
|
||||
* @return bool Success status
|
||||
*/
|
||||
public function assignTicket(int $ticketId, int $userId, int $assignedBy): bool
|
||||
public function assignTicket(string $ticketId, int $userId, int $assignedBy): bool
|
||||
{
|
||||
$sql = "UPDATE tickets SET assigned_to = ?, updated_by = ?, updated_at = NOW() WHERE ticket_id = ?";
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param("iii", $userId, $assignedBy, $ticketId);
|
||||
$stmt->bind_param("iis", $userId, $assignedBy, $ticketId);
|
||||
$result = $stmt->execute();
|
||||
$stmt->close();
|
||||
return $result;
|
||||
@@ -574,11 +589,11 @@ class TicketModel
|
||||
* @param int $updatedBy User ID performing the unassignment
|
||||
* @return bool Success status
|
||||
*/
|
||||
public function unassignTicket(int $ticketId, int $updatedBy): bool
|
||||
public function unassignTicket(string $ticketId, int $updatedBy): bool
|
||||
{
|
||||
$sql = "UPDATE tickets SET assigned_to = NULL, updated_by = ?, updated_at = NOW() WHERE ticket_id = ?";
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param("ii", $updatedBy, $ticketId);
|
||||
$stmt->bind_param("is", $updatedBy, $ticketId);
|
||||
$result = $stmt->execute();
|
||||
$stmt->close();
|
||||
return $result;
|
||||
@@ -733,7 +748,7 @@ class TicketModel
|
||||
* @param int $updatedBy User ID
|
||||
* @return bool
|
||||
*/
|
||||
public function updateVisibility(int $ticketId, string $visibility, ?string $visibilityGroups, int $updatedBy): bool
|
||||
public function updateVisibility(string $ticketId, string $visibility, ?string $visibilityGroups, int $updatedBy): bool
|
||||
{
|
||||
$allowedVisibilities = ['public', 'internal', 'confidential'];
|
||||
if (!in_array($visibility, $allowedVisibilities)) {
|
||||
@@ -752,7 +767,7 @@ class TicketModel
|
||||
|
||||
$sql = "UPDATE tickets SET visibility = ?, visibility_groups = ?, updated_by = ?, updated_at = NOW() WHERE ticket_id = ?";
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param("ssii", $visibility, $visibilityGroups, $updatedBy, $ticketId);
|
||||
$stmt->bind_param("ssis", $visibility, $visibilityGroups, $updatedBy, $ticketId);
|
||||
$result = $stmt->execute();
|
||||
$stmt->close();
|
||||
return $result;
|
||||
@@ -790,7 +805,7 @@ class TicketModel
|
||||
"DELETE FROM ticket_watchers WHERE ticket_id = ?",
|
||||
"DELETE FROM ticket_dependencies WHERE ticket_id = ? OR depends_on_id = ?",
|
||||
"DELETE FROM ticket_attachments WHERE ticket_id = ?",
|
||||
"DELETE FROM ticket_custom_fields WHERE ticket_id = ?",
|
||||
"DELETE FROM custom_field_values WHERE ticket_id = ?",
|
||||
];
|
||||
|
||||
foreach ($children as $sql) {
|
||||
|
||||
+66
-37
@@ -26,31 +26,38 @@ class WorkflowModel
|
||||
*/
|
||||
private function getAllTransitions(): array
|
||||
{
|
||||
return CacheHelper::remember(self::$CACHE_PREFIX, 'all_transitions', function () {
|
||||
$sql = "SELECT from_status, to_status, requires_comment, requires_admin
|
||||
FROM status_transitions
|
||||
WHERE is_active = TRUE";
|
||||
$result = $this->conn->query($sql);
|
||||
$cached = CacheHelper::get(self::$CACHE_PREFIX, 'all_transitions', self::$CACHE_TTL);
|
||||
if ($cached !== null) {
|
||||
return $cached;
|
||||
}
|
||||
|
||||
if (!$result) {
|
||||
return [];
|
||||
$sql = "SELECT from_status, to_status, requires_comment, requires_admin
|
||||
FROM status_transitions
|
||||
WHERE is_active = TRUE";
|
||||
$result = $this->conn->query($sql);
|
||||
|
||||
if (!$result) {
|
||||
// A transient DB failure must NOT be cached as "no transitions" — that
|
||||
// would block every status change for the whole TTL. Fail safe by
|
||||
// returning empty without storing it, so the next call retries.
|
||||
return [];
|
||||
}
|
||||
|
||||
$transitions = [];
|
||||
while ($row = $result->fetch_assoc()) {
|
||||
$from = $row['from_status'];
|
||||
if (!isset($transitions[$from])) {
|
||||
$transitions[$from] = [];
|
||||
}
|
||||
$transitions[$from][$row['to_status']] = [
|
||||
'to_status' => $row['to_status'],
|
||||
'requires_comment' => (bool)$row['requires_comment'],
|
||||
'requires_admin' => (bool)$row['requires_admin']
|
||||
];
|
||||
}
|
||||
|
||||
$transitions = [];
|
||||
while ($row = $result->fetch_assoc()) {
|
||||
$from = $row['from_status'];
|
||||
if (!isset($transitions[$from])) {
|
||||
$transitions[$from] = [];
|
||||
}
|
||||
$transitions[$from][$row['to_status']] = [
|
||||
'to_status' => $row['to_status'],
|
||||
'requires_comment' => (bool)$row['requires_comment'],
|
||||
'requires_admin' => (bool)$row['requires_admin']
|
||||
];
|
||||
}
|
||||
|
||||
return $transitions;
|
||||
}, self::$CACHE_TTL);
|
||||
CacheHelper::set(self::$CACHE_PREFIX, 'all_transitions', $transitions);
|
||||
return $transitions;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -107,24 +114,29 @@ class WorkflowModel
|
||||
*/
|
||||
public function getAllStatuses(): array
|
||||
{
|
||||
return CacheHelper::remember(self::$CACHE_PREFIX, 'all_statuses', function () {
|
||||
$sql = "SELECT DISTINCT from_status as status FROM status_transitions
|
||||
UNION
|
||||
SELECT DISTINCT to_status as status FROM status_transitions
|
||||
ORDER BY status";
|
||||
$result = $this->conn->query($sql);
|
||||
$cached = CacheHelper::get(self::$CACHE_PREFIX, 'all_statuses', self::$CACHE_TTL);
|
||||
if ($cached !== null) {
|
||||
return $cached;
|
||||
}
|
||||
|
||||
if (!$result) {
|
||||
return [];
|
||||
}
|
||||
$sql = "SELECT DISTINCT from_status as status FROM status_transitions
|
||||
UNION
|
||||
SELECT DISTINCT to_status as status FROM status_transitions
|
||||
ORDER BY status";
|
||||
$result = $this->conn->query($sql);
|
||||
|
||||
$statuses = [];
|
||||
while ($row = $result->fetch_assoc()) {
|
||||
$statuses[] = $row['status'];
|
||||
}
|
||||
if (!$result) {
|
||||
// Do not cache an empty list on a transient DB failure.
|
||||
return [];
|
||||
}
|
||||
|
||||
return $statuses;
|
||||
}, self::$CACHE_TTL);
|
||||
$statuses = [];
|
||||
while ($row = $result->fetch_assoc()) {
|
||||
$statuses[] = $row['status'];
|
||||
}
|
||||
|
||||
CacheHelper::set(self::$CACHE_PREFIX, 'all_statuses', $statuses);
|
||||
return $statuses;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -149,6 +161,23 @@ class WorkflowModel
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a given transition requires a comment.
|
||||
*
|
||||
* Convenience accessor so callers (e.g. the update-ticket endpoint) can
|
||||
* enforce requires_comment server-side without inspecting the full row.
|
||||
* Returns false for an undefined transition or a no-op (same status).
|
||||
*
|
||||
* @param string $fromStatus Current status
|
||||
* @param string $toStatus Desired status
|
||||
* @return bool True if the transition requires a comment
|
||||
*/
|
||||
public function transitionRequiresComment(string $fromStatus, string $toStatus): bool
|
||||
{
|
||||
$requirements = $this->getTransitionRequirements($fromStatus, $toStatus);
|
||||
return $requirements !== null && !empty($requirements['requires_comment']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear workflow cache (call when transitions are modified)
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user