Organisation Chart PR Review

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.