Skip to content

Checklist Closure Gating Developer Guide

Ed Mozley edited this page Sep 24, 2026 · 1 revision

Checklist closure gating β€” developer guide

User-facing page: Stopping a ticket closing with a checklist outstanding. Contributor review of what changed before merge: Checklists β€” closure gating.


The gate

assertClosureAllowed() is the single choke point. Every route that closes a ticket reaches it β€” the reading pane, the context menu, bulk actions and the workflow engine β€” because an unenforced gate reports control that is not there, which is worse than no gate: it tells an auditor the steps were followed.

The level lives on the template, not on the install. The person writing the procedure knows whether skipping a step is a judgement call; an install-wide switch would force a new-starter checklist and a fire-panel isolation to be the same kind of list.

Standard is the default, so an upgrade blocks nothing that was not blocking before.


πŸ”΄ Both halves must read the same company

assertClosureAllowed() resolves the "ticket with no checklist attached" setting, and the attached-checklist path resolved it separately. On a multi-company install those two could resolve different companies for one ticket, so a ticket could be blocked by one half and allowed by the other.

⚠️ tenantSetting() memoises in a static. Reading it once per request per key is fine; reading it for two different companies in one request returns the first answer both times. This trap is already written up in the multi-tenancy developer guide and was walked into anyway.

πŸ”΄ The audit note that named the wrong person

As contributed, the workflow closure path wrote its audit note with a hardcoded analyst_id = 1, inside an empty catch. Two failures at once:

  • ticket_audit.analyst_id is a NOT NULL foreign key, so on an install where analyst 1 has been removed the insert throws β€” and the empty catch swallowed it, so the note simply vanished with nothing recorded anywhere.
  • Where analyst 1 does exist, the note named a real person who had not done it.

A workflow is not a person. The actor id is NULL for that case now, and the column is nullable for exactly this reason β€” the same shape as a requester closing their own ticket.

πŸ”΄ A loop variable that shadowed t()

The padlock tooltip was written typeof t === 'function' ? t('tickets.checklists.locked') : '…' inside a loop whose variable was also called t. The global translation function was shadowed, the guard therefore always took the fallback branch, and the tooltip could never be translated in any language.

A locale coverage score cannot see this: the key exists in every locale, nothing is missing and nothing is blank. It is one of the defects a 100% score cannot see.


Related i18n work

The Checklists screens were showing in English in every language, and no coverage report could see that either β€” the screens had never been wired to t() at all, so the strings were missing from lang/en itself. A locale cannot be missing a key that the source of truth does not have.

scripts/i18n_unwired.php exists to find that class of gap: markup that renders literal English rather than calling t().


See also

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Blue sky thinking

Bugs resolved

Links

Clone this wiki locally