Skip to content

Change Impact Wording PR Review

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

Change impact and risk impact - PR review

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

A change has two fields with "impact" in their name, and the product described them as if they were linked. The Impacts settings page said impact bands "combine with priority to drive risk scoring". They never did. This PR corrects the settings text and puts a one-line explanation under each field in the change form. User page: Change Management. Risk scoring itself: Change risk policies.

Wording or code?

The first question on review was whether the wording was wrong or the code was. It was the wording:

  • Risk score and level come from the risk policy (includes/changes/risk_policy.php, a configurable matrix), from Risk likelihood x Risk impact (risk_likelihood, risk_impact_score). RiskPolicy::assess() takes those two numbers and nothing else.
  • Impact (impact_id, the Low / Medium / High lookup from Settings β†’ Impacts) is a classification of how widely the change reaches. It appears in the approval material-fields list (includes/changes/approvals.php - editing it can reset an approval round) and in templates (includes/changes/templates.php). It never reaches the risk score, and priority never did either.

So the PR changes text only, which is the right fix.

What was checked

Question How Result
Does Impact feed the risk score anywhere? Read RiskPolicy::assess(); grepped impact_id across api/change-management and includes/changes No. Only list/get joins, delete-in-use checks, approvals' material fields and templates
Does the new text match the code? Compared impact_help, risk_impact_help and impacts_scope_intro with the above Yes
Accessibility Diff of change-management/index.php Each help line has an id and its select points to it with aria-describedby; the Impact label gained for="changeImpact"
Were existing English strings changed? Diff of lang/en/change-management.php No. The settings page now shows the new key impacts_scope_intro; impacts_intro is kept
Does another module share the key? Grepped impacts_intro service-status has its own impacts_intro in its own namespace; untouched
Does anything else repeat the old claim? Grepped lang/ and the wiki The in-app help step on assessing risk said "pick Likelihood and Impact" - see below. The wiki's Change Management page already said Impacts do not feed the score

What was changed

  • #2807 - the three new strings in the five complete languages (de, el, es, id, pl).
  • The help step change-management ... rec_5 ("Assess risk") said pick Likelihood (1-5) and Impact (1-5), which pointed at the wrong Impact. It now names Risk likelihood and Risk impact, says the score comes from the risk policy, and says the Impact field near the top is separate - in all six languages.

Merged with git merge --no-ff under the contributor's name (035a842db), then the follow-up commit (cef35e8ba).

Worth knowing

  • Two fields, two jobs. Impact = how widely (one team, the whole organisation). Risk impact = how bad if it goes wrong. If a future change does want Impact to influence risk, that is a design change to the risk policy, and these three help texts would then be wrong - change them together.
  • impacts_intro is no longer shown in Change Management but is kept in every language file, so a translation is not orphaned while the old key is still referenced anywhere. Do not reuse it for new text.

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Responses to feedback

Blue sky thinking

Bugs resolved

Pull requests

Links

Clone this wiki locally