Skip to content

refactor!: rename template field body_html to email_body_html (v1.2.0 breaking)#29

Merged
fxthiry merged 1 commit intorelease/1.2.0from
fix/rename-body-html-to-email-body-html
Apr 15, 2026
Merged

refactor!: rename template field body_html to email_body_html (v1.2.0 breaking)#29
fxthiry merged 1 commit intorelease/1.2.0from
fix/rename-body-html-to-email-body-html

Conversation

@fxthiry
Copy link
Copy Markdown
Owner

@fxthiry fxthiry commented Apr 15, 2026

Summary

Breaking rename of the template output field body_htmlemail_body_html. Only the email notifier has ever consumed this field; the old name suggested a generic HTML slot that directly misled the user in #26. Renaming removes the ambiguity.

Bundled with the fixes for #25 and #26 (already merged into release/1.2.0) to ship as v1.2.0.

Breaking change

Configs using body_html: are rejected at load. Thanks to the pre-existing #[serde(deny_unknown_fields)] on TemplateConfig, serde produces an actionable error:

unknown field `body_html`, expected one of `title`, `body`, `email_body_html`, `accent_color`

The expected-fields list names email_body_html directly, so valerter --validate points users at the migration on the first run. The migration applies equally to inline templates and split templates.d/ files.

No alias, no custom deserializer — the rename is intentionally nondeniable per project memory "no backward-compat shims when you can just change the code".

Semantic preservation

Email fallback unchanged: email_body_html.or(body). Telegram / Mattermost / webhook continue to ignore the field (behavior identical — it was always ignored, just less obvious).

Test plan

  • cargo test --lib — 444 passed (baseline 443 + 1 new regression test)
  • cargo test full suite — 498 passed, 22 ignored (SMTP MailHog + doctests)
  • cargo clippy --all-targets --all-features -- -D warnings — clean
  • cargo run --bin valerter -- --validate -c config/config.example.yaml — exit 0
  • Manual regression: YAML with body_html: rejected with error mentioning both body_html and email_body_html
  • Smoke-test live on release/1.2.0 rc build (reporter validation)

Review hints

Spec + suggested review order: _bmad-output/implementation-artifacts/spec-rename-body-html-to-email-body-html.md (local, not in VCS).

Key entry points:

  • src/config/types.rs:156TemplateConfig field rename
  • src/template.rs:38RenderedMessage field rename
  • src/notify/email.rs:409 — consumer logic (preserved semantics)
  • src/config/tests.rs:750 — new regression test for breaking rejection
  • CHANGELOG.md:8 — v1.2.0 entry

Three adversarial reviewers ran on the diff. One raised five false-positive findings that did not reproduce (fixture existence, deny_unknown_fields presence, test behavior — all verified green locally). Legitimate findings patched in the same PR: fixture file added to index, CHANGELOG Unreleased/[1.2.0] duplication resolved, docs/architecture.md:160 double-email heading fixed, CHANGELOG migration note now explicitly mentions templates.d/. Deferred items logged in deferred-work.md.

Only the email notifier consumes this field. Telegram, Mattermost, and
webhook have always ignored it. The old name suggested a generic HTML
output slot, which directly misled at least one user into putting
Telegram content in body_html and leaving body empty (issue #26).

Breaking change: configs using `body_html:` are rejected at load with
serde's "unknown field" error, which lists `email_body_html` among the
expected fields. Migration is a one-liner per template and applies
equally to inline templates and split `templates.d/` files.

No behavior change elsewhere. Email fallback preserved exactly:
`email_body_html.or(body)`. The runtime semantics are identical after
the rename — only the field identifier changed, at every call site.

CHANGELOG entry added for v1.2.0 bundling this breaking change with the
previously merged fixes for #25 (dotted fields) and #26 (empty telegram
guard + honest example).
@fxthiry fxthiry merged commit f2d4fa6 into release/1.2.0 Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant