Skip to content

Preference Template Coverage PR Review

Ed Mozley edited this page Nov 11, 2026 · 1 revision

Two preferences missing from templates - PR review

PR #299 by Dr4inder Β· Verdict: merge + fix

Two settings on the Preferences page could not be put in a preference template: Empty work sections on a task (tasks_collapse_empty_work) and My company for new tickets (tickets_new_ticket_company). The PR adds them as two sections of PREF_TEMPLATE_SECTIONS in includes/preference_templates.php - work-fold and new-ticket-company, each in the place the Preferences page shows it. Developer notes: Preference templates Β§ For developers.

Why it was worth having

Templates are meant to hold every Preferences section except the personal ones. These two were added to the Preferences page later and never added here - the exact gap the TRAP in preference_templates.php warns about. The guard test caught it: tests/preference-templates.php fails when a key the Preferences page saves is in neither PREF_TEMPLATE_SECTIONS nor PREF_TEMPLATE_PERSONAL, so it had been a known failing test until this PR.

What was checked

Question How Result
Are they template material, or personal? Read both settings Both are layout/behaviour choices a team lead would want new starters to share, not personal data - a section each, not PREF_TEMPLATE_PERSONAL
Right headings and anchors? Compared with the Preferences page system.preferences.work_fold_heading and nt_company_heading, ids matching the page's section anchors
Does the test cover them? Read the test diff Extended: captured, stored, loaded, applied, given to a new analyst by default, and set / empty / unset each round-trip
Does the test run safely? Ran it on the test install 31/31. The PR also added a guard so the test only writes to the test database - see below

What was changed

  • #2808 - the test's new database guard exited 1 when FREEITSM_TEST_DATABASE was missing or did not match the connected database. That turned a suite run with no test database into a red run, when nothing had failed. It now prints SKIPPED: ... and exits 0, as every other writing test does (tests/README.md). The guard itself is kept - it still refuses to write anywhere but the throwaway database.

No strings were needed: both headings already existed in all six complete languages.

Merged with git merge --no-ff under the contributor's name (c547384a1), then the follow-up commit (0910e925e).

Worth knowing

  • The trap is still there for the next setting. A new Preferences-page setting is not in templates until its key is added to PREF_TEMPLATE_SECTIONS (or PREF_TEMPLATE_PERSONAL). The test is what catches it - keep it in the run before a release.
  • Skip, not fail. A writing test with no test database must skip with exit 0. Do not change the guard back to exit(1).

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Responses to feedback

Blue sky thinking

Bugs resolved

Pull requests

Links

Clone this wiki locally