Ticket Notes Developer Guide

Ticket notes — developer guide

How a note's visibility works, how a shared note's files reach the portal, and how the note_shared email trigger fires — with the rules that keep an internal note internal.

See Ticket notes: internal or shared for what the feature does.

---

1. One flag decides everything

ticket_notes.is_internal — 1 by default, and every consumer keys off it.

// includes/services/tickets.php — createNote()
$isInternal = array_key_exists('is_internal', $in) ? (bool)$in['is_internal'] : true;

Default to internal, always. A note whose visibility is unstated must be private: the failure of guessing wrong in that direction is a private remark reaching a customer.

Never key off the status name, or the word "Done"

The same rule that bit issue #88 applies here. Ask the flag, never a label:

// ✅
const isShared = note.is_internal === false;

// ❌ there is no such field, and if there were it would be a display string
const isShared = note.visibility === 'Shared';

---

2. Rendering the label

get_notes.php already returns is_internal, cast to a real bool:

$note['is_internal'] = (bool)$note['is_internal'];

// assets/js/inbox.js — renderNotes()
const isShared = note.is_internal === false;

// A tracker-imported note is NEITHER — it is not ours to describe as internal
// or shared — so it gets no visibility label at all.
const visClass = note.source ? '' : (isShared ? ' note-item-shared' : ' note-item-internal');

Label both states. If only shared notes were marked, "no label" would have to mean internal by implication — and any note kind added later would silently inherit that meaning.

The styling reuses an existing signal

.note-item already says what kind of note something is through its left border colour — that is how a tracker-imported note is distinguished. Visibility extends the same idiom rather than introducing a second visual language:

.note-item          { border-left: 3px solid var(--accent); }   /* internal — unchanged */
.note-item-external { border-left-color: var(--text-muted); }   /* from a tracker */
.note-item-shared   { border-left-color: var(--success-accent); }

Use the real semantic token sets (--success-bg / --success-text / --success-border), which are defined for both themes. Check any token you reach for actually exists in theme.css before using it.

---

3. Serving a shared note's files to the portal

api/self-service/get_document.php is the portal twin of api/documents/download.php, exactly as get_attachment.php is the twin of the analyst-side attachment route.

The whole security model is one join:

"SELECT d.id, d.kind, d.title, d.storage_key, d.original_name, d.external_url
   FROM documents d
   JOIN document_links dl ON dl.document_id = d.id
                         AND dl.parent_type = 'ticket_note'
   JOIN ticket_notes n    ON n.id = dl.parent_id
                         AND n.is_internal = 0
   JOIN tickets t         ON t.id = n.ticket_id
  WHERE d.id = ?
    AND d.deleted_datetime IS NULL
    AND t.user_id = ?
    AND t.deleted_datetime IS NULL
  LIMIT 1"

Four rules, none of them a separate step:

Condition	Stops
dl.parent_type = 'ticket_note'	a document on an asset or contract becoming portal-reachable
n.is_internal = 0	an internal note's files being served
t.user_id = ?	another customer's files
d.deleted_datetime IS NULL	a deleted document

Put the visibility condition in the ownership query, not beside it. A second if is something a later change can reorder, short-circuit or forget. A join condition is not.

Every miss is a 404, whatever the reason, so a wrong id and someone else's id are indistinguishable from outside and ids cannot be enumerated.

Listing is not authorisation

get_ticket_detail.php lists a shared note's documents, but the download endpoint re-checks everything anyway:

Hiding a link while the URL still works is decoration, and the file is the sensitive part.

The portal renders analyst-typed URLs

A DMS entry's external_url is typed by an analyst and now reaches a customer's browser. It must never become an href on trust:

// self-service/tickets.php
function safeExternalUrl(url) {
    return /^https?:\/\//i.test(String(url || '')) ? String(url) : '';
}

Without that, javascript: in that box runs for every requester who opens the ticket. The analyst side has the same guard in two places; the portal had none until note files arrived there.

---

4. The note_shared email trigger

Sent from the choke point every note already passes through:

// includes/services/tickets.php — createNote(), after the insert
if (!$isInternal) {
    require_once __DIR__ . '/../template_email.php';
    sendTemplateEmail($conn, $ticketId, 'note_shared', [
        'note_text' => $text,
    ]);
}

Four things worth copying if you add a trigger of your own:

Guard first. if (!$isInternal) is the outermost condition, not a check somewhere inside.

Name the trigger for what fires it. note_shared, not note_added. An administrator reading the dropdown should not need to know a hidden rule to predict its behaviour — and the misnaming is precisely what caused the original report.

Off until configured. getActiveTemplate() returns null when no active template carries the trigger, and sendTemplateEmail() then returns silently. Adding a trigger therefore changes nothing on an existing install. Make that your first test.

Non-fatal. Wrap it. The note is saved whether or not the mail goes, and it is already visible in the portal:

} catch (Exception $mailEx) {
    error_log('note_shared template email failed for ticket ' . $ticketId . ': ' . $mailEx->getMessage());
}

Adding a trigger — the full checklist

Where	What
api/tickets/save_email_template.php	add to $validEvents — it fails closed, so a missing entry rejects the save
tickets/settings/index.php	three places: the template modal, the simulator dropdown, and EVENT_LABELS
lang/*/tickets.php	event_<name> label
the sending code	one guarded sendTemplateEmail() call

Merge codes

$extraMergeData is how a trigger contributes placeholders the ticket cannot supply — CSAT passes csat_link the same way:

sendTemplateEmail($conn, $ticketId, 'note_shared', ['note_text' => $text]);

Letting the template decide how much it says is what keeps this from duplicating Reply: include [note_text] and the email carries the note; leave it out and it points at [ticket_url].

---

5. ⚠️ Escaping: the template decides, not the data

This one caught all four pre-existing triggers, and is the trap to understand before touching template_email.php.

Merge codes are substituted before the body is assembled. So sniffing the assembled body to decide whether it is HTML means a data value decides it:

template : "Hi,\n[note_text]"                      ← plain text
note     : "use the <table> in room 2"
merged   : "Hi,\nuse the <table> in room 2"        ← now contains a tag
verdict  : "this is HTML"  →  nothing is escaped

A note mentioning a tag turned an otherwise plain-text template into an HTML one, and everything else in it silently stopped being escaped.

Whether a template is HTML is a property of what the administrator wrote, not of today's data. So decide it first, from the template:

$bodyIsHtml = strip_tags($template['body_template']) !== $template['body_template'];

Then escape accordingly, and pass the verdict along rather than letting it be re-sniffed:

if ($bodyIsHtml) {
    foreach ($extraMergeData as $k => $v) {
        $mergeData[$k] = nl2br(htmlspecialchars((string)$v, ENT_QUOTES, 'UTF-8'));
    }
}
…
$fullBody = buildTemplateEmailBody($body, $ticketNumber, $bodyIsHtml);

Escape only for HTML templates. For a plain-text template buildTemplateEmailBody() escapes the whole body afterwards — escaping first as well shows the customer literal <br> and <.

buildTemplateEmailBody() keeps the old sniff as a null default, for the one caller that has no template to inspect.

---

6. How to test this area

Prove it is off before proving it is on. The first assertion should be that no template configured sends nothing — otherwise a passing "it sends!" test tells you nothing about the default.

Pair every refusal with a positive control. An endpoint that always 404s passes every security test ever written. The portal document tests are:

a shared note on my own ticket	200, bytes returned ← the control
an internal note on my own ticket	404
a shared note on another customer's ticket	404
a document hanging off an asset	404
a deleted document on a shared note	404
no portal session at all	401

Test the escaping with a value that fights back. "Use the <table> in room 2\nLine two & \"quoted\"" exercises a tag, an ampersand, a quote and a newline in one string — and check it is escaped exactly once, not twice.

Drive the real page for anything in inbox.js. It is a large file where a stray backtick inside a template literal stops the whole thing parsing while the page still renders perfectly, because the HTML is server-side. Ask window whether the functions exist.

---

Related pages

• Ticket notes: internal or shared
• Attached documents — developer guide
• Self-Service — developer guide
• Email template sender rules — developer guide