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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
This commit is contained in:
@@ -101,115 +101,27 @@ class TicketController
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Handle visibility groups (comes as array from checkboxes)
|
// Validation (incl. required custom fields), creation, audit log,
|
||||||
$visibilityGroups = null;
|
// stats cache, custom field values, duplicate link and notification
|
||||||
if (isset($_POST['visibility_groups']) && is_array($_POST['visibility_groups'])) {
|
// live in TicketCreationService so the MCP create_ticket tool runs
|
||||||
$visibilityGroups = implode(',', array_map('trim', $_POST['visibility_groups']));
|
// the same code path.
|
||||||
}
|
require_once dirname(__DIR__) . '/services/TicketCreationService.php';
|
||||||
|
$result = TicketCreationService::create($this->conn, $currentUser ?? [], [
|
||||||
// Honor the posted status, validated against the app's canonical list
|
'title' => $_POST['title'] ?? '',
|
||||||
$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'] ?? ''),
|
|
||||||
'description' => $_POST['description'] ?? '',
|
'description' => $_POST['description'] ?? '',
|
||||||
'priority' => $_POST['priority'] ?? '4',
|
'priority' => $_POST['priority'] ?? '4',
|
||||||
'category' => $_POST['category'] ?? 'General',
|
'category' => $_POST['category'] ?? 'General',
|
||||||
'type' => $_POST['type'] ?? 'Issue',
|
'type' => $_POST['type'] ?? 'Issue',
|
||||||
'status' => $status,
|
'status' => $_POST['status'] ?? 'Open',
|
||||||
'visibility' => $_POST['visibility'] ?? 'public',
|
'visibility' => $_POST['visibility'] ?? 'public',
|
||||||
'visibility_groups' => $visibilityGroups,
|
'visibility_groups' => (isset($_POST['visibility_groups']) && is_array($_POST['visibility_groups']))
|
||||||
'assigned_to' => !empty($_POST['assigned_to']) ? $_POST['assigned_to'] : null
|
? $_POST['visibility_groups'] : null,
|
||||||
];
|
'assigned_to' => $_POST['assigned_to'] ?? null,
|
||||||
|
'custom_fields' => $_POST['custom_fields'] ?? [],
|
||||||
// Validate input (server-side; form is novalidate)
|
'link_duplicate_of' => $_POST['link_duplicate_of'] ?? '',
|
||||||
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);
|
|
||||||
|
|
||||||
if ($result['success']) {
|
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
|
// Redirect to the new ticket
|
||||||
header("Location: " . $GLOBALS['config']['BASE_URL'] . "/ticket/" . $result['ticket_id']);
|
header("Location: " . $GLOBALS['config']['BASE_URL'] . "/ticket/" . $result['ticket_id']);
|
||||||
exit;
|
exit;
|
||||||
|
|||||||
@@ -0,0 +1,134 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Creating a ticket as a user: required-field and required-custom-field
|
||||||
|
* validation, TicketModel::createTicket (which enforces visibility rules),
|
||||||
|
* audit log, stats-cache invalidation, custom field values, optional
|
||||||
|
* "duplicates" link, and the new-ticket Matrix notification.
|
||||||
|
*
|
||||||
|
* Shared by the web UI (TicketController::create) and the MCP create_ticket
|
||||||
|
* tool so both run one code path (tinker_tickets#111). Extracted from
|
||||||
|
* TicketController::create with the same checks, order and error messages.
|
||||||
|
* Callers own request parsing, CSRF, and rendering.
|
||||||
|
*/
|
||||||
|
|
||||||
|
require_once dirname(__DIR__) . '/models/TicketModel.php';
|
||||||
|
require_once dirname(__DIR__) . '/models/AuditLogModel.php';
|
||||||
|
require_once dirname(__DIR__) . '/models/CustomFieldModel.php';
|
||||||
|
require_once dirname(__DIR__) . '/models/StatsModel.php';
|
||||||
|
require_once dirname(__DIR__) . '/helpers/NotificationHelper.php';
|
||||||
|
|
||||||
|
class TicketCreationService
|
||||||
|
{
|
||||||
|
/**
|
||||||
|
* @param array $currentUser Authenticated user row
|
||||||
|
* @param array $input title, description, priority, category, type, status,
|
||||||
|
* visibility, visibility_groups (string or list), assigned_to,
|
||||||
|
* custom_fields (field_id => 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']];
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user