Approvals Inbox Said Error

The approvals inbox said "Error" and nothing else

Reported with screenshots by a user · Fixed in #1850, #1851 · Released in 2.3.1

---

What you saw

Forms → Approvals showed the word Error where the list of waiting requests should be, on both pending tabs, for every analyst including administrators.

The part that made it confusing: the counts were right. The sidebar said 2 waiting for me, 2 pending in total, 0 decided by me - correct numbers, beside a list that would not load. And clicking Decided by me, the one tab with nothing in it, produced a perfectly normal empty state.

So the screen looked as though the whole list endpoint was broken, while simultaneously proving it was not. That is the wrong conclusion, and it is the one the screen pushed you towards.

It only happened once a request waiting for approval contained a table question - the field type added in 2.3.0. An install whose pending requests contained no table never saw it at all, which is why it could not be reproduced here until one was deliberately created.

---

What was actually wrong

One line, in one function that three different things call.

catalogueAnswerText() turns a stored answer into readable text. Most answers are a plain string; a checkboxes answer is stored as a JSON array, so the function decodes and flattens it:

$decoded = json_decode($val, true);          // checkboxes are stored as a JSON array
if (is_array($decoded)) $val = implode(', ', $decoded);

A table's answer is also a JSON array - but of objects, not strings. One entry per row, each a map of column id to cell value. implode() cannot render that. It produces the literal text Array, Array and emits a PHP warning for every row.

Why a warning broke the whole screen

The warning is the interesting half. On an install with display_errors on, PHP prints it into the response body, ahead of everything the page meant to send:

<br />
<b>Warning</b>:  Array to string conversion in .../includes/catalogue_approvals.php on line 380<br />
{"success":true,"items":[...]}

That is no longer JSON. The browser's res.json() threw, the page's catch block ran, and all it had to show was a hard-coded fallback: the word Error.

Why the counts survived

The page asks for one filter at a time, and every response carries all three counts. "Decided by me" had no rows to flatten, so that request produced no warning, returned clean JSON, and populated the counts. The two tabs that did have rows failed.

Correct counts beside a broken list is therefore not a contradiction - it is the fingerprint of a fault in rendering a row, not in fetching them. Worth remembering, because it points at exactly the opposite of where it looks.

It was never only the inbox

The same function renders two other things:

• the approval notification email sent to the approver
• the {{submission.fields.*}} merge codes available to workflow rules

Neither is JSON, so neither broke visibly. They simply said Array, Array where the table should have been - including on installs with display_errors off, where nothing appeared wrong at all. Those installs had the bug the whole time and no symptom to report.

🔑 The same mistake was already written up in the code. FormsService::submitForm() special-cases a grid before its own implode(), with a comment saying why: "A grid's rows are OBJECTS, so the generic implode below would render them as 'Array, Array' and emit a PHP warning." The note existed. This copy of the flattener never received it.

---

How it was fixed

A table now renders through FormsService::gridToText() - the renderer that already existed for exactly this - rather than through a third hand-written flattener. It produces one line per row with each cell labelled from the form's own column definitions, so the approver reads "Item: Pencil, Qty: 1" rather than a count or a shrug.

That needed the column definitions, which live in form_fields.config, so the three queries feeding this function now select it.

Two further changes, because the first one only fixes the case we know about:

• The generic branch can no longer emit that warning at all. Any non-scalar is JSON-encoded before implode() sees it. A field type nobody has taught this function yet will render awkwardly; it will not take out every endpoint that calls it.
• The endpoint catches Throwable, not Exception. A PHP Error - a TypeError, a call to something that is not there - is not an Exception, so it walked straight past the handler, and a fatal still answers HTTP 200. That is the other way this screen ends up with nothing to say.

And the screen now says something useful

Even with all of the above, the next unexpected failure would have shown the same bare Error. So the page now reads the response as text first and parses second, keeping the body when the parse fails, and shows a copyable diagnostic box: the screen and tab, the request, the HTTP status, the FreeITSM version, the browser, and the first 800 characters of what the server actually sent.

🔑 Reading the text first is the whole trick. res.json() throws away the body along with the parse, which is precisely why the original failure had nothing to report. The one piece of evidence that identifies the fault is destroyed by the convenience method.

The server's response is rendered with textContent, never innerHTML - a PHP fatal is an HTML page, and injecting one would turn a diagnostic into a second bug.

---

Files changed

File	What changed
includes/catalogue_approvals.php	a table renders through gridToText(); the generic branch is warning-proof; three queries select config
api/forms/catalogue_approvals.php	catches Throwable rather than Exception, and logs
forms/approvals.php	reads the response as text first; the diagnostic box
lang/en,es,de,pl,id/forms.php	strings for the box
tests/catalogue-approvals-grid.php	new

---

How it was proved

Not by reading the code - by building the conditions and watching it fail first.

• Reproduced deliberately: a form with a table question, a submission left pending approval, and the list endpoint called exactly as the page calls it. Two Array to string conversion warnings, and a response body that json_decode returns null for.
• The new test drives the real endpoint with a forged analyst session, and asserts the thing the bug failed: not "did it return items" but "is the body JSON at all", and that nothing precedes the opening brace.
• Run against the old code as a control it fails 7 of 14, with the body starting <br />.
• The diagnostic box was driven with the genuine failing response, lifted out of the page rather than retyped: nine assertions including that the server's <b> tags arrive as text and that a 5,000-character body is truncated with a count of what was cut.

---

See also

• Forms - what a table question is
• A table's answers were missing from the PDF - the same field type, a different reader, found the same week
• Bugs resolved