Skip to content

Service Layer Progress

Ed Mozley edited this page Jul 5, 2026 · 18 revisions

Service Layer β€” progress tracker

Rollout status of the service-layer refactor: collapsing each module's duplicated UI + REST API write logic into one shared service, module by module, behind a before/after parity harness.

Strategy: prove the pattern on the smallest, lowest-risk module first; do tickets last (biggest, most integrations). Each module is its own revertable commit; nothing ships unless the harness proves the API is byte-identical.

βœ… COMPLETE (2026-07-05). All 14 modules are on the service layer β€” every write that a feature exposes both in the UI and the REST API now runs through one business-logic layer behind two thin adapters. Roughly 8,600 lines of duplicated adapter logic collapsed into shared services. Every module shipped behind a before/after parity harness proving the API byte-identical.

Per-module status

Module Status Adapter lines removed Parity Notes
Service Status βœ… Done (#718) 474 removed / 75 kept (9 files) βœ… byte-identical Pilot. Established the primitives + pattern. Display-UTC fix #719.
Morning Checks βœ… Done (#720) 424 removed / 94 kept (10 files) βœ… byte-identical Unified documented UI/API drift. Added ActorContext.actorName + shared apiFailFromService().
Software βœ… Done (#722) 175 removed / 40 kept (3 files) βœ… byte-identical Only licences overlap UI+API; inventory is read-only and widgets/apikeys are UI-only (no twin), so left untouched.
Calendar βœ… Done (#723) 302 removed / 70 kept (5 files) βœ… byte-identical Events overlap UI+API; UI-only categories folded in. NaΓ―ve-local datetime contract preserved (not converted to UTC).
Forms βœ… Done (#724) 722 removed / 55 kept (6 files) βœ… byte-identical Largest so far. Positional field sync + version chains + the form.submitted workflow dispatch all in one write path.
Contracts βœ… Done (#725) 462 removed / 56 kept (6 files) βœ… byte-identical Contracts + suppliers + term upsert. Lookup settings (UI-only) and supplier contacts (UI-flat vs API-nested mismatch) left on their own code.
Knowledge βœ… Done (#726) 453 removed / 53 kept (4 files) βœ… byte-identical Articles + tags + save-as-version + archive/restore/purge. Embedding regen (side effect) preserved with its decryptValue() guard.
Tasks βœ… Done (#728) 619 removed / 51 kept (4 files) βœ… byte-identical Task CRUD + move + comments + subtasks + task.completed dispatch. First service to use ActorContext.companyScope (ticket-link scope). reorder/toggle/lookups left UI-only.
CMDB βœ… Done (#729) 638 removed / 50 kept (5 files) βœ… byte-identical Object CRUD (typed property system, 6 types) + descendant-tree delete + relationships. Service normalises both property shapes (API key-map + UI id-list). Classes/rel-types read-only, ticket-links API-only.
Network Mapper βœ… Done (#730) 610 removed / 79 kept (5 files) βœ… byte-identical Diagram-level writes (create / full node+connector replace / delete / version). The API's incremental node/connector endpoints are API-only and stay put; service holds its own copy of the shared validators.
Assets βœ… Done (#731) 499 removed / 67 kept (4 files) βœ… byte-identical Field update + assign + unassign (and API-only create) share one service. Two UI-only behaviours (previous_user_id re-assign chain, skip_audit) preserved as optional input so the API stays byte-identical. Dashboard/vCenter/reference-settings are UI-only and stay put.
Change Management βœ… Done (#732) 1252 removed / 124 kept (8 files) βœ… byte-identical Seven writes (create/update, inline field edit, delete, comments, CAB roster + vote) in one service. Audit trail, risk scoring, and the change.approved event fire from one path. Attachments / lookup settings / field-layout / share-email are UI-only and stay put. Biggest de-dup so far.
Problem Management βœ… Done (#733) 467 removed / 98 kept (8 files) βœ… byte-identical First company-scoped module. Seven writes (create/update, delete, notes, link/unlink incident & change) in one service; every by-id op gated by ctx->companyScope. Proven with a scoped-key isolation battery. AI helpers / lookup settings / linkable pickers are UI-only and stay put.
Service Status settings βœ… folded into pilot β€” n/a Incident-statuses + impact-levels are UI-only; moved into the service.
Tickets βœ… Done (#734) 1272 removed / 186 kept (10 files) βœ… byte-identical The last + biggest. Seven writes (create, unified update, delete/restore, notes, time entries) in one company-scoped service. UI update endpoints (assign/owner/schedule) flow through one path; audit is a toggle (UI client-side, API server-side). Vast UI-only surface (mailboxes/SLA/rota/teams/CSAT/dashboards/email) untouched.

Line savings β€” the honest picture

The first module is roughly line-neutral: it pays a one-time cost (the shared ActorContext + ServiceError primitives, ~105 lines) and its service includes UI-only logic that was never duplicated. The thousands of lines of net removal arrive as the larger, heavily-mirrored modules migrate β€” tickets above all.

Lines
One-time primitives (includes/service_context.php + apiFailFromService) +122 (paid once, for all modules)
Service Status: adapter logic removed βˆ’474
Service Status: adapters kept (thin) +75
Service Status: service (incl. UI-only ops) +390
Morning Checks: adapter logic removed βˆ’424
Morning Checks: adapters kept (thin) +94
Morning Checks: service (incl. UI-only ops) +310
Software: adapter logic removed βˆ’175
Software: adapters kept (thin) +40
Software: service (licences only) +168
Calendar: adapter logic removed βˆ’302
Calendar: adapters kept (thin) +70
Calendar: service (events + UI-only categories) +251
Forms: adapter logic removed βˆ’722
Forms: adapters kept (thin) +55
Forms: service (forms + versions + submissions + dispatch) +445
Contracts: adapter logic removed βˆ’462
Contracts: adapters kept (thin) +56
Contracts: service (contracts + suppliers + terms) +341
Knowledge: adapter logic removed βˆ’453
Knowledge: adapters kept (thin) +53
Knowledge: service (articles + recycle bin) +299
Tasks: adapter logic removed βˆ’619
Tasks: adapters kept (thin) +51
Tasks: service (tasks + move + comments) +506
CMDB: adapter logic removed βˆ’638
CMDB: adapters kept (thin) +50
CMDB: service (objects + properties + relationships) +472
Network Mapper: adapter logic removed βˆ’610
Network Mapper: adapters kept (thin) +79
Network Mapper: service (diagrams + full-replace + versions) +485
Assets: adapter logic removed βˆ’499
Assets: adapters kept (thin) +67
Assets: service (create + field update + assign/unassign) +360
Change Management: adapter logic removed βˆ’1252
Change Management: adapters kept (thin) +124
Change Management: service (change + comments + CAB) +709
Problem Management: adapter logic removed βˆ’467
Problem Management: adapters kept (thin) +98
Problem Management: service (problem + notes + links, tenant-gated) +465
Tickets: adapter logic removed βˆ’1272
Tickets: adapters kept (thin) +186
Tickets: service (create + unified update + delete/restore + notes + time, tenant-gated) +691

Convergence log

Where a module's UI and API already behaved differently, we converge to the API's behaviour (the API is the external contract; it stays identical, the UI shifts to match). Record each convergence here so nothing is a silent surprise.

Service Status (#718):

  • Create timestamps: server-local β†’ UTC (SLA-correct; same bug class as Forms #701).
  • Empty description: "" β†’ null.
  • Affected-service validation: silently-skip unknowns β†’ strict 422.
  • Incident status: name-only β†’ name or id.
  • UI deletes: idempotent β†’ 404 on missing (matches the API).

Morning Checks (#720):

  • Result date: malformed silently-swapped-for-today β†’ strict 422.
  • Unknown check id on a result: raw FK 500 β†’ 422 with a clear message.
  • Result CreatedBy: left NULL by the UI β†’ the recording analyst's name (attribution; the API already did this).
  • Check validation messages align with the API wording; updating/deleting a missing check β†’ 404 (was a silent no-op).

Software (#722):

  • Licence app id: no check (raw FK error on a bad id) β†’ the app must exist, 422 with a clear message.
  • Quantity / cost / dates: silently coerced β†’ validated (422) β€” a negative quantity, a non-numeric cost or a malformed date is rejected.
  • Empty optional fields (licence_key, portal_url, vendor_contact, notes): stored as "" β†’ null; missing currency/status fall back to GBP / Active.
  • Saving or deleting a missing licence: silent success β†’ "Licence not found." (matches the API's 404).

Calendar (#723):

  • Event category id: no check (raw FK error on a bad id) β†’ the category must exist, 422.
  • Event times: no check β†’ end_at before start_at is rejected (422); naive-datetime format is validated (Z/offset rejected) β€” the naive-local contract is kept, not converted to UTC.
  • Generated events (source = 'asset_warranty' etc.): the UI let you edit/delete one, only for it to reappear on the next sync β†’ now refused (409) on both edit and delete, matching the API.
  • Saving or deleting a missing event: silent success β†’ "Event not found." (matches the API's 404).
  • Categories are a UI-only operation (no API twin) β€” moved into the service with behaviour preserved verbatim (name required, #RRGGBB colour check, in-use delete guard).

Forms (#724):

  • Field validation: a blank label was silently dropped, an unrecognised field type blindly stored β†’ both now 422.
  • Submission with an unknown field id: raw FK error β†’ 422 with a clear message.
  • Editing a frozen (non-leaf) version β†’ 409; deleting a form that still has newer versions β†’ 409 (was a raw FK error). Whole-chain delete stays an API-only option (?chain=true).
  • Submitting to an inactive form β†’ 409 (the UI merged not-found + inactive into one message; now split into 404 / 409).
  • Saving/deleting a missing form or submission β†’ 404 (was a silent success).
  • The form.submitted workflow dispatch now fires from the shared write path, so UI and API submissions trigger workflows identically β€” the first webhook-relevant side effect to land on a service (see the eventing note above).

Network Mapper (#730):

  • The diagram save gains the API's strict validation (nodes bind to a real CMDB object with valid size/icon; connector endpoints resolve, no self-link; line style validated) β€” the raw UI silently dropped anything it couldn't place.
  • Leaf-only rules now apply in the UI too: saving a frozen (non-current) version, or deleting a version that still has newer ones after it, is refused (409).
  • Scope: only the diagram-level writes were duplicated. The API's incremental node/connector endpoints (add/move/delete one item β€” the agent editing surface) have no UI twin, so they keep their own handlers; the service holds its own throwing copies of the validators they share, rather than migrating those API-only ops for no drift benefit.
  • The UI adapter maps the UI's payload shape onto the service's canonical form (node id/tempId β†’ ref, connector node-id β†’ from_ref/to_ref, flat branding keys β†’ nested) β€” an instance of the "normalise both shapes" resolution.

CMDB (#729):

  • Dropdown property values are validated against the option list; date properties are parsed (both were previously stored as-is). Numbers, object_ref (existence / target-class / no-self-ref), required-property and parent-cycle rules were already shared behaviour.
  • The service accepts both property input shapes and normalises them: the API's properties {key: value} map, and the UI's property_values [{property_id, value}] id-list (unknown ids dropped, as the UI did). Neither transport had to change its payload.
  • Scope: class + property-definition design and relationship-type management stay UI-only admin (the API exposes classes + rel-types read-only); the object↔ticket link endpoints are API-only (no UI twin). Both keep their own code.

Tasks (#728):

  • Status / priority / tags / assignee / team are validated (unknown values were silently accepted or defaulted); dates validated; a task can't be its own parent.
  • taskβ†’ticket links are now company-scoped in the UI too β€” the first use of ActorContext.companyScope in the service layer. A generic ticketAccessible($conn, $ctx, $id) mirrors apiKeyCanAccessTicket using the actor's scope (null = all, else the accessible tenant ids), so a UI session and an API key both enforce the same isolation. The UI previously stored any ticket id unchecked (a known MT gap) β€” now closed.
  • The task.completed workflow fires from the shared write path exactly as before: on a PATCH that moves an open task into a closed status, not on a kanban move (the product's asymmetry, preserved).
  • Scope: the UI drag (reorder.php) sends client-computed positions while the API's /move re-packs the column server-side β€” a structural mismatch β€” so it stays UI-only, along with the subtask-toggle convenience and the lookup settings.

Knowledge (#726):

  • A title is now required and capped at 255 characters; the owner must be a real active analyst (was a raw FK error); the review date is validated.
  • Editing an article that's in the recycle bin is refused (409 β€” restore it first); restoring a live article, or permanently-deleting one that isn't archived, returns a clear 409 instead of silently doing nothing.
  • The OpenAI embedding regeneration (a best-effort side effect after each save) moved into the service. Follow-up #727: the service now requires encryption.php, so the embedding regenerates on both the UI and the API β€” previously the API path lacked decryptValue() and silently skipped it, leaving API-written articles invisible to AI search. It stays best-effort (runs after commit, swallows failures), so a missing/blank/invalid key never blocks the save.

Contracts (#725):

  • A contract now needs a contract_number + title; its supplier / status / schedule / owner id must exist (were raw FK errors); dates are validated; the value must be numeric and the currency a 3-letter code.
  • A duplicate contract_number is refused (409) on create and update β€” the raw UI allowed duplicates.
  • Deleting a supplier cleanly unlinks every referrer (assets, contacts, contracts, RFP invites/scores) before the delete, and created timestamps are stamped UTC (the UI relied on the server-local column default).
  • Scope: only the cleanly-duplicated overlaps (contracts, suppliers, term values) were migrated. The lookup settings (contract/supplier statuses, supplier types, payment schedules, term tabs) are UI-only (the API exposes them read-only) and stay on their own endpoints; supplier contacts are modelled flat by the UI (a supplier_id in the body, movable) but nested under a supplier by the API β€” a structural mismatch not worth forcing into one shape β€” so they stay separate too.

Assets (#731):

  • Editing a field on an asset that no longer exists is refused (404) β€” the UI used to run an UPDATE that matched 0 rows yet still wrote a history entry for the ghost id.
  • Lookup ids (type / status / location / supplier) and dates are validated (were stored blindly); a no-op edit records no history row (the UI logged one every time you re-picked the same value).
  • Assigning a user requires the requester to exist (422 β€” was a raw insert relying on the FK).
  • Two UI-only behaviours are preserved as optional input, defaulting to the API's behaviour so the API bytes don't move: on a re-assign the UI passes previous_user_id so the audit records the "was X, now Y" transition (the API always logs null), and it passes skip_audit so the intermediate removal is logged silently before the assign logs the transition.
  • The UI keeps its narrower editable-field whitelist β€” the service's field map is broader (it also covers agent-synced hardware/OS columns for the API), but update_asset_field.php still rejects anything outside its classification/lifecycle set rather than silently widening the UI surface.
  • Scope: assets have no UI create path (they're agent-maintained), and the dashboard-widget / vCenter / asset type-status-location settings are UI-only β€” all stay on their own code. apiParseDateOnly stayed in the resource because contracts + tasks share it; the service carries its own copy with the identical message.

Change Management (#732):

  • The UI's audit trail becomes more correct to match the API: people changes (Requester / Assigned To / Approver) are recorded by name, not raw id; a category_id change is now audited (the UI only audited the legacy free-text category); and the creation entry reads "Created as " instead of a hard-coded "Draft".
  • Validation the UI lacked: an unknown lookup value β†’ 422 (was a silent fall-back to the default row); an analyst must exist and be active; dates are validated; editing/deleting a missing change or comment β†’ 404 (was a silent success).
  • Schedule/PIR datetimes are not shifted β€” a naive datetime-local value (no offset) parses to the same wall-clock string, so stored times are unchanged.
  • The change.approved workflow event fires from the one write path on both triggers: a manual status move into Approved, and a CAB vote that tips the required-member threshold.
  • Scope: only the seven cleanly-duplicated writes were unified. Attachments, the lookup settings (statuses / types / priorities / impacts / categories), the field-layout customiser, and share-by-email are UI-only (no API twin) and stay on their own code. The single-field inline edit (update_field.php) and the unscoped comment delete map onto the same service methods (one-key update; optional change-id on delete).
  • One rare cosmetic quirk: a CAB vote by a non-member now shows the API's wording ("The analyst this key acts as is not a CAB member…") in the UI too β€” the UI only surfaces the vote control to members, so it's an edge path.

Problem Management (#733) β€” the first company-scoped module:

  • Tenant isolation now lives in shared code. Every by-id read/write loads the problem and checks its owning company against the caller's ctx->companyScope (null = all companies, else the accessible tenant ids). A problem in another company is a clean 404 β€” indistinguishable from "doesn't exist" β€” for an API key and a UI analyst alike. The generic check mirrors apiKeyCanAccessTenantRow / analystCanAccessProblem exactly (a NULL tenant_id normalises to the Default company), so the API stays byte-identical while the UI's session-scoped check routes through the same code.
  • The incident-link same-company rule (the incident and the problem must belong to the same company) moved into the service too.
  • The acting company for a create is transport-specific and stays with the adapter (the API's explicit company_id / key default; the UI's active tenant) β€” it's resolved there and passed to the service as a parameter, exactly like the acting analyst id. Auth (who + which company) is the adapter's job; the rules are the service's.
  • UI convergences: unknown status / priority / analyst β†’ 422 (were stored blindly); an empty description β†’ null; a duplicate link β†’ 409 (the UI's INSERT IGNORE silently succeeded); unlinking or editing something already gone β†’ 404 (was a silent success).
  • Scope: the AI helpers (root-cause, suggestions), the lookup settings (statuses / priorities), and the linkable-item pickers are UI-only (no API twin) and stay on their own code.
  • This establishes the pattern for the remaining tenant-scoped module (tickets): gate via ctx->companyScope in the service, resolve the acting company in the adapter.

Tickets (#734) β€” the last + biggest, company-scoped:

  • The UI's fragmented update β€” assign_ticket, update_ticket_owner, schedule_ticket were three separate endpoints β€” now flows through one updateTicket, so an owner change fires the same workflow events (ticket.assigned) that an assignment does. Previously the event depended on which endpoint the UI happened to call; that drift is gone.
  • Audit is a per-call toggle. The API writes the ticket_audit trail server-side; the UI writes it client-side (JS β†’ log_ticket_audit.php). So the API adapters pass writeAudit=true (byte-identical trail) and the UI update adapters pass false (the JS keeps auditing) β€” while delete/restore, which the UI does audit server-side, pass true. The shared rules (closed-datetime, owner sync, template emails, CSAT, workflow dispatches) run either way.
  • Two other legitimate behaviour differences are preserved as params, defaulting per transport: a manually-created ticket auto-assigns to its creator ($defaultAnalystId; the API leaves it unassigned), and a UI analyst may delete only their own time entry ($restrictToOwner; an API key may delete any entry on a ticket it can see).
  • UI convergences: unknown status / priority / lookup / analyst β†’ 422 (were raw DB failures); a note now bumps the ticket's updated_datetime; date fields validated.
  • Scope: the enormous UI-only surface β€” mailboxes, SLA config, rota, teams, departments, CSAT settings, dashboards, triage, email sending, the trash's permanent-delete β€” has no API twin and is untouched. The company-move endpoint (its own bespoke dual-company screen + response) and the client-side audit logger keep their own code and are flagged rather than forced through the service.

The rollout, in one line

All 14 modules β€” service-status, morning-checks, software, calendar, forms, contracts, knowledge, tasks, CMDB, network-mapper, assets, change management, problems, tickets β€” now run one business-logic layer behind two thin adapters. The single write path is what makes the webhooks "wider event catalogue" phase a one-liner per module, and it's why UI and API can never drift again.

Deferred / follow-up work

  • Display-layer UTC parsing β€” storage is now consistently UTC, but module JS parses "Y-m-d H:i:s" as local (new Date(...)), so UTC data can render an hour off. Fix is a small per-module parseUtc() helper (mark the naive string UTC before toLocale*). Done for service-status (#719); still to do on calendar, morning-checks and other timestamp-rendering modules. See Watch out for #2.
  • Webhook events from the write path β€” once a module is on its service, emitting lifecycle events (for outbound webhooks) is one line from the single write path, firing identically for UI, API and automation. This is the webhooks "wider event catalogue" phase.

How each module is done

  1. Read the complete surface β€” every UI endpoint and the API resource for the module.
  2. Enumerate the UI-vs-API divergences; agree the convergence.
  3. Build/adapt the parity harness; capture golden-before on untouched code.
  4. Extract <Module>Service; repoint the API write handlers + the UI endpoints at it.
  5. Run the harness after; require the API byte-identical, UI converged as designed.
  6. Ship as one commit. Update this page.

See Service Layer β€” architecture for the conventions and the full "Watch out for…" list.

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Blue sky thinking

Bugs resolved

Links

Clone this wiki locally