LotusGuild/tinker_tickets
4
0
Fork 0
Code Issues 20 Pull Requests Actions Packages Projects Releases Wiki Activity

Fire notifications and fix audit_log shape for bulk status changes (#67, #74)

Browse Source 
BulkOperationsModel's bulk_close/bulk_status paths had zero references
to NotificationHelper — the exact same status transition (e.g.
Open->Closed) silently produced no Matrix/watcher notification when
performed via bulk actions, while the single-ticket edit page and
Bearer API both notify on every status change. Separately, their
audit_log entries used a bare ['status' => 'Closed', ...] shape
instead of the {'status': {'from': X, 'to': Y}} shape every other
status-change path uses, which broke two downstream consumers:
TicketView.php's timeline fell back to a generic "updated this
ticket" instead of "updated status", and notifications.php's
$details['status']['from'] on a string produced a broken "? -> ?"
notification title.

Fixed the audit_log shape for both operation types, and added a
notification queue collected during the per-ticket loop and flushed
only after a successful commit (so atomic-mode rollback correctly
sends zero notifications, matching how nothing else about a rolled-
back batch takes effect either). Also fixed an incidental bug found
while matching this to the single-ticket path: update_ticket.php's
notifyWatchers() call never passed the ticket's visibility, silently
defaulting to 'public' and always including the shared notify list
even for confidential/internal tickets — the exact leak #71 fixed
elsewhere in NotificationHelper itself, just never reaching this
call site.

Verified against real MariaDB with a real local webhook-capturing
server: bulk_close correctly fires sendStatusChangeNotification() +
notifyWatchers() with the right old/new status and a redacted title
for a confidential ticket; audit_log rows show the correct {from,to}
shape; and an atomic-mode rollback (one ticket's transition invalid)
sends zero notifications and leaves both tickets unchanged.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
This commit is contained in:
jared
2026-09-11 12:37:37 -04:00
co-authored by Claude Sonnet 5
parent 5a69c41f48
commit 6adbb29964
2 changed files with 63 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
api/update_ticket.php
+2 -1
View File
@@ -276,7 +276,8 @@ try {
$updateData['title'],
'status_changed',
['old_status' => $currentTicket['status'], 'new_status' => $updateData['status'], 'changed_by' => $changedBy],
(int)$this->userId
(int)$this->userId,
$currentTicket['visibility'] ?? 'public'
);
}
models/BulkOperationsModel.php
+61 -2
View File
@@ -125,13 +125,23 @@ class BulkOperationsModel
$processed = 0;
$failed = 0;
$errors = [];
// Status-change notifications collected during the loop below and
// sent only after a successful commit, matching how the single-ticket
// and Bearer API paths never notify for a change that didn't durably
// land (and how an atomic-mode rollback must not fire any at all).
$notificationQueue = [];
// Load required models
require_once dirname(__DIR__) . '/models/TicketModel.php';
require_once dirname(__DIR__) . '/models/AuditLogModel.php';
require_once dirname(__DIR__) . '/models/UserModel.php';
require_once dirname(__DIR__) . '/helpers/NotificationHelper.php';
$ticketModel = new TicketModel($this->conn);
$auditLogModel = new AuditLogModel($this->conn);
$userModel = new UserModel($this->conn);
$actor = $operation['performed_by'] ? $userModel->getUserById((int)$operation['performed_by']) : null;
$changedByDisplay = $actor['display_name'] ?? $actor['username'] ?? null;
// Batch load all tickets in one query to eliminate N+1 problem
$ticketsById = $ticketModel->getTicketsByIds($ticketIds);
@@ -221,8 +231,18 @@ class BulkOperationsModel
'update',
'ticket',
$ticketId,
['status' => 'Closed', 'bulk_operation_id' => $operationId]
[
'status' => ['from' => $currentTicket['status'], 'to' => 'Closed'],
'bulk_operation_id' => $operationId,
]
);
$notificationQueue[] = [
'ticketId' => $ticketId,
'title' => $currentTicket['title'],
'visibility' => $currentTicket['visibility'] ?? 'public',
'oldStatus' => $currentTicket['status'],
'newStatus' => 'Closed',
];
}
}
break;
@@ -291,8 +311,18 @@ class BulkOperationsModel
'update',
'ticket',
$ticketId,
['status' => $parameters['status'], 'bulk_operation_id' => $operationId]
[
'status' => ['from' => $currentTicket['status'], 'to' => $parameters['status']],
'bulk_operation_id' => $operationId,
]
);
$notificationQueue[] = [
'ticketId' => $ticketId,
'title' => $currentTicket['title'],
'visibility' => $currentTicket['visibility'] ?? 'public',
'oldStatus' => $currentTicket['status'],
'newStatus' => $parameters['status'],
];
}
}
}
@@ -364,6 +394,35 @@ class BulkOperationsModel
@unlink($path);
}
}
// Fire the same Matrix/watcher notifications the single-ticket and
// Bearer API status-change paths send, now that every change in
// this batch is durably committed. Best-effort: a notification
// failure must never turn an otherwise-successful bulk operation
// into an error.
foreach ($notificationQueue as $n) {
try {
NotificationHelper::sendStatusChangeNotification(
$n['ticketId'],
$n['oldStatus'],
$n['newStatus'],
$n['title'],
$changedByDisplay,
$n['visibility']
);
NotificationHelper::notifyWatchers(
$this->conn,
$n['ticketId'],
$n['title'],
'status_changed',
['old_status' => $n['oldStatus'], 'new_status' => $n['newStatus'], 'changed_by' => $changedByDisplay],
(int)$operation['performed_by'],
$n['visibility']
);
} catch (Throwable $e) {
error_log("Bulk operation $operationId: notification failed for ticket {$n['ticketId']}: " . $e->getMessage());
}
}
} catch (Exception $e) {
// Rollback on any unexpected error
$this->conn->rollback();