Catalogue Request Approvals Developer Guide - edmozley/freeitsm GitHub Wiki

Catalogue Request Approvals — Developer Guide

How catalogue-request approvals work, and why the shape is the way it is. Shipped as #928 (the gate + approver inbox + auto-raise) and #929 (the requester's dashboard view).

The user-facing page is Catalogue Request Approvals.

1. 📁 The files involved

Colour key: 🗄️ schema · ⚙️ engine · 🔌 API · 🖥️ UI · 🎨 CSS · 🌍 i18n · 📄 docs

🎨	File	What it does
🗄️	`database/freeitsm.sql`	`forms.requires_approval` / `approver_id`; `form_submissions.approval_status` / `approver_id` / `approval_decided_by_id` / `approval_decided_datetime` / `approval_comment`; the four FKs to `analysts`
🗄️	`includes/db_verify_schema.php`	the same columns (columns only — FKs live in `freeitsm.sql`)
⚙️	`includes/catalogue_approvals.php`	the engine. `catalogueApprovalDecide()`, `catalogueCreateTicketFromSubmission()`, `catalogueApprovalsList()`, `catalogueApprovalGate()`, `catalogueSubmissionBodyHtml()`
⚙️	`includes/services/forms.php`	`FormsService::submitForm()` gates the submission; `saveForm()` stores the per-item settings
🔌	`api/forms/catalogue_approvals.php`	the inbox list (`?filter=mine\|all\|decided`)
🔌	`api/forms/decide_catalogue_approval.php`	approve / reject one request
🔌	`api/forms/save_form.php`	unchanged — passes `requires_approval` / `approver_id` straight through to the service
🔌	`api/forms/get_forms.php`	returns the approval columns + `approver_name` for the list
🔌	`api/self-service/get_dashboard.php`	returns the requester's `requests[]` with their state (#929)
🖥️	`forms/index.php`	the 🛡️ shield button, the Approval settings modal, the amber pill
🖥️	`forms/approvals.php`	the approver inbox (cloned from `change-management/approvals.php`)
🖥️	`forms/includes/header.php`	the Approvals nav item
🖥️	`self-service/index.php`	the Your requests dashboard section (#929)
🌍	`lang/en/forms.php` + `lang/pt-BR/forms.php`	the `approval` block, same commit
🌍	`lang/en/self-service.php` + `lang/pt-BR/self-service.php`	the dashboard `req_*` keys (#929)
📄	`CHANGELOG.local.md`, this wiki	#928 / #929

There is no new table: a catalogue item is a forms row and a request is a form_submissions row, so the approval state hangs off columns on those, mirroring how is_portal_visible was added.

2. 🔑 Why a designated analyst, and why it clones (not shares) the Change model

The classic version routes a request to the requester's line manager. Portal users (users) have no manager relationship — only tenant_id (their company) — so v1 reuses Change Management's single-approver idea instead: one analyst signs a catalogue item off.

The Change approval code is a good template but nothing is shareable: it's welded to the changes table and to analysts, with status-name string literals. So this is a parallel, smaller implementation of the approver + inbox pattern from Change Management.

3. ⚙️ Gating a submission

FormsService::submitForm() decides at submit time whether to gate:

$gateApproverId = ($portalUserId !== null && !empty($form['requires_approval']) && !empty($form['approver_id']))
    ? (int) $form['approver_id'] : null;
$approvalStatus = $gateApproverId !== null ? 'pending' : 'not_required';

Three deliberate rules live in that one line:

Portal only ($portalUserId !== null). The feature auto-raises a ticket for the requester; an analyst filling a form internally has none, so it's never gated.
Unconfigured ≠ gated. requires_approval on with no approver_id falls through to not_required — a form must never strand a request with nobody able to clear it.
The approver is snapshotted onto form_submissions.approver_id here, not read from the form at decision time. Editing the catalogue item later re-routes future requests, never ones already waiting.

🔑 A gated submission fires catalogue_request.submitted, NOT form.submitted. An admin's create-ticket workflow rule on form.submitted would otherwise raise the ticket immediately and jump the gate. The outcome events are catalogue_request.approved / .rejected. All three are best-effort (guarded by class_exists('WorkflowEngine') and try/catch) — notification is a bonus, never the mechanism.

4. ⚙️ Deciding, and the auto-raise

catalogueApprovalDecide($conn, $actorId, $submissionId, $decision, $comment):

Loads the submission; refuses if it isn't pending.
Refuses unless the actor is approver_id — or sessionIsAdmin().
On approve, inside one transaction: raise the ticket, then stamp approval_status='approved', approval_decided_by_id, timestamp, comment and ticket_id. On reject: the same minus the ticket.

catalogueCreateTicketFromSubmission() deliberately mirrors api/self-service/create_ticket.php — the correct portal path:

requester resolved by users.id (never an email string — that's the bug in WorkflowEngine::action_create_ticket());
company from the requester's own tenant_id, falling to NULL/Default exactly as the portal's new-ticket path does;
status Open, the install's default priority;
body built by catalogueSubmissionBodyHtml() — the answers as a fully-escaped table, so a customer's field values can't inject markup into the analyst's reading pane.

🔑 This is also where promote-to-ticket finally landed. form_submissions.ticket_id had been a reserved "not yet actioned" column that nothing wrote. The approve path is its first writer.

🧩 A form's own action lists (#95)

forms.submission_actions is one TEXT column holding JSON:

{ "submitted": [ {"type":"create_ticket","args":{…}} ], "approved": […], "rejected": […] }

Each list is {type, args} in the workflow engine's own action shape, because they are workflow actions — WorkflowEngine::runActionList() runs them through the same executeActionList() loop a stored workflow uses, so chaining ({{last.ticket_id}}), error isolation and the step log all behave identically. JSON-in-TEXT for the same reason workflows.actions is: no migration each time a new action kind lands.

Runs are written to workflow_executions with workflow_id NULL and the form's name in workflow_name (the column is nullable and carries a name snapshot exactly so a run can have no parent row). Without that, a form's automation would be the one kind nobody could audit — and "it didn't do anything and I can't see why" is the worst failure mode automation has.

Executed from FormsService::submitForm() and catalogueApprovalDecide(), never from an adapter, so the UI, the portal and REST API v1 behave identically without any of them knowing the feature exists.

🔑 NULL is not the same as `[]` — and that is what removes the migration

Stored	Means	Behaviour
`NULL`	never configured	pre-#95 behaviour: nothing on submit; an approval raises a ticket the hard-coded way
`[]`	somebody opened the panel and chose nothing	obeyed — an approval raises no ticket
a list	configured	the list runs instead of the hard-coded raise

This is the whole safety argument. Every form that existed before the upgrade has NULL and keeps doing exactly what it did, so no data migration seeds anything and nothing can be half-migrated. The alternative — seeding every gated form with a create_ticket action — had two failure modes (a form missed by the seed silently stops raising tickets; a form seeded and still running the hard-coded path raises two) and this design has neither.

⚠️ A configured approved list raises its ticket AFTER the commit, not inside the decision transaction like the hard-coded path. An action list can send email, and a rolled-back email is still an email somebody received. The submission's ticket_id is stamped afterwards from the create_ticket step's result — that column is what the requester's Your requests panel reads, so a request showing no ticket after one was raised would look like a failure.

⚠️ A gated submission runs no submitted list, for the same reason it fires a different event: raising a ticket at submission time steps straight over the gate.

⚠️ submission_actions is written by createVersion(). Leaving it out would repeat the exact fault below — a per-form setting that Save deletes — except this time the form would quietly stop raising tickets.

⚠️ Saving validates every type against WorkflowEngine::availableActions(). An unknown type stored here would fail at run time inside a form submission, where the person who typed it will never see the error.

⚠️ Three faults this shipped with — fixed under discussion #95

All three were found while working out where form-submission actions should live. None had been reported: each one fails silently, which is what made them survive.

1. Editing a gated form removed its gate

FormsService::createVersion() copies the leaf into a new row, and its INSERT named title, description, is_active, is_portal_visible — not requires_approval or approver_id. requires_approval defaults to 0, so the new version came out ungated.

The catalogue lists leaves, so the ungated copy is the one the portal offers from that moment. Two behaviours changed at once and neither announced itself: sign-off stopped being required, and — because the approve path is the only thing that raises the ticket — requests stopped becoming tickets at all.

🔑 The general shape: a per-form setting that does not travel through createVersion() is a setting that Save deletes. The fix carries both columns; approver_id is passed as NULL rather than 0, because "nobody assigned" is a real state the gate reads as unconfigured.

⚠️ Not retrospectively repairable. A form edited before the fix lost the value at the moment of editing; there is nothing to recover it from. Any install that used approvals needs its forms checked by hand.

2. All three approval events fired into nothing

catalogueApprovalDecide() dispatches catalogue_request.approved / .rejected, and FormsService::submitForm() dispatches catalogue_request.submitted for a gated request. None of the three was in WorkflowEngine::availableTriggers().

dispatch() matches on trigger_event, and the editor's trigger dropdown is built from availableTriggers() — so no workflow could ever hold one of those values, and every dispatch matched zero rows. The code comments describe these as the hook to notify the approver and to tell the requester the outcome. Neither could happen; an approver had to think to go and look.

🔑 A dispatch() call is only half a trigger. The catalogue entry is the other half, and nothing links them — no error, no warning, no failing test. Adding a dispatch() without the availableTriggers() line produces an event that exists and can never be heard.

⚠️ The event is catalogue_request.**rejected**, not .declined. The name is built as 'catalogue_request.' . $decision, and $decision is validated against ['approved','rejected'] — the same word the column, the API contract and the UI button all use.

3. The decision events couldn't see the answers

The decide dispatch carried form and request (comment, ticket id, ticket number) but no submission. So {{submission.fields.Device type}} resolved to nothing on approve/reject while working fine on form.submitted — the same merge code meaning two different things depending on when it ran.

catalogueSubmissionAnswerMap() now supplies the label-keyed map plus the first email answer, and the four form-shaped triggers share one SUBMISSION_FIELD_TRIGGERS const so the merge-code hint and the editor's unknown-variable warning can't drift apart.

⚠️ catalogueSubmissionAnswerMap() and catalogueSubmissionAnswers() are not duplicates and must not be merged. The map is keyed by label because a merge code addresses an answer by name; the list is ordered because the approval card renders every row. A map cannot represent two fields sharing a label — the second would silently swallow the first — so the card keeps its list.

⚠️ Values in the map go through catalogueAnswerText(), so a checkbox reads Yes/No. form.submitted still flattens raw (1/0). Unifying them changes what already-built rules receive, so it is a deliberate decision rather than a tidy-up.

5. 🖥️ The three surfaces

Config (forms/index.php): the shield button opens a self-contained modal (#approvalModal). Active analysts are fetched once from api/tickets/get_analysts.php. Save posts only {id, requires_approval, approver_id} — a partial update, the same contract as the portal togglePortal, so nothing else about the form is touched.
Approver inbox (forms/approvals.php): cloned from change-management/approvals.php — filter sidebar (mine/all/decided) + card list, each card showing the answers and Approve/Reject with an optional note. showToast comes from the shared header, so the page doesn't load toast.js itself.
Requester view (self-service/index.php, #929): a Your requests section, hidden until the dashboard payload carries any. api/self-service/get_dashboard.php reads them in a guarded query — a pre-upgrade instance without the approval columns degrades to no requests rather than a broken dashboard.

6. ✅ How this was verified

php -l across every changed file.
Engine end-to-end against the dev DB: seed a pending request → catalogueApprovalsList shows it → catalogueApprovalDecide(approve) raises a ticket, stamps ticket_id, sets approved; requester (user_id), company and the answers-in-body all correct; re-approve and wrong-approver both refused with their specific messages.
Both analyst pages rendered with a forged analyst session (HTTP 200, no PHP errors), inline JS parsed clean in headless Chrome.
#929: get_dashboard.php returns the three states (pending / approved+ticket_number / rejected) via a forged portal session; the dashboard renders clean.

7. Extending it

Manager-based routing — the headline next step. Needs users.manager_id (or equivalent) and a way to populate it (LDAP/SSO/manual), then gate on the requester's manager instead of a fixed analyst. That relationship is its own mini-project.
A multi-approver board — lift Change Management's voteCab (enum + membership check + double-vote block + all/majority threshold) into a catalogue_request_approvers table.
Show the rejection reason to the requester — the note is stored (approval_comment); surfacing it on the portal is a product decision (flip the approver's note from internal to requester-visible).
Attachments — forms have no file fields today; if they gain uploads, carry them onto the ticket in catalogueCreateTicketFromSubmission() (the pattern is in create_ticket.php).
A "raise ticket" button for NON-gated submissions — forms/submissions.php still has none; the approve path only covers gated ones.