Say who sent a request form, to whom, and how to get help (PMS-748) #505
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/PMS-748-request-form-email-polish"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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_EMAILrather 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_infocolumn onform_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 fulljust test-integrationsuite. 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.