Organisation chart - PR review

PR #242 by vsc55 · Proposal #241 · Verdict: merge, with small fixes

A read-only feature built on data FreeITSM already held, with its visibility rule applied in one place and tested from both sides. It went in under its author's name; the fixes and the write-up followed in their own commit. User page: Organisation chart. Internals: Organisation chart - Developer Guide.

What was checked

Question	How	Result
Can it show somebody the reader may not see?	Read every query in includes/people.php; ran tests/people-org.php, which asks as a restricted analyst	No. Every node, count, path and gap goes through peopleScope() / analystCanAccessUser(), and the reports count applies the same rule, so it cannot leak a number either
Does "not found" give anything away?	Endpoint read; tests/people-org-api.php	Out of reach and not there answer the same
Can bad data hang it?	Read the walks; the test's looping line and long chain	Bounded at 32 levels with a visited set upwards, 150 boxes downwards
Does it write anything?	Endpoint read	No: GET only, no INSERT/UPDATE, so no CSRF surface
What does a URL do to it?	tests/people-org-api.php - levels padded, in hex, as a list, past the limit; id as a list	All held to a sensible value; no warnings, no exception text
Are names escaped on the page?	Read every innerHTML in people/org.php	All values through esc(), which escapes quotes; ids are numbers
Does it work?	The test install, desktop and phone width, with the people from the Entra review	Tree, gaps and the person link work; the phone layout stacks the sidebar above the tree

Tests on the test install: 89 (people-org.php) and 43 (people-org-api.php), all passing; the escape, module-access and web-exposure guards too.

What was changed

Change	Why
Changelog id #2701 → #2716	main had used #2701 for the background-job locks in the meantime
German and Spanish	The PR shipped English only; 120 strings, both locales at 100%
The Feature Bingo card people.managers now names the chart and links to it	It lights from the same data (somebody has a manager), so a second card would only duplicate it
README, user page, this page, the developer guide	The full write-up

No code was changed.

Worth knowing

• One file holds the page's JavaScript - about 1,000 lines inside people/org.php. It is self-contained and only loads on that page, so it was left as it is; if it grows, moving it to assets/js/people-org.js would let browsers cache it.
• Export draws from the data, not the screen. The author first used the bundled html2canvas and found it cannot parse modern CSS colours - a useful thing to know before anybody reaches for it elsewhere.
• The chart only reads. Reporting lines are still owned by the directory or by Tickets → Users, which is right: an edit made in the chart would be overwritten by the next import.

---

PR #265 - the script file, the wheel and the totals

PR #265, by vsc55, followed the review above: it moved the chart's script to assets/js/people-org.js as suggested there, and added three things on the way.

Verdict: merge, with small fixes. Merged as it was, so the commits stay vsc55's.

What was checked

Check	Result
The script moved unchanged	The old inline script and the new file were compared line by line. The only differences are the three server values now read from window.PEOPLE_ORG, the whole XML declaration, and the three features below - nothing else moved or changed
The new page	The four server values go through json_encode() with JSON_HEX_TAG, JSON_HEX_AMP, JSON_HEX_APOS, JSON_HEX_QUOT; every string in the Mouse and keyboard dialog goes through pplE(); the new file holds no PHP
peopleOrgTotals()	Scope and the leavers rule on both halves of the recursive query, a depth bound, COUNT(DISTINCT), placeholders in SQL order. WITH RECURSIVE needs MySQL 8, which is already the minimum and already used by Knowledge; users.manager_id is indexed
Tests	tests/people-org.php 94/94 and tests/people-org-api.php 43/43 on the test install
Live, on a small test organisation	A director showed 2 · 13 and the head of IT 2 · 8 (both correct), and the side panel said 2 report to them, 8 in all under them. A plain wheel turn left the zoom at 50% and the page free to scroll; Ctrl + wheel zoomed to 78% without zooming the page; Cmd + wheel zoomed back; switching to Compact kept 50%

What was changed

Change	Why
Changelog ids #2750-#2751 → #2756-#2757	main had used them for Knowledge in the meantime
German and Spanish	The PR shipped English only: 30 strings, and the rewritten help paragraph org.p2_v2, built from each language's own org.p2 with only the two changed sentences replaced, so the rest keeps its reviewed wording
This section, the user page and the developer guide	The write-up, done here for consistency (the author offered)

No code was changed.

Worth knowing

• A help paragraph rewritten after it was translated gets a new key (org.p2_v2), read in place of the old one through a small $rewritten map in people/help.php. That follows the rule that the English of a key never changes, so a translator can see which text is new.
• The wheel change is a behaviour change. Anyone used to zooming with the wheel alone will find it scrolls now; the help, the Mouse and keyboard dialog and the user page all say Ctrl (Cmd on a Mac).