Say who sent a request form, to whom, and how to get help (PMS-748) #505

Merged
longjacksonle merged 3 commits from feat/PMS-748-request-form-email-polish into main 2026-08-09 19:34:49 +02:00

PMS-748. A client received a request for personal details from an organisation name alone, closing with "if you were not expecting this, you can ignore this message" - a sentence that gives a recipient nothing to check. Nobody was named, the client company was never mentioned, there was no way to ask a question, and no way to complain. The page the link opened was equally anonymous.

The SPA half (the builder field and the attribution on the client's page) is mokosh-apps #499. This PR is the data, the copy and the config.

What the email says now

It names the person who issued the link, the MSP they work for, the form, and the client company it was intended for, and it calls the thing a "request form" throughout instead of alternating with "form". The sender falls back to the same email-derived name the JIT user insert uses, rather than growing a second copy of the rules that already know not to derive a name from a UUID local part.

The contact line is never empty. With contact details it offers them; without them it names the MSP who sent it. It deliberately does not say "reply to this email", because the from-address is a noreply on every deployed environment and a line that promises a channel the deployment does not have is worse than the one it replaces.

Report abuse

An abuse report that reaches the sender is not an abuse channel, so the address is a per-deployment ABUSE_CONTACT_EMAIL rather than anything per tenant. Unset by default, and unset means no line at all: a report-abuse link pointing at nothing, or at the noreply from-address, is worse than no link.

That collides with the template renderer, which is flat {{key}} substitution with no conditionals and which ships an unresolved key to the client as literal braces (the MAPPS-425 defect). So the notice is composed server-side into two keys, a text flavour and an HTML flavour that carries its own paragraph wrapper. Every other new key is supplied unconditionally too, and the ones that can be empty are empty strings rather than absent.

Contact details on a form

New contact_info column on form_definitions. Free text, because what a client should be told to do varies and the value is rendered as a sentence rather than parsed. Per definition rather than per tenant because different request types route to different desks. A tenant-level default that a form can override is the obvious follow-up and is deliberately not built here.

Migration 102

Migrations are immutable once applied, so the seeded template from 101 is rewritten by a new file rather than edited in place. The UPDATE matches the seeded body verbatim, so a tenant that has customised its own copy through the notification CRUD API keeps it. Migration 101's INSERT took the same care in the other direction.

One unrelated fix carried here

The request-form integration test still asserted First name: Dana, the plain-newline description format that PMS-747 (#504) replaced with a Markdown list. Unit tests covered the new shape but that assertion was not updated with them, so #504 merged red on the Postgres-backed suite. Fixed in its own commit at the head of this branch.

Testing

cargo fmt --all --check, cargo clippy --all-targets, cargo test --lib, and the full just test-integration suite. New unit tests cover the contact line in both shapes, the abuse notice configured and unconfigured (including the percent-encoding of a form name carrying a space and an ampersand), and the sender-name fallback. Two new integration tests assert the queued email names the sender, the client, the contact line and the abuse address with no unresolved placeholder, and that the public form payload carries the MSP name. Those two also exercise migration 102 end to end, since the suite applies it.

concurrent_wrong_mfa_codes_all_count (tests/auth.rs) failed once under the full parallel run and passes alone. Unrelated to this change; flagged, not chased.

PMS-748. A client received a request for personal details from an organisation name alone, closing with "if you were not expecting this, you can ignore this message" - a sentence that gives a recipient nothing to check. Nobody was named, the client company was never mentioned, there was no way to ask a question, and no way to complain. The page the link opened was equally anonymous. The SPA half (the builder field and the attribution on the client's page) is mokosh-apps #499. This PR is the data, the copy and the config. ## What the email says now It names the person who issued the link, the MSP they work for, the form, and the client company it was intended for, and it calls the thing a "request form" throughout instead of alternating with "form". The sender falls back to the same email-derived name the JIT user insert uses, rather than growing a second copy of the rules that already know not to derive a name from a UUID local part. The contact line is never empty. With contact details it offers them; without them it names the MSP who sent it. It deliberately does not say "reply to this email", because the from-address is a noreply on every deployed environment and a line that promises a channel the deployment does not have is worse than the one it replaces. ## Report abuse An abuse report that reaches the sender is not an abuse channel, so the address is a per-deployment `ABUSE_CONTACT_EMAIL` rather than anything per tenant. Unset by default, and unset means no line at all: a report-abuse link pointing at nothing, or at the noreply from-address, is worse than no link. That collides with the template renderer, which is flat `{{key}}` substitution with no conditionals and which ships an unresolved key to the client as literal braces (the MAPPS-425 defect). So the notice is composed server-side into two keys, a text flavour and an HTML flavour that carries its own paragraph wrapper. Every other new key is supplied unconditionally too, and the ones that can be empty are empty strings rather than absent. ## Contact details on a form New `contact_info` column on `form_definitions`. Free text, because what a client should be told to do varies and the value is rendered as a sentence rather than parsed. Per definition rather than per tenant because different request types route to different desks. A tenant-level default that a form can override is the obvious follow-up and is deliberately not built here. ## Migration 102 Migrations are immutable once applied, so the seeded template from 101 is rewritten by a new file rather than edited in place. The UPDATE matches the seeded body verbatim, so a tenant that has customised its own copy through the notification CRUD API keeps it. Migration 101's INSERT took the same care in the other direction. ## One unrelated fix carried here The request-form integration test still asserted `First name: Dana`, the plain-newline description format that PMS-747 (#504) replaced with a Markdown list. Unit tests covered the new shape but that assertion was not updated with them, so #504 merged red on the Postgres-backed suite. Fixed in its own commit at the head of this branch. ## Testing `cargo fmt --all --check`, `cargo clippy --all-targets`, `cargo test --lib`, and the full `just test-integration` suite. New unit tests cover the contact line in both shapes, the abuse notice configured and unconfigured (including the percent-encoding of a form name carrying a space and an ampersand), and the sender-name fallback. Two new integration tests assert the queued email names the sender, the client, the contact line and the abuse address with no unresolved placeholder, and that the public form payload carries the MSP name. Those two also exercise migration 102 end to end, since the suite applies it. `concurrent_wrong_mfa_codes_all_count` (tests/auth.rs) failed once under the full parallel run and passes alone. Unrelated to this change; flagged, not chased.
The submission integration test still asserted `First name: Dana`, the plain-newline format PMS-747 replaced with a Markdown list. The unit tests on `render_answers` covered the new shape, but this assertion was not updated with them, so the change merged red on the Postgres-backed suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E48b8YHWYwmaKjFBy5esXp
PMS-748 needs two facts the system did not hold. A form definition gains `contact_info`, free text shown to a client on the form page and in the email, optional because the MSP's NAME is shown either way and attribution is what matters. Per definition rather than per tenant because different request types route to different desks; a tenant-level default a form can override is the obvious follow-up and is deliberately not built here.

`ABUSE_CONTACT_EMAIL` is the address a client can report an unwanted request-form email to. Per deployment rather than per tenant, because an abuse report that reaches the sender is not an abuse channel. Unset by default, and unset means the email carries no such line at all: a report-abuse link pointing at nothing, or at the noreply from-address, is worse than no link. Both the authenticated and the public forms service are given it from one value, so the two cannot drift.

Migration 102 adds the column. Migrations are immutable once applied, so this is a new file rather than an edit to 100.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E48b8YHWYwmaKjFBy5esXp
fix(forms): say who sent the request form, to whom, and how to get help
All checks were successful
E2E / Playwright against staging (pull_request) Successful in 3m23s
Check / fmt + clippy + build + tests (pull_request) Successful in 3m24s
Integration / integration tests (pull_request) Successful in 6m8s
Create release / Gate (release-branch merges only) (pull_request) Successful in 3s
Create release / Create release from merged PR (pull_request) Has been skipped
60dcd2b5dd
PMS-748. A client received a request for personal details from an organisation name alone, closing with "if you were not expecting this, you can ignore this message" - a sentence that gives a recipient nothing to check. Nobody was named, the client company was never mentioned, there was no way to ask a question, and no way to complain.

The email now names the person who issued the link, the MSP they work for, the form, and the client company it was intended for, and it calls the thing a "request form" throughout rather than alternating with "form". The sender falls back to the same email-derived name the JIT user insert uses rather than growing a second copy of those rules.

The contact line is never empty: with contact details it offers them, without them it names the MSP who sent it. It promises no channel the deployment does not have, which is why it does not say "reply to this email" against a noreply from-address.

The abuse notice is composed server-side into a text flavour and an HTML flavour that carries its own paragraph wrapper, and both are empty strings when no address is configured. `render_template` is flat substitution with no conditionals, so a wrapper written into the template would leave an empty paragraph on every deployment without an abuse address, and an unsupplied key would ship literal braces to the client, which was the MAPPS-425 defect. Every new key is therefore always present, and the ones that can be empty are empty strings rather than absent.

Migration 102 rewrites the seeded template. It matches the seeded body verbatim, so a tenant that has customised its own copy through the notification CRUD API keeps it.

The client-facing form page now names the MSP too, plus the contact details when the definition carries them. It is reached from an email by someone with no account here, so it has to carry its own attribution rather than assume the message that linked to it is still open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E48b8YHWYwmaKjFBy5esXp
longjacksonle deleted branch feat/PMS-748-request-form-email-polish 2026-08-09 19:34:50 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
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!505
No description provided.