Extract comment creation from add_comment.php into CommentService (#111)
Validation, the ticket access check, reply-parent validation, @mention extraction (audit-logged, notified only to mentioned users who can see the ticket), and comment/watcher notifications move into services/CommentService.php, so the MCP add_comment tool runs one code path with the web UI. add_comment.php keeps session, CSRF, JSON parsing and response codes. Error messages and status codes are unchanged; the extracted body diffs against the original only where each 'emit error and exit' became a 'return [..., http_status]'. Verified the web endpoint over real HTTP: a comment is trimmed, saved with its @mention and the rotated CSRF token returned; a confidential ticket the user can't see still gets 403 'Access denied'; empty text still gets 400. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
This commit is contained in:
+11
-144
@@ -80,155 +80,22 @@ try {
|
||||
exit;
|
||||
}
|
||||
|
||||
$ticketId = isset($data['ticket_id']) ? trim((string)$data['ticket_id']) : '';
|
||||
if (!ctype_digit($ticketId) || (int)$ticketId <= 0) {
|
||||
http_response_code(400);
|
||||
// Validation, access check, mentions, audit log and notifications live in
|
||||
// CommentService so the MCP add_comment tool runs the same code path.
|
||||
require_once dirname(__DIR__) . '/services/CommentService.php';
|
||||
$result = CommentService::addComment($conn, $currentUser, $data);
|
||||
|
||||
if (!empty($result['http_status'])) {
|
||||
http_response_code($result['http_status']);
|
||||
unset($result['http_status']);
|
||||
ob_end_clean();
|
||||
header('Content-Type: application/json');
|
||||
echo json_encode(['success' => false, 'error' => 'Invalid ticket ID']);
|
||||
echo json_encode($result);
|
||||
exit;
|
||||
}
|
||||
|
||||
// Reject empty/whitespace-only comments
|
||||
$commentTextRaw = isset($data['comment_text']) ? trim((string)$data['comment_text']) : '';
|
||||
if ($commentTextRaw === '') {
|
||||
http_response_code(400);
|
||||
ob_end_clean();
|
||||
header('Content-Type: application/json');
|
||||
echo json_encode(['success' => false, 'error' => 'Comment text cannot be empty']);
|
||||
exit;
|
||||
}
|
||||
|
||||
// Persist the trimmed text (not the raw client value) — matches update_comment.php
|
||||
// and keeps stored comment_text free of leading whitespace that could shift a
|
||||
// markdown-enabled comment's first line out of column 0 on reload.
|
||||
$data['comment_text'] = $commentTextRaw;
|
||||
|
||||
// Never trust a client-supplied display name — always attribute the comment to
|
||||
// the authenticated session user.
|
||||
$data['user_name'] = $currentUser['display_name'] ?? $currentUser['username'] ?? 'User';
|
||||
|
||||
// Verify user can access the ticket before allowing a comment
|
||||
$ticketModel = new TicketModel($conn);
|
||||
$ticket = $ticketModel->getTicketById($ticketId);
|
||||
if (!$ticket) {
|
||||
http_response_code(404);
|
||||
ob_end_clean();
|
||||
header('Content-Type: application/json');
|
||||
echo json_encode(['success' => false, 'error' => 'Ticket not found']);
|
||||
exit;
|
||||
}
|
||||
if (!$ticketModel->canUserAccessTicket($ticket, $currentUser)) {
|
||||
http_response_code(403);
|
||||
ob_end_clean();
|
||||
header('Content-Type: application/json');
|
||||
echo json_encode(['success' => false, 'error' => 'Access denied']);
|
||||
exit;
|
||||
}
|
||||
|
||||
// Initialize models
|
||||
$commentModel = new CommentModel($conn);
|
||||
$auditLog = new AuditLogModel($conn);
|
||||
|
||||
// If replying, the parent comment must belong to this same (accessible) ticket.
|
||||
if (isset($data['parent_comment_id']) && $data['parent_comment_id'] !== null && $data['parent_comment_id'] !== '') {
|
||||
$parentComment = $commentModel->getCommentById((int)$data['parent_comment_id']);
|
||||
if (!$parentComment || (string)$parentComment['ticket_id'] !== (string)$ticketId) {
|
||||
http_response_code(400);
|
||||
ob_end_clean();
|
||||
header('Content-Type: application/json');
|
||||
echo json_encode(['success' => false, 'error' => 'Invalid parent comment']);
|
||||
exit;
|
||||
}
|
||||
}
|
||||
|
||||
// Extract @mentions from comment text
|
||||
$mentions = $commentModel->extractMentions($data['comment_text'] ?? '');
|
||||
$mentionedUsers = [];
|
||||
if (!empty($mentions)) {
|
||||
$mentionedUsers = $commentModel->getMentionedUsers($mentions);
|
||||
}
|
||||
|
||||
// Add comment with user tracking
|
||||
$result = $commentModel->addComment($ticketId, $data, $userId);
|
||||
|
||||
// Log comment creation to audit log
|
||||
if ($result['success'] && isset($result['comment_id'])) {
|
||||
$auditLog->logCommentCreate($userId, $result['comment_id'], $ticketId);
|
||||
|
||||
// Log mentions to audit log
|
||||
foreach ($mentionedUsers as $mentionedUser) {
|
||||
$auditLog->log(
|
||||
$userId,
|
||||
'mention',
|
||||
'user',
|
||||
(string)$mentionedUser['user_id'],
|
||||
[
|
||||
'ticket_id' => $ticketId,
|
||||
'comment_id' => $result['comment_id'],
|
||||
'mentioned_username' => $mentionedUser['username']
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
// Matrix notifications
|
||||
$authorDisplay = $currentUser['display_name'] ?? $currentUser['username'] ?? null;
|
||||
$commentText = $data['comment_text'] ?? '';
|
||||
$ticketTitle = $ticket['title'] ?? "Ticket #{$ticketId}";
|
||||
$ticketVisibility = $ticket['visibility'] ?? 'public';
|
||||
|
||||
// @mention notifications — resolve usernames → Matrix IDs via Synapse Admin API.
|
||||
// Only notify mentioned users who actually have access to this ticket;
|
||||
// otherwise a mention would DM them the ticket's title and comment text
|
||||
// even though canUserAccessTicket() would deny them the ticket itself.
|
||||
$accessibleMentionedUsers = array_filter(
|
||||
$mentionedUsers,
|
||||
fn($u) => $ticketModel->canUserAccessTicket($ticket, $u)
|
||||
);
|
||||
if (!empty($accessibleMentionedUsers)) {
|
||||
$mentionedUsernames = array_column($accessibleMentionedUsers, 'username');
|
||||
$mentionedMatrixIds = SynapseHelper::resolveUsernames($mentionedUsernames);
|
||||
if (!empty($mentionedMatrixIds)) {
|
||||
NotificationHelper::sendMentionNotification($ticketId, $ticketTitle, $commentText, $authorDisplay, $mentionedMatrixIds);
|
||||
}
|
||||
}
|
||||
|
||||
// General comment notification (opt-in via MATRIX_NOTIFY_COMMENTS)
|
||||
if (!empty($GLOBALS['config']['MATRIX_NOTIFY_COMMENTS'])) {
|
||||
NotificationHelper::sendCommentNotification(
|
||||
$ticketId,
|
||||
$ticketTitle,
|
||||
$commentText,
|
||||
$authorDisplay,
|
||||
$ticketVisibility !== 'public',
|
||||
$ticketVisibility
|
||||
);
|
||||
}
|
||||
|
||||
// Notify watchers of the new comment
|
||||
NotificationHelper::notifyWatchers(
|
||||
$conn,
|
||||
$ticketId,
|
||||
$ticketTitle,
|
||||
'comment_added',
|
||||
['author' => $authorDisplay, 'preview' => mb_strimwidth($commentText, 0, 200, '…')],
|
||||
(int)$userId,
|
||||
$ticketVisibility
|
||||
);
|
||||
|
||||
// Add mentioned users to result for frontend
|
||||
$result['mentions'] = array_map(function ($u) {
|
||||
return $u['username'];
|
||||
}, $mentionedUsers);
|
||||
}
|
||||
|
||||
// Add user info to result for frontend avatar rendering
|
||||
if ($result['success']) {
|
||||
$result['user_name'] = $currentUser['display_name'] ?? $currentUser['username'];
|
||||
$result['user_id'] = $userId;
|
||||
if (isset($newCsrfToken)) {
|
||||
$result['csrf_token'] = $newCsrfToken;
|
||||
}
|
||||
if ($result['success'] && isset($newCsrfToken)) {
|
||||
$result['csrf_token'] = $newCsrfToken;
|
||||
}
|
||||
|
||||
// Discard any unexpected output
|
||||
|
||||
@@ -0,0 +1,160 @@
|
||||
<?php
|
||||
|
||||
/**
|
||||
* Adding a comment to a ticket: validation, access check, reply-parent check,
|
||||
* @mention extraction (audit-logged, and notified only to mentioned users who
|
||||
* can see the ticket), comment + watcher notifications.
|
||||
*
|
||||
* Shared by the web UI (api/add_comment.php) and the MCP add_comment tool so
|
||||
* both run one code path (tinker_tickets#111). Extracted verbatim from
|
||||
* add_comment.php. Returns result arrays; on validation failure the array
|
||||
* carries 'http_status' for HTTP callers. Callers own sessions/CSRF/responses.
|
||||
*/
|
||||
|
||||
require_once dirname(__DIR__) . '/models/TicketModel.php';
|
||||
require_once dirname(__DIR__) . '/models/CommentModel.php';
|
||||
require_once dirname(__DIR__) . '/models/AuditLogModel.php';
|
||||
require_once dirname(__DIR__) . '/helpers/NotificationHelper.php';
|
||||
require_once dirname(__DIR__) . '/helpers/SynapseHelper.php';
|
||||
|
||||
class CommentService
|
||||
{
|
||||
/**
|
||||
* @param array $currentUser Authenticated user row (user_id, username, display_name, groups, is_admin)
|
||||
* @param array $data ticket_id, comment_text, optional markdown_enabled / parent_comment_id
|
||||
*/
|
||||
public static function addComment(mysqli $conn, array $currentUser, array $data): array
|
||||
{
|
||||
$userId = $currentUser['user_id'];
|
||||
|
||||
$ticketId = isset($data['ticket_id']) ? trim((string)$data['ticket_id']) : '';
|
||||
if (!ctype_digit($ticketId) || (int)$ticketId <= 0) {
|
||||
return ['success' => false, 'error' => 'Invalid ticket ID', 'http_status' => 400];
|
||||
}
|
||||
|
||||
// Reject empty/whitespace-only comments
|
||||
$commentTextRaw = isset($data['comment_text']) ? trim((string)$data['comment_text']) : '';
|
||||
if ($commentTextRaw === '') {
|
||||
return ['success' => false, 'error' => 'Comment text cannot be empty', 'http_status' => 400];
|
||||
}
|
||||
|
||||
// Persist the trimmed text (not the raw client value) — matches update_comment.php
|
||||
// and keeps stored comment_text free of leading whitespace that could shift a
|
||||
// markdown-enabled comment's first line out of column 0 on reload.
|
||||
$data['comment_text'] = $commentTextRaw;
|
||||
|
||||
// Never trust a client-supplied display name — always attribute the comment to
|
||||
// the authenticated user.
|
||||
$data['user_name'] = $currentUser['display_name'] ?? $currentUser['username'] ?? 'User';
|
||||
|
||||
// Verify user can access the ticket before allowing a comment
|
||||
$ticketModel = new TicketModel($conn);
|
||||
$ticket = $ticketModel->getTicketById($ticketId);
|
||||
if (!$ticket) {
|
||||
return ['success' => false, 'error' => 'Ticket not found', 'http_status' => 404];
|
||||
}
|
||||
if (!$ticketModel->canUserAccessTicket($ticket, $currentUser)) {
|
||||
return ['success' => false, 'error' => 'Access denied', 'http_status' => 403];
|
||||
}
|
||||
|
||||
// Initialize models
|
||||
$commentModel = new CommentModel($conn);
|
||||
$auditLog = new AuditLogModel($conn);
|
||||
|
||||
// If replying, the parent comment must belong to this same (accessible) ticket.
|
||||
if (isset($data['parent_comment_id']) && $data['parent_comment_id'] !== null && $data['parent_comment_id'] !== '') {
|
||||
$parentComment = $commentModel->getCommentById((int)$data['parent_comment_id']);
|
||||
if (!$parentComment || (string)$parentComment['ticket_id'] !== (string)$ticketId) {
|
||||
return ['success' => false, 'error' => 'Invalid parent comment', 'http_status' => 400];
|
||||
}
|
||||
}
|
||||
|
||||
// Extract @mentions from comment text
|
||||
$mentions = $commentModel->extractMentions($data['comment_text'] ?? '');
|
||||
$mentionedUsers = [];
|
||||
if (!empty($mentions)) {
|
||||
$mentionedUsers = $commentModel->getMentionedUsers($mentions);
|
||||
}
|
||||
|
||||
// Add comment with user tracking
|
||||
$result = $commentModel->addComment($ticketId, $data, $userId);
|
||||
|
||||
// Log comment creation to audit log
|
||||
if ($result['success'] && isset($result['comment_id'])) {
|
||||
$auditLog->logCommentCreate($userId, $result['comment_id'], $ticketId);
|
||||
|
||||
// Log mentions to audit log
|
||||
foreach ($mentionedUsers as $mentionedUser) {
|
||||
$auditLog->log(
|
||||
$userId,
|
||||
'mention',
|
||||
'user',
|
||||
(string)$mentionedUser['user_id'],
|
||||
[
|
||||
'ticket_id' => $ticketId,
|
||||
'comment_id' => $result['comment_id'],
|
||||
'mentioned_username' => $mentionedUser['username']
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
// Matrix notifications
|
||||
$authorDisplay = $currentUser['display_name'] ?? $currentUser['username'] ?? null;
|
||||
$commentText = $data['comment_text'] ?? '';
|
||||
$ticketTitle = $ticket['title'] ?? "Ticket #{$ticketId}";
|
||||
$ticketVisibility = $ticket['visibility'] ?? 'public';
|
||||
|
||||
// @mention notifications — resolve usernames → Matrix IDs via Synapse Admin API.
|
||||
// Only notify mentioned users who actually have access to this ticket;
|
||||
// otherwise a mention would DM them the ticket's title and comment text
|
||||
// even though canUserAccessTicket() would deny them the ticket itself.
|
||||
$accessibleMentionedUsers = array_filter(
|
||||
$mentionedUsers,
|
||||
fn($u) => $ticketModel->canUserAccessTicket($ticket, $u)
|
||||
);
|
||||
if (!empty($accessibleMentionedUsers)) {
|
||||
$mentionedUsernames = array_column($accessibleMentionedUsers, 'username');
|
||||
$mentionedMatrixIds = SynapseHelper::resolveUsernames($mentionedUsernames);
|
||||
if (!empty($mentionedMatrixIds)) {
|
||||
NotificationHelper::sendMentionNotification($ticketId, $ticketTitle, $commentText, $authorDisplay, $mentionedMatrixIds);
|
||||
}
|
||||
}
|
||||
|
||||
// General comment notification (opt-in via MATRIX_NOTIFY_COMMENTS)
|
||||
if (!empty($GLOBALS['config']['MATRIX_NOTIFY_COMMENTS'])) {
|
||||
NotificationHelper::sendCommentNotification(
|
||||
$ticketId,
|
||||
$ticketTitle,
|
||||
$commentText,
|
||||
$authorDisplay,
|
||||
$ticketVisibility !== 'public',
|
||||
$ticketVisibility
|
||||
);
|
||||
}
|
||||
|
||||
// Notify watchers of the new comment
|
||||
NotificationHelper::notifyWatchers(
|
||||
$conn,
|
||||
$ticketId,
|
||||
$ticketTitle,
|
||||
'comment_added',
|
||||
['author' => $authorDisplay, 'preview' => mb_strimwidth($commentText, 0, 200, '…')],
|
||||
(int)$userId,
|
||||
$ticketVisibility
|
||||
);
|
||||
|
||||
// Add mentioned users to result for frontend
|
||||
$result['mentions'] = array_map(function ($u) {
|
||||
return $u['username'];
|
||||
}, $mentionedUsers);
|
||||
}
|
||||
|
||||
// Add user info to result for frontend avatar rendering
|
||||
if ($result['success']) {
|
||||
$result['user_name'] = $currentUser['display_name'] ?? $currentUser['username'];
|
||||
$result['user_id'] = $userId;
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user