The password-reset email's HTML is a template literal inside backend/src/routes/customers.ts, so changing a word means a code change, a review and a deploy. Moving it into a Brevo template would let the content be edited where the rest of the sending already lives.
It is not a drop-in, and the reason matters
mailer.ts sends over SMTP via nodemailer. SMTP transmits the message it is handed — there is no way to tell an SMTP server "render template 4 with these parameters". Selecting a Brevo template means calling Brevo's HTTP transactional API (POST /v3/smtp/email with templateId and params) instead.
I have not verified whether Brevo's SMTP relay supports template selection through custom headers; some providers do. That is worth ten minutes of checking before designing anything, because if it does, this becomes a much smaller change that keeps nodemailer and the provider-agnostic shape.
Assuming it does not, the consequences are worth stating before agreeing to them:
The mailer stops being provider-agnostic. Today SMTP_HOST can point anywhere — Gmail, Brevo, a catcher, a colleague's relay. An HTTP-API send path is Brevo-specific, and swapping providers later means rewriting rather than reconfiguring.
The email content leaves the repository. It stops being reviewable in a diff, stops being version-controlled alongside the code that triggers it, and stops appearing in a git log when someone asks why the wording changed. Anyone with Brevo access can change what customers receive, with no review and no record here.
A template edit can break the email with nothing to catch it. If the template stops referencing the reset-link parameter, the send still succeeds and every recipient gets a link-less email. That failure is silent and on the customer's side.
What must not be lost
The #87 allowlist has to apply to the new path too.MAIL_ALLOWLIST is enforced inside sendMail. A second send path that calls Brevo's HTTP API directly would bypass it completely, and QA would be able to email real customers again — the exact thing #87 exists to prevent. Whatever shape this takes, the guard has to sit in front of both, not inside one.
Graceful degradation has to survive.sendMail skips with a warning when SMTP is unconfigured, which is what lets local development and the test suite run without credentials. An API-based sender needs the same behaviour for a missing API key, or every developer without one starts seeing failures.
Worth deciding
This email only, or all four? Verification, password reset, favorite-sold alerts and cart reminders are all inline HTML today. Moving one leaves the codebase with two ways of sending mail, which is worse than either consistently. Moving all four is a bigger change than this issue as written.
Does the template's parameter contract get tested? The reset link is the entire point of the message. Something should fail loudly if the template stops interpolating it — otherwise the first report will come from a customer who cannot reset their password.
Whether the tracking Brevo adds is wanted here. Brevo templates can carry open and click tracking. On a password-reset email that means logging when someone opened a security message, which is a different privacy question from tracking a marketing send. Related to #56.
Severity
Low. Nothing is broken and the current email works. This is a content-workflow improvement, and it should be weighed against giving up a provider-agnostic mailer and moving customer-facing copy out of code review.
The password-reset email's HTML is a template literal inside `backend/src/routes/customers.ts`, so changing a word means a code change, a review and a deploy. Moving it into a Brevo template would let the content be edited where the rest of the sending already lives.
## It is not a drop-in, and the reason matters
`mailer.ts` sends over **SMTP** via nodemailer. SMTP transmits the message it is handed — there is no way to tell an SMTP server "render template 4 with these parameters". Selecting a Brevo template means calling Brevo's HTTP transactional API (`POST /v3/smtp/email` with `templateId` and `params`) instead.
I have not verified whether Brevo's SMTP relay supports template selection through custom headers; some providers do. That is worth ten minutes of checking before designing anything, because if it does, this becomes a much smaller change that keeps nodemailer and the provider-agnostic shape.
Assuming it does not, the consequences are worth stating before agreeing to them:
- **The mailer stops being provider-agnostic.** Today `SMTP_HOST` can point anywhere — Gmail, Brevo, a catcher, a colleague's relay. An HTTP-API send path is Brevo-specific, and swapping providers later means rewriting rather than reconfiguring.
- **The email content leaves the repository.** It stops being reviewable in a diff, stops being version-controlled alongside the code that triggers it, and stops appearing in a `git log` when someone asks why the wording changed. Anyone with Brevo access can change what customers receive, with no review and no record here.
- **A template edit can break the email with nothing to catch it.** If the template stops referencing the reset-link parameter, the send still succeeds and every recipient gets a link-less email. That failure is silent and on the customer's side.
## What must not be lost
**The #87 allowlist has to apply to the new path too.** `MAIL_ALLOWLIST` is enforced inside `sendMail`. A second send path that calls Brevo's HTTP API directly would bypass it completely, and QA would be able to email real customers again — the exact thing #87 exists to prevent. Whatever shape this takes, the guard has to sit in front of both, not inside one.
**Graceful degradation has to survive.** `sendMail` skips with a warning when SMTP is unconfigured, which is what lets local development and the test suite run without credentials. An API-based sender needs the same behaviour for a missing API key, or every developer without one starts seeing failures.
## Worth deciding
- **This email only, or all four?** Verification, password reset, favorite-sold alerts and cart reminders are all inline HTML today. Moving one leaves the codebase with two ways of sending mail, which is worse than either consistently. Moving all four is a bigger change than this issue as written.
- **Does the template's parameter contract get tested?** The reset link is the entire point of the message. Something should fail loudly if the template stops interpolating it — otherwise the first report will come from a customer who cannot reset their password.
- **Whether the tracking Brevo adds is wanted here.** Brevo templates can carry open and click tracking. On a password-reset email that means logging when someone opened a security message, which is a different privacy question from tracking a marketing send. Related to #56.
## Severity
Low. Nothing is broken and the current email works. This is a content-workflow improvement, and it should be weighed against giving up a provider-agnostic mailer and moving customer-facing copy out of code review.
Better idea: an editable template in Admin → Settings, not a Brevo template
Proposed instead of the Brevo route, and it is the stronger option for this codebase. It answers the same need — change the wording without a code change and a deploy — while giving up none of what the Brevo approach would have cost:
The infrastructure is largely already here. admin_settings is a key/value table with a settled GET/PUT pattern in routes/adminSettings.ts, and the admin already edits rich content with MDEditor for item descriptions, with MarkdownView rendering it.
The one thing that makes this safe or dangerous
The reset link is the entire point of the email, and an admin editing free text can delete it. If the stored body loses its link placeholder, every password-reset email still sends, still looks fine in the log, and is useless to every recipient — and the first report comes from a customer who cannot get into their account.
So saving must refuse a body that does not contain the placeholder, naming what is missing. Not a warning, not a lint — a refused save. That single validation is the difference between this being a convenience and being a way to break password resets silently from a settings screen.
A stored body should also fall back to the built-in default when unset, so the feature degrades to exactly today's behaviour rather than to an empty email.
Open decisions
Markdown or HTML? Markdown matches how item descriptions are already edited and avoids handing an admin raw HTML, but the backend currently has no markdown renderer — MarkdownView is client-side React, and an email needs HTML generated server-side, which means a new backend dependency. Storing HTML avoids that and is uglier to edit. This is the main thing to settle.
Subject line too, or body only?
This email only, or all four? Verification, password reset, favorite alerts and cart reminders are all inline HTML today. One editable and three not is a worse state than either extreme.
A "send test email" button. This pairs unusually well with #87: the allowlist already guarantees a test send can only reach an approved address, so the button is safe by construction. It would let the wording be checked in a real client before customers see it — which is the same instinct as the preview panel in #89.
Status of the Brevo option
Not deleted from this issue, because it remains worth knowing that Brevo templates need the HTTP API rather than SMTP, and the ten-minute check on whether their relay supports template selection by header is still worth doing before anyone assumes otherwise. But the Settings card is the recommendation.
Retitling this issue to match.
## Better idea: an editable template in Admin → Settings, not a Brevo template
Proposed instead of the Brevo route, and it is the stronger option for this codebase. It answers the same need — change the wording without a code change and a deploy — while giving up none of what the Brevo approach would have cost:
| | Brevo template | Settings card |
| --- | --- | --- |
| Mailer stays provider-agnostic | ❌ becomes Brevo-specific | ✅ `sendMail` unchanged |
| Works with no Brevo credentials | ❌ | ✅ local dev and tests unaffected |
| #87's `MAIL_ALLOWLIST` still applies | ⚠️ needs care, easy to bypass | ✅ same `sendMail`, guard untouched |
| Content editable without deploy | ✅ | ✅ |
| Content under version control | ❌ | ❌ — same cost either way |
The infrastructure is largely already here. `admin_settings` is a key/value table with a settled GET/PUT pattern in `routes/adminSettings.ts`, and the admin already edits rich content with MDEditor for item descriptions, with `MarkdownView` rendering it.
## The one thing that makes this safe or dangerous
**The reset link is the entire point of the email, and an admin editing free text can delete it.** If the stored body loses its link placeholder, every password-reset email still sends, still looks fine in the log, and is useless to every recipient — and the first report comes from a customer who cannot get into their account.
So saving must **refuse** a body that does not contain the placeholder, naming what is missing. Not a warning, not a lint — a refused save. That single validation is the difference between this being a convenience and being a way to break password resets silently from a settings screen.
A stored body should also fall back to the built-in default when unset, so the feature degrades to exactly today's behaviour rather than to an empty email.
## Open decisions
- **Markdown or HTML?** Markdown matches how item descriptions are already edited and avoids handing an admin raw HTML, but the backend currently has no markdown renderer — `MarkdownView` is client-side React, and an email needs HTML generated server-side, which means a new backend dependency. Storing HTML avoids that and is uglier to edit. This is the main thing to settle.
- **Subject line too, or body only?**
- **This email only, or all four?** Verification, password reset, favorite alerts and cart reminders are all inline HTML today. One editable and three not is a worse state than either extreme.
- **A "send test email" button.** This pairs unusually well with #87: the allowlist already guarantees a test send can only reach an approved address, so the button is safe by construction. It would let the wording be checked in a real client before customers see it — which is the same instinct as the preview panel in #89.
## Status of the Brevo option
Not deleted from this issue, because it remains worth knowing that Brevo templates need the HTTP API rather than SMTP, and the ten-minute check on whether their relay supports template selection by header is still worth doing before anyone assumes otherwise. But the Settings card is the recommendation.
Retitling this issue to match.
bermudalamb
changed title from Move the password-reset email into a Brevo template instead of inline HTML to Make the password-reset email editable from Admin → Settings2026-08-21 12:23:35 -05:00
bermudalamb
self-assigned this 2026-08-21 12:41:25 -05:00
bermudalamb
added this to the Make the password-reset email editable from Admin project 2026-08-21 12:42:15 -05:00
Implemented on feature/92-editable-email-templates — two commits, not pushed
Branched on top of #106, which has since merged, so {{greeting}} uses a first name from the start rather than the formal whole name.
Five templates, not four. The favorite alerts have separate copy for sold and withdrawn, which the issue counted as one.
What landed
markdown-it with html: false — its default, and the reason for choosing it over marked. Raw HTML in a stored body is escaped rather than passed through, so editing copy cannot put script into a customer's inbox. That is stronger than sanitising output, because there is no output to sanitise.
Saving is refused when a body drops a placeholder it needs, naming all of them rather than the first, and the admin screen shows that message verbatim. Unset templates fall back to the built-in copy, so an install that never opens the screen behaves exactly as before.
The favorite-alert consent sentence is appended by the server and is not editable, as agreed.
The markdown consequence the design predicted
The cart reminder built <li> elements by hand. With HTML escaped those would have reached customers as literal angle brackets, so it emits a markdown list instead. Same for the greeting: one {{greeting}} placeholder rather than Hi {{firstName}},, which reads as "Hi ," for anyone who registered before first names were required.
Three things found by verifying rather than by reasoning
Five favorite-alert tests failed with no error and no mail. The cause was not the code. resetDb does not truncate admin_settings, so a subject of "Gone" stored by the new template tests survived into a later suite and changed the mail it was asserting on. Cleaning up inside the template tests would have fixed only that one pairing, so resetDb now clears stored templates for every suite — they are test data like any other, and one outliving its suite makes a failure surface somewhere unrelated. This is the fourth time cross-suite state has cost time here.
The withdrawal notification then failed on timing. Loading copy from the database made the sender async, and the removal path was fire-and-forget — so the response could beat the mail out of the door. Dispatch was previously synchronous even though the sends were not awaited; restored by awaiting it.
The end-to-end spec raced itself. Four tests editing one shared stored template under a fully-parallel runner: one asserted a template was unset while another had just saved it. That block runs serially now, which is the honest fix for tests that mutate shared server state — rather than loosening the assertions until they stop noticing.
One design choice worth flagging
The body editor is a textarea, not MDEditor. The markdown editor already used for item descriptions is a heavy dependency to pull into the settings screen for five short bodies, and its preview renders markdown as the browser shows it rather than as the email renderer will — a preview that quietly disagrees with the output is worse than no preview. Worth revisiting if the copy grows.
Verification
197 unit, 195 integration, 99 end-to-end on a freshly created container — the whole suite, not just the new spec, precisely because these tests write state other suites read. Lint unchanged at 4 backend and 27 frontend warnings.
Not done
The send-test-email button from the scoping comment. It pairs well with #87's allowlist and would let wording be checked in a real client, but it is a separate capability rather than part of making the copy editable. Say the word and it gets its own issue.
Not pushed, per the usual arrangement.
## Implemented on `feature/92-editable-email-templates` — two commits, not pushed
Branched on top of #106, which has since merged, so `{{greeting}}` uses a first name from the start rather than the formal whole name.
**Five templates, not four.** The favorite alerts have separate copy for sold and withdrawn, which the issue counted as one.
## What landed
`markdown-it` with `html: false` — its default, and the reason for choosing it over `marked`. Raw HTML in a stored body is escaped rather than passed through, so editing copy cannot put script into a customer's inbox. That is stronger than sanitising output, because there is no output to sanitise.
Saving is refused when a body drops a placeholder it needs, naming all of them rather than the first, and the admin screen shows that message verbatim. Unset templates fall back to the built-in copy, so an install that never opens the screen behaves exactly as before.
The favorite-alert consent sentence is appended by the server and is not editable, as agreed.
## The markdown consequence the design predicted
The cart reminder built `<li>` elements by hand. With HTML escaped those would have reached customers as literal angle brackets, so it emits a markdown list instead. Same for the greeting: one `{{greeting}}` placeholder rather than `Hi {{firstName}},`, which reads as "Hi ," for anyone who registered before first names were required.
## Three things found by verifying rather than by reasoning
**Five favorite-alert tests failed with no error and no mail.** The cause was not the code. `resetDb` does not truncate `admin_settings`, so a subject of `"Gone"` stored by the *new* template tests survived into a later suite and changed the mail it was asserting on. Cleaning up inside the template tests would have fixed only that one pairing, so `resetDb` now clears stored templates for every suite — they are test data like any other, and one outliving its suite makes a failure surface somewhere unrelated. This is the fourth time cross-suite state has cost time here.
**The withdrawal notification then failed on timing.** Loading copy from the database made the sender async, and the removal path was fire-and-forget — so the response could beat the mail out of the door. Dispatch was previously synchronous even though the sends were not awaited; restored by awaiting it.
**The end-to-end spec raced itself.** Four tests editing one shared stored template under a fully-parallel runner: one asserted a template was unset while another had just saved it. That block runs serially now, which is the honest fix for tests that mutate shared server state — rather than loosening the assertions until they stop noticing.
## One design choice worth flagging
The body editor is a **textarea, not MDEditor**. The markdown editor already used for item descriptions is a heavy dependency to pull into the settings screen for five short bodies, and its preview renders markdown as the browser shows it rather than as the email renderer will — a preview that quietly disagrees with the output is worse than no preview. Worth revisiting if the copy grows.
## Verification
**197 unit**, **195 integration**, **99 end-to-end** on a freshly created container — the whole suite, not just the new spec, precisely because these tests write state other suites read. Lint unchanged at 4 backend and 27 frontend warnings.
## Not done
The **send-test-email button** from the scoping comment. It pairs well with #87's allowlist and would let wording be checked in a real client, but it is a separate capability rather than part of making the copy editable. Say the word and it gets its own issue.
Not pushed, per the usual arrangement.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The password-reset email's HTML is a template literal inside
backend/src/routes/customers.ts, so changing a word means a code change, a review and a deploy. Moving it into a Brevo template would let the content be edited where the rest of the sending already lives.It is not a drop-in, and the reason matters
mailer.tssends over SMTP via nodemailer. SMTP transmits the message it is handed — there is no way to tell an SMTP server "render template 4 with these parameters". Selecting a Brevo template means calling Brevo's HTTP transactional API (POST /v3/smtp/emailwithtemplateIdandparams) instead.I have not verified whether Brevo's SMTP relay supports template selection through custom headers; some providers do. That is worth ten minutes of checking before designing anything, because if it does, this becomes a much smaller change that keeps nodemailer and the provider-agnostic shape.
Assuming it does not, the consequences are worth stating before agreeing to them:
SMTP_HOSTcan point anywhere — Gmail, Brevo, a catcher, a colleague's relay. An HTTP-API send path is Brevo-specific, and swapping providers later means rewriting rather than reconfiguring.git logwhen someone asks why the wording changed. Anyone with Brevo access can change what customers receive, with no review and no record here.What must not be lost
The #87 allowlist has to apply to the new path too.
MAIL_ALLOWLISTis enforced insidesendMail. A second send path that calls Brevo's HTTP API directly would bypass it completely, and QA would be able to email real customers again — the exact thing #87 exists to prevent. Whatever shape this takes, the guard has to sit in front of both, not inside one.Graceful degradation has to survive.
sendMailskips with a warning when SMTP is unconfigured, which is what lets local development and the test suite run without credentials. An API-based sender needs the same behaviour for a missing API key, or every developer without one starts seeing failures.Worth deciding
Severity
Low. Nothing is broken and the current email works. This is a content-workflow improvement, and it should be weighed against giving up a provider-agnostic mailer and moving customer-facing copy out of code review.
Better idea: an editable template in Admin → Settings, not a Brevo template
Proposed instead of the Brevo route, and it is the stronger option for this codebase. It answers the same need — change the wording without a code change and a deploy — while giving up none of what the Brevo approach would have cost:
sendMailunchangedMAIL_ALLOWLISTstill appliessendMail, guard untouchedThe infrastructure is largely already here.
admin_settingsis a key/value table with a settled GET/PUT pattern inroutes/adminSettings.ts, and the admin already edits rich content with MDEditor for item descriptions, withMarkdownViewrendering it.The one thing that makes this safe or dangerous
The reset link is the entire point of the email, and an admin editing free text can delete it. If the stored body loses its link placeholder, every password-reset email still sends, still looks fine in the log, and is useless to every recipient — and the first report comes from a customer who cannot get into their account.
So saving must refuse a body that does not contain the placeholder, naming what is missing. Not a warning, not a lint — a refused save. That single validation is the difference between this being a convenience and being a way to break password resets silently from a settings screen.
A stored body should also fall back to the built-in default when unset, so the feature degrades to exactly today's behaviour rather than to an empty email.
Open decisions
MarkdownViewis client-side React, and an email needs HTML generated server-side, which means a new backend dependency. Storing HTML avoids that and is uglier to edit. This is the main thing to settle.Status of the Brevo option
Not deleted from this issue, because it remains worth knowing that Brevo templates need the HTTP API rather than SMTP, and the ten-minute check on whether their relay supports template selection by header is still worth doing before anyone assumes otherwise. But the Settings card is the recommendation.
Retitling this issue to match.
Move the password-reset email into a Brevo template instead of inline HTMLto Make the password-reset email editable from Admin → SettingsImplemented on
feature/92-editable-email-templates— two commits, not pushedBranched on top of #106, which has since merged, so
{{greeting}}uses a first name from the start rather than the formal whole name.Five templates, not four. The favorite alerts have separate copy for sold and withdrawn, which the issue counted as one.
What landed
markdown-itwithhtml: false— its default, and the reason for choosing it overmarked. Raw HTML in a stored body is escaped rather than passed through, so editing copy cannot put script into a customer's inbox. That is stronger than sanitising output, because there is no output to sanitise.Saving is refused when a body drops a placeholder it needs, naming all of them rather than the first, and the admin screen shows that message verbatim. Unset templates fall back to the built-in copy, so an install that never opens the screen behaves exactly as before.
The favorite-alert consent sentence is appended by the server and is not editable, as agreed.
The markdown consequence the design predicted
The cart reminder built
<li>elements by hand. With HTML escaped those would have reached customers as literal angle brackets, so it emits a markdown list instead. Same for the greeting: one{{greeting}}placeholder rather thanHi {{firstName}},, which reads as "Hi ," for anyone who registered before first names were required.Three things found by verifying rather than by reasoning
Five favorite-alert tests failed with no error and no mail. The cause was not the code.
resetDbdoes not truncateadmin_settings, so a subject of"Gone"stored by the new template tests survived into a later suite and changed the mail it was asserting on. Cleaning up inside the template tests would have fixed only that one pairing, soresetDbnow clears stored templates for every suite — they are test data like any other, and one outliving its suite makes a failure surface somewhere unrelated. This is the fourth time cross-suite state has cost time here.The withdrawal notification then failed on timing. Loading copy from the database made the sender async, and the removal path was fire-and-forget — so the response could beat the mail out of the door. Dispatch was previously synchronous even though the sends were not awaited; restored by awaiting it.
The end-to-end spec raced itself. Four tests editing one shared stored template under a fully-parallel runner: one asserted a template was unset while another had just saved it. That block runs serially now, which is the honest fix for tests that mutate shared server state — rather than loosening the assertions until they stop noticing.
One design choice worth flagging
The body editor is a textarea, not MDEditor. The markdown editor already used for item descriptions is a heavy dependency to pull into the settings screen for five short bodies, and its preview renders markdown as the browser shows it rather than as the email renderer will — a preview that quietly disagrees with the output is worse than no preview. Worth revisiting if the copy grows.
Verification
197 unit, 195 integration, 99 end-to-end on a freshly created container — the whole suite, not just the new spec, precisely because these tests write state other suites read. Lint unchanged at 4 backend and 27 frontend warnings.
Not done
The send-test-email button from the scoping comment. It pairs well with #87's allowlist and would let wording be checked in a real client, but it is a separate capability rather than part of making the copy editable. Say the word and it gets its own issue.
Not pushed, per the usual arrangement.