Directory Maintenance PR Review

Directory sources and Imported people - PR review

PR #264 by vsc55 · Closes #261 and the Maintenance part of #263 · Builds on #291 (review) · Verdict: merge, with fixes · 5.3.0

Deleting an LDAP, CardDAV or Entra source used to leave its people flagged as imported with no source behind them - and with no source, every directory field was locked on them for good, with nothing ever updating them again. vsc55's design fixes it with one rule for every kind of source - a person is managed by a directory only while that source exists - and adds Imported people, the second tool in System → Maintenance, for the decisions nobody else will make: people whose source was deleted, people a source no longer brings in, and an import that should be undone. Before anything is deleted, a "what if" lists in full what would happen, as a PDF or emailed to yourself. It came as a design issue first, waited for my answers on #261 and #263, then rebased onto #291 once that was in. User guide: Imported people. Internals: Imported people - Developer Guide. The email half: Emailed reports (developer guide).

What was checked

Question	Result
Is the rule applied everywhere is_managed decides something?	Yes - userManagedSql() is read by the locked fields (Tickets → Users, the portal profile, Assets → Users), the delete refusal (#238), Starters, People and its gaps, and the merge preview (the last one added on merge)
Are people already orphaned freed without a migration?	Yes - the rule checks the source with EXISTS, so a dangling auth_provider_id stops counting the moment the code is in
Can a stale page or a typed id act on somebody outside the list?	No - apply() works the list out again on every write and acts only on ids still in it, in the analyst's companies
Can anybody with history be deleted?	No - tickets and equipment (userDeleteBlockedByHistory()) and, on merge, any other record that names them (userDeleteBlockedByRecords()) skip the person with the reason
Can Remove, or Delete on the stale list, run while the next import would bring everybody back?	No - refused while the source and its import are both switched on
Can an administrator limited to some companies change people outside them?	No - lists and previews are scoped; deleting a source whose people reach further is refused (added on merge)
Do the previews and the writes agree on who is affected?	Yes - previewSourceDelete() and previewRemove() read with the same WHERE as the writes they describe, and the test compares them
Does deleting a source leave anything dangling on a verified install (no foreign keys)?	Not now - sign-in links, group → team mappings and groups seen at sign-in are deleted by hand, in one transaction with the release (the transaction added on merge)
Do the endpoints leak exception text?	No - delete_user.php and delete_sso_provider.php return a fixed sentence and log the detail
Is every write logged?	Yes - system_logs, type directory_maintenance: who, which list, what was done to whom, what was skipped and why

On the test install: tests/directory-maintenance.php 89 checks (62 in the PR as submitted), always rolled back; vsc55 also reported tests/system-maintenance.php 28/28, tests/people-org.php 94/94, the System search suite 358/358 and tests/directory-managed-users.php 15/15 on a database verified against 5.2.0.

What was changed on merge

1. The System page and dialog standards: Imported people and its preview dialog use sys-page and sys-modal-xl, with the standard buttons - PDF, Email, Cancel, Delete.
2. Mark as leavers on the orphans list also releases them, so they leave the list and its counts clear; before, they stayed on it for good.
3. Delete on the orphans and stale lists removes only people an import created. Adopted people are kept (my rule on #261), and so is anybody nothing records either way. Delete on the stale list is refused while the source still imports, as Remove is, and skips anybody no import has recorded finding yet.
4. One way a person is deleted: userDeletePerson() in includes/users.php clears or NULLs, by hand, everything database/freeitsm.sql would cascade, because an install built by Database Verification has no foreign keys. The maintenance deletes and Tickets → Users both use it.
5. The bulk tools skip anybody another record still names - a starters and leavers case, a contract, a form submission (userDeleteBlockedByRecords()). Tickets → Users, deleting one person on purpose, does not ask this.
6. Deleting a source runs in one transaction, with the release first, and copes with an install that has no import-history tables.
7. Company scope on the delete: an administrator limited to some companies sees only their companies' people in the preview and cannot delete a source whose people reach further.
8. Analysts who sign in through the source are told a password must be set for them first; the analyst reading the preview is marked (you).
9. The merge preview reads userManagedSql(), like everything else.
10. Changelog ids renumbered to #2914-#2918 (the PR's #2889-#2893 had been taken on main since); the merge fixes are #2919.
11. Email for the "what if" report, built with attachments as I chose on #261 - ssSendSystemEmail() now carries files, and System → Emailed reports decides what the email holds (#2920, Emailed reports).
12. Translations of the new strings into the eight maintained languages; this write-up, the user page, the developer guide, the in-app help, a Feature Bingo card and vsc55's entry on the Contributors page.

Decided

• Adopted people are never deleted - not by Remove, not by Delete on any list. An import that adopted somebody found a person who was already here; removing them would delete somebody who existed before the import (Ed, on #261).
• The emailed report carries the file itself, not only a link: some installs are not reachable from the internet, so a link in an email opens nowhere there. What the email holds is a setting, attachment on by default (Ed, 2026-11-14).
• A download link is guarded and audited: it opens only for the analyst it was sent to, signed in, until it expires or is revoked, and every create, download, refusal and revoke is logged with IP address and browser (Ed, 2026-11-14).

Worth knowing

• system_logs has no index on log_type. DirectoryMaintenance::deletedSources() reads the directory_maintenance rows to find deleted sources' names and whom their imports created, so it scans the table. Fine at today's sizes; an index is the fix if a large install notices.
• The skip reasons are server-side English - Existed before the import - kept., Not in this list any more. and the rest come from includes/services/directory_maintenance.php and includes/users.php and are shown as they are, in every language.
• A source deleted before 5.3.0 has no record of whom its imports created; it is offered under Remove imported people only where its import history survived (a verified install keeps it, one made from database/freeitsm.sql cascades it away).