Checklists on Tasks Developer Guide

Checklists on tasks - Developer Guide

Shipped in 2.10.0 · Round three of discussion #138 · User-facing page: Checklists & SOPs § Using one on a task

How checklists reach the Tasks module, file by file, with the real code - and the security hole in the ticket side of the module that was found while building it. Read The traps before changing how a task is completed.

Earlier rounds: Checklists module - house style (round one, the module itself) and Closure gating - Developer Guide / the review (round two, Standard and Critical).

---

Where this came from

Templates have always had an Applies to field - Tickets, Tasks or Both - but nothing on the Tasks side used it. In his round-two post Santhosh (Sandy) asked whether to tidy the editor so it only advertised tickets, or build the Tasks half, and gave his own view: probably not, because

Subtasks: assigning chunks of work across people/teams with due dates. Checklists: in-the-moment procedural steps to get a specific job done right.

and teams without good process hygiene would use the two interchangeably.

Ed's answer was to build it behind a switch - the same shape as time recording on tasks. A team that would be confused never sees it; a team doing procedure-led work (patching, decommissioning, monthly checks) gets it. That answers Sandy's concern rather than overruling it, and the switch is off by default.

---

The design in one paragraph

A task checklist is the ticket feature with the noun changed. Attaching copies the template's steps; ticking records who and when; a step that asks for a value needs one; Standard warns and records, Critical blocks; the company's block all floor applies. The rules live in ChecklistsService, the completion gate is called from every path that completes a task, and switching the feature off stops enforcing it.

---

Files

🗄️ schema · ⚙️ service · 🔌 endpoint · 🔀 completion paths · 🖥️ UI · 🌐 strings · ❓ help · 🧪 Feature Bingo · 🔒 security fix

🎨	File	What it does
🗄️	database/freeitsm.sql, includes/db_verify_schema.php	task_checklists, task_checklist_items - the ticket tables' shape, keyed on a task; task_checklists.sort_order for the order of a task's checklists
🗄️	includes/db_verify_indexes.php	Regenerated (two new keys) - generated, never hand-edited
⚙️	includes/services/checklists.php	The whole task section: setting, list, attach, tick, remove, gate, audit, recurrence copy, delete
⚙️	includes/services/tasks.php	Gate + record in updateTask() and moveTask(); cleanup in deleteTask()
⚙️	includes/services/task_recurrence.php	Fresh checklist copies on each new occurrence
🔀	api/tasks/reorder.php	The kanban drag - outside the service, so it calls the gate itself
🔀	api/tasks/toggle_subtask.php	The subtask tick - same
🔌	api/tasks/checklists.php	New. The panel's endpoint
🔌	api/tasks/get_settings.php, save_settings.php	checklists_enabled, default '0'
🖥️	includes/capabilities.php, tasks/settings/manifest.php, tasks/settings/index.php	Cap::TASKS_CHECKLISTS and the Checklists settings tab
🖥️	assets/js/tasks.js, assets/css/tasks.css	The panel section; the warning before every completion route
🖥️	checklists/edit/index.php	Hint under Applies to while the feature is off
🔒	api/tickets/ticket_checklists.php	Company and module checks added to every action - see section 7
🔒	checklists/ticket_view.js	Two root-absolute URLs fixed
🌐	lang/en/tasks.php, lang/en/checklists.php	New English strings (owed to the other locales)
❓	tasks/help.php, checklists/help.php	Settings section; the Applies to step explains the switch
🧪	includes/feature_bingo/cards/tasks.php	tasks.checklists_enabled, tasks.checklist_attached

---

1. The switch

// ChecklistsService
public static function tasksEnabled(PDO $conn): bool
{
    try {
        $st = $conn->prepare("SELECT setting_value FROM system_settings WHERE setting_key = 'tasks_checklists_enabled'");
        $st->execute();
        return (string)$st->fetchColumn() === '1';
    } catch (Throwable $e) {
        return false;
    }
}

• Install-wide, in system_settings, like tasks_time_scope and tasks_collaborator_completion beside it. Not per company: it decides whether a feature exists, not how a company runs it.
• Exactly '1'. save_settings.php normalises anything truthy to '1' and everything else to '0', so the column only ever holds one of two values.
• Its own permission, Cap::TASKS_CHECKLISTS, declared once in tasks/settings/manifest.php; the settings writer authorises per key from that manifest, so the tab that shows the switch and the permission that guards it cannot drift apart.

🔴 Off means off

public static function taskCompletionCheck(PDO $conn, int $taskId): array
{
    $out = ['blocking' => [], 'warning' => []];
    if (!self::tasksEnabled($conn)) return $out;
    …

Switching the feature off stops enforcing it, even on tasks that still carry checklists. The alternative - a Critical checklist refusing completion from behind a section nobody can see - is a rule the admin has switched off still being applied. Nothing is deleted, so switching it back on restores everything.

The endpoint follows the same rule: GET says enabled: false and returns nothing, and every write is refused.

---

2. The data

task_checklists / task_checklist_items are the ticket tables with ticket_id swapped for task_id:

CREATE TABLE IF NOT EXISTS `task_checklists` (
    `id`, `task_id`, `template_id`, `title`,
    `closure_mode` ENUM('warn','block') NOT NULL DEFAULT 'warn',
    `created_by_id`, `created_datetime`, `is_demo`, …
);
CREATE TABLE IF NOT EXISTS `task_checklist_items` (
    `id`, `task_checklist_id`, `title`, `suggested_role`, `is_mandatory`, `is_completed`,
    `completed_by_id`, `completed_by_name`, `completed_datetime`,
    `requires_input`, `input_placeholder`, `response_value`, `sort_order`, `is_demo`, …
);

Why two new tables rather than one generic table with an entity_type? Every existing query, index, the workflow engine's checklist step ticked trigger and the round-two gate read the ticket tables by name. A generic table would have meant migrating all of that to add a feature beside it. Two tables with the same shape keep the ticket side untouched, and the service methods make the parallel obvious.

Children are deleted by the code. The fresh-install dump declares ON DELETE CASCADE, but Database Verification creates tables without foreign keys, so on a grown install the cascade does not exist. removeTaskChecklist() and deleteForTasks() delete the steps first, then the parent - the round-one rule.

---

3. Attaching, ticking, removing

public static function attachTemplateToTask(PDO $conn, ActorContext $ctx, int $taskId, int $templateId): int
{
    …
    if (!in_array((string)$t['scope'], ['task', 'both'], true)) {
        throw new ServiceError('validation', 'invalid_field', 'That checklist is for tickets only.');
    }
    return self::copyTemplateToTask($conn, $taskId, $templateId, (string)$t['title'],
        (string)$t['closure_mode'], $ctx->actorId > 0 ? $ctx->actorId : null);
}

• The copy is the point. Rewriting a procedure must not change a task half-way through it - exactly as on tickets.
• Scope is enforced on the server, not only by what the picker offers. A tickets-only checklist is refused even if somebody posts its id by hand.
• copyTemplateToTask() is the one place a task checklist is written, used by attach and by recurrence, in a transaction (or inside the caller's).

Ticking:

public static function toggleTaskItem(PDO $conn, ActorContext $ctx, int $itemId, bool $completed, ?string $value): array
{
    …
    // A value is only kept on a step that asks for one
    $value = ((int)$row['requires_input'] === 1 && $value !== null) ? trim($value) : null;
    if ($completed && (int)$row['requires_input'] === 1 && ($value === null || $value === '')) {
        throw new ServiceError('validation', 'missing_field', 'This step needs a value before it can be ticked.');
    }
    … UTC_TIMESTAMP() …

A step that asks for a value cannot be ticked without one - on the server, because a tick with an empty answer is a record claiming something was checked. (The ticket side does not enforce this; see Known gaps.) A value sent with a plain step is dropped, so it can't show up as an answer to a question nobody asked.

---

4. Completing a task: the gate

Where a task gets completed

Path	Code	Notes
Status box in the panel, REST PATCH	TasksService::updateTask()
REST /move	TasksService::moveTask()
Drag a card into a closed column	api/tasks/reorder.php	Off the service (client-computed positions)
Tick a subtask	api/tasks/toggle_subtask.php	Off the service

Two of the four sit outside TasksService - the same situation round one found on tickets, where four close paths existed and the gate lived in one. Every one now calls the gate before the status is written:

// TasksService::updateTask()
if ($status[2]) {
    if (!$wasClosed) {
        ChecklistsService::assertTaskCompletionAllowed($conn, $taskId);
    }
    $updates[] = 'completed_datetime = COALESCE(completed_datetime, UTC_TIMESTAMP())';
    $firesCompleted = !$wasClosed;

// api/tasks/reorder.php
$completing = !empty($sts['is_closed']) && empty($wasClosedStmt->fetchColumn());
if ($completing) {
    try {
        ChecklistsService::assertTaskCompletionAllowed($conn, $taskId);
    } catch (ServiceError $se) {
        echo json_encode(['success' => false, 'error' => $se->getMessage(), 'code' => $se->errorCode]);
        exit;
    }
}
$conn->beginTransaction();

"Completing" means open → closed. Moving between two closed statuses (Done → Cancelled) or reopening is never gated.

The gate itself

public static function taskCompletionCheck(PDO $conn, int $taskId): array
{
    …
    foreach (self::taskOutstandingMandatorySteps($conn, $taskId) as $r) {
        $mode = self::effectiveClosureMode($conn, (string)($r['closure_mode'] ?? 'warn'), $tenantId);
        $out[$mode === 'block' ? 'blocking' : 'warning'][$name][] = $r['step'];
    }
    return $out;
}

public static function assertTaskCompletionAllowed(PDO $conn, int $taskId): void
{
    $check = self::taskCompletionCheck($conn, $taskId);
    if (!$check['blocking']) return;
    throw new ServiceError('validation', 'mandatory_steps_outstanding', '… cannot be completed until …');
}

• One function decides, two use it. taskCompletionCheck() is both the gate's input and what the panel asks before posting (GET ?check=complete), so the warning a person sees is computed by the code that will enforce it.
• effectiveClosureMode() is the round-two function, reused. So the company's block all floor (Tickets → Settings → Checklists) reaches tasks too. A zero-tolerance company that blocked tickets but let tasks through would have a gap exactly where its procedures are.
• The task's company, not the analyst's: taskTenantId() reads tasks.tenant_id (NULL = the Default company, read as the install default).

Standard: recorded after the write

// TasksService::updateTask(), after the UPDATE
if ($firesCompleted) {
    ChecklistsService::recordTaskCompletionOverride($conn, $ctx, $taskId);
    …

public static function recordTaskCompletionOverride(PDO $conn, ActorContext $ctx, int $taskId): void
{
    … INSERT INTO task_audit (task_id, analyst_id, field_name, old_value, new_value, source, created_datetime)
      VALUES (?, ?, 'Completed with checklist steps outstanding', NULL, ?, ?, UTC_TIMESTAMP())
      → [$taskId, $ctx->actorId > 0 ? $ctx->actorId : null, $note, $ctx->source === 'ui' ? 'app' : $ctx->source]
}

task_audit, not a comment. Its analyst_id is nullable, so a workflow or an API key with no analyst behind it is recorded as nobody instead of being stamped with a colleague's name. That is the round-two lesson (writeClosureNote(), "a workflow must never write to ticket_notes"), applied before it could happen rather than after.

---

5. Recurrence and delete

A repeating task gets fresh, unticked copies of its checklists on every new occurrence, from TaskRecurrence::createOccurrence():

public static function copyTaskChecklists(PDO $conn, int $fromTaskId, int $toTaskId): void
{
    if (!self::tasksEnabled($conn)) return;
    … LEFT JOIN checklist_templates t ON t.id = c.template_id …
    if ($c['tpl_exists']) { self::copyTemplateToTask(…current template…); continue; }
    … otherwise copy the finished task's own steps …
}

• From the template where it still exists, so a monthly check picks up this month's version of the procedure.
• From the finished task's own steps where the template has since been deleted, rather than the checklist silently vanishing from the series.
• Always, not behind a copy_* option: a repeating task that has a checklist is usually a repeating procedure, and the checklist is the procedure.
• Wrapped so a failure here never stops the occurrence being created.

Deleting a task removes the checklists of the whole subtask tree, whether or not the feature is on now:

// TasksService::deleteTask()
ChecklistsService::deleteForTasks($conn, $ids);   // before the task rows go

---

6. The panel

assets/js/tasks.js - a Checklist section between the time section and Tags, hidden unless the endpoint says enabled. It shows each checklist (padlock if Critical, progress bar, Remove), each step (tick, Mandatory badge, suggested role, value box, who and when), and an attach picker of templates that apply to tasks.

One question before every completion route:

async function confirmChecklistCompletion(taskId, newStatusName, currentStatusName) {
    const isClosed = name => !!((statusList || []).find(s => s.name === name) || {}).is_closed;
    if (!isClosed(newStatusName) || (currentStatusName && isClosed(currentStatusName))) return true;
    return confirmChecklistCompletionById(taskId);   // GET ?check=complete → alert (Critical) or confirm (Standard)
}

Called from the status box (saveField), the drag (endDrag) and the subtask tick (toggleSubtask(id, completing)). A declined drag reloads the board so the card goes back; a declined status change re-opens the panel so the box shows the real status.

The server enforces the Critical case whatever the browser does - this exists so nobody meets a bare refusal, and so completing past a Standard checklist is a decision rather than an accident. If the drag is refused anyway, the toast now says which steps (code: mandatory_steps_outstanding) instead of the generic "could not reorder", which read as the drag having broken.

The existing Involved warning was not reused for this - confirmCloseWithInvolved() only ran from the status box, never the drag. Checklists warn on all three routes; that is a deliberate difference, worth knowing if the two are ever merged.

Every colour is a theme variable that exists in theme.css (checked against the file). The same week, the Telegram settings box shipped with two variables that did not exist and showed a light panel in dark mode; this section was measured in dark mode before it shipped.

---

7. The ticket-side hole found on the way

Reading api/tickets/ticket_checklists.php to build its task twin, it turned out to check neither company nor Tickets access on any action. Replaying it with an analyst restricted to company A:

As an analyst who can only see company A	Before	After
Read company B's ticket checklist	✅ returned it	"Ticket not found"
Tick one of its steps (addressed by step id)	✅ ticked, stored as done by them	"Ticket not found"
Remove it (by checklist id)	would have	"Ticket not found"
Attach one to company B's ticket	would have	"Ticket not found"

The fix gates every action on the ticket, resolving a child id to its ticket first:

function checklistTicketGate(PDO $conn, int $ticketId): void
{
    if ($ticketId <= 0 || !analystCanAccessTicket($conn, (int) $_SESSION["analyst_id"], $ticketId)) {
        throw new Exception("Ticket not found");
    }
}
…
case "toggle_item":
    …
    checklistTicketGate($conn, checklistTicketForItem($conn, $itemId));

plus requireModuleAccessJson('tickets'). The task endpoint was written the same way from the start (checklistTaskGate()).

Also fixed in ticket_view.js: round one replaced the module's root-absolute URLs with the page's API_BASE, but two survived - saving a note from the checklist (/api/tickets/save_note.php) and the suggest a checklist call. Both broke on an install in a subfolder (localhost/freeitsm-app/). Now API_BASE / CHK_API like the rest of the file.

---

Re-ordering checklists and steps

Added late in 2.10.0, at Ed's request: a task can carry several checklists, and both the checklists and the steps inside each one can be dragged into a new order.

Schema. Steps always had task_checklist_items.sort_order (copied from the template). The checklists themselves had no order beyond id, so task_checklists gained sort_order INT NOT NULL DEFAULT 0. Existing rows are all 0, and every read sorts ORDER BY sort_order ASC, id ASC, so an upgraded install shows the old order until somebody drags. A new attachment goes to the bottom:

private static function nextTaskChecklistOrder(PDO $conn, int $taskId): int
{
    $st = $conn->prepare("SELECT COALESCE(MAX(sort_order), 0) + 1 FROM task_checklists WHERE task_id = ?");
    $st->execute([$taskId]);
    return (int)$st->fetchColumn();
}

used by both copyTemplateToTask() and the non-template branch of copyTaskChecklists() (recurrence), which now also copies in sort_order order.

Service. Two methods, each taking the whole list top first:

public static function reorderTaskChecklists(PDO $conn, int $taskId, array $ids): void
{
    $st = $conn->prepare("UPDATE task_checklists SET sort_order = ? WHERE id = ? AND task_id = ?");
    foreach (array_values($ids) as $i => $id) {
        $st->execute([$i + 1, (int)$id, $taskId]);
    }
}
// reorderTaskChecklistItems(): the same, pinned to task_checklist_id

🔒 The AND task_id = ? is the security, not a nicety. The ids come from the browser. The endpoint gates on the task (or, for steps, on the task that owns the checklist), and pinning every UPDATE to that parent means an id belonging to another task - another company's included - matches no row and changes nothing.

Endpoint. Two more actions on api/tasks/checklists.php, returning the whole state like the others:

Action	Body	Gate
reorder	{task_id, ids}	checklistTaskGate($conn, $taskId)
reorder_items	{checklist_id, ids}	the task from taskIdForChecklist(), then the same gate

UI (assets/js/tasks.js). Each .chk-card and .chk-step carries data-sort-id, draggable="true" and a ⋮⋮ handle; steps sit in a .chk-steps container per checklist, checklists in one .chk-lists. chkDragStart / chkDragOver / chkDragEnd use the HTML5 drag events with three rules:

• Only the handle starts a drag - the handle's mousedown sets data-grab, and a dragstart without it is cancelled. Otherwise selecting a step's text, or a slightly wobbly click on a tick box, would start a drag.
• A step is inside a draggable card, so dragstart bubbles from the step to the card. Each handler acts only when e.target === e.currentTarget.
• A container only re-orders its own children (chkDragRow.parentElement !== box returns), so a step can never be dropped into another checklist, nor a checklist among steps.

chkDragEnd posts the new order and re-renders from the server's answer, so a refusal puts the list back as stored.

Verified on a throwaway database through the real endpoint: reorder with [3,1,2] returned the checklists in that order; reorder_items on checklist 1 with a step id from checklist 2 smuggled in re-ordered checklist 1 and left the other step untouched. In headless Chrome, dragging the first checklist below the last, and the third step above the first, both saved - the panel re-drew in the new order from the server's answer.

---

The traps

1. A new way to complete a task must call the gate

assertTaskCompletionAllowed() before the status is written, recordTaskCompletionOverride() after. Two of today's four paths live outside TasksService; a fifth (a bulk action, a workflow "set task status" action) will be tempted to as well. Grep for UPDATE tasks SET with status_id and check each one.

2. Gate on open → closed only

Reopening, or moving between two closed statuses, is not completing. All four paths compute $completing from the old status as well as the new.

3. A child id needs its own gate

Steps and checklists are addressed by their own ids. Resolve to the task (or ticket) and check that - the ticket endpoint's hole was exactly this.

4. Switched off must not enforce

Anything new that reads task checklists for a decision should go through taskCompletionCheck() / tasksEnabled(), not query the tables directly.

5. Don't write automation into a NOT NULL analyst column

task_audit.analyst_id is nullable for this. task_comments / ticket_notes are not.

6. Delete children yourself

No foreign keys on Verification-grown tables. Steps, then checklist.

7. tenantSetting() memoises per process

The company floor is read through it. A test that changes the setting and reads it back in one PHP process gets the first answer. One process per scenario - the trap recorded in round one and walked into in round two.

---

How it was tested

On a throwaway copy - a git worktree served by WAMP, a new database from freeitsm.sql and a real Database Verification run - never on Ed's data. Requests through the real endpoints with forged sessions; the API and workflow cases through TasksService in a separate PHP process each.

Area	Cases	Result
Switch	default off; GET empty; attach refused; truthy value stored as '1'	✅
Templates offered	tickets-only excluded; gate shown per template	✅
Attach	to a task and a subtask; a tickets-only id posted by hand refused	✅
Tick	value step with no / whitespace value refused; with value stored, who + when; unknown step "not found"; stray value on a plain step dropped	✅
Gate	status box, drag, subtask tick and REST move all refuse a Critical checklist, and the tasks stay open; Standard completes and writes task_audit (analyst 1, app)	✅
Positive control	tick the Critical step → drag completes; no audit row	✅
Workflow	completion with Standard outstanding → task_audit with NULL analyst, source workflow	✅
Company floor	block_all turns a Standard checklist into a block on a task	✅
Switched off	Critical outstanding no longer enforced; no audit row; GET reports off	✅
Recurrence	next occurrence gets fresh, unticked copies - one from the template, one from the finished task's steps after its template was deleted	✅
Delete	task with a subtask, both with checklists → no rows left, no orphan steps anywhere	✅
Company scope (task)	restricted analyst: read, attach, tick (real step id), remove, pre-check on another company's task → "not found"; own-company task → works	✅
Company scope (ticket)	the same on the old endpoint: read and tick succeeded (the hole); on the fixed one: all "not found", nothing changed	✅
Browser (headless Chrome)	settings tab and switch; panel attach; value rule; tick shows who/when; Standard asks (declined → still open); Critical refuses with the list; padlock; dark mode measured (dark card, light text, dark amber badge); ticket page loads the fixed script; no JS errors	✅
Existing suites	task-recurrence-spawn 21/21, task-recurrence-dates 60/60, tasks-priority 51/51, task-collaborators 43/43, security-findings (live) 193/193, web-exposure-guard 12/12, config-not-load-bearing 13/13, db-verify-indexes 32/32, schema drift up to date, i18n gate OK, Feature Bingo 594 cards / 0 malformed	pass

Not tested in the browser: the drag itself (the drop is hard to drive headlessly; the server side of the drag was tested directly, and the drag shares the pre-check function with the status box).

---

Known gaps

• The ticket side doesn't require a value for a step that asks for one; the task side does. Worth aligning.
• No REST endpoints for checklists on either side - the gate applies to the API, but attaching and ticking are UI-only.
• No workflow trigger for a task checklist step being ticked (the ticket side has one).
• The no checklist at all rule is tickets-only by design.
• Strings owed: the new English strings for tasks and the editor hint, to the other locales.

---

Related

• Checklists & SOPs · Stopping a ticket closing with a checklist outstanding
• Checklists module - house style (round one) · Closure gating - Developer Guide (round two)
• Tasks