Use the organisation identity across every client-facing email (PMS-761) #513

Merged

longjacksonle merged 5 commits from feat/PMS-761-org-identity-across-email into main 2026-08-11 16:37:08 +02:00

Conversation 0 Commits 5 Files changed 12 +1000 -368

longjacksonle commented 2026-08-11 16:30:15 +02:00

Owner

Copy link

Closes PMS-761.

Most of that story had already shipped: the organisation record holds contact name, phone, email and logo (MAPPS-429, PMS-755), onboarding requires them (PMS-752), settings edits them (PMS-758), and the request-form email renders the lot (PMS-747, PMS-748). What was missing is the other half of its title: use it across the system.

Every other client-facing email was anonymous

Where	Goes to	What the client read
send_quote_ready (PMS-673)	the billing contact	"A quote is ready for your review and approval."
send_invoice_pay_now (PMS-711)	the billing contact	"An invoice is ready for payment."
send_note_email (PMS-15)	the ticket's contact	"A new update has been added to ticket TCK-1042:"

None named the MSP or offered a way to reach them, and From is the deployment-wide SMTP_FROM, so it did not identify the sender either. All three now open by naming the organisation and close with the contact line. The quote and invoice name it in the subject too, since those arrive cold and the subject is what a recipient decides to open on; the ticket-note subject is left alone because it is the thread key for a conversation the client is already in.

The composition helpers lived privately inside modules::forms::request_links, which is exactly why no other email could use them. They are now modules::tenants::identity::OrgIdentity, and the request-form path reads through it rather than keeping a copy.

And the ticket-note email was not being sent at all

Found while wiring the identity into it. ticket.note_added was seeded by migration 021 for the default tenant only, and unlike appointment/SLA (030), auth (097) and the request link (101) it never got a backfill nor a place in TenantService::copy_default_config. dispatch resolves rules by (tenant_id, event_type) and skips silently when there are none, so for every real tenant it fanned out to zero recipients while the note row was still stamped is_email_sent = TRUE. The UI has been reporting delivery of messages that were never sent.

The same shape, one step less severe, for forms.request_link: PMS-730 added its template to the copy list but not its rule, so any tenant created since has had the template sitting unreachable. Migration 104 backfills both, guarded the same way as 030 and 097, and the copy list gains both events.

Deliberately not done

• Security and account email stays unbranded, with a note in the trait saying so. send_new_login_location and send_login_approval_code are mokosh speaking to its own user about their account; a tenant's name and logo on "approve your sign-in" is what phishing looks like. A unit test asserts they carry no organisation identity.
• No logo on the three emails. All are plain-text sends; a logo needs an HTML alternative, which is a second body each and a second thing to keep in step. Name and contact are what an anonymous message is actually missing.
• No per-tenant From header. It is the right end state and it is not a code change: it needs a sending domain per tenant with SPF, DKIM and a bounce path, or the mail lands in spam.

Verification

just check clean (including the five script gates), 370 unit tests, and the full Postgres integration suite green. New coverage: the mailer bodies and subjects for both money emails, the security emails staying unbranded, OrgIdentity::load reading the caller's own tenant, a new tenant actually having a usable rule for both client-facing events, and the seeded ticket-note body referencing the keys the service supplies.

One duplication is left in place and now commented: the no-dispatcher fallback in send_note_email composes the same copy as migration 104's template. PMS-700 removed exactly this for the auth emails, but that fallback has no template to read. Change one and change the other.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB

Closes PMS-761. Most of that story had already shipped: the organisation record holds contact name, phone, email and logo (MAPPS-429, PMS-755), onboarding requires them (PMS-752), settings edits them (PMS-758), and the request-form email renders the lot (PMS-747, PMS-748). What was missing is the other half of its title: **use it across the system**. ## Every other client-facing email was anonymous | Where | Goes to | What the client read | |---|---|---| | `send_quote_ready` (PMS-673) | the billing contact | "A quote is ready for your review and approval." | | `send_invoice_pay_now` (PMS-711) | the billing contact | "An invoice is ready for payment." | | `send_note_email` (PMS-15) | the ticket's contact | "A new update has been added to ticket TCK-1042:" | None named the MSP or offered a way to reach them, and `From` is the deployment-wide `SMTP_FROM`, so it did not identify the sender either. All three now open by naming the organisation and close with the contact line. The quote and invoice name it in the subject too, since those arrive cold and the subject is what a recipient decides to open on; the ticket-note subject is left alone because it is the thread key for a conversation the client is already in. The composition helpers lived privately inside `modules::forms::request_links`, which is exactly why no other email could use them. They are now `modules::tenants::identity::OrgIdentity`, and the request-form path reads through it rather than keeping a copy. ## And the ticket-note email was not being sent at all Found while wiring the identity into it. `ticket.note_added` was seeded by migration 021 for the default tenant only, and unlike appointment/SLA (030), auth (097) and the request link (101) it never got a backfill nor a place in `TenantService::copy_default_config`. `dispatch` resolves rules by (tenant_id, event_type) and skips silently when there are none, so for every real tenant it fanned out to zero recipients while the note row was still stamped `is_email_sent = TRUE`. The UI has been reporting delivery of messages that were never sent. The same shape, one step less severe, for `forms.request_link`: PMS-730 added its template to the copy list but not its rule, so any tenant created since has had the template sitting unreachable. Migration 104 backfills both, guarded the same way as 030 and 097, and the copy list gains both events. ## Deliberately not done - **Security and account email stays unbranded**, with a note in the trait saying so. `send_new_login_location` and `send_login_approval_code` are mokosh speaking to its own user about their account; a tenant's name and logo on "approve your sign-in" is what phishing looks like. A unit test asserts they carry no organisation identity. - **No logo on the three emails.** All are plain-text sends; a logo needs an HTML alternative, which is a second body each and a second thing to keep in step. Name and contact are what an anonymous message is actually missing. - **No per-tenant `From` header.** It is the right end state and it is not a code change: it needs a sending domain per tenant with SPF, DKIM and a bounce path, or the mail lands in spam. ## Verification `just check` clean (including the five script gates), 370 unit tests, and the full Postgres integration suite green. New coverage: the mailer bodies and subjects for both money emails, the security emails staying unbranded, `OrgIdentity::load` reading the caller's own tenant, a new tenant actually having a usable rule for both client-facing events, and the seeded ticket-note body referencing the keys the service supplies. One duplication is left in place and now commented: the no-dispatcher fallback in `send_note_email` composes the same copy as migration 104's template. PMS-700 removed exactly this for the auth emails, but that fallback has no template to read. Change one and change the other. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB

longjacksonle added 5 commits 2026-08-11 16:30:15 +02:00

refactor(utils): give hand-composed email markup one escaping helper ... 6484c04bd7

`html_escape` and `urlencoded` were private to `modules::forms::request_links`, which is fine while one email composes markup in Rust and a problem the moment a second one does. `render_template` has no conditionals, so every element that must sometimes disappear is composed whole in Rust and handed over as a single key, and each of those call sites needs the same two escapes.

No behaviour change: the functions and their tests move as they are.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB

refactor(tenants): make the organisation identity a shared type ... b2b9a33d95

The organisation's name, contact person, phone, email and logo were assembled by private helpers inside `modules::forms::request_links`. That is why the request-form email is the only client-facing message mokosh sends that identifies the MSP: no other caller could reach the wording, so the quote, invoice and ticket-note emails said nothing about who was writing.

`OrgIdentity` moves the loader and the sentence composition into `modules::tenants::identity`, where any caller can use it. It normalises on the way in (trim, whitespace-only reads as unset) so a caller never has to decide whether `Some("")` means set, and keeps its fields private to hold that invariant. `contact_line` now takes the caller's opening question, because only the caller knows whether the message is about a request form, a quote or an invoice.

It is deliberately outside this module's `multi-tenant` gate: its callers are unconditional, and a single-tenant build still has an organisation with a name.

No behaviour change for the request-form email. Its wording, its per-form contact override and its logo block are the same code, reached through the shared type; the unit tests that covered those helpers move with them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB

feat(email): name the MSP on the quote and invoice emails ... 6d05085235

Both messages went to a client contact without saying who they were from. "A quote is ready for your review and approval" and "An invoice is ready for payment", with a portal link, a `From` header holding the deployment-wide `SMTP_FROM`, and nothing else. A client with several vendors had no way to tell which one was asking them to commit money, and an unattributed request for payment is the exact shape of invoice fraud.

Both now open by naming the organisation, name it in the subject too (the subject is what the recipient decides to open on, and these arrive cold), and close with the contact line `OrgIdentity` composes, so a client who wants to check before acting has someone to ask.

`SenderIdentity` is two plain strings rather than the `OrgIdentity` they come from: this module owns transport and body copy, and knows nothing about tenants. The message details move into `QuoteReady` / `InvoicePayNow` for the same reason the request-form email uses a struct: four strings in a row means a transposed pair emails the title as the total and nothing fails.

When the identity cannot be read the send is skipped and logged rather than degraded to the old anonymous body. The quote and the invoice are both already visible in the portal, so the message is not the only route to them, and an unsigned payment request is worse than a missing one.

The account-security emails are pointedly left alone, with a note saying so. They are mokosh speaking to its own user about their account, not an MSP speaking to a client; a tenant's name on "approve your sign-in" is what phishing looks like.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB

fix(tickets): send the public note email at all, and say who it is from ... b7fbe8ac8a

Two defects, either of which alone leaves the feature broken.

The body said "A new update has been added to ticket TCK-1042" and no more. The recipient is a client contact, so the message named neither the organisation writing to them nor a way to ask about it. It now opens by naming the organisation and closes with the contact line, from the same `OrgIdentity` the request-form email uses. The subject is deliberately unchanged: it is the thread key for a ticket the client already associates with this MSP, unlike a quote or an invoice, which arrive cold and are named in the subject line too.

The larger one: `ticket.note_added` was seeded by migration 021 for the default tenant only, and unlike appointment/SLA (030), auth (097) and the request link (101) it never got a backfill, nor a place in `TenantService::copy_default_config`. `dispatch` resolves rules by (tenant_id, event_type) and skips silently when a tenant has none, so for every real tenant this email fanned out to zero recipients while the note row was still stamped `is_email_sent = TRUE`. The UI has been reporting delivery of messages that were never sent.

Migration 104 re-words the source copy, then backfills the template and its rule for every tenant that lacks them, guarded the same way as 030 and 097. `copy_default_config` gains the event so tenants created from here on get it directly.

The same gap is fixed for `forms.request_link`: PMS-730 added its template to the copy list but not its rule, and `dispatch` iterates rules, so any tenant created since has had the template sitting unreachable and has sent no request-form email at all. Migration 104 covers the tenants already created that way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB

test(tenants): cover the identity loader and the rules that carry it ... f6b57330cb

Three checks, each for a way this can silently regress.

`OrgIdentity::load` reads through `begin_with_tenant`, and the identity in a client's email has to be the identity of the tenant that owns the thing the email is about. The test asserts a second tenant's contact line and that the default tenant reads as a different organisation, so threading the wrong tenant_id produces a visibly wrong email rather than a plausible one.

A newly created tenant needs an active rule pointing at its own template for `ticket.note_added` and `forms.request_link`, or the email is never sent. This is the check that would have caught both gaps: a template with no rule looks correct in the templates table and delivers nothing.

The seeded ticket-note body has to reference the keys the service supplies. `render_template` leaves an unresolved key as literal braces in the delivered message, so template and context are asserted against each other rather than trusted to stay in step.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X29MwLvt6mgSKrpqzh5zqB

longjacksonle merged commit 9247b8c773 into main 2026-08-11 16:37:08 +02:00

longjacksonle deleted branch feat/PMS-761-org-identity-across-email 2026-08-11 16:37:08 +02:00

longjacksonle referenced this pull request from a commit 2026-08-11 16:37:09 +02:00

Merge pull request 'Use the organisation identity across every client-facing email (PMS-761)' (#513) from feat/PMS-761-org-identity-across-email into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

psa-systems/mokosh-server!513

No description provided.