From f206bb5889bac1f8bb1bd52d16e16627e66f834c Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Thu, 24 Sep 2026 19:13:31 -0400 Subject: [PATCH] Extract ticket creation from TicketController into TicketCreationService (#111) Required-field and required-custom-field validation, createTicket (which enforces visibility rules), the audit log, stats-cache invalidation, custom field values, the optional 'duplicates' link and the new-ticket notification move into services/TicketCreationService.php with the same order and error messages, so the MCP create_ticket tool runs one code path with the web form. TicketController::create keeps CSRF, the redirect, and re-rendering the form on error (the view never read the removed locals). The service also accepts visibility_groups as a string for API callers; the controller still passes only the form's array, so web behaviour is unchanged. Verified the real web form over HTTP (logged in via Remote-User, real CSRF token): a valid submit redirects to the new ticket, created and audit-logged as the user; a blank description re-renders the form with 'Description is required' and creates nothing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X --- controllers/TicketController.php | 116 +++---------------------- services/TicketCreationService.php | 134 +++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 102 deletions(-) create mode 100644 services/TicketCreationService.php diff --git a/controllers/TicketController.php b/controllers/TicketController.php index 2825d29..fd4dc51 100644 --- a/controllers/TicketController.php +++ b/controllers/TicketController.php @@ -101,115 +101,27 @@ class TicketController return; } - // Handle visibility groups (comes as array from checkboxes) - $visibilityGroups = null; - if (isset($_POST['visibility_groups']) && is_array($_POST['visibility_groups'])) { - $visibilityGroups = implode(',', array_map('trim', $_POST['visibility_groups'])); - } - - // Honor the posted status, validated against the app's canonical list - $validStatuses = $GLOBALS['config']['TICKET_STATUSES'] ?? ['Open', 'Pending', 'In Progress', 'Closed']; - $status = $_POST['status'] ?? 'Open'; - if (!in_array($status, $validStatuses, true)) { - $status = 'Open'; - } - - $ticketData = [ - 'title' => trim($_POST['title'] ?? ''), + // Validation (incl. required custom fields), creation, audit log, + // stats cache, custom field values, duplicate link and notification + // live in TicketCreationService so the MCP create_ticket tool runs + // the same code path. + require_once dirname(__DIR__) . '/services/TicketCreationService.php'; + $result = TicketCreationService::create($this->conn, $currentUser ?? [], [ + 'title' => $_POST['title'] ?? '', 'description' => $_POST['description'] ?? '', 'priority' => $_POST['priority'] ?? '4', 'category' => $_POST['category'] ?? 'General', 'type' => $_POST['type'] ?? 'Issue', - 'status' => $status, + 'status' => $_POST['status'] ?? 'Open', 'visibility' => $_POST['visibility'] ?? 'public', - 'visibility_groups' => $visibilityGroups, - 'assigned_to' => !empty($_POST['assigned_to']) ? $_POST['assigned_to'] : null - ]; - - // Validate input (server-side; form is novalidate) - if ($ticketData['title'] === '') { - $error = "Title is required"; - $templates = $this->templateModel->getAllTemplates(); - $allUsers = $this->userModel->getAllUsers(); - $conn = $this->conn; // Make $conn available to view - include dirname(__DIR__) . '/views/CreateTicketView.php'; - return; - } - - if (trim($ticketData['description']) === '') { - $error = "Description is required"; - $templates = $this->templateModel->getAllTemplates(); - $allUsers = $this->userModel->getAllUsers(); - $conn = $this->conn; // Make $conn available to view - include dirname(__DIR__) . '/views/CreateTicketView.php'; - return; - } - - // Custom fields applicable to the submitted category — validate - // is_required server-side (the form is novalidate, and a field - // hidden by the client-side category toggle must not silently - // bypass a requirement that applies to the category actually - // submitted). - $submittedCustomFields = is_array($_POST['custom_fields'] ?? null) ? $_POST['custom_fields'] : []; - $applicableFieldDefs = array_filter( - $allCustomFieldDefs, - fn($def) => $def['category'] === null || $def['category'] === $ticketData['category'] - ); - $customFieldsToSave = []; - foreach ($applicableFieldDefs as $def) { - $fieldId = (int)$def['field_id']; - $raw = $submittedCustomFields[$fieldId] ?? null; - $normalized = $def['field_type'] === 'checkbox' - ? (!empty($raw) ? '1' : '0') - : (is_scalar($raw) ? trim((string)$raw) : ''); - - if (!empty($def['is_required']) && $def['field_type'] !== 'checkbox' && $normalized === '') { - $error = $def['field_label'] . ' is required'; - $templates = $this->templateModel->getAllTemplates(); - $allUsers = $this->userModel->getAllUsers(); - $conn = $this->conn; - include dirname(__DIR__) . '/views/CreateTicketView.php'; - return; - } - - if ($normalized !== '') { - $customFieldsToSave[$fieldId] = $normalized; - } - } - - // Create ticket with user tracking - $result = $this->ticketModel->createTicket($ticketData, $userId); + 'visibility_groups' => (isset($_POST['visibility_groups']) && is_array($_POST['visibility_groups'])) + ? $_POST['visibility_groups'] : null, + 'assigned_to' => $_POST['assigned_to'] ?? null, + 'custom_fields' => $_POST['custom_fields'] ?? [], + 'link_duplicate_of' => $_POST['link_duplicate_of'] ?? '', + ]); if ($result['success']) { - // Log ticket creation to audit log - if (isset($GLOBALS['auditLog']) && $userId) { - $GLOBALS['auditLog']->logTicketCreate($userId, $result['ticket_id'], $ticketData); - } - - // Ticket counts changed — invalidate the cached dashboard stats - require_once dirname(__DIR__) . '/models/StatsModel.php'; - (new StatsModel($this->conn))->invalidateCache(); - - // Persist custom field values for the fields applicable to - // this ticket's category - if (!empty($customFieldsToSave)) { - $this->customFieldModel->setValues($result['ticket_id'], $customFieldsToSave); - } - - // Auto-link as duplicate if requested from create form - $linkDupOfRaw = trim($_POST['link_duplicate_of'] ?? ''); - if ($linkDupOfRaw !== '' && ctype_digit($linkDupOfRaw)) { - $depSql = "INSERT IGNORE INTO ticket_dependencies (ticket_id, depends_on_id, dependency_type, created_by) - VALUES (?, ?, 'duplicates', ?)"; - $depStmt = $this->conn->prepare($depSql); - $depStmt->bind_param("ssi", $result['ticket_id'], $linkDupOfRaw, $userId); - $depStmt->execute(); - $depStmt->close(); - } - - // Send Matrix notification for new ticket - NotificationHelper::sendTicketNotification($result['ticket_id'], $ticketData, 'manual'); - // Redirect to the new ticket header("Location: " . $GLOBALS['config']['BASE_URL'] . "/ticket/" . $result['ticket_id']); exit; diff --git a/services/TicketCreationService.php b/services/TicketCreationService.php new file mode 100644 index 0000000..f60138b --- /dev/null +++ b/services/TicketCreationService.php @@ -0,0 +1,134 @@ + value), link_duplicate_of + * + * @return array ['success' => true, 'ticket_id' => ...] or ['success' => false, 'error' => ...] + */ + public static function create(mysqli $conn, array $currentUser, array $input): array + { + $userId = $currentUser['user_id'] ?? null; + + // Handle visibility groups (a list from the web form's checkboxes, or a string) + $visibilityGroups = null; + if (isset($input['visibility_groups']) && is_array($input['visibility_groups'])) { + $visibilityGroups = implode(',', array_map('trim', $input['visibility_groups'])); + } elseif (isset($input['visibility_groups']) && is_string($input['visibility_groups']) && trim($input['visibility_groups']) !== '') { + $visibilityGroups = trim($input['visibility_groups']); + } + + // Honor the posted status, validated against the app's canonical list + $validStatuses = $GLOBALS['config']['TICKET_STATUSES'] ?? ['Open', 'Pending', 'In Progress', 'Closed']; + $status = $input['status'] ?? 'Open'; + if (!in_array($status, $validStatuses, true)) { + $status = 'Open'; + } + + $ticketData = [ + 'title' => trim($input['title'] ?? ''), + 'description' => $input['description'] ?? '', + 'priority' => $input['priority'] ?? '4', + 'category' => $input['category'] ?? 'General', + 'type' => $input['type'] ?? 'Issue', + 'status' => $status, + 'visibility' => $input['visibility'] ?? 'public', + 'visibility_groups' => $visibilityGroups, + 'assigned_to' => !empty($input['assigned_to']) ? $input['assigned_to'] : null + ]; + + if ($ticketData['title'] === '') { + return ['success' => false, 'error' => 'Title is required']; + } + + if (trim($ticketData['description']) === '') { + return ['success' => false, 'error' => 'Description is required']; + } + + // Custom fields applicable to the submitted category — validate + // is_required server-side (the form is novalidate, and a field + // hidden by the client-side category toggle must not silently + // bypass a requirement that applies to the category actually + // submitted). + $customFieldModel = new CustomFieldModel($conn); + $allCustomFieldDefs = $customFieldModel->getAllDefinitions(null, true); + $submittedCustomFields = is_array($input['custom_fields'] ?? null) ? $input['custom_fields'] : []; + $applicableFieldDefs = array_filter( + $allCustomFieldDefs, + fn($def) => $def['category'] === null || $def['category'] === $ticketData['category'] + ); + $customFieldsToSave = []; + foreach ($applicableFieldDefs as $def) { + $fieldId = (int)$def['field_id']; + $raw = $submittedCustomFields[$fieldId] ?? null; + $normalized = $def['field_type'] === 'checkbox' + ? (!empty($raw) ? '1' : '0') + : (is_scalar($raw) ? trim((string)$raw) : ''); + + if (!empty($def['is_required']) && $def['field_type'] !== 'checkbox' && $normalized === '') { + return ['success' => false, 'error' => $def['field_label'] . ' is required']; + } + + if ($normalized !== '') { + $customFieldsToSave[$fieldId] = $normalized; + } + } + + // Create ticket with user tracking + $result = (new TicketModel($conn))->createTicket($ticketData, $userId); + if (!$result['success']) { + return ['success' => false, 'error' => $result['error']]; + } + + // Log ticket creation to audit log + if ($userId) { + (new AuditLogModel($conn))->logTicketCreate($userId, $result['ticket_id'], $ticketData); + } + + // Ticket counts changed — invalidate the cached dashboard stats + (new StatsModel($conn))->invalidateCache(); + + // Persist custom field values for the fields applicable to this ticket's category + if (!empty($customFieldsToSave)) { + $customFieldModel->setValues($result['ticket_id'], $customFieldsToSave); + } + + // Auto-link as duplicate if requested + $linkDupOfRaw = trim((string)($input['link_duplicate_of'] ?? '')); + if ($linkDupOfRaw !== '' && ctype_digit($linkDupOfRaw)) { + $depSql = "INSERT IGNORE INTO ticket_dependencies (ticket_id, depends_on_id, dependency_type, created_by) + VALUES (?, ?, 'duplicates', ?)"; + $depStmt = $conn->prepare($depSql); + $depStmt->bind_param("ssi", $result['ticket_id'], $linkDupOfRaw, $userId); + $depStmt->execute(); + $depStmt->close(); + } + + // Send Matrix notification for new ticket + NotificationHelper::sendTicketNotification($result['ticket_id'], $ticketData, 'manual'); + + return ['success' => true, 'ticket_id' => $result['ticket_id']]; + } +}