Skip to content

Time Tracking Developer Guide

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

Time tracking β€” Developer Guide

Two switches, per company, over an install-wide default β€” and the settings layer built to carry them, which is the part worth reusing.

The user-facing page is Time tracking. Built for discussion #72.


1. πŸ“ The files involved

🟒 The settings layer (new, and generic)

File Role
includes/tenant_settings.php tenantSetting(), tenantSettingOn(), setTenantSetting(), tenantSettingsForKey() β€” per-company answers falling back to install-wide. Plus the two time-tracking helpers
tenant_settings table (tenant_id, setting_key, setting_value), unique on the pair, FK to tenants

πŸ”΅ The switches

File Role
tickets/settings/manifest.php Registers the Time tracking tab under Cap::TICKETS_MANAGE
tickets/settings/index.php The tab: two switches, a per-company table, and the failed-load guard
api/tickets/get_time_tracking_settings.php The default plus every company's override
api/tickets/save_time_tracking_settings.php Writes both; null for a company deletes its override
includes/settings_keys.php Registers the two install-wide keys against Tickets

🟠 Where it is enforced

File Role
api/tickets/get_time_entries.php Returns {disabled: true} and no rows
api/tickets/save_time_entry.php Refuses the write
api/v1/resources/tickets.php apiRequireTimeTracking() on list, create and delete
assets/js/inbox.js Renders nothing when the server says disabled

2. Why Tickets settings and not System

Every surface time tracking has is a ticket surface: the panel in the reading pane and the two endpoints behind it. There is no time report, no time menu, no dashboard widget.

So dschipfel's "remove time tracking menus, conceal related statistics and reports" was asking for less than it sounds β€” those do not exist β€” and nothing about the feature is install-wide in character. It belongs on the settings page of the module it lives in.


3. The settings layer

FreeITSM had no way to answer a per-company yes/no question before this:

  • system_settings is a flat key/value table for the whole install
  • getTenantConfigRows() handles per-company lookup lists (statuses, ticket types)

Neither can express "this company bills for time and that one does not".

tenant_settings   this company's answer, if it has been given one
     ↓ falls back to
system_settings   the install-wide default
     ↓ falls back to
the caller's default
tenantSettingOn($conn, $tenantId, 'time_tracking_enabled', true);
setTenantSetting($conn, $tenantId, 'time_tracking_enabled', '0');
setTenantSetting($conn, $tenantId, 'time_tracking_enabled', null);   // back to the default

⚠️ A COMPANY WITH NO ROW IS NOT "OFF". It follows the install default. That distinction is doing three jobs: it keeps the whole thing invisible at N=1, it makes the default meaningful, and it is why setTenantSetting(…, null) deletes the row rather than storing an empty string. Without a way back, a company given an answer once could never follow the default again.

This is deliberately generic rather than a time-tracking flag with a table around it. "Per company, defaulting to the install-wide value" is the shape most settings take once an install has more than one company in it β€” expect the next one to reuse this rather than invent a second mechanism.

Why this was worth building, when the ACL table on #76 was not

Both were "the obvious design needs a subsystem FreeITSM does not have". The difference is size and reuse: a generic per-object ACL would have been bigger than the feature it served and is needed by exactly one thing. This is one small table with a three-step fallback, and it is the natural home for every per-company setting that follows.


4. Why two switches

Hiding a panel is about interface clutter. Silently emptying an API endpoint breaks an integration belonging to somebody who changed nothing β€” a billing export, a reporting job, something built months ago by a person who has left.

Different decisions, different people affected, so they are different switches. An install can tidy its screens without breaking an export it has forgotten about.

timeTrackingUiOn($conn, ticketTenantId($conn, $ticketId));    // the panel
timeTrackingApiOn($conn, ticketTenantId($conn, $ticketId));   // the REST endpoints

Both resolve from the ticket's company, because that is the only sensible unit β€” see below.


5. Why per company is clean here

The usual difficulty with per-company settings is "what does an analyst working across two companies see?"

Here it evaporates: every surface hangs off a ticket, and a ticket always belongs to a company. The panel appears on one client's ticket and not the next, which is correct rather than confusing.

Had there been a cross-company time report β€” one screen, many companies, one answer needed β€” this would have been a genuinely awkward feature. It is worth noticing when a design is easy because of the shape of what already exists, rather than because you were clever.


6. Enforced in three places, not one

Hiding the panel is not turning a feature off. An endpoint that still answers is one URL away from putting the panel back, and a stale tab should not go on filing time into something that is supposed to be disabled.

Where Behaviour
get_time_entries.php {success: true, time_entries: [], disabled: true}
save_time_entry.php Refused, with a message naming the reason
REST list / create / delete 404, not an empty list

⚠️ The REST endpoints refuse rather than returning nothing, deliberately. An empty list says "this ticket has no time recorded" β€” a different statement, and an untrue one. A caller that has to tell "off" from "none" cannot, if you answer both the same way.

Nothing anywhere deletes a row. ticket_time_entries is untouched by either switch; both decide what is shown and what is served.


7. The trap this feature fell into twice in one day

Both switches are drawn unticked in the markup and filled in from the server afterwards. So a failed load renders exactly like time tracking being disabled β€” and pressing Save then writes that guess back as fact.

That is not hypothetical. The first version of this tab fetched API_BASE + 'tickets/…' when API_BASE already ended in api/tickets/. It 404'd, the catch swallowed it, and the tab showed both switches off while time recording worked perfectly. Ed spotted it within minutes of it shipping.

The same failure had been fixed on the Authentication page earlier the same day, with a warning written about exactly this β€” and then reproduced eight hours later by the person who wrote the warning.

The remedy, both times:

  • a banner saying the settings could not be read
  • the switches and Save disabled until real values arrive
  • Save refusing outright if the load never succeeded

πŸ”‘ If a settings screen renders a boolean before it knows the answer, the unloaded state must not be a valid-looking answer. Either show a loading state, or lock the controls β€” but never let "we do not know" render identically to "no".


8. Adding another per-company setting

  1. Register the install-wide key in includes/settings_keys.php (module, capability, tab).
  2. Read it with tenantSettingOn($conn, $tenantId, 'your_key', $default).
  3. Resolve the tenant from whatever the setting is about β€” for a ticket, ticketTenantId().
  4. Write it with setTenantSetting(), passing null to mean "follow the default".
  5. Enforce it server-side, wherever the feature actually happens β€” not only in the browser.

Step 5 is the one that gets skipped.


Related pages

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Blue sky thinking

Bugs resolved

Links

Clone this wiki locally