processBulkOperation() validated each ticket's status/priority transition
against a pre-transaction snapshot (ticketsById) instead of re-reading
inside the transaction, so two concurrent bulk operations touching the
same ticket could both pass validation against stale data and one
transition could silently clobber the other. Add lockTicketForUpdate(),
which re-fetches a ticket via SELECT ... FOR UPDATE, and use it wherever
the loop needs current status/priority, removing the three redundant
re-reads from the stale snapshot in the bulk_close/bulk_priority/
bulk_status branches.
Verified against real MariaDB with two concurrent OS processes: the
second process blocked ~1.2s on the first's held row lock, then correctly
observed the first's committed status and rejected an otherwise-stale-
data-permitted invalid transition. Also regression-tested normal
bulk_close/bulk_priority operation on real tickets.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
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
Found while verifying #21 against the live schema: the model writes
'completed_with_errors' (21 chars) when a bulk operation finishes with
per-ticket failures, but bulk_operations.status was varchar(20), so the
write failed with "Data too long for column 'status'".
This was latent — bulk status changes previously forced every transition
through, so failed was always 0. Now that they honour the Workflow
Designer, a partially-skipped batch is a normal outcome and hits it.
- migrations/001 widens the column to varchar(32) (idempotent).
- The baseline is updated to match, for fresh installs.
- The bookkeeping UPDATE is wrapped in a try/catch: it runs after the
ticket changes are committed, so an instance deployed ahead of its
migrations must not turn a completed operation into an error response.
Verified against the live database with a disposable-ticket harness:
comment-required rejection changes nothing, undefined transitions are
refused per ticket with a reason, allowed transitions still work, mixed
batches apply the valid half, and an already-Closed ticket is a no-op.
Bulk status changes previously bypassed the workflow entirely — the model
carried an explicit "admin-only escape hatch" note — so bulk edit could
drive tickets through transitions the designer forbids and skip comments
the designer requires.
BulkOperationsModel now applies the same rules as the single-ticket path:
- Transitions absent from status_transitions are refused per ticket and
reported with a reason, instead of being forced through.
- requires_comment is checked up front across the whole selection, so a
batch is rejected before any ticket is mutated rather than half-applied.
- The reason is persisted as a comment on each ticket changed, matching
what a single-ticket close records.
- Tickets already in the target status are a no-op success, not a failure.
requires_admin needs no extra check: api/bulk_operation.php already gates
the endpoint on admin.
Client: both bulk modals now collect a reason, the close path gets a real
modal instead of a bare confirm, and per-ticket skip reasons surface in
the result toast instead of a bare failure count.
- 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>
- SynapseHelper: memoize username->Matrix-ID lookups per-request (incl. negative
results) and add an overall time budget to resolveUsernames() plus a 2s connect
timeout, so notifying N watchers with a slow/unreachable Synapse can't stall the
request for N x 5s. (Chosen over async/queue per maintainer.)
- DependencyModel: fix cycle detection treating 'blocks' and 'blocked_by' as the
same edge direction. They are inverse relationships (single row each, no mirror
row), so the traversal now walks a unified precedence graph (blocks: ticket->
depends_on; blocked_by: depends_on->ticket) and wouldCreateCycle normalizes the
new edge's direction. Prevents both false-positive and missed cycles.
- CacheHelper: anchor prefix-delete to exact key boundaries (bare prefix or
prefix + '_' + md5) so delete('workflow') can't wipe a 'workflow_rules' cache.
- RateLimitMiddleware: hold an exclusive flock across the per-IP counter's
read-modify-write so concurrent requests can't both read N and write N+1
(undercounting past the limit). Fails open if the file can't be locked.
- dashboard.js: kanban status update now uses lt.api.post (per no-raw-fetch
convention) and reverts the card AND the optimistic column counts on failure
(the old raw-fetch catch left the card moved without reverting).
- BulkOperationsModel: document that bulk_status/bulk_close intentionally bypass
workflow transition validation (admin override, by design).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Continued fixes from the multi-agent review:
- recurring tickets cron: now that the parse error is fixed the job runs,
exposing two latent bugs. (1) next_run_at was only advanced after the
full success path, so any failure (e.g. a NULL created_by passed to the
non-nullable assignTicket() $assignedBy -> TypeError) left it in the past
and re-created a duplicate ticket every cron cycle. Added an atomic
claimForRun() (conditional UPDATE gated on still-due) called BEFORE
creation, which also prevents overlapping runs from double-creating.
(2) The cron used a raw mysqli with no utf8mb4, corrupting non-ASCII
content; it now uses Database::getConnection(). Also guard the assignment
so created_by NULL falls back to the assignee.
- bulk delete: attachment files were unlinked inside the DB transaction, so
an atomic-mode rollback restored rows but the files were already gone.
deleteTicket() can now defer file removal to the caller, and
BulkOperationsModel deletes files only after a successful commit.
- UserModel: back-tick the `groups` column (reserved word on MySQL 8.0.2+).
- create_ticket_api.php: stop leaking raw DB/exception messages to callers;
log server-side and return a generic error. (Also includes a pre-existing
working-tree tweak that adds title to the manual-ticket dedupe hash.)
- CI: semgrep install failed under PEP 668; add --break-system-packages.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- TicketModel: add deleteTicket() that removes all child records
(comments, watchers, dependencies, attachments, custom fields)
then deletes the ticket and cleans up physical attachment files
- BulkOperationsModel: add bulk_delete case to processBulkOperation()
so the "Bulk Delete" UI button actually works instead of silently
failing with N failures
- bulk_operation.php: validate operation_type against whitelist to
reject unknown operations early with a proper error
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Consolidate StatsModel queries from 12 to 3 using conditional aggregation
- Add input validation to DashboardController (sort columns, dates, priorities)
- Combine getCategories/getTypes into single query
- Add transaction support to BulkOperationsModel with atomic mode option
- Add depth limit (20) to dependency cycle detection to prevent DoS
- Add caching to UserModel.getAllGroups() with 5-minute TTL
- Improve ticket ID generation with 50 attempts, exponential backoff, and fallback
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Consolidate all 20 API files to use centralized Database helper
- Add optimistic locking to ticket updates to prevent concurrent conflicts
- Add caching to StatsModel (60s TTL) for dashboard performance
- Add health check endpoint (api/health.php) for monitoring
- Improve rate limit cleanup with cron script and efficient DirectoryIterator
- Enable rate limit response headers (X-RateLimit-*)
- Add audit logging for workflow transitions
- Log Discord webhook failures instead of silencing
- Fix visibility check on export_tickets.php
- Add database migration system with performance indexes
- Fix cron recurring tickets to use assignTicket method
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive bulk operations system for admins:
- Created BulkOperationsModel.php with operation tracking and processing
- Added bulk_operation.php API endpoint for bulk operations
- Created get_users.php API endpoint for user dropdown in bulk assign
- Updated DashboardView.php with checkboxes and bulk actions toolbar
- Added JavaScript functions for:
- Select all/clear selection
- Bulk close tickets
- Bulk assign tickets
- Bulk change priority
- Added comprehensive CSS for bulk actions toolbar and modals
- All bulk operations are admin-only (enforced server-side)
- Operations tracked in bulk_operations table with audit logging
- Supports bulk_close, bulk_assign, and bulk_priority operations
Admins can now select multiple tickets and perform batch operations, significantly improving workflow efficiency.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
jaredandClaude Sonnet 5