Files
NextChair/docs/phase-8-email-sender-summary.md
T
2026-08-04 09:57:58 +01:00

14 KiB

Phase 8 Follow-Up — Business Name as Email Sender: Summary

A focused fix on top of Phase 8: every outgoing email's visible sender name now comes from the installed business's own name, not EMAIL_FROM_NAME or the internal product name. The sender email address stays exactly as it was — environment-configured, since it has to match whatever address/domain is actually verified with the SMTP relay.


Where the business name is resolved

In src/services/notifications/render-email.ts's renderEmailForNotification() — the one function that turns a stored Notification row into an actual email, already responsible for loading the business (for the existing name/logo/address/policy fields every template uses). It now also does:

const business = await prisma.business.findUnique({ where: { id: notification.businessId }, include: {...} });
const sender: EmailSender = { name: resolveSenderName(business.name) };

notification.businessId is a required, non-nullable column (prisma/schema.prisma) — every Notification in this schema is already business-scoped, so this is a findUnique on the notification's own explicit relationship, never a global findFirst(). Verified directly with an integration test that creates two businesses and confirms a notification scoped to business A never resolves business B's name, even though B exists in the same database (sender-identity.integration.test.ts, "never lets one business's notification resolve another business's name").

This lookup happens fresh, every time — renderEmailForNotification is called by the background processor right before a PENDING delivery is actually sent (reminder-worker.ts), never at notification-creation time. That was already true before this change (see that file's own header, written in Phase 7); this change just rides the same guarantee for the sender name. No caching beyond the implicit request-scoped Prisma round trip — nothing is cached across requests or worker runs, so a business rename takes effect on the very next email sent, with no restart and no environment-variable change. Verified directly: sender-identity.integration.test.ts's "uses the updated business name for the very next email after a rename" test sends one email, renames the business via a plain prisma.business.update() (the same call BusinessService.updateBusinessProfile makes from Owner Settings), sends a second email in the same process, and asserts the second one uses the new name.

How it reaches the Brevo provider

renderEmailForNotification() now returns RenderedEmailWithSender ({ subject, html, text, sender }) instead of the old RenderedEmail. reminder-worker.ts's processEmailDelivery() — the only call site of EmailProvider.sendEmail() in the entire codebase (confirmed by grep before making this change) — passes sender: email.sender straight through:

const email = await renderEmailForNotification(prisma, delivery.notification);
const sendResult = await emailProvider.sendEmail({
  to: { email: recipientEmail },
  sender: email.sender,
  subject: email.subject,
  html: email.html,
  text: email.text,
  idempotencyKey: delivery.id,
});

SendEmailParams (src/services/notifications/providers/email-provider.ts) gained a required sender: EmailSender field ({ name: string; email?: string }). BrevoEmailProvider.sendEmail() uses params.sender.name for the SMTP from display name, and always its own this.fromAddress (from BrevoEmailProviderConfig, env-configured) for the address — params.sender.email is accepted in the shape for forward-compatibility with a future provider that might need it, but Brevo's own implementation ignores it, documented inline and covered by a test (brevo-email-provider.test.ts, "sends with the per-message sender name and the provider's own configured address, ignoring any sender.email"). BrevoEmailProviderConfig itself dropped fromName entirely — it now only carries host/port/username/password/fromAddress, i.e. only static, non-business-derived provider settings, per the architecture this was explicitly asked to follow. loadBrevoEmailProviderConfigFromEnv() no longer reads EMAIL_FROM_NAME at all — that env var is read exactly once, inside resolveSenderName() (see below), not at provider-construction time.

ConsoleEmailProvider (the dev default) also prints the resolved sender name (From: {name}) so local development shows exactly what would be sent — this is what the manual verification below used.

The sender-name fallback order

src/services/notifications/sender-identity.ts — a small, pure, DB-free function:

export function resolveSenderName(businessName: string | null | undefined): string {
  const trimmedBusinessName = businessName?.trim();
  if (trimmedBusinessName) return trimmedBusinessName;

  const envFallback = process.env.EMAIL_FROM_NAME?.trim();
  if (envFallback) return envFallback;

  return "Booking Notifications";
}
  1. The installed business's own name — the normal, always-taken path for a real installed application (every Notification is business-scoped, and Business.name is a required, non-empty column populated by the installer/seed).
  2. EMAIL_FROM_NAME, only reached if tier 1 somehow resolves to nothing (a blank business name, or, in principle, a future notification type with no business association at all — no such type exists in this codebase today). Never required to be kept in sync with the business name.
  3. "Booking Notifications" — a neutral, non-product name, reached only if neither of the above is set. NextChair never appears anywhere in this function.

Unit-tested directly (sender-identity.test.ts): business name wins even when EMAIL_FROM_NAME is also set; EMAIL_FROM_NAME is used only when the business name is null/undefined/empty/whitespace; the neutral fallback is used when neither is available; and a regression guard asserting the function never returns "NextChair" at any tier.

Why the sender email stays environment-configured

Brevo (and SMTP relays generally) require the sending address to belong to a domain/address actually verified with the account — an arbitrary per-message address would either be silently rewritten by the relay or rejected outright. The business name has no such constraint (any string is a valid display name), so it was safe to make per-message and business-derived; the address was not, and stays exactly where it was before this change — a single environment variable (EMAIL_FROM_ADDRESS) loaded once into BrevoEmailProviderConfig. .env.example and .env.local.example were updated to make this explicit for EMAIL_FROM_NAME specifically (now documented as "optional... only used when no business name can be resolved... you do not need to set or keep this in sync with Business Settings"); EMAIL_FROM_ADDRESS's own existing documentation was untouched, since its behaviour didn't change.

Delivery-record snapshotting

Not snapshotted. NotificationDelivery gained no new column for this. This follows the existing, explicit design philosophy stated in render-email.ts's own header (written in Phase 7, unchanged by this follow-up): never store rendered content on the row, always re-derive it from current data at send time. Adding a senderNameUsed column would cut against that stated design for a "where useful" (the brief's own wording — optional) requirement, and — because Business itself has no history/versioning table — a snapshot column would only be reliable going forward, not retroactively reconstructable for anything sent before it existed. providerMessageId/providerName already exist on NotificationDelivery for send-forensics; reconstructing "what name did we actually show" for a historical delivery would require resolving what the business's name was at sentAt, which this app cannot do today (documented here as a known limitation, not fixed, since it wasn't asked for and would need a real audit/versioning mechanism to do properly).

Background notification processing

No change was needed to processDueNotificationDeliveries's own structure — it already calls renderEmailForNotification() fresh for every delivery it processes, so the "resolve current business identity when rendering/sending, don't assume creation-time branding is still current" requirement was already satisfied by the existing Phase 7 architecture. This follow-up only added the sender-name resolution to that same, already-correct call. Verified directly: sender-identity.integration.test.ts's "passes the resolved business name all the way through the background processor to the email provider" test goes through the real processDueNotificationDeliveries() entry point (not just renderEmailForNotification() directly) and asserts the MockEmailProvider captured the correct sender name.

Email types covered

Every email type is covered by construction, not type-by-type special-casing: renderEmailForNotification() is the only path to an outgoing email (confirmed: EmailProvider.sendEmail() has exactly one call site, reminder-worker.ts), and it resolves and attaches the sender identically regardless of which NotificationType it's rendering or which template (a specific booking/loyalty template, or the generic fallback template) ends up producing the subject/html/text. This includes STAFF_INVITATION_RECEIVED (currently rendered via the generic fallback template — the specialized renderStaffInvitationEmail/renderStaffScheduleChangedEmail functions in templates/staff-emails.ts are not wired into renderEmailForNotification's dispatch switch, a pre-existing gap from Phase 7 left untouched here since it's out of this follow-up's scope), which is why a dedicated integration test exists for it specifically (sender-identity.integration.test.ts, "resolves the business name for a staff-invitation notification").

Tests added

Unit (src/services/notifications/__tests__/sender-identity.test.ts, 6 tests): business name used when present; business name wins over EMAIL_FROM_NAME; EMAIL_FROM_NAME used only when no business name; neutral fallback used when neither is set (including whitespace-only business names); NextChair never returned at any tier.

Unit, updated (brevo-email-provider.test.ts): existing tests updated for the new sender field; one new test asserts the provider uses the per-message sender name and its own configured address, explicitly ignoring sender.email.

Integration, real MySQL (src/services/notifications/__tests__/sender-identity.integration.test.ts, 5 tests): a booking/loyalty-style notification resolves the installed business's name; a staff-invitation notification resolves it too (via the generic template); the full background-processor path (processDueNotificationDeliveries) passes the resolved name through to the email provider; renaming the business mid-process changes the very next email's sender with no restart; one business's notification never resolves another business's name even when both exist in the same database.

Full regression run after these changes: 236 unit tests (was 230), 163 integration tests (was 158) — all passing, zero new failures.

Manual verification

Ran directly against the real seeded local MySQL database (not a scripted mock), using the actual ConsoleEmailProvider the app selects when EMAIL_PROVIDER=console (this environment's local .env setting) — deliberately with .env's EMAIL_FROM_NAME left at its old, stale, misconfigured value ("NextChair") specifically to prove that value is no longer used as the primary source:

  • Every real, seeded notification (loyalty stamp, staff new-appointment, owner new-booking) rendered and "sent" through the actual processDueNotificationDeliveries → renderEmailForNotification → ConsoleEmailProvider pipeline showed From: Blake Heritage Barbershop — never From: NextChair, despite the misconfigured env var.
  • Renamed the business directly via prisma.business.update() (the same underlying call Owner Settings' "Business Details" form makes) to "Verify Rename Barbershop", in the same running process, no restart.
  • The very next notification sent immediately afterward showed From: Verify Rename Barbershop — confirming no caching, no restart required, no environment-variable change required.
  • Reverted the business name back to its original value afterward, leaving the seeded database as found.

Not independently re-verified this specific change through a full Docker Compose install cycle: docker-compose.local.yaml/Dockerfile were not touched by this follow-up, the email-sending code path is identical in a container (same source, same ConsoleEmailProvider/BrevoEmailProvider classes), and Phase 8's own summary already recorded a full, real Docker Compose lifecycle verification (build → empty volume → install → seed → restart-persists → teardown). Re-running that entire cycle here would not exercise anything Docker-specific about this particular change; the sender-name behaviour itself was verified against the real database and real (non-mocked, for the "would send" log) code path instead, which is the part that's actually new.

Not verified (unchanged from Phase 8's own "Unverified external-provider behaviour" section, still true here): a real Brevo SMTP send with the new sender-name behaviour — no live Brevo credentials were available in this environment. BrevoEmailProvider's exact nodemailer.sendMail({ from: { address, name } }) call is unit-tested (mocked transporter, asserting the correct from object is constructed), which is the same level of provider-boundary confidence this whole project has used for Brevo throughout every prior phase.