Tasks on Tickets Developer Guide - edmozley/freeitsm GitHub Wiki

Tasks on a ticket β€” Developer Guide

How follow-up tasks are created from and attached to a ticket: why there is no new button, why creating and linking are one control, and the two invariants that are not expressible as scope checks.

Requested in discussion #83 by dschipfel. Shipped as #1176 (company scoping) and #1177 (the feature), commits 5de974cf, 5161b5ff, e4de579c.

The user-facing pages are Tickets β†’ Follow-up tasks and Tasks β†’ Linking.

Related: Linking equipment to tickets Β· Multi-Tenancy Developer Guide Β· Tasks

1. Half of it already existed, backwards

Worth knowing before you touch anything: tasks.ticket_id had always been a column, and api/tasks/search_links.php plus assets/js/tasks.js already let you open a task and attach a ticket to it.

So the relationship, the storage and one direction of the UI were all built. What was missing was the ticket's side: no way to get from a ticket to its tasks, and no way to see them without leaving the ticket. That is what #83 actually asked for, and it made the job far smaller than the request reads.

The general lesson: before scoping any "link A to B" request, look for the reverse direction. A feature that sounds absent is sometimes present and pointing the other way.

2. πŸ“ The files involved

🟒 The endpoints

Named to match the equipment linking work, because they do the same job for a different thing.

File Role
api/tickets/get_ticket_tasks.php The strip's pills: title, status, assignee, subtask progress, plus open_count for the close warning
api/tickets/search_linkable_tasks.php The picker's results. Excludes tasks already on this ticket, and subtasks
api/tickets/save_ticket_task.php Both verbs β€” link an existing task, or create one via TasksService
api/tickets/delete_ticket_task.php Unlink. Never deletes

πŸ”΅ The client

File Role
assets/js/inbox.js loadTicketTasks / renderTicketTasks (pills), openLinkTaskPicker (the one picker), openContextLinkTask (right-click), and the close warning inside assignStatus
tickets/index.php The Link to task… context-menu item
assets/css/inbox.css .task-badge / .task-done

βšͺ Groundwork

File Role
includes/services/tasks.php Company scoping and the creation rules β€” see Multi-Tenancy Progress

3. No new button, on purpose

The ticket screen is busy. Rather than add a control, tasks became the seventh entry in the Link to… menu that already existed, and task pills joined the strip that already held problems, changes, tickets, equipment, CMDB objects and tracker issues.

That strip is where an analyst already looks to answer "what else is this connected to", and the codebase had already made this argument once β€” the comment introducing tracker pills reads:

a Jira issue IS a link, so it belongs in this strip rather than in a panel of its own

A task is the same kind of thing. The cost of the whole feature, in screen furniture, is one menu row.

4. One picker, two verbs

Creating a task and linking an existing one are the same gesture. Typing searches; if nothing matches, the first row creates a task with exactly what was typed.

This is not a space saving. Two separate actions β€” Create task and Link task β€” make the user decide which they are doing before they know whether the task already exists, which is precisely the decision they cannot make yet. One box shows them the near matches as they type, so the duplicate is prevented rather than tidied up afterwards.

Structurally the create row is just entry 0 of the same flat list:

current = [{ kind: 'create', title: typed }]
    .concat(rows.map(r => Object.assign({ kind: 'task' }, r)));

which keeps one render path and one click handler:

const pick = (r) => {
    if (r.kind === 'create') createTaskForTicket(ticketId, r.title);
    else                     linkTaskToTicket(ticketId, r.id);
};

The right-click entry deliberately has no modal

Every other link item on the context menu opens its own modal. openContextLinkTask() does not: it loads the ticket and opens the real picker on the strip.

A second copy of the picker in a modal would be two implementations of "find me a task", and issue #77 is what happens when the same question gets answered in two places. The ticket you right-clicked is the one you are about to work on, so opening it costs nothing.

5. ⚠️ Two rules that a scope check cannot express

Both endpoints gate the ticket (analystCanAccessTicket) and the task (analystCanAccessTask). Both gates passing is not sufficient.

A task and its ticket must belong to the same company. An analyst with access to two companies passes both checks and could still attach one client's task to another client's ticket, creating exactly the straddle the scoping work exists to prevent. So the companies are compared explicitly:

if (isMultiTenant($conn)) {
    $default  = getDefaultTenantId($conn);
    $taskCo   = $task['tenant_id']  === null ? $default : (int)$task['tenant_id'];
    $ticketCo = $ticketTenant       === null ? $default : (int)$ticketTenant;
    if ($taskCo !== $ticketCo) {
        throw new Exception('That task belongs to a different company from this ticket.');
    }
}

Note both sides resolve NULL to Default before comparing. Skip that and a Default-company ticket never matches a NULL-company task β€” which is every task predating the scoping work. CMDB draws the same line for the same reason, and the Multi-Tenancy Developer Guide explains why NULL means Default here rather than "shared".

A subtask cannot be linked to a ticket on its own. It belongs to its parent's work, and a pill whose parent is nowhere in sight is a dead end. search_linkable_tasks.php excludes them and save_ticket_task.php refuses them.

Scope both ends of a read, too

get_ticket_tasks.php filters the tasks it hydrates even though the ticket is already gated:

[$tSql, $tArgs] = activeTenantFilter($conn, $analystId, 'tk');

A link made before a company was split, or by an all-access analyst, can straddle. The gate on the parent is not evidence about the children β€” the same reasoning get_ticket_assets.php documents.

6. What is copied, and what deliberately is not
Title What was typed
Company Inherited from the ticket
Assignee Whoever created it
Link back tasks.ticket_id
Priority No
Dates No
Requester No

Copying priority or dates makes a second copy of the truth that drifts the moment either side changes, and a task showing a stale priority is worse than one showing none. The link is the connection; the ticket is one click away.

The assignee changed after review. It started as unassigned, on the argument that handing work to a person should be deliberate. That is right in the Tasks module and wrong here: raising a task while working a ticket almost always means "I am going to do this", and an unassigned task appears on nobody's list. Reassigning takes one click; noticing an unowned task can take days. The Tasks module keeps its own behaviour β€” this applies to the ticket path only.

7. The close warning

Closing a ticket with unfinished tasks warns and never blocks, matching collision detection.

It costs no request: the strip has already loaded the tasks, and ticketStatuses already carries is_closed, so assignStatus() can answer from what is in memory.

const closing   = ticketStatuses.some(s => s.name === status && s.is_closed);
const openTasks = (tasksForTicket || []).filter(tk => !tk.status_is_closed).length;

Two details that matter more than they look:

  • tasksForTicket is cleared at the start of every load, not just on success. Otherwise a failed fetch leaves the previous ticket's tasks in place and the warning reports a count belonging to a ticket the analyst is no longer looking at.
  • If the load failed, the array is empty and the warning simply does not appear. That is the right way round: a warning that cannot be shown must never become a block that cannot be cleared.

8. πŸ› The bug this shipped with, and why it was invisible

The first cut of the picker looked completely dead β€” type, and nothing happened.

.strip-picker-results is display: none until an active class is added:

.strip-picker-results        { display: none; }
.strip-picker-results.active { display: block; }

The equipment picker adds it. Mine wrote innerHTML and never did, so the results were fetched, rendered and painted into a hidden container. Everything worked except being visible.

A second bug sat behind it: the rows used .link-search-result, which is a Tasks-module class with no rule in inbox.css, so they would have been unstyled even once shown.

Two lessons. Copy the working example's behaviour, not just its shape β€” the class that reveals the container is as load-bearing as the markup. And an end-to-end test that stops at "the endpoint returned the right JSON" would have passed this bug twice: the API was correct throughout, and so was the rendering. Only driving the real page caught it.

The rewrite also removed a genuine escaping bug: the create row's click handler was built by concatenating the typed text into an onclick attribute, so a task title containing an apostrophe would have broken it. Handlers are now bound with data-idx.

9. Testing notes

Driving the real page beats asserting on JSON, and this feature is the proof. A same-origin harness that iframes tickets/index.php, seeds the session cookie from PHP, then:

  1. loadTicketById(...) so the strip and its picker host exist
  2. openLinkTaskPicker(...)
  3. sets input.value and dispatches a bubbling input event
  4. waits out the debounce and the fetch before asserting

Step 4 is not optional β€” an early check reported one row where there were two, which looks exactly like a bug and was not one.

Assert on getComputedStyle(results).display, not just on innerHTML. The whole defect above was content that existed and could not be seen.

Server-side, the differential that matters is the cross-company one: attempt the link as an all-access analyst. A test using a company-restricted analyst passes whether or not the explicit company comparison exists, because the scope gate already refuses it β€” so it proves nothing about the invariant that needed proving.

10. The task editor inside a ticket's workspace (PR #212)

Since PR #212 (by @Dr4inder), a task pill on the workspace's Work tab opens the task's own editor inside the ticket. This section is what that embedded page relies on - the design asked for it, because it means every change to the task detail panel now also runs inside a ticket. The workspace side is in the Ticket workspace developer guide.

How it is built

File Role
tasks/embedded.php Requires both Tickets and Tasks, checks the context with taskEmbedCanOpen(), sets $taskEmbedContext, then requires tasks/index.php. A wrong context is a 404, never a 403
tasks/index.php With $taskEmbedContext set: no board, header, context menus or assets/js/mobile.js; adds assets/css/task-embed.css and assets/js/task-embed-bridge.js; TASK_HOST = 'ticket', TASK_EMBED_CONTEXT = {taskId, ticketId}, and the detail view forced to the large window
tasks/includes/detail-shell.php The detail panel's markup, shared by both - one copy, not two that drift
includes/task_embed.php taskEmbedCanOpen($conn, $analystId, $taskId, $rootId, $ticketId)
api/tasks/get.php Given embed_root_id and ticket_id, re-runs taskEmbedCanOpen() on every read, so an unlink or a lost permission shows on the next click
assets/js/tasks.js On TASK_HOST === 'ticket': opens the one task instead of building the board, asks the bridge before switching task, sends the embed parameters on reads, and loadTasks() reports a change instead of redrawing a board
assets/js/task-embed-bridge.js The frame's side of the conversation with the ticket

The checks

taskEmbedCanOpen() needs all of:

  1. the analyst can access the ticket, the linked task (the root) and the task asked for, each by its own module's rules;
  2. the root really is linked to this ticket (tasks.ticket_id), and the ticket is not in the trash;
  3. walking up parent_task_id from the task asked for reaches the root, through tasks the analyst can each see - a subtask is reachable only through parents they can open, as on the board. A seen-list and a 100-step limit stop a corrupt parent chain from looping.

Writes need nothing new. The frame saves through the ordinary Tasks endpoints, with their own module, Read only and company checks and the CSRF token. A Read only Tasks analyst sees a read-only editor in the ticket exactly as on the board.

Rules for anyone changing the task detail panel

  • Keep new controls on the existing writers. A field that saves through saveField() / postTaskChange() needs only a data-task-field="<payload key>" attribute, so the bridge can move its saved baseline when the save succeeds. Without it, a value that saved still looks unsaved, and leaving asks to discard it.
  • A compound control with its own endpoint (moving company, the work-view preference) calls TaskEmbedBridge?.saved({...}) after its own success - see moveToCompanyFromPanel().
  • A new nested editor (another iframe, another TinyMCE) must take part in the unsaved check - the bridge watches inputs in .tf-frame documents and the one tinyEditor, and nothing else.
  • No window.top, no parent assumptions, no full-page navigation from the panel. Open other records in a new tab, as Open separately does.
  • Size from the panel. The bridge measures #detailPanel; the page body is as tall as the frame, so it can report growing but never shrinking (the same lesson as PR #198).

The conversation (postMessage only, same origin only)

Direction Message Meaning
frame β†’ ticket ready (hasActions) Drawn; whether the header has its own Open separately / Close
frame β†’ ticket resize (height) Size the frame to this
frame β†’ ticket dirty (dirty) Something is, or is no longer, unsaved
frame β†’ ticket changed A save succeeded - reload the task pills
frame β†’ ticket error The task can no longer be edited from this ticket
frame β†’ ticket closed The editor's own Close
ticket β†’ frame freeitsm:task-embed:leave (requestId) May I close you?
frame β†’ ticket freeitsm:task-embed:leave-result (requestId, canLeave) The answer, after flushing the description, waiting up to 10 s for writes, and asking to Discard
ticket β†’ frame freeitsm:task-embed:context-lost The task was unlinked

Every message carries {ticketId, taskId}, is posted to location.origin only, and is checked on arrival for origin, sending window and context. A leave answer for a frame that has since been replaced is ignored (frameGeneration). No answer within 45 seconds counts as no - keeping the editor is the safe way to be wrong.

πŸ”΄ TRAP: the bridge wraps window.fetch - but only inside the frame's own document. It counts writes in flight and marks a failed one (a non-JSON or success: false response) as unsaved, without changing any Tasks writer. It sits on top of assets/js/csrf.js. Do not "tidy" it into assets/js/tasks.js: there it would run on the board too.

Testing

tests/task-embedded-workspace.php (forged roots, unrelated tasks, another company, missing module access, read-only writes, unlinking), tests/task-embed-bridge.php (sizing, messages, drafts, nested forms, editor baselines - Chrome, no database) and tests/ticket-task-live.php (the real editor in the real ticket page: scheduled work, checklists, forms, subtasks). The last two drive Chrome from PHP; see Developer tests - Tickets.

Related pages