Skip to content

Notifications Developer Guide

Ed Mozley edited this page Aug 12, 2026 · 1 revision

Notifications β€” Developer Guide

Why the bell is a subscriber rather than a new instrumentation layer, where the four noise rules are enforced, and the four bugs the build surfaced β€” three of which would have presented as "notifications just never arrive". The plain-language version is Notifications.

Asked for in discussion #55. Shipped in bbd49c50, deep-link fix in 75803509.


1. πŸ“ The files involved

🟒 The feature

File Role
includes/services/notifications.php NotificationsService β€” the write path and all four noise rules
includes/notifications_router.php Event β†’ recipient β†’ display text

🟠 Where it hooks in

File Role
workflow/includes/engine.php One hook at the top of dispatch() β€” the single funnel
api/tickets/bulk_update_tickets.php Wraps its loop in duringBulk()
api/tickets/bulk_delete_tickets.php Same

πŸ”΅ New events

File Role
includes/services/tickets.php createNote() now dispatches ticket.note_added
api/self-service/reply_ticket.php ticket.reply_received, source portal
api/tickets/check_mailbox_email.php ticket.reply_received, source email, non-initial only

βšͺ UI and API

File Role
includes/waffle-menu.php renderNotificationBell() β€” on every module header
api/notifications/get_notifications.php List, plus ?count_only=1 for the badge poll
api/notifications/mark_read.php {ids:[…]} or {all:true}
system/preferences/index.php Per-type switches
lang/en/common.php notifications.event.* and notifications.pref.*

πŸ”΄ Schema

Table Role
notifications One row per thing an analyst is told, with event_count for coalescing and read_datetime (NULL = unread)

2. 🧠 The bell is a SUBSCRIBER, not instrumentation

This is the decision that made a large-sounding feature small.

WorkflowEngine::dispatch() was already a system-wide event bus: ~32 event types fired from 48 call sites across tickets, tasks, changes, problems, knowledge, assets, SLA, forms, service status and suppliers. Four of the six things Daniel asked for were already being announced.

So the bell hooks that one funnel and answers two questions per event:

public static function dispatch(string $event, array $payload): void
{
    // Its own try/catch, and BEFORE the workflow loop, so neither can stop the
    // other: a broken workflow must not cost somebody their notification.
    try {
        require_once __DIR__ . '/../../includes/notifications_router.php';
        notificationsHandleEvent($event, $payload);
    } catch (Throwable $nEx) { … }

    try { /* existing workflow dispatch */ }
}

⚠️ Adding a notification type should never mean editing call sites. If the event already dispatches, add it to NotificationsService::types() and give it a case in the router. If it does not dispatch yet, add the dispatch β€” workflows gain the trigger too, which is the right trade either way.


3. 🧠 The noise rules, and where they live

All four are enforced inside NotificationsService::notify(), not at the call sites, so nothing can bypass them by accident.

Rule 1 β€” never notify you about your own action

The single biggest source of noise. It needed something that did not exist: no dispatch payload carried an actor. The ticket payload has owner_id, created_by, assigned_analyst_id β€” nothing saying who made this change.

Rather than threading an actor through 48 call sites, it is resolved centrally from the session:

function notificationsCurrentActor(): array
{
    $id   = isset($_SESSION['analyst_id']) ? (int)$_SESSION['analyst_id'] : 0;
    $name = isset($_SESSION['analyst_name']) ? (string)$_SESSION['analyst_name'] : '';
    return [$id, $name !== '' ? $name : null];
}

Two properties fall out of that, both wanted:

  • A web request is caused by whoever is signed in. Correct.
  • A cron run has no session, so the actor is nobody β€” which is why a system-generated SLA breach still reaches the assignee even though they own the ticket. Correct, and it would have been fiddly to special-case.

Safe after session_start(['read_and_close' => true]): the array stays in memory for the rest of the request even though the lock is gone.

Rule 2 β€” bulk

⚠️ Both bulk endpoints loop the service one record at a time, deliberately (see the long comment at the top of bulk_update_tickets.php) so every record gets the same validation, audit and dispatch a single edit would. That also means fifty tickets fire fifty events.

NotificationsService::duringBulk(function () use (…) {
    foreach ($ids as $rawId) { TicketsService::updateTicket(…); }
});

duringBulk() restores the previous state in a finally, so an exception mid-loop cannot leave suppression stuck on for the rest of the request.

Rule 3 β€” coalesce

Keyed on the object, not the event type:

WHERE analyst_id = ? AND entity_type = ? AND entity_id = ?
  AND read_datetime IS NULL
  AND updated_datetime >= DATE_SUB(UTC_TIMESTAMP(), INTERVAL ? MINUTE)

That is the point: "this ticket moved three times" is one piece of news. Keying on event type would give three rows that happen to be about one ticket, which is the thing being avoided.

The row keeps the most recent event type and body, so an assignment followed by a reply shows the reply.

Rule 4 β€” per-type defaults

NotificationsService::types() is the single registry: the type list, its default, and its entity kind. The Preferences page renders from it, so adding a type there adds the switch. Overrides are one JSON row in user_preferences, because this is read on every event written and one row keeps that to a single lookup.


4. πŸ› Four bugs, three of which looked like "nothing happened"

This is the part worth reading. Every one of these failed silently β€” the feature appeared to work and simply produced no notification.

I18n::t() splits keys on every dot

The lang file had:

'event' => [ 'ticket.assigned' => 'Assigned to you by {actor}', … ]

I18n::t() does explode('.', $path) over the whole key, so notifications.event.ticket.assigned looks for event β†’ ticket β†’ assigned. A literal 'ticket.assigned' key at that level is unreachable.

The bell fell back to showing a bare name β€” "Sarah Williams" with no indication of what she had done β€” and the reply notification, which has no actor, rendered a blank line. Only caught by looking at a screenshot.

⚠️ Event types contain dots. Lang keys built from them must be NESTED, never flat.

requester_email is not a column on tickets

It lives on the user and reaches the payload through a join in loadTicket(). Both new dispatch sites selected it directly from tickets, which throws β€” and the throw landed in the dispatch's own catch, so the symptom was that replies never notified anyone. Both now LEFT JOIN users.

The portal reply loaded only id, subject

So the payload had no assigned_analyst_id, the router found no recipient, and the single most-wanted notification in the whole feature reached nobody. The query is now widened, with a comment saying why those columns are there.

The deep link used a parameter nothing reads

Three spellings exist in the codebase β€” ?ticket_id=, ?id= and ?ticket= β€” and assets/js/inbox.js reads exactly one of them:

const ticketId = urlParams.get('ticket_id');
if (ticketId) setTimeout(() => loadTicketById(ticketId), 500);

The wrong parameter still returns HTTP 200 with the inbox rendered. It looks like a working link right up until you notice the ticket never opened.

πŸ”΄ Two other places in the codebase still use tickets/?id= (api/integrations/escalate_ticket.php, workflow/includes/engine.php) and one uses tickets/?ticket= with a ticket number (forms/approvals.php). Those links do not open a ticket either. Not fixed here β€” noted so somebody can.


5. ⚠️ Verifying this feature

Every failure mode looks identical to success. A suppressed notification, a rejected one, a thrown-and-swallowed one and a correctly-quiet one all present as an empty table. So every rule needs a positive control, not just a negative one:

Rule Negative Positive
Own action Admin edits admin's ticket β†’ 0 rows Admin edits another analyst's ticket β†’ 1 row
Bulk Bulk of 3 β†’ 0 rows Same change made singly β†’ 1 row
Coalesce β€” 3 changes β†’ 1 row, event_count = 3
Type off Type disabled β†’ 0 rows Type re-enabled β†’ 1 row

Forging a session to act as another analyst is the practical way to test rule 1. ⚠️ Include analyst_name β€” ActorContext and the router both read it, and its absence produces a notification with no actor rather than an obvious error.


6. Extending it

  • A new type on an existing event: add to types(), add a case in the router's body/entity helpers, add the two lang keys (nested).
  • A new type on a new event: add the dispatch() call and register the trigger in availableTriggers() β€” workflows benefit too.
  • A different recipient rule: notificationsRecipientFor() is the only function that decides. A ticket_watchers table would change that one function and nothing else.

7. πŸ”΄ Outstanding

  • War Room mentions are a separate widget. warroom_mentions has no read/unread column, so folding it into the bell means giving it one. Daniel's "mentions of the user" bullet is really this.
  • No workflow action to raise a notification from a rule β€” an admin cannot yet wire up an arbitrary one without code.
  • No browser notifications. Requires a secure context; many installs are plain HTTP.
  • 23 locales. All new strings are English-only and fall back silently.
  • Nothing prunes old read notifications. Worth a retention sweep before this has been running a year.

Related pages

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Blue sky thinking

Bugs resolved

Links

Clone this wiki locally